VED-1252: Site & Route - In case of multiple lists of valid system, it's storing only 1st list and ignores rest all lists in Imms Event table (#1446)

* Refactor FhirService validation to use deep copy of immunization data

- Updated `_validate_immunization` method to validate a deep copy of the immunization input, ensuring original data remains unchanged.
- Enhanced test cases to validate against the modified structure, ensuring all SNOMED codings are persisted during creation and update of immunizations.

* Add scenario to verify preservation of site and route codings in immunization event table

- Introduced a new scenario in the create.feature file to validate that site and route codings are preserved in the imms event table when the first coding system is invalid.
- Implemented corresponding step definitions in test_create_steps.py to create a valid JSON payload with multiple SNOMED codings and validate the response against expected outcomes.
- Enhanced the test to ensure that the delta table uses the first valid SNOMED site and route coding and that all codings from the request are preserved in the imms event table.

Author: Thomas-Boyle

lambdas/backend/src/service/fhir_service.py

@@ -97,10 +97,11 @@ def _normalize_single_snomed_codeable_concepts(cls, immunization: dict) -> None:
                 field["coding"] = cls._keep_first_snomed_coding(coding)
 
     def _validate_immunization(self, immunization: dict) -> None:
-        self._normalize_single_snomed_codeable_concepts(immunization)
+        immunization_to_validate = copy.deepcopy(immunization)
+        self._normalize_single_snomed_codeable_concepts(immunization_to_validate)
 
         try:
-            self.validator.validate(immunization)
+            self.validator.validate(immunization_to_validate)
         except (ValueError, MandatoryError) as error:
             raise CustomValidationError(message=str(error)) from error
 

lambdas/backend/tests/service/test_fhir_service.py

@@ -36,10 +36,19 @@
 NHS_NUMBER_USED_IN_SAMPLE_DATA = "9000000009"
 
 
-def add_snomed_coding(immunization: dict, field_name: str, code: str, display: str) -> dict:
-    first_coding = deepcopy(immunization[field_name]["coding"][0])
-    immunization[field_name]["coding"].append({"system": first_coding["system"], "code": code, "display": display})
-    return first_coding
+def add_duplicate_snomed_with_leading_non_snomed_coding(
+    immunization: dict, field_name: str, code: str, display: str
+) -> list:
+    first_snomed_coding = deepcopy(immunization[field_name]["coding"][0])
+    leading_non_snomed_coding = deepcopy(first_snomed_coding)
+    leading_non_snomed_coding["system"] = "http://snomed.info/test"
+    duplicate_snomed_coding = {**first_snomed_coding, "code": code, "display": display}
+    immunization[field_name]["coding"] = [
+        leading_non_snomed_coding,
+        first_snomed_coding,
+        duplicate_snomed_coding,
+    ]
+    return deepcopy(immunization[field_name]["coding"])
 
 
 class TestFhirServiceBase(unittest.TestCase):
@@ -354,6 +363,7 @@ def test_create_immunization(self):
 
         nhs_number = VALID_NHS_NUMBER
         req_imms = create_covid_immunization_dict_no_id(nhs_number)
+        expected_validated_imms = deepcopy(req_imms)
 
         # When
         created_id, created_version = self.fhir_service.create_immunization(req_imms, "Test")
@@ -366,30 +376,31 @@ def test_create_immunization(self):
         self.assertEqual(create_identifier.value, "ACME-vacc123456")
         self.imms_repo.create_immunization.assert_called_once_with(Immunization.parse_obj(req_imms), "Test")
 
-        self.validator.validate.assert_called_once_with(req_imms)
+        self.validator.validate.assert_called_once_with(expected_validated_imms)
         self.assertEqual(self._MOCK_NEW_UUID, created_id)
         self.assertEqual(1, created_version)
 
-    def test_create_immunization_keeps_first_site_and_route_snomed_coding(self):
-        """it should keep the first SNOMED coding for site and route during API create"""
+    def test_create_immunization_persists_all_site_and_route_codings(self):
+        """it should validate against the first SNOMED coding and persist all codings during API create"""
         self.mock_redis.hget.return_value = "COVID"
         self.mock_redis_getter.return_value = self.mock_redis
         self.authoriser.authorise.return_value = True
         self.imms_repo.get_immunization_by_identifier.return_value = (None, None)
         self.imms_repo.create_immunization.return_value = self._MOCK_NEW_UUID
 
         req_imms = create_covid_immunization_dict_no_id(VALID_NHS_NUMBER)
