Double assignment of attributes on a collection association occurs #1467

Closed
radar opened this Issue Jun 3, 2011 · 6 comments

Comments

Projects
None yet
5 participants
Contributor

radar commented Jun 3, 2011

Greetings,

I'll try to keep this short and sweet because once you see it, it's obvious:

The build_record method in activerecord/lib/active_record/associations/collection_association.rb is currently defined like this:

def build_record(attributes, options)
  record = reflection.build_association(attributes, options)
  record.assign_attributes(create_scope.except(*record.changed), :without_protection => true)
  record.assign_attributes(attributes, options)
  record
end

This method is causing the attributes to be loaded twice, once when build_association is called, and again when assign_attributes is called. In my engine (http://github.com/radar/forem), this causes topics that has_many :posts that are also nested attributes to have two posts when they should only have one. By commenting out / removing the assign_attributes line it does not do a double assignment of attributes.

You can run the tests on forem by running bundle install and then bundle exec rspec spec/integration/topics_spec.rb:35.

Thanks!

Contributor

joshk commented Jun 3, 2011

Actually, its doing it three times isn't it? On build_association and the two assign_attributes.

@jonleighton to the rescue!

Contributor

radar commented Jun 3, 2011

Nope, just twice. The first one doesn't send through the attributes the first time.

Member

jonleighton commented Jun 3, 2011

Yes, the second assign_attributes shouldn't be there. @pixeltrix tweaked this method recently so I reckon he accidentally left it in.

Pull request please? Also please make sure the equivalent change in made in SingularAssociation.

@pixeltrix pixeltrix closed this in 67f5c07 Jun 3, 2011

raghunadhd pushed a commit to castlerock/rails that referenced this issue Jun 3, 2011

Owner

pixeltrix commented Jun 3, 2011

Looks like SingularAssociation is fine. Sorry about that - I don't know how I missed it!

Contributor

radar commented Jun 3, 2011

Thank you for the quick fix! <3

Contributor

samuelkadolph commented Jun 4, 2011

@radar, you fail. It's :heart: ❤️

jake3030 pushed a commit to jake3030/rails that referenced this issue Jun 28, 2011

Remove dead commented out code [#1467 state:resolved]
Signed-off-by: Frederick Cheung <frederick.cheung@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment