-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix duplicate patients with multiple visits in CSV upload #212
Conversation
I'm thinking it's probably easier to break out creating patients to a separate function - so we group the input CSV by NHS number, create the patients then create all the visits. That would still be async compatible with #200 and doesn't require any decisions on data modelling as uploading a CSV will always create a new submission anyway. Marking this PR as DO NOT MERGE until I've made those changes |
As opposed to looking them up within the Audit Cohort. This method is compatible with the async work in #200. We're able to do this as CSV uploads always involve creating a new cohort.
organisation_ods_code=organisation_ods_code, | ||
) | ||
except Exception as error: | ||
raise Exception(f"Could not save site: {error}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These try/catches just tended to hide the stack trace during development which was very annoying so I've removed them. We may want to add them back to avoid an angry blank screen, but tbh custom error pages are probably the better way to go (#21) so that we are sure we will get full stack trace logging in the backend logs
) | ||
else None | ||
), | ||
"glucose_monitoring": row[ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is the same as before, just at one level of indentation lower. It was tempting to do some additional refactoring here but I'm leaving it for #31
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is working for me.
Fixes #206
Correctly handle patients with multiple visits in a CSV upload by grouping by NHS number and only adding the patient once.