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 predicate builder for polymorphic models referencing models with composite primary keys #50319

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

fatkodima
Copy link
Member

Fixes #50318.

I needed to create 2 models for the test case, because there is no currently Cpk model where both primary key columns are not "id". When one of them is named "id", we do not get the error as in the linked issue

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 = primary_key(value)

if value.is_a?(Base)
Copy link

Choose a reason for hiding this comment

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

Maybe it's better to move the primary key assignment inside the if clause so that it's not called when hitting the else clause.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it was for DRYing the code, but turned out this is not working, thanks.

@eileencodes eileencodes merged commit 0745043 into rails:main Dec 11, 2023
4 checks passed
@htcarr3
Copy link

htcarr3 commented Feb 20, 2024

Is it possible to merge this fix into 7-1-stable (similar to #50412) ?

@fatkodima
Copy link
Member Author

Opened a backport PR.

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.

Where(relation: ) broken for polymorphic records when using composite primary keys
4 participants