From e666139e793ac260c18905bec1b87ad0a951ac75 Mon Sep 17 00:00:00 2001 From: Matt Jones Date: Tue, 3 Nov 2009 23:01:06 -0500 Subject: [PATCH] delete correct records for a has_many with :primary_key and :dependent => :delete_all --- activerecord/lib/active_record/associations.rb | 2 +- activerecord/lib/active_record/reflection.rb | 4 ++++ .../associations/has_many_associations_test.rb | 12 ++++++++++++ activerecord/test/cases/reflection_test.rb | 4 ++-- activerecord/test/models/company.rb | 2 ++ 5 files changed, 21 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 03c8d4b..482e33e 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.class.connection.quote(record.send(reflection.owner_primary_key_name), record.column_for_attribute(reflection.owner_primary_key_name))}" 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/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index db5d2b2..ea2ea84 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -189,6 +189,10 @@ module ActiveRecord @primary_key_name ||= options[:foreign_key] || derive_primary_key_name end + def owner_primary_key_name + @owner_primary_key_name ||= options[:primary_key] || active_record.primary_key + end + def association_foreign_key @association_foreign_key ||= @options[:association_foreign_key] || class_name.foreign_key end 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