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

Bugfix: Ensure has_one associations saved when part of CPK has changed #48491

Merged

Conversation

adrianna-chang-shopify
Copy link
Contributor

Motivation / Background

If a has_one association uses a composite primary key, and part of the composite primary key changes on the owner, these changes need to be reflected on the belonging object's foreign key.

This was not working previously, because #_record_changed? was not equipped to handle composite primary key associations, so we were not recognizing that the belonging object's foreign key needed to be updated when the owner's primary key changed.

Detail

Maintain the same check as was previously used in #_record_changed?, but adjust it to accommodate composite primary | foreign keys. This means checking that the record has all attributes that form the primary key on the owner, and then checking that the composite FK values don't match the owner's PK. This indicates that the record has changed, and that we should update its foreign key values accordingly.

Additional information

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. (N/A)

If a has_one association uses a composite primary key, and part of the
composite primary key changes on the owner, these changes need to be
reflected on the belonging object's foreign key.

This was not working previously, because `#_record_changed?` was not
equipped to handle composite primary key associations, so we were not
recognizing that the belonging object's foreign key needed to be updated
when the owner's primary key changed.
@adrianna-chang-shopify
Copy link
Contributor Author

cc @eileencodes

@eileencodes eileencodes merged commit 3f24fa8 into rails:main Jun 16, 2023
9 checks passed
@eileencodes eileencodes deleted the ac-fk-changed-for-cpk branch June 16, 2023 12:48
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