Merge pull request rails#17808 from yuki24/fix-bug-where-record-not-saved-loses-error-message

Fixed a bug where AR::RecordNotSaved loses the given error message

Author: rafaelfranca

Diff for: activerecord/lib/active_record/errors.rb

@@ -54,9 +54,9 @@ class RecordNotFound < ActiveRecordError
   class RecordNotSaved < ActiveRecordError
     attr_reader :record
 
-    def initialize(record)
+    def initialize(message, record = nil)
       @record = record
-      super()
+      super(message)
     end
   end
 

Diff for: activerecord/lib/active_record/persistence.rb

@@ -139,7 +139,7 @@ def save(*)
     # Attributes marked as readonly are silently ignored if the record is
     # being updated.
     def save!(*)
-      create_or_update || raise(RecordNotSaved, self)
+      create_or_update || raise(RecordNotSaved.new(nil, self))
     end
 
     # Deletes the record in the database and freezes this instance to

Diff for: activerecord/test/cases/associations/has_many_associations_test.rb

@@ -589,17 +589,21 @@ def test_adding_using_create
   end
 
   def test_create_with_bang_on_has_many_when_parent_is_new_raises
-    assert_raise(ActiveRecord::RecordNotSaved) do
+    error = assert_raise(ActiveRecord::RecordNotSaved) do
       firm = Firm.new
       firm.plain_clients.create! :name=>"Whoever"
     end
+
+    assert_equal "You cannot call create unless the parent is saved", error.message
   end
 
   def test_regular_create_on_has_many_when_parent_is_new_raises
-    assert_raise(ActiveRecord::RecordNotSaved) do
+    error = assert_raise(ActiveRecord::RecordNotSaved) do
       firm = Firm.new
       firm.plain_clients.create :name=>"Whoever"
     end
+
+    assert_equal "You cannot call create unless the parent is saved", error.message
   end
 
   def test_create_with_bang_on_has_many_raises_when_record_not_saved
@@ -610,9 +614,11 @@ def test_create_with_bang_on_has_many_raises_when_record_not_saved
   end
 
   def test_create_with_bang_on_habtm_when_parent_is_new_raises
-    assert_raise(ActiveRecord::RecordNotSaved) do
+    error = assert_raise(ActiveRecord::RecordNotSaved) do
       Developer.new("name" => "Aredridel").projects.create!
     end
+
+    assert_equal "You cannot call create unless the parent is saved", error.message
   end
 
   def test_adding_a_mismatch_class
@@ -1353,10 +1359,13 @@ def test_replace_failure
 
     assert !account.valid?
     assert !orig_accounts.empty?
-    assert_raise ActiveRecord::RecordNotSaved do
+    error = assert_raise ActiveRecord::RecordNotSaved do
       firm.accounts = [account]
     end
+
     assert_equal orig_accounts, firm.accounts
+    assert_equal "Failed to replace accounts because one or more of the " \
+                 "new records could not be saved.", error.message
   end
 
   def test_replace_with_same_content

Diff for: activerecord/test/cases/associations/has_many_through_associations_test.rb

@@ -615,8 +615,11 @@ def test_associate_with_create_exclamation_and_no_options
   def test_create_on_new_record
     p = Post.new
 
-    assert_raises(ActiveRecord::RecordNotSaved) { p.people.create(:first_name => "mew") }
-    assert_raises(ActiveRecord::RecordNotSaved) { p.people.create!(:first_name => "snow") }
+    error = assert_raises(ActiveRecord::RecordNotSaved) { p.people.create(:first_name => "mew") }
+    assert_equal "You cannot call create unless the parent is saved", error.message
+
+    error = assert_raises(ActiveRecord::RecordNotSaved) { p.people.create!(:first_name => "snow") }
+    assert_equal "You cannot call create unless the parent is saved", error.message
   end
 
   def test_associate_with_create_and_invalid_options

Diff for: activerecord/test/cases/associations/has_one_associations_test.rb

@@ -410,9 +410,11 @@ def test_creation_failure_due_to_new_record_should_raise_error
     pirate = pirates(:redbeard)
     new_ship = Ship.new
 
-    assert_raise(ActiveRecord::RecordNotSaved) do
+    error = assert_raise(ActiveRecord::RecordNotSaved) do
       pirate.ship = new_ship
     end
+
+    assert_equal "Failed to save the new associated ship.", error.message
     assert_nil pirate.ship
     assert_nil new_ship.pirate_id
   end
@@ -422,20 +424,25 @@ def test_replacement_failure_due_to_existing_record_should_raise_error
     pirate.ship.name = nil
 
     assert !pirate.ship.valid?
-    assert_raise(ActiveRecord::RecordNotSaved) do
+    error = assert_raise(ActiveRecord::RecordNotSaved) do
       pirate.ship = ships(:interceptor)
     end
+
     assert_equal ships(:black_pearl), pirate.ship
     assert_equal pirate.id, pirate.ship.pirate_id
+    assert_equal "Failed to remove the existing associated ship. " +
+                 "The record failed to save after its foreign key was set to nil.", error.message
   end
 
   def test_replacement_failure_due_to_new_record_should_raise_error
     pirate = pirates(:blackbeard)
     new_ship = Ship.new
 
-    assert_raise(ActiveRecord::RecordNotSaved) do
+    error = assert_raise(ActiveRecord::RecordNotSaved) do
       pirate.ship = new_ship
     end
+
+    assert_equal "Failed to save the new associated ship.", error.message
     assert_equal ships(:black_pearl), pirate.ship
     assert_equal pirate.id, pirate.ship.pirate_id
     assert_equal pirate.id, ships(:black_pearl).reload.pirate_id