Browse files

Don't validate records of an :autosave association if they're marked …

…for destruction. [#2064 status:resolved]

Signed-off-by: David Heinemeier Hansson <david@loudthinking.com>
  • Loading branch information...
1 parent 3d1d422 commit e2b925fa68203607fb4f9a9aa457c0435c969f9a @alloy alloy committed with dhh Feb 26, 2009
View
11 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, <tt>self</tt>, if it wasn't.
+ # the parent, <tt>self</tt>, if it wasn't. Skips any <tt>:autosave</tt>
+ # 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)
View
30 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 }

0 comments on commit e2b925f

Please sign in to comment.