Merge pull request rails#51788 from fatkodima/batch_enumerator-destroy_all-returns-rows-number

Change `BatchEnumerator#destroy_all` to return the total number of affected rows

Author: kamipo

activerecord/CHANGELOG.md

@@ -1,2 +1,7 @@
+*   Change `BatchEnumerator#destroy_all` to return the total number of affected rows.
+
+    Previously, it always returned `nil`.
+
+    *fatkodima*
 
 Please check [7-2-stable](https://github.com/rails/rails/blob/7-2-stable/activerecord/CHANGELOG.md) for previous changes.

activerecord/lib/active_record/relation/batches/batch_enumerator.rb

@@ -88,13 +88,15 @@ def touch_all(...)
         end
       end
 
-      # Destroys records in batches.
+      # Destroys records in batches. Returns the total number of rows affected.
       #
       #   Person.where("age < 10").in_batches.destroy_all
       #
       # See Relation#destroy_all for details of how each batch is destroyed.
       def destroy_all
-        each(&:destroy_all)
+        sum do |relation|
+          relation.destroy_all.count(&:destroyed?)
+        end
       end
 
       # Yields an ActiveRecord::Relation object for each batch of records.

activerecord/test/cases/batches_test.rb

@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require "cases/helper"
-require "models/comment"
 require "models/post"
 require "models/subscriber"
 require "models/developer"
@@ -416,6 +415,22 @@ def test_in_batches_delete_all_returns_zero_when_no_batches
     assert_equal 0, Post.where("1=0").in_batches(of: 2).delete_all
   end
 
+  def test_in_batches_destroy_all_should_not_destroy_records_in_other_batches
+    not_destroyed_count = Post.where("id <= 2").count
+    Post.where("id > 2").in_batches(of: 2).destroy_all
+    assert_equal 0, Post.where("id > 2").count
+    assert_equal not_destroyed_count, Post.count
+  end
+
+  def test_in_batches_destroy_all_returns_rows_affected
+    # 1 records is not destroyed because of the callback.
+    assert_equal 10, PostWithDestroyCallback.in_batches(of: 2).destroy_all
+  end
+
+  def test_in_batches_destroy_all_returns_zero_when_no_batches
+    assert_equal 0, Post.where("1=0").in_batches(of: 2).destroy_all
+  end
+
   def test_in_batches_should_not_be_loaded
     Post.in_batches(of: 1) do |relation|
       assert_not_predicate relation, :loaded?

activerecord/test/models/post.rb

@@ -1,5 +1,10 @@
 # frozen_string_literal: true
 
+require "models/tag"
+require "models/tagging"
+require "models/comment"
+require "models/category"
+
 class Post < ActiveRecord::Base
   class CategoryPost < ActiveRecord::Base
     self.table_name = "categories_posts"
@@ -329,6 +334,15 @@ class ConditionalStiPost < Post
 class SubConditionalStiPost < ConditionalStiPost
 end
 
+class PostWithDestroyCallback < ActiveRecord::Base
+  self.inheritance_column = :disabled
+  self.table_name = "posts"
+
+  before_destroy do
+    throw :abort if id == 1
+  end
+end
+
 class FakeKlass
   extend ActiveRecord::Delegation::DelegateCache