save on parent should not cascade to child unless child changed [#3353 state:resolved]

Signed-off-by: José Valim <jose.valim@gmail.com>

2 files changed, 21 insertions, 1 deletions

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 7517896235..6af384367f 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -372,7 +372,9 @@ module ActiveRecord
         if autosave && association.marked_for_destruction?
           association.destroy
         elsif autosave != false
-          saved = association.save(:validate => !autosave) if association.new_record? || autosave
+          if association.new_record? || ( autosave && association.changed? )
+            saved = association.save(:validate => !autosave) 
+          end
 
           if association.updated?
             association_id = association.send(reflection.options[:primary_key] || :id)
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 3b89c12a3f..48479bb429 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -632,6 +632,8 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
   end
 
   def test_should_rollback_destructions_if_an_exception_occurred_while_saving_a_parent
+    #association save method only trigged when association is changed 
+    @ship.pirate.catchphrase = "new catch phrase"
     # Stub the save method of the @ship.pirate instance to destroy and then raise an exception
     class << @ship.pirate
       def save(*args)
@@ -880,6 +882,22 @@ class TestAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
     @pirate = @ship.create_pirate(:catchphrase => "Don' botharrr talkin' like one, savvy?")
   end
 
+  def test_should_not_call_belongs_to_after_save_callbacks_if_no_changes
+    @ship.attributes = { :name => "Titanic", :pirate_attributes => {:id => @pirate.id} }
+    #here there are no changes to pirate so if save on ship causes save on pirate
+    #this callback will fail pirate save.(pirate save shouldn't happen)
+    @ship.pirate.cancel_save_from_callback = true
+    @ship.save
+    assert_equal 'Titanic', @ship.reload.name
+  end
+
+  def test_should_call_belongs_to_save_if_belongs_to_has_changes
+    @ship.attributes = { :name => "Titanic", :pirate_attributes => { :catchphrase => 'Jack', :id => @pirate.id} }
+    @ship.save
+    assert_equal 'Titanic', @ship.reload.name
+    assert_equal 'Jack', @pirate.reload.catchphrase
+  end
+
   def test_should_still_work_without_an_associated_model
     @pirate.destroy
     @ship.reload.name = "The Vile Insanity"