Browse files

When assigning a has_one, if anything fails, the assignment should be…

… rolled back entirely
  • Loading branch information...
1 parent 4e19ec5 commit 1d6e2184283d15d20ed3102ca462d905e5efa73d @jonleighton jonleighton committed with tenderlove Jan 9, 2011
View
42 activerecord/lib/active_record/associations/has_one_association.rb
@@ -19,23 +19,25 @@ def replace(record, save = true)
raise_on_type_mismatch(record) unless record.nil?
load_target
- if @target && @target != record
- remove_target!(@reflection.options[:dependent])
- end
+ @reflection.klass.transaction do
+ if @target && @target != record
+ remove_target!(@reflection.options[:dependent])
+ end
+
+ if record
+ set_inverse_instance(record)
+ set_owner_attributes(record)
- if record
- set_owner_attributes(record)
- set_inverse_instance(record)
+ if @owner.persisted? && save && !record.save
+ nullify_owner_attributes(record)
+ set_owner_attributes(@target)
+ raise RecordNotSaved, "Failed to save the new associated #{@reflection.name}."
+ end
+ end
end
@target = record
loaded
-
- if @owner.persisted? && record && save
- unless record.save
- raise RecordNotSaved, "Failed to save the new associated #{@reflection.name}."
- end
- end
end
private
@@ -59,17 +61,19 @@ def remove_target!(method)
if [:delete, :destroy].include?(method)
@target.send(method)
else
- @target[@reflection.foreign_key] = nil
+ nullify_owner_attributes(@target)
- 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
+ if @target.persisted? && @owner.persisted? && !@target.save
+ set_owner_attributes(@target)
+ 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
+
+ def nullify_owner_attributes(record)
+ record[@reflection.foreign_key] = nil
+ end
end
end
end
View
7 activerecord/test/cases/associations/has_one_associations_test.rb
@@ -6,6 +6,7 @@
require 'models/pirate'
class HasOneAssociationsTest < ActiveRecord::TestCase
+ self.use_transactional_fixtures = false unless supports_savepoints?
fixtures :accounts, :companies, :developers, :projects, :developers_projects, :ships, :pirates
def setup
@@ -317,7 +318,9 @@ def test_replacement_failure_due_to_new_record_should_raise_error
assert_raise(ActiveRecord::RecordNotSaved) do
pirate.ship = new_ship
end
- assert_equal new_ship, pirate.ship
- assert_equal pirate.id, new_ship.pirate_id
+ 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
+ assert_nil new_ship.pirate_id
end
end

0 comments on commit 1d6e218

Please sign in to comment.