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

Fix wrong sample code about allow_deprecated_singular_associations_name [skip ci] #45438

Conversation

willnet
Copy link
Contributor

@willnet willnet commented Jun 23, 2022

Summary

The only time #45163 and #45344 have an effect is when the hash value passed to where is a model object. The current sample code does not change behavior between Rails 7.0 and 7.1

…me [skip ci]

The only time rails#45163 and rails#45344 have an effect is when the hash value passed to `where` is a model object. The current sample code does not change behavior between Rails 7.0 and 7.1
@rails-bot rails-bot bot added the docs label Jun 23, 2022
Copy link
Member

@jonathanhefner jonathanhefner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only time #45163 and #45344 have an effect is when the hash value passed to where is a model object.

Your example is correct, but also applies when passing an ID:

class Post < ActiveRecord::Base
end

class Comment < ActiveRecord::Base
  belongs_to :post
end

ActiveRecord.allow_deprecated_singular_associations_name = false

Comment.where(post: 1).count
# => 3

Comment.where(posts: 1).count
# => no such column: comments.posts (ActiveRecord::StatementInvalid)

ActiveRecord.allow_deprecated_singular_associations_name = true

Comment.where(posts: 1).count
# DEPRECATION WARNING: In Rails 7.2, referring to singular associations by their plural name will be deprecated.
# To continue querying `post` use `posts` instead.
# You can get the new more performant behavior now by setting config.active_record.allow_deprecated_singular_associations_name = false
# => 3

Unless I am misunderstanding, the original example appears to be unaffected by allow_deprecated_singular_associations_name:

class AlsoPost < ActiveRecord::Base
  self.table_name = "posts"
end

class Comment < ActiveRecord::Base
  belongs_to :also_post, foreign_key: "post_id"
end

ActiveRecord.allow_deprecated_singular_associations_name = false

Comment.joins(:post).where(post: { id: 1 }).count
# => 3

Comment.joins(:post).where(posts: { id: 1 }).count
# => 3

Comment.joins(:also_post).where(also_post: { id: 1 }).count
# => 3

Comment.joins(:also_post).where(also_posts: { id: 1 }).count
# => no such column: also_posts.id (ActiveRecord::StatementInvalid)

ActiveRecord.allow_deprecated_singular_associations_name = true

Comment.joins(:also_post).where(also_posts: { id: 1 }).count
# => no such column: also_posts.id (ActiveRecord::StatementInvalid)

@jonathanhefner jonathanhefner merged commit 9badb0f into rails:main Jun 23, 2022
@jonathanhefner
Copy link
Member

Thank you, @willnet! 👍

@willnet willnet deleted the fix-sample-code-about-allow_deprecated_singular_associations_name branch June 24, 2022 07:36
@willnet
Copy link
Contributor Author

willnet commented Jun 24, 2022

@jonathanhefner Thanks, it helps me a lot 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants