What?
=====
Within FactoryBot::Evaluator, the build strategy was reported either as
a class OR symbol.
A side-effect of this is misreporting within
ActiveSupport::Notifications hooks, since the instrumentation payload
may have strategies listed as e.g. `:create` or
`FactoryBot::Strategy::Create`, even though they semantically represent
the same information.
This introduces a new instance method for all strategies, `to_sym`,
to be called rather than `class`.
Some codebases have long builds that create over 1000 records, and may
have id collisions between records created with FactoryBot.create and
FactoryBot.build_stubbed
Co-authored-by: Daniel Colson <danieljamescolson@gmail.com>
Fixes#1305
Before Rails 6 when the build_stubbed strategy assigned an id on a
record without a primary key column, the `id=` method would no-op, and
the record would end up looking like a new_record (i.e. `new_record?`
would return true because the id was nil).
This problem surfaced because of a [change in Rails 6][rails], which
caused `id=` to raise a potentially confusing
`ActiveModel::MissingAttributeError: can't write unknown attribute ''`
for records without a primary key column.
Since build_stubbed stubbed was calling `id=` for all instances,
regardless of whether they had primary keys, it was raising the above
error.
To avoid this error, we check whether the instance has a primary_key
defined before setting the id.
We also changed `persisted?` and `new_record?` to be less dependent on
the id. That way those methods will work as expected for records without
primary keys.
[rails]: b6828fc915
Co-authored-by: Jesse Bailey <jbailey117@gmail.com>
We had configured RuboCop to allow lines with up to 142 characters. This
PR fixes a few of the worst offenders so we can bring that number down
to 110.
e7f66fb2 fixed one problem and introduced a new one. Setting timestamps
after clearing_changes_information doesn't work, since using the
created_at and updated_at setters will change the object again. I added
some additional specs for this case (our only spec related to clearing
changes used an object without timestamps).
For the timezone problem, I did a little digging and realized the
casting behavior was actually correct. The reason it was behaving in a
different way than I was expecting is because we had
`time_zone_aware_attributes` set to false, whereas the AR railtie
[sets it to true](54652d886a/activerecord/lib/active_record/railtie.rb (L71)). Setting it to true causes date and datetime attributes
[to get decorated with a TimeZoneConverter](54652d886a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb (L68..L87))
Rather than redefining #created_at and #updated_at, we can use the
existing methods and just set them if there aren't already values
present. We need to do this after clearing changes information,
otherwise the times end up getting converted to the wrong zone.
factory_bot 5 only supports Rails 4.2 and higher,
and #clear_changes_information has been around since then
(66d0a01535).
Fixes#941, since we will are no longer managing any instance variables.
decrement, increment, and toggle do not persist the toggled values, so
there is not reason to disable those methods.
Co-authored-by: Alex Tsui <alextsui05@gmail.com>
