-
Notifications
You must be signed in to change notification settings - Fork 2
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
Use get_or_create_patient in the IPC status table #1443
base: cerner-merge-branch
Are you sure you want to change the base?
Use get_or_create_patient in the IPC status table #1443
Conversation
This is because the ipc status table can have multiple rows per MRN.
ad04a98
to
8de49cc
Compare
# The table contains multiple results for the same | ||
# MRN so get or create in case they have already | ||
# recently been created. | ||
patient, _ = loader.get_or_create_patient( |
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.
Wouldn't this mean that we care about the ordering of the results in some way?
this overwrites with the arbitrarily last - is that the behaviour we want?
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.
So there is an ongoing discussion on how to merge ipc status https://github.com/openhealthcare/Development/issues/166
you're right there is an arbitrary behaviour that already exists, this is more so that the we can run the ipc command as with the new create_rfh_patient_from_hospital_number error handling this will blow up and so the command can't be run.
2 options the way I see it...
- We leave it breaking as a reminder its somethings we have to fix
- We take add the merge issue to 166 and move on.
I'm happy with either
I'm removing the Cerner Merge label as this is not required for that release. |
Marking as needs discussion as https://github.com/openhealthcare/Development/issues/166 is in needs discussion |
The IPC Status tables has multiple rows per MRN, make sure we don't error when loading these in.