From 0ebefff96447a17c07e5724688d5759b71bdf64b Mon Sep 17 00:00:00 2001 From: Will Bryant Date: Tue, 7 Sep 2010 21:33:35 +1200 Subject: [PATCH] Don't add non-new records back to the target array after loading targets on associations, as that makes destroy_all destroy any created records that don't match the scope destroy_all is called on --- .../associations/association_collection.rb | 2 +- .../associations/has_many_associations_test.rb | 11 +++++++++++ activerecord/test/models/company.rb | 2 ++ 3 files changed, 14 insertions(+), 1 deletions(-) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 0953fa5..320b1f0 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -360,7 +360,7 @@ module ActiveRecord else f end - end + @target + end + @target.find_all {|t| t.new_record?} else @target = find_target end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 59f7a03..bf47ef7 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -843,6 +843,17 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert companies(:first_firm).clients_of_firm(true).empty?, "37signals has no clients after destroy all and refresh" end + def test_destroy_all_with_creates_and_scope_that_doesnt_match_created_records + company = companies(:first_firm) + unloaded_client_matching_scope = companies(:second_client) + created_client_matching_scope = company.clients_of_firm.create!(:name => "Somesoft") + created_client_not_matching_scope = company.clients_of_firm.create!(:name => "OtherCo") + destroyed = company.clients_of_firm.with_oft_in_name.destroy_all + assert destroyed.include?(unloaded_client_matching_scope), "unloaded clients matching the scope destroy_all on should have been destroyed" + assert destroyed.include?(created_client_matching_scope), "loaded clients matching the scope destroy_all on should have been destroyed" + assert !destroyed.include?(created_client_not_matching_scope), "loaded clients not matching the scope destroy_all on should not have been destroyed" + end + def test_dependence firm = companies(:first_firm) assert_equal 2, firm.clients.size diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index de1a1cc..a849f48 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -11,6 +11,8 @@ class Company < AbstractCompany has_one :dummy_account, :foreign_key => "firm_id", :class_name => "Account" has_many :contracts has_many :developers, :through => :contracts + + named_scope :with_oft_in_name, :conditions => "name LIKE '%oft%'" def arbitrary_method "I am Jack's profound disappointment" -- 1.7.0