From 84dee04214ceb1bd6b37540ac4e32d104f8f8061 Mon Sep 17 00:00:00 2001
From: Joshua Young <djry1999@gmail.com>
Date: Sun, 11 Feb 2024 20:35:36 +1000
Subject: [PATCH] [Fix #51044] Raise an ArgumentError when providing
 non-boolean arguments to distinct

---
 activerecord/CHANGELOG.md                            |  4 ++++
 .../lib/active_record/relation/query_methods.rb      |  4 ++++
 activerecord/test/cases/relation/mutation_test.rb    | 12 +++++++++---
 activerecord/test/cases/relations_test.rb            |  6 ++++++
 4 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 0374d86a69314..5079aaefe373b 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Deprecate passing a non-boolean argument to `#distinct`.
+
+    *Joshua Young*
+
 *   Fix an issue where `ActiveRecord::Encryption` configurations are not ready before the loading
     of Active Record models, when an application is eager loaded. As a result, encrypted attributes
     could be misconfigured in some cases.
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index b9dc7392323c1..1b79f37ddc7c3 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -1312,6 +1312,10 @@ def distinct(value = true)
 
     # Like #distinct, but modifies relation in place.
     def distinct!(value = true) # :nodoc:
+      unless value == true || value == false
+        ActiveRecord.deprecator.warn("Passing a non-boolean value to distinct is deprecated and will raise in Rails 7.3")
+      end
+
       self.distinct_value = value
       self
     end
diff --git a/activerecord/test/cases/relation/mutation_test.rb b/activerecord/test/cases/relation/mutation_test.rb
index d1c5fd3c33256..9f9a7d5c07544 100644
--- a/activerecord/test/cases/relation/mutation_test.rb
+++ b/activerecord/test/cases/relation/mutation_test.rb
@@ -52,7 +52,7 @@ class RelationMutationTest < ActiveRecord::TestCase
       assert_equal [], relation.extending_values
     end
 
-    (Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with, :skip_query_cache, :strict_loading]).each do |method|
+    (Relation::SINGLE_VALUE_METHODS - [:lock, :reordering, :reverse_order, :create_with, :skip_query_cache, :strict_loading, :distinct]).each do |method|
       test "##{method}!" do
         assert relation.public_send("#{method}!", :foo).equal?(relation)
         assert_equal :foo, relation.public_send("#{method}_value")
@@ -121,8 +121,14 @@ class RelationMutationTest < ActiveRecord::TestCase
     end
 
     test "distinct!" do
-      relation.distinct! :foo
-      assert_equal :foo, relation.distinct_value
+      assert relation.distinct!(true).equal?(relation)
+      assert_equal true, relation.distinct_value
+    end
+
+    test "distinct! with a non-boolean value argument" do
+      assert_deprecated(ActiveRecord.deprecator) do
+        relation.distinct!(:name)
+      end
     end
 
     test "skip_query_cache!" do
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 448aee9bdae03..479f973a0c1f8 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -1795,6 +1795,12 @@ def test_distinct
     assert_equal ["Foo", "Foo"], query.distinct(true).distinct(false).map(&:name)
   end
 
+  def test_distinct_with_a_non_boolean_value_argument
+    assert_deprecated(ActiveRecord.deprecator) do
+      Tag.distinct(:name)
+    end
+  end
+
   def test_doesnt_add_having_values_if_options_are_blank
     scope = Post.having("")
     assert_empty scope.having_clause