Deprecate read_attribute(:id)
returning the primary key
#49019
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation / Background
This PR 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)
should 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.read_attribute
should not translate "id" to mean "primary key" -- this method ought to return the raw attribute value, so that applications have an easy way to access theid
attribute on a model, regardless of whether that is the model's primary key.Detail
This pull request changes
read_attribute
to raise a deprecation warning if we are reading:id
, butid
is not the primary key. Additionally, it changes the behaviour for composite primary key models to start returning the id column value. Note that this will also result incpk_record["id"]
returning the id column, rather than the composite primary key.I'm recommending
#id
be used instead. There have been other discussions about deprecating#id
returning the primary key (example), but that would require introducing a new getter method for retrieving the PK value. As it stands, those changes are a long ways off 😄Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]