From a22c39e9cc08c052bb5c828b45533e5a01c896f1 Mon Sep 17 00:00:00 2001 From: "Eileen M. Uchitelle" Date: Wed, 28 Jun 2017 08:20:57 -0400 Subject: [PATCH] Merge pull request #29593 from kratob/master ActiveRecord: do not create "has many through" records that have been removed --- activerecord/CHANGELOG.md | 8 ++++++++ .../associations/has_many_through_association.rb | 5 +++++ .../has_many_through_associations_test.rb | 11 +++++++++++ 3 files changed, 24 insertions(+) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4f30b49f7371c..2df4a16d62245 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* 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* + + ## Rails 5.1.2 (June 26, 2017) ## * Restore previous behavior of collection proxies: their values can have 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 53ffb3b68d3eb..2fd20b4368114 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 @@ def build_record(attributes) 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 ea52fb5a677b3..208e1404d3059 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -337,6 +337,17 @@ def test_build_then_save_with_has_one_inverse 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")