Before this commit, factory_bot registered default callbacks in a global
registry, and offered a `register_callback` method for registering any
custom callbacks (which would be fired within custom strategies).
The idea here seems to have been that we could check against this global
registry and then raise an error when referring to a callback that was
not registered.
However, nobody ever saw this error because we have always had the line:
`FactoryBot::Internal.register_callback(name)`, which registers
callbacks on the fly whenever they are referred to. (I noticed this when
a [change to default callbacks] accidentally duplicated `after_create`
instead of including `before_create`, and we didn't get any tests
failures.)
We have two options here:
1. Preserve the existing behavior by deleting all the callback
registration code. (That is what this commit does)
2. Change to the code try and capture what I assume was the original
intention. This would be a breaking change, so we would want to
introduce it in a major release. (See #1379)
I prefer preserving the existing behavior because we haven't seen any
issues around this, and making a breaking change for this doesn't seem
worthwhile.
[change to default callbacks]: f82e40c8c5 (diff-c6d30be672f880311a7df0820dc4fb21R12-R14)
This commit deprecates several undocumented top-level FactoryBot
methods:
* callbacks
* configuration
* constructor
* initialize_with
* register_sequence
* resent_configuration
* skip_create
* to_create
factory_bot users are not meant to access these methods directly.
Instead they are available for internal use through the new Internal
module introduced in #1262.
---
While addressing the new deprecation warnings, I tried to make the
delegation a bit more consistent, always delegating to
`FactoryBot::Internal`.
This involved requiring "factory_bot/internal" sooner. To avoid having
to fight with the order of requires and potential circular dependencies,
I moved the default strategy and callback registration out of constants
(this is how they used to be before #1297), so it doesn't matter if
those strategies are loaded before the `Internal` module. I also deleted
a pair of tests that were using these constants - these strategies and
callbacks are covered elsewhere in our test suite, and I don't think
these tests were adding much value (they missed #1379, for example).
One annoyance is that the thoughtbot defaults include the Rails
cops, which don't quite make sense here. I am forced into loading the
rubocop-rails gem but then disabling all of the cops in it.
Why:
These methods are used internally for the functionality of the library
and are subject to change. Therefore shouldn't be part of the public
interface.
This PR:
- Moves the ```register_strategy```, ```register_callback```,
```register_default_factories```, ```register_default_callbacks```
```strategies```, ```callback_names```
and ```strategy_by_name``` methods to the ```FactoryBot::Internal```
class.
- Deprecates the use of ```register_callback``` from the ```FactoryBot```
module.
Why:
These methods are used internally for the functionality of the library
and are subject to change. Therefore shouldn't be part of the public
interface.
This PR:
- Moves the ```register_factory``` and ```factory_by_name``` methods to
the ```FactoryBot::Internal``` class.
- Deprecates the use of ```register_factory``` and ```factory_by_name```
from the ```FactoryBot``` module.
- Improve formatting of the specs
Why:
Another run of internal methods that should not be publicly
available from the base namespace.
This time the sequence involving methods were moved. It's worth noticing
that the ```Internal``` class is getting crowded. Maybe we can start
name-spacing the internal groups into modules under
```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts?
This PR:
- Moves the ```register_sequence```, ```sequence_by_name```,
```sequences``` and ```rewind_sequences``` to the
```FactoryBot::Internal``` module.
- Deprecates uses of ```sequence_by_name```, and ```sequences```
from the ```FactoryBot``` module.
- Refactor rewind sequences test to use spies
This is one of the steps towards fixing [this
issue](https://github.com/thoughtbot/factory_bot/pull/1285#1281)
Why:
These are essentially internal methods that should not be publicly
available from the base namespace.
One thing worth noticing is that the use of this methods internally was
almost exclusively in the `syntax/default` except for one use on the
`factory_bot/definition`.
Also the deprecation silencing module was referring to the singleton
instance of the ```ActiveRecord::Deprecation``` class and not to the new
deprecation instance that was being used in the ```FactoryBot``` module.
This PR:
- Moves the `trait_by_name` and `register_trait` into the
`FactoryBot::Internal` module
- Deprecates uses of `trait_by_name`, `register_trait` and `traits` from
the `FactoryBot` module.
- Rename DEPRECATOR => Deprecation
This is one of the steps towards fixing [this
issue](https://github.com/thoughtbot/factory_bot/issues/1281)
Fixes#1257
When sequence rewinding was first introduced in #1078 it only applied to
globally defined sequences. To get rewinding to work for inline
sequences as well we registered them "privately" in the global registry
in #1164. Unfortunately in #1164 we did not take inline sequences inside
traits into consideration. Since trait names are not unique, it is
possibly to get a `FactoryBot::DuplicateDefinitionError` when defining
two sequences that have the same name in two traits that have the same
name.
This PR abandons the idea of "privately" registering inline sequences,
and instead keeps a separate list of all the inline sequences just for
the purpose of rewinding them.
The `FactoryBot` module has a mixture of methods that are meant for use
by people using the library, and methods that are meant only for
internal use. The methods meant for internal use are cluttering the
documentation, and may be confusing to users.
This change was prompted by [#1258]. Rather than introduce yet another
public method on `FactoryBot` meant only for internal use,
we can introduce a `FactoryBot::Internal` module,
and avoid generating documentation for that module.
The `FactoryBot::Internal.register_inline_sequence` method in [#1258]
will need access to the configuration instance, so this PR moves that
into `FactoryBot::Internal`. Eventually I plan to deprecate
`FactoryBot.configuration` and `FactoryBot.reset_configuration`, and to
move more of the `FactoryBot` methods into `FactoryBot::Internal`, but I
would rather hold off on all that until the dust settles on the 5.0
release.
[#1258]: https://github.com/thoughtbot/factory_bot/pull/1258
