Merge branch 'master' into ved-1138-allow-create-to-reinstate-deleted-records

Author: motola

.github/workflows/dependabot-auto-approve.yml

@@ -14,7 +14,7 @@ jobs:
       - name: Fetch Dependabot metadata
         id: metadata
         continue-on-error: true
-        uses: dependabot/fetch-metadata@ffa630c65fa7e0ecfa0625b5ceda64399aea1b36
+        uses: dependabot/fetch-metadata@25dd0e34f4fe68f24cc83900b1fe3fe149efef98
 
       - name: Auto-approve minor and patch updates
         if: steps.metadata.outcome == 'success' && contains(fromJSON('["version-update:semver-minor", "version-update:semver-patch"]'), steps.metadata.outputs.update-type)

.github/workflows/quality-checks.yml

@@ -18,7 +18,7 @@ jobs:
     steps:
       - uses: actions/checkout@0c366fd6a839edf440554fa01a7085ccba70ac98
 
-      - uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f
+      - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e
         with:
           node-version: "23.11.0"
           cache: "npm"

config/dev/permissions_config.json

@@ -46,6 +46,7 @@
       "FLU.CRUDS",
       "HPV.CRUDS",
       "MENACWY.CRUDS",
+      "MENB.CRUDS",
       "MMR.CRUDS",
       "MMRV.CRUDS",
       "PERTUSSIS.CRUDS",

config/preprod/permissions_config.json

@@ -43,6 +43,7 @@
       "FLU.CRUDS",
       "HPV.CRUDS",
       "MENACWY.CRUDS",
+      "MENB.CRUDS",
       "MMR.CRUDS",
       "MMRV.CRUDS",
       "PERTUSSIS.CRUDS",

config/prod/permissions_config.json

@@ -25,7 +25,12 @@
   },
   {
     "supplier": "RAVS",
-    "permissions": ["MMR.CRUDS", "RSV.CRUDS", "PNEUMOCOCCAL.CRUDS"],
+    "permissions": [
+      "MENB.CRUDS",
+      "MMR.CRUDS",
+      "RSV.CRUDS",
+      "PNEUMOCOCCAL.CRUDS"
+    ],
     "ods_codes": ["X26", "X8E5B"]
   },
   {

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) -> tuple
         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)
 
@@ -155,10 +183,7 @@ def create_immunization(self, immunization: dict, supplier_system: str) -> tuple
         return created_id, 1
 
     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)
 

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"""
 
@@ -364,6 +370,27 @@ def test_create_immunization(self):
         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"""
+        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")
@@ -586,6 +613,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"

lambdas/delta_backend/src/converter.py

@@ -1,32 +1,29 @@
 # Main validation engine
+from typing import Any
+
 import exception_messages
 from conversion_layout import ConversionField, ConversionLayout
 from extractor import Extractor
 from mappings import ActionFlag
 
+ConversionErrorRecord = dict[str, Any]
+ConvertedRecord = dict[str, Any]
+
 
 class Converter:
-    def __init__(self, fhir_data, action_flag=ActionFlag.UPDATE, report_unexpected_exception=True):
-        self.converted = {}
-        self.error_records = []
+    def __init__(self, fhir_data: str | dict[str, Any], action_flag: str = ActionFlag.UPDATE) -> None:
+        self.converted: ConvertedRecord = {}
+        self.error_records: list[ConversionErrorRecord] = []
         self.action_flag = action_flag
-        self.report_unexpected_exception = report_unexpected_exception
-
-        try:
-            if not fhir_data:
-                raise ValueError("FHIR data is required for initialization.")
 
-            self.extractor = Extractor(fhir_data, self.report_unexpected_exception)
-            self.conversion_layout = ConversionLayout(self.extractor)
-        except Exception as e:
-            if report_unexpected_exception:
-                self._log_error(f"Initialization failed: [{e.__class__.__name__}] {e}")
-            raise
+        if not fhir_data:
+            raise ValueError("FHIR data is required for initialization.")
 
-    def run_conversion(self):
-        conversions = self.conversion_layout.get_conversion_layout()
+        self.extractor = Extractor(fhir_data)
+        self.conversion_layout = ConversionLayout(self.extractor)
 
-        for conversion in conversions:
+    def run_conversion(self) -> ConvertedRecord:
+        for conversion in self.conversion_layout.get_conversion_layout():
             self._convert_data(conversion)
 
         self.error_records.extend(self.extractor.get_error_records())
@@ -35,29 +32,33 @@ def run_conversion(self):
         self.converted["CONVERSION_ERRORS"] = self.error_records
         return self.converted
 
-    def _convert_data(self, conversion: ConversionField):
-        try:
-            flat_field = conversion.field_name_flat
+    def _convert_data(self, conversion: ConversionField) -> None:
+        flat_field = conversion.field_name_flat
 
+        try:
             if flat_field == "ACTION_FLAG":
                 self.converted[flat_field] = self.action_flag
-            else:
-                converted = conversion.expression_rule()
-                if converted is not None:
-                    self.converted[flat_field] = converted
+                return
 
-        except Exception as e:
+            if (converted := conversion.expression_rule()) is not None:
+                self.converted[flat_field] = converted
+        except Exception as error:
             self._log_error(
-                f"Conversion error [{e.__class__.__name__}]: {e}",
+                flat_field,
+                f"Conversion error [{error.__class__.__name__}]: {error}",
                 code=exception_messages.PARSING_ERROR,
             )
             self.converted[flat_field] = ""
 
-    def _log_error(self, e, code=exception_messages.UNEXPECTED_EXCEPTION):
-        error_obj = {"code": code, "message": str(e)}
-
-        if self.report_unexpected_exception:
-            self.error_records.append(error_obj)
+    def _log_error(self, field_name: str, e: Exception | str, code: str) -> None:
+        self.error_records.append(
+            {
+                "code": code,
+                "field": field_name,
+                "value": None,
+                "message": str(e),
+            }
+        )
 
-    def get_error_records(self):
+    def get_error_records(self) -> list[ConversionErrorRecord]:
         return self.error_records

lambdas/delta_backend/src/extractor.py

@@ -22,13 +22,12 @@ class Extractor:
     DATE_CONVERT_FORMAT = "%Y%m%d"
     DEFAULT_POSTCODE = "ZZ99 3CZ"
 
-    def __init__(self, fhir_json_data, report_unexpected_exception=True):
+    def __init__(self, fhir_json_data):
         self.fhir_json_data = (
             json.loads(fhir_json_data, parse_float=decimal.Decimal)
             if isinstance(fhir_json_data, str)
             else fhir_json_data
         )
-        self.report_unexpected_exception = report_unexpected_exception
         self.error_records = []
 
     def _get_patient(self):
@@ -174,23 +173,20 @@ def _get_site_information(self):
         return site_code, site_code_type_uri
 
     def _log_error(self, field_name, field_value, e, code=exception_messages.RECORD_CHECK_FAILED):
-        if self.report_unexpected_exception:
-            if isinstance(e, Exception):
-                message = exception_messages.MESSAGES[exception_messages.UNEXPECTED_EXCEPTION] % (
-                    e.__class__.__name__,
-                    str(e),
-                )
-            else:
-                message = str(e)
-
-            self.error_records.append(
-                {
-                    "code": code,
-                    "field": field_name,
-                    "value": field_value,
-                    "message": message,
-                }
-            )
+        message = (
+            exception_messages.MESSAGES[exception_messages.UNEXPECTED_EXCEPTION] % (e.__class__.__name__, str(e))
+            if isinstance(e, Exception)
+            else str(e)
+        )
+
+        self.error_records.append(
+            {
+                "code": code,
+                "field": field_name,
+                "value": field_value,
+                "message": message,
+            }
+        )
 
     def _convert_date(self, field_name, date, format) -> str:
         """