From 6d0d83a33f59d9415685852cf77818c41e2e2700 Mon Sep 17 00:00:00 2001
From: Bogdan Gusiev <agresso@gmail.com>
Date: Mon, 23 Nov 2015 16:22:03 +0200
Subject: =?UTF-8?q?Bugfix=20collection=20association=20#create=20method=20?=
 =?UTF-8?q?=E2=80=A6?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When same association is loaded in the model creation callback
The new object is inserted into association twice
---
 .../lib/active_record/associations/collection_association.rb | 12 ++++++++----
 .../test/cases/associations/has_many_associations_test.rb    |  6 ++++++
 activerecord/test/models/bulb.rb                             |  6 ++++++
 3 files changed, 20 insertions(+), 4 deletions(-)

(limited to 'activerecord')

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
-- 
cgit v1.2.3