Merge remote-tracking branch 'origin/main' into feature/eja-eli-385-remove-wildcard-resource-and-passrole-permissions

Author: eddalmond1

infrastructure/stacks/api-layer/patient_check.tf

@@ -1,4 +1,3 @@
-
 resource "aws_api_gateway_request_validator" "patient_check_validator" {
   rest_api_id                 = module.eligibility_signposting_api_gateway.rest_api_id
   name                        = "validate-path-params"
@@ -27,12 +26,12 @@ resource "aws_api_gateway_method" "get_patient_check" {
 }
 
 resource "aws_api_gateway_integration" "get_patient_check" {
-  rest_api_id             = module.eligibility_signposting_api_gateway.rest_api_id
-  resource_id             = aws_api_gateway_resource.patient.id
-  http_method             = aws_api_gateway_method.get_patient_check.http_method
+  rest_api_id = module.eligibility_signposting_api_gateway.rest_api_id
+  resource_id = aws_api_gateway_resource.patient.id
+  http_method = aws_api_gateway_method.get_patient_check.http_method
   integration_http_method = "POST" # Needed for lambda proxy integration
-  type                    = "AWS_PROXY"
-  uri                     = module.eligibility_signposting_lambda_function.aws_lambda_invoke_arn
+  type        = "AWS_PROXY"
+  uri         = module.eligibility_signposting_lambda_function.aws_lambda_invoke_arn
 
   depends_on = [
     aws_api_gateway_method.get_patient_check
@@ -47,3 +46,43 @@ resource "aws_lambda_permission" "get_patient_check" {
 
   source_arn = "${module.eligibility_signposting_api_gateway.execution_arn}/*/*"
 }
+
+resource "aws_api_gateway_gateway_response" "bad_request_parameters" {
+  rest_api_id   = module.eligibility_signposting_api_gateway.rest_api_id
+  response_type = "BAD_REQUEST_PARAMETERS"
+  status_code   = "400"
+
+  response_templates = {
+    "application/fhir+json" = jsonencode({
+      resourceType = "OperationOutcome"
+      id           = "$context.requestId"
+      meta = {
+        lastUpdated = "$context.requestTime"
+      }
+      issue = [
+        {
+          severity = "error"
+          code     = "invalid"
+          details = {
+            coding = [
+              {
+                system  = "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1",
+                code    = "BAD_REQUEST",
+                display = "Bad Request"
+              }
+            ]
+          }
+          diagnostics = "Missing required NHS Number from path parameters",
+          location = [
+            "parameters/id"
+          ]
+        }
+      ]
+    })
+  }
+
+  response_parameters = {
+    "gatewayresponse.header.Access-Control-Allow-Origin" = "'*'"
+    "gatewayresponse.header.Content-Type"                = "'application/fhir+json'"
+  }
+}

src/eligibility_signposting_api/common/api_error_response.py

@@ -25,26 +25,25 @@ class FHIRIssueCode(str, Enum):
 
 
 class FHIRSpineErrorCode(str, Enum):
-    INVALID_NHS_NUMBER = "INVALID_NHS_NUMBER"
+    ACCESS_DENIED = "ACCESS_DENIED"
     INVALID_PARAMETER = "INVALID_PARAMETER"
     INTERNAL_SERVER_ERROR = "INTERNAL_SERVER_ERROR"
     REFERENCE_NOT_FOUND = "REFERENCE_NOT_FOUND"
 
 
 class APIErrorResponse:
-    def __init__(  # noqa: PLR0913
+    def __init__(
         self,
         status_code: HTTPStatus,
         fhir_issue_code: FHIRIssueCode,
         fhir_issue_severity: FHIRIssueSeverity,
-        fhir_coding_system: str,
         fhir_error_code: str,
         fhir_display_message: str,
     ) -> None:
         self.status_code = status_code
         self.fhir_issue_code = fhir_issue_code
         self.fhir_issue_severity = fhir_issue_severity
-        self.fhir_coding_system = fhir_coding_system
+        self.fhir_coding_system = "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1"
         self.fhir_error_code = fhir_error_code
         self.fhir_display_message = fhir_display_message
 
@@ -94,7 +93,6 @@ def log_and_generate_response(
     status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
     fhir_issue_code=FHIRIssueCode.VALUE,
     fhir_issue_severity=FHIRIssueSeverity.ERROR,
-    fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1",
     fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER,
     fhir_display_message="The supplied value was not recognised by the API.",
 )
@@ -103,7 +101,6 @@ def log_and_generate_response(
     status_code=HTTPStatus.UNPROCESSABLE_ENTITY,
     fhir_issue_code=FHIRIssueCode.VALUE,
     fhir_issue_severity=FHIRIssueSeverity.ERROR,
-    fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1",
     fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER,
     fhir_display_message="The supplied category was not recognised by the API.",
 )
@@ -112,7 +109,6 @@ def log_and_generate_response(
     status_code=HTTPStatus.BAD_REQUEST,
     fhir_issue_code=FHIRIssueCode.VALUE,
     fhir_issue_severity=FHIRIssueSeverity.ERROR,
-    fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1",
     fhir_error_code=FHIRSpineErrorCode.INVALID_PARAMETER,
     fhir_display_message="The given conditions were not in the expected format.",
 )
@@ -121,7 +117,6 @@ def log_and_generate_response(
     status_code=HTTPStatus.NOT_FOUND,
     fhir_issue_code=FHIRIssueCode.PROCESSING,
     fhir_issue_severity=FHIRIssueSeverity.ERROR,
-    fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1",
     fhir_error_code=FHIRSpineErrorCode.REFERENCE_NOT_FOUND,
     fhir_display_message="The given NHS number was not found in our datasets. "
     "This could be because the number is incorrect or "
@@ -132,7 +127,6 @@ def log_and_generate_response(
     status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
     fhir_issue_code=FHIRIssueCode.PROCESSING,
     fhir_issue_severity=FHIRIssueSeverity.ERROR,
-    fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1",
     fhir_error_code=FHIRSpineErrorCode.INTERNAL_SERVER_ERROR,
     fhir_display_message="An unexpected internal server error occurred.",
 )
@@ -141,7 +135,6 @@ def log_and_generate_response(
     status_code=HTTPStatus.FORBIDDEN,
     fhir_issue_code=FHIRIssueCode.FORBIDDEN,
     fhir_issue_severity=FHIRIssueSeverity.ERROR,
-    fhir_coding_system="https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1",
-    fhir_error_code=FHIRSpineErrorCode.INVALID_NHS_NUMBER,
-    fhir_display_message="The provided NHS number does not match the record.",
+    fhir_error_code=FHIRSpineErrorCode.ACCESS_DENIED,
+    fhir_display_message="Access has been denied to process this request.",
 )

src/eligibility_signposting_api/common/request_validator.py

@@ -16,7 +16,7 @@
 
 logger = logging.getLogger(__name__)
 
-condition_pattern = re.compile(r"^\s*[a-zA-Z0-9]+\s*$", re.IGNORECASE)
+condition_pattern = re.compile(r"^\s*[a-z0-9]+\s*$", re.IGNORECASE)
 category_pattern = re.compile(r"^\s*(VACCINATIONS|SCREENING|ALL)\s*$", re.IGNORECASE)
 include_actions_pattern = re.compile(r"^\s*([YN])\s*$", re.IGNORECASE)
 
@@ -60,10 +60,8 @@ def wrapper(event: LambdaEvent, context: LambdaContext) -> dict[str, Any] | None
             header_nhs_no = event.get("headers", {}).get(NHS_NUMBER_HEADER)
 
             if not validate_nhs_number(path_nhs_no, header_nhs_no):
-                message = f"NHS Number {path_nhs_no or ''} does not match the header NHS Number {header_nhs_no or ''}"
-                return NHS_NUMBER_MISMATCH_ERROR.log_and_generate_response(
-                    log_message=message, diagnostics=message, location_param="id"
-                )
+                message = "You are not authorised to request information for the supplied NHS Number"
+                return NHS_NUMBER_MISMATCH_ERROR.log_and_generate_response(log_message=message, diagnostics=message)
 
             query_params = event.get("queryStringParameters")
             if query_params:

src/eligibility_signposting_api/model/campaign_config.py

@@ -228,21 +228,6 @@ def check_no_overlapping_iterations(self) -> typing.Self:
             raise ValueError(message)
         return self
 
-    @model_validator(mode="after")
-    def check_has_iteration_from_start(self) -> typing.Self:
-        iterations_by_date = sorted(self.iterations, key=attrgetter("iteration_date"))
-        if first_iteration := next(iter(iterations_by_date), None):
-            if first_iteration.iteration_date > self.start_date:
-                message = (
-                    f"campaign {self.id} starts on {self.start_date}, "
-                    f"1st iteration starts later - {first_iteration.iteration_date}"
-                )
-                raise ValueError(message)
-            return self
-        # Should never happen, since we are constraining self.iterations with a min_length of 1
-        message = f"campaign {self.id} has no iterations."
-        raise ValueError(message)
-
     @cached_property
     def campaign_live(self) -> bool:
         today = datetime.now(tz=UTC).date()

src/eligibility_signposting_api/model/eligibility_status.py

@@ -113,6 +113,7 @@ class Condition:
     condition_name: ConditionName
     status: Status
     cohort_results: list[CohortGroupResult]
+    suitability_rules: list[Reason]
     status_text: StatusText
     actions: list[SuggestedAction] | None = None
 

src/eligibility_signposting_api/repos/person_repo.py

@@ -40,7 +40,9 @@ def get_eligibility_data(self, nhs_number: NHSNumber) -> Person:
         response = self.table.query(KeyConditionExpression=Key("NHS_NUMBER").eq(nhs_number))
         logger.debug("response %r for %r", response, nhs_number, extra={"response": response, "nhs_number": nhs_number})
 
-        if not (items := response.get("Items")):
+        if not (items := response.get("Items")) or not next(
+            (item for item in items if item.get("ATTRIBUTE_TYPE") == "PERSON"), None
+        ):
             message = f"Person not found with nhs_number {nhs_number}"
             raise NotFoundError(message)
 

src/eligibility_signposting_api/services/calculators/eligibility_calculator.py

@@ -3,6 +3,7 @@
 import logging
 from collections import defaultdict
 from dataclasses import dataclass, field
+from itertools import chain
 from typing import TYPE_CHECKING
 
 from wireup import service
@@ -16,6 +17,7 @@
     ConditionName,
     EligibilityStatus,
     IterationResult,
+    Reason,
     Status,
 )
 from eligibility_signposting_api.services.processors.action_rule_handler import ActionRuleHandler
@@ -100,8 +102,10 @@ def get_eligibility_status(self, include_actions: str, conditions: list[str], ca
             condition_results[condition_name] = best_iteration_result.iteration_result
             condition_results[condition_name].actions = matched_action_detail.actions
 
-            condition_result = self.build_condition_results(condition_results[condition_name], condition_name)
-            final_result.append(condition_result)
+            condition: Condition = self.build_condition(
+                iteration_result=condition_results[condition_name], condition_name=condition_name
+            )
+            final_result.append(condition)
 
             AuditContext.append_audit_condition(
                 condition_name,
@@ -150,39 +154,61 @@ def get_iteration_results(self, campaign_group: list[CampaignConfig]) -> dict[It
         return iteration_results
 
     @staticmethod
-    def build_condition_results(iteration_result: IterationResult, condition_name: ConditionName) -> Condition:
+    def build_condition(iteration_result: IterationResult, condition_name: ConditionName) -> Condition:
         grouped_cohort_results = defaultdict(list)
 
         for cohort_result in iteration_result.cohort_results:
             if iteration_result.status == cohort_result.status:
                 grouped_cohort_results[cohort_result.cohort_code].append(cohort_result)
 
-        deduplicated_cohort_results = []
-
-        for group_cohort_code, group in grouped_cohort_results.items():
-            if group:
-                unique_rule_codes = set()
-                deduplicated_reasons = []
-                for cohort in group:
-                    for reason in cohort.reasons:
-                        if reason.rule_name not in unique_rule_codes and reason.rule_description:
-                            unique_rule_codes.add(reason.rule_name)
-                            deduplicated_reasons.append(reason)
-
-                non_empty_description = next((c.description for c in group if c.description), group[0].description)
-                cohort_group_result = CohortGroupResult(
-                    cohort_code=group_cohort_code,
-                    status=group[0].status,
-                    reasons=deduplicated_reasons,
-                    description=non_empty_description,
-                    audit_rules=[],
-                )
-                deduplicated_cohort_results.append(cohort_group_result)
+        deduplicated_cohort_results: list[CohortGroupResult] = EligibilityCalculator.deduplicate_cohort_results(
+            grouped_cohort_results
+        )
+
+        overall_deduplicated_reasons_for_condition = EligibilityCalculator.deduplicate_reasons(
+            deduplicated_cohort_results
+        )
 
         return Condition(
             condition_name=condition_name,
             status=iteration_result.status,
             cohort_results=list(deduplicated_cohort_results),
+            suitability_rules=list(overall_deduplicated_reasons_for_condition),
             actions=iteration_result.actions,
             status_text=iteration_result.status.get_status_text(condition_name),
         )
+
+    @staticmethod
+    def deduplicate_cohort_results(
+        grouped_cohort_results: dict[str, list[CohortGroupResult]],
+    ) -> list[CohortGroupResult]:
+        results = []
+
+        for cohort_code, group_results in grouped_cohort_results.items():
+            if not group_results:
+                continue
+
+            deduped_reasons: list[Reason] = EligibilityCalculator.deduplicate_reasons(group_results)
+
+            description = next((c.description for c in group_results if c.description), group_results[0].description)
+
+            results.append(
+                CohortGroupResult(
+                    cohort_code=cohort_code,
+                    status=group_results[0].status,
+                    reasons=list(deduped_reasons),
+                    description=description,
+                    audit_rules=[],
+                )
+            )
+
+        return results
+
+    @staticmethod
+    def deduplicate_reasons(group_results: list[CohortGroupResult]) -> list[Reason]:
+        all_reasons = chain.from_iterable(group_result.reasons for group_result in group_results)
+        deduped = {}
+        for reason in all_reasons:
+            key = (reason.rule_type, reason.rule_priority)
+            deduped.setdefault(key, reason)
+        return list(deduped.values())

src/eligibility_signposting_api/services/processors/rule_processor.py

@@ -50,10 +50,13 @@ def is_eligible(
     ) -> bool:
         is_eligible = True
         priority_getter = attrgetter("priority")
-        sorted_rules_by_priority = sorted(self.get_exclusion_rules(cohort, filter_rules), key=priority_getter)
+        sorted_rules_by_priority = sorted(filter_rules, key=priority_getter)
 
         for _, rule_group in groupby(sorted_rules_by_priority, key=priority_getter):
-            status, group_exclusion_reasons, _ = self.evaluate_rules_priority_group(person, rule_group)
+            group_rules = list(rule_group)
+            if self._should_skip_rule_group(cohort, group_rules):
+                continue
+            status, group_exclusion_reasons, _ = self.evaluate_rules_priority_group(person, iter(group_rules))
             if status.is_exclusion:
                 if cohort.cohort_label is not None:
                     cohort_results[cohort.cohort_label] = CohortGroupResult(
@@ -65,6 +68,7 @@ def is_eligible(
                     )
                 is_eligible = False
                 break
+
         return is_eligible
 
     def is_actionable(
@@ -78,10 +82,14 @@ def is_actionable(
         priority_getter = attrgetter("priority")
         suppression_reasons = []
 
-        sorted_rules_by_priority = sorted(self.get_exclusion_rules(cohort, suppression_rules), key=priority_getter)
+        sorted_rules_by_priority = sorted(suppression_rules, key=priority_getter)
 
         for _, rule_group in groupby(sorted_rules_by_priority, key=priority_getter):
-            status, group_exclusion_reasons, rule_stop = self.evaluate_rules_priority_group(person, rule_group)
+            group_rules = list(rule_group)
+            if self._should_skip_rule_group(cohort, group_rules):
+                continue
+
+            status, group_exclusion_reasons, rule_stop = self.evaluate_rules_priority_group(person, iter(group_rules))
             if status.is_exclusion:
                 is_actionable = False
                 suppression_reasons.extend(group_exclusion_reasons)
@@ -103,6 +111,12 @@ def is_actionable(
                     suppression_reasons,
                 )
 
+    @staticmethod
+    def _should_skip_rule_group(cohort: IterationCohort, group_rules: list[IterationRule]) -> bool:
+        cohort_specific_rules = [rule for rule in group_rules if rule.cohort_label is not None]
+        matching_specific_rules = [rule for rule in cohort_specific_rules if rule.cohort_label == cohort.cohort_label]
+        return bool(cohort_specific_rules and not matching_specific_rules)
+
     def evaluate_rules_priority_group(
         self, person: Person, rules_group: Iterator[IterationRule]
     ) -> tuple[eligibility_status.Status, list[eligibility_status.Reason], bool]: