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

Make has_many through singular associations build CPK records #48650

Merged
merged 1 commit into from Jul 5, 2023

Conversation

gmcgibbon
Copy link
Member

@gmcgibbon gmcgibbon commented Jul 4, 2023

Motivation / Background

This Pull Request has been created because composite primary key records cannot be build when using has_many through singular (has_one) associations.

Detail

This Pull Request changes build_record on ActiveRecord::Associations::ThroughAssociation to support composite primary key associations.

Additional information

I noticed some odd behaviour for build_record that I'll be submitting fixes for in separate patches.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@gmcgibbon
Copy link
Member Author

gmcgibbon commented Jul 4, 2023

Hm, I think this should actually include has_ones through has_ones as well. I'll add another test and update the description.

Never mind, that doesn't work because of

raise HasOneThroughCantAssociateThroughHasOneOrManyReflection.new(owner, reflection)

@gmcgibbon gmcgibbon force-pushed the cpk_hmt_singular branch 2 times, most recently from db646d8 to cf3850e Compare July 4, 2023 21:15
@eileencodes
Copy link
Member

Can you fix the conflicts?

Adds support for building records in has_many through has_one
composite primary key associations.

Also updates query constraints on associations affected by rails#48564.
@gmcgibbon
Copy link
Member Author

Fixed!

@eileencodes eileencodes merged commit 807bd54 into rails:main Jul 5, 2023
9 checks passed
@gmcgibbon gmcgibbon deleted the cpk_hmt_singular branch July 5, 2023 22:34
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