Skip to content

Commit ac83311

Browse files
Deprecate read_attribute(:id) returning the primary key
This commit deprecates `read_attribute(:id)` returning the primary key if the model's primary key is not the id column. Starting in Rails 7.2, `read_attribute(:id)` will always return the value of the id column. This commit also changes `read_attribute(:id)` for composite primary key models to return the value of the id column, not the composite primary key.
1 parent dd6d931 commit ac83311

File tree

4 files changed

+35
-15
lines changed

4 files changed

+35
-15
lines changed

activerecord/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
* Deprecate `read_attribute(:id)` returning the primary key if the primary key is not `:id`.
2+
3+
Starting in Rails 7.2, `read_attribute(:id)` will return the value of the id column, regardless of the model's
4+
primary key. To retrieve the value of the primary key, use `#id` instead. `read_attribute(:id)` for composite
5+
primary key models will now return the value of the id column.
6+
7+
*Adrianna Chang*
8+
19
* Fix `change_table` setting datetime precision for 6.1 Migrations
210

311
*Hartley McGuire*

activerecord/lib/active_record/associations/collection_association.rb

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -333,11 +333,7 @@ def merge_target_lists(persisted, memory)
333333
if mem_record = memory.delete(record)
334334

335335
((record.attribute_names & mem_record.attribute_names) - mem_record.changed_attribute_names_to_save - mem_record.class._attr_readonly).each do |name|
336-
if name == "id" && mem_record.class.composite_primary_key?
337-
mem_record.class.primary_key.zip(record[name]) { |attr, value| mem_record._write_attribute(attr, value) }
338-
else
339-
mem_record._write_attribute(name, record[name])
340-
end
336+
mem_record._write_attribute(name, record[name])
341337
end
342338

343339
mem_record

activerecord/lib/active_record/attribute_methods/read.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,14 @@ def read_attribute(attr_name, &block)
3333
return @attributes.fetch_value(name, &block) unless name == "id" && @primary_key
3434

3535
if self.class.composite_primary_key?
36-
@primary_key.map { |col| @attributes.fetch_value(col, &block) }
36+
@attributes.fetch_value("id", &block)
3737
else
38+
if @primary_key != "id"
39+
ActiveRecord.deprecator.warn(<<-MSG.squish)
40+
Using read_attribute(:id) to read the primary key value is deprecated.
41+
Use #id instead.
42+
MSG
43+
end
3844
@attributes.fetch_value(@primary_key, &block)
3945
end
4046
end

activerecord/test/cases/primary_keys_test.rb

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,32 @@ def test_to_key_with_composite_primary_key
3636
assert_equal [1, 2], order.to_key
3737
end
3838

39+
def test_read_attribute_id
40+
topic = Topic.find(1)
41+
id = assert_not_deprecated(ActiveRecord.deprecator) do
42+
topic.read_attribute(:id)
43+
end
44+
45+
assert_equal 1, id
46+
end
47+
3948
def test_read_attribute_with_custom_primary_key
4049
keyboard = Keyboard.create!
41-
assert_equal keyboard.key_number, keyboard.read_attribute(:id)
50+
msg = "Using read_attribute(:id) to read the primary key value is deprecated. Use #id instead."
51+
id = assert_deprecated(msg, ActiveRecord.deprecator) do
52+
keyboard.read_attribute(:id)
53+
end
54+
55+
assert_equal keyboard.key_number, id
4256
end
4357

4458
def test_read_attribute_with_composite_primary_key
4559
book = Cpk::Book.new(id: [1, 2])
46-
assert_equal [1, 2], book.read_attribute(:id)
47-
end
48-
49-
def test_read_attribute_with_composite_primary_key_and_column_named_id
50-
order = Cpk::Order.new
51-
order.id = [1, 2]
60+
id = assert_not_deprecated(ActiveRecord.deprecator) do
61+
book.read_attribute(:id)
62+
end
5263

53-
assert_equal [1, 2], order.read_attribute(:id)
54-
assert_equal 2, order.attributes["id"]
64+
assert_equal 2, id
5565
end
5666

5767
def test_to_key_with_primary_key_after_destroy

0 commit comments

Comments
 (0)