Merge pull request #29262 from kamipo/deprecate_passing_arguments_wit…

…h_block_to_count_and_sum

Deprecate passing arguments and block at the same time to `count` and `sum` in `ActiveRecord::Calculations`

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Deprecate passing arguments and block at the same time to `count` and `sum` in `ActiveRecord::Calculations`.
+
+    *Ryuta Kamizono*
+
 *   Loading model schema from database is now thread-safe.
 
     Fixes #28589.

activerecord/lib/active_record/relation/calculations.rb

@@ -37,7 +37,16 @@ module Calculations
     # Note: not all valid {Relation#select}[rdoc-ref:QueryMethods#select] expressions are valid #count expressions. The specifics differ
     # between databases. In invalid cases, an error from the database is thrown.
     def count(column_name = nil)
-      return super() if block_given?
+      if block_given?
+        unless column_name.nil?
+          ActiveSupport::Deprecation.warn \
+            "When `count' is called with a block, it ignores other arguments. " \
+            "This behavior is now deprecated and will result in an ArgumentError in Rails 5.3."
+        end
+
+        return super()
+      end
+
       calculate(:count, column_name)
     end
 
@@ -73,7 +82,16 @@ def maximum(column_name)
     #
     #   Person.sum(:age) # => 4562
     def sum(column_name = nil)
-      return super() if block_given?
+      if block_given?
+        unless column_name.nil?
+          ActiveSupport::Deprecation.warn \
+            "When `sum' is called with a block, it ignores other arguments. " \
+            "This behavior is now deprecated and will result in an ArgumentError in Rails 5.3."
+        end
+
+        return super()
+      end
+
       calculate(:sum, column_name)
     end
 

activerecord/test/cases/calculations_test.rb

@@ -809,4 +809,16 @@ def permit!
   def test_group_by_attribute_with_custom_type
     assert_equal({ "proposed" => 2, "published" => 2 }, Book.group(:status).count)
   end
+
+  def test_deprecate_count_with_block_and_column_name
+    assert_deprecated do
+      assert_equal 6, Account.count(:firm_id) { true }
+    end
+  end
+
+  def test_deprecate_sum_with_block_and_column_name
+    assert_deprecated do
+      assert_equal 6, Account.sum(:firm_id) { 1 }
+    end
+  end
 end