Rollback the transaction when one of the autosave associations fails …

…to save. [#3391 state:resolved]

activerecord/lib/active_record/autosave_association.rb

@@ -297,13 +297,15 @@ def save_collection_association(reflection)
               association.destroy(record)
             elsif autosave != false && (@new_record_before_save || record.new_record?)
               if autosave
-                association.send(:insert_record, record, false, false)
+                saved = association.send(:insert_record, record, false, false)
               else
                 association.send(:insert_record, record)
               end
             elsif autosave
-              record.save(false)
+              saved = record.save(false)
             end
+
+            raise ActiveRecord::Rollback if saved == false
           end
         end
 
@@ -330,7 +332,9 @@ def save_has_one_association(reflection)
           key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id
           if autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != key || autosave)
             association[reflection.primary_key_name] = key
-            association.save(!autosave)
+            saved = association.save(!autosave)
+            raise ActiveRecord::Rollback if !saved && autosave
+            saved
           end
         end
       end
@@ -351,7 +355,7 @@ def save_belongs_to_association(reflection)
         if autosave && association.marked_for_destruction?
           association.destroy
         elsif autosave != false
-          association.save(!autosave) if association.new_record? || autosave
+          saved = association.save(!autosave) if association.new_record? || autosave
 
           if association.updated?
             association_id = association.send(reflection.options[:primary_key] || :id)
@@ -361,6 +365,8 @@ def save_belongs_to_association(reflection)
               self[reflection.options[:foreign_type]] = association.class.base_class.name.to_s
             end
           end
+
+          saved if autosave
         end
       end
     end

activerecord/test/cases/autosave_association_test.rb

@@ -813,6 +813,18 @@ def test_should_still_raise_an_ActiveRecordRecord_Invalid_exception_if_we_want_t
     end
   end
 
+  def test_should_not_save_and_return_false_if_a_callback_cancelled_saving
+    pirate = Pirate.new(:catchphrase => 'Arr')
+    ship = pirate.build_ship(:name => 'The Vile Insanity')
+    ship.cancel_save_from_callback = true
+
+    assert_no_difference 'Pirate.count' do
+      assert_no_difference 'Ship.count' do
+        assert !pirate.save
+      end
+    end
+  end
+
   def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
     before = [@pirate.catchphrase, @pirate.ship.name]
 
@@ -891,6 +903,18 @@ def test_should_still_raise_an_ActiveRecordRecord_Invalid_exception_if_we_want_t
     end
   end
 
+  def test_should_not_save_and_return_false_if_a_callback_cancelled_saving
+    ship = Ship.new(:name => 'The Vile Insanity')
+    pirate = ship.build_pirate(:catchphrase => 'Arr')
+    pirate.cancel_save_from_callback = true
+
+    assert_no_difference 'Ship.count' do
+      assert_no_difference 'Pirate.count' do
+        assert !ship.save
+      end
+    end
+  end
+
   def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
     before = [@ship.pirate.catchphrase, @ship.name]
 
@@ -906,7 +930,6 @@ def save(*args)
     end
 
     assert_raise(RuntimeError) { assert !@ship.save }
-    # TODO: Why does using reload on @ship looses the associated pirate?
     assert_equal before, [@ship.pirate.reload.catchphrase, @ship.reload.name]
   end
 
@@ -983,6 +1006,26 @@ def test_should_allow_to_bypass_validations_on_the_associated_models_on_create
     end
   end
 
+  def test_should_not_save_and_return_false_if_a_callback_cancelled_saving_in_either_create_or_update
+    @pirate.catchphrase = 'Changed'
+    @child_1.name = 'Changed'
+    @child_1.cancel_save_from_callback = true
+
+    assert !@pirate.save
+    assert_equal "Don' botharrr talkin' like one, savvy?", @pirate.reload.catchphrase
+    assert_equal "Posideons Killer", @child_1.reload.name
+
+    new_pirate = Pirate.new(:catchphrase => 'Arr')
+    new_child = new_pirate.send(@association_name).build(:name => 'Grace OMalley')
+    new_child.cancel_save_from_callback = true
+
+    assert_no_difference 'Pirate.count' do
+      assert_no_difference "#{new_child.class.name}.count" do
+        assert !new_pirate.save
+      end
+    end
+  end
+
   def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
     before = [@pirate.catchphrase, *@pirate.send(@association_name).map(&:name)]
     new_names = ['Grace OMalley', 'Privateers Greed']

activerecord/test/models/bird.rb

@@ -1,3 +1,9 @@
 class Bird < ActiveRecord::Base
   validates_presence_of :name
+
+  attr_accessor :cancel_save_from_callback
+  before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+  def cancel_save_callback_method
+    false
+  end
 end

activerecord/test/models/parrot.rb

@@ -6,6 +6,12 @@ class Parrot < ActiveRecord::Base
   alias_attribute :title, :name
 
   validates_presence_of :name
+
+  attr_accessor :cancel_save_from_callback
+  before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+  def cancel_save_callback_method
+    false
+  end
 end
 
 class LiveParrot < Parrot

activerecord/test/models/pirate.rb

@@ -51,6 +51,12 @@ def reject_empty_ships_on_create(attributes)
     attributes.delete('_reject_me_if_new').present? && new_record?
   end
 
+  attr_accessor :cancel_save_from_callback
+  before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+  def cancel_save_callback_method
+    false
+  end
+
   private
     def log_before_add(record)
       log(record, "before_adding_method")

activerecord/test/models/ship.rb

@@ -9,4 +9,10 @@ class Ship < ActiveRecord::Base
   accepts_nested_attributes_for :update_only_pirate, :update_only => true
 
   validates_presence_of :name
+
+  attr_accessor :cancel_save_from_callback
+  before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+  def cancel_save_callback_method
+    false
+  end
 end