When a has_many association is not :uniq, appending the same record multiple times should append it to the @target multiple times [#5964 state:resolved]

2 files changed, 14 insertions, 3 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index c513e8ab08..7964f4fa2b 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -462,10 +462,10 @@ module ActiveRecord
           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 050b521b6a..16023defe3 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -405,7 +405,18 @@ module ActiveRecord
           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