Fix bug in filtering in Participants::Query
#2003
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review app deployed to https://npq-registration-review-2003-web.test.teacherservices.cloud/ |
ethax-ross
force-pushed
the
3798-fix-bug-participant-applications
branch
from
fe6180e
to
3c6ef6b
Compare
ethax-ross
changed the title
Participants::QueryParticipants::Query
cwrw
approved these changes
Nov 22, 2024
leandroalemao
approved these changes
Nov 22, 2024
When we filter on `includes` with ActiveRecord the included table is filtered down to the records that match the conditional. For the `Participants::Query` this means when filtering by `updated_since` the related applications/id changes are filtered as well as the base users. Instead, we want the users to be filtered but retain all the applications and id changes in the preloaded association. To achieve this we can instead use a subquery to find the applications/id changes updated within the timeframe and filter on the selected `user_id` of those. This will be a little less efficient, but is heaps simpler than trying to do an additional join on applications/id changes to try and get Rails to retain them all after filtering.
ethax-ross
force-pushed
the
3798-fix-bug-participant-applications
branch
from
3c6ef6b
to
01f04d5
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Jira-3798
Context
When we filter on
includeswith ActiveRecord the included table is filtered down to the records that match the conditional. For theParticipants::Querythis means when filtering byupdated_sincethe related applications/idchanges are filtered as well as the base users. Instead, we want the users to be filtered but retain all the applications and id changes in the preloaded association.
Changes proposed in this pull request
Participants::QueryTo achieve this we can instead use a subquery to find the applications/id changes updated within the timeframe and filter on the selected
user_idof those.This will be a little less efficient, but is heaps simpler than trying to do an additional join on applications/id changes to try and get Rails to retain them all after filtering.
Guidance for review
I checked the other queries for similar filtering and couldn't find anywhere else we filter on a to-many relationship that may cause the same issue.