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 polymorphic belongs_to to correctly use parent's query_constraints #50321

Conversation

fatkodima
Copy link
Member

Fixes #50315.

For polymorphic association with query constraints, we get incorrect value for primary key (specifically, "id" in the referenced issue) here

def association_primary_key(klass = nil)
if primary_key = options[:primary_key]
@association_primary_key ||= -primary_key.to_s
elsif !polymorphic? && ((klass || self.klass).has_query_constraints? || options[:query_constraints])
(klass || self.klass).composite_query_constraints_list
elsif (klass || self.klass).composite_primary_key?
# If klass has composite primary key of shape [:<tenant_key>, :id], infer primary_key as :id
primary_key = (klass || self.klass).primary_key
primary_key.include?("id") ? "id" : primary_key
else
primary_key(klass || self.klass)
end
end

which then incorrectly matches with foreign key (specifically, ["tenant_id", "commentable_id"] in the referenced issue) here

def last_chain_scope(scope, reflection, owner)
primary_key = Array(reflection.join_primary_key)
foreign_key = Array(reflection.join_foreign_key)
table = reflection.aliased_table
primary_key_foreign_key_pairs = primary_key.zip(foreign_key)

With this change, association_primary_key now correctly returns ["tenant_id", "id"].

@@ -870,7 +870,7 @@ def association_class
def association_primary_key(klass = nil)
if primary_key = options[:primary_key]
@association_primary_key ||= -primary_key.to_s
elsif !polymorphic? && ((klass || self.klass).has_query_constraints? || options[:query_constraints])
elsif (klass || self.klass).has_query_constraints? || options[:query_constraints]
Copy link
Member

Choose a reason for hiding this comment

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

Hrm, I originally added the !polymorphic? check because klass can't always be computed on those associations, but this seems to be working fine based on the tests.

@eileencodes eileencodes merged commit 0a6532d into rails:main Dec 11, 2023
4 checks passed
eileencodes added a commit that referenced this pull request Dec 11, 2023
…ith-query_constraints

Fix polymorphic `belongs_to` to correctly use parent's `query_constraints`
@eileencodes
Copy link
Member

Thanks @fatkodima - I backported this to 7-1-stable in 59e72b0

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.

Polymorphic association in model with query constraints pointing to incorrect records
2 participants