Skip to content
Browse files

has_one should always remove the old record (properly), even if not s…

…aving the new record, so we don't get the database into a pickle
  • Loading branch information...
1 parent 665880c commit c6e10b0f600a56e962ff7d1614c50fca20630379 @jonleighton jonleighton committed with tenderlove Jan 7, 2011
View
2 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(save && @reflection.options[:dependent])
+ remove_target(@reflection.options[:dependent])
end
if record
View
35 activerecord/test/cases/associations/has_one_associations_test.rb
@@ -2,9 +2,11 @@
require 'models/developer'
require 'models/project'
require 'models/company'
+require 'models/ship'
+require 'models/pirate'
class HasOneAssociationsTest < ActiveRecord::TestCase
- fixtures :accounts, :companies, :developers, :projects, :developers_projects
+ fixtures :accounts, :companies, :developers, :projects, :developers_projects, :ships, :pirates
def setup
Account.destroyed_account_ids.clear
@@ -164,15 +166,6 @@ def test_successful_build_association
assert_equal account, firm.account
end
- def test_build_association_twice_without_saving_affects_nothing
- count_of_account = Account.count
- firm = Firm.find(:first)
- firm.build_account("credit_limit" => 1000)
- firm.build_account("credit_limit" => 2000)
-
- assert_equal count_of_account, Account.count
- end
-
def test_create_association
firm = Firm.create(:name => "GlobalMegaCorp")
account = firm.create_account(:credit_limit => 1000)
@@ -293,4 +286,26 @@ def test_attributes_are_being_set_when_initialized_from_has_one_association_with
new_account = companies(:first_firm).build_account(:firm_name => 'Account')
assert_equal new_account.firm_name, "Account"
end
+
+ def test_creation_failure_without_dependent_option
+ pirate = pirates(:blackbeard)
+ orig_ship = pirate.ship.target
+
+ assert_equal ships(:black_pearl), orig_ship
+ new_ship = pirate.create_ship
+ assert_not_equal ships(:black_pearl), new_ship
+ assert_equal new_ship, pirate.ship
+ assert new_ship.new_record?
+ assert_nil orig_ship.pirate_id
+ assert !orig_ship.changed? # check it was saved
+ end
+
+ def test_creation_failure_with_dependent_option
+ pirate = pirates(:blackbeard).becomes(DestructivePirate)
+ orig_ship = pirate.dependent_ship.target
+
+ new_ship = pirate.create_dependent_ship
+ assert new_ship.new_record?
+ assert orig_ship.destroyed?
+ end
end
View
1 activerecord/test/fixtures/ships.yml
@@ -1,5 +1,6 @@
black_pearl:
name: "Black Pearl"
+ pirate: blackbeard
interceptor:
id: 2
name: "Interceptor"
View
4 activerecord/test/models/pirate.rb
@@ -78,3 +78,7 @@ def log(record, callback)
ship_log << "#{callback}_#{record.class.name.downcase}_#{record.id || '<new>'}"
end
end
+
+class DestructivePirate < Pirate
+ has_one :dependent_ship, :class_name => 'Ship', :foreign_key => :pirate_id, :dependent => :destroy
+end

0 comments on commit c6e10b0

Please sign in to comment.
Something went wrong with that request. Please try again.