Merge pull request #29593 from kratob/master

ActiveRecord: do not create "has many through" records that have been removed

3 files changed, 23 insertions, 0 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index d4f4041910..f780029a18 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Previously, when building records using a `has_many :through` association,
+    if the child records were deleted before the parent was saved, they would
+    still be persisted. Now, if child records are deleted before the parent is saved
+    on a `has_many :through` association, the child records will not be persisted.
+
+    *Tobias Kraze*
+
 *   Merging two relations representing nested joins no longer transforms the joins of
     the merged relation into LEFT OUTER JOIN. Example to clarify:
 
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 53ffb3b68d..2fd20b4368 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -109,6 +109,11 @@ module ActiveRecord
           record
         end
 
+        def remove_records(existing_records, records, method)
+          super
+          delete_through_records(records)
+        end
+
         def target_reflection_has_associated_record?
           !(through_reflection.belongs_to? && owner[through_reflection.foreign_key].blank?)
         end
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 9156f6d57a..1c2138a3d0 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -319,6 +319,17 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert_includes post.single_people, person
   end
 
+  def test_build_then_remove_then_save
+    post = posts(:thinking)
+    post.people.build(first_name: "Bob")
+    ted = post.people.build(first_name: "Ted")
+    post.people.delete(ted)
+    post.save!
+    post.reload
+
+    assert_equal ["Bob"], post.people.collect(&:first_name)
+  end
+
   def test_both_parent_ids_set_when_saving_new
     post = Post.new(title: "Hello", body: "world")
     person = Person.new(first_name: "Sean")