diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index c513e8ab08c1ff2fcd9112c4f2d10576863bf7dc..7964f4fa2b42b70f670088182a0816125c591782 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -462,10 +462,10 @@ def add_record_to_target_with_callbacks(record) callback(:before_add, record) yield(record) if block_given? @target ||= [] unless loaded? - if index = @target.index(record) + if @reflection.options[:uniq] && index = @target.index(record) @target[index] = record else - @target << record + @target << record end callback(:after_add, record) set_inverse_instance(record, @owner) diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 050b521b6a3e6259f1f011674cd383e01762e134..16023defe38d89d444b3dc4dccc8dd5491e6cb5d 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -405,7 +405,18 @@ def assign_nested_attributes_for_collection_association(association_name, attrib end elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s } - association.send(:add_record_to_target_with_callbacks, existing_record) if !association.loaded? && !call_reject_if(association_name, attributes) + unless association.loaded? || call_reject_if(association_name, attributes) + # Make sure we are operating on the actual object which is in the association's + # proxy_target array (either by finding it, or adding it if not found) + target_record = association.proxy_target.detect { |record| record == existing_record } + + if target_record + existing_record = target_record + else + association.send(:add_record_to_target_with_callbacks, existing_record) + end + end + assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) else diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 6b71e7371835c3589c93e7f8552db537ed37d640..dfb3292502e62eee5138f8eefb688b2f7f1754c7 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -45,6 +45,16 @@ def test_associate_existing assert posts(:thinking).reload.people(true).include?(people(:david)) end + def test_associate_existing_record_twice_should_add_to_target_twice + post = posts(:thinking) + person = people(:david) + + assert_difference 'post.people.to_a.count', 2 do + post.people << person + post.people << person + end + end + def test_associating_new assert_queries(1) { posts(:thinking) } new_person = nil # so block binding catches it