diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 6e09b13150fa0..1c3d0567c1bb1 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -243,13 +243,16 @@ def validate_collection_association(reflection) end # Returns whether or not the association is valid and applies any errors to - # the parent, self, if it wasn't. + # the parent, self, if it wasn't. Skips any :autosave + # enabled records if they're marked_for_destruction?. def association_valid?(reflection, association) unless valid = association.valid? if reflection.options[:autosave] - association.errors.each do |attribute, message| - attribute = "#{reflection.name}_#{attribute}" - errors.add(attribute, message) unless errors.on(attribute) + unless association.marked_for_destruction? + association.errors.each do |attribute, message| + attribute = "#{reflection.name}_#{attribute}" + errors.add(attribute, message) unless errors.on(attribute) + end end else errors.add(reflection.name) diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 6ced84e0b6cc4..b179bd827a57c 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -13,9 +13,6 @@ require 'models/ship_part' require 'models/treasure' -# TODO: -# - add test case for new parent and children with invalid data and saving with validate = false - class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase def test_autosave_should_be_a_valid_option_for_has_one assert base.valid_keys_for_has_one_association.include?(:autosave) @@ -449,6 +446,14 @@ def test_should_destroy_a_child_association_as_part_of_the_save_transaction_if_i assert_nil Ship.find_by_id(id) end + def test_should_skip_validation_on_a_child_association_if_marked_for_destruction + @pirate.ship.name = '' + assert !@pirate.valid? + + @pirate.ship.mark_for_destruction + assert_difference('Ship.count', -1) { @pirate.save! } + end + def test_should_rollback_destructions_if_an_exception_occurred_while_saving_a_child # Stub the save method of the @pirate.ship instance to destroy and then raise an exception class << @pirate.ship @@ -478,6 +483,14 @@ def test_should_destroy_a_parent_association_as_part_of_the_save_transaction_if_ assert_nil Pirate.find_by_id(id) end + def test_should_skip_validation_on_a_parent_association_if_marked_for_destruction + @ship.pirate.catchphrase = '' + assert !@ship.valid? + + @ship.pirate.mark_for_destruction + assert_difference('Pirate.count', -1) { @ship.save! } + end + def test_should_rollback_destructions_if_an_exception_occurred_while_saving_a_parent # Stub the save method of the @ship.pirate instance to destroy and then raise an exception class << @ship.pirate @@ -511,6 +524,17 @@ def save(*args) ids.each { |id| assert_nil klass.find_by_id(id) } end + define_method("test_should_skip_validation_on_the_#{association_name}_association_if_marked_for_destruction") do + 2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") } + children = @pirate.send(association_name) + + children.each { |child| child.name = '' } + assert !@pirate.valid? + + children.each { |child| child.mark_for_destruction } + assert_difference("#{association_name.classify}.count", -2) { @pirate.save! } + end + define_method("test_should_rollback_destructions_if_an_exception_occurred_while_saving_#{association_name}") do 2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") } before = @pirate.send(association_name).map { |c| c }