Skip to content
This repository
Browse code

fix ActiveRecord `destroy_all` so it returns destroyed records

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
commit 00f1cd71a97bfa79e6b62a870b09ca914c48e421 1 parent 951dbf0
Mislav Marohnić mislav authored jeremy committed
7 activerecord/lib/active_record/associations/association_collection.rb
@@ -253,9 +253,10 @@ def clear
253 253 # See destroy for more info.
254 254 def destroy_all
255 255 load_target
256   - destroy(@target)
257   - reset_target!
258   - reset_named_scopes_cache!
  256 + destroy(@target).tap do
  257 + reset_target!
  258 + reset_named_scopes_cache!
  259 + end
259 260 end
260 261
261 262 def create(attrs = {})
3  activerecord/lib/active_record/relation.rb
@@ -213,8 +213,7 @@ def destroy_all(conditions = nil)
213 213 if conditions
214 214 where(conditions).destroy_all
215 215 else
216   - to_a.each {|object| object.destroy}
217   - reset
  216 + to_a.each {|object| object.destroy }.tap { reset }
218 217 end
219 218 end
220 219
7 activerecord/test/cases/associations/has_many_associations_test.rb
@@ -817,8 +817,11 @@ def test_destroying_a_collection
817 817
818 818 def test_destroy_all
819 819 force_signal37_to_load_all_clients_of_firm
820   - assert !companies(:first_firm).clients_of_firm.empty?, "37signals has clients after load"
821   - companies(:first_firm).clients_of_firm.destroy_all
  820 + clients = companies(:first_firm).clients_of_firm.to_a
  821 + assert !clients.empty?, "37signals has clients after load"
  822 + destroyed = companies(:first_firm).clients_of_firm.destroy_all
  823 + assert_equal clients.sort_by(&:id), destroyed.sort_by(&:id)
  824 + assert destroyed.all? { |client| client.frozen? }, "destroyed clients should be frozen"
822 825 assert companies(:first_firm).clients_of_firm.empty?, "37signals has no clients after destroy all"
823 826 assert companies(:first_firm).clients_of_firm(true).empty?, "37signals has no clients after destroy all and refresh"
824 827 end
20 activerecord/test/cases/base_test.rb
@@ -651,16 +651,24 @@ def test_table_name_guesses_with_inherited_prefixes_and_suffixes
651 651 end
652 652
653 653 def test_destroy_all
654   - original_count = Topic.count
655   - topics_by_mary = Topic.count(:conditions => mary = "author_name = 'Mary'")
656   -
657   - Topic.destroy_all mary
658   - assert_equal original_count - topics_by_mary, Topic.count
  654 + conditions = "author_name = 'Mary'"
  655 + topics_by_mary = Topic.all(:conditions => conditions, :order => 'id')
  656 + assert ! topics_by_mary.empty?
  657 +
  658 + assert_difference('Topic.count', -topics_by_mary.size) do
  659 + destroyed = Topic.destroy_all(conditions).sort_by(&:id)
  660 + assert_equal topics_by_mary, destroyed
  661 + assert destroyed.all? { |topic| topic.frozen? }, "destroyed topics should be frozen"
  662 + end
659 663 end
660 664
661 665 def test_destroy_many
  666 + clients = Client.find([2, 3], :order => 'id')
  667 +
662 668 assert_difference('Client.count', -2) do
663   - Client.destroy([2, 3])
  669 + destroyed = Client.destroy([2, 3]).sort_by(&:id)
  670 + assert_equal clients, destroyed
  671 + assert destroyed.all? { |client| client.frozen? }, "destroyed clients should be frozen"
664 672 end
665 673 end
666 674

0 comments on commit 00f1cd7

Please sign in to comment.
Something went wrong with that request. Please try again.