diff --git a/src/eligibility_signposting_api/model/eligibility_status.py b/src/eligibility_signposting_api/model/eligibility_status.py index 4026d552a..9cc8809fc 100644 --- a/src/eligibility_signposting_api/model/eligibility_status.py +++ b/src/eligibility_signposting_api/model/eligibility_status.py @@ -113,6 +113,7 @@ class Condition: condition_name: ConditionName status: Status cohort_results: list[CohortGroupResult] + suitability_rules: list[Reason] status_text: StatusText actions: list[SuggestedAction] | None = None diff --git a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py index 985613fcb..66cba4664 100644 --- a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py +++ b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py @@ -3,6 +3,7 @@ import logging from collections import defaultdict from dataclasses import dataclass, field +from itertools import chain from typing import TYPE_CHECKING from wireup import service @@ -16,6 +17,7 @@ ConditionName, EligibilityStatus, IterationResult, + Reason, Status, ) from eligibility_signposting_api.services.processors.action_rule_handler import ActionRuleHandler @@ -100,8 +102,10 @@ def get_eligibility_status(self, include_actions: str, conditions: list[str], ca condition_results[condition_name] = best_iteration_result.iteration_result condition_results[condition_name].actions = matched_action_detail.actions - condition_result = self.build_condition_results(condition_results[condition_name], condition_name) - final_result.append(condition_result) + condition: Condition = self.build_condition( + iteration_result=condition_results[condition_name], condition_name=condition_name + ) + final_result.append(condition) AuditContext.append_audit_condition( condition_name, @@ -150,39 +154,61 @@ def get_iteration_results(self, campaign_group: list[CampaignConfig]) -> dict[It return iteration_results @staticmethod - def build_condition_results(iteration_result: IterationResult, condition_name: ConditionName) -> Condition: + def build_condition(iteration_result: IterationResult, condition_name: ConditionName) -> Condition: grouped_cohort_results = defaultdict(list) for cohort_result in iteration_result.cohort_results: if iteration_result.status == cohort_result.status: grouped_cohort_results[cohort_result.cohort_code].append(cohort_result) - deduplicated_cohort_results = [] - - for group_cohort_code, group in grouped_cohort_results.items(): - if group: - unique_rule_codes = set() - deduplicated_reasons = [] - for cohort in group: - for reason in cohort.reasons: - if reason.rule_name not in unique_rule_codes and reason.rule_description: - unique_rule_codes.add(reason.rule_name) - deduplicated_reasons.append(reason) - - non_empty_description = next((c.description for c in group if c.description), group[0].description) - cohort_group_result = CohortGroupResult( - cohort_code=group_cohort_code, - status=group[0].status, - reasons=deduplicated_reasons, - description=non_empty_description, - audit_rules=[], - ) - deduplicated_cohort_results.append(cohort_group_result) + deduplicated_cohort_results: list[CohortGroupResult] = EligibilityCalculator.deduplicate_cohort_results( + grouped_cohort_results + ) + + overall_deduplicated_reasons_for_condition = EligibilityCalculator.deduplicate_reasons( + deduplicated_cohort_results + ) return Condition( condition_name=condition_name, status=iteration_result.status, cohort_results=list(deduplicated_cohort_results), + suitability_rules=list(overall_deduplicated_reasons_for_condition), actions=iteration_result.actions, status_text=iteration_result.status.get_status_text(condition_name), ) + + @staticmethod + def deduplicate_cohort_results( + grouped_cohort_results: dict[str, list[CohortGroupResult]], + ) -> list[CohortGroupResult]: + results = [] + + for cohort_code, group_results in grouped_cohort_results.items(): + if not group_results: + continue + + deduped_reasons: list[Reason] = EligibilityCalculator.deduplicate_reasons(group_results) + + description = next((c.description for c in group_results if c.description), group_results[0].description) + + results.append( + CohortGroupResult( + cohort_code=cohort_code, + status=group_results[0].status, + reasons=list(deduped_reasons), + description=description, + audit_rules=[], + ) + ) + + return results + + @staticmethod + def deduplicate_reasons(group_results: list[CohortGroupResult]) -> list[Reason]: + all_reasons = chain.from_iterable(group_result.reasons for group_result in group_results) + deduped = {} + for reason in all_reasons: + key = (reason.rule_type, reason.rule_priority) + deduped.setdefault(key, reason) + return list(deduped.values()) diff --git a/src/eligibility_signposting_api/views/eligibility.py b/src/eligibility_signposting_api/views/eligibility.py index 7dee05144..383d73d13 100644 --- a/src/eligibility_signposting_api/views/eligibility.py +++ b/src/eligibility_signposting_api/views/eligibility.py @@ -158,22 +158,15 @@ def build_eligibility_cohorts(condition: Condition) -> list[eligibility_response def build_suitability_results(condition: Condition) -> list[eligibility_response.SuitabilityRule]: - """Make only one entry if there are duplicate rules""" if condition.status != Status.not_actionable: return [] - suitability_results = [] - - for cohort_result in condition.cohort_results: - if cohort_result.status == Status.not_actionable: - suitability_results.extend( - eligibility_response.SuitabilityRule( - ruleType=eligibility_response.RuleType(reason.rule_type.value), - ruleCode=eligibility_response.RuleCode(reason.rule_name), - ruleText=eligibility_response.RuleText(reason.rule_description), - ) - for reason in cohort_result.reasons - if reason.rule_description - ) - - return suitability_results + return [ + eligibility_response.SuitabilityRule( + ruleType=eligibility_response.RuleType(reason.rule_type.value), + ruleCode=eligibility_response.RuleCode(reason.rule_name), + ruleText=eligibility_response.RuleText(reason.rule_description), + ) + for reason in condition.suitability_rules + if reason.rule_description + ] diff --git a/tests/fixtures/builders/model/eligibility.py b/tests/fixtures/builders/model/eligibility.py index 894b61c47..7406bc38e 100644 --- a/tests/fixtures/builders/model/eligibility.py +++ b/tests/fixtures/builders/model/eligibility.py @@ -33,6 +33,7 @@ class CohortResultFactory(DataclassFactory[eligibility_status.CohortGroupResult] class ConditionFactory(DataclassFactory[eligibility_status.Condition]): actions = Use(SuggestedActionFactory.batch, size=2) cohort_results = Use(CohortResultFactory.batch, size=2) + suitability_rules = Use(ReasonFactory.batch, size=2) class EligibilityStatusFactory(DataclassFactory[eligibility_status.EligibilityStatus]): diff --git a/tests/unit/services/calculators/test_eligibility_calculator.py b/tests/unit/services/calculators/test_eligibility_calculator.py index 610b88437..37a13e8ff 100644 --- a/tests/unit/services/calculators/test_eligibility_calculator.py +++ b/tests/unit/services/calculators/test_eligibility_calculator.py @@ -26,6 +26,7 @@ ActionDescription, ActionType, CohortGroupResult, + Condition, ConditionName, DateOfBirth, InternalActionCode, @@ -40,6 +41,7 @@ ) from eligibility_signposting_api.services.calculators.eligibility_calculator import EligibilityCalculator from tests.fixtures.builders.model import rule as rule_builder +from tests.fixtures.builders.model.eligibility import ReasonFactory from tests.fixtures.builders.repos.person import person_rows_builder from tests.fixtures.matchers.eligibility import ( is_cohort_result, @@ -689,7 +691,7 @@ def test_build_condition_results_single_condition_single_cohort_actionable(self) ] iteration_result = IterationResult(Status.actionable, cohort_group_results, suggested_actions) - result = EligibilityCalculator.build_condition_results(iteration_result, ConditionName("RSV")) + result = EligibilityCalculator.build_condition(iteration_result, ConditionName("RSV")) assert_that(result.condition_name, is_(ConditionName("RSV"))) assert_that(result.status, is_(Status.actionable)) @@ -703,6 +705,7 @@ def test_build_condition_results_single_condition_single_cohort_actionable(self) assert_that(deduplicated_cohort.reasons, is_([])) assert_that(deduplicated_cohort.description, is_("Cohort A Description")) assert_that(deduplicated_cohort.audit_rules, is_([])) + assert_that(result.suitability_rules, is_([])) def test_build_condition_results_single_condition_single_cohort_not_eligible_with_reasons(self): cohort_group_results = [CohortGroupResult("COHORT_A", Status.not_eligible, [], "Cohort A Description", [])] @@ -718,7 +721,7 @@ def test_build_condition_results_single_condition_single_cohort_not_eligible_wit ] iteration_result = IterationResult(Status.not_eligible, cohort_group_results, suggested_actions) - result = EligibilityCalculator.build_condition_results(iteration_result, ConditionName("RSV")) + result = EligibilityCalculator.build_condition(iteration_result, ConditionName("RSV")) assert_that(result.condition_name, is_(ConditionName("RSV"))) assert_that(result.status, is_(Status.not_eligible)) @@ -732,17 +735,18 @@ def test_build_condition_results_single_condition_single_cohort_not_eligible_wit assert_that(deduplicated_cohort.reasons, is_([])) assert_that(deduplicated_cohort.description, is_("Cohort A Description")) assert_that(deduplicated_cohort.audit_rules, is_([])) + assert_that(result.suitability_rules, is_([])) def test_build_condition_results_single_condition_multiple_cohorts_same_cohort_code_same_status(self): reason_1 = Reason( - RuleType.filter, + RuleType.suppression, eligibility_status.RuleName("Filter Rule 1"), RulePriority("1"), RuleDescription("Filter Rule Description 2"), matcher_matched=True, ) reason_2 = Reason( - RuleType.filter, + RuleType.suppression, eligibility_status.RuleName("Filter Rule 2"), RulePriority("2"), RuleDescription("Filter Rule Description 2"), @@ -766,7 +770,7 @@ def test_build_condition_results_single_condition_multiple_cohorts_same_cohort_c ] iteration_result = IterationResult(Status.not_eligible, cohort_group_results, suggested_actions) - result = EligibilityCalculator.build_condition_results(iteration_result, ConditionName("RSV")) + result: Condition = EligibilityCalculator.build_condition(iteration_result, ConditionName("RSV")) assert_that(len(result.cohort_results), is_(1)) @@ -776,17 +780,18 @@ def test_build_condition_results_single_condition_multiple_cohorts_same_cohort_c assert_that(deduplicated_cohort.reasons, contains_inanyorder(reason_1, reason_2)) assert_that(deduplicated_cohort.description, is_("Cohort A Description 2")) assert_that(deduplicated_cohort.audit_rules, is_([])) + assert_that(result.suitability_rules, contains_inanyorder(reason_1, reason_2)) def test_build_condition_results_multiple_cohorts_different_cohort_code_same_status(self): reason_1 = Reason( - RuleType.filter, + RuleType.suppression, eligibility_status.RuleName("Filter Rule 1"), RulePriority("1"), RuleDescription("Filter Rule Description 2"), matcher_matched=True, ) reason_2 = Reason( - RuleType.filter, + RuleType.suppression, eligibility_status.RuleName("Filter Rule 2"), RulePriority("2"), RuleDescription("Filter Rule Description 2"), @@ -808,7 +813,7 @@ def test_build_condition_results_multiple_cohorts_different_cohort_code_same_sta ] iteration_result = IterationResult(Status.not_eligible, cohort_group_results, suggested_actions) - result = EligibilityCalculator.build_condition_results(iteration_result, ConditionName("RSV")) + result = EligibilityCalculator.build_condition(iteration_result, ConditionName("RSV")) assert_that(len(result.cohort_results), is_(2)) @@ -820,14 +825,14 @@ def test_build_condition_results_multiple_cohorts_different_cohort_code_same_sta def test_build_condition_results_cohorts_status_not_matching_iteration_status(self): reason_1 = Reason( - RuleType.filter, + RuleType.suppression, eligibility_status.RuleName("Filter Rule 1"), RulePriority("1"), RuleDescription("Matching"), matcher_matched=True, ) reason_2 = Reason( - RuleType.filter, + RuleType.suppression, eligibility_status.RuleName("Filter Rule 2"), RulePriority("2"), RuleDescription("Not matching"), @@ -840,8 +845,189 @@ def test_build_condition_results_cohorts_status_not_matching_iteration_status(se iteration_result = IterationResult(Status.not_eligible, cohort_group_results, []) - result = EligibilityCalculator.build_condition_results(iteration_result, ConditionName("RSV")) + result = EligibilityCalculator.build_condition(iteration_result, ConditionName("RSV")) assert_that(len(result.cohort_results), is_(1)) assert_that(result.cohort_results[0].cohort_code, is_("COHORT_X")) assert_that(result.cohort_results[0].status, is_(Status.not_eligible)) + + +@pytest.mark.parametrize( + ("reason_1", "reason_2", "reason_3", "expected_reasons"), + [ + # Same rule name, type, and priority, different description + ( + ReasonFactory.build(rule_description="description1", matcher_matched=True), + ReasonFactory.build(rule_description="description2", matcher_matched=True), + ReasonFactory.build(rule_description="description3", matcher_matched=True), + [ReasonFactory.build(rule_description="description1", matcher_matched=True)], + ), + # Different rule name, same type, same priority + ( + ReasonFactory.build(rule_name="Supress Rule 1", rule_description="description1", matcher_matched=True), + ReasonFactory.build(rule_name="Supress Rule 2", rule_description="description2", matcher_matched=True), + ReasonFactory.build(rule_name="Supress Rule 1", rule_description="description3", matcher_matched=True), + [ReasonFactory.build(rule_name="Supress Rule 1", rule_description="description1", matcher_matched=True)], + ), + # Same rule name, same type, different priority + ( + ReasonFactory.build(rule_priority="1", rule_description="description1", matcher_matched=True), + ReasonFactory.build(rule_priority="2", rule_description="description2", matcher_matched=True), + ReasonFactory.build(rule_priority="1", rule_description="description3", matcher_matched=True), + [ + ReasonFactory.build(rule_priority="1", rule_description="description1", matcher_matched=True), + ReasonFactory.build(rule_priority="2", rule_description="description2", matcher_matched=True), + ], + ), + # Same rule name, same priority, different type + ( + ReasonFactory.build(rule_type=RuleType.suppression, rule_description="description1", matcher_matched=True), + ReasonFactory.build(rule_type=RuleType.filter, rule_description="description2", matcher_matched=True), + ReasonFactory.build(rule_type=RuleType.suppression, rule_description="description3", matcher_matched=True), + [ + ReasonFactory.build( + rule_type=RuleType.suppression, rule_description="description1", matcher_matched=True + ), + ReasonFactory.build(rule_type=RuleType.filter, rule_description="description2", matcher_matched=True), + ], + ), + ], +) +def test_build_condition_results_grouping_reasons(reason_1, reason_2, reason_3, expected_reasons): + cohort_group_results = [ + CohortGroupResult( + "COHORT_X", + Status.not_actionable, + [reason_1, reason_3], + "Cohort X Description", + [], + ), + CohortGroupResult( + "COHORT_Y", + Status.not_actionable, + [reason_2, reason_3], + "Cohort Y Description", + [], + ), + ] + + iteration_result = IterationResult(Status.not_actionable, cohort_group_results, []) + + result: Condition = EligibilityCalculator.build_condition(iteration_result, ConditionName("RSV")) + + assert_that(result.suitability_rules, contains_inanyorder(*expected_reasons)) + + +@pytest.mark.parametrize( + ("reason_2", "expected_reasons"), + [ + # Same rule name, type, and priority, different description + ( + ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Matching", + rule_name="Supress Rule 1", + rule_priority="1", + matcher_matched=True, + ), + [ + ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Not matching", + rule_name="Supress Rule 1", + rule_priority="1", + matcher_matched=True, + ) + ], + ), + # Different rule name + ( + ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Matching", + rule_name="Supress Rule 2", + rule_priority="1", + matcher_matched=True, + ), + [ + ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Not matching", + rule_name="Supress Rule 1", + rule_priority="1", + matcher_matched=True, + ) + ], + ), + # Different priority + ( + ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Matching", + rule_name="Supress Rule 1", + rule_priority="2", + matcher_matched=True, + ), + [ + ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Not matching", + rule_name="Supress Rule 1", + rule_priority="1", + matcher_matched=True, + ), + ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Matching", + rule_name="Supress Rule 1", + rule_priority="2", + matcher_matched=True, + ), + ], + ), + # Different type + ( + ReasonFactory.build( + rule_type=RuleType.filter, + rule_description="Matching", + rule_name="Supress Rule 1", + rule_priority="2", + matcher_matched=True, + ), + [ + ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Not matching", + rule_name="Supress Rule 1", + rule_priority="1", + matcher_matched=True, + ), + ReasonFactory.build( + rule_type=RuleType.filter, + rule_description="Matching", + rule_name="Supress Rule 1", + rule_priority="2", + matcher_matched=True, + ), + ], + ), + ], +) +def test_build_condition_results_single_cohort(reason_2, expected_reasons): + reason_1 = ReasonFactory.build( + rule_type=RuleType.suppression, + rule_description="Not matching", + rule_name="Supress Rule 1", + rule_priority="1", + matcher_matched=True, + ) + + cohort_group_results = [ + CohortGroupResult("COHORT_Y", Status.not_actionable, [reason_1, reason_2], "Cohort Y Description", []) + ] + + iteration_result = IterationResult(Status.not_actionable, cohort_group_results, []) + result = EligibilityCalculator.build_condition(iteration_result, ConditionName("RSV")) + + assert_that(len(result.cohort_results), is_(1)) + assert_that(result.cohort_results[0].reasons, contains_inanyorder(*expected_reasons))