From 9a7740d9e58262b9a1f6b095f533eac90acdfec3 Mon Sep 17 00:00:00 2001 From: Dimitri Krassovski Date: Mon, 10 Nov 2008 19:21:30 +0200 Subject: [PATCH] A test to show the problem --- .../associations/has_one_associations_test.rb | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 14032a6..0902852 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -79,6 +79,14 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_raises(ActiveRecord::RecordNotFound) { Account.find(old_account_id) } end + def test_nullification_on_association_change + firm = companies(:rails_core) + old_account_id = firm.account.id + firm.account = Account.new + # account is dependent with nullify, therefore its firm_id should be nil + assert_nil Account.find(old_account_id).firm_id + end + def test_natural_assignment_to_already_associated_record company = companies(:first_firm) account = accounts(:signals37) -- 1.5.3.8 From e40024c290ec1b3cc8bfe3d7ed78aa4ee3948fbc Mon Sep 17 00:00:00 2001 From: Dimitri Krassovski Date: Mon, 10 Nov 2008 19:52:09 +0200 Subject: [PATCH] Fix the case when has_one association is destroyed instead of being nullified --- .../associations/has_one_association.rb | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 9603230..a29fbf6 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -28,7 +28,7 @@ module ActiveRecord load_target unless @target.nil? || @target == obj - if dependent? && !dont_save + if dependent? && !dont_save && (@reflection.options[:dependent] != :nullify) @target.destroy unless @target.new_record? @owner.clear_association_cache else -- 1.5.3.8 From 37d11efb2fe39cd42fa97793d8be101d09a7dfb0 Mon Sep 17 00:00:00 2001 From: Dimitri Krassovski Date: Mon, 10 Nov 2008 21:06:37 +0200 Subject: [PATCH] Call destroy or delete respecting the :dependent option on the has_one association --- .../associations/has_one_association.rb | 15 ++++++++++++--- .../associations/has_one_associations_test.rb | 14 ++++++++++++++ activerecord/test/models/company.rb | 1 + 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index a29fbf6..b92cbbd 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -28,9 +28,18 @@ module ActiveRecord load_target unless @target.nil? || @target == obj - if dependent? && !dont_save && (@reflection.options[:dependent] != :nullify) - @target.destroy unless @target.new_record? - @owner.clear_association_cache + if dependent? && !dont_save + case @reflection.options[:dependent] + when :delete + @target.delete unless @target.new_record? + @owner.clear_association_cache + when :destroy + @target.destroy unless @target.new_record? + @owner.clear_association_cache + when :nullify + @target[@reflection.primary_key_name] = nil + @target.save unless @owner.new_record? || @target.new_record? + end else @target[@reflection.primary_key_name] = nil @target.save unless @owner.new_record? || @target.new_record? diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 0902852..37567d0 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -86,6 +86,20 @@ class HasOneAssociationsTest < ActiveRecord::TestCase # account is dependent with nullify, therefore its firm_id should be nil assert_nil Account.find(old_account_id).firm_id end + + uses_mocha "association should recieve delete" do + def test_association_changecalls_delete + Account.any_instance.expects(:delete) + companies(:first_firm).deletable_account = Account.new + end + end + + uses_mocha "association should recieve destroy" do + def test_association_change_calls_destroy + Account.any_instance.expects(:destroy) + companies(:first_firm).account = Account.new + end + end def test_natural_assignment_to_already_associated_record company = companies(:first_firm) diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 0e3fafa..e431175 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -69,6 +69,7 @@ class Firm < Company has_one :account_with_select, :foreign_key => "firm_id", :select => "id, firm_id", :class_name=>'Account' has_one :readonly_account, :foreign_key => "firm_id", :class_name => "Account", :readonly => true has_one :account_using_primary_key, :primary_key => "firm_id", :class_name => "Account" + has_one :deletable_account, :foreign_key => "firm_id", :class_name => "Account", :dependent => :delete end class DependentFirm < Company -- 1.5.3.8