Skip to content

Commit

Permalink
Change BatchEnumerator#destroy_all to return the total number of af…
Browse files Browse the repository at this point in the history
…fected rows
  • Loading branch information
fatkodima committed May 13, 2024
1 parent 8eae753 commit 94af7c1
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 3 deletions.
5 changes: 5 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
17 changes: 16 additions & 1 deletion activerecord/test/cases/batches_test.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require "cases/helper"
require "models/comment"
require "models/post"
require "models/subscriber"
require "models/developer"
Expand Down Expand Up @@ -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?
Expand Down
14 changes: 14 additions & 0 deletions activerecord/test/models/post.rb
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 94af7c1

Please sign in to comment.