From 68067b5e8d26bdb7a519ae098b04d8d9935e6c4f Mon Sep 17 00:00:00 2001 From: Matt Jones Date: Sat, 7 Nov 2009 14:00:54 -0500 Subject: [PATCH] delete correct records for a has_many with :primary_key and :dependent => :delete_all --- activerecord/lib/active_record/associations.rb | 2 +- .../associations/has_many_associations_test.rb | 12 ++++++++++++ activerecord/test/cases/reflection_test.rb | 4 ++-- activerecord/test/models/company.rb | 2 ++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 03c8d4b..3a5f3ed 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1480,7 +1480,7 @@ module ActiveRecord if reflection.options.include?(:dependent) # Add polymorphic type if the :as option is present dependent_conditions = [] - dependent_conditions << "#{reflection.primary_key_name} = \#{record.quoted_id}" + dependent_conditions << "#{reflection.primary_key_name} = \#{record.#{reflection.name}.send(:owner_quoted_id)}" dependent_conditions << "#{reflection.options[:as]}_type = '#{base_class.name}'" if reflection.options[:as] dependent_conditions << sanitize_sql(reflection.options[:conditions], reflection.quoted_table_name) if reflection.options[:conditions] dependent_conditions << extra_conditions if extra_conditions diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index b193f8d..86d14c9 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -659,6 +659,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, Client.find_all_by_client_of(firm.id).size end + def test_delete_all_association_with_primary_key_deletes_correct_records + firm = Firm.find(:first) + # break the vanilla firm_id foreign key + assert_equal 2, firm.clients.count + firm.clients.first.update_attribute(:firm_id, nil) + assert_equal 1, firm.clients(true).count + assert_equal 1, firm.clients_using_primary_key_with_delete_all.count + old_record = firm.clients_using_primary_key_with_delete_all.first + firm = Firm.find(:first) + firm.destroy + assert Client.find_by_id(old_record.id).nil? + end def test_creation_respects_hash_condition ms_client = companies(:first_firm).clients_like_ms_with_hash_conditions.build diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 99e2487..acd214e 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -176,8 +176,8 @@ class ReflectionTest < ActiveRecord::TestCase def test_reflection_of_all_associations # FIXME these assertions bust a lot - assert_equal 36, Firm.reflect_on_all_associations.size - assert_equal 26, Firm.reflect_on_all_associations(:has_many).size + assert_equal 37, Firm.reflect_on_all_associations.size + assert_equal 27, Firm.reflect_on_all_associations(:has_many).size assert_equal 10, Firm.reflect_on_all_associations(:has_one).size assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 469f539..7e93fda 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -68,6 +68,8 @@ class Firm < Company has_many :readonly_clients, :class_name => 'Client', :readonly => true has_many :clients_using_primary_key, :class_name => 'Client', :primary_key => 'name', :foreign_key => 'firm_name' + has_many :clients_using_primary_key_with_delete_all, :class_name => 'Client', + :primary_key => 'name', :foreign_key => 'firm_name', :dependent => :delete_all has_many :clients_grouped_by_firm_id, :class_name => "Client", :group => "firm_id", :select => "firm_id" has_many :clients_grouped_by_name, :class_name => "Client", :group => "name", :select => "name" -- 1.5.3.1