diff --git a/lambdas/backend/src/service/fhir_service.py b/lambdas/backend/src/service/fhir_service.py index 36f1213062..98a15afa8f 100644 --- a/lambdas/backend/src/service/fhir_service.py +++ b/lambdas/backend/src/service/fhir_service.py @@ -22,7 +22,7 @@ from authorisation.api_operation_code import ApiOperationCode from authorisation.authoriser import Authoriser from common.get_service_url import get_service_url -from common.models.constants import Constants +from common.models.constants import Constants, Urls from common.models.errors import ( Code, CustomValidationError, @@ -62,6 +62,7 @@ class FhirService: _DATA_MISSING_DATE_TIME_ERROR_MSG = ( "Data quality issue - immunisation with ID %s was found containing no occurrenceDateTime" ) + _SINGLE_SNOMED_CODEABLE_CONCEPT_FIELDS = ("site", "route") def __init__( self, @@ -73,6 +74,36 @@ def __init__( self.immunization_repo = imms_repo self.validator = validator + @staticmethod + def _keep_first_snomed_coding(coding: list) -> list: + snomed_seen = False + filtered_coding = [] + for coding_entry in coding: + is_snomed_coding = isinstance(coding_entry, dict) and coding_entry.get("system") == Urls.SNOMED + if is_snomed_coding and snomed_seen: + continue + + snomed_seen = snomed_seen or is_snomed_coding + filtered_coding.append(coding_entry) + + return filtered_coding + + @classmethod + def _normalize_single_snomed_codeable_concepts(cls, immunization: dict) -> None: + for field_name in cls._SINGLE_SNOMED_CODEABLE_CONCEPT_FIELDS: + field = immunization.get(field_name) + coding = field.get("coding") if isinstance(field, dict) else None + if isinstance(coding, list): + field["coding"] = cls._keep_first_snomed_coding(coding) + + def _validate_immunization(self, immunization: dict) -> None: + self._normalize_single_snomed_codeable_concepts(immunization) + + try: + self.validator.validate(immunization) + except (ValueError, MandatoryError) as error: + raise CustomValidationError(message=str(error)) from error + def get_immunization_by_identifier( self, identifier: Identifier, supplier_name: str, elements: set[str] | None ) -> FhirBundle: @@ -117,10 +148,7 @@ def create_immunization(self, immunization: dict, supplier_system: str) -> Id: if immunization.get("id") is not None: raise CustomValidationError("id field must not be present for CREATE operation") - try: - self.validator.validate(immunization) - except (ValueError, MandatoryError) as error: - raise CustomValidationError(message=str(error)) from error + self._validate_immunization(immunization) vaccination_type = get_vaccine_type(immunization) @@ -139,10 +167,7 @@ def create_immunization(self, immunization: dict, supplier_system: str) -> Id: return self.immunization_repo.create_immunization(immunization_fhir_entity, supplier_system) def update_immunization(self, imms_id: str, immunization: dict, supplier_system: str, resource_version: int) -> int: - try: - self.validator.validate(immunization) - except (ValueError, MandatoryError) as error: - raise CustomValidationError(message=str(error)) from error + self._validate_immunization(immunization) immunization_to_update = Immunization.parse_obj(immunization) diff --git a/lambdas/backend/tests/service/test_fhir_service.py b/lambdas/backend/tests/service/test_fhir_service.py index 6f6edcbe18..91a1ec6f3a 100644 --- a/lambdas/backend/tests/service/test_fhir_service.py +++ b/lambdas/backend/tests/service/test_fhir_service.py @@ -36,6 +36,12 @@ 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 + + class TestFhirServiceBase(unittest.TestCase): """Base class for all tests to set up common fixtures""" @@ -362,6 +368,27 @@ def test_create_immunization(self): self.validator.validate.assert_called_once_with(req_imms) self.assertEqual(self._MOCK_NEW_UUID, created_id) + 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""" + 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.check_immunization_identifier_exists.return_value = False + 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( + req_imms, "route", "888888888", "Replacement route that should be ignored" + ) + + created_id = self.pre_validate_fhir_service.create_immunization(req_imms, "Test") + + self.assertEqual(self._MOCK_NEW_UUID, created_id) + self.assertEqual(req_imms["site"]["coding"], [first_site_coding]) + self.assertEqual(req_imms["route"]["coding"], [first_route_coding]) + self.imms_repo.create_immunization.assert_called_once_with(Immunization.parse_obj(req_imms), "Test") + def test_create_immunization_with_id_throws_error(self): """it should throw exception if id present in create Immunization""" imms = create_covid_immunization_dict("an-id", "9990548609") @@ -539,6 +566,39 @@ 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""" + 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( + updated_immunisation, "site", "999999999", "Replacement site that should be ignored" + ) + first_route_coding = add_snomed_coding( + updated_immunisation, "route", "888888888", "Replacement route that should be ignored" + ) + existing_resource_meta = ImmunizationRecordMetadata( + identifier=identifier, resource_version=1, is_deleted=False, is_reinstated=False + ) + + self.imms_repo.get_immunization_resource_and_metadata_by_id.return_value = ( + original_immunisation, + existing_resource_meta, + ) + self.imms_repo.update_immunization.return_value = 2 + self.authoriser.authorise.return_value = True + + 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.imms_repo.update_immunization.assert_called_once() + 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""" imms_id = "an-id" diff --git a/lambdas/recordforwarder/tests/service/test_fhir_batch_service.py b/lambdas/recordforwarder/tests/service/test_fhir_batch_service.py index 1cd2b5f334..6b5056b518 100644 --- a/lambdas/recordforwarder/tests/service/test_fhir_batch_service.py +++ b/lambdas/recordforwarder/tests/service/test_fhir_batch_service.py @@ -10,6 +10,10 @@ from test_common.testing_utils.immunization_utils import create_covid_immunization_dict_no_id +def duplicate_first_coding(immunization: dict, field_name: str) -> None: + immunization[field_name]["coding"].append(deepcopy(immunization[field_name]["coding"][0])) + + class TestFhirBatchServiceBase(unittest.TestCase): """Base class for all tests to set up common fixtures""" @@ -97,6 +101,25 @@ def test_create_immunization_post_validation_error(self): self.assertTrue(expected_msg in error.exception.message) self.mock_repo.create_immunization.assert_not_called() + def test_create_immunization_duplicate_site_snomed_still_rejected_for_batch(self): + """it should keep batch validation unchanged for duplicate site SNOMED codings""" + + imms = create_covid_immunization_dict_no_id() + duplicate_first_coding(imms, "site") + expected_msg = "Validation errors: site.coding[?(@.system=='http://snomed.info/sct')] must be unique" + + with self.assertRaises(CustomValidationError) as error: + self.pre_validate_fhir_service.create_immunization( + immunization=imms, + supplier_system="test_supplier", + vax_type="test_vax", + table=self.mock_table, + imms_pk=None, + ) + + self.assertEqual(expected_msg, error.exception.message) + self.mock_repo.create_immunization.assert_not_called() + class TestUpdateImmunizationBatchService(TestFhirBatchServiceBase): def setUp(self): diff --git a/tests/e2e_automation/features/APITests/create.feature b/tests/e2e_automation/features/APITests/create.feature index 072feae92f..877dd873d4 100644 --- a/tests/e2e_automation/features/APITests/create.feature +++ b/tests/e2e_automation/features/APITests/create.feature @@ -75,7 +75,7 @@ Feature: Create the immunization event for a patient And MNS event will be triggered with correct data for created event @Delete_cleanUp @vaccine_type_BCG @patient_id_InvalidInPDS @supplier_name_EMIS - Scenario: Verify that VACCINATION_PROCEDURE_TERM, VACCINE_PRODUCT_TERM fields are mapped to first instance of coding.display fields in imms delta table + Scenario: Verify that VACCINATION_PROCEDURE_TERM, VACCINE_PRODUCT_TERM , SITE_OF_VACCINATION_TERM, ROUTE_OF_VACCINATION_TERM fields are mapped to first instance of coding.display fields in imms delta table Given Valid json payload is created where vaccination terms has multiple instances of coding When Trigger the post create request Then The request will be successful with the status code '201' diff --git a/tests/e2e_automation/features/APITests/steps/test_create_steps.py b/tests/e2e_automation/features/APITests/steps/test_create_steps.py index 181ea11547..2ceeaf2fe5 100644 --- a/tests/e2e_automation/features/APITests/steps/test_create_steps.py +++ b/tests/e2e_automation/features/APITests/steps/test_create_steps.py @@ -94,9 +94,13 @@ def createValidJsonPayloadWithProcedureMultipleCodings(context): valid_json_payload_is_created(context) procedures_list = get_all_the_vaccination_codes(VACCINATION_PROCEDURE_MAP[context.vaccine_type.upper()]) product_list = get_all_the_vaccination_codes(VACCINE_CODE_MAP[context.vaccine_type.upper()]) + site_list = get_all_the_vaccination_codes(SITE_MAP) + route_list = get_all_the_vaccination_codes(ROUTE_MAP) context.immunization_object.extension[0].valueCodeableConcept.coding = procedures_list context.immunization_object.vaccineCode.coding = product_list + context.immunization_object.site.coding = site_list + context.immunization_object.route.coding = route_list @given( diff --git a/tests/e2e_automation/utilities/vaccination_constants.py b/tests/e2e_automation/utilities/vaccination_constants.py index 2de8376a6f..02f5a9a2ca 100644 --- a/tests/e2e_automation/utilities/vaccination_constants.py +++ b/tests/e2e_automation/utilities/vaccination_constants.py @@ -672,8 +672,16 @@ ] REASON_CODE_MAP = [ - {"system": "http://snomed.info/sct", "code": "443684005", "display": "Disease outbreak (event)"}, - {"system": "http://snomed.info/sct", "code": "310578008", "display": "Routine immunization schedule"}, + { + "system": "http://snomed.info/sct", + "code": "443684005", + "display": "Disease outbreak (event)", + }, + { + "system": "http://snomed.info/sct", + "code": "310578008", + "display": "Routine immunization schedule", + }, ] PROTOCOL_DISEASE_MAP = { @@ -686,9 +694,19 @@ ], "FLU": [{"system": "http://snomed.info/sct", "code": "6142004", "display": "Influenza"}], "RSV": [ - {"system": "http://snomed.info/sct", "code": "55735004", "display": "Respiratory syncytial virus infection"} + { + "system": "http://snomed.info/sct", + "code": "55735004", + "display": "Respiratory syncytial virus infection", + } + ], + "HPV": [ + { + "system": "http://snomed.info/sct", + "code": "240532009", + "display": "Human papilloma virus infection", + } ], - "HPV": [{"system": "http://snomed.info/sct", "code": "240532009", "display": "Human papilloma virus infection"}], "MMR": [ {"system": "http://snomed.info/sct", "code": "14189004", "display": "Measles"}, {"system": "http://snomed.info/sct", "code": "36989005", "display": "Mumps"}, @@ -698,15 +716,33 @@ {"system": "http://snomed.info/sct", "code": "14189004", "display": "Measles"}, {"system": "http://snomed.info/sct", "code": "36989005", "display": "Mumps"}, {"system": "http://snomed.info/sct", "code": "36653000", "display": "Rubella"}, - {"system": "http://snomed.info/sct", "code": "38907003", "display": "Varicella"}, + { + "system": "http://snomed.info/sct", + "code": "38907003", + "display": "Varicella", + }, ], "PERTUSSIS": [{"system": "http://snomed.info/sct", "code": "27836007", "display": "Pertussis"}], - "SHINGLES": [{"system": "http://snomed.info/sct", "code": "4740000", "display": "Herpes zoster"}], + "SHINGLES": [ + { + "system": "http://snomed.info/sct", + "code": "4740000", + "display": "Herpes zoster", + } + ], "PNEUMOCOCCAL": [ - {"system": "http://snomed.info/sct", "code": "16814004", "display": "Pneumococcal infectious disease"} + { + "system": "http://snomed.info/sct", + "code": "16814004", + "display": "Pneumococcal infectious disease", + } ], "3IN1": [ - {"system": "http://snomed.info/sct", "code": "398102009", "display": "Acute poliomyelitis"}, + { + "system": "http://snomed.info/sct", + "code": "398102009", + "display": "Acute poliomyelitis", + }, { "system": "http://snomed.info/sct", "code": "397430003", @@ -714,33 +750,79 @@ }, {"system": "http://snomed.info/sct", "code": "76902006", "display": "Tetanus"}, ], - "MENACWY": [{"system": "http://snomed.info/sct", "code": "23511006", "display": "Meningococcal infectious disease"}], + "MENACWY": [ + { + "system": "http://snomed.info/sct", + "code": "23511006", + "display": "Meningococcal infectious disease", + } + ], "4IN1": [ - {"system": "http://snomed.info/sct", "code": "398102009", "display": "Acute poliomyelitis"}, + { + "system": "http://snomed.info/sct", + "code": "398102009", + "display": "Acute poliomyelitis", + }, { "system": "http://snomed.info/sct", "code": "397430003", "display": "Diphtheria caused by Corynebacterium diphtheriae", }, - {"system": "http://snomed.info/sct", "code": "27836007", "display": "Pertussis"}, + { + "system": "http://snomed.info/sct", + "code": "27836007", + "display": "Pertussis", + }, {"system": "http://snomed.info/sct", "code": "76902006", "display": "Tetanus"}, ], "6IN1": [ - {"system": "http://snomed.info/sct", "code": "398102009", "display": "Acute poliomyelitis"}, + { + "system": "http://snomed.info/sct", + "code": "398102009", + "display": "Acute poliomyelitis", + }, { "system": "http://snomed.info/sct", "code": "397430003", "display": "Diphtheria caused by Corynebacterium diphtheriae", }, - {"system": "http://snomed.info/sct", "code": "709410003", "display": "Haemophilus influenzae type b infection"}, - {"system": "http://snomed.info/sct", "code": "27836007", "display": "Pertussis"}, + { + "system": "http://snomed.info/sct", + "code": "709410003", + "display": "Haemophilus influenzae type b infection", + }, + { + "system": "http://snomed.info/sct", + "code": "27836007", + "display": "Pertussis", + }, {"system": "http://snomed.info/sct", "code": "76902006", "display": "Tetanus"}, - {"system": "http://snomed.info/sct", "code": "66071002", "display": "Type B viral hepatitis"}, + { + "system": "http://snomed.info/sct", + "code": "66071002", + "display": "Type B viral hepatitis", + }, + ], + "BCG": [ + { + "system": "http://snomed.info/sct", + "code": "56717001", + "display": "Tuberculosis", + } + ], + "HEPB": [ + { + "system": "http://snomed.info/sct", + "code": "66071002", + "display": "Type B viral hepatitis", + } ], - "BCG": [{"system": "http://snomed.info/sct", "code": "56717001", "display": "Tuberculosis"}], - "HEPB": [{"system": "http://snomed.info/sct", "code": "66071002", "display": "Type B viral hepatitis"}], "HIB": [ - {"system": "http://snomed.info/sct", "code": "709410003", "display": "Haemophilus influenzae type b infection"} + { + "system": "http://snomed.info/sct", + "code": "709410003", + "display": "Haemophilus influenzae type b infection", + } ], "MENB": [ { @@ -749,5 +831,11 @@ "display": "Meningococcal infectious disease caused by Neisseria meningitidis serogroup B", } ], - "ROTAVIRUS": [{"system": "http://snomed.info/sct", "code": "186150001", "display": "Enteritis caused by rotavirus"}], + "ROTAVIRUS": [ + { + "system": "http://snomed.info/sct", + "code": "186150001", + "display": "Enteritis caused by rotavirus", + } + ], }