Merge pull request #18155 from bogdan/collection_association_double_element_fix

Bugfix collection association #create method

3 files changed, 20 insertions, 4 deletions

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index f32dddb8f0..473b80a658 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -414,12 +414,16 @@ module ActiveRecord
 
       def replace_on_target(record, index, skip_callbacks)
         callback(:before_add, record) unless skip_callbacks
+
+        was_loaded = loaded?
         yield(record) if block_given?
 
-        if index
-          @target[index] = record
-        else
-          @target << record
+        unless !was_loaded && loaded?
+          if index
+            @target[index] = record
+          else
+            @target << record
+          end
         end
 
         callback(:after_add, record) unless skip_callbacks
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 50ca6537cc..ad157582a4 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -2348,6 +2348,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [first_bulb, second_bulb], car.bulbs
   end
 
+  test 'double insertion of new object to association when same association used in the after create callback of a new object' do
+    car = Car.create!
+    car.bulbs << TrickyBulb.new
+    assert_equal 1, car.bulbs.size
+  end
+
   def test_association_force_reload_with_only_true_is_deprecated
     company = Company.find(1)
 
diff --git a/activerecord/test/models/bulb.rb b/activerecord/test/models/bulb.rb
index c1e491e5c5..dc0296305a 100644
--- a/activerecord/test/models/bulb.rb
+++ b/activerecord/test/models/bulb.rb
@@ -50,3 +50,9 @@ class FailedBulb < Bulb
     throw(:abort)
   end
 end
+
+class TrickyBulb < Bulb
+  after_create do |record|
+    record.car.bulbs.to_a
+  end
+end