davydovanton / shallow_attributes

Simple and lightweight Virtus analog.
MIT License
101 stars 18 forks source link

Mass-assignment can work with a stringified hash #14

Closed tatey closed 7 years ago

tatey commented 7 years ago

Hello,

We came across an issue where mass assigning a stringified hash of attributes would raise a KeyError if any of those attributes were uninitialized on the object that mixes in ShallowAttributes.

Example:

class MainUser
  include ShallowAttributes
  attributes :age, Integer
end

u = MainUser.new
u.attributes # {}
u.attributes = {"age" => 21} # => KeyError

And this is the type of error you would get:

ShallowAttributes::#attributes=#test_0003_sets attributes which have not been initialized:
RuntimeError: can't add a new key into hash during iteration
    /shallow_attributes/lib/shallow_attributes/class_methods.rb:105:in `age='
    /shallow_attributes/lib/shallow_attributes/instance_methods.rb:207:in `block in define_attributes'
    /shallow_attributes/lib/shallow_attributes/instance_methods.rb:206:in `each'
    /shallow_attributes/lib/shallow_attributes/instance_methods.rb:206:in `define_attributes'
    /shallow_attributes/lib/shallow_attributes/instance_methods.rb:90:in `attributes='
    /shallow_attributes/test/shallow_attributes_test.rb:121:in `block (3 levels) in <top (required)>'

The reason we get this error is because of how we use ShallowAttributes with our form objects.

  1. The form object is built in a before_action and then we update the form object with the POST data.
  2. We perform mass assignment with a stringified hash (It's actually ActionController::StrongParameters) rather than a symbolized hash.

This patch changes the behaviour to make mass assignment work with both stringified and symbolized hashes. Given to_sym on symbols is a no-op there should be minimal overhead to projects using symbolized hashes.

Happy to have any feedback :smile:

Cheers, Tate

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.007%) to 99.26% when pulling de185d3537a8a7303cbf9bb7c202695251fd89ce on fivegoodfriends:fix-mass-assignment-with-uninitialized-attributes into 141489ab1135d5a0dc3def0470f09bbf4565d6e2 on davydovanton:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.007%) to 99.26% when pulling de185d3537a8a7303cbf9bb7c202695251fd89ce on fivegoodfriends:fix-mass-assignment-with-uninitialized-attributes into 141489ab1135d5a0dc3def0470f09bbf4565d6e2 on davydovanton:master.

davydovanton commented 7 years ago

thanks!

tatey commented 7 years ago

Appreciate you merging @davydovanton. Would you mind cutting a new release? 😀

tatey commented 7 years ago

Bump? 😄

I'm happy to help if you need a hand.