diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 6 | ||||
| -rw-r--r-- | activerecord/lib/active_record/autosave_association.rb | 8 | ||||
| -rw-r--r-- | activerecord/test/cases/autosave_association_test.rb | 18 | ||||
| -rw-r--r-- | activerecord/test/models/reply.rb | 5 | ||||
| -rw-r--r-- | activerecord/test/models/topic.rb | 1 |
5 files changed, 35 insertions, 3 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index dda7d19915..5cefb5dfd7 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Fix parent record should not get saved with duplicate children records. + + Fixes #32940. + + *Santosh Wadghule* + * Fix logic on disabling commit callbacks so they are not called unexpectedly when errors occur. *Brian Durand* diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 9575cc24c8..a405f05e0b 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -392,7 +392,7 @@ module ActiveRecord records -= records_to_destroy end - records.each do |record| + records.each_with_index do |record, index| next if record.destroyed? saved = true @@ -401,9 +401,11 @@ module ActiveRecord if autosave saved = association.insert_record(record, false) elsif !reflection.nested? - association_saved = association.insert_record(record) if reflection.validate? - saved = association_saved + valid = association_valid?(reflection, record, index) + saved = valid ? association.insert_record(record, false) : false + else + association.insert_record(record) end end elsif autosave diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 7355f4cd62..cf93d42942 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -27,6 +27,8 @@ require "models/member_detail" require "models/organization" require "models/guitar" require "models/tuning_peg" +require "models/topic" +require "models/reply" class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase def test_autosave_validation @@ -557,6 +559,22 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa assert_equal no_of_clients + 1, Client.count end + def test_parent_should_not_get_saved_with_duplicate_children_records + Topic.delete_all + + content = "Best content" + reply1 = ValidateUniqueContentReply.new(content: content) + reply2 = ValidateUniqueContentReply.new(content: content) + + topic = Topic.new(validate_unique_content_replies: [reply1, reply2]) + + assert_not topic.save + assert topic.errors.any? + + assert_equal 0, Topic.count + assert_equal 0, ValidateUniqueContentReply.count + end + def test_invalid_build new_client = companies(:first_firm).clients_of_firm.build assert_not_predicate new_client, :persisted? diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb index d1cee58788..d178413fd7 100644 --- a/activerecord/test/models/reply.rb +++ b/activerecord/test/models/reply.rb @@ -16,6 +16,11 @@ end class SillyUniqueReply < UniqueReply end +class ValidateUniqueContentReply < Reply + belongs_to :topic, foreign_key: "parent_id" + validates :content, uniqueness: true +end + class WrongReply < Reply validate :errors_on_empty_content validate :title_is_wrong_create, on: :create diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 72699046f9..8368405cef 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -47,6 +47,7 @@ class Topic < ActiveRecord::Base has_many :unique_replies, dependent: :destroy, foreign_key: "parent_id" has_many :silly_unique_replies, dependent: :destroy, foreign_key: "parent_id" + has_many :validate_unique_content_replies, dependent: :destroy, foreign_key: "parent_id" serialize :content |