-        first_site_coding = add_snomed_coding(req_imms, "site", "999999999", "Replacement site that should be ignored")
-        first_route_coding = add_snomed_coding(
+        expected_site_codings = add_duplicate_snomed_with_leading_non_snomed_coding(
+            req_imms, "site", "999999999", "Replacement site that should be ignored"
+        )
+        expected_route_codings = add_duplicate_snomed_with_leading_non_snomed_coding(
             req_imms, "route", "888888888", "Replacement route that should be ignored"
         )
 
         created_id, created_version = self.pre_validate_fhir_service.create_immunization(req_imms, "Test")
 
         self.assertEqual(self._MOCK_NEW_UUID, created_id)
-        self.assertEqual(1, created_version)
-        self.assertEqual(req_imms["site"]["coding"], [first_site_coding])
-        self.assertEqual(req_imms["route"]["coding"], [first_route_coding])
+        self.assertEqual(req_imms["site"]["coding"], expected_site_codings)
+        self.assertEqual(req_imms["route"]["coding"], expected_route_codings)
         self.imms_repo.create_immunization.assert_called_once_with(Immunization.parse_obj(req_imms), "Test")
 
     def test_create_immunization_with_id_throws_error(self):
@@ -511,6 +522,7 @@ def test_raises_duplicate_error_if_identifier_already_exits(self):
 
         nhs_number = VALID_NHS_NUMBER
         req_imms = create_covid_immunization_dict_no_id(nhs_number)
+        expected_validated_imms = deepcopy(req_imms)
 
         # When
         with self.assertRaises(IdentifierDuplicationError) as error:
@@ -523,8 +535,7 @@ def test_raises_duplicate_error_if_identifier_already_exits(self):
         self.assertEqual(duplicate_identifier.system, "https://supplierABC/identifiers/vacc")
         self.assertEqual(duplicate_identifier.value, "ACME-vacc123456")
         self.imms_repo.create_immunization.assert_not_called()
-        self.imms_repo.update_immunization.assert_not_called()
-        self.validator.validate.assert_called_once_with(req_imms)
+        self.validator.validate.assert_called_once_with(expected_validated_imms)
         self.assertEqual(
             "The provided identifier: https://supplierABC/identifiers/vacc#ACME-vacc123456 is duplicated",
             str(error.exception),
@@ -614,19 +625,19 @@ def test_update_immunization(self):
         self.assertEqual(call_args[3], "Test")
         self.authoriser.authorise.assert_called_once_with("Test", ApiOperationCode.UPDATE, {"COVID"})
 
-    def test_update_immunization_keeps_first_site_and_route_snomed_coding(self):
-        """it should keep the first SNOMED coding for site and route during API update"""
+    def test_update_immunization_persists_all_site_and_route_codings(self):
+        """it should validate against the first SNOMED coding and persist all codings during API update"""
         imms_id = "an-id"
         original_immunisation = create_covid_immunization_dict(imms_id, VALID_NHS_NUMBER)
         identifier = Identifier(
             system=original_immunisation["identifier"][0]["system"],
             value=original_immunisation["identifier"][0]["value"],
         )
         updated_immunisation = create_covid_immunization_dict(imms_id, VALID_NHS_NUMBER, "2021-02-07T13:28:00+00:00")
-        first_site_coding = add_snomed_coding(
+        expected_site_codings = add_duplicate_snomed_with_leading_non_snomed_coding(
             updated_immunisation, "site", "999999999", "Replacement site that should be ignored"
         )
-        first_route_coding = add_snomed_coding(
+        expected_route_codings = add_duplicate_snomed_with_leading_non_snomed_coding(
             updated_immunisation, "route", "888888888", "Replacement route that should be ignored"
         )
         existing_resource_meta = ImmunizationRecordMetadata(
@@ -643,9 +654,11 @@ def test_update_immunization_keeps_first_site_and_route_snomed_coding(self):
         updated_version = self.fhir_service.update_immunization(imms_id, updated_immunisation, "Test", 1)
 
         self.assertEqual(updated_version, 2)
-        self.assertEqual(updated_immunisation["site"]["coding"], [first_site_coding])
-        self.assertEqual(updated_immunisation["route"]["coding"], [first_route_coding])
+        self.assertEqual(updated_immunisation["site"]["coding"], expected_site_codings)
+        self.assertEqual(updated_immunisation["route"]["coding"], expected_route_codings)
         self.imms_repo.update_immunization.assert_called_once()
+        call_args = self.imms_repo.update_immunization.call_args[0]
+        self.assertEqual(call_args[1], Immunization.parse_obj(updated_immunisation))
 
     def test_update_immunization_raises_validation_exception_when_nhs_number_invalid(self):
         """it should raise a CustomValidationError when the patient's NHS number in the payload is invalid"""

tests/e2e_automation/features/APITests/create.feature

@@ -92,6 +92,16 @@ Feature: Create the immunization event for a patient
         And The terms are mapped to correct instance of coding.display fields in imms delta table
         And MNS event will not be triggered for the event
 
+    @Delete_cleanUp @vaccine_type_RSV @patient_id_Random @supplier_name_Postman_Auth
+    Scenario: Verify that site and route codings are preserved in imms event table when the first coding system is invalid
+        Given Valid json payload is created where site and route have multiple SNOMED codings after an invalid system
+        When Trigger the post create request
+        Then The request will be successful with the status code '201'
+        And The location key and Etag in header will contain the Immunization Id and version
+        And The delta table will use the first valid SNOMED site and route coding
+        And The imms event table will preserve every site and route coding from the request
+        And MNS event will be triggered with correct data for created event
+
     @smoke
     @Delete_cleanUp @vaccine_type_PERTUSSIS @patient_id_Random @supplier_name_EMIS
     Scenario: Verify that VACCINATION_PROCEDURE_TERM, VACCINE_PRODUCT_TERM, SITE_OF_VACCINATION_TERM, ROUTE_OF_VACCINATION_TERM fields are mapped to coding.display in imms delta table in case of only one instance of coding

tests/e2e_automation/features/APITests/steps/test_create_steps.py

@@ -13,10 +13,12 @@
     parse_imms_int_imms_event_response,
     validate_imms_delta_record_with_created_event,
 )
+from src.objectModels.api_data_objects import CodeableConcept, Coding
 from src.objectModels.api_immunization_builder import (
     build_site_route,
     build_vaccine_procedure_code,
     build_vaccine_procedure_extension,
+    create_extension,
     get_vaccine_details,
 )
 from utilities.api_fhir_immunization_helper import (
@@ -121,6 +123,81 @@ def createValidJsonPayloadWithProcedureMultipleCodingsDifferentSystem(context):
     context.immunization_object.route.coding[0].system = "http://example.com/different-system"
 
 
+def build_coding(system, code, display, value_string, value_id):
+    return Coding(
+        system=system,
+        code=code,
+        display=display,
+        extension=[
+            create_extension(
+                "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-CodingSCTDescDisplay",
+                stringValue=value_string,
+            ),
+            create_extension(
+                "http://hl7.org/fhir/StructureDefinition/coding-sctdescid",
+                idValue=value_id,
+            ),
+        ],
+    )
+
+
+@given("Valid json payload is created where site and route have multiple SNOMED codings after an invalid system")
+def create_valid_json_payload_with_multiple_site_route_snomed_codings(context):
+    valid_json_payload_is_created(context)
+    context.immunization_object.site = CodeableConcept(
+        coding=[
+            build_coding(
+                "http://snomed.info/test",
+                "368208006",
+                "Right upper arm structure (body structure)",
+                "Test Value string site 111",
+                "5306706018",
+            ),
+            build_coding(
+                "http://snomed.info/sct",
+                "368209003",
+                "Left upper arm structure (body structure)",
+                "Test Value string site 222",
+                "5306706020",
+            ),
+            build_coding(
+                "http://snomed.info/sct",
+                "368208008",
+                "Mid upper arm structure (body structure)",
+                "Test Value string site 333",
+                "5306706030",
+            ),
+        ],
+        text="Test String for site",
+    )
+    context.immunization_object.route = CodeableConcept(
+        coding=[
+            build_coding(
+                "http://snomed.info/test",
+                "78421000",
+                "Intramuscular route (qualifier value)",
+                "Test Value string route 111",
+                "5306706040",
+            ),
+            build_coding(
+                "http://snomed.info/sct",
+                "78421000",
+                "Intramuscular route (qualifier value)",
+                "Test Value string route 222",
+                "5306706050",
+            ),
+            build_coding(
+                "http://snomed.info/sct",
+                "34206005",
+                "Subcutaneous route (qualifier value)",
+                "Test Value string route 333",
+                "5306706060",
+            ),
+        ],
+        text="Test String for route",
+    )
+
+
 @given(
     "Valid json payload is created where vaccination terms has one instance of coding with no text or value string field"
 )
@@ -267,6 +344,37 @@ def validate_procedure_term_correct_coding_in_delta_table(context):
     )
 
 
+@then("The delta table will use the first valid SNOMED site and route coding")
+def validate_delta_table_uses_first_valid_snomed_site_route_coding(context):
+    actual_terms = get_all_term_text(context)
+    expected_site_term = context.create_object.site.coding[1].extension[0].valueString
+    expected_route_term = context.create_object.route.coding[1].extension[0].valueString
+    assert actual_terms["site_term"] == expected_site_term, (
+        f"Expected site of vaccination term '{expected_site_term}', but got '{actual_terms['site_term']}'"
+    )
+    assert actual_terms["route_term"] == expected_route_term, (
+        f"Expected route of vaccination term '{expected_route_term}', but got '{actual_terms['route_term']}'"
+    )
+
+
+@then("The imms event table will preserve every site and route coding from the request")
+def validate_imms_event_table_preserves_all_site_route_codings(context):
+    table_query_response = fetch_immunization_events_detail(context.aws_profile_name, context.ImmsID, context.S3_env)
+    assert "Item" in table_query_response, f"Item not found in response for ImmsID: {context.ImmsID}"
+
+    resource_json_str = table_query_response["Item"].get("Resource")
+    assert resource_json_str, "Resource field missing in item."
+    resource = json.loads(resource_json_str)
+
+    for field_name in ("site", "route"):
+        expected = context.request[field_name]
+        actual = resource[field_name]
+        assert len(actual["coding"]) == 3, (
+            f"Expected {field_name}.coding to contain 3 entries, but got {len(actual['coding'])}"
+        )
+        assert actual == expected, f"Expected {field_name} to match request, but got {actual}"
+
+
 @then("The terms are mapped to correct coding.display fields in imms delta table")
 def validate_procedure_term_second_display_in_delta_table(context):
     actual_terms = get_all_term_text(context)