From 62a1ca65a4bc27b6215e127793578a1adce41172 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Tue, 29 Jul 2025 16:31:10 +0100 Subject: [PATCH 1/4] ELI-351: Refactor --- .../audit/audit_context.py | 35 ++- .../model/eligibility_status.py | 22 +- .../calculators/eligibility_calculator.py | 226 +++++------------- .../processors/action_rule_handler.py | 86 +++++++ .../services/processors/cohort_handler.py | 12 +- .../services/processors/rule_processor.py | 18 +- tests/unit/audit/test_audit_context.py | 19 +- .../test_eligibility_calculator.py | 41 ---- 8 files changed, 213 insertions(+), 246 deletions(-) create mode 100644 src/eligibility_signposting_api/services/processors/action_rule_handler.py diff --git a/src/eligibility_signposting_api/audit/audit_context.py b/src/eligibility_signposting_api/audit/audit_context.py index 8a0606983..48f4389bf 100644 --- a/src/eligibility_signposting_api/audit/audit_context.py +++ b/src/eligibility_signposting_api/audit/audit_context.py @@ -18,17 +18,12 @@ RequestAuditQueryParams, ) from eligibility_signposting_api.audit.audit_service import AuditService -from eligibility_signposting_api.model.campaign_config import ( - CampaignID, - CampaignVersion, - Iteration, - RuleName, - RulePriority, -) from eligibility_signposting_api.model.eligibility_status import ( + BestIterationResult, CohortGroupResult, ConditionName, IterationResult, + MatchedActionDetail, Status, SuggestedAction, ) @@ -65,17 +60,15 @@ def add_request_details(request: Request) -> None: @staticmethod def append_audit_condition( - suggested_actions: list[SuggestedAction] | None, condition_name: ConditionName, - best_results: tuple[Iteration | None, IterationResult | None, dict[str, CohortGroupResult] | None], - campaign_details: tuple[CampaignID | None, CampaignVersion | None], - action_rule_details: tuple[RulePriority | None, RuleName | None], + best_iteration_result: BestIterationResult, + action_detail: MatchedActionDetail, ) -> None: audit_eligibility_cohorts, audit_eligibility_cohort_groups, audit_actions = [], [], [] audit_filter_rule, audit_suitability_rule, audit_action_rule = None, None, None - best_active_iteration = best_results[0] - best_candidate = best_results[1] - best_cohort_results = best_results[2] + best_active_iteration = best_iteration_result.active_iteration + best_candidate = best_iteration_result.iteration_result + best_cohort_results = best_iteration_result.cohort_results if best_cohort_results: for cohort_label, result in sorted(best_cohort_results.items(), key=lambda item: item[1].cohort_code): @@ -94,13 +87,13 @@ def append_audit_condition( audit_filter_rule = AuditContext.create_audit_filter_rule(best_candidate, result) audit_suitability_rule = AuditContext.create_audit_suitability_rule(best_candidate, result) - audit_action_rule = AuditContext.add_rule_name_and_priority_to_audit(best_candidate, action_rule_details) + audit_action_rule = AuditContext.add_rule_name_and_priority_to_audit(best_candidate, action_detail) - audit_actions = AuditContext.create_audit_actions(suggested_actions) + audit_actions = AuditContext.create_audit_actions(action_detail.actions) audit_condition = AuditCondition( - campaign_id=campaign_details[0], - campaign_version=campaign_details[1], + campaign_id=best_iteration_result.campaign_id, + campaign_version=best_iteration_result.campaign_version, iteration_id=best_active_iteration.id if best_active_iteration else None, iteration_version=best_active_iteration.version if best_active_iteration else None, condition_name=condition_name, @@ -119,15 +112,15 @@ def append_audit_condition( @staticmethod def add_rule_name_and_priority_to_audit( best_candidate: IterationResult | None, - action_rule_details: tuple[RulePriority | None, RuleName | None] | None, + action_detail: MatchedActionDetail, ) -> AuditRedirectRule | None: audit_action_rule = None if best_candidate and best_candidate.status: - if action_rule_details is None or (action_rule_details[0] is None and action_rule_details[1] is None): + if action_detail.rule_priority is None and action_detail.rule_name is None: audit_action_rule = None else: audit_action_rule = AuditRedirectRule( - rule_priority=str(action_rule_details[0]), rule_name=action_rule_details[1] + rule_priority=str(action_detail.rule_priority), rule_name=action_detail.rule_name ) return audit_action_rule diff --git a/src/eligibility_signposting_api/model/eligibility_status.py b/src/eligibility_signposting_api/model/eligibility_status.py index 61ca94a1d..a35058ce5 100644 --- a/src/eligibility_signposting_api/model/eligibility_status.py +++ b/src/eligibility_signposting_api/model/eligibility_status.py @@ -4,10 +4,14 @@ from datetime import date from enum import Enum, StrEnum, auto from functools import total_ordering -from typing import NewType, Self +from typing import TYPE_CHECKING, NewType, Self from pydantic import HttpUrl +if TYPE_CHECKING: + from eligibility_signposting_api.model import campaign_config + from eligibility_signposting_api.model.campaign_config import CampaignID, CampaignVersion, CohortLabel, Iteration + NHSNumber = NewType("NHSNumber", str) DateOfBirth = NewType("DateOfBirth", date) Postcode = NewType("Postcode", str) @@ -122,6 +126,22 @@ class IterationResult: actions: list[SuggestedAction] | None +@dataclass +class BestIterationResult: + iteration_result: IterationResult + active_iteration: Iteration | None = None + campaign_id: CampaignID | None = None + campaign_version: CampaignVersion | None = None + cohort_results: dict[CohortLabel, CohortGroupResult] | None = None + + +@dataclass +class MatchedActionDetail: + rule_name: campaign_config.RuleName | None = None + rule_priority: campaign_config.RulePriority | None = None + actions: list[SuggestedAction] | None = None + + @dataclass class EligibilityStatus: """Represents a person's eligibility for vaccination.""" diff --git a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py index 2e452b7c5..ccb1ea5fa 100644 --- a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py +++ b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py @@ -1,9 +1,7 @@ from __future__ import annotations -from _operator import attrgetter from collections import defaultdict from dataclasses import dataclass, field -from itertools import groupby from typing import TYPE_CHECKING from wireup import service @@ -11,33 +9,23 @@ from eligibility_signposting_api.audit.audit_context import AuditContext from eligibility_signposting_api.model import eligibility_status from eligibility_signposting_api.model.campaign_config import ( - ActionsMapper, CampaignConfig, - CampaignID, - CampaignVersion, + CohortLabel, Iteration, - IterationRule, - RuleName, - RulePriority, + IterationName, RuleType, ) from eligibility_signposting_api.model.eligibility_status import ( - ActionCode, - ActionDescription, - ActionType, + BestIterationResult, CohortGroupResult, Condition, ConditionName, - InternalActionCode, + EligibilityStatus, IterationResult, + MatchedActionDetail, Status, - SuggestedAction, - UrlLabel, - UrlLink, -) -from eligibility_signposting_api.services.calculators.rule_calculator import ( - RuleCalculator, ) +from eligibility_signposting_api.services.processors.action_rule_handler import ActionRuleHandler from eligibility_signposting_api.services.processors.campaign_evaluator import CampaignEvaluator from eligibility_signposting_api.services.processors.rule_processor import RuleProcessor @@ -61,12 +49,13 @@ class EligibilityCalculator: campaign_evaluator: CampaignEvaluator = field(default_factory=CampaignEvaluator) rule_processor: RuleProcessor = field(default_factory=RuleProcessor) + action_rule_handler: ActionRuleHandler = field(default_factory=ActionRuleHandler) results: list[eligibility_status.Condition] = field(default_factory=list) @staticmethod def get_the_best_cohort_memberships( - cohort_results: dict[str, CohortGroupResult], + cohort_results: dict[CohortLabel, CohortGroupResult], ) -> tuple[Status, list[CohortGroupResult]]: if not cohort_results: return eligibility_status.Status.not_eligible, [] @@ -87,153 +76,84 @@ def get_the_best_cohort_memberships( return best_status, best_cohorts - @staticmethod - def get_action_rules_components( - active_iteration: Iteration, rule_type: RuleType - ) -> tuple[tuple[IterationRule, ...], ActionsMapper, str | None]: - action_rules = tuple(rule for rule in active_iteration.iteration_rules if rule.type in rule_type) - - routing_map = { - RuleType.redirect: active_iteration.default_comms_routing, - RuleType.not_eligible_actions: active_iteration.default_not_eligible_routing, - RuleType.not_actionable_actions: active_iteration.default_not_actionable_routing, - } - - default_comms = routing_map.get(rule_type) - action_mapper = active_iteration.actions_mapper - return action_rules, action_mapper, default_comms - - def get_eligibility_status( - self, include_actions: str, conditions: list[str], category: str - ) -> eligibility_status.EligibilityStatus: + def get_eligibility_status(self, include_actions: str, conditions: list[str], category: str) -> EligibilityStatus: include_actions_flag = include_actions.upper() == "Y" condition_results: dict[ConditionName, IterationResult] = {} - action_rule_priority, action_rule_name = None, None requested_grouped_campaigns = self.campaign_evaluator.get_requested_grouped_campaigns( self.campaign_configs, conditions, category ) for condition_name, campaign_group in requested_grouped_campaigns: - actions: list[SuggestedAction] | None = [] - best_active_iteration: Iteration | None - best_candidate: IterationResult - best_campaign_id: CampaignID | None - best_campaign_version: CampaignVersion | None - best_cohort_results: dict[str, CohortGroupResult] | None - - iteration_results = self.get_iteration_results(actions, campaign_group) - - # Determine results between iterations - get the best - if iteration_results: - ( - 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.Status.not_eligible, [], actions) - best_campaign_id = None - best_campaign_version = None - best_active_iteration = None - best_cohort_results = None - - condition_results[condition_name] = best_candidate - - status_to_rule_type = { - Status.actionable: RuleType.redirect, - Status.not_eligible: RuleType.not_eligible_actions, - Status.not_actionable: RuleType.not_actionable_actions, - } - - if best_candidate.status in status_to_rule_type and best_active_iteration is not None: - if include_actions_flag: - rule_type = status_to_rule_type[best_candidate.status] - actions, matched_action_rule_priority, matched_action_rule_name = self.handle_action_rules( - best_active_iteration, rule_type - ) - action_rule_name = matched_action_rule_name - action_rule_priority = matched_action_rule_priority - else: - actions = None - - 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 - - # add audit data - 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), - (action_rule_priority, action_rule_name), + best_iteration_result = self.get_best_iteration_result(campaign_group) + matched_action_detail = self.get_actions( + best_iteration_result.active_iteration, + best_iteration_result.iteration_result, + include_actions_flag=include_actions_flag, ) + condition_results[condition_name] = best_iteration_result.iteration_result + condition_results[condition_name].actions = matched_action_detail.actions + + AuditContext.append_audit_condition(condition_name, best_iteration_result, matched_action_detail) + # Consolidate all the results and return final_result = self.build_condition_results(condition_results) return eligibility_status.EligibilityStatus(conditions=final_result) - def get_iteration_results( - self, actions: list[SuggestedAction] | None, campaign_group: list[CampaignConfig] - ) -> dict[str, tuple[Iteration, IterationResult, CampaignID, CampaignVersion, dict[str, CohortGroupResult]]]: - iteration_results: dict[ - str, tuple[Iteration, IterationResult, CampaignID, CampaignVersion, dict[str, CohortGroupResult]] - ] = {} + def get_actions( + self, active_iteration: Iteration | None, best_iteration_result: IterationResult, *, include_actions_flag: bool + ) -> MatchedActionDetail: + status_to_rule_type = { + Status.actionable: RuleType.redirect, + Status.not_eligible: RuleType.not_eligible_actions, + Status.not_actionable: RuleType.not_actionable_actions, + } + + action_detail = MatchedActionDetail() + + if ( + best_iteration_result.status in status_to_rule_type + and active_iteration is not None + and include_actions_flag + ): + rule_type = status_to_rule_type[best_iteration_result.status] + action_detail = self.action_rule_handler.handle(self.person, active_iteration, rule_type) + + return action_detail + + def get_best_iteration_result(self, campaign_group: list[CampaignConfig]) -> BestIterationResult: + iteration_results = self.get_iteration_results(campaign_group) + + if iteration_results: + (best_iteration_name, best_iteration_result) = max( + iteration_results.items(), + key=lambda item: next(iter(item[1].cohort_results.values())).status.value + # Below handles the case where there are no cohort results + if item[1].cohort_results + else -1, + ) + else: + iteration_result = IterationResult(eligibility_status.Status.not_eligible, [], []) + best_iteration_result = BestIterationResult(iteration_result, None, None, None, {}) + + return best_iteration_result + + def get_iteration_results(self, campaign_group: list[CampaignConfig]) -> dict[IterationName, BestIterationResult]: + iteration_results: dict[IterationName, BestIterationResult] = {} + for cc in campaign_group: active_iteration = cc.current_iteration - cohort_results: dict[str, CohortGroupResult] = self.rule_processor.get_cohort_group_results( + cohort_results: dict[CohortLabel, CohortGroupResult] = self.rule_processor.get_cohort_group_results( self.person, active_iteration ) # Determine Result between cohorts - get the best status, best_cohorts = self.get_the_best_cohort_memberships(cohort_results) - iteration_results[active_iteration.name] = ( - active_iteration, - IterationResult(status, best_cohorts, actions), - cc.id, - cc.version, - cohort_results, + iteration_results[active_iteration.name] = BestIterationResult( + IterationResult(status, best_cohorts, []), active_iteration, cc.id, cc.version, cohort_results ) return iteration_results - def handle_action_rules( - self, best_active_iteration: Iteration, rule_type: RuleType - ) -> tuple[list[SuggestedAction] | None, RulePriority | None, RuleName | None]: - action_rules, action_mapper, default_comms = self.get_action_rules_components(best_active_iteration, rule_type) - priority_getter = attrgetter("priority") - sorted_rules_by_priority = sorted(action_rules, key=priority_getter) - - actions: list[SuggestedAction] | None = self.get_actions_from_comms(action_mapper, default_comms) # pyright: ignore[reportArgumentType] - - matched_action_rule_priority, matched_action_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 = [ - RuleCalculator(person=self.person, rule=rule).evaluate_exclusion()[1].matcher_matched - for rule in rule_group_list - ] - - comms_routing = rule_group_list[0].comms_routing - if comms_routing and all(matcher_matched_list): - rule_actions = self.get_actions_from_comms(action_mapper, comms_routing) - if rule_actions and len(rule_actions) > 0: - actions = rule_actions - matched_action_rule_priority = rule_group_list[0].priority - matched_action_rule_name = rule_group_list[0].name - break - - return actions, matched_action_rule_priority, matched_action_rule_name - @staticmethod def build_condition_results(condition_results: dict[ConditionName, IterationResult]) -> list[Condition]: conditions: list[Condition] = [] @@ -271,23 +191,3 @@ def build_condition_results(condition_results: dict[ConditionName, IterationResu ) ) return conditions - - @staticmethod - def get_actions_from_comms(action_mapper: ActionsMapper, comms: str) -> list[SuggestedAction] | None: - suggested_actions: list[SuggestedAction] = [] - for comm in comms.split("|"): - action = action_mapper.get(comm) - if action is not None: - suggested_actions.append( - SuggestedAction( - internal_action_code=InternalActionCode(comm), - action_type=ActionType(action.action_type), - action_code=ActionCode(action.action_code), - action_description=ActionDescription(action.action_description) - if action.action_description - else None, - url_link=UrlLink(action.url_link) if action.url_link else None, - url_label=UrlLabel(action.url_label) if action.url_label else None, - ) - ) - return suggested_actions diff --git a/src/eligibility_signposting_api/services/processors/action_rule_handler.py b/src/eligibility_signposting_api/services/processors/action_rule_handler.py new file mode 100644 index 000000000..bcfc76cbf --- /dev/null +++ b/src/eligibility_signposting_api/services/processors/action_rule_handler.py @@ -0,0 +1,86 @@ +from itertools import groupby +from operator import attrgetter + +from eligibility_signposting_api.model.campaign_config import ( + ActionsMapper, + Iteration, + IterationRule, + RuleType, +) +from eligibility_signposting_api.model.eligibility_status import ( + ActionCode, + ActionDescription, + ActionType, + InternalActionCode, + MatchedActionDetail, + SuggestedAction, + UrlLabel, + UrlLink, +) +from eligibility_signposting_api.model.person import Person +from eligibility_signposting_api.services.calculators.rule_calculator import RuleCalculator + + +class ActionRuleHandler: + def handle(self, person: Person, best_active_iteration: Iteration, rule_type: RuleType) -> MatchedActionDetail: + action_rules, action_mapper, default_comms = self.get_action_rules_components(best_active_iteration, rule_type) + + priority_getter = attrgetter("priority") + sorted_rules_by_priority = sorted(action_rules, key=priority_getter) + + actions: list[SuggestedAction] | None = self.get_actions_from_comms(action_mapper, default_comms) # pyright: ignore[reportArgumentType] + + matched_action_rule_priority, matched_action_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 = [ + RuleCalculator(person=person, rule=rule).evaluate_exclusion()[1].matcher_matched + for rule in rule_group_list + ] + + comms_routing = rule_group_list[0].comms_routing + if comms_routing and all(matcher_matched_list): + rule_actions = self.get_actions_from_comms(action_mapper, comms_routing) + if rule_actions and len(rule_actions) > 0: + actions = rule_actions + matched_action_rule_priority = rule_group_list[0].priority + matched_action_rule_name = rule_group_list[0].name + break + + return MatchedActionDetail(matched_action_rule_name, matched_action_rule_priority, actions) + + @staticmethod + def get_action_rules_components( + active_iteration: Iteration, rule_type: RuleType + ) -> tuple[tuple[IterationRule, ...], ActionsMapper, str | None]: + action_rules = tuple(rule for rule in active_iteration.iteration_rules if rule.type in rule_type) + + routing_map = { + RuleType.redirect: active_iteration.default_comms_routing, + RuleType.not_eligible_actions: active_iteration.default_not_eligible_routing, + RuleType.not_actionable_actions: active_iteration.default_not_actionable_routing, + } + + default_comms = routing_map.get(rule_type) + action_mapper = active_iteration.actions_mapper + return action_rules, action_mapper, default_comms + + @staticmethod + def get_actions_from_comms(action_mapper: ActionsMapper, comms: str) -> list[SuggestedAction] | None: + suggested_actions: list[SuggestedAction] = [] + for comm in comms.split("|"): + action = action_mapper.get(comm) + if action is not None: + suggested_actions.append( + SuggestedAction( + internal_action_code=InternalActionCode(comm), + action_type=ActionType(action.action_type), + action_code=ActionCode(action.action_code), + action_description=ActionDescription(action.action_description) + if action.action_description + else None, + url_link=UrlLink(action.url_link) if action.url_link else None, + url_label=UrlLabel(action.url_label) if action.url_label else None, + ) + ) + return suggested_actions diff --git a/src/eligibility_signposting_api/services/processors/cohort_handler.py b/src/eligibility_signposting_api/services/processors/cohort_handler.py index 6ffe18f5d..d5848e52b 100644 --- a/src/eligibility_signposting_api/services/processors/cohort_handler.py +++ b/src/eligibility_signposting_api/services/processors/cohort_handler.py @@ -8,7 +8,7 @@ if TYPE_CHECKING: from collections.abc import Iterable - from eligibility_signposting_api.model.campaign_config import IterationCohort, IterationRule + from eligibility_signposting_api.model.campaign_config import CohortLabel, IterationCohort, IterationRule from eligibility_signposting_api.model.person import Person from eligibility_signposting_api.services.processors.rule_processor import RuleProcessor @@ -24,7 +24,7 @@ def handle( self, person: Person, cohort: IterationCohort, - cohort_results: dict[str, CohortGroupResult], + cohort_results: dict[CohortLabel, CohortGroupResult], rules_processor: RuleProcessor, ) -> None: """Handles a part of the eligibility/actionability check or passes to the next handler.""" @@ -38,7 +38,7 @@ def pass_to_next( self, person: Person, cohort: IterationCohort, - cohort_results: dict[str, CohortGroupResult], + cohort_results: dict[CohortLabel, CohortGroupResult], rules_processor: RuleProcessor, ) -> None: """Passes the request to the next handler in the chain if one exists.""" @@ -53,7 +53,7 @@ def handle( self, person: Person, cohort: IterationCohort, - cohort_results: dict[str, CohortGroupResult], + cohort_results: dict[CohortLabel, CohortGroupResult], rules_processor: RuleProcessor, ) -> None: if not rules_processor.is_base_eligible(person, cohort): @@ -82,7 +82,7 @@ def handle( self, person: Person, cohort: IterationCohort, - cohort_results: dict[str, CohortGroupResult], + cohort_results: dict[CohortLabel, CohortGroupResult], rules_processor: RuleProcessor, ) -> None: if not rules_processor.is_eligible(person, cohort, cohort_results, self.filter_rules): @@ -104,7 +104,7 @@ def handle( self, person: Person, cohort: IterationCohort, - cohort_results: dict[str, CohortGroupResult], + cohort_results: dict[CohortLabel, CohortGroupResult], rules_processor: RuleProcessor, ) -> None: rules_processor.is_actionable(person, cohort, cohort_results, self.suppression_rules) diff --git a/src/eligibility_signposting_api/services/processors/rule_processor.py b/src/eligibility_signposting_api/services/processors/rule_processor.py index 52d0d25dc..05addf584 100644 --- a/src/eligibility_signposting_api/services/processors/rule_processor.py +++ b/src/eligibility_signposting_api/services/processors/rule_processor.py @@ -8,7 +8,13 @@ from wireup import service from eligibility_signposting_api.model import eligibility_status -from eligibility_signposting_api.model.campaign_config import Iteration, IterationCohort, IterationRule, RuleType +from eligibility_signposting_api.model.campaign_config import ( + CohortLabel, + Iteration, + IterationCohort, + IterationRule, + RuleType, +) from eligibility_signposting_api.model.eligibility_status import CohortGroupResult, Status from eligibility_signposting_api.services.calculators.rule_calculator import RuleCalculator from eligibility_signposting_api.services.processors.cohort_handler import ( @@ -39,7 +45,7 @@ def is_eligible( self, person: Person, cohort: IterationCohort, - cohort_results: dict[str, CohortGroupResult], + cohort_results: dict[CohortLabel, CohortGroupResult], filter_rules: Iterable[IterationRule], ) -> bool: is_eligible = True @@ -65,7 +71,7 @@ def is_actionable( self, person: Person, cohort: IterationCohort, - cohort_results: dict[str, CohortGroupResult], + cohort_results: dict[CohortLabel, CohortGroupResult], suppression_rules: Iterable[IterationRule], ) -> None: is_actionable: bool = True @@ -126,8 +132,10 @@ def get_exclusion_rules(cohort: IterationCohort, rules: Iterable[IterationRule]) or (isinstance(ir.cohort_label, (list, set, tuple)) and cohort.cohort_label in ir.cohort_label) ) - def get_cohort_group_results(self, person: Person, active_iteration: Iteration) -> dict[str, CohortGroupResult]: - cohort_results: dict[str, CohortGroupResult] = {} + def get_cohort_group_results( + self, person: Person, active_iteration: Iteration + ) -> dict[CohortLabel, CohortGroupResult]: + cohort_results: dict[CohortLabel, CohortGroupResult] = {} filter_rules, suppression_rules = self.get_rules_by_type(active_iteration) cohort_base_handler = BaseEligibilityHandler() diff --git a/tests/unit/audit/test_audit_context.py b/tests/unit/audit/test_audit_context.py index eabcdc767..ccd55a5f7 100644 --- a/tests/unit/audit/test_audit_context.py +++ b/tests/unit/audit/test_audit_context.py @@ -9,15 +9,17 @@ from eligibility_signposting_api.audit.audit_context import AuditContext from eligibility_signposting_api.audit.audit_models import AuditAction, AuditEvent from eligibility_signposting_api.audit.audit_service import AuditService -from eligibility_signposting_api.model.campaign_config import CampaignID, CampaignVersion, Iteration, RuleType +from eligibility_signposting_api.model.campaign_config import CampaignID, CampaignVersion, RuleType from eligibility_signposting_api.model.eligibility_status import ( ActionCode, ActionDescription, ActionType, + BestIterationResult, CohortGroupResult, ConditionName, InternalActionCode, IterationResult, + MatchedActionDetail, Reason, RuleDescription, RuleName, @@ -83,9 +85,7 @@ def test_add_request_details_when_headers_are_empty_sets_audit_log_on_g(app): def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): suggested_actions: list[SuggestedAction] | 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 = [ SuggestedAction( @@ -119,16 +119,17 @@ def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): 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")) + matched_action_detail = MatchedActionDetail(RuleName("RedirectRuleName1"), RulePriority("1"), suggested_actions) + + best_iteration_results = BestIterationResult( + iteration_result, iteration, campaign_details[0], campaign_details[1], {"CohortCode1": cohort_group_result} + ) with app.app_context(): g.audit_log = AuditEvent() - AuditContext.append_audit_condition( - suggested_actions, condition_name, best_results, campaign_details, redirect_rule_details - ) + AuditContext.append_audit_condition(condition_name, best_iteration_results, matched_action_detail) expected_audit_action = [ AuditAction( @@ -148,7 +149,7 @@ def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): assert cond.campaign_version == campaign_details[1] assert cond.iteration_id == iteration.id assert cond.iteration_version == iteration.version - assert cond.status == best_results[1].status.name + assert cond.status == "actionable" assert cond.status_text == "You should have the Condition1 vaccine" assert cond.actions == expected_audit_action assert cond.action_rule.rule_priority == "1" diff --git a/tests/unit/services/calculators/test_eligibility_calculator.py b/tests/unit/services/calculators/test_eligibility_calculator.py index 0f7798b8f..11ea20273 100644 --- a/tests/unit/services/calculators/test_eligibility_calculator.py +++ b/tests/unit/services/calculators/test_eligibility_calculator.py @@ -50,7 +50,6 @@ is_eligibility_status, is_reason, ) -from tests.fixtures.matchers.rules import is_iteration_rule @pytest.fixture @@ -58,46 +57,6 @@ def app(): return Flask(__name__) -class TestEligibilityCalculator: - @staticmethod - def test_get_action_rules_components(): - # Given - - iteration = rule_builder.IterationFactory.build( - iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort2")], - default_comms_routing="defaultcomms", - actions_mapper=rule_builder.ActionsMapperFactory.build( - root={ - "ActionCode1": AvailableAction( - ActionType="ActionType1", - ExternalRoutingCode="ActionCode1", - ActionDescription="ActionDescription1", - UrlLink=HttpUrl("https://www.ActionUrl1.com"), - UrlLabel="ActionLabel1", - ), - "defaultcomms": AvailableAction( - ActionType="ActionType2", - ExternalRoutingCode="defaultcomms", - ActionDescription="ActionDescription2", - UrlLink=HttpUrl("https://www.ActionUrl2.com"), - UrlLabel="ActionLabel2", - ), - } - ), - iteration_rules=[rule_builder.ICBRedirectRuleFactory.build()], - ) - - # when - actual_rules, actual_action_mapper, actual_default_comms = EligibilityCalculator.get_action_rules_components( - iteration, RuleType.redirect - ) - - # then - assert_that(actual_rules, has_item(is_iteration_rule().with_name(iteration.iteration_rules[0].name))) - assert actual_action_mapper == iteration.actions_mapper - assert actual_default_comms == iteration.default_comms_routing - - def test_not_base_eligible(faker: Faker): # Given nhs_number = NHSNumber(faker.nhs_number()) From 7b2b7adb70aa95616026abc1cee217c225da816d Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Wed, 30 Jul 2025 14:05:22 +0100 Subject: [PATCH 2/4] ELI-351: Adds tests for action rule handler --- .../processors/test_action_rule_handler.py | 622 ++++++++++++++++++ 1 file changed, 622 insertions(+) create mode 100644 tests/unit/services/processors/test_action_rule_handler.py diff --git a/tests/unit/services/processors/test_action_rule_handler.py b/tests/unit/services/processors/test_action_rule_handler.py new file mode 100644 index 000000000..d0b1d7dce --- /dev/null +++ b/tests/unit/services/processors/test_action_rule_handler.py @@ -0,0 +1,622 @@ +from unittest.mock import Mock, call, patch + +import pytest +from hamcrest import assert_that, is_ +from pydantic import HttpUrl + +from eligibility_signposting_api.model.campaign_config import AvailableAction, RuleName, RulePriority, RuleType +from eligibility_signposting_api.model.eligibility_status import ( + ActionCode, + ActionDescription, + ActionType, + InternalActionCode, + Status, + SuggestedAction, + UrlLabel, + UrlLink, +) +from eligibility_signposting_api.model.person import Person +from eligibility_signposting_api.services.processors.action_rule_handler import ActionRuleHandler +from tests.fixtures.builders.model import rule as rule_builder +from tests.fixtures.builders.model.rule import ActionsMapperFactory + + +@pytest.fixture +def action_rule_handler(): + return ActionRuleHandler() + + +MOCK_PERSON = Person([{"ATTRIBUTE_TYPE": "PERSON", "AGE": "30"}]) + +BOOK_NBS_COMMS = AvailableAction( + ActionType="ButtonAuthLink", + ExternalRoutingCode="BookNBS", + ActionDescription="Action description", + UrlLink=HttpUrl("https://www.nhs.uk/book-rsv"), + UrlLabel="Continue to booking", +) + +DEFAULT_COMMS_DETAIL = AvailableAction( + ActionType="CareCardWithText", + ExternalRoutingCode="BookLocal", + ActionDescription="You can get an RSV vaccination at your GP surgery", +) + + +def test_get_action_rules_components_redirect_type(): + iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_redirect", + default_not_eligible_routing="default_not_eligible", + default_not_actionable_routing="default_not_actionable", + actions_mapper=ActionsMapperFactory.build(), + iteration_rules=[rule_builder.ICBRedirectRuleFactory.build(name="RedirectRule")], + ) + rules_found, mapper, default_comms = ActionRuleHandler.get_action_rules_components(iteration, RuleType.redirect) + assert_that(len(rules_found), is_(1)) + assert_that(rules_found[0].name, is_(RuleName("RedirectRule"))) + assert_that(mapper, is_(iteration.actions_mapper)) + assert_that(default_comms, is_("default_redirect")) + + +def test_get_action_rules_components_not_eligible_actions_type(): + iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_redirect", + default_not_eligible_routing="default_not_eligible", + default_not_actionable_routing="default_not_actionable", + actions_mapper=ActionsMapperFactory.build(), + iteration_rules=[rule_builder.ICBNonEligibleActionRuleFactory.build(name="NonEligibleRule")], + ) + rules_found, mapper, default_comms = ActionRuleHandler.get_action_rules_components( + iteration, RuleType.not_eligible_actions + ) + assert_that(len(rules_found), is_(1)) + assert_that(rules_found[0].name, is_(RuleName("NonEligibleRule"))) + assert_that(mapper, is_(iteration.actions_mapper)) + assert_that(default_comms, is_("default_not_eligible")) + + +def test_get_action_rules_components_no_matching_rules(): + iteration = rule_builder.IterationFactory.build( + iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build()] + ) + rules_found, _, _ = ActionRuleHandler.get_action_rules_components(iteration, RuleType.redirect) + assert_that(len(rules_found), is_(0)) + + +def test_get_actions_from_comms_single_comm(): + action_mapper = ActionsMapperFactory.build(root={"book_nbs": BOOK_NBS_COMMS}) + actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "book_nbs") + assert_that(len(actions), is_(1)) + assert_that(actions[0].internal_action_code, is_(InternalActionCode("book_nbs"))) + assert_that(actions[0].action_code, is_(ActionCode("BookNBS"))) + + +def test_get_actions_from_comms_multiple_comms(): + action_mapper = ActionsMapperFactory.build(root={"book_nbs": BOOK_NBS_COMMS, "default_comms": DEFAULT_COMMS_DETAIL}) + actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "book_nbs|default_comms") + assert_that(len(actions), is_(2)) + assert_that(actions[0].internal_action_code, is_(InternalActionCode("book_nbs"))) + assert_that(actions[1].internal_action_code, is_(InternalActionCode("default_comms"))) + + +def test_get_actions_from_comms_unknown_comm_code(): + action_mapper = ActionsMapperFactory.build(root={"book_nbs": BOOK_NBS_COMMS}) + actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "book_nbs|unknown_code") + assert_that(len(actions), is_(1)) + assert_that(actions[0].internal_action_code, is_(InternalActionCode("book_nbs"))) + + +def test_get_actions_from_comms_empty_string(): + action_mapper = ActionsMapperFactory.build(root={"book_nbs": BOOK_NBS_COMMS}) + actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "") + assert_that(len(actions), is_(0)) + + +def test_get_actions_from_comms_no_actions_found(): + action_mapper = ActionsMapperFactory.build(root={}) + actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "unknown_code") + assert_that(len(actions), is_(0)) + + +@patch("eligibility_signposting_api.services.calculators.rule_calculator.RuleCalculator") +@patch.object(ActionRuleHandler, "get_actions_from_comms") +@patch.object(ActionRuleHandler, "get_action_rules_components") +def test_handle_actions_no_matching_rules_returns_default( + mock_get_action_rules_components, + mock_get_actions_from_comms, + mock_rule_calculator_class, + action_rule_handler: ActionRuleHandler, +): + active_iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_action_code", + actions_mapper=ActionsMapperFactory.build(root={"default_action_code": DEFAULT_COMMS_DETAIL}), + iteration_rules=[], + ) + + mock_get_action_rules_components.return_value = ( + [], + active_iteration.actions_mapper, + active_iteration.default_comms_routing, + ) + + mock_get_actions_from_comms.side_effect = [ + [ + SuggestedAction( + internal_action_code=InternalActionCode("default_action_code"), + action_type=ActionType(DEFAULT_COMMS_DETAIL.action_type), + action_code=ActionCode(DEFAULT_COMMS_DETAIL.action_code), + action_description=ActionDescription(DEFAULT_COMMS_DETAIL.action_description), + url_link=DEFAULT_COMMS_DETAIL.url_link, + url_label=DEFAULT_COMMS_DETAIL.url_label, + ) + ], + [], + ] + + matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, RuleType.redirect) + + assert_that(len(matched_action_detail.actions), is_(1)) + assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("default_action_code"))) + assert_that(matched_action_detail.rule_priority, is_(None)) + assert_that(matched_action_detail.rule_name, is_(None)) + mock_get_action_rules_components.assert_called_once_with(active_iteration, RuleType.redirect) + mock_get_actions_from_comms.assert_called_once_with(active_iteration.actions_mapper, "default_action_code") + mock_rule_calculator_class.assert_not_called() + + +@patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") +@patch.object(ActionRuleHandler, "get_actions_from_comms") +@patch.object(ActionRuleHandler, "get_action_rules_components") +def test_handle_actions_matching_rule_overrides_default( + mock_get_action_rules_components, + mock_get_actions_from_comms, + mock_rule_calculator_class, + action_rule_handler: ActionRuleHandler, +): + matching_rule = rule_builder.ICBRedirectRuleFactory.build( + priority=10, comms_routing="rule_specific_action", name="RuleSpecificAction" + ) + active_iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_action_code", + actions_mapper=ActionsMapperFactory.build( + root={"default_action_code": DEFAULT_COMMS_DETAIL, "rule_specific_action": BOOK_NBS_COMMS} + ), + iteration_rules=[matching_rule], + ) + mock_get_action_rules_components.return_value = ( + (matching_rule,), + active_iteration.actions_mapper, + active_iteration.default_comms_routing, + ) + + mock_get_actions_from_comms.side_effect = [ + [ + SuggestedAction( + internal_action_code=InternalActionCode("default_action_code"), + action_type=ActionType(DEFAULT_COMMS_DETAIL.action_type), + action_code=ActionCode(DEFAULT_COMMS_DETAIL.action_code), + action_description=ActionDescription(DEFAULT_COMMS_DETAIL.action_description), + url_link=DEFAULT_COMMS_DETAIL.url_link, + url_label=DEFAULT_COMMS_DETAIL.url_label, + ) + ], + [ + SuggestedAction( + internal_action_code=InternalActionCode("rule_specific_action"), + action_type=ActionType(BOOK_NBS_COMMS.action_type), + action_code=ActionCode(BOOK_NBS_COMMS.action_code), + action_description=ActionDescription(BOOK_NBS_COMMS.action_description), + url_link=BOOK_NBS_COMMS.url_link, + url_label=BOOK_NBS_COMMS.url_label, + ) + ], + ] + + mock_rule_instance = Mock() + mock_rule_instance.evaluate_exclusion.return_value = (Status.actionable, Mock(matcher_matched=True)) + mock_rule_calculator_class.return_value = mock_rule_instance + + matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, RuleType.redirect) + + assert_that(len(matched_action_detail.actions), is_(1)) + assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("rule_specific_action"))) + assert_that(matched_action_detail.rule_priority, is_(RulePriority(10))) + assert_that(matched_action_detail.rule_name, is_(RuleName("RuleSpecificAction"))) + + mock_get_action_rules_components.assert_called_once_with(active_iteration, RuleType.redirect) + assert_that(mock_get_actions_from_comms.call_count, is_(2)) + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "default_action_code") + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "rule_specific_action") + mock_rule_calculator_class.assert_called_once_with(person=MOCK_PERSON, rule=matching_rule) + + +@patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") +@patch.object(ActionRuleHandler, "get_actions_from_comms") +@patch.object(ActionRuleHandler, "get_action_rules_components") +def test_handle_rule_mismatch_returns_default( + mock_get_action_rules_components, + mock_get_actions_from_comms, + mock_rule_calculator_class, + action_rule_handler: ActionRuleHandler, +): + mismatching_rule = rule_builder.ICBRedirectRuleFactory.build( + priority=10, comms_routing="rule_specific_action", name="RuleSpecificAction" + ) + active_iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_action_code", + actions_mapper=ActionsMapperFactory.build( + root={"default_action_code": DEFAULT_COMMS_DETAIL, "rule_specific_action": BOOK_NBS_COMMS} + ), + iteration_rules=[mismatching_rule], + ) + rule_type = RuleType.redirect + + mock_get_action_rules_components.return_value = ( + (mismatching_rule,), + active_iteration.actions_mapper, + active_iteration.default_comms_routing, + ) + + mock_get_actions_from_comms.side_effect = [ + [ + SuggestedAction( + internal_action_code=InternalActionCode("default_action_code"), + action_type=ActionType(DEFAULT_COMMS_DETAIL.action_type), + action_code=ActionCode(DEFAULT_COMMS_DETAIL.action_code), + action_description=ActionDescription(DEFAULT_COMMS_DETAIL.action_description), + url_link=DEFAULT_COMMS_DETAIL.url_link, + url_label=DEFAULT_COMMS_DETAIL.url_label, + ) + ], + [ + SuggestedAction( + internal_action_code=InternalActionCode("rule_specific_action"), + action_type=ActionType(BOOK_NBS_COMMS.action_type), + action_code=ActionCode(BOOK_NBS_COMMS.action_code), + action_description=ActionDescription(BOOK_NBS_COMMS.action_description), + url_link=BOOK_NBS_COMMS.url_link, + url_label=BOOK_NBS_COMMS.url_label, + ) + ], + ] + + mock_rule_calculator_class.return_value.evaluate_exclusion.return_value = ( + Status.actionable, + Mock(matcher_matched=False), + ) + + matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + + assert_that(len(matched_action_detail.actions), is_(1)) + assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("default_action_code"))) + assert_that(matched_action_detail.rule_priority, is_(None)) + assert_that(matched_action_detail.rule_name, is_(None)) + + mock_get_action_rules_components.assert_called_once_with(active_iteration, rule_type) + assert_that(mock_get_actions_from_comms.call_count, is_(1)) + mock_get_actions_from_comms.assert_called_once_with(active_iteration.actions_mapper, "default_action_code") + mock_rule_calculator_class.assert_called_once_with(person=MOCK_PERSON, rule=mismatching_rule) + + +@patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") +@patch.object(ActionRuleHandler, "get_actions_from_comms") +@patch.object(ActionRuleHandler, "get_action_rules_components") +def test_handle_multiple_rules_same_priority_all_match( + mock_get_action_rules_components, + mock_get_actions_from_comms, + mock_rule_calculator_class, + action_rule_handler: ActionRuleHandler, +): + rule1 = rule_builder.ICBRedirectRuleFactory.build(priority=10, comms_routing="action_a", name="RuleA") + rule2 = rule_builder.ICBRedirectRuleFactory.build(priority=10, comms_routing="action_b", name="RuleB") + active_iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_action_code", + actions_mapper=ActionsMapperFactory.build( + root={ + "default_action_code": DEFAULT_COMMS_DETAIL, + "action_a": BOOK_NBS_COMMS, + "action_b": DEFAULT_COMMS_DETAIL, + } + ), + iteration_rules=[rule1, rule2], + ) + + mock_get_action_rules_components.return_value = ( + (rule1, rule2), + active_iteration.actions_mapper, + active_iteration.default_comms_routing, + ) + + mock_get_actions_from_comms.side_effect = [ + [ + SuggestedAction( + internal_action_code=InternalActionCode("default_action_code"), + action_type=ActionType(DEFAULT_COMMS_DETAIL.action_type), + action_code=ActionCode(DEFAULT_COMMS_DETAIL.action_code), + action_description=ActionDescription(DEFAULT_COMMS_DETAIL.action_description), + url_link=DEFAULT_COMMS_DETAIL.url_link, + url_label=DEFAULT_COMMS_DETAIL.url_label, + ) + ], + [ + SuggestedAction( + internal_action_code=InternalActionCode("action_a"), + action_type=ActionType(BOOK_NBS_COMMS.action_type), + action_code=ActionCode(BOOK_NBS_COMMS.action_code), + action_description=ActionDescription(BOOK_NBS_COMMS.action_description), + url_link=BOOK_NBS_COMMS.url_link, + url_label=BOOK_NBS_COMMS.url_label, + ) + ], + [ + SuggestedAction( + internal_action_code=InternalActionCode("action_b"), + action_type=ActionType(DEFAULT_COMMS_DETAIL.action_type), + action_code=ActionCode(DEFAULT_COMMS_DETAIL.action_code), + action_description=ActionDescription(DEFAULT_COMMS_DETAIL.action_description), + url_link=DEFAULT_COMMS_DETAIL.url_link, + url_label=DEFAULT_COMMS_DETAIL.url_label, + ) + ], + ] + + mock_rule_calculator_class.side_effect = [ + Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=True)))), + Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=True)))), + ] + + matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, RuleType.redirect) + + assert_that(len(matched_action_detail.actions), is_(1)) + assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("action_a"))) + assert_that(matched_action_detail.rule_priority, is_(RulePriority(10))) + assert_that(matched_action_detail.rule_name, is_(RuleName("RuleA"))) + + assert_that(mock_rule_calculator_class.call_count, is_(2)) + assert_that(mock_get_actions_from_comms.call_count, is_(2)) + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "default_action_code") + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "action_a") + + +@patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") +@patch.object(ActionRuleHandler, "get_actions_from_comms") +@patch.object(ActionRuleHandler, "get_action_rules_components") +def test_handle_multiple_rules_same_priority_one_mismatch( + mock_get_action_rules_components, + mock_get_actions_from_comms, + mock_rule_calculator_class, + action_rule_handler: ActionRuleHandler, +): + rule1 = rule_builder.ICBRedirectRuleFactory.build(priority=10, comms_routing="action_a", name="RuleA") + rule2 = rule_builder.ICBRedirectRuleFactory.build(priority=10, comms_routing="action_b", name="RuleB") + active_iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_action_code", + actions_mapper=ActionsMapperFactory.build( + root={ + "default_action_code": DEFAULT_COMMS_DETAIL, + "action_a": BOOK_NBS_COMMS, + "action_b": DEFAULT_COMMS_DETAIL, + } + ), + iteration_rules=[rule1, rule2], + ) + rule_type = RuleType.redirect + + mock_get_action_rules_components.return_value = ( + (rule1, rule2), + active_iteration.actions_mapper, + active_iteration.default_comms_routing, + ) + + mock_get_actions_from_comms.side_effect = [ + [ + SuggestedAction( + internal_action_code=InternalActionCode("default_action_code"), + action_type=ActionType("DefaultInfoText"), + action_code=ActionCode("DefaultHealthcareProInfo"), + action_description=ActionDescription("Default Speak to your healthcare professional."), + url_link=None, + url_label=None, + ) + ] + ] + + mock_rule_calculator_class.side_effect = [ + Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=True)))), + Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=False)))), + ] + + matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + + assert_that(len(matched_action_detail.actions), is_(1)) + assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("default_action_code"))) + assert_that(matched_action_detail.rule_priority, is_(None)) + assert_that(matched_action_detail.rule_name, is_(None)) + + mock_get_action_rules_components.assert_called_once_with(active_iteration, rule_type) + assert_that(mock_get_actions_from_comms.call_count, is_(1)) + mock_get_actions_from_comms.assert_called_once_with(active_iteration.actions_mapper, "default_action_code") + assert_that( + mock_rule_calculator_class.call_args_list, + is_([call(person=MOCK_PERSON, rule=rule1), call(person=MOCK_PERSON, rule=rule2)]), + ) + + +@patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") +@patch.object(ActionRuleHandler, "get_actions_from_comms") +@patch.object(ActionRuleHandler, "get_action_rules_components") +def test_handle_different_priority_rules_highest_priority_wins( + mock_get_action_rules_components, + mock_get_actions_from_comms, + mock_rule_calculator_class, + action_rule_handler: ActionRuleHandler, +): + lower_priority_rule = rule_builder.ICBRedirectRuleFactory.build( + priority=20, comms_routing="action_low", name="LowP" + ) + higher_priority_rule = rule_builder.ICBRedirectRuleFactory.build( + priority=10, comms_routing="action_high", name="HighP" + ) + active_iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_action_code", + actions_mapper=ActionsMapperFactory.build( + root={ + "default_action_code": DEFAULT_COMMS_DETAIL, + "action_low": DEFAULT_COMMS_DETAIL, + "action_high": BOOK_NBS_COMMS, + } + ), + iteration_rules=[lower_priority_rule, higher_priority_rule], + ) + rule_type = RuleType.redirect + + mock_get_action_rules_components.return_value = ( + (lower_priority_rule, higher_priority_rule), + active_iteration.actions_mapper, + active_iteration.default_comms_routing, + ) + + mock_get_actions_from_comms.side_effect = [ + [ + SuggestedAction( + internal_action_code=InternalActionCode("default_action_code"), + action_type=ActionType("DefaultInfoText"), + action_code=ActionCode("DefaultHealthcareProInfo"), + action_description=ActionDescription("Default Speak to your healthcare professional."), + url_link=None, + url_label=None, + ) + ], + [ + SuggestedAction( + internal_action_code=InternalActionCode("action_high"), + action_type=ActionType("ButtonAuthLink"), + action_code=ActionCode("BookNBS"), + action_description=ActionDescription("Action description"), + url_link=UrlLink(HttpUrl("https://www.nhs.uk/book-rsv")), + url_label=UrlLabel("Continue to booking"), + ) + ], + [ + SuggestedAction( + internal_action_code=InternalActionCode("action_low"), + action_type=ActionType("CareCardWithText"), + action_code=ActionCode("BookLocal"), + action_description=ActionDescription("You can get an RSV vaccination at your GP surgery"), + url_link=None, + url_label=None, + ) + ], + ] + + mock_rule_calculator_class.side_effect = [ + Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=True)))), + Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=True)))), + ] + + matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + + assert_that(len(matched_action_detail.actions), is_(1)) + assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("action_high"))) + assert_that(matched_action_detail.rule_priority, is_(RulePriority(10))) + assert_that(matched_action_detail.rule_name, is_(RuleName("HighP"))) + + assert_that(mock_rule_calculator_class.call_count, is_(1)) + mock_rule_calculator_class.assert_called_once_with(person=MOCK_PERSON, rule=higher_priority_rule) + assert_that(mock_get_actions_from_comms.call_count, is_(2)) + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "default_action_code") + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "action_high") + + +def test_handle_no_actions_mapper_entry_for_rule_comms_returns_default(action_rule_handler: ActionRuleHandler): + matching_rule = rule_builder.ICBRedirectRuleFactory.build( + priority=10, comms_routing="non_existent_action", name="RuleSpecificAction" + ) + active_iteration = rule_builder.IterationFactory.build( + default_comms_routing="default_action_code", + actions_mapper=ActionsMapperFactory.build(root={"default_action_code": DEFAULT_COMMS_DETAIL}), + iteration_rules=[matching_rule], + ) + rule_type = RuleType.redirect + + with ( + patch.object(ActionRuleHandler, "get_action_rules_components") as mock_get_action_rules_components, + patch.object(ActionRuleHandler, "get_actions_from_comms") as mock_get_actions_from_comms, + patch( + "eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator" + ) as mock_rule_calculator_class, + ): + mock_get_action_rules_components.return_value = ( + (matching_rule,), + active_iteration.actions_mapper, + active_iteration.default_comms_routing, + ) + mock_get_actions_from_comms.side_effect = [ + [ + SuggestedAction( + internal_action_code=InternalActionCode("default_action_code"), + action_type=ActionType("DefaultInfoText"), + action_code=ActionCode("DefaultHealthcareProInfo"), + action_description=ActionDescription("Default Speak to your healthcare professional."), + url_link=None, + url_label=None, + ) + ], + None, + ] + mock_rule_calculator_class.return_value.evaluate_exclusion.return_value = ( + Status.actionable, + Mock(matcher_matched=True), + ) + + matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + + assert_that(len(matched_action_detail.actions), is_(1)) + assert_that( + matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("default_action_code")) + ) + assert_that(matched_action_detail.rule_priority, is_(RulePriority(10))) + assert_that(matched_action_detail.rule_name, is_(RuleName("RuleSpecificAction"))) + + assert_that(mock_get_actions_from_comms.call_count, is_(2)) + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "default_action_code") + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "non_existent_action") + mock_rule_calculator_class.assert_called_once() + + +def test_handle_no_default_comms_and_no_matching_rule(action_rule_handler: ActionRuleHandler): + active_iteration = rule_builder.IterationFactory.build( + default_comms_routing="", + actions_mapper=ActionsMapperFactory.build(root={}), + iteration_rules=[rule_builder.ICBRedirectRuleFactory.build(comms_routing="some_action")], + ) + rule_type = RuleType.redirect + + with ( + patch.object(ActionRuleHandler, "get_action_rules_components") as mock_get_action_rules_components, + patch.object(ActionRuleHandler, "get_actions_from_comms") as mock_get_actions_from_comms, + patch( + "eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator" + ) as mock_rule_calculator_class, + ): + mock_get_action_rules_components.return_value = ( + (rule_builder.ICBRedirectRuleFactory.build(comms_routing="some_action"),), + active_iteration.actions_mapper, + None, + ) + mock_get_actions_from_comms.side_effect = [None, None] + mock_rule_calculator_class.return_value.evaluate_exclusion.return_value = ( + Status.actionable, + Mock(matcher_matched=True), + ) + + matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + + assert_that(matched_action_detail.actions, is_(None)) + assert_that(matched_action_detail.rule_priority, is_(RulePriority(20))) + assert_that(matched_action_detail.rule_name, is_(RuleName("In QE1"))) + + assert_that(mock_get_actions_from_comms.call_count, is_(2)) + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, None) + mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "some_action") + mock_rule_calculator_class.assert_called_once() From 332928118f271e1ebf5cae98b53b3ded37a3fcdb Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Wed, 30 Jul 2025 14:38:07 +0100 Subject: [PATCH 3/4] ELI-351: Renames and fixes tests --- .../processors/test_action_rule_handler.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/unit/services/processors/test_action_rule_handler.py b/tests/unit/services/processors/test_action_rule_handler.py index d0b1d7dce..b7fd3d3c9 100644 --- a/tests/unit/services/processors/test_action_rule_handler.py +++ b/tests/unit/services/processors/test_action_rule_handler.py @@ -233,13 +233,13 @@ def test_handle_actions_matching_rule_overrides_default( @patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") @patch.object(ActionRuleHandler, "get_actions_from_comms") @patch.object(ActionRuleHandler, "get_action_rules_components") -def test_handle_rule_mismatch_returns_default( +def test_handle_non_matching_rule_returns_default( mock_get_action_rules_components, mock_get_actions_from_comms, mock_rule_calculator_class, action_rule_handler: ActionRuleHandler, ): - mismatching_rule = rule_builder.ICBRedirectRuleFactory.build( + non_matching_rule = rule_builder.ICBRedirectRuleFactory.build( priority=10, comms_routing="rule_specific_action", name="RuleSpecificAction" ) active_iteration = rule_builder.IterationFactory.build( @@ -247,12 +247,12 @@ def test_handle_rule_mismatch_returns_default( actions_mapper=ActionsMapperFactory.build( root={"default_action_code": DEFAULT_COMMS_DETAIL, "rule_specific_action": BOOK_NBS_COMMS} ), - iteration_rules=[mismatching_rule], + iteration_rules=[non_matching_rule], ) rule_type = RuleType.redirect mock_get_action_rules_components.return_value = ( - (mismatching_rule,), + (non_matching_rule,), active_iteration.actions_mapper, active_iteration.default_comms_routing, ) @@ -295,7 +295,7 @@ def test_handle_rule_mismatch_returns_default( mock_get_action_rules_components.assert_called_once_with(active_iteration, rule_type) assert_that(mock_get_actions_from_comms.call_count, is_(1)) mock_get_actions_from_comms.assert_called_once_with(active_iteration.actions_mapper, "default_action_code") - mock_rule_calculator_class.assert_called_once_with(person=MOCK_PERSON, rule=mismatching_rule) + mock_rule_calculator_class.assert_called_once_with(person=MOCK_PERSON, rule=non_matching_rule) @patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") @@ -346,9 +346,7 @@ def test_handle_multiple_rules_same_priority_all_match( action_description=ActionDescription(BOOK_NBS_COMMS.action_description), url_link=BOOK_NBS_COMMS.url_link, url_label=BOOK_NBS_COMMS.url_label, - ) - ], - [ + ), SuggestedAction( internal_action_code=InternalActionCode("action_b"), action_type=ActionType(DEFAULT_COMMS_DETAIL.action_type), @@ -356,7 +354,7 @@ def test_handle_multiple_rules_same_priority_all_match( action_description=ActionDescription(DEFAULT_COMMS_DETAIL.action_description), url_link=DEFAULT_COMMS_DETAIL.url_link, url_label=DEFAULT_COMMS_DETAIL.url_label, - ) + ), ], ] @@ -367,8 +365,9 @@ def test_handle_multiple_rules_same_priority_all_match( matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, RuleType.redirect) - assert_that(len(matched_action_detail.actions), is_(1)) + assert_that(len(matched_action_detail.actions), is_(2)) assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("action_a"))) + assert_that(matched_action_detail.actions[1].internal_action_code, is_(InternalActionCode("action_b"))) assert_that(matched_action_detail.rule_priority, is_(RulePriority(10))) assert_that(matched_action_detail.rule_name, is_(RuleName("RuleA"))) From 5500011a30e333528d2bc81b5763d382702679b7 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Wed, 30 Jul 2025 15:37:48 +0100 Subject: [PATCH 4/4] ELI-351: Renames and fixes tests --- .../model/eligibility_status.py | 7 ++ .../calculators/eligibility_calculator.py | 38 ++---- .../processors/action_rule_handler.py | 31 +++-- tests/unit/model/test_status.py | 9 +- .../processors/test_action_rule_handler.py | 119 +++++++++++------- 5 files changed, 124 insertions(+), 80 deletions(-) diff --git a/src/eligibility_signposting_api/model/eligibility_status.py b/src/eligibility_signposting_api/model/eligibility_status.py index a35058ce5..4026d552a 100644 --- a/src/eligibility_signposting_api/model/eligibility_status.py +++ b/src/eligibility_signposting_api/model/eligibility_status.py @@ -81,6 +81,13 @@ def get_status_text(self, condition_name: ConditionName) -> StatusText: } return status_to_text_mapping.get(self, lambda: StatusText("Unknown status provided"))() + def get_action_rule_type(self) -> RuleType: + return { + self.not_eligible: RuleType.not_eligible_actions, + self.not_actionable: RuleType.not_actionable_actions, + self.actionable: RuleType.redirect, + }[self] + @dataclass class Reason: diff --git a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py index ccb1ea5fa..6ea60ec55 100644 --- a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py +++ b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py @@ -8,13 +8,6 @@ from eligibility_signposting_api.audit.audit_context import AuditContext from eligibility_signposting_api.model import eligibility_status -from eligibility_signposting_api.model.campaign_config import ( - CampaignConfig, - CohortLabel, - Iteration, - IterationName, - RuleType, -) from eligibility_signposting_api.model.eligibility_status import ( BestIterationResult, CohortGroupResult, @@ -22,7 +15,6 @@ ConditionName, EligibilityStatus, IterationResult, - MatchedActionDetail, Status, ) from eligibility_signposting_api.services.processors.action_rule_handler import ActionRuleHandler @@ -32,6 +24,11 @@ if TYPE_CHECKING: from collections.abc import Collection + from eligibility_signposting_api.model.campaign_config import ( + CampaignConfig, + CohortLabel, + IterationName, + ) from eligibility_signposting_api.model.person import Person @@ -85,7 +82,9 @@ def get_eligibility_status(self, include_actions: str, conditions: list[str], ca ) for condition_name, campaign_group in requested_grouped_campaigns: best_iteration_result = self.get_best_iteration_result(campaign_group) - matched_action_detail = self.get_actions( + + matched_action_detail = self.action_rule_handler.get_actions( + self.person, best_iteration_result.active_iteration, best_iteration_result.iteration_result, include_actions_flag=include_actions_flag, @@ -100,27 +99,6 @@ def get_eligibility_status(self, include_actions: str, conditions: list[str], ca final_result = self.build_condition_results(condition_results) return eligibility_status.EligibilityStatus(conditions=final_result) - def get_actions( - self, active_iteration: Iteration | None, best_iteration_result: IterationResult, *, include_actions_flag: bool - ) -> MatchedActionDetail: - status_to_rule_type = { - Status.actionable: RuleType.redirect, - Status.not_eligible: RuleType.not_eligible_actions, - Status.not_actionable: RuleType.not_actionable_actions, - } - - action_detail = MatchedActionDetail() - - if ( - best_iteration_result.status in status_to_rule_type - and active_iteration is not None - and include_actions_flag - ): - rule_type = status_to_rule_type[best_iteration_result.status] - action_detail = self.action_rule_handler.handle(self.person, active_iteration, rule_type) - - return action_detail - def get_best_iteration_result(self, campaign_group: list[CampaignConfig]) -> BestIterationResult: iteration_results = self.get_iteration_results(campaign_group) diff --git a/src/eligibility_signposting_api/services/processors/action_rule_handler.py b/src/eligibility_signposting_api/services/processors/action_rule_handler.py index bcfc76cbf..6ff38bd05 100644 --- a/src/eligibility_signposting_api/services/processors/action_rule_handler.py +++ b/src/eligibility_signposting_api/services/processors/action_rule_handler.py @@ -5,14 +5,15 @@ ActionsMapper, Iteration, IterationRule, - RuleType, ) from eligibility_signposting_api.model.eligibility_status import ( ActionCode, ActionDescription, ActionType, InternalActionCode, + IterationResult, MatchedActionDetail, + RuleType, SuggestedAction, UrlLabel, UrlLink, @@ -22,13 +23,29 @@ class ActionRuleHandler: - def handle(self, person: Person, best_active_iteration: Iteration, rule_type: RuleType) -> MatchedActionDetail: - action_rules, action_mapper, default_comms = self.get_action_rules_components(best_active_iteration, rule_type) + def get_actions( + self, + person: Person, + active_iteration: Iteration | None, + best_iteration_result: IterationResult, + *, + include_actions_flag: bool, + ) -> MatchedActionDetail: + action_detail = MatchedActionDetail() + + if active_iteration is not None and include_actions_flag: + rule_type = best_iteration_result.status.get_action_rule_type() + action_detail = self._handle(person, active_iteration, rule_type) + + return action_detail + + def _handle(self, person: Person, best_active_iteration: Iteration, rule_type: RuleType) -> MatchedActionDetail: + action_rules, action_mapper, default_comms = self._get_action_rules_components(best_active_iteration, rule_type) priority_getter = attrgetter("priority") sorted_rules_by_priority = sorted(action_rules, key=priority_getter) - actions: list[SuggestedAction] | None = self.get_actions_from_comms(action_mapper, default_comms) # pyright: ignore[reportArgumentType] + actions: list[SuggestedAction] | None = self._get_actions_from_comms(action_mapper, default_comms) # pyright: ignore[reportArgumentType] matched_action_rule_priority, matched_action_rule_name = None, None for _, rule_group in groupby(sorted_rules_by_priority, key=priority_getter): @@ -40,7 +57,7 @@ def handle(self, person: Person, best_active_iteration: Iteration, rule_type: Ru comms_routing = rule_group_list[0].comms_routing if comms_routing and all(matcher_matched_list): - rule_actions = self.get_actions_from_comms(action_mapper, comms_routing) + rule_actions = self._get_actions_from_comms(action_mapper, comms_routing) if rule_actions and len(rule_actions) > 0: actions = rule_actions matched_action_rule_priority = rule_group_list[0].priority @@ -50,7 +67,7 @@ def handle(self, person: Person, best_active_iteration: Iteration, rule_type: Ru return MatchedActionDetail(matched_action_rule_name, matched_action_rule_priority, actions) @staticmethod - def get_action_rules_components( + def _get_action_rules_components( active_iteration: Iteration, rule_type: RuleType ) -> tuple[tuple[IterationRule, ...], ActionsMapper, str | None]: action_rules = tuple(rule for rule in active_iteration.iteration_rules if rule.type in rule_type) @@ -66,7 +83,7 @@ def get_action_rules_components( return action_rules, action_mapper, default_comms @staticmethod - def get_actions_from_comms(action_mapper: ActionsMapper, comms: str) -> list[SuggestedAction] | None: + def _get_actions_from_comms(action_mapper: ActionsMapper, comms: str) -> list[SuggestedAction] | None: suggested_actions: list[SuggestedAction] = [] for comm in comms.split("|"): action = action_mapper.get(comm) diff --git a/tests/unit/model/test_status.py b/tests/unit/model/test_status.py index eff2b68f9..cc3c018d9 100644 --- a/tests/unit/model/test_status.py +++ b/tests/unit/model/test_status.py @@ -1,4 +1,4 @@ -from eligibility_signposting_api.model.eligibility_status import ConditionName, Status, StatusText +from eligibility_signposting_api.model.eligibility_status import ConditionName, RuleType, Status, StatusText class TestStatus: @@ -38,3 +38,10 @@ def test_get_status_text(self): assert Status.actionable.get_status_text(ConditionName("COVID")) == StatusText( "You should have the COVID vaccine" ) + + def test_get_action_rule_type(self): + assert Status.not_eligible.get_action_rule_type() == RuleType(RuleType.not_eligible_actions) + + assert Status.not_actionable.get_action_rule_type() == RuleType(RuleType.not_actionable_actions) + + assert Status.actionable.get_action_rule_type() == RuleType(RuleType.redirect) diff --git a/tests/unit/services/processors/test_action_rule_handler.py b/tests/unit/services/processors/test_action_rule_handler.py index b7fd3d3c9..bc8d04ca1 100644 --- a/tests/unit/services/processors/test_action_rule_handler.py +++ b/tests/unit/services/processors/test_action_rule_handler.py @@ -10,6 +10,8 @@ ActionDescription, ActionType, InternalActionCode, + IterationResult, + MatchedActionDetail, Status, SuggestedAction, UrlLabel, @@ -18,11 +20,13 @@ from eligibility_signposting_api.model.person import Person from eligibility_signposting_api.services.processors.action_rule_handler import ActionRuleHandler from tests.fixtures.builders.model import rule as rule_builder -from tests.fixtures.builders.model.rule import ActionsMapperFactory +from tests.fixtures.builders.model.rule import ActionsMapperFactory, IterationFactory + +# flake8: noqa: SLF001 @pytest.fixture -def action_rule_handler(): +def handler(): return ActionRuleHandler() @@ -51,7 +55,7 @@ def test_get_action_rules_components_redirect_type(): actions_mapper=ActionsMapperFactory.build(), iteration_rules=[rule_builder.ICBRedirectRuleFactory.build(name="RedirectRule")], ) - rules_found, mapper, default_comms = ActionRuleHandler.get_action_rules_components(iteration, RuleType.redirect) + rules_found, mapper, default_comms = ActionRuleHandler._get_action_rules_components(iteration, RuleType.redirect) assert_that(len(rules_found), is_(1)) assert_that(rules_found[0].name, is_(RuleName("RedirectRule"))) assert_that(mapper, is_(iteration.actions_mapper)) @@ -66,7 +70,7 @@ def test_get_action_rules_components_not_eligible_actions_type(): actions_mapper=ActionsMapperFactory.build(), iteration_rules=[rule_builder.ICBNonEligibleActionRuleFactory.build(name="NonEligibleRule")], ) - rules_found, mapper, default_comms = ActionRuleHandler.get_action_rules_components( + rules_found, mapper, default_comms = ActionRuleHandler._get_action_rules_components( iteration, RuleType.not_eligible_actions ) assert_that(len(rules_found), is_(1)) @@ -79,13 +83,13 @@ def test_get_action_rules_components_no_matching_rules(): iteration = rule_builder.IterationFactory.build( iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build()] ) - rules_found, _, _ = ActionRuleHandler.get_action_rules_components(iteration, RuleType.redirect) + rules_found, _, _ = ActionRuleHandler._get_action_rules_components(iteration, RuleType.redirect) assert_that(len(rules_found), is_(0)) def test_get_actions_from_comms_single_comm(): action_mapper = ActionsMapperFactory.build(root={"book_nbs": BOOK_NBS_COMMS}) - actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "book_nbs") + actions = ActionRuleHandler._get_actions_from_comms(action_mapper, "book_nbs") assert_that(len(actions), is_(1)) assert_that(actions[0].internal_action_code, is_(InternalActionCode("book_nbs"))) assert_that(actions[0].action_code, is_(ActionCode("BookNBS"))) @@ -93,7 +97,7 @@ def test_get_actions_from_comms_single_comm(): def test_get_actions_from_comms_multiple_comms(): action_mapper = ActionsMapperFactory.build(root={"book_nbs": BOOK_NBS_COMMS, "default_comms": DEFAULT_COMMS_DETAIL}) - actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "book_nbs|default_comms") + actions = ActionRuleHandler._get_actions_from_comms(action_mapper, "book_nbs|default_comms") assert_that(len(actions), is_(2)) assert_that(actions[0].internal_action_code, is_(InternalActionCode("book_nbs"))) assert_that(actions[1].internal_action_code, is_(InternalActionCode("default_comms"))) @@ -101,31 +105,31 @@ def test_get_actions_from_comms_multiple_comms(): def test_get_actions_from_comms_unknown_comm_code(): action_mapper = ActionsMapperFactory.build(root={"book_nbs": BOOK_NBS_COMMS}) - actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "book_nbs|unknown_code") + actions = ActionRuleHandler._get_actions_from_comms(action_mapper, "book_nbs|unknown_code") assert_that(len(actions), is_(1)) assert_that(actions[0].internal_action_code, is_(InternalActionCode("book_nbs"))) def test_get_actions_from_comms_empty_string(): action_mapper = ActionsMapperFactory.build(root={"book_nbs": BOOK_NBS_COMMS}) - actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "") + actions = ActionRuleHandler._get_actions_from_comms(action_mapper, "") assert_that(len(actions), is_(0)) def test_get_actions_from_comms_no_actions_found(): action_mapper = ActionsMapperFactory.build(root={}) - actions = ActionRuleHandler.get_actions_from_comms(action_mapper, "unknown_code") + actions = ActionRuleHandler._get_actions_from_comms(action_mapper, "unknown_code") assert_that(len(actions), is_(0)) @patch("eligibility_signposting_api.services.calculators.rule_calculator.RuleCalculator") -@patch.object(ActionRuleHandler, "get_actions_from_comms") -@patch.object(ActionRuleHandler, "get_action_rules_components") +@patch.object(ActionRuleHandler, "_get_actions_from_comms") +@patch.object(ActionRuleHandler, "_get_action_rules_components") def test_handle_actions_no_matching_rules_returns_default( mock_get_action_rules_components, mock_get_actions_from_comms, mock_rule_calculator_class, - action_rule_handler: ActionRuleHandler, + handler: ActionRuleHandler, ): active_iteration = rule_builder.IterationFactory.build( default_comms_routing="default_action_code", @@ -153,7 +157,7 @@ def test_handle_actions_no_matching_rules_returns_default( [], ] - matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, RuleType.redirect) + matched_action_detail = handler._handle(MOCK_PERSON, active_iteration, RuleType.redirect) assert_that(len(matched_action_detail.actions), is_(1)) assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("default_action_code"))) @@ -165,13 +169,13 @@ def test_handle_actions_no_matching_rules_returns_default( @patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") -@patch.object(ActionRuleHandler, "get_actions_from_comms") -@patch.object(ActionRuleHandler, "get_action_rules_components") +@patch.object(ActionRuleHandler, "_get_actions_from_comms") +@patch.object(ActionRuleHandler, "_get_action_rules_components") def test_handle_actions_matching_rule_overrides_default( mock_get_action_rules_components, mock_get_actions_from_comms, mock_rule_calculator_class, - action_rule_handler: ActionRuleHandler, + handler: ActionRuleHandler, ): matching_rule = rule_builder.ICBRedirectRuleFactory.build( priority=10, comms_routing="rule_specific_action", name="RuleSpecificAction" @@ -216,7 +220,7 @@ def test_handle_actions_matching_rule_overrides_default( mock_rule_instance.evaluate_exclusion.return_value = (Status.actionable, Mock(matcher_matched=True)) mock_rule_calculator_class.return_value = mock_rule_instance - matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, RuleType.redirect) + matched_action_detail = handler._handle(MOCK_PERSON, active_iteration, RuleType.redirect) assert_that(len(matched_action_detail.actions), is_(1)) assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("rule_specific_action"))) @@ -231,13 +235,13 @@ def test_handle_actions_matching_rule_overrides_default( @patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") -@patch.object(ActionRuleHandler, "get_actions_from_comms") -@patch.object(ActionRuleHandler, "get_action_rules_components") +@patch.object(ActionRuleHandler, "_get_actions_from_comms") +@patch.object(ActionRuleHandler, "_get_action_rules_components") def test_handle_non_matching_rule_returns_default( mock_get_action_rules_components, mock_get_actions_from_comms, mock_rule_calculator_class, - action_rule_handler: ActionRuleHandler, + handler: ActionRuleHandler, ): non_matching_rule = rule_builder.ICBRedirectRuleFactory.build( priority=10, comms_routing="rule_specific_action", name="RuleSpecificAction" @@ -285,7 +289,7 @@ def test_handle_non_matching_rule_returns_default( Mock(matcher_matched=False), ) - matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + matched_action_detail = handler._handle(MOCK_PERSON, active_iteration, rule_type) assert_that(len(matched_action_detail.actions), is_(1)) assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("default_action_code"))) @@ -299,13 +303,13 @@ def test_handle_non_matching_rule_returns_default( @patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") -@patch.object(ActionRuleHandler, "get_actions_from_comms") -@patch.object(ActionRuleHandler, "get_action_rules_components") +@patch.object(ActionRuleHandler, "_get_actions_from_comms") +@patch.object(ActionRuleHandler, "_get_action_rules_components") def test_handle_multiple_rules_same_priority_all_match( mock_get_action_rules_components, mock_get_actions_from_comms, mock_rule_calculator_class, - action_rule_handler: ActionRuleHandler, + handler: ActionRuleHandler, ): rule1 = rule_builder.ICBRedirectRuleFactory.build(priority=10, comms_routing="action_a", name="RuleA") rule2 = rule_builder.ICBRedirectRuleFactory.build(priority=10, comms_routing="action_b", name="RuleB") @@ -363,7 +367,7 @@ def test_handle_multiple_rules_same_priority_all_match( Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=True)))), ] - matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, RuleType.redirect) + matched_action_detail = handler._handle(MOCK_PERSON, active_iteration, RuleType.redirect) assert_that(len(matched_action_detail.actions), is_(2)) assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("action_a"))) @@ -378,13 +382,13 @@ def test_handle_multiple_rules_same_priority_all_match( @patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") -@patch.object(ActionRuleHandler, "get_actions_from_comms") -@patch.object(ActionRuleHandler, "get_action_rules_components") +@patch.object(ActionRuleHandler, "_get_actions_from_comms") +@patch.object(ActionRuleHandler, "_get_action_rules_components") def test_handle_multiple_rules_same_priority_one_mismatch( mock_get_action_rules_components, mock_get_actions_from_comms, mock_rule_calculator_class, - action_rule_handler: ActionRuleHandler, + handler: ActionRuleHandler, ): rule1 = rule_builder.ICBRedirectRuleFactory.build(priority=10, comms_routing="action_a", name="RuleA") rule2 = rule_builder.ICBRedirectRuleFactory.build(priority=10, comms_routing="action_b", name="RuleB") @@ -425,7 +429,7 @@ def test_handle_multiple_rules_same_priority_one_mismatch( Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=False)))), ] - matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + matched_action_detail = handler._handle(MOCK_PERSON, active_iteration, rule_type) assert_that(len(matched_action_detail.actions), is_(1)) assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("default_action_code"))) @@ -442,13 +446,13 @@ def test_handle_multiple_rules_same_priority_one_mismatch( @patch("eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator") -@patch.object(ActionRuleHandler, "get_actions_from_comms") -@patch.object(ActionRuleHandler, "get_action_rules_components") +@patch.object(ActionRuleHandler, "_get_actions_from_comms") +@patch.object(ActionRuleHandler, "_get_action_rules_components") def test_handle_different_priority_rules_highest_priority_wins( mock_get_action_rules_components, mock_get_actions_from_comms, mock_rule_calculator_class, - action_rule_handler: ActionRuleHandler, + handler: ActionRuleHandler, ): lower_priority_rule = rule_builder.ICBRedirectRuleFactory.build( priority=20, comms_routing="action_low", name="LowP" @@ -513,7 +517,7 @@ def test_handle_different_priority_rules_highest_priority_wins( Mock(evaluate_exclusion=Mock(return_value=(Status.actionable, Mock(matcher_matched=True)))), ] - matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + matched_action_detail = handler._handle(MOCK_PERSON, active_iteration, rule_type) assert_that(len(matched_action_detail.actions), is_(1)) assert_that(matched_action_detail.actions[0].internal_action_code, is_(InternalActionCode("action_high"))) @@ -527,7 +531,7 @@ def test_handle_different_priority_rules_highest_priority_wins( mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "action_high") -def test_handle_no_actions_mapper_entry_for_rule_comms_returns_default(action_rule_handler: ActionRuleHandler): +def test_handle_no_actions_mapper_entry_for_rule_comms_returns_default(handler: ActionRuleHandler): matching_rule = rule_builder.ICBRedirectRuleFactory.build( priority=10, comms_routing="non_existent_action", name="RuleSpecificAction" ) @@ -539,8 +543,8 @@ def test_handle_no_actions_mapper_entry_for_rule_comms_returns_default(action_ru rule_type = RuleType.redirect with ( - patch.object(ActionRuleHandler, "get_action_rules_components") as mock_get_action_rules_components, - patch.object(ActionRuleHandler, "get_actions_from_comms") as mock_get_actions_from_comms, + patch.object(ActionRuleHandler, "_get_action_rules_components") as mock_get_action_rules_components, + patch.object(ActionRuleHandler, "_get_actions_from_comms") as mock_get_actions_from_comms, patch( "eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator" ) as mock_rule_calculator_class, @@ -568,7 +572,7 @@ def test_handle_no_actions_mapper_entry_for_rule_comms_returns_default(action_ru Mock(matcher_matched=True), ) - matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + matched_action_detail = handler._handle(MOCK_PERSON, active_iteration, rule_type) assert_that(len(matched_action_detail.actions), is_(1)) assert_that( @@ -583,7 +587,7 @@ def test_handle_no_actions_mapper_entry_for_rule_comms_returns_default(action_ru mock_rule_calculator_class.assert_called_once() -def test_handle_no_default_comms_and_no_matching_rule(action_rule_handler: ActionRuleHandler): +def test_handle_no_default_comms_and_no_matching_rule(handler: ActionRuleHandler): active_iteration = rule_builder.IterationFactory.build( default_comms_routing="", actions_mapper=ActionsMapperFactory.build(root={}), @@ -592,8 +596,8 @@ def test_handle_no_default_comms_and_no_matching_rule(action_rule_handler: Actio rule_type = RuleType.redirect with ( - patch.object(ActionRuleHandler, "get_action_rules_components") as mock_get_action_rules_components, - patch.object(ActionRuleHandler, "get_actions_from_comms") as mock_get_actions_from_comms, + patch.object(ActionRuleHandler, "_get_action_rules_components") as mock_get_action_rules_components, + patch.object(ActionRuleHandler, "_get_actions_from_comms") as mock_get_actions_from_comms, patch( "eligibility_signposting_api.services.processors.action_rule_handler.RuleCalculator" ) as mock_rule_calculator_class, @@ -609,7 +613,7 @@ def test_handle_no_default_comms_and_no_matching_rule(action_rule_handler: Actio Mock(matcher_matched=True), ) - matched_action_detail = action_rule_handler.handle(MOCK_PERSON, active_iteration, rule_type) + matched_action_detail = handler._handle(MOCK_PERSON, active_iteration, rule_type) assert_that(matched_action_detail.actions, is_(None)) assert_that(matched_action_detail.rule_priority, is_(RulePriority(20))) @@ -619,3 +623,34 @@ def test_handle_no_default_comms_and_no_matching_rule(action_rule_handler: Actio mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, None) mock_get_actions_from_comms.assert_any_call(active_iteration.actions_mapper, "some_action") mock_rule_calculator_class.assert_called_once() + + +@patch.object(ActionRuleHandler, "_handle") +def test_handle_when_active_iteration_present_and_include_actions_is_true(mock_handle, handler: ActionRuleHandler): + mock_handle.side_effect = [MatchedActionDetail()] + + handler.get_actions( + MOCK_PERSON, IterationFactory.build(), IterationResult(Status.actionable, [], []), include_actions_flag=True + ) + + assert_that(mock_handle.call_count, is_(1)) + + +@patch.object(ActionRuleHandler, "_handle") +def test_handle_when_active_iteration_absent_and_include_actions_is_true(mock_handle, handler: ActionRuleHandler): + mock_handle.side_effect = [MatchedActionDetail()] + + handler.get_actions(MOCK_PERSON, None, IterationResult(Status.actionable, [], []), include_actions_flag=True) + + assert_that(mock_handle.call_count, is_(0)) + + +@patch.object(ActionRuleHandler, "_handle") +def test_handle_is_not_called_when_include_actions_is_false(mock_handle, handler: ActionRuleHandler): + mock_handle.side_effect = [MatchedActionDetail()] + + handler.get_actions( + MOCK_PERSON, IterationFactory.build(), IterationResult(Status.actionable, [], []), include_actions_flag=False + ) + + assert_that(mock_handle.call_count, is_(0))