Repository navigation
Add per model automatically_define_enum_traits option - #1598
neilvcarvalho merged 9 commits into
Conversation
|
Hey @mike-burns, this has been sat for a while. I'm just bumping it, as I'm wondering if there is any interest in solving the underlying issue or not? It's something that would be really useful for our 15 year old code base :-) |
f3e9265 to
ca52bc2
Compare
Currently you can only specify whether to automatically define enum traits at
a global level, through `FactoryBot.automatically_define_enum_traits`.
This means that an entire codebase has to either opt-in, or opt-out
from automatically defining enum traits. If you are in a large,
established codebase with lots of enum's, this is quite hard to change
globally when you find that automatically defining them doesn't fit for
your new use case.
If we could override this at a per-factory level, we could allow
individual factories to override the global setting where appropriate,
in order to do customise them where necessary.
Given `FactoryBot.automatically_define_enum_traits` being set to `true`,
and a model called `Task` with the following enum definition:
```
class Task
enum :confirmed_by, [:user, :organisation], prefix: true
end
```
You would be able to override disable this on a per-factory basis like so:
```
FactoryBot.define do
factory :task, automatically_define_enum_traits: false do
confirmed_by { :user }
trait :org_confirmed do
confirmed_by { :organisation }
end
end
end
```
If `FactoryBot.automatically_define_enum_traits` was instead set to
`false`, then the same model with a factory override set to `true` you
would end up with the following automatic traits:
```
FactoryBot.define do
factory :task, automatically_define_enum_traits: true do
# The :user and :organisation traits below would be automatically
# defined in the following way:
#
# trait :user do
# confirmed_by { :user }
# end
# trait :organisation do
# confirmed_by { :organisation }
# end
end
end
```
Fixes: thoughtbot#1597
Co-Authored-By: Julia Chan <julia.chan@freeagent.com>
ca52bc2 to
b25b722
Compare
|
Hey @neilvcarvalho and @vburzynski, I've updated this today in the hopes it might be considered for merging into upstream FactoryBot? Since I originally created it, we've found some more use cases for this internally. I'd be happy to iterate on this with any feedback too, if you have any. |
|
Hi @neilvcarvalho and @vburzynski, apologies for the ping, but we'd appreciate your thoughts on this PR, whenever you have some time. It would be really handy to be able to define enums per model. Having it as a global setting overcomplicates linting, but turning it off for everything would bloat factories that already rely on automatically defined enums. |
The fallback from the per-factory option to the global setting was an inline if/else assigning a local that shadowed the option name. Moving it into a private predicate keeps automatically_register_defined_enums? to one line and gives the fallback a single place to change.
Any truthy value enabled enum traits, so passing a string such as "false" (for example, read from configuration) silently turned the feature on. Factory now raises an ArgumentError unless the option is true, false, or omitted.
A child factory, nested or declared with parent:, started with no setting of its own and fell back to the global one. An opt-out on the parent was therefore undone in the child, which regenerated the enum traits the parent suppressed. Opt-in already reached children because the parent's enum traits are copied by inherit_parent_traits, so the two directions behaved differently. Factory#compile now passes the parent's resolved setting down when the child does not set one, before compiling the child's definition.
Each example called Task.reset_column_information as its last line, so a failing expectation skipped the reset. A single after hook runs it regardless of the outcome and removes the repeated line. try is used because some examples define Task as a plain Ruby class.
The guide called the option per-model, although it is set per factory and now carries over to child factories. Its example had a formatting typo and did not say which traits the opt-out removes. The factory options reference did not list the option at all. This rewords the guide, fixes the example, explains its effect, and adds the option to docs/src/ref/factory.md.
Main now appends the registered traits and the definition name to the KeyError raised for an unknown trait, so the exact-message assertion in the per-factory opt-out spec no longer matched. It now expects the same message as the existing global opt-out spec.
Records the new automatically_define_enum_traits factory option under Unreleased so it appears in the next release notes.
|
I found a few issues in the code and documentation. Nothing too big, but since this was already sitting here for so long, I pushed the changes to this branch so that we can merge it faster. The main issue was that subfactories didn't inherit this option when it was set to |
|
Hey @neilvcarvalho, when I originally opened the PR, I was expecting some feedback to fix up. So thank you for handling it yourself and merging it! |
|
That's what we'd usually do, but given the huge delay from the time it was opened to now, I thought it wouldn't make much sense to request changes from you. |
Currently you can only specify whether to automatically define enum traits at a global level, through
FactoryBot.automatically_define_enum_traits. This means that an entire codebase has to either opt-in, or opt-out from automatically defining enum traits. If you are in a large, established codebase with lots of enum's, this is quite hard to change globally when you find that automatically defining them doesn't fit for your new use case.If we could override this at a per-factory level, we could allow individual factories to override the global setting where appropriate, in order to do customise them where necessary.
Given
FactoryBot.automatically_define_enum_traitsbeing set totrue, and a model calledTaskwith the following enum definition:You would be able to override disable this on a per-factory basis like so:
If
FactoryBot.automatically_define_enum_traitswas instead set tofalse, then the same model with a factory override set totrueyou would end up with the following automatic traits:Fixes: #1597