Skip to content

Commit

Permalink
Fix parent record should not get saved with duplicate children records
Browse files Browse the repository at this point in the history
- Fixes #32940
  • Loading branch information
mechanicles committed May 28, 2018
1 parent 2f76256 commit d7a3f33
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 3 deletions.
6 changes: 6 additions & 0 deletions 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*
Expand Down
8 changes: 5 additions & 3 deletions activerecord/lib/active_record/autosave_association.rb
Expand Up @@ -392,7 +392,7 @@ def save_collection_association(reflection)
records -= records_to_destroy
end

records.each do |record|
records.each_with_index do |record, index|
next if record.destroyed?

saved = true
Expand All @@ -401,9 +401,11 @@ def save_collection_association(reflection)
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
Expand Down
18 changes: 18 additions & 0 deletions activerecord/test/cases/autosave_association_test.rb
Expand Up @@ -27,6 +27,8 @@
require "models/organization"
require "models/guitar"
require "models/tuning_peg"
require "models/topic"
require "models/reply"

class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
def test_autosave_validation
Expand Down Expand Up @@ -557,6 +559,22 @@ def test_valid_adding_with_validate_false
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?
Expand Down
5 changes: 5 additions & 0 deletions activerecord/test/models/reply.rb
Expand Up @@ -16,6 +16,11 @@ class UniqueReply < Reply
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
Expand Down
1 change: 1 addition & 0 deletions activerecord/test/models/topic.rb
Expand Up @@ -47,6 +47,7 @@ def two

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

Expand Down

0 comments on commit d7a3f33

Please sign in to comment.