bug found in qb_timeline overlap check #4368
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
33) added by v5. the assumption that the resulting month 33 would start before the v3 month 30 is not always the case.
mcjustin
approved these changes
Mar 12, 2024
ivan-c
approved these changes
Mar 12, 2024
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.
between protocol v3 and v5, additional months were added, i.e. month 33.
if v3 month 36 didn't have any committed work and it's time to change protocols for the user, v5 month 33 should follow v3 month 30.
the code looking for this case had an extra check which prevented the above from functioning, in the rare case where v3 month 30 ended before v5 month 33 starts.
this was discovered with the new withdrawal handling in adherence data, as the insertion of the withdrawal row could not predictably insert before the time point just after withdrawal, when the timeline was found to have such an overlap.