VED-375 Search Parameter Error

backend/src/fhir_service.py

@@ -332,11 +332,6 @@ def search_immunizations(
         Bundles the resources with the relevant patient resource and returns the bundle along with a boolean to state
         whether the supplier requested vaccine types they were not authorised for.
         """
-        # TODO: is disease type a mandatory field? (I assumed it is)
-        #  i.e. Should we provide a search option for getting Patient's entire imms history?
-        if not nhs_number_mod11_check(nhs_number):
-            return create_diagnostics(), False
-
         permitted_vacc_types = self.authoriser.filter_permitted_vacc_types(
             supplier_system, ApiOperationCode.SEARCH, set(vaccine_types)
         )

backend/src/parameter_parser.py

@@ -8,6 +8,7 @@
 
 from clients import redis_client, logger
 from models.errors import ParameterException
+from models.utils.generic_utils import nhs_number_mod11_check
 from models.constants import Constants
 
 ParamValue = list[str]
@@ -35,6 +36,110 @@ class SearchParams:
     def __repr__(self):
         return str(self.__dict__)
 
+def process_patient_identifier(identifier_params: ParamContainer) -> str:
+    """Validate and parse patient identifier parameter.
+
+    :raises ParameterException:
+    """
+    patient_identifiers = identifier_params.get(patient_identifier_key, [])
+    patient_identifier = patient_identifiers[0] if len(patient_identifiers) == 1 else None
+
+    if patient_identifier is None:
+        raise ParameterException(f"Search parameter {patient_identifier_key} must have one value.")
+
+    patient_identifier_parts = patient_identifier.split("|")
+    identifier_system = patient_identifier_parts[0]
+    if len(patient_identifier_parts) != 2 or identifier_system != patient_identifier_system:
+        raise ParameterException("patient.identifier must be in the format of "
+                      f"\"{patient_identifier_system}|{{NHS number}}\" "
+                      f"e.g. \"{patient_identifier_system}|9000000009\"")
+
+    nhs_number = patient_identifier_parts[1]
+    if not nhs_number_mod11_check(nhs_number):
+        raise ParameterException("Search parameter patient.identifier must be a valid NHS number.")
+
+    return nhs_number
+
+
+def process_immunization_target(imms_params: ParamContainer) -> list[str]:
+    """Validate and parse immunization target parameter.
+
+    :raises ParameterException:
+    """
+    vaccine_types = [vaccine_type for vaccine_type in set(imms_params.get(immunization_target_key, [])) if
+                     vaccine_type is not None]
+    if len(vaccine_types) < 1:
+        raise ParameterException(f"Search parameter {immunization_target_key} must have one or more values.")
+
+    valid_vaccine_types = redis_client.hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY)
+    if any(x not in valid_vaccine_types for x in vaccine_types):
+        raise ParameterException(
+            f"immunization-target must be one or more of the following: {', '.join(valid_vaccine_types)}")
+
+    return vaccine_types
+
+
+def process_mandatory_params(params: ParamContainer) -> tuple[str, list[str]]:
+    """Validate mandatory params and return (patient_identifier, vaccine_types).
+    Raises ParameterException for any validation error.
+    """
+    # patient.identifier
+    patient_identifier = process_patient_identifier(params)
+
+    # immunization.target
+    vaccine_types = process_immunization_target(params)
+
+    return patient_identifier, vaccine_types
+
+
+def process_optional_params(params: ParamContainer) -> tuple[datetime.date, datetime.date, Optional[str], list[str]]:
+    """Parse optional params (date.from, date.to, _include).
+    Returns (date_from, date_to, include, errors).
+    """
+    errors: list[str] = []
+    date_from = None
+    date_to = None
+
+    date_froms = params.get(date_from_key, [])
+    if len(date_froms) > 1:
+        errors.append(f"Search parameter {date_from_key} may have one value at most.")
+
+    try:
+        date_from = datetime.datetime.strptime(date_froms[0], "%Y-%m-%d").date() if len(date_froms) == 1 else date_from_default
+    except ValueError:
+        errors.append(f"Search parameter {date_from_key} must be in format: YYYY-MM-DD")
+
+    date_tos = params.get(date_to_key, [])
+    if len(date_tos) > 1:
+        errors.append(f"Search parameter {date_to_key} may have one value at most.")
+
+    try:
+        date_to = datetime.datetime.strptime(date_tos[0], "%Y-%m-%d").date() if len(date_tos) == 1 else date_to_default
+    except ValueError:
+        errors.append(f"Search parameter {date_to_key} must be in format: YYYY-MM-DD")
+
+    includes = params.get(include_key, [])
+    if includes and includes[0].lower() != "immunization:patient":
+        errors.append(f"Search parameter {include_key} may only be 'Immunization:patient' if provided.")
+    include = includes[0] if len(includes) > 0 else None
+
+    return date_from, date_to, include, errors
+
+def process_search_params(params: ParamContainer) -> SearchParams:
+    """Validate and parse search parameters.
+    :raises ParameterException:
+    """
+    patient_identifier, vaccine_types = process_mandatory_params(params)
+    date_from, date_to, include, errors = process_optional_params(params)
+
+    if date_from and date_to and date_from > date_to:
+        errors.append(f"Search parameter {date_from_key} must be before {date_to_key}")
+
+    if errors:
+        raise ParameterException("; ".join(errors))
+
+    return SearchParams(patient_identifier, vaccine_types, date_from, date_to, include)
+
 
 def process_params(aws_event: APIGatewayProxyEventV1) -> ParamContainer:
     """Combines query string and content parameters. Duplicates not allowed. Splits on a comma."""
@@ -80,74 +185,6 @@ def parse_body_params(aws_event: APIGatewayProxyEventV1) -> ParamContainer:
     return parsed_params
 
 
-def process_search_params(params: ParamContainer) -> SearchParams:
-    """Validate and parse search parameters.
-
-    :raises ParameterException:
-    """
-
-    # patient.identifier
-    patient_identifiers = params.get(patient_identifier_key, [])
-    patient_identifier = patient_identifiers[0] if len(patient_identifiers) == 1 else None
-
-    if patient_identifier is None:
-        raise ParameterException(f"Search parameter {patient_identifier_key} must have one value.")
-
-    patient_identifier_parts = patient_identifier.split("|")
-    if len(patient_identifier_parts) != 2 or not patient_identifier_parts[
-                                                     0] == patient_identifier_system:
-        raise ParameterException("patient.identifier must be in the format of "
-                      f"\"{patient_identifier_system}|{{NHS number}}\" "
-                      f"e.g. \"{patient_identifier_system}|9000000009\"")
-
-    patient_identifier = patient_identifier.split("|")[1]
-
-    # immunization.target
-    params[immunization_target_key] = list(set(params.get(immunization_target_key, [])))
-    vaccine_types = [vaccine_type for vaccine_type in params[immunization_target_key] if
-                     vaccine_type is not None]
-    if len(vaccine_types) < 1:
-        raise ParameterException(f"Search parameter {immunization_target_key} must have one or more values.")
-
-    valid_vaccine_types = redis_client.hkeys(Constants.VACCINE_TYPE_TO_DISEASES_HASH_KEY)
-    if any(x not in valid_vaccine_types for x in vaccine_types):
-        raise ParameterException(
-            f"immunization-target must be one or more of the following: {', '.join(valid_vaccine_types)}")
-
-    # date.from
-    date_froms = params.get(date_from_key, [])
-
-    if len(date_froms) > 1:
-        raise ParameterException(f"Search parameter {date_from_key} may have one value at most.")
-
-    try:
-        date_from = datetime.datetime.strptime(date_froms[0], "%Y-%m-%d").date() \
-            if len(date_froms) == 1 else date_from_default
-    except ValueError:
-        raise ParameterException(f"Search parameter {date_from_key} must be in format: YYYY-MM-DD")
-
-    # date.to
-    date_tos = params.get(date_to_key, [])
-
-    if len(date_tos) > 1:
-        raise ParameterException(f"Search parameter {date_to_key} may have one value at most.")
-
-    try:
-        date_to = datetime.datetime.strptime(date_tos[0], "%Y-%m-%d").date() \
-            if len(date_tos) == 1 else date_to_default
-    except ValueError:
-        raise ParameterException(f"Search parameter {date_to_key} must be in format: YYYY-MM-DD")
-
-    if date_from and date_to and date_from > date_to:
-        raise ParameterException(f"Search parameter {date_from_key} must be before {date_to_key}")
-
-    # include
-    includes = params.get(include_key, [])
-    include = includes[0] if len(includes) > 0 else None
-
-    return SearchParams(patient_identifier, vaccine_types, date_from, date_to, include)
-
-
 def create_query_string(search_params: SearchParams) -> str:
     params = [
         (immunization_target_key, ",".join(map(quote, search_params.immunization_targets))),

backend/tests/test_parameter_parser.py

@@ -11,6 +11,8 @@
     process_params,
     process_search_params,
     create_query_string,
+    create_query_string,
+    include_key,
     SearchParams,
 )
 
@@ -106,7 +108,7 @@ def test_process_search_params_checks_patient_identifier_format(self):
                 self.immunization_target_key: ["RSV"],
             }
         )
-        self.assertIsNotNone(params)
+
 
     def test_process_search_params_whitelists_immunization_target(self):
         mock_redis_key = "RSV"
@@ -115,14 +117,18 @@ def test_process_search_params_whitelists_immunization_target(self):
             process_search_params(
                 {
                     self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"],
-                    self.immunization_target_key: ["not-a-code"],
+                    self.immunization_target_key: ["FLU", "COVID-19", "NOT-A-REAL-VALUE"],
                 }
             )
         self.assertEqual(
             str(e.exception), f"immunization-target must be one or more of the following: {mock_redis_key}",
             f"Unexpected exception message: {str(e.exception)}"
         )
 
+
+    def test_process_search_params_immunization_target(self):
+        mock_redis_key = "RSV"
+        self.mock_redis_client.hkeys.return_value = [mock_redis_key]
         params = process_search_params(
             {
                 self.patient_identifier_key: ["https://fhir.nhs.uk/Id/nhs-number|9000000009"],
@@ -132,6 +138,7 @@ def test_process_search_params_whitelists_immunization_target(self):
 
         self.assertIsNotNone(params)
 
+
     def test_search_params_date_from_must_be_before_date_to(self):
         self.mock_redis_client.hkeys.return_value = ["RSV"]
         params = process_search_params(
@@ -210,3 +217,83 @@ def test_create_query_string_with_multiple_immunization_targets_comma_separated(
         expected = "-immunization.target=b,c&patient.identifier=https%3A%2F%2Ffhir.nhs.uk%2FId%2Fnhs-number%7Ca"
 
         self.assertEqual(expected, query_string)
+
+    def test_process_search_params_dedupes_immunization_targets_and_respects_include(self):
+        """Ensure duplicate immunization targets are deduped and include is preserved."""
+        self.mock_redis_client.hkeys.return_value = ["RSV", "FLU"]
+
+        params = process_search_params(
+            {
+                self.patient_identifier_key: [
+                    "https://fhir.nhs.uk/Id/nhs-number|9000000009"
+                ],
+                self.immunization_target_key: ["RSV", "RSV", "FLU"],
+                "_include": ["immunization:patient"],
+            }
+        )
+
+        # immunization targets should be deduped and preserve valid entries
+        self.assertIsInstance(params.immunization_targets, list)
+        self.assertCountEqual(params.immunization_targets, ["RSV", "FLU"])
+
+        # include should be returned as provided
+        self.assertEqual(params.include, "immunization:patient")
+
+    def test_process_search_params_aggregates_date_errors(self):
+        """When multiple date-related errors occur they should be returned together."""
+        self.mock_redis_client.hkeys.return_value = ["RSV"]
+
+        with self.assertRaises(ParameterException) as e:
+            process_search_params(
+                {
+                    self.patient_identifier_key: [
+                        "https://fhir.nhs.uk/Id/nhs-number|9000000009"
+                    ],
+                    self.immunization_target_key: ["RSV"],
+                    self.date_from_key: ["2021-01-01", "2021-01-02"],  # too many values
+                    self.date_to_key: ["invalid-date"],  # invalid format
+                }
+            )
+
+        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):
+        """If the NHS number fails mod11 check a ParameterException is raised."""
+        # redis returns a valid vaccine type
+        self.mock_redis_client.hkeys.return_value = ["RSV"]
+
+        with self.assertRaises(ParameterException) as e:
+            process_search_params(
+                {
+                    self.patient_identifier_key: [
+                        "https://fhir.nhs.uk/Id/nhs-number|1234567890"  # invalid mod11
+                    ],
+                    self.immunization_target_key: ["RSV"],
+                }
+            )
+
+        self.assertEqual(str(e.exception), f"Search parameter {self.patient_identifier_key} must be a valid NHS number.")
+
+    def test_process_search_params_invalid_include_value_is_rejected(self):
+        """_include may only be 'Immunization:patient' if provided."""
+        self.mock_redis_client.hkeys.return_value = ["RSV"]
+
+        with self.assertRaises(ParameterException) as e:
+            process_search_params(
+                {
+                    self.patient_identifier_key: [
+                        "https://fhir.nhs.uk/Id/nhs-number|9000000009"
+                    ],
+                    self.immunization_target_key: ["RSV"],
+                    "_include": ["Patient:practitioner"],
+                }
+            )
+
+        self.assertEqual(
+            str(e.exception),
+            f"Search parameter {include_key} may only be 'Immunization:patient' if provided.",
+        )