Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow overriding of FactoryBot.automatically_define_enum_traits on a per-factory basis #1597

Open
mikespokefire opened this issue Nov 7, 2023 · 3 comments · May be fixed by #1598
Open

Allow overriding of FactoryBot.automatically_define_enum_traits on a per-factory basis #1597

mikespokefire opened this issue Nov 7, 2023 · 3 comments · May be fixed by #1598
Labels
feature

Comments

@mikespokefire
Copy link

Problem this feature will solve

Having FactoryBot.automatically_define_enum_traits default to true is useful when using ActiveRecord::Enum to get up and running with factories quickly. However this is an all-or-nothing option for an entire codebase, with no way to override this on a per-factory basis where we need to customise, or deviate, from the automatic enum trait behaviour.

This is problematic when given a large codebase, with lots of ActiveRecord::Enum usage, as disabling this globally would mean re-working a lot of factories that already work perfectly fine, they would need to be updated to use traits_for_enum instead.

It would be great if we could allow a single factory to stop the automatic definition of enum's from happening, or vice versa if FactoryBot.automatically_define_enum_traits was false and a factory wanted to automatically define them.

Desired solution

If there was an additional option when defining a factory, it would allow us to opt-in, or opt-out of the global automatic enum trait definition behaviour. Something like the following when defining a factory:

FactoryBot.define do
  factory :task, automatically_define_enum_traits: false do
    # Normal FactoryBot scoping, only with no automatically defined enum traits just for this factory.
  end
end

Alternatives considered

Additional context

We have a monolithic codebase, with lots of models and lots of uses of ActiveRecord::Enum across various models. However 1 model has a complex Single Table Inheritance model that uses ActiveRecord::Enum. One of the types of this model doesn't work with all values of the enum. Which in turn doesn't work well with the global default of automatically defining enum traits. I've added a contrived example below as a demonstration, for the sake of a worked example, let's call this Task.

A Task has an ActiveRecord::Enum attribute of confirmed_by that uses the prefix option. On top of the prefixing, when the enums value is :user, it doesn't require extra metadata. When the enums value is :organisation, a validation kicks in to ensure extra metadata is populated to be considered valid.

FactoryBot doesn't seem to take prefixing into account when creating the enum's, it just uses the enums values. However, even if prefixing was supported, we don't refer to this within the tests as :confirmed_by_organisation, we refer to them as :org_confirmed. If FactoryBot supported an enum's :prefix and :suffix options, it would be helpful here, however we want to define our own set of traits that make more sense from this models perspective.

It would be great to have an additional config option, or something similar, when defining a complex factory. E.g. given the following contrived example model:

class Task
  enum :confirmed_by, [:user, :organisation], prefix: true
  validates :additional_metadata_for_organisation, presence: true, if: :confirmed_by_organisation?
end

If we could disable enum traits just for this factory, we could have full control over the defined traits. E.g.

FactoryBot.define do
  factory :task, automatically_define_enum_traits: false do
    confirmed_by { :user }
    
    trait :org_confirmed do
      confirmed_by { :organisation }
      organisation_metadata { { some_data: "with a value" } }
    end
  end
end

Without this option, the above factory would have been automatically defined with the following traits, of which the :organisation trait wouldn't pass linting, as it doesn't pass validation. This is fine in itself, however to fix it, we would need to redefine the :organisation trait to add in the extra attributes, when what we really want to do here is completely customise the traits. E.g.

FactoryBot.define do
  factory :task, automatically_define_enum_traits: false do
    trait :user do  # Ideally we don't want/need this trait
      confirmed_by { :user }
    end
    
    trait :organisation do  # Ideally we would name this trait something else if we could opt-out of automatically_define_enum_traits
      confirmed_by { :organisation }
      organisation_metadata { { some_data: "with a value" } }
    end
  end
end
mikespokefire added a commit to mikespokefire/factory_bot that referenced this issue Nov 7, 2023
@mikespokefire @juliachn
Add per model automatically_define_enum_traits option
e477fcd 
Current 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 `false` 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 <[email protected]>
mikespokefire added a commit to mikespokefire/factory_bot that referenced this issue Nov 7, 2023
@mikespokefire @juliachn
Add per model automatically_define_enum_traits option
bfcf29d 
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 `false` 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 <[email protected]>
mikespokefire added a commit to mikespokefire/factory_bot that referenced this issue Nov 7, 2023
@mikespokefire @juliachn
Add per model automatically_define_enum_traits option
f3e9265 
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 <[email protected]>
@mikespokefire mikespokefire linked a pull request Nov 7, 2023 that will close this issue
Open
@mikespokefire
Copy link
Author

I've had a stab at implementing what this might look like, which I've just opened a PR for here: #1598

@mikespokefire
Copy link
Author

👋 Hey, this has been sat for a while, with a PR associated with it that may also fix the issue. I'm just bumping it, as I'm wondering if there is any interest in solving this issue or not?

@applestump
Copy link

Hey! 👋 Just a follow up on this too, as this would be pretty useful for our codebase. Just wondering if there's a desire/indication on adopting this as a feature or not (perhaps with the attached PR?). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add per model automatically_define_enum_traits option mikespokefire/factory_bot
2 participants
@mikespokefire @applestump