From 7f7b480098fa780dd76b4c1243230d74be67b3ca Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sat, 8 Jan 2011 19:59:30 +0000
Subject: [PATCH] When assigning a has_one, if the new record fails to save,
 raise an error

---
 .../associations/has_one_association.rb       |  6 ++--
 .../associations/has_one_associations_test.rb | 28 +++++++++----------
 .../test/cases/autosave_association_test.rb   |  4 +--
 3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index a4332a0511..693c80163a 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -32,9 +32,9 @@ def replace(record, save = true)
         loaded
 
         if @owner.persisted? && record && save
-          record.save && self
-        else
-          record && self
+          unless record.save
+            raise RecordNotSaved, "Failed to save the new associated #{@reflection.name}."
+          end
         end
       end
 
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 46459df7c5..b9719fa983 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -93,18 +93,18 @@ def test_natural_assignment_to_nil
   def test_nullification_on_association_change
     firm = companies(:rails_core)
     old_account_id = firm.account.id
-    firm.account = Account.new
+    firm.account = Account.new(:credit_limit => 5)
     # account is dependent with nullify, therefore its firm_id should be nil
     assert_nil Account.find(old_account_id).firm_id
   end
 
   def test_association_change_calls_delete
-    companies(:first_firm).deletable_account = Account.new
+    companies(:first_firm).deletable_account = Account.new(:credit_limit => 5)
     assert_equal [], Account.destroyed_account_ids[companies(:first_firm).id]
   end
 
   def test_association_change_calls_destroy
-    companies(:first_firm).account = Account.new
+    companies(:first_firm).account = Account.new(:credit_limit => 5)
     assert_equal [companies(:first_firm).id], Account.destroyed_account_ids[companies(:first_firm).id]
   end
 
@@ -182,17 +182,6 @@ def test_build
     assert_equal account, firm.account
   end
 
-  def test_failing_build_association
-    firm = Firm.new("name" => "GlobalMegaCorp")
-    firm.save
-
-    firm.account = account = Account.new
-    assert_equal account, firm.account
-    assert !account.save
-    assert_equal account, firm.account
-    assert_equal ["can't be empty"], account.errors["credit_limit"]
-  end
-
   def test_create
     firm = Firm.new("name" => "GlobalMegaCorp")
     firm.save
@@ -320,4 +309,15 @@ def test_replacement_failure_due_to_existing_record_should_raise_error
     assert_equal ships(:black_pearl), pirate.ship
     assert_equal pirate.id, pirate.ship.pirate_id
   end
+
+  def test_replacement_failure_due_to_new_record_should_raise_error
+    pirate = pirates(:blackbeard)
+    new_ship = Ship.new
+
+    assert_raise(ActiveRecord::RecordNotSaved) do
+      pirate.ship = new_ship
+    end
+    assert_equal new_ship, pirate.ship
+    assert_equal pirate.id, new_ship.pirate_id
+  end
 end
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 71fd3fd836..2e43a20a73 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -90,7 +90,7 @@ def test_save_fails_for_invalid_has_one
     firm = Firm.find(:first)
     assert firm.valid?
 
-    firm.account = Account.new
+    firm.build_account
 
     assert !firm.account.valid?
     assert !firm.valid?
@@ -102,7 +102,7 @@ def test_save_succeeds_for_invalid_has_one_with_validate_false
     firm = Firm.find(:first)
     assert firm.valid?
 
-    firm.unvalidated_account = Account.new
+    firm.build_unvalidated_account
 
     assert !firm.unvalidated_account.valid?
     assert firm.valid?
-- 
GitLab