merge branch

Author: Akol125

lambdas/backend/src/controller/parameter_parser.py

@@ -107,6 +107,7 @@ def process_optional_params(
     """Parse optional params (date.from, date.to, _include).
     Raises ParameterExceptionError for any validation error.
     """
+    errors = []
     include = None
     date_from = None
     date_to = None
@@ -117,36 +118,31 @@ def process_optional_params(
 
     if date_froms:
         if len(date_froms) != 1:
-            raise ParameterExceptionError(
-                f"Search parameter {ImmunizationSearchParameterName.DATE_FROM} may have one value at most."
-            )
+            errors.append(f"Search parameter {ImmunizationSearchParameterName.DATE_FROM} may have one value at most.")
         try:
             date_from = datetime.datetime.strptime(date_froms[0], "%Y-%m-%d").date()
         except ValueError:
-            raise ParameterExceptionError(
-                f"Search parameter {ImmunizationSearchParameterName.DATE_FROM} must be in format: YYYY-MM-DD"
-            )
+            errors.append(f"Search parameter {ImmunizationSearchParameterName.DATE_FROM} must be in format: YYYY-MM-DD")
 
     if date_tos:
         if len(date_tos) != 1:
-            raise ParameterExceptionError(
-                f"Search parameter {ImmunizationSearchParameterName.DATE_TO} may have one value at most."
-            )
+            errors.append(f"Search parameter {ImmunizationSearchParameterName.DATE_TO} may have one value at most.")
         try:
             date_to = datetime.datetime.strptime(date_tos[0], "%Y-%m-%d").date()
         except ValueError:
-            raise ParameterExceptionError(
-                f"Search parameter {ImmunizationSearchParameterName.DATE_TO} must be in format: YYYY-MM-DD"
-            )
+            errors.append(f"Search parameter {ImmunizationSearchParameterName.DATE_TO} must be in format: YYYY-MM-DD")
 
     if includes:
         if includes[0].lower() != "immunization:patient":
-            raise ParameterExceptionError(
+            errors.append(
                 f"Search parameter {ImmunizationSearchParameterName.INCLUDE} may only be "
                 f"'Immunization:patient' if provided."
             )
         include = includes[0]
 
+    if errors:
+        raise ParameterExceptionError("; ".join(errors))
+
     return date_from, date_to, include
 
 

lambdas/backend/src/service/fhir_service.py

@@ -4,7 +4,7 @@
 import os
 import urllib.parse
 import uuid
-from typing import Optional, cast
+from typing import Any, Optional, cast
 from uuid import uuid4
 
 from fhir.resources.R4B.bundle import (
@@ -36,7 +36,6 @@
 from common.models.utils.generic_utils import (
     get_contained_patient,
     get_occurrence_datetime,
-    make_search_bundle,
 )
 from common.models.utils.validation_utils import (
     get_vaccine_type,
@@ -103,7 +102,7 @@ def get_immunization_by_identifier(
         resource, resource_metadata = self.immunization_repo.get_immunization_by_identifier(identifier)
 
         if not resource:
-            return make_search_bundle(resource, None, elements, identifier, base_url)
+            return self.make_empty_identifier_search_bundle(base_url)
 
         vaccination_type = get_vaccine_type(resource)
 
@@ -113,7 +112,9 @@ def get_immunization_by_identifier(
         patient_full_url = f"urn:uuid:{str(uuid4())}"
         filtered_resource = Filter.search(resource, patient_full_url)
 
-        return make_search_bundle(filtered_resource, resource_metadata.resource_version, elements, identifier, base_url)
+        return self.make_identifier_search_bundle(
+            filtered_resource, resource_metadata.resource_version, elements, identifier, base_url
+        )
 
     def get_immunization_and_version_by_id(self, imms_id: str, supplier_system: str) -> tuple[Immunization, str]:
         """
@@ -336,6 +337,55 @@ def is_valid_date_to(self, immunization: dict, date_to: Optional[datetime.date])
 
         return occurrence_datetime.date() <= date_to
 
+    @staticmethod
+    def make_identifier_search_bundle(
+        resource: Optional[dict],
+        version_id: Optional[int],
+        elements: Optional[set[str]],
+        identifier: Identifier,
+        base_url: str,
+    ) -> FhirBundle:
+        searched_url = f"{base_url}?identifier={identifier.system}|{identifier.value}" + (
+            f"&_elements={','.join(sorted(elements))}" if elements else ""
+        )
+
+        meta = {"versionId": version_id}
+
+        # Full Immunization payload to be returned if only the identifier parameter was provided and truncated when
+        # _elements is used
+        if elements is not None:
+            resource_for_bundle: dict[str, Any] = {"resourceType": "Immunization"}
+            if "id" in elements:
+                resource_for_bundle["id"] = resource["id"]
+            if "meta" in elements:
+                resource_for_bundle["meta"] = meta
+
+        else:
+            resource_for_bundle = copy.deepcopy(resource)
+            resource_for_bundle["meta"] = meta
+
+        entry = BundleEntry.construct(
+            fullUrl=f"{base_url}/{resource['id']}",
+            resource=(
+                Immunization.construct(**resource_for_bundle)
+                if elements
+                else Immunization.parse_obj(resource_for_bundle)
+            ),
+            search=BundleEntrySearch.construct(mode="match") if not elements else None,
+        )
+
+        return FhirBundle(
+            type="searchset",
+            link=[BundleLink(relation="self", url=searched_url)],
+            entry=[entry],
+            total=1,
+        )
+
+    @staticmethod
+    def make_empty_identifier_search_bundle(base_url: str) -> FhirBundle:
+        no_results_url = f"{base_url}?identifier=None"
+        return FhirBundle(entry=[], link=[BundleLink(relation="self", url=no_results_url)], type="searchset", total=0)
+
     @staticmethod
     def process_patient_for_bundle(patient: dict):
         """

lambdas/backend/tests/controller/test_parameter_parser.py

@@ -178,8 +178,10 @@ def test_process_search_params_raises_date_errors(self):
                     self.date_to_key: ["invalid-date"],  # invalid format
                 }
             )
-
-        expected = f"Search parameter {self.date_from_key} may have one value at most."
+        expected = (
+            f"Search parameter {self.date_from_key} may have one value at most.; "
+            f"Search parameter {self.date_to_key} must be in format: YYYY-MM-DD"
+        )
         self.assertEqual(str(e.exception), expected)
 
     def test_process_search_params_invalid_nhs_number_is_rejected(self):

lambdas/backend/tests/service/test_fhir_service.py

@@ -280,8 +280,7 @@ def test_get_immunization_by_identifier_returns_empty_search_when_not_found(self
             result.link[0],
             BundleLink.construct(
                 relation="self",
-                url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?identifier=some-"
-                "system|some-value",
+                url="https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/FHIR/R4/Immunization?identifier=None",
             ),
         )
         self.assertEqual(result.entry, [])

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

@@ -1,21 +1,10 @@
 """Generic utilities"""
 
 import base64
-import copy
 import datetime
 import urllib.parse
-from typing import Any, Literal, Optional
-
-from fhir.resources.R4B.bundle import (
-    Bundle as FhirBundle,
-)
-from fhir.resources.R4B.bundle import (
-    BundleEntry,
-    BundleEntrySearch,
-    BundleLink,
-)
-from fhir.resources.R4B.identifier import Identifier
-from fhir.resources.R4B.immunization import Immunization
+from typing import Literal, Optional
+
 from stdnum.verhoeff import validate
 
 from common.models.constants import Constants
@@ -155,56 +144,6 @@ def create_diagnostics_error(value):
     return exp_error
 
 
-def make_empty_search_bundle(searched_url: str) -> FhirBundle:
-    return FhirBundle(entry=[], link=[BundleLink(relation="self", url=searched_url)], type="searchset", total=0)
-
-
-# A lot of stuff in here is not very generic. Consider moving to relevant lambdas
-def make_search_bundle(
-    resource: Optional[dict],
-    version_id: Optional[int],
-    elements: Optional[set[str]],
-    identifier: Identifier,
-    base_url: str,
-) -> FhirBundle:
-    searched_url = f"{base_url}?identifier={identifier.system}|{identifier.value}" + (
-        f"&_elements={','.join(sorted(elements))}" if elements else ""
-    )
-
-    if not resource:
-        return make_empty_search_bundle(searched_url)
-
-    meta = {"versionId": version_id}
-
-    # Full Immunization payload to be returned if only the identifier parameter was provided and truncated when
-    # _elements is used
-    if elements is not None:
-        resource_for_bundle: dict[str, Any] = {"resourceType": "Immunization"}
-        if "id" in elements:
-            resource_for_bundle["id"] = resource["id"]
-        if "meta" in elements:
-            resource_for_bundle["meta"] = meta
-
-    else:
-        resource_for_bundle = copy.deepcopy(resource)
-        resource_for_bundle["meta"] = meta
-
-    entry = BundleEntry.construct(
-        fullUrl=f"{base_url}/{resource['id']}",
-        resource=(
-            Immunization.construct(**resource_for_bundle) if elements else Immunization.parse_obj(resource_for_bundle)
-        ),
-        search=BundleEntrySearch.construct(mode="match") if not elements else None,
-    )
-
-    return FhirBundle(
-        type="searchset",
-        link=[BundleLink(relation="self", url=searched_url)],
-        entry=[entry],
-        total=1,
-    )
-
-
 def check_keys_in_sources(event, not_required_keys):
     # Decode and parse the body, assuming it is JSON and base64-encoded
     def decode_and_parse_body(encoded_body):