Skip to content

Commit

Permalink
[Fix #51044] Raise an ArgumentError when providing non-boolean argume…
Browse files Browse the repository at this point in the history
…nts to distinct
  • Loading branch information
joshuay03 committed Feb 13, 2024
1 parent a42ca9c commit 84dee04
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 3 deletions.
4 changes: 4 additions & 0 deletions 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.
Expand Down
4 changes: 4 additions & 0 deletions activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions activerecord/test/cases/relation/mutation_test.rb
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/cases/relations_test.rb
Expand Up @@ -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
Expand Down

0 comments on commit 84dee04

Please sign in to comment.