Skip to content

Commit d54b8da

Browse files
alistair-white-horne-tngalistair-white-horne-tng
committed
Make sure duplicate_of_vaccination_record is persisted
There existed a bug where `duplicate_of_vaccination_record_id` would be incorrectly set to `nil` if a second `Search...Job` were run on the same records. This bug was caused by the `update!` function implicitly referencing the `id` of a `VaccinationRecord` which was in memory, and thus whose `id` was `nil`. This fix instead tries to do a DB lookup of the duplicate record, only falling back to the in-memory record if it doesn't exist yet in the DB; i.e. it's never been saved before. This fix also introduces more strict ordering when saving and updating the existing and incoming records, to ensure that `duplicate_of_vaccination_record_id` always references a record which is persisted in the DB. Jira-Issue: MAV-5903
1 parent 992f985 commit d54b8da

3 files changed

Lines changed: 215 additions & 7 deletions

File tree

‎app/jobs/search_vaccination_records_in_nhs_job.rb‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,23 @@ def perform(patient_id)
2020

2121
return unless feature_flags_enabled
2222

23-
existing_vaccination_records.find_each do |vaccination_record|
23+
existing_vaccination_records.each do |vaccination_record|
2424
incoming_vaccination_record =
2525
incoming_vaccination_records.find do
2626
it.nhs_immunisations_api_id ==
2727
vaccination_record.nhs_immunisations_api_id
2828
end
2929

3030
if incoming_vaccination_record
31-
vaccination_record.update!(
31+
vaccination_record.assign_attributes(
3232
incoming_vaccination_record
3333
.attributes
3434
.except("id", "uuid", "created_at")
35-
.merge(updated_at: Time.current)
35+
.merge(
36+
duplicate_of_vaccination_record:
37+
incoming_vaccination_record.duplicate_of_vaccination_record,
38+
updated_at: Time.current
39+
)
3640
)
3741

3842
incoming_vaccination_records.delete(incoming_vaccination_record)
@@ -41,10 +45,12 @@ def perform(patient_id)
4145
end
4246
end
4347

44-
# Remaining incoming_vaccination_records are new.
4548
# Save non-discarded records first so they have IDs before discarded
4649
# duplicates reference them via duplicate_of_vaccination_record_id.
47-
incoming_vaccination_records.sort_by { it.discarded? ? 1 : 0 }.each(&:save!)
50+
(
51+
existing_vaccination_records.reject(&:destroyed?) +
52+
incoming_vaccination_records
53+
).sort_by { it.discarded? ? 1 : 0 }.each(&:save!)
4854

4955
update_vaccination_search_timestamps if patient.nhs_number.present?
5056

@@ -151,8 +157,23 @@ def deduplicate_vaccination_records(incoming_vaccination_records)
151157
end
152158
elsif records.any?(&:nhs_immunisations_api_primary_source)
153159
# If some records have `primarySource: true`, set `discarded_at` for all `primarySource: false` records,
154-
# pointing each at the first `primarySource: true` record
155-
canonical = records.find(&:nhs_immunisations_api_primary_source)
160+
# pointing each at the first `primarySource: true` record.
161+
162+
canonical_incoming =
163+
records.find(&:nhs_immunisations_api_primary_source)
164+
canonical_existing =
165+
patient
166+
.vaccination_records
167+
.sourced_from_nhs_immunisations_api
168+
.with_discarded
169+
.find_by(
170+
nhs_immunisations_api_id:
171+
canonical_incoming.nhs_immunisations_api_id
172+
)
173+
174+
# Prefer the persisted DB record (if it already exists from a previous run) so that
175+
# `duplicate_of_vaccination_record_id` is non-nil when the non-primary record is saved.
176+
canonical = canonical_existing || canonical_incoming
156177
records
157178
.select(&:sourced_from_nhs_immunisations_api?)
158179
.reject(&:nhs_immunisations_api_primary_source)

‎spec/fixtures/files/fhir/search_responses/1_result_primary_source_false.json‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
{
2+
"type": "searchset",
3+
"total": 1,
4+
"link": [
5+
{
6+
"relation": "self",
7+
"url": "https://api.service.nhs.uk/immunisation-fhir-api/Immunization?-immunization.target=FLU,HPV,MENACWY,3IN1,MMR,MMRV&patient.identifier=https%3A%2F%2Ffhir.nhs.uk%2FId%2Fnhs-number%7C9449308357"
8+
}
9+
],
10+
"entry": [
11+
{
12+
"fullUrl": "https://api.service.nhs.uk/immunisation-fhir-api/Immunization/322a54c7-acd8-4eb7-adbc-4006938df8f2",
13+
"resource": {
14+
"id": "322a54c7-acd8-4eb7-adbc-4006938df8f2",
15+
"extension": [
16+
{
17+
"url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-VaccinationProcedure",
18+
"valueCodeableConcept": {
19+
"coding": [
20+
{
21+
"system": "http://snomed.info/sct",
22+
"code": "884861000000100",
23+
"display": "Administration of first intranasal seasonal influenza vaccination"
24+
}
25+
]
26+
}
27+
}
28+
],
29+
"identifier": [
30+
{
31+
"use": "official",
32+
"system": "YGA",
33+
"value": "TPP_V_429814877"
34+
}
35+
],
36+
"status": "completed",
37+
"vaccineCode": {
38+
"coding": [
39+
{
40+
"system": "http://snomed.info/sct",
41+
"code": "46233009",
42+
"display": "Influenza vaccine"
43+
}
44+
]
45+
},
46+
"patient": {
47+
"reference": "urn:uuid:c3e7be44-bb52-4df7-8232-7500ed90c137",
48+
"type": "Patient",
49+
"identifier": {
50+
"system": "https://fhir.nhs.uk/Id/nhs-number",
51+
"value": "9449308357"
52+
}
53+
},
54+
"occurrenceDateTime": "2025-09-24T00:00:00+00:00",
55+
"recorded": "2025-10-02",
56+
"primarySource": false,
57+
"location": {
58+
"identifier": {
59+
"system": "https://fhir.nhs.uk/Id/ods-organization-code",
60+
"value": "100001"
61+
}
62+
},
63+
"lotNumber": "BU5086",
64+
"site": {
65+
"coding": [
66+
{
67+
"system": "http://snomed.info/sct",
68+
"code": "279549004",
69+
"display": "Nasal cavity structure"
70+
}
71+
]
72+
},
73+
"performer": [
74+
{
75+
"actor": {
76+
"type": "Organization",
77+
"identifier": {
78+
"system": "https://fhir.nhs.uk/Id/ods-organization-code",
79+
"value": "R1L"
80+
}
81+
}
82+
}
83+
],
84+
"protocolApplied": [
85+
{
86+
"targetDisease": [
87+
{
88+
"coding": [
89+
{
90+
"system": "http://snomed.info/sct",
91+
"code": "6142004",
92+
"display": "Influenza caused by seasonal influenza virus (disorder)"
93+
}
94+
]
95+
}
96+
],
97+
"doseNumberPositiveInt": 1
98+
}
99+
],
100+
"resourceType": "Immunization"
101+
},
102+
"search": {
103+
"mode": "match"
104+
}
105+
},
106+
{
107+
"fullUrl": "urn:uuid:c3e7be44-bb52-4df7-8232-7500ed90c137",
108+
"resource": {
109+
"id": "7101120547",
110+
"identifier": [
111+
{
112+
"system": "https://fhir.nhs.uk/Id/nhs-number",
113+
"value": "7101120547"
114+
}
115+
],
116+
"resourceType": "Patient"
117+
},
118+
"search": {
119+
"mode": "include"
120+
}
121+
}
122+
],
123+
"resourceType": "Bundle"
124+
}

‎spec/jobs/search_vaccination_records_in_nhs_job_spec.rb‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,9 +964,72 @@
964964
expect(non_primary).to be_discarded
965965
end
966966

967+
it "points the non-primary-source record at the primary source record" do
968+
perform
969+
primary =
970+
VaccinationRecord.find_by(
971+
nhs_immunisations_api_primary_source: true
972+
)
973+
non_primary =
974+
VaccinationRecord.find_by(
975+
nhs_immunisations_api_primary_source: false
976+
)
977+
expect(non_primary.duplicate_of_vaccination_record).to eq(primary)
978+
end
979+
967980
include_examples "sends discovery comms if required n times", 0
968981
include_examples "calls StatusUpdater"
969982
end
983+
984+
context "when a single non-primary source record exists, but a primary source record has been added" do
985+
# The first run created a kept (non-primary) record. When another search is completed, where there is now
986+
# also a primary record, then the outcome should be the same as if the first search had never happened
987+
988+
let(:existing_bundle_body) do
989+
file_fixture(
990+
"fhir/search_responses/1_result_primary_source_false.json"
991+
).read
992+
end
993+
let(:body) { file_fixture("fhir/search_responses/duplicate.json").read }
994+
995+
it "creates 1 new record" do
996+
expect { perform }.to(change(VaccinationRecord, :count).by(1))
997+
end
998+
999+
it "retains the existing record ID" do
1000+
perform
1001+
expect(VaccinationRecord.all.map(&:id)).to include(
1002+
existing_records.map(&:id).sole
1003+
)
1004+
end
1005+
1006+
it "sets the existing record as discarded" do
1007+
perform
1008+
expect(existing_records.sole.reload).to be_discarded
1009+
end
1010+
1011+
it "doesn't set the new record as discarded" do
1012+
perform
1013+
primary =
1014+
VaccinationRecord.find_by(
1015+
nhs_immunisations_api_primary_source: true
1016+
)
1017+
expect(primary).not_to be_discarded
1018+
end
1019+
1020+
it "points the non-primary-source record at the primary source record" do
1021+
perform
1022+
primary =
1023+
VaccinationRecord.find_by(
1024+
nhs_immunisations_api_primary_source: true
1025+
)
1026+
non_primary =
1027+
VaccinationRecord.find_by(
1028+
nhs_immunisations_api_primary_source: false
1029+
)
1030+
expect(non_primary.duplicate_of_vaccination_record).to eq(primary)
1031+
end
1032+
end
9701033
end
9711034

9721035
context "with a record for each programme (total 6)" do

0 commit comments

Comments
 (0)