Support for rails 5.2?

[ryy]

Is this support going to be added?

"activemodel/lib/active_model/attribute_mutation_tracker.rb" added since 5.2
I use in urgent response because it is not supported

override the method in which the error occurred

ryy@af5e331

[andrykonchin]

How can we reproduce the issue? Could you please add a code snippet do demonstrate the lack of support?

[richardhsu]

I was able to reproduce this in my own by just creating a Dynamoid backed object as well and getting a nil exception.

They added mutations_from_database as @ryo19910801 mentioned:

def mutations_from_database
  unless defined?(@mutations_from_database)
    @mutations_from_database = nil
  end
  @mutations_from_database ||= if @attributes
    ActiveModel::AttributeMutationTracker.new(@attributes)
  else
    NullMutationTracker.instance
  end
end

It looks like Rails 5.2 ActiveModel::Dirty changed attributes to attribute sets.
Basically if you look at our attributes we return something like:

{
  :name => nil,
  :age => nil,
}

At creation but seems like ActiveModel now returns:

#<ActiveModel::AttributeSet:0x000...
  @attributes=
  { "name" => #<ActiveModel::Attribute::FromDatabase:0x000... @name="name"  ...>
    "age" => #<ActiveModel::Attribute::FromDatabase:0x000... @name="age" ...> }
>

Which based on the commit that affects this:
https://github.com/lugray/rails/blob/c3675f50d2e59b7fc173d7b332860c4b1a24a726/activemodel/lib/active_model/attribute.rb

Has defined changed? method on the value of @attributes["name"].changed? which is the issue.

For the temporary fix would prefer if we were explicit in the exception catch:

      NoMethodError:
        undefined method `changed?' for nil:NilClass

So then to catch and be specific which even then is still scary but only error we get:

rescue NoMethodError => e
  return false if e.message == 'undefined method `changed?' for nil:NilClass'
  raise e
end

That way we only raise for this particular mishandling. As for long term in terms of handling this not sure what we'd want to do if we should support similarly. Will continue to experiment.

[richardhsu]

Oy, okay guess this goes deeper, I'm scared that we hide some exceptions with this temporary patch but there are other errors that can crop up and it isn't always a nil exception since the value could be something else of course.

rescue NoMethodError => e
    if e.message =~ /undefined method `(changed\?|original_value)' for/
      return ActiveSupport::HashWithIndifferentAccess.new # Or false for the 
    end
    raise e
end

This is also due to:
https://github.com/lugray/rails/blob/c3675f50d2e59b7fc173d7b332860c4b1a24a726/activemodel/lib/active_model/attribute_mutation_tracker.rb

Which will call:

result[attr_name] = attributes[attr_name].original_value

[andrykonchin]

Will add Rails 5.2 support ASAP

[richardhsu]

@andrykonchin do you have a timeline when you might look at this? I may look at it some more at some point this week but wouldn't want to duplicate our efforts.

[andrykonchin]

@richardhsu The hot fix is ready. Please review.

[richardhsu]

Thanks @andrykonchin and thanks @ryo19910801 for bring this to our attention! 👏