Explicitely setting `autosave => false' should override new_record autosaving. [#2214 state:resolved]

Original author is Jacob.

4 files changed, 75 insertions, 6 deletions

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index ebd47ec634..aff29dcc4e 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -285,7 +285,7 @@ module ActiveRecord
           records.each do |record|
             if autosave && record.marked_for_destruction?
               association.destroy(record)
-            elsif @new_record_before_save || record.new_record?
+            elsif autosave != false && (@new_record_before_save || record.new_record?)
               if autosave
                 association.send(:insert_record, record, false, false)
               else
@@ -316,7 +316,7 @@ module ActiveRecord
 
         if autosave && association.marked_for_destruction?
           association.destroy
-        elsif new_record? || association.new_record? || association[reflection.primary_key_name] != id || autosave
+        elsif autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != id || autosave)
           association[reflection.primary_key_name] = id
           association.save(!autosave)
         end
@@ -337,7 +337,7 @@ module ActiveRecord
 
         if autosave && association.marked_for_destruction?
           association.destroy
-        else
+        elsif autosave != false
           association.save(!autosave) if association.new_record? || autosave
 
           if association.updated?
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 00e64ca51f..fc89b83728 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -443,6 +443,70 @@ class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCa
   end
 end
 
+class TestDefaultAutosaveAssociationOnNewRecord < ActiveRecord::TestCase
+  def test_autosave_new_record_on_belongs_to_can_be_disabled_per_relationship
+    new_account = Account.new("credit_limit" => 1000)
+    new_firm = Firm.new("name" => "some firm")
+
+    assert new_firm.new_record?
+    new_account.firm = new_firm
+    new_account.save!
+
+    assert !new_firm.new_record?
+
+    new_account = Account.new("credit_limit" => 1000)
+    new_autosaved_firm = Firm.new("name" => "some firm")
+
+    assert new_autosaved_firm.new_record?
+    new_account.unautosaved_firm = new_autosaved_firm
+    new_account.save!
+
+    assert new_autosaved_firm.new_record?
+  end
+
+  def test_autosave_new_record_on_has_one_can_be_disabled_per_relationship
+    firm = Firm.new("name" => "some firm")
+    account = Account.new("credit_limit" => 1000)
+
+    assert account.new_record?
+    firm.account = account
+    firm.save!
+
+    assert !account.new_record?
+
+    firm = Firm.new("name" => "some firm")
+    account = Account.new("credit_limit" => 1000)
+
+    firm.unautosaved_account = account
+
+    assert account.new_record?
+    firm.unautosaved_account = account
+    firm.save!
+
+    assert account.new_record?
+  end
+
+  def test_autosave_new_record_on_has_many_can_be_disabled_per_relationship
+    firm = Firm.new("name" => "some firm")
+    account = Account.new("credit_limit" => 1000)
+
+    assert account.new_record?
+    firm.accounts << account
+
+    firm.save!
+    assert !account.new_record?
+
+    firm = Firm.new("name" => "some firm")
+    account = Account.new("credit_limit" => 1000)
+
+    assert account.new_record?
+    firm.unautosaved_accounts << account
+
+    firm.save!
+    assert account.new_record?
+  end
+end
+
 class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
   self.use_transactional_fixtures = false
 
diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb
index a164f5e060..aced946b1e 100644
--- a/activerecord/test/cases/reflection_test.rb
+++ b/activerecord/test/cases/reflection_test.rb
@@ -176,9 +176,9 @@ class ReflectionTest < ActiveRecord::TestCase
 
   def test_reflection_of_all_associations
     # FIXME these assertions bust a lot
-    assert_equal 31, Firm.reflect_on_all_associations.size
-    assert_equal 24, Firm.reflect_on_all_associations(:has_many).size
-    assert_equal 7, Firm.reflect_on_all_associations(:has_one).size
+    assert_equal 34, Firm.reflect_on_all_associations.size
+    assert_equal 26, Firm.reflect_on_all_associations(:has_many).size
+    assert_equal 8, Firm.reflect_on_all_associations(:has_one).size
     assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size
   end
 
diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb
index 9242c209ea..d69152ec34 100644
--- a/activerecord/test/models/company.rb
+++ b/activerecord/test/models/company.rb
@@ -79,6 +79,10 @@ class Firm < Company
   # Oracle tests were failing because of that as the second fixture was selected
   has_one :account_using_primary_key, :primary_key => "firm_id", :class_name => "Account", :order => "id"
   has_one :deletable_account, :foreign_key => "firm_id", :class_name => "Account", :dependent => :delete
+  
+  has_one :unautosaved_account, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false
+  has_many :accounts
+  has_many :unautosaved_accounts, :foreign_key => "firm_id", :class_name => 'Account', :autosave => false
 end
 
 class DependentFirm < Company
@@ -149,6 +153,7 @@ end
 
 class Account < ActiveRecord::Base
   belongs_to :firm
+  belongs_to :unautosaved_firm, :foreign_key => "firm_id", :class_name => "Firm", :autosave => false
 
   def self.destroyed_account_ids
     @destroyed_account_ids ||= Hash.new { |h,k| h[k] = [] }