From 4d7354aa352b94ec3a10ad053cff04890c534080 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Tue, 4 Dec 2018 14:33:31 +0900
Subject: Ensure that `delete_all` on collection proxy returns affected count
Unlike the `Relation#delete_all`, `delete_all` on collection proxy
doesn't return affected count. Since the `CollectionProxy` is a subclass
of the `Relation`, this inconsistency is probably not intended, so it
should return the count consistently.
---
activerecord/CHANGELOG.md | 4 ++++
.../active_record/associations/has_many_association.rb | 1 +
.../associations/has_many_through_association.rb | 2 ++
.../cases/associations/has_many_associations_test.rb | 16 +++++++++++++---
.../associations/has_many_through_associations_test.rb | 6 +++---
5 files changed, 23 insertions(+), 6 deletions(-)
(limited to 'activerecord')
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index d9488bd0f0..ba1b9a304f 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+* Ensure that `delete_all` on collection proxy returns affected count.
+
+ *Ryuta Kamizono*
+
* Add the ability to prevent writes to a database for the duration of a block.
Allows the application to prevent writes to a database. This can be useful when
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index e224d3456a..f6fdbcde54 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -99,6 +99,7 @@ module ActiveRecord
def delete_or_nullify_all_records(method)
count = delete_count(method, scope)
update_counter(-count)
+ count
end
# Deletes the records according to the <tt>:dependent</tt> option.
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 2322a49931..84a9797aa5 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -161,6 +161,8 @@ module ActiveRecord
else
update_counter(-count)
end
+
+ count
end
def difference(a, b)
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 6024a9e354..b4bb6e7227 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -264,7 +264,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
car = Car.create(name: "honda")
car.funky_bulbs.create!
assert_equal 1, car.funky_bulbs.count
- assert_nothing_raised { car.reload.funky_bulbs.delete_all }
+ assert_equal 1, car.reload.funky_bulbs.delete_all
assert_equal 0, car.funky_bulbs.count, "bulbs should have been deleted using :delete_all strategy"
end
@@ -1405,7 +1405,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
assert_equal 3, clients.count
assert_difference "Client.count", -(clients.count) do
- companies(:first_firm).dependent_clients_of_firm.delete_all
+ assert_equal clients.count, companies(:first_firm).dependent_clients_of_firm.delete_all
end
end
@@ -1502,10 +1502,20 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
def test_delete_all_with_option_delete_all
firm = companies(:first_firm)
client_id = firm.dependent_clients_of_firm.first.id
- firm.dependent_clients_of_firm.delete_all(:delete_all)
+ count = firm.dependent_clients_of_firm.count
+ assert_equal count, firm.dependent_clients_of_firm.delete_all(:delete_all)
assert_nil Client.find_by_id(client_id)
end
+ def test_delete_all_with_option_nullify
+ firm = companies(:first_firm)
+ client_id = firm.dependent_clients_of_firm.first.id
+ count = firm.dependent_clients_of_firm.count
+ assert_equal firm, Client.find(client_id).firm
+ assert_equal count, firm.dependent_clients_of_firm.delete_all(:nullify)
+ assert_nil Client.find(client_id).firm
+ end
+
def test_delete_all_accepts_limited_parameters
firm = companies(:first_firm)
assert_raise(ArgumentError) do
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index cf514957ca..3827729ac4 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -200,7 +200,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
assert_no_difference "Job.count" do
assert_difference "Reference.count", -1 do
- person.reload.jobs_with_dependent_destroy.delete_all
+ assert_equal 1, person.reload.jobs_with_dependent_destroy.delete_all
end
end
end
@@ -211,7 +211,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
assert_no_difference "Job.count" do
assert_no_difference "Reference.count" do
- person.reload.jobs_with_dependent_nullify.delete_all
+ assert_equal 1, person.reload.jobs_with_dependent_nullify.delete_all
end
end
end
@@ -222,7 +222,7 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
assert_no_difference "Job.count" do
assert_difference "Reference.count", -1 do
- person.reload.jobs_with_dependent_delete_all.delete_all
+ assert_equal 1, person.reload.jobs_with_dependent_delete_all.delete_all
end
end
end
--
cgit v1.2.3