Skip to content

Commit

Permalink
Fix a bug where AR::RecordNotSaved loses error messages
Browse files Browse the repository at this point in the history
Since 3e30c5d, it started ignoring the given error message. This commit
changes the behavior of AR::RecordNotSaved#initialize so that it no
longer loses the given error message.
  • Loading branch information
yuki24 committed Nov 28, 2014
1 parent 200b903 commit 5142d54
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 12 deletions.
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/errors.rb
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/persistence.rb
Expand Up @@ -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
Expand Down
17 changes: 13 additions & 4 deletions activerecord/test/cases/associations/has_many_associations_test.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand Down
13 changes: 10 additions & 3 deletions activerecord/test/cases/associations/has_one_associations_test.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 5142d54

Please sign in to comment.