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

Multiple visits for the same patient in CSV upload causes duplicate entries #206

Closed
mbarton opened this issue Jul 18, 2024 · 4 comments · Fixed by #212
Closed

Multiple visits for the same patient in CSV upload causes duplicate entries #206

mbarton opened this issue Jul 18, 2024 · 4 comments · Fixed by #212
Assignees
Labels
bug Something isn't working high high priority (max 3 items)

Comments

@mbarton
Copy link
Member

mbarton commented Jul 18, 2024

Duplicate the final row in dummy_sheet.csv and change the visit date.

  • Expected: you see one patient with two visits in the patients tab
  • Actual: you see two patients with the same NHS number, each with one visit
@mbarton mbarton added bug Something isn't working high high priority (max 3 items) labels Jul 18, 2024
@mbarton
Copy link
Member Author

mbarton commented Jul 22, 2024

I thought this was as simple as changing Patient.objects.create to Patients.objects.update_or_create here:

But I ended up with a few questions about how we normalise our data model.

At the moment there is not a unique constraint on NHS number. So even fixing this bug within csv_upload would lead to multiple Patient rows with the same number but for different submissions (AuditCohort).

The relation between Patient and AuditCohort is defined on the AuditCohort model. So that implies we do actually want to have a single Patient across all submissions. The only argument against that I can think of is if we wanted to capture changing GP details but we could still do that with point exports of the database rather than storing those changes. Transferring patients between PDUs is fine as they can be within multiple AuditCohorts.

So I've probably talked myself into an update_or_create and a unique constraint on NHS number?

Either way we also need a way of showing validation errors if the same patient is in the CSV with different details.

@mbarton
Copy link
Member Author

mbarton commented Jul 22, 2024

I have a PR up here that doesn't use update_or_create (#212). So at least we can get the functionality in.

@eatyourpeas
Copy link
Member

I thought I had made the NHS Number unique and required. If it is not then it should be because there can only be one patient per NHS Number. This does mean though that it will not be possible to save a patient with an invalid NHS number.

However, since the same patient will be resubmitted every cohort as they get older (unless they die, I suppose), it needs to be possible that they have multiple records in the AuditCohort table (though only one per cohort, unless the requirement is to store all historical uploads - I am not sure we ever got a clear answer to that).

In which case, update_or_create would be the way to go during the upload process. I thought that was how it had been originally written actually. As you say, this would overwrite any previous entries in address or GP practice, but I don't think the NPDA team mind this. As I remember Holly said that even if the patients move centres midway through the audit year, NPDA currently do not track this - they just update the PDU and the new centre inherits all the scores from previous as if their own.

@mbarton
Copy link
Member Author

mbarton commented Jul 23, 2024

I wonder if there is value in not normalising Patient but rather having them duplicated but unique within a single submission. My thinking is that gives us the maximum protection against invalid data from a CSV upload overwriting good data by accident, because at worst you can revert to a previous submission.

I've written up my thoughts on how this interacts with AuditCohort here (RCPCH internal): https://forum.rcpch.tech/t/submission-design/154

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high high priority (max 3 items)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants