From 00f1cd71a97bfa79e6b62a870b09ca914c48e421 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav.marohnic@gmail.com>
Date: Thu, 8 Jul 2010 13:58:23 +0200
Subject: [PATCH] fix ActiveRecord `destroy_all` so it returns destroyed
 records

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 .../associations/association_collection.rb     |  7 ++++---
 activerecord/lib/active_record/relation.rb     |  3 +--
 .../associations/has_many_associations_test.rb |  7 +++++--
 activerecord/test/cases/base_test.rb           | 18 +++++++++++++-----
 4 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index a4e08c7d41..615b7d2719 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -253,9 +253,10 @@ module ActiveRecord
       # See destroy for more info.
       def destroy_all
         load_target
-        destroy(@target)
-        reset_target!
-        reset_named_scopes_cache!
+        destroy(@target).tap do
+          reset_target!
+          reset_named_scopes_cache!
+        end
       end
 
       def create(attrs = {})
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index bc708b573f..d9fc1b4940 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -213,8 +213,7 @@ module ActiveRecord
       if conditions
         where(conditions).destroy_all
       else
-        to_a.each {|object| object.destroy}
-        reset
+        to_a.each {|object| object.destroy }.tap { reset }
       end
     end
 
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 5e3ba778f3..a52cedd8c2 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -817,8 +817,11 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_destroy_all
     force_signal37_to_load_all_clients_of_firm
-    assert !companies(:first_firm).clients_of_firm.empty?, "37signals has clients after load"
-    companies(:first_firm).clients_of_firm.destroy_all
+    clients = companies(:first_firm).clients_of_firm.to_a
+    assert !clients.empty?, "37signals has clients after load"
+    destroyed = companies(:first_firm).clients_of_firm.destroy_all
+    assert_equal clients.sort_by(&:id), destroyed.sort_by(&:id)
+    assert destroyed.all? { |client| client.frozen? }, "destroyed clients should be frozen"
     assert companies(:first_firm).clients_of_firm.empty?, "37signals has no clients after destroy all"
     assert companies(:first_firm).clients_of_firm(true).empty?, "37signals has no clients after destroy all and refresh"
   end
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 5a72e9c6e0..329ca38f99 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -651,16 +651,24 @@ class BasicsTest < ActiveRecord::TestCase
   end
 
   def test_destroy_all
-    original_count = Topic.count
-    topics_by_mary = Topic.count(:conditions => mary = "author_name = 'Mary'")
+    conditions = "author_name = 'Mary'"
+    topics_by_mary = Topic.all(:conditions => conditions, :order => 'id')
+    assert ! topics_by_mary.empty?
 
-    Topic.destroy_all mary
-    assert_equal original_count - topics_by_mary, Topic.count
+    assert_difference('Topic.count', -topics_by_mary.size) do
+      destroyed = Topic.destroy_all(conditions).sort_by(&:id)
+      assert_equal topics_by_mary, destroyed
+      assert destroyed.all? { |topic| topic.frozen? }, "destroyed topics should be frozen"
+    end
   end
 
   def test_destroy_many
+    clients = Client.find([2, 3], :order => 'id')
+
     assert_difference('Client.count', -2) do
-      Client.destroy([2, 3])
+      destroyed = Client.destroy([2, 3]).sort_by(&:id)
+      assert_equal clients, destroyed
+      assert destroyed.all? { |client| client.frozen? }, "destroyed clients should be frozen"
     end
   end