Fix factory loading

[elia]

Description

The current FactoryBot support code doesn't take into account that Solidus factories need to be loaded before app-level factories, to allow modifications.

With this change using factory_bot_rails it will work out of the box (the initializer works with v4.8 up to v6).
Nothing is done if FactoryBot is used directly, assuming the a more fine grained control/configuration is being used.
The code will be activated only if FactoryBotRails is defined, so to avoid running it in production is enough to keep the gem in the developmen/test groups as advised by the factory_bot readme.

Ref #3906 (this issue will be fixed by a back port of this PR to v2.11).

Checklist:

• I have followed Pull Request guidelines
• I have added a detailed description into each commit message
• I have updated Guides and README accordingly to this change (if needed)
• I have added tests to cover this change (if needed)
• I have attached screenshots to this PR for visual changes (if needed)

[kennyadsl]

Left a preliminary question about the approach used, thanks @elia!

[tvdeyen]

FYI: This change and the PR that introduced it breaks every gem/app in our eco system in a minor version that loads individual factories. We probably should revert it in 2.11 or add a fix that still supports loading individual factories and only remove it in 3.0

[kennyadsl]

@tvdeyen thanks for the feedback, the goal of this PR is to still provide support for loading individual factories without breaking any application. Do you have any evidence that it is breaking things? I tried this PR with a couple of real stores and they just give deprecation warnings. Of course, I don't expect to have covered all possible scenarios for factory_bot configuration.

[tvdeyen]

@kennyadsl I have not tested this PR, but this CI run is from a recent Solidus v2.11

https://github.com/AlchemyCMS/alchemy-solidus/runs/1888940075?check_suite_focus=true

[tvdeyen]

@elia @kennyadsl the switch to mutate FactoryBot factory load paths to load factories breaks the current way of requiring individual factories in tests suites of gems and apps. The errors you are getting are missing factories, duplicated factories and missing sequences. I think there is no easy way of supporting both ways the same time.

I agree that the FactoryBot way is better and we should not require individual factories in test suites, but this is an widely used approach and we should figure out a way to a) support both or b) revert this change in 2.11

[kennyadsl]

For gems, I made this PR on Solidus Support that, once this is merged, "should" fix all extensions. I only tried with a couple so I'm not really sure it works for everyone.

For stores, reverting breaking changes is exactly what this PR has been made for. If you can help us understand the exact problem you are facing, we'd be happy to explore it or if not solvable, I agree we should revert and add this breaking behavior in a major.

[kennyadsl]

@tvdeyen I'm taking a look at alchemy-solidus (checking that out to the commit before your fix in master) with this branch and I think you are right.

There's one big thing we didn't consider: when cherry-picking a file we are showing a deprecation message but not loading the specific dependency of that factory anymore.

For example when requiring the product_factory individually, we print the deprecation message but we are not re-requiring all the factories that this file was relying on, which were:

require 'spree/testing_support/sequences'
require 'spree/testing_support/factories/shipping_category_factory'
require 'spree/testing_support/factories/stock_location_factory'
require 'spree/testing_support/factories/tax_category_factory'
require 'spree/testing_support/factories/product_option_type_factory'

I'll talk with @elia to see if there's a way to make it work but it's becoming uncomfortable to keep adding code again and again to fix all the possible cases, so I'd strongly consider a revert.

Thanks again for pointing this out.

[kennyadsl]

@tvdeyen the last pushed code restores backward compatibility to require factories individually, I tested it with alchemy-solidus (checking out to a commit previous to your fix in AlchemyCMS/alchemy-solidus#72).

I think that after some git commit improvements we can merge and backport this into 2.11. Projects that are requiring factories individually will keep their stores/gems working and we also print a deprecation warning since this behavior will not be supported anymore with Solidus 3.0.

What do you think?

[tvdeyen]

@kennyadsl wow. Thanks. This looks promising 🙏🏻 @mamhoff and I will test this and provide feedback asap.

[tvdeyen]

Looks great.

[kennyadsl]

Worked together with @elia here.

We tested this change on a real application, 2-3 extensions and an external gem (alchemy-solidus) that was cherry-picking individual factories. Apparently fixed the backward-compatibility issues in all of them, printing a deprecation warning as well.