Ensure the parent record is always saved when the child is invalid. [#2249 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>

2 files changed, 31 insertions, 5 deletions

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 741aa2acbe..9717ca3d8b 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -311,11 +311,13 @@ module ActiveRecord
     # ActiveRecord::Base after the AutosaveAssociation module, which it does by default.
     def save_has_one_association(reflection)
       if (association = association_instance_get(reflection.name)) && !association.target.nil?
-        if reflection.options[:autosave] && association.marked_for_destruction?
+        autosave = reflection.options[:autosave]
+
+        if autosave && association.marked_for_destruction?
           association.destroy
-        elsif new_record? || association.new_record? || association[reflection.primary_key_name] != id || reflection.options[:autosave]
+        elsif new_record? || association.new_record? || association[reflection.primary_key_name] != id || autosave
           association[reflection.primary_key_name] = id
-          association.save(false)
+          association.save(!autosave)
         end
       end
     end
@@ -330,10 +332,12 @@ module ActiveRecord
     # ActiveRecord::Base after the AutosaveAssociation module, which it does by default.
     def save_belongs_to_association(reflection)
       if association = association_instance_get(reflection.name)
-        if reflection.options[:autosave] && association.marked_for_destruction?
+        autosave = reflection.options[:autosave]
+
+        if autosave && association.marked_for_destruction?
           association.destroy
         else
-          association.save(false) if association.new_record? || reflection.options[:autosave]
+          association.save(!autosave) if association.new_record? || autosave
 
           if association.updated?
             self[reflection.primary_key_name] = association.id
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 436f50d395..919b6f857c 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -38,6 +38,17 @@ class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
 end
 
 class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
+  def test_should_save_parent_but_not_invalid_child
+    firm = Firm.new(:name => 'GlobalMegaCorp')
+    assert firm.valid?
+
+    firm.build_account_using_primary_key
+    assert !firm.build_account_using_primary_key.valid?
+
+    assert firm.save
+    assert firm.account_using_primary_key.new_record?
+  end
+
   def test_save_fails_for_invalid_has_one
     firm = Firm.find(:first)
     assert firm.valid?
@@ -126,6 +137,17 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas
 end
 
 class TestDefaultAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
+  def test_should_save_parent_but_not_invalid_child
+    client = Client.new(:name => 'Joe (the Plumber)')
+    assert client.valid?
+
+    client.build_firm
+    assert !client.firm.valid?
+
+    assert client.save
+    assert client.firm.new_record?
+  end
+
   def test_save_fails_for_invalid_belongs_to
     assert log = AuditLog.create(:developer_id => 0, :message => "")