VED-1088-Validate-system-URL-in-patient-identifier-resource

lambdas/backend/tests/service/test_fhir_service.py

@@ -433,7 +433,8 @@ def test_patient_error(self):
         invalid_nhs_number = "9434765911"  # check digit 1 doesn't match result (9)
         imms = create_covid_immunization_dict_no_id(invalid_nhs_number)
         expected_msg = (
-            "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value is not a valid NHS number"
+            "Validation errors: contained[?(@.resourceType=='Patient')].identifier"
+            "[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value is not a valid NHS number"
         )
 
         with self.assertRaises(CustomValidationError) as error:
@@ -552,7 +553,8 @@ def test_update_immunization_raises_validation_exception_when_nhs_number_invalid
         self.imms_repo.update_immunization.assert_not_called()
         self.assertEqual(
             error.exception.message,
-            "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value must be 10 characters",
+            "Validation errors: contained[?(@.resourceType=='Patient')].identifier"
+            "[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value must be 10 characters",
         )
 
     def test_update_immunization_raises_not_found_error_when_no_existing_immunisation(self):

lambdas/recordforwarder/tests/controller/test_fhir_batch_controller.py

@@ -2,6 +2,7 @@
 import uuid
 from unittest.mock import Mock, create_autospec
 
+from common.models.constants import Urls
 from common.models.errors import (
     CustomValidationError,
     IdentifierDuplicationError,
@@ -18,6 +19,15 @@ def _make_immunization_pk(_id):
     return f"Immunization#{_id}"
 
 
+def _missing_patient_nhs_number_error():
+    return CustomValidationError(
+        message=(
+            "Validation errors: contained[?(@.resourceType=='Patient')].identifier"
+            f"[?(@.system=='{Urls.NHS_NUMBER}')].value does not exists"
+        )
+    )
+
+
 class TestCreateImmunizationBatchController(unittest.TestCase):
     def setUp(self):
         self.mock_repo = create_autospec(ImmunizationBatchRepository)
@@ -57,9 +67,7 @@ def test_send_request_to_dynamo_create_badrequest(self):
         imms_id = str(uuid.uuid4())
         imms_pk = _make_immunization_pk(imms_id)
         imms = create_covid_immunization(imms_id)
-        create_result = CustomValidationError(
-            message="Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists"
-        )
+        create_result = _missing_patient_nhs_number_error()
 
         message_body = {
             "supplier": "test_supplier",
@@ -177,9 +185,7 @@ def test_send_request_to_dynamo_update_badrequest(self):
         imms_id = str(uuid.uuid4())
         imms_pk = _make_immunization_pk(imms_id)
         imms = create_covid_immunization(imms_id)
-        update_result = CustomValidationError(
-            message="Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists"
-        )
+        update_result = _missing_patient_nhs_number_error()
         message_body = {
             "supplier": "test_supplier",
             "fhir_json": imms.json(),
@@ -296,9 +302,7 @@ def test_send_request_to_dynamo_delete_badrequest(self):
         imms_id = str(uuid.uuid4())
         imms_pk = _make_immunization_pk(imms_id)
         imms = create_covid_immunization(imms_id)
-        update_result = CustomValidationError(
-            message="Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists"
-        )
+        update_result = _missing_patient_nhs_number_error()
         message_body = {
             "supplier": "test_supplier",
             "fhir_json": imms.json(),

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

@@ -45,6 +45,7 @@ def validate(self):
             self.pre_validate_practitioner_reference,
             self.pre_validate_patient_identifier_extension,
             self.pre_validate_patient_identifier,
+            self.pre_validate_patient_identifier_system,
             self.pre_validate_patient_identifier_value,
             self.pre_validate_patient_name,
             self.pre_validate_patient_name_given,
@@ -271,17 +272,29 @@ def pre_validate_patient_identifier(self, values: dict) -> None:
         except (KeyError, IndexError):
             pass
 
-    def pre_validate_patient_identifier_value(self, values: dict) -> None:
+    def pre_validate_patient_identifier_system(self, values: dict) -> None:
         """
-        Pre-validate that, if contained[?(@.resourceType=='Patient')].identifier[0].value (
-        legacy CSV field name: NHS_NUMBER) exists, then it is a string of 10 characters
-        which does not contain spaces
+        Pre-validate that, if contained[?(@.resourceType=='Patient')].identifier[0].system exists,
+        then it is a non-empty string.
         """
-        field_location = "contained[?(@.resourceType=='Patient')].identifier[0].value"
+        field_location = "contained[?(@.resourceType=='Patient')].identifier[0].system"
         try:
             field_value = [x for x in values["contained"] if x.get("resourceType") == "Patient"][0]["identifier"][0][
-                "value"
+                "system"
             ]
+            PreValidation.for_string(field_value, field_location)
+        except (KeyError, IndexError):
+            pass
+
+    def pre_validate_patient_identifier_value(self, values: dict) -> None:
+        """
+        Pre-validate that, if the contained Patient has an NHS-number identifier,
+        its value is a string of 10 characters which does not contain spaces.
+        """
+        field_location = f"contained[?(@.resourceType=='Patient')].identifier[?(@.system=='{Urls.NHS_NUMBER}')].value"
+        try:
+            patient = [x for x in values["contained"] if x.get("resourceType") == "Patient"][0]
+            field_value = [x for x in patient["identifier"] if x.get("system") == Urls.NHS_NUMBER][0]["value"]
             PreValidation.for_string(field_value, field_location, defined_length=10, spaces_allowed=False)
             PreValidation.for_nhs_number(field_value, field_location)
         except (KeyError, IndexError):

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

@@ -25,7 +25,7 @@ class FieldLocations:
     target_disease = "protocolApplied[0].targetDisease"
     target_disease_codes = f"protocolApplied[0].targetDisease[0].coding[?(@.system=='{Urls.SNOMED}')].code"
     patient_identifier_value = (
-        "contained[?(@.resourceType=='Patient')].identifier[0].value"  # VED-1088 TODO: Fix to use nhs number url lookup
+        f"contained[?(@.resourceType=='Patient')].identifier[?(@.system=='{Urls.NHS_NUMBER}')].value"
     )
     patient_name_given: str = field(init=False)
     patient_name_family: str = field(init=False)

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

@@ -128,7 +128,10 @@ def get_occurrence_datetime(immunization: dict) -> datetime.datetime | None:
 
 
 def create_diagnostics():
-    diagnostics = "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value does not exists."
+    diagnostics = (
+        "Validation errors: contained[?(@.resourceType=='Patient')].identifier"
+        "[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value does not exists."
+    )
     exp_error = {"diagnostics": diagnostics}
     return exp_error
 

lambdas/shared/tests/test_common/test_immunization_post_validator.py

@@ -206,7 +206,9 @@ def test_post_patient_identifier_value(self):
         """
         Test that the JSON data is accepted when it does not contain patient_identifier_value
         """
-        field_location = "contained[?(@.resourceType=='Patient')].identifier[0].value"
+        field_location = (
+            "contained[?(@.resourceType=='Patient')].identifier[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value"
+        )
         self.mock_redis.hget.return_value = "COVID"
         self.mock_redis_getter.return_value = self.mock_redis
         MandationTests.test_missing_field_accepted(self, field_location)

lambdas/shared/tests/test_common/test_immunization_pre_validator.py

@@ -7,7 +7,7 @@
 
 from jsonpath_ng.ext import parse
 
-from common.models.constants import Constants
+from common.models.constants import Constants, Urls
 from common.models.fhir_immunization import ImmunizationValidator
 from common.models.fhir_immunization_pre_validators import PreValidators
 from common.models.utils.generic_utils import (
@@ -457,7 +457,7 @@ def test_pre_validate_patient_identifier_value(self):
         """Test pre_validate_patient_identifier_value accepts valid values and rejects invalid values"""
         ValidatorModelTests.test_string_value(
             self,
-            field_location="contained[?(@.resourceType=='Patient')].identifier[0].value",
+            field_location=f"contained[?(@.resourceType=='Patient')].identifier[?(@.system=='{Urls.NHS_NUMBER}')].value",
             valid_strings_to_test=["9990548609"],
             defined_length=10,
             invalid_length_strings_to_test=["999054860", "99905486091", ""],
@@ -470,6 +470,26 @@ def test_pre_validate_patient_identifier_value(self):
             ],
         )
 
+    def test_pre_validate_patient_identifier_value_accepts_non_nhs_identifier(self):
+        """Test pre_validate_patient_identifier_value ignores non-NHS patient identifiers"""
+        valid_json_data = deepcopy(self.json_data)
+        valid_json_data["contained"][1]["identifier"] = [
+            {
+                "system": "https://someother.codeableconcept.com/",
+                "value": "TVC15",
+            }
+        ]
+
+        self.assertIsNone(self.validator.validate(valid_json_data))
+
+    def test_pre_validate_patient_identifier_system(self):
+        """Test pre_validate_patient_identifier_system accepts valid values and rejects invalid values"""
+        ValidatorModelTests.test_string_value(
+            self,
+            field_location="contained[?(@.resourceType=='Patient')].identifier[0].system",
+            valid_strings_to_test=[Urls.NHS_NUMBER, "https://someother.codeableconcept.com/"],
+        )
+
     def test_pre_validate_patient_name(self):
         """Test pre_validate_patient_name accepts valid values and rejects invalid values"""
         ValidatorModelTests.test_list_value(

tests/e2e_automation/utilities/error_constants.py

@@ -15,7 +15,7 @@
     },
     "empty_NHSNumber": {
         "code": "INVALID",
-        "diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value must be a non-empty string",
+        "diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value must be a non-empty string",
     },
     "invalid_include": {
         "code": "INVALID",
@@ -99,7 +99,7 @@
     },
     "invalid_nhsnumber_length": {
         "code": "INVARIANT",
-        "diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value must be 10 characters",
+        "diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value must be 10 characters",
     },
     "no_forename": {
         "code": "INVARIANT",
@@ -139,7 +139,7 @@
     },
     "invalid_mod11_nhsnumber": {
         "code": "INVARIANT",
-        "diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[0].value is not a valid NHS number",
+        "diagnostics": "Validation errors: contained[?(@.resourceType=='Patient')].identifier[?(@.system=='https://fhir.nhs.uk/Id/nhs-number')].value is not a valid NHS number",
     },
     "should_be_string": {
         "code": "INVARIANT",