Permalink
Browse files

When assigning a has_one, if the existing record fails to be removed …

…from the association, raise an error
  • Loading branch information...
1 parent c47c541 commit 1bc71ed9607ba88c21c14b670a4308c8549f3941 @jonleighton jonleighton committed with tenderlove Jan 8, 2011
View
16 activerecord/lib/active_record/associations/has_one_association.rb
@@ -20,7 +20,7 @@ def replace(record, save = true)
load_target
if @target && @target != record
- remove_target(@reflection.options[:dependent])
+ remove_target!(@reflection.options[:dependent])
end
if record
@@ -55,13 +55,19 @@ def new_record(method, attributes)
record
end
- def remove_target(method)
- case method
- when :delete, :destroy
+ def remove_target!(method)
+ if [:delete, :destroy].include?(method)
@target.send(method)
else
@target[@reflection.foreign_key] = nil
- @target.save if @target.persisted? && @owner.persisted?
+
+ if @target.persisted? && @owner.persisted?
+ unless @target.save
+ @target[@reflection.foreign_key] = @target.send("#{@reflection.foreign_key}_was")
+ raise RecordNotSaved, "Failed to remove the existing associated #{@reflection.name}. " +
+ "The record failed to save when after its foreign key was set to nil."
+ end
+ end
end
end
end
View
12 activerecord/test/cases/associations/has_one_associations_test.rb
@@ -308,4 +308,16 @@ def test_creation_failure_with_dependent_option
assert new_ship.new_record?
assert orig_ship.destroyed?
end
+
+ def test_replacement_failure_due_to_existing_record_should_raise_error
+ pirate = pirates(:blackbeard)
+ pirate.ship.name = nil
+
+ assert !pirate.ship.valid?
+ 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
+ end
end

0 comments on commit 1bc71ed

Please sign in to comment.