Fix autosave association to skip validation if it is marked for destruction. [#2064 state:resolved]

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>

2 files changed, 11 insertions, 8 deletions

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index aff29dcc4e..10dd0b4f05 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -243,17 +243,15 @@ module ActiveRecord
 
     # Returns whether or not the association is valid and applies any errors to
     # the parent, <tt>self</tt>, if it wasn't. Skips any <tt>:autosave</tt>
-    # enabled records if they're marked_for_destruction?.
+    # enabled records if they're marked_for_destruction? or destroyed.
     def association_valid?(reflection, association)
-      return true if association.destroyed?
+      return true if association.destroyed? || association.marked_for_destruction?
 
       unless valid = association.valid?
         if reflection.options[:autosave]
-          unless association.marked_for_destruction?
-            association.errors.each do |attribute, message|
-              attribute = "#{reflection.name}_#{attribute}"
-              errors[attribute] << message if errors[attribute].empty?
-            end
+          association.errors.each do |attribute, message|
+            attribute = "#{reflection.name}_#{attribute}"
+            errors[attribute] << message if errors[attribute].empty?
           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 fc89b83728..d51c4398d4 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -544,6 +544,7 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
     assert !@pirate.valid?
 
     @pirate.ship.mark_for_destruction
+    @pirate.ship.expects(:valid?).never
     assert_difference('Ship.count', -1) { @pirate.save! }
   end
 
@@ -581,6 +582,7 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
     assert !@ship.valid?
 
     @ship.pirate.mark_for_destruction
+    @ship.pirate.expects(:valid?).never
     assert_difference('Pirate.count', -1) { @ship.save! }
   end
 
@@ -624,7 +626,10 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
       children.each { |child| child.name = '' }
       assert !@pirate.valid?
 
-      children.each { |child| child.mark_for_destruction }
+      children.each do |child|
+        child.mark_for_destruction
+        child.expects(:valid?).never
+      end
       assert_difference("#{association_name.classify}.count", -2) { @pirate.save! }
     end