search

solnic / virtus

[DISCONTINUED ] Attributes on Steroids for Plain Old Ruby Objects
MIT License
3.77k stars 229 forks source link

325 mass assignment bug fix #326

Closed novikserg closed 9 years ago

novikserg commented 9 years ago

So I decided to fix the bug referenced in #325. This PR reverts commit a385ca2e6b724ec400c05a4bc7e55bfb9976c1c9 and adds specs for situation when user wants to use "attributes" as attribute name.

I tried to first keep the initial intention of a385ca2e6b724ec400c05a4bc7e55bfb9976c1c9 and make AllowedWriterMethods module be included only when mass-assignment is turned on, however I found it being tied into object initialization pretty tightly: https://github.com/solnic/virtus/blob/d494b95e4181cda44c44d58566cee2185f9a6d9e/lib/virtus/attribute_set.rb#L169 https://github.com/solnic/virtus/blob/d494b95e4181cda44c44d58566cee2185f9a6d9e/lib/virtus/instance_methods.rb#L16 so it wouldn't be that easy of a change. So my suggestion is to revert that old commit (since it's intended goal is not reached anyway, unfortunately) to fix a bug and keep it as a reference for future when this refactoring can be actually done.

@solnic what do you think?

solnic commented 9 years ago

Sorry for late response. This makes sense. The build is failing due to a change in the equalizer so I'm gonna merge it in and deal with equalizer in master.

novikserg commented 9 years ago

No problem. I also tried to look into failing specs by the way, I will reply in related issue.