VED-755: Handling NHS Number confusions #816
Conversation
|
This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket: VED-755 |
Akol125
changed the title
dlzhry2nhs
left a comment
dlzhry2nhs
left a comment
There was a problem hiding this comment.
Couple of comments added. There are a couple of key comments we need to resolve regarding the ticket acceptance criteria e.g. business logic for applying updates.
| if ieds_check_exist(nhs_number): | ||
| # Fetch PDS details for demographic comparison | ||
| try: | ||
| pds_details = pds_get_patient_details(nhs_number) |
There was a problem hiding this comment.
We already have called pds_get_patient_id which calls this pds_get_patient_details function. This is a bit inefficient. Could just make 1 API call rather than 2 and get the relevant data that we need.
There was a problem hiding this comment.
Discussed on call - optional. Can do if it does not take too much time, but is an existing issue. Would be nice as improves efficiency.
There was a problem hiding this comment.
We are still calling PDS twice in this flow. Both times we retrieve the full patient details. On the first call, we retrieve the details and then just obtain the ID/NHS Number.
On the second call we retrieve the full patient details again so we can use it to compare the demographic details etc.
It would be really nice if we could just perform the call once. Please confirm if you plan to do this or are leaving out of this PR.
There was a problem hiding this comment.
I have given this some more thought. Maybe it is too much for this ticket, but can we raise a ticket to remove to 2 calls to PDS and replace with 1. It is an external service that already faces a lot of traffic, so I would like to minimise this in future.
There was a problem hiding this comment.
that would be nice, we could have another ticket to tighten up some other checks, including this and add backoffs and retries for the ieds_db operations. A ticket basically to make things more durable and cater for scenarios that might also come up in the test??
dlzhry2nhs
left a comment
dlzhry2nhs
left a comment
There was a problem hiding this comment.
Request looking into the comments raised. Happy to jump on call if required.
dlzhry2nhs
left a comment
dlzhry2nhs
left a comment
There was a problem hiding this comment.
Really good. All major concerns addressed. I've just added some comments which could be nice to fix - most are small/minor changes for readability. I'll approve after.
|
3 participants
Summary
Add any other relevant notes or explanations here. Remove this line if you have nothing to add.
Reviews Required
Review Checklist
ℹ️ This section is to be filled in by the reviewer.