Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Do not re-save destroyed association on saving parent object #11451

Merged
merged 1 commit into from

2 participants

@pftg

Closes #11450

activerecord/test/cases/autosave_association_test.rb
@@ -705,6 +705,13 @@ def test_should_destroy_has_many_as_part_of_the_save_transaction_if_they_were_ma
ids.each { |id| assert_nil klass.find_by_id(id) }
end
+ def test_should_not_resave_destroyed_association
+ @pirate.birds.create!(name: :parrot)
+ @pirate.birds.first.destroy
+ @pirate.save
@senny Owner
senny added a note

We should make sure that the save actually worked:

save! or assert @pirate.save

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pftg

Thanks @senny. Updated PR.

@senny senny merged commit a618e9e into rails:master
@senny
Owner

@pftg thanks :yellow_heart:

@pftg

Thank you very much! :green_heart:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 15, 2013
  1. @pftg
This page is out of date. Refresh to see the latest.
View
1  activerecord/lib/active_record/autosave_association.rb
@@ -343,6 +343,7 @@ def save_collection_association(reflection)
end
records.each do |record|
+ next if record.destroyed?
saved = true
View
7 activerecord/test/cases/autosave_association_test.rb
@@ -705,6 +705,13 @@ def test_should_destroy_has_many_as_part_of_the_save_transaction_if_they_were_ma
ids.each { |id| assert_nil klass.find_by_id(id) }
end
+ def test_should_not_resave_destroyed_association
+ @pirate.birds.create!(name: :parrot)
+ @pirate.birds.first.destroy
+ @pirate.save!
+ assert @pirate.reload.birds.empty?
+ end
+
def test_should_skip_validation_on_has_many_if_marked_for_destruction
2.times { |i| @pirate.birds.create!(:name => "birds_#{i}") }
Something went wrong with that request. Please try again.