davydovanton / shallow_attributes

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

Fix coercion for array type: do not coerce array value if type is the same as specified #32

Open evheny0 opened 5 years ago

evheny0 commented 5 years ago

Hi!

I've noticed some inconsistency in array type coercion. Right now if you specify attribute type

attribute :some_attribute, Array, of: SomeClass

it will coerce array values no matter of their class, even if they are the same class as specified in of param. This is a bit different from other types, which will not coerce if class is the same as declared in type.

Here is an example of current behavior:

class Person
  include ShallowAttributes

  attribute :addresses_array, Array, of: Hash
  attribute :address_hash, Hash
end

Person.new(address_hash: { street: "abc" })
# <Person address_hash={:street=>"abc"} addresses_array=[]>

Person.new(addresses_array: [{ street: "abc" }])
# NoMethodError: undefined method `coerce' for {}:Hash

And after fix behavior:

class Person
  include ShallowAttributes

  attribute :addresses_array, Array, of: Hash
  attribute :address_hash, Hash
end

Person.new(address_hash: { street: "abc" })
# <Person address_hash={:street=>"abc"} addresses_array=[]>

Person.new(addresses_array: [{ street: "abc" }])
# #<Person addresses_array=[{:street=>"abc"}]>

I hope this way there will be less confusion πŸ˜„

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.0002%) to 99.778% when pulling f299bd028b7c138b5de1fc00aab7f056ea2e3b43 on evheny0:fix-array-subtypes-coercion into 51f5c1b017e6d389446a87bf02598fee844605fd on davydovanton:master.

lmmendes commented 5 years ago

Hi @davydovanton when can we expect this bug fix to get merged and a release pushed to rubygems?

evheny0 commented 4 years ago

Hi @davydovanton! CI is successful except for the jruby head, which fails on master as well. I believe the branch is ready to be merged :)

evheny0 commented 4 years ago

Sorry for the delay πŸ˜„

javierseixas commented 4 years ago

Hi! Waiting for this fix to be released! :point_up: