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

Support stale state detections in CPK through associations #48606

Merged
merged 1 commit into from Jul 5, 2023

Conversation

gmcgibbon
Copy link
Member

@gmcgibbon gmcgibbon commented Jun 29, 2023

Motivation / Background

This Pull Request has been created because stale stack checking for has_many / has_one through relations doesn't currently work for composite primary key relations.

Detail

This Pull Request changes stale states so that they are tracked as an array of columns to properly check through associations with composite primary keys.

Additional information

My tests might be too close to the implementation. Let me know if I should improve that.

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.

Stale states need to be tracked as arrays to properly check through
associations with composite primary keys.
@eileencodes eileencodes merged commit d1a79da into rails:main Jul 5, 2023
9 checks passed
@gmcgibbon gmcgibbon deleted the stale_state_cpk branch July 5, 2023 17:00
JoeDupuis added a commit to JoeDupuis/rails that referenced this pull request Aug 21, 2023
PR rails#48606 introduced a regression preventing the creation of a chain
of models using a has_one through association when strict_loading is
activated.

The association relies on the initial stale_state being `nil`
to skip loading the target.
https://github.com/gmcgibbon/rails/blob/755c13f167af4fddc6960ed3768ed7e178a2395a/activerecord/lib/active_record/associations/association.rb#L175

PR rails#48606 initializes the stale_state to `[nil]`
https://github.com/rails/rails/pull/48606/files#diff-39c3da3c5f3fbff01b0a32d3b7ec613fda6bc6225fdbe2629134d91babe37786R84-R86

This change converts the `[nil]` state back to `nil`.
paulreece pushed a commit to paulreece/rails-paulreece that referenced this pull request Aug 26, 2023
PR rails#48606 introduced a regression preventing the creation of a chain
of models using a has_one through association when strict_loading is
activated.

The association relies on the initial stale_state being `nil`
to skip loading the target.
https://github.com/gmcgibbon/rails/blob/755c13f167af4fddc6960ed3768ed7e178a2395a/activerecord/lib/active_record/associations/association.rb#L175

PR rails#48606 initializes the stale_state to `[nil]`
https://github.com/rails/rails/pull/48606/files#diff-39c3da3c5f3fbff01b0a32d3b7ec613fda6bc6225fdbe2629134d91babe37786R84-R86

This change converts the `[nil]` state back to `nil`.
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