Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not use alias_attribute to alias an association #48783

Merged

Conversation

nvasilevski
Copy link
Contributor

@nvasilevski nvasilevski commented Jul 21, 2023

For reference, the code I'm proposing to remove was added in #40835

Using alias_attribute to alias an association is not an indented use case. This commit removes alias_attribute calls to alias an association along with the relevant tests. However, it doesn't mean that the commit brakes any current behaviors.

Currently running Active Record tests result in a bunch of deprecation warnings issued related to alias_attribute :entry, :post definition.
Example:

DEPRECATION WARNING: Comment model aliases `post` and has a method called `post_changed?` defined. In Rails 7.2 `entry_changed?` will not call `post_changed?` anymore.

This is a falsey warning so in order to get rid of it we have two options:

  • Fix the code that issues the warning to account for alias_attribute being used to alias an association
  • Remove alias_attribute usages to alias an association so the warning goes away with them

Since aliasing an association using alias_attribute is not an intended use case I'm leaning towards removing it from the test suite.

Using `alias_attribute` to alias an association is not an indented
use case. This commit removes `alias_attribute` calls to alias an
association along with the relevant tests. However, it doesn't mean
that the commit brakes any current behaviors.
@nvasilevski nvasilevski force-pushed the do-not-use-alias-attribute-for-associations branch from 18ee82f to 686336f Compare July 24, 2023 14:45
@byroot
Copy link
Member

byroot commented Jul 24, 2023

the code I'm proposing to remove was added in #40835

Alright, it was added back because it used to work, but IMO that never was a feature and shouldn't have been fixed in the first place.

People running into this can do manual aliasing, e.g.

def topic
  community_topic
end

def topic=(topic)
  self.community_topic = topic
end

Or contribute an alias_association helper.

@byroot byroot merged commit 19c6614 into rails:main Jul 24, 2023
9 checks passed
@nvasilevski nvasilevski deleted the do-not-use-alias-attribute-for-associations branch July 24, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants