diff --git a/src/eligibility_signposting_api/audit_context.py b/src/eligibility_signposting_api/audit_context.py new file mode 100644 index 000000000..756cb197e --- /dev/null +++ b/src/eligibility_signposting_api/audit_context.py @@ -0,0 +1,142 @@ +import logging +from dataclasses import asdict +from datetime import UTC, datetime +from operator import attrgetter + +from flask import Request, g + +from eligibility_signposting_api.audit_models import ( + AuditAction, + AuditCondition, + AuditEligibilityCohortGroups, + AuditEligibilityCohorts, + AuditEvent, + AuditFilterRule, + AuditRedirectRule, + AuditSuitabilityRule, + RequestAuditData, + RequestAuditHeader, + RequestAuditQueryParams, +) +from eligibility_signposting_api.model.eligibility import ( + CohortGroupResult, + ConditionName, + IterationResult, + Status, + SuggestedActions, +) +from eligibility_signposting_api.model.rules import CampaignID, CampaignVersion, Iteration, RuleName, RulePriority +from eligibility_signposting_api.services.audit_service import AuditService + +logger = logging.getLogger(__name__) + + +class AuditContext: + @staticmethod + def add_request_details(request: Request) -> None: + g.audit_log = AuditEvent() + resource_id = None + if "nhs_number" in request.view_args: + resource_id = request.view_args["nhs_number"] + g.audit_log.request = RequestAuditData( + nhs_number=resource_id, + request_timestamp=datetime.now(tz=UTC), + headers=( + RequestAuditHeader( + x_request_id=request.headers.get("X-Request-ID"), + x_correlation_id=request.headers.get("X-Correlation-ID"), + nhsd_end_user_organisation_ods=request.headers.get("NHSD-End-User-Organisation-ODS"), + nhsd_application_id=request.headers.get("nhsd-application-id"), + ) + ), + query_params=( + RequestAuditQueryParams( + category=request.args.get("category"), + conditions=request.args.get("conditions"), + include_actions=request.args.get("includeActions"), + ) + ), + ) + + @staticmethod + def append_audit_condition( + suggested_actions: SuggestedActions | None, + condition_name: ConditionName, + best_results: tuple[Iteration, IterationResult, dict[str, CohortGroupResult]], + campaign_details: tuple[CampaignID | None, CampaignVersion | None], + redirect_rule_details: tuple[RulePriority | None, RuleName | None], + ) -> None: + audit_eligibility_cohorts, audit_eligibility_cohort_groups, audit_actions = [], [], [] + audit_filter_rule, audit_suitability_rule, audit_redirect_rule = None, None, None + best_active_iteration = best_results[0] + best_candidate = best_results[1] + best_cohort_results = best_results[2] + + for value in sorted(best_cohort_results.values(), key=attrgetter("cohort_code")): + audit_eligibility_cohorts.append( + AuditEligibilityCohorts(cohort_code=value.cohort_code, cohort_status=value.status.name) + ) + + audit_eligibility_cohort_groups.append( + AuditEligibilityCohortGroups( + cohort_code=value.cohort_code, cohort_status=value.status.name, cohort_text=value.description + ) + ) + + if value.audit_rules: + if best_candidate.status.name == Status.not_eligible.name: + audit_filter_rule = AuditFilterRule( + rule_priority=int(value.audit_rules[0].rule_priority), rule_name=value.audit_rules[0].rule_name + ) + if best_candidate.status.name == Status.not_actionable.name: + audit_suitability_rule = AuditSuitabilityRule( + rule_priority=int(value.audit_rules[0].rule_priority), + rule_name=value.audit_rules[0].rule_name, + rule_message=value.audit_rules[0].rule_description, + ) + + if best_candidate.status.name == Status.actionable.name: + audit_redirect_rule = AuditRedirectRule(redirect_rule_details[0], redirect_rule_details[1]) + + if suggested_actions is None or suggested_actions == []: + audit_actions = suggested_actions + + elif len(suggested_actions.actions) > 0: + for action in suggested_actions.actions: + audit_actions.append( + AuditAction( + action_code=action.action_code, + action_type=action.action_type, + action_description=action.action_description, + action_url=action.url_link, + action_url_label=action.url_label, + ) + ) + + audit_condition = AuditCondition( + campaign_id=campaign_details[0], + campaign_version=campaign_details[1], + iteration_id=best_active_iteration.id, + iteration_version=best_active_iteration.version, + condition_name=condition_name, + status=best_candidate.status.name, + status_text=best_candidate.status.name, + eligibility_cohorts=audit_eligibility_cohorts, + eligibility_cohort_groups=audit_eligibility_cohort_groups, + filter_rules=audit_filter_rule, + suitability_rules=audit_suitability_rule, + action_rule=audit_redirect_rule, + actions=audit_actions, + ) + + g.audit_log.response.condition.append(audit_condition) + + @staticmethod + def add_response_details(response) -> None: + g.audit_log.response.response_id = response.response_id + g.audit_log.response.last_updated = response.meta.last_updated + + @staticmethod + def write_to_firehose(service: AuditService, response) -> None: + AuditContext.add_response_details(response) + service.audit(asdict(g.audit_log)) diff --git a/src/eligibility_signposting_api/audit_models.py b/src/eligibility_signposting_api/audit_models.py new file mode 100644 index 000000000..0a4c60e56 --- /dev/null +++ b/src/eligibility_signposting_api/audit_models.py @@ -0,0 +1,98 @@ +from dataclasses import dataclass, field +from datetime import UTC, datetime +from uuid import UUID + + +@dataclass +class RequestAuditHeader: + x_request_id: str = None + x_correlation_id: str = None + nhsd_end_user_organisation_ods: str = None + nhsd_application_id: str = None + + +@dataclass +class RequestAuditQueryParams: + category: str | None = None + conditions: str | None = None + include_actions: str | None = None + + +@dataclass +class RequestAuditData: + request_timestamp: datetime = field(default_factory=lambda: datetime.now(UTC)) + headers: RequestAuditHeader = field(default_factory=RequestAuditHeader) + query_params: RequestAuditQueryParams = field(default_factory=RequestAuditQueryParams) + nhs_number: str | None = None + + +@dataclass +class AuditEligibilityCohorts: + cohort_code: str | None = None + cohort_status: str | None = None + + +@dataclass +class AuditEligibilityCohortGroups: + cohort_code: str | None = None + cohort_text: str | None = None + cohort_status: str | None = None + + +@dataclass +class AuditFilterRule: + rule_priority: int | None = None + rule_name: str | None = None + + +@dataclass +class AuditSuitabilityRule: + rule_priority: int | None = None + rule_name: str | None = None + rule_message: str | None = None + + +@dataclass +class AuditRedirectRule: + rule_priority: int | None = None + rule_name: str | None = None + + +@dataclass +class AuditAction: + internal_name: str | None = None + action_type: str | None = None + action_code: str | None = None + action_description: str | None = None + action_url: str | None = None + action_url_label: str | None = None + + +@dataclass +class AuditCondition: + campaign_id: str = None + campaign_version: str = None + iteration_id: str = None + iteration_version: str = None + condition_name: str = None + status: str = None + status_text: str = None + eligibility_cohorts: list[AuditEligibilityCohorts] | None = None + eligibility_cohort_groups: list[AuditEligibilityCohortGroups] | None = None + filter_rules: AuditFilterRule | None = None + suitability_rules: AuditSuitabilityRule | None = None + action_rule: AuditRedirectRule | None = None + actions: list[AuditAction] = field(default_factory=list) + + +@dataclass +class ResponseAuditData: + response_id: UUID | None = None + last_updated: str | None = None + condition: list[AuditCondition] = field(default_factory=list) + + +@dataclass +class AuditEvent: + request: RequestAuditData = field(default_factory=RequestAuditData) + response: ResponseAuditData = field(default_factory=ResponseAuditData) diff --git a/src/eligibility_signposting_api/model/eligibility.py b/src/eligibility_signposting_api/model/eligibility.py index 7e3582c3b..0295e7727 100644 --- a/src/eligibility_signposting_api/model/eligibility.py +++ b/src/eligibility_signposting_api/model/eligibility.py @@ -13,6 +13,7 @@ RuleName = NewType("RuleName", str) RuleDescription = NewType("RuleDescription", str) +RulePriority = NewType("RulePriority", str) ActionType = NewType("ActionType", str) ActionCode = NewType("ActionCode", str) @@ -66,6 +67,7 @@ def best(*statuses: Status) -> Status: class Reason: rule_type: RuleType rule_name: RuleName + rule_priority: RulePriority rule_description: RuleDescription | None matcher_matched: bool @@ -98,6 +100,7 @@ class CohortGroupResult: status: Status reasons: list[Reason] description: str | None + audit_rules: list[Reason] @dataclass diff --git a/src/eligibility_signposting_api/services/audit_service.py b/src/eligibility_signposting_api/services/audit_service.py index b1d8b411e..7494661cf 100644 --- a/src/eligibility_signposting_api/services/audit_service.py +++ b/src/eligibility_signposting_api/services/audit_service.py @@ -31,8 +31,9 @@ def audit(self, audit_record: dict) -> None: Returns: str: The Firehose record ID. """ + data = json.dumps(audit_record, default=str) response = self.firehose.put_record( DeliveryStreamName=self.audit_delivery_stream, - Record={"Data": (json.dumps(audit_record) + "\n").encode("utf-8")}, + Record={"Data": (data + "\n").encode("utf-8")}, ) logger.info("Successfully sent to the Firehose", extra={"firehose_record_id": response["RecordId"]}) diff --git a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py index 304261a04..b771193e6 100644 --- a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py +++ b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py @@ -7,8 +7,18 @@ from itertools import groupby from typing import TYPE_CHECKING, Any +from eligibility_signposting_api.audit_context import AuditContext + if TYPE_CHECKING: - from eligibility_signposting_api.model.rules import ActionsMapper, Iteration, IterationCohort + from eligibility_signposting_api.model.rules import ( + ActionsMapper, + CampaignID, + CampaignVersion, + Iteration, + IterationCohort, + RuleName, + RulePriority, + ) from wireup import service @@ -87,6 +97,7 @@ def get_the_best_cohort_memberships( status=cc.status, reasons=cc.reasons, description=(cc.description or "").strip() if cc.description else "", + audit_rules=cc.audit_rules, # TODO: remove this or fix it! ) for cc in best_cohorts ] @@ -130,11 +141,15 @@ def evaluate_eligibility(self, *, include_actions_flag: bool = True) -> eligibil """Iterates over campaign groups, evaluates eligibility, and returns a consolidated status.""" condition_results: dict[ConditionName, IterationResult] = {} actions: SuggestedActions | None = SuggestedActions([]) + redirect_rule_priority, redirect_rule_name = None, None for condition_name, campaign_group in self.campaigns_grouped_by_condition_name: - iteration_results: dict[str, tuple[Iteration, IterationResult]] = {} + iteration_results: dict[ + str, tuple[Iteration, IterationResult, CampaignID, CampaignVersion, dict[str, CohortGroupResult]] + ] = {} - for active_iteration in [cc.current_iteration for cc in campaign_group]: + for cc in campaign_group: + active_iteration = cc.current_iteration cohort_results: dict[str, CohortGroupResult] = self.get_cohort_results(active_iteration) # Determine Result between cohorts - get the best @@ -142,35 +157,74 @@ def evaluate_eligibility(self, *, include_actions_flag: bool = True) -> eligibil iteration_results[active_iteration.name] = ( active_iteration, IterationResult(status, best_cohorts, actions), + cc.id, + cc.version, + cohort_results, ) # Determine results between iterations - get the best if iteration_results: - best_iteration_name, (best_active_iteration, best_candidate) = max( - iteration_results.items(), key=lambda item: item[1][1].status.value - ) + ( + best_iteration_name, + ( + best_active_iteration, + best_candidate, + best_campaign_id, + best_campaign_version, + best_cohort_results, + ), + ) = max(iteration_results.items(), key=lambda item: item[1][1].status.value) else: best_candidate = IterationResult(eligibility.Status.not_eligible, [], actions) + # TODO: Refactor? + best_campaign_id = None + best_campaign_version = None best_active_iteration = None + best_cohort_results = None + condition_results[condition_name] = best_candidate if best_candidate.status == Status.actionable and best_active_iteration is not None: - actions = self.handle_redirect_rules(best_active_iteration) if include_actions_flag else None + if include_actions_flag: + actions, matched_r_rule_priority, matched_r_rule_name = self.handle_redirect_rules( + best_active_iteration + ) + redirect_rule_name = matched_r_rule_name + redirect_rule_priority = matched_r_rule_priority + else: + actions = None + if best_candidate.status in (Status.not_eligible, Status.not_actionable) and not include_actions_flag: actions = None # add actions to condition results condition_results[condition_name].actions = actions + # reset actions for the next condition + actions: SuggestedActions | None = SuggestedActions([]) + + # add audit data + # TODO: Do we need to use deduplicated cohort results from build_condition_results instead of here? + AuditContext.append_audit_condition( + condition_results[condition_name].actions, + condition_name, + (best_active_iteration, best_candidate, best_cohort_results), + (best_campaign_id, best_campaign_version), + (redirect_rule_priority, redirect_rule_name), + ) + # Consolidate all the results and return final_result = self.build_condition_results(condition_results) return eligibility.EligibilityStatus(conditions=final_result) - def handle_redirect_rules(self, best_active_iteration: Iteration) -> SuggestedActions | None: + def handle_redirect_rules( + self, best_active_iteration: Iteration + ) -> tuple[SuggestedActions | None, RulePriority | None, RuleName | None]: redirect_rules, action_mapper, default_comms = self.get_redirect_rules(best_active_iteration) priority_getter = attrgetter("priority") sorted_rules_by_priority = sorted(redirect_rules, key=priority_getter) actions: SuggestedActions | None = self.get_actions_from_comms(action_mapper, default_comms) + matched_redirect_rule_priority, matched_redirect_rule_name = None, None for _, rule_group in groupby(sorted_rules_by_priority, key=priority_getter): rule_group_list = list(rule_group) matcher_matched_list = [ @@ -183,9 +237,11 @@ def handle_redirect_rules(self, best_active_iteration: Iteration) -> SuggestedAc rule_actions = self.get_actions_from_comms(action_mapper, comms_routing) if rule_actions and len(rule_actions.actions) > 0: actions = rule_actions + matched_redirect_rule_priority = rule_group_list[0].priority + matched_redirect_rule_name = rule_group_list[0].name break - return actions + return actions, matched_redirect_rule_priority, matched_redirect_rule_name def get_cohort_results(self, active_iteration: rules.Iteration) -> dict[str, CohortGroupResult]: cohort_results: dict[str, CohortGroupResult] = {} @@ -201,10 +257,11 @@ def get_cohort_results(self, active_iteration: rules.Iteration) -> dict[str, Coh # Not base eligible elif cohort.cohort_label is not None: cohort_results[cohort.cohort_label] = CohortGroupResult( - (cohort.cohort_group), + cohort.cohort_group, Status.not_eligible, [], cohort.negative_description, + [], # TODO: remove this or fix it! ) return cohort_results @@ -228,6 +285,7 @@ def build_condition_results(condition_results: dict[ConditionName, IterationResu reasons=[reason for cohort in group for reason in cohort.reasons], # get the first nonempty description description=next((c.description for c in group if c.description), group[0].description), + audit_rules=[], # TODO: remove this or fix it! ) for group_cohort_code, group in grouped_cohort_results.items() if group @@ -265,6 +323,7 @@ def is_eligible_by_filter_rules( Status.not_eligible, [], cohort.negative_description, + group_exclusion_reasons, ) is_eligible = False break @@ -296,10 +355,7 @@ def evaluate_suppression_rules( key = cohort.cohort_label if is_actionable: cohort_results[key] = CohortGroupResult( - cohort.cohort_group, - Status.actionable, - [], - cohort.positive_description, + cohort.cohort_group, Status.actionable, [], cohort.positive_description, suppression_reasons ) else: cohort_results[key] = CohortGroupResult( @@ -307,6 +363,7 @@ def evaluate_suppression_rules( Status.not_actionable, suppression_reasons, cohort.positive_description, + suppression_reasons, ) def evaluate_rules_priority_group( diff --git a/src/eligibility_signposting_api/services/calculators/rule_calculator.py b/src/eligibility_signposting_api/services/calculators/rule_calculator.py index 5834c678e..c24fb591b 100644 --- a/src/eligibility_signposting_api/services/calculators/rule_calculator.py +++ b/src/eligibility_signposting_api/services/calculators/rule_calculator.py @@ -24,6 +24,7 @@ def evaluate_exclusion(self) -> tuple[eligibility.Status, eligibility.Reason]: reason = eligibility.Reason( rule_name=eligibility.RuleName(self.rule.name), rule_type=eligibility.RuleType(self.rule.type), + rule_priority=eligibility.RulePriority(self.rule.priority), rule_description=eligibility.RuleDescription(self.rule.description), matcher_matched=matcher_matched, ) diff --git a/src/eligibility_signposting_api/services/eligibility_services.py b/src/eligibility_signposting_api/services/eligibility_services.py index 032b68816..c8d9c5b50 100644 --- a/src/eligibility_signposting_api/services/eligibility_services.py +++ b/src/eligibility_signposting_api/services/eligibility_services.py @@ -4,7 +4,6 @@ from eligibility_signposting_api.model import eligibility from eligibility_signposting_api.repos import CampaignRepo, NotFoundError, PersonRepo -from eligibility_signposting_api.services.audit_service import AuditService from eligibility_signposting_api.services.calculators import eligibility_calculator as calculator logger = logging.getLogger(__name__) @@ -24,13 +23,11 @@ def __init__( self, person_repo: PersonRepo, campaign_repo: CampaignRepo, - audit_service: AuditService, calculator_factory: calculator.EligibilityCalculatorFactory, ) -> None: super().__init__() self.person_repo = person_repo self.campaign_repo = campaign_repo - self.audit_service = audit_service self.calculator_factory = calculator_factory def get_eligibility_status( @@ -54,7 +51,6 @@ def get_eligibility_status( raise UnknownPersonError from e else: calc: calculator.EligibilityCalculator = self.calculator_factory.get(person_data, campaign_configs) - self.audit_service.audit({"test_audit": "check if audit works"}) return calc.evaluate_eligibility(include_actions_flag=include_actions_flag) raise UnknownPersonError # pragma: no cover diff --git a/src/eligibility_signposting_api/views/eligibility.py b/src/eligibility_signposting_api/views/eligibility.py index aafe65134..253850af6 100644 --- a/src/eligibility_signposting_api/views/eligibility.py +++ b/src/eligibility_signposting_api/views/eligibility.py @@ -9,8 +9,10 @@ from flask.typing import ResponseReturnValue from wireup import Injected +from eligibility_signposting_api.audit_context import AuditContext from eligibility_signposting_api.model.eligibility import Condition, EligibilityStatus, NHSNumber, Status from eligibility_signposting_api.services import EligibilityService, UnknownPersonError +from eligibility_signposting_api.services.audit_service import AuditService from eligibility_signposting_api.services.eligibility_services import InvalidQueryParamError from eligibility_signposting_api.views.response_model import eligibility from eligibility_signposting_api.views.response_model.eligibility import ProcessedSuggestion @@ -26,9 +28,16 @@ eligibility_blueprint = Blueprint("eligibility", __name__) +@eligibility_blueprint.before_request +def before_request() -> None: + AuditContext.add_request_details(request) + + @eligibility_blueprint.get("/", defaults={"nhs_number": ""}) @eligibility_blueprint.get("/") -def check_eligibility(nhs_number: NHSNumber, eligibility_service: Injected[EligibilityService]) -> ResponseReturnValue: +def check_eligibility( + nhs_number: NHSNumber, eligibility_service: Injected[EligibilityService], audit_service: Injected[AuditService] +) -> ResponseReturnValue: logger.info("checking nhs_number %r in %r", nhs_number, eligibility_service, extra={"nhs_number": nhs_number}) try: eligibility_status = eligibility_service.get_eligibility_status( @@ -40,6 +49,7 @@ def check_eligibility(nhs_number: NHSNumber, eligibility_service: Injected[Eligi return handle_unknown_person_error(nhs_number) else: eligibility_response = build_eligibility_response(eligibility_status) + AuditContext.write_to_firehose(audit_service, eligibility_response) return make_response( eligibility_response.model_dump(by_alias=True, mode="json", exclude_none=True), HTTPStatus.OK ) diff --git a/tests/conftest.py b/tests/conftest.py index 5d90004aa..4c43c14c6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,10 +4,11 @@ import pytest from faker import Faker from faker.providers import BaseProvider -from flask import Flask +from flask import Flask, g from flask.testing import FlaskClient from eligibility_signposting_api.app import create_app +from eligibility_signposting_api.audit_models import AuditEvent @pytest.fixture(scope="session") @@ -20,6 +21,14 @@ def client(app) -> FlaskClient: return app.test_client() +@pytest.fixture(autouse=True) +def audit_log_context(app): + with app.app_context(): + g.audit_log = AuditEvent() + yield + g.pop("audit_log", None) + + @pytest.fixture(scope="session") def faker() -> Faker: faker = Faker("en_UK") diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index d6ebc0f4f..2054579b7 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -350,6 +350,28 @@ def persisted_77yo_person(person_table: Any, faker: Faker) -> Generator[eligibil person_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) +@pytest.fixture +def persisted_person_all_cohorts(person_table: Any, faker: Faker) -> Generator[eligibility.NHSNumber]: + nhs_number = eligibility.NHSNumber(faker.nhs_number()) + date_of_birth = eligibility.DateOfBirth(faker.date_of_birth(minimum_age=74, maximum_age=74)) + + for row in ( + rows := person_rows_builder( + nhs_number, + date_of_birth=date_of_birth, + postcode="hp1", + cohorts=["cohort1", "cohort2", "cohort3"], + icb="QE1", + ) + ): + person_table.put_item(Item=row) + + yield nhs_number + + for row in rows: + person_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) + + @pytest.fixture def persisted_person_no_cohorts(person_table: Any, faker: Faker) -> Generator[eligibility.NHSNumber]: nhs_number = eligibility.NHSNumber(faker.nhs_number()) @@ -442,6 +464,50 @@ def campaign_config(s3_client: BaseClient, rules_bucket: BucketName) -> Generato s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") +@pytest.fixture(scope="class") +def multiple_campaign_configs(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[list[rules.CampaignConfig]]: + """Create and upload multiple campaign configs to S3, then clean up after tests.""" + campaigns, campaign_data_keys = [], [] + + targets = ["RSV", "COVID", "FLU"] + target_rules_map = { + targets[0]: [rule.PersonAgeSuppressionRuleFactory.build(type=rules.RuleType.filter)], + targets[1]: [rule.PersonAgeSuppressionRuleFactory.build()], + targets[2]: [rule.ICBRedirectRuleFactory.build()], + } + + for i in range(3): + campaign = rule.CampaignConfigFactory.build( + name=f"campaign_{i}", + target=targets[i], + iterations=[ + rule.IterationFactory.build( + iteration_rules=target_rules_map.get(targets[i]), + iteration_cohorts=[ + rule.IterationCohortFactory.build( + cohort_label="cohort1", + cohort_group=f"cohort_group{i + 1}", + positive_description=f"positive_desc_{i + 1}", + negative_description=f"negative_desc_{i + 1}", + ) + ], + ) + ], + ) + campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} + key = f"{campaign.name}.json" + s3_client.put_object( + Bucket=rules_bucket, Key=key, Body=json.dumps(campaign_data), ContentType="application/json" + ) + campaigns.append(campaign) + campaign_data_keys.append(key) + + yield campaigns + + for key in campaign_data_keys: + s3_client.delete_object(Bucket=rules_bucket, Key=key) + + @pytest.fixture(scope="class") def campaign_config_with_magic_cohort( s3_client: BaseClient, rules_bucket: BucketName diff --git a/tests/integration/lambda/test_app_running_as_lambda.py b/tests/integration/lambda/test_app_running_as_lambda.py index cdd1149d3..793234fa7 100644 --- a/tests/integration/lambda/test_app_running_as_lambda.py +++ b/tests/integration/lambda/test_app_running_as_lambda.py @@ -10,7 +10,17 @@ from brunns.matchers.data import json_matching as is_json_that from brunns.matchers.response import is_response from faker import Faker -from hamcrest import assert_that, contains_exactly, contains_string, has_entries, has_item, has_key +from hamcrest import ( + assert_that, + contains_exactly, + contains_inanyorder, + contains_string, + equal_to, + has_entries, + has_item, + has_key, + is_not, +) from yarl import URL from eligibility_signposting_api.model.eligibility import NHSNumber @@ -153,10 +163,10 @@ def get_log_messages(flask_function: str, logs_client: BaseClient) -> list[str]: return [e["message"] for e in log_events["events"]] -def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers( # noqa: PLR0913 +def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers_and_return_response( # noqa: PLR0913 lambda_client: BaseClient, # noqa:ARG001 persisted_person: NHSNumber, - campaign_config: CampaignConfig, # noqa:ARG001 + campaign_config: CampaignConfig, s3_client: BaseClient, audit_bucket: BucketName, api_gateway_endpoint: URL, @@ -166,7 +176,14 @@ def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers( # noqa: P invoke_url = f"{api_gateway_endpoint}/patient-check/{persisted_person}" response = httpx.get( invoke_url, - headers={"nhs-login-nhs-number": str(persisted_person)}, + headers={ + "nhs-login-nhs-number": str(persisted_person), + "x_request_id": "x_request_id", + "x_correlation_id": "x_correlation_id", + "nhsd_end_user_organisation_ods": "nhsd_end_user_organisation_ods", + "nhsd_application_id": "nhsd_application_id", + }, + params={"includeActions": "Y"}, timeout=10, ) @@ -180,7 +197,51 @@ def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers( # noqa: P object_keys = [obj["Key"] for obj in objects] latest_key = sorted(object_keys)[-1] audit_data = json.loads(s3_client.get_object(Bucket=audit_bucket, Key=latest_key)["Body"].read()) - assert_that(audit_data, has_entries(test_audit="check if audit works")) + + expected_headers = { + "x_request_id": "x_request_id", + "x_correlation_id": "x_correlation_id", + "nhsd_end_user_organisation_ods": "nhsd_end_user_organisation_ods", + "nhsd_application_id": "nhsd_application_id", + } + expected_query_params = {"category": None, "conditions": None, "include_actions": "Y"} + + assert_that(audit_data["request"]["request_timestamp"], is_not(equal_to(""))) + assert_that(audit_data["request"]["headers"], equal_to(expected_headers)) + assert_that(audit_data["request"]["nhs_number"], equal_to(persisted_person)) + assert_that(audit_data["request"]["query_params"], equal_to(expected_query_params)) + + expected_conditions = [ + { + "campaign_id": campaign_config.id, + "campaign_version": campaign_config.version, + "iteration_id": campaign_config.iterations[0].id, + "iteration_version": campaign_config.iterations[0].version, + "condition_name": campaign_config.target, + "status": "not_actionable", + "status_text": "not_actionable", + "eligibility_cohorts": [{"cohort_code": "cohort_group1", "cohort_status": "not_actionable"}], + "eligibility_cohort_groups": [ + { + "cohort_code": "cohort_group1", + "cohort_text": "positive_description", + "cohort_status": "not_actionable", + } + ], + "filter_rules": None, + "suitability_rules": { + "rule_priority": 10, + "rule_name": "Exclude too young less than 75", + "rule_message": "Exclude too young less than 75", + }, + "action_rule": None, + "actions": [], + } + ] + + assert_that(audit_data["response"]["response_id"], is_not(equal_to(""))) + assert_that(audit_data["response"]["last_updated"], is_not(equal_to(""))) + assert_that(audit_data["response"]["condition"], equal_to(expected_conditions)) def test_given_nhs_number_in_path_does_not_match_with_nhs_number_in_headers_results_in_error_response( @@ -203,3 +264,138 @@ def test_given_nhs_number_in_path_does_not_match_with_nhs_number_in_headers_resu response, is_response().with_status_code(HTTPStatus.FORBIDDEN).and_body("NHS number mismatch"), ) + + +def test_given_person_has_unique_status_for_different_conditions_with_audit( # noqa: PLR0913 + lambda_client: BaseClient, # noqa:ARG001 + persisted_person_all_cohorts: NHSNumber, + multiple_campaign_configs: list[CampaignConfig], + s3_client: BaseClient, + audit_bucket: BucketName, + api_gateway_endpoint: URL, +): + # Given + # When + invoke_url = f"{api_gateway_endpoint}/patient-check/{persisted_person_all_cohorts}" + response = httpx.get( + invoke_url, + headers={ + "nhs-login-nhs-number": str(persisted_person_all_cohorts), + "x_request_id": "x_request_id", + "x_correlation_id": "x_correlation_id", + "nhsd_end_user_organisation_ods": "nhsd_end_user_organisation_ods", + "nhsd_application_id": "nhsd_application_id", + }, + params={"includeActions": "Y"}, + timeout=10, + ) + + # Then + assert_that( + response, + is_response().with_status_code(HTTPStatus.OK).and_body(is_json_that(has_key("processedSuggestions"))), + ) + + objects = s3_client.list_objects_v2(Bucket=audit_bucket).get("Contents", []) + object_keys = [obj["Key"] for obj in objects] + latest_key = sorted(object_keys)[-1] + audit_data = json.loads(s3_client.get_object(Bucket=audit_bucket, Key=latest_key)["Body"].read()) + + expected_headers = { + "x_request_id": "x_request_id", + "x_correlation_id": "x_correlation_id", + "nhsd_end_user_organisation_ods": "nhsd_end_user_organisation_ods", + "nhsd_application_id": "nhsd_application_id", + } + expected_query_params = {"category": None, "conditions": None, "include_actions": "Y"} + + rsv_campaign = multiple_campaign_configs[0] + covid_campaign = multiple_campaign_configs[1] + flu_campaign = multiple_campaign_configs[2] + + expected_conditions = [ + { + "campaign_id": rsv_campaign.id, + "campaign_version": rsv_campaign.version, + "iteration_id": rsv_campaign.iterations[0].id, + "iteration_version": rsv_campaign.iterations[0].version, + "condition_name": rsv_campaign.target, + "status": "not_eligible", + "status_text": "not_eligible", + "eligibility_cohorts": [{"cohort_code": "cohort_group1", "cohort_status": "not_eligible"}], + "eligibility_cohort_groups": [ + { + "cohort_code": "cohort_group1", + "cohort_text": "negative_desc_1", + "cohort_status": "not_eligible", + } + ], + "filter_rules": {"rule_priority": 10, "rule_name": "Exclude too young less than 75"}, + "suitability_rules": None, + "action_rule": None, + "actions": [], + }, + { + "campaign_id": covid_campaign.id, + "campaign_version": covid_campaign.version, + "iteration_id": covid_campaign.iterations[0].id, + "iteration_version": covid_campaign.iterations[0].version, + "condition_name": covid_campaign.target, + "status": "not_actionable", + "status_text": "not_actionable", + "eligibility_cohorts": [{"cohort_code": "cohort_group2", "cohort_status": "not_actionable"}], + "eligibility_cohort_groups": [ + { + "cohort_code": "cohort_group2", + "cohort_text": "positive_desc_2", + "cohort_status": "not_actionable", + } + ], + "filter_rules": None, + "suitability_rules": { + "rule_priority": 10, + "rule_name": "Exclude too young less than 75", + "rule_message": "Exclude too young less than 75", + }, + "action_rule": None, + "actions": [], + }, + { + "campaign_id": flu_campaign.id, + "campaign_version": flu_campaign.version, + "iteration_id": flu_campaign.iterations[0].id, + "iteration_version": flu_campaign.iterations[0].version, + "condition_name": flu_campaign.target, + "status": "actionable", + "status_text": "actionable", + "eligibility_cohorts": [{"cohort_code": "cohort_group3", "cohort_status": "actionable"}], + "eligibility_cohort_groups": [ + { + "cohort_code": "cohort_group3", + "cohort_text": "positive_desc_3", + "cohort_status": "actionable", + } + ], + "filter_rules": None, + "suitability_rules": None, + "action_rule": {"rule_priority": 20, "rule_name": "In QE1"}, + "actions": [ + { + "internal_name": None, # TODO: FIX! + "action_type": "defaultcomms", + "action_code": "action_code", + "action_description": None, + "action_url": None, + "action_url_label": None, + } + ], + }, + ] + + assert_that(audit_data["request"]["request_timestamp"], is_not(equal_to(""))) + assert_that(audit_data["request"]["headers"], equal_to(expected_headers)) + assert_that(audit_data["request"]["nhs_number"], equal_to(persisted_person_all_cohorts)) + assert_that(audit_data["request"]["query_params"], equal_to(expected_query_params)) + assert_that(audit_data["response"]["response_id"], is_not(equal_to(""))) + assert_that(audit_data["response"]["last_updated"], is_not(equal_to(""))) + assert_that(audit_data["response"]["condition"], contains_inanyorder(*expected_conditions)) diff --git a/tests/unit/services/test_eligibility_services.py b/tests/unit/services/test_eligibility_services.py index a96c8b2bd..c99f3b73a 100644 --- a/tests/unit/services/test_eligibility_services.py +++ b/tests/unit/services/test_eligibility_services.py @@ -6,7 +6,6 @@ from eligibility_signposting_api.model.eligibility import NHSNumber from eligibility_signposting_api.repos import CampaignRepo, NotFoundError, PersonRepo from eligibility_signposting_api.services import EligibilityService, UnknownPersonError -from eligibility_signposting_api.services.audit_service import AuditService from eligibility_signposting_api.services.calculators.eligibility_calculator import EligibilityCalculatorFactory from tests.fixtures.matchers.eligibility import is_eligibility_status @@ -15,9 +14,8 @@ def test_eligibility_service_returns_from_repo(): # Given person_repo = MagicMock(spec=PersonRepo) campaign_repo = MagicMock(spec=CampaignRepo) - audit_service = MagicMock(spec=AuditService) person_repo.get_eligibility = MagicMock(return_value=[]) - service = EligibilityService(person_repo, campaign_repo, audit_service, EligibilityCalculatorFactory()) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber("1234567890")) @@ -30,9 +28,8 @@ def test_eligibility_service_for_nonexistent_nhs_number(): # Given person_repo = MagicMock(spec=PersonRepo) campaign_repo = MagicMock(spec=CampaignRepo) - audit_service = MagicMock(spec=AuditService) person_repo.get_eligibility_data = MagicMock(side_effect=NotFoundError) - service = EligibilityService(person_repo, campaign_repo, audit_service, EligibilityCalculatorFactory()) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When with pytest.raises(UnknownPersonError): diff --git a/tests/unit/test_audit_context.py b/tests/unit/test_audit_context.py new file mode 100644 index 000000000..97e65e83a --- /dev/null +++ b/tests/unit/test_audit_context.py @@ -0,0 +1,175 @@ +import uuid +from dataclasses import asdict +from datetime import UTC, datetime +from unittest.mock import Mock + +import pytest +from flask import Flask, g, request + +from eligibility_signposting_api.audit_context import AuditContext +from eligibility_signposting_api.audit_models import AuditEvent +from eligibility_signposting_api.model.eligibility import ( + ActionCode, + ActionDescription, + ActionType, + CohortGroupResult, + ConditionName, + IterationResult, + Reason, + RuleDescription, + RuleName, + RulePriority, + Status, + SuggestedAction, + SuggestedActions, + UrlLabel, + UrlLink, +) +from eligibility_signposting_api.model.rules import CampaignID, CampaignVersion, Iteration, RuleType +from eligibility_signposting_api.services.audit_service import AuditService +from tests.fixtures.builders.model.rule import IterationFactory +from tests.fixtures.builders.views.response_model.eligibility import EligibilityResponseFactory + + +@pytest.fixture +def app(): + return Flask(__name__) + + +def test_add_request_details_sets_audit_log_on_g(app): + headers = { + "X-Request-ID": "test-x-request-id", + "X-Correlation-ID": "test-x-correlation-id", + "NHSD-End-User-Organisation-ODS": "test-org", + "nhsd-application-id": "test-app-id", + } + + nhs_number = "1234567890" + url = "/patient-check?includeActions=Y" + + with app.test_request_context(url, headers=headers, method="GET"): + request.view_args = {"nhs_number": nhs_number} + AuditContext.add_request_details(request) + + assert hasattr(g, "audit_log") + audit_req = g.audit_log.request + assert audit_req.nhs_number == nhs_number + assert audit_req.headers.x_request_id == "test-x-request-id" + assert audit_req.headers.x_correlation_id == "test-x-correlation-id" + assert audit_req.headers.nhsd_end_user_organisation_ods == "test-org" + assert audit_req.headers.nhsd_application_id == "test-app-id" + assert audit_req.query_params.include_actions == "Y" + assert isinstance(audit_req.request_timestamp, datetime) + + +def test_add_request_details_when_headers_are_empty_sets_audit_log_on_g(app): + nhs_number = "1234567890" + url = "/patient-check?includeActions=Y" + + with app.test_request_context(url, method="GET"): + request.view_args = {"nhs_number": nhs_number} + AuditContext.add_request_details(request) + + assert hasattr(g, "audit_log") + audit_req = g.audit_log.request + assert audit_req.nhs_number == nhs_number + assert audit_req.headers.x_request_id is None + assert audit_req.headers.x_correlation_id is None + assert audit_req.headers.nhsd_end_user_organisation_ods is None + assert audit_req.headers.nhsd_application_id is None + assert audit_req.query_params.include_actions == "Y" + assert isinstance(audit_req.request_timestamp, datetime) + + +def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): + suggested_actions: SuggestedActions | None + condition_name: ConditionName + best_results: tuple[Iteration, IterationResult, dict[str, CohortGroupResult]] + campaign_details: tuple[CampaignID | None, CampaignVersion | None] + redirect_rule_details: tuple[RulePriority | None, RuleName | None] + + suggested_actions = SuggestedActions( + actions=[ + SuggestedAction( + action_code=ActionCode("ActionCode1"), + action_type=ActionType("ActionType1"), + action_description=ActionDescription("ActionDescription1"), + url_link=UrlLink("https://www.example.com"), + url_label=UrlLabel("ActionLabel1"), + ) + ] + ) + + condition_name = ConditionName("Condition1") + iteration = IterationFactory.build() + audit_rules = [ + Reason( + rule_type=RuleType.filter, + rule_name=RuleName("FilterRuleName1"), + rule_description=RuleDescription("FilterRuleDescription1"), + matcher_matched=True, + rule_priority=RulePriority("1"), + ) + ] + cohort_group_result = CohortGroupResult( + status=Status.actionable, + cohort_code="CohortCode1", + description="CohortDescription1", + audit_rules=audit_rules, + reasons=audit_rules, + ) + iteration_result = IterationResult( + status=Status.actionable, cohort_results=[cohort_group_result], actions=suggested_actions + ) + best_results = (iteration, iteration_result, {"CohortCode1": cohort_group_result}) + campaign_details = (CampaignID("CampaignID1"), CampaignVersion("CampaignVersion1")) + redirect_rule_details = (RulePriority("1"), RuleName("RedirectRuleName1")) + + with app.app_context(): + g.audit_log = AuditEvent() + + AuditContext.append_audit_condition( + suggested_actions, condition_name, best_results, campaign_details, redirect_rule_details + ) + + assert g.audit_log.response.condition, condition_name + cond = g.audit_log.response.condition[0] + assert cond.condition_name == condition_name + assert cond.campaign_id == campaign_details[0] + assert cond.status == best_results[1].status.name + assert cond.status_text == best_results[1].status.name + + +def test_add_response_details_adds_to_audit_log_on_g(app): + eligibility_response = EligibilityResponseFactory.build( + response_id=uuid.uuid4(), + meta={"last_updated": datetime(2023, 1, 1, 0, 0, tzinfo=UTC)}, + processed_suggestions=[], + ) + + with app.app_context(): + g.audit_log = AuditEvent() + + AuditContext.add_response_details(eligibility_response) + + assert g.audit_log.response.response_id == eligibility_response.response_id + assert g.audit_log.response.last_updated is eligibility_response.meta.last_updated + + +def test_write_to_firehose_calls_audit_service_with_correct_data_from_g(app): + mock_audit_service = Mock(spec=AuditService) + eligibility_response = EligibilityResponseFactory.build( + response_id=(uuid.uuid4()), + meta={"last_updated": (datetime(2023, 1, 1, 0, 0, tzinfo=UTC))}, + processed_suggestions=[], + ) + + with app.app_context(): + g.audit_log = AuditEvent() + + AuditContext.write_to_firehose(mock_audit_service, eligibility_response) + + assert g.audit_log.response.response_id == eligibility_response.response_id + assert g.audit_log.response.last_updated == eligibility_response.meta.last_updated + + mock_audit_service.audit.assert_called_once_with(asdict(g.audit_log)) diff --git a/tests/unit/views/test_eligibility.py b/tests/unit/views/test_eligibility.py index 56e1becb2..2e83ba21c 100644 --- a/tests/unit/views/test_eligibility.py +++ b/tests/unit/views/test_eligibility.py @@ -18,10 +18,12 @@ Reason, RuleDescription, RuleName, + RulePriority, RuleType, Status, ) from eligibility_signposting_api.services import EligibilityService, UnknownPersonError +from eligibility_signposting_api.services.audit_service import AuditService from eligibility_signposting_api.services.eligibility_services import InvalidQueryParamError from eligibility_signposting_api.views.eligibility import ( build_eligibility_cohorts, @@ -38,6 +40,11 @@ logger = logging.getLogger(__name__) +class FakeAuditService: + def audit(self, audit_record): + pass + + class FakeEligibilityService(EligibilityService): def __init__(self): pass @@ -79,7 +86,10 @@ def get_eligibility_status( def test_nhs_number_given(app: Flask, client: FlaskClient): # Given - with get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()): + with ( + get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()), + get_app_container(app).override.service(AuditService, new=FakeAuditService()), + ): # When response = client.get("/patient-check/12345") @@ -212,18 +222,21 @@ def test_build_suitability_results_with_deduplication(): rule_name=RuleName("Exclude too young less than 75"), rule_description=RuleDescription("your age is greater than 75"), matcher_matched=False, + rule_priority=RulePriority(1), ), Reason( rule_type=RuleType.suppression, rule_name=RuleName("Exclude too young less than 75"), rule_description=RuleDescription("your age is greater than 75"), matcher_matched=False, + rule_priority=RulePriority(1), ), Reason( rule_type=RuleType.suppression, rule_name=RuleName("Exclude more than 100"), rule_description=RuleDescription("your age is greater than 100"), matcher_matched=False, + rule_priority=RulePriority(1), ), ], ), @@ -236,6 +249,7 @@ def test_build_suitability_results_with_deduplication(): rule_name=RuleName("Exclude too young less than 75"), rule_description=RuleDescription("your age is greater than 75"), matcher_matched=False, + rule_priority=RulePriority(1), ) ], ), @@ -248,6 +262,7 @@ def test_build_suitability_results_with_deduplication(): rule_name=RuleName("Exclude is present in sw1"), rule_description=RuleDescription("your a member of sw1"), matcher_matched=False, + rule_priority=RulePriority(1), ) ], ), @@ -261,6 +276,7 @@ def test_build_suitability_results_with_deduplication(): rule_name=RuleName("Already vaccinated"), rule_description=RuleDescription("you have already vaccinated"), matcher_matched=False, + rule_priority=RulePriority(1), ) ], ), @@ -294,18 +310,21 @@ def test_build_suitability_results_when_rule_text_is_empty_or_null(): rule_name=RuleName("Exclude too young less than 75"), rule_description=RuleDescription("your age is greater than 75"), matcher_matched=False, + rule_priority=RulePriority(1), ), Reason( rule_type=RuleType.suppression, rule_name=RuleName("Exclude more than 100"), rule_description=RuleDescription(""), matcher_matched=False, + rule_priority=RulePriority(1), ), Reason( rule_type=RuleType.suppression, rule_name=RuleName("Exclude more than 100"), matcher_matched=False, rule_description=None, + rule_priority=RulePriority(1), ), ], ), @@ -318,6 +337,7 @@ def test_build_suitability_results_when_rule_text_is_empty_or_null(): rule_name=RuleName("Exclude is present in sw1"), rule_description=RuleDescription(""), matcher_matched=False, + rule_priority=RulePriority(1), ) ], ), @@ -330,6 +350,7 @@ def test_build_suitability_results_when_rule_text_is_empty_or_null(): rule_name=RuleName("Exclude is present in sw1"), rule_description=None, matcher_matched=False, + rule_priority=RulePriority(1), ) ], ), @@ -358,7 +379,10 @@ def test_no_suitability_rules_for_actionable(): def test_nhs_number_and_include_actions_param_given_and_is_yes(app: Flask, client: FlaskClient): # Given - with get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()): + with ( + get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()), + get_app_container(app).override.service(AuditService, new=FakeAuditService()), + ): # When response = client.get("/patient-check/12345?includeActions=Y") @@ -368,7 +392,10 @@ def test_nhs_number_and_include_actions_param_given_and_is_yes(app: Flask, clien def test_nhs_number_and_include_actions_param_no_given(app: Flask, client: FlaskClient): # Given - with get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()): + with ( + get_app_container(app).override.service(EligibilityService, new=FakeEligibilityService()), + get_app_container(app).override.service(AuditService, new=FakeAuditService()), + ): # When response = client.get("/patient-check/12345?includeActions=N")