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>
This commit is contained in:
parent
3d1d422b8b
commit
e2b925fa68
@ -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)
|
||||
|
@ -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 }
|
||||
|
Loading…
Reference in New Issue
Block a user