Fix attribute type cast for duplicated record.

[mihyaeru21]

Problem

The minimum code to reproduce the bug is as follows

class Foo < ActiveRecord::Base
  enumerize :bar, :in => { a: 0, b: 1 }
end

foo1 = Foo.new(bar: :b)
foo2 = foo1.dup

foo2.bar
=> "a"

foo2.bar should be "b" but was actually "a".

If a copy is made by dup, foo2.attributes_before_type_cast["bar"] will hold "b"(Enumerize::Value) instead of 1(Integer). If the argument value of Enumerize::ActiveRecordSupport::Type#cast is "b"(Enumerize::Value), @subtype.cast(value) will return 0(Integer). This is probably because "b".to_i is 0. Therefore, @attr.find_value(0) returns "a"(Enumerize::Value).

On the master branch, the test for the #dup I just added will fail.

Fixes

If the argument value of Enumerize::ActiveRecordSupport::Type#cast is Enumerize::Value, it is returned as is.

[nashby]

@mihyaeru21 hey! Thanks for the PR! I think we should fix underlying issue and not the consequences. I tried to look into why it's happening in the first place when you clone AR object but couldn't really figure out but what we can do to ensure that we have AR object in correct state is to do what we're doing in reload: https://github.com/brainspec/enumerize/blob/4589e4b2dc3ab4ed8d9398a70efa74045ac5dc07/lib/enumerize/activerecord.rb#L75-L88 So I think we can extract it to some method and call it in become, reload and add a new one for dup:

def initialize_dup(other)
  super

  self._reassign_enumerized_attributes(other) # new method we extracted
end

does it make sense?

[mihyaeru21]

My apologies. I should have written more about the process as the cause is complex. Like you, I initially thought it was an incorrect state in the case of dup.

In the dup sequence, @attributes are first copied from the original record by deep_dup in ActiveRecord::Core#initialize_dup. Then it is converted to ActiveModel::Attribute::FromUser in ActiveModel::Dirty#initialize_dup.

# in ActiveModel::Dirty#initialize_dup
@attributes = self.class._default_attributes.map do |attr|
  attr.with_value_from_user(@attributes.fetch_value(attr.name))
end

In the case of an enumerated attribute, @attributes.fetch_value(attr.name) is an instance of Enumerize::Value. The actual process of attr.with_value_from_user is ActiveModel::Attribute.from_user. And the object (Enumerize::Value) passed as the argument of attr.with_value_from_user becomes @value_before_type_cast of @attributes['some_enumerized_attr'].

On the other hand, @attributes['some_enumerized_attr'] is ActiveModel::Attribute::FromDatabase when it is retrieved from DB. @value_before_type_cast is the value stored in the DB.

I did not consider this to be an incorrect state, since it appears that the @value_before_type_cast in ActiveModel::Attribute::FromUser is expected to contain the value after casting. So instead of modifying @value_before_type_cast in ActiveModel::Attribute::FromUser, we chose to add a branch at cast time. What do you think?

I will also try the method you suggested of creating _reassign_enumerized_attributes.

[nashby]

@mihyaeru21 alright, it makes sense, yes. I think I can proceed and merge this PR unless you want to try something else first?

[mihyaeru21]

@nashby If this PR approach seems to be acceptable, no additional attempts will be made. Is there any work needed to merge this PR?

[nashby]

@mihyaeru21 thanks!