Skip to content

Commit f506f92

Browse files
authored
VED-1230: Invalid NHS Number still creates the event (#1436)
* Add pre-validation for NHS number system in immunization services - Implemented a new pre-validation method to ensure that patient identifiers use the NHS number system. - Added unit tests to verify that both creation and update of immunization records reject invalid patient identifier systems. - Updated existing tests to reflect changes in validation logic for patient identifiers. * chore: empty commit * Update test for NHS number validation by removing an invalid test string from the pre-validation unit test. * Add unit test for duplicate site SNOMED validation in immunization batch service - Implemented a new test to ensure that duplicate site SNOMED codings are rejected during batch validation. - The test verifies that the appropriate validation error message is raised and that the create_immunization method is not called when duplicates are present.
1 parent f7866f4 commit f506f92

3 files changed

Lines changed: 74 additions & 6 deletions

File tree

lambdas/recordforwarder/tests/service/test_fhir_batch_service.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,29 @@ def test_create_immunization_post_validation_error(self):
101101
self.assertTrue(expected_msg in error.exception.message)
102102
self.mock_repo.create_immunization.assert_not_called()
103103

104+
def test_create_immunization_invalid_patient_identifier_system(self):
105+
"""it should reject create when the patient identifier system is not the NHS number URI"""
106+
107+
imms = create_covid_immunization_dict_no_id()
108+
patient = next(resource for resource in imms["contained"] if resource["resourceType"] == "Patient")
109+
patient["identifier"][0]["system"] = "https://1234/Id/nhs-number"
110+
patient["identifier"][0]["value"] = "ABCD"
111+
112+
with self.assertRaises(CustomValidationError) as error:
113+
self.pre_validate_fhir_service.create_immunization(
114+
immunization=imms,
115+
supplier_system="test_supplier",
116+
vax_type="test_vax",
117+
table=self.mock_table,
118+
imms_pk=None,
119+
)
120+
121+
self.assertIn(
122+
"contained[?(@.resourceType=='Patient')].identifier[0].system must equal",
123+
error.exception.message,
124+
)
125+
self.mock_repo.create_immunization.assert_not_called()
126+
104127
def test_create_immunization_duplicate_site_snomed_still_rejected_for_batch(self):
105128
"""it should keep batch validation unchanged for duplicate site SNOMED codings"""
106129

@@ -192,6 +215,29 @@ def test_update_immunization_post_validation_error(self):
192215
self.assertTrue(expected_msg in error.exception.message)
193216
self.mock_repo.update_immunization.assert_not_called()
194217

218+
def test_update_immunization_invalid_patient_identifier_system(self):
219+
"""it should reject update when the patient identifier system is not the NHS number URI"""
220+
221+
imms = create_covid_immunization_dict_no_id()
222+
patient = next(resource for resource in imms["contained"] if resource["resourceType"] == "Patient")
223+
patient["identifier"][0]["system"] = "https://1234/Id/nhs-number"
224+
patient["identifier"][0]["value"] = "ABCD"
225+
226+
with self.assertRaises(CustomValidationError) as error:
227+
self.pre_validate_fhir_service.update_immunization(
228+
immunization=imms,
229+
supplier_system="test_supplier",
230+
vax_type="test_vax",
231+
table=self.mock_table,
232+
imms_pk=None,
233+
)
234+
235+
self.assertIn(
236+
"contained[?(@.resourceType=='Patient')].identifier[0].system must equal",
237+
error.exception.message,
238+
)
239+
self.mock_repo.update_immunization.assert_not_called()
240+
195241

196242
class TestDeleteImmunizationBatchService(unittest.TestCase):
197243
def setUp(self):

lambdas/shared/src/common/models/fhir_immunization_pre_validators.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ def validate(self):
4646
self.pre_validate_patient_identifier_extension,
4747
self.pre_validate_patient_identifier,
4848
self.pre_validate_patient_identifier_system,
49+
self.pre_validate_patient_identifier_nhs_system,
4950
self.pre_validate_patient_identifier_value,
5051
self.pre_validate_patient_name,
5152
self.pre_validate_patient_name_given,
@@ -286,6 +287,21 @@ def pre_validate_patient_identifier_system(self, values: dict) -> None:
286287
except (KeyError, IndexError):
287288
pass
288289

290+
def pre_validate_patient_identifier_nhs_system(self, values: dict) -> None:
291+
"""
292+
Pre-validate that, if the contained Patient has an identifier system,
293+
it uses the NHS number system.
294+
"""
295+
field_location = "contained[?(@.resourceType=='Patient')].identifier[0].system"
296+
try:
297+
field_value = [x for x in values["contained"] if x.get("resourceType") == "Patient"][0]["identifier"][0][
298+
"system"
299+
]
300+
if field_value != Urls.NHS_NUMBER:
301+
raise ValueError(f"{field_location} must equal '{Urls.NHS_NUMBER}'")
302+
except (KeyError, IndexError):
303+
pass
304+
289305
def pre_validate_patient_identifier_value(self, values: dict) -> None:
290306
"""
291307
Pre-validate that, if the contained Patient has an NHS-number identifier,

lambdas/shared/tests/test_common/test_immunization_pre_validator.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -470,24 +470,30 @@ def test_pre_validate_patient_identifier_value(self):
470470
],
471471
)
472472

473-
def test_pre_validate_patient_identifier_value_accepts_non_nhs_identifier(self):
474-
"""Test pre_validate_patient_identifier_value ignores non-NHS patient identifiers"""
475-
valid_json_data = deepcopy(self.json_data)
476-
valid_json_data["contained"][1]["identifier"] = [
473+
def test_pre_validate_patient_identifier_rejects_non_nhs_identifier_system(self):
474+
"""Test pre_validate_patient_identifier rejects non-NHS patient identifier systems"""
475+
invalid_json_data = deepcopy(self.json_data)
476+
invalid_json_data["contained"][1]["identifier"] = [
477477
{
478478
"system": "https://someother.codeableconcept.com/",
479479
"value": "TVC15",
480480
}
481481
]
482482

483-
self.assertIsNone(self.validator.validate(valid_json_data))
483+
with self.assertRaises(ValueError) as error:
484+
self.validator.validate(invalid_json_data)
485+
486+
self.assertIn(
487+
"contained[?(@.resourceType=='Patient')].identifier[0].system must equal '",
488+
str(error.exception),
489+
)
484490

485491
def test_pre_validate_patient_identifier_system(self):
486492
"""Test pre_validate_patient_identifier_system accepts valid values and rejects invalid values"""
487493
ValidatorModelTests.test_string_value(
488494
self,
489495
field_location="contained[?(@.resourceType=='Patient')].identifier[0].system",
490-
valid_strings_to_test=[Urls.NHS_NUMBER, "https://someother.codeableconcept.com/"],
496+
valid_strings_to_test=[Urls.NHS_NUMBER],
491497
)
492498

493499
def test_pre_validate_patient_name(self):

0 commit comments

Comments
 (0)