Skip to content

Commit f421457

Browse files
amarauzomaJamesW1-NHS
amarauzoma
and
JamesW1-NHS
authored
VED-1088-Validate-system-URL-in-patient-identifier-resource (#1272)
* VED-1088-Validate-system-URL-in-patient-identifier-resource * pre_validate_patient_identifier_system * Updated changes in relation to the comments * Updated Error message log * Updated Error Constants * Updated comments --------- Co-authored-by: James Wharmby <james.wharmby1@nhs.net>
1 parent 5415e01 commit f421457

8 files changed

Lines changed: 69 additions & 25 deletions

File tree

‎lambdas/backend/tests/service/test_fhir_service.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,8 @@ def test_patient_error(self):
433433
invalid_nhs_number = "9434765911" # check digit 1 doesn't match result (9)
434434
imms = create_covid_immunization_dict_no_id(invalid_nhs_number)
435435
expected_msg = (
436-
"Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value is not a valid NHS number"
436+
"Validation errors: contained[?(@.resourceType=='Patient')].identifier"
437+
"[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value is not a valid NHS number"
437438
)
438439

439440
with self.assertRaises(CustomValidationError) as error:
@@ -552,7 +553,8 @@ def test_update_immunization_raises_validation_exception_when_nhs_number_invalid
552553
self.imms_repo.update_immunization.assert_not_called()
553554
self.assertEqual(
554555
error.exception.message,
555-
"Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value must be 10 characters",
556+
"Validation errors: contained[?(@.resourceType=='Patient')].identifier"
557+
"[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value must be 10 characters",
556558
)
557559

558560
def test_update_immunization_raises_not_found_error_when_no_existing_immunisation(self):

‎lambdas/recordforwarder/tests/controller/test_fhir_batch_controller.py‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import uuid
33
from unittest.mock import Mock, create_autospec
44

5+
from common.models.constants import Urls
56
from common.models.errors import (
67
CustomValidationError,
78
IdentifierDuplicationError,
@@ -18,6 +19,15 @@ def _make_immunization_pk(_id):
1819
return f"Immunization#{_id}"
1920

2021

22+
def _missing_patient_nhs_number_error():
23+
return CustomValidationError(
24+
message=(
25+
"Validation errors: contained[?(@.resourceType=='Patient')].identifier"
26+
f"[?(@.system=='{Urls.NHS_NUMBER}')].value does not exists"
27+
)
28+
)
29+
30+
2131
class TestCreateImmunizationBatchController(unittest.TestCase):
2232
def setUp(self):
2333
self.mock_repo = create_autospec(ImmunizationBatchRepository)
@@ -57,9 +67,7 @@ def test_send_request_to_dynamo_create_badrequest(self):
5767
imms_id = str(uuid.uuid4())
5868
imms_pk = _make_immunization_pk(imms_id)
5969
imms = create_covid_immunization(imms_id)
60-
create_result = CustomValidationError(
61-
message="Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists"
62-
)
70+
create_result = _missing_patient_nhs_number_error()
6371

6472
message_body = {
6573
"supplier": "test_supplier",
@@ -177,9 +185,7 @@ def test_send_request_to_dynamo_update_badrequest(self):
177185
imms_id = str(uuid.uuid4())
178186
imms_pk = _make_immunization_pk(imms_id)
179187
imms = create_covid_immunization(imms_id)
180-
update_result = CustomValidationError(
181-
message="Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists"
182-
)
188+
update_result = _missing_patient_nhs_number_error()
183189
message_body = {
184190
"supplier": "test_supplier",
185191
"fhir_json": imms.json(),
@@ -296,9 +302,7 @@ def test_send_request_to_dynamo_delete_badrequest(self):
296302
imms_id = str(uuid.uuid4())
297303
imms_pk = _make_immunization_pk(imms_id)
298304
imms = create_covid_immunization(imms_id)
299-
update_result = CustomValidationError(
300-
message="Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists"
301-
)
305+
update_result = _missing_patient_nhs_number_error()
302306
message_body = {
303307
"supplier": "test_supplier",
304308
"fhir_json": imms.json(),

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

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ def validate(self):
4545
self.pre_validate_practitioner_reference,
4646
self.pre_validate_patient_identifier_extension,
4747
self.pre_validate_patient_identifier,
48+
self.pre_validate_patient_identifier_system,
4849
self.pre_validate_patient_identifier_value,
4950
self.pre_validate_patient_name,
5051
self.pre_validate_patient_name_given,
@@ -271,17 +272,29 @@ def pre_validate_patient_identifier(self, values: dict) -> None:
271272
except (KeyError, IndexError):
272273
pass
273274

274-
def pre_validate_patient_identifier_value(self, values: dict) -> None:
275+
def pre_validate_patient_identifier_system(self, values: dict) -> None:
275276
"""
276-
Pre-validate that, if contained[?(@.resourceType=='Patient')].identifier[0].value (
277-
legacy CSV field name: NHS_NUMBER) exists, then it is a string of 10 characters
278-
which does not contain spaces
277+
Pre-validate that, if contained[?(@.resourceType=='Patient')].identifier[0].system exists,
278+
then it is a non-empty string.
279279
"""
280-
field_location = "contained[?(@.resourceType=='Patient')].identifier[0].value"
280+
field_location = "contained[?(@.resourceType=='Patient')].identifier[0].system"
281281
try:
282282
field_value = [x for x in values["contained"] if x.get("resourceType") == "Patient"][0]["identifier"][0][
283-
"value"
283+
"system"
284284
]
285+
PreValidation.for_string(field_value, field_location)
286+
except (KeyError, IndexError):
287+
pass
288+
289+
def pre_validate_patient_identifier_value(self, values: dict) -> None:
290+
"""
291+
Pre-validate that, if the contained Patient has an NHS-number identifier,
292+
its value is a string of 10 characters which does not contain spaces.
293+
"""
294+
field_location = f"contained[?(@.resourceType=='Patient')].identifier[?(@.system=='{Urls.NHS_NUMBER}')].value"
295+
try:
296+
patient = [x for x in values["contained"] if x.get("resourceType") == "Patient"][0]
297+
field_value = [x for x in patient["identifier"] if x.get("system") == Urls.NHS_NUMBER][0]["value"]
285298
PreValidation.for_string(field_value, field_location, defined_length=10, spaces_allowed=False)
286299
PreValidation.for_nhs_number(field_value, field_location)
287300
except (KeyError, IndexError):

‎lambdas/shared/src/common/models/field_locations.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class FieldLocations:
2525
target_disease = "protocolApplied[0].targetDisease"
2626
target_disease_codes = f"protocolApplied[0].targetDisease[0].coding[?(@.system=='{Urls.SNOMED}')].code"
2727
patient_identifier_value = (
28-
"contained[?(@.resourceType=='Patient')].identifier[0].value" # VED-1088 TODO: Fix to use nhs number url lookup
28+
f"contained[?(@.resourceType=='Patient')].identifier[?(@.system=='{Urls.NHS_NUMBER}')].value"
2929
)
3030
patient_name_given: str = field(init=False)
3131
patient_name_family: str = field(init=False)

‎lambdas/shared/src/common/models/utils/generic_utils.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,10 @@ def get_occurrence_datetime(immunization: dict) -> datetime.datetime | None:
128128

129129

130130
def create_diagnostics():
131-
diagnostics = "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists."
131+
diagnostics = (
132+
"Validation errors: contained[?(@.resourceType=='Patient')].identifier"
133+
"[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value does not exists."
134+
)
132135
exp_error = {"diagnostics": diagnostics}
133136
return exp_error
134137

‎lambdas/shared/tests/test_common/test_immunization_post_validator.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,9 @@ def test_post_patient_identifier_value(self):
206206
"""
207207
Test that the JSON data is accepted when it does not contain patient_identifier_value
208208
"""
209-
field_location = "contained[?(@.resourceType=='Patient')].identifier[0].value"
209+
field_location = (
210+
"contained[?(@.resourceType=='Patient')].identifier[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value"
211+
)
210212
self.mock_redis.hget.return_value = "COVID"
211213
self.mock_redis_getter.return_value = self.mock_redis
212214
MandationTests.test_missing_field_accepted(self, field_location)

‎lambdas/shared/tests/test_common/test_immunization_pre_validator.py‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from jsonpath_ng.ext import parse
99

10-
from common.models.constants import Constants
10+
from common.models.constants import Constants, Urls
1111
from common.models.fhir_immunization import ImmunizationValidator
1212
from common.models.fhir_immunization_pre_validators import PreValidators
1313
from common.models.utils.generic_utils import (
@@ -457,7 +457,7 @@ def test_pre_validate_patient_identifier_value(self):
457457
"""Test pre_validate_patient_identifier_value accepts valid values and rejects invalid values"""
458458
ValidatorModelTests.test_string_value(
459459
self,
460-
field_location="contained[?(@.resourceType=='Patient')].identifier[0].value",
460+
field_location=f"contained[?(@.resourceType=='Patient')].identifier[?(@.system=='{Urls.NHS_NUMBER}')].value",
461461
valid_strings_to_test=["9990548609"],
462462
defined_length=10,
463463
invalid_length_strings_to_test=["999054860", "99905486091", ""],
@@ -470,6 +470,26 @@ 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"] = [
477+
{
478+
"system": "https://someother.codeableconcept.com/",
479+
"value": "TVC15",
480+
}
481+
]
482+
483+
self.assertIsNone(self.validator.validate(valid_json_data))
484+
485+
def test_pre_validate_patient_identifier_system(self):
486+
"""Test pre_validate_patient_identifier_system accepts valid values and rejects invalid values"""
487+
ValidatorModelTests.test_string_value(
488+
self,
489+
field_location="contained[?(@.resourceType=='Patient')].identifier[0].system",
490+
valid_strings_to_test=[Urls.NHS_NUMBER, "https://someother.codeableconcept.com/"],
491+
)
492+
473493
def test_pre_validate_patient_name(self):
474494
"""Test pre_validate_patient_name accepts valid values and rejects invalid values"""
475495
ValidatorModelTests.test_list_value(

‎tests/e2e_automation/utilities/error_constants.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
},
1616
"empty_NHSNumber": {
1717
"code": "INVALID",
18-
"diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value must be a non-empty string",
18+
"diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value must be a non-empty string",
1919
},
2020
"invalid_include": {
2121
"code": "INVALID",
@@ -99,7 +99,7 @@
9999
},
100100
"invalid_nhsnumber_length": {
101101
"code": "INVARIANT",
102-
"diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value must be 10 characters",
102+
"diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value must be 10 characters",
103103
},
104104
"no_forename": {
105105
"code": "INVARIANT",
@@ -139,7 +139,7 @@
139139
},
140140
"invalid_mod11_nhsnumber": {
141141
"code": "INVARIANT",
142-
"diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value is not a valid NHS number",
142+
"diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value is not a valid NHS number",
143143
},
144144
"should_be_string": {
145145
"code": "INVARIANT",

0 commit comments

Comments
 (0)