Skip to content

Commit

Permalink
Disallow id as an enum value in Active Record
Browse files Browse the repository at this point in the history
Fixes #48524

The test case in the issue breaks because `value.respond_to?(:id)` returns true [here](https://github.com/rails/rails/blob/51f2e2f80b86e0c3769cf5272b7d97035730f19d/activerecord/lib/active_record/relation/predicate_builder.rb#L58). This effectively adds a default scope to queries where it shouldn't.

There might be a way to fix this in Active Record but I'd be surprised if nothing else breaks from defining `id` instance and class methods. I think it is simpler to not allow it as a value since it really should be treated as a reserved method.
  • Loading branch information
ghiculescu committed Jun 20, 2023
1 parent 51f2e2f commit a22b942
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
2 changes: 2 additions & 0 deletions activerecord/lib/active_record/enum.rb
Expand Up @@ -322,6 +322,8 @@ def detect_enum_conflict!(enum_name, method_name, klass_method = false)
raise_conflict_error(enum_name, method_name, type: "class")
elsif klass_method && method_defined_within?(method_name, Relation)
raise_conflict_error(enum_name, method_name, type: "class", source: Relation.name)
elsif klass_method && method_name == primary_key
raise_conflict_error(enum_name, method_name)
elsif !klass_method && dangerous_attribute_method?(method_name)
raise_conflict_error(enum_name, method_name)
elsif !klass_method && method_defined_within?(method_name, _enum_methods_module, Module)
Expand Down
13 changes: 12 additions & 1 deletion activerecord/test/cases/enum_test.rb
Expand Up @@ -497,7 +497,8 @@ class EnumTest < ActiveRecord::TestCase
:save, # generates #save!, which conflicts with an AR method
:proposed, # same value as an existing enum
:public, :private, :protected, # some important methods on Module and Class
:name, :parent, :superclass
:name, :parent, :superclass,
:id # conflicts with AR querying
]

conflicts.each_with_index do |value, i|
Expand All @@ -508,6 +509,16 @@ class EnumTest < ActiveRecord::TestCase
end
end

test "can use id as a value with a prefix or suffix" do
assert_nothing_raised do
Class.new(ActiveRecord::Base) do
self.table_name = "books"
enum status_1: [:id], _prefix: true
enum status_2: [:id], _suffix: true
end
end
end

test "reserved enum values for relation" do
relation_method_samples = [
:records,
Expand Down

0 comments on commit a22b942

Please sign in to comment.