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

user consent withdrawal date regression #4361

Merged
merged 18 commits into from
Mar 6, 2024

Conversation

pbugni
Copy link
Collaborator

@pbugni pbugni commented Feb 7, 2024

reverts bug introduced by #4343 , which incorrectly started using the oldest (not most recent) user_consent as last valid consent prior to withdrawal.

includes migration to clean up problem cases. required careful re-assessment of consent/withdrawal dates for a number of patients found to have qb_timeline revisions after applying patch above. see https://movember.atlassian.net/browse/IRONN-210 for more detail.

NB: #4363 merges into this branch, to be done prior to merging this PR.

@pep8speaks
Copy link

pep8speaks commented Feb 7, 2024

Hello @pbugni! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 843:80: E501 line too long (88 > 79 characters)

Line 3:1: E302 expected 2 blank lines, found 1

Line 177:80: E501 line too long (91 > 79 characters)
Line 192:80: E501 line too long (90 > 79 characters)
Line 223:1: E302 expected 2 blank lines, found 1

Line 38:80: E501 line too long (96 > 79 characters)
Line 80:80: E501 line too long (92 > 79 characters)

Line 767:80: E501 line too long (89 > 79 characters)
Line 767:90: W291 trailing whitespace
Line 768:80: E501 line too long (81 > 79 characters)

Line 509:80: E501 line too long (85 > 79 characters)
Line 510:80: E501 line too long (81 > 79 characters)

Line 901:80: E501 line too long (82 > 79 characters)

Comment last updated at 2024-03-05 23:23:17 UTC

… order - the user of `acceptance_date` assumed the newest date would always be best - that's false logic.
… order - the user of `acceptance_date` assumed the newest date would always be best - that's false logic.
…ents, when looking for consent date after user's consent has been suspended (withdrawn)
…s completed prior to withdrawal - was reporting status "withdrawn"
@pbugni pbugni force-pushed the feature/user_consent_withdrawal_date_regression branch from 5f201e7 to bef7625 Compare February 9, 2024 20:18
Adherence report changes:
- always include a row for withdrawal, and use 'completed date' column
for date of withdrawal
- include completed visits even if they are completed after date of
withdrawal

qb_timeline changes:
- include rows after withdrawal for implementation of the above

migration changes:
- now that we must rebuild the timeline and adherence data for every
withdrawn user, it takes far too long to run and would render the
service unavailable until the migration completes. therefore, it only
removes the timelines and adherence data for that set of users, and will
rebuild when first requested.

NB: this PR merges into another branch, such that all may be a single
update on the develop branch.

---------

Co-authored-by: Justin McReynolds <mcjustin@uw.edu>
Co-authored-by: Ivan Cvitkovic <ivanc@uw.edu>
@pbugni pbugni marked this pull request as ready for review March 5, 2024 23:24
@pbugni pbugni requested a review from ivan-c March 5, 2024 23:28
@pbugni
Copy link
Collaborator Author

pbugni commented Mar 5, 2024

@ivan-c , the bulk of the work here you've already seen in the two additional branches that were cut deeper off of this one. a review here is primarily a rubber stamp.

@pbugni pbugni merged commit 1ff18a7 into develop Mar 6, 2024
2 checks passed
@pbugni pbugni deleted the feature/user_consent_withdrawal_date_regression branch March 6, 2024 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants