From 829ba9ea73d9f4098dd52c2ef5716f726328f0c6 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Thu, 28 Aug 2025 17:07:14 +0100 Subject: [PATCH 1/2] ELI-405: Regardless of final status audit all types of cohort status rules --- .../audit/audit_context.py | 16 ++- .../calculators/eligibility_calculator.py | 1 - tests/unit/audit/test_audit_context.py | 12 +- .../test_eligibility_calculator.py | 110 ++++++++++++++++++ 4 files changed, 120 insertions(+), 19 deletions(-) diff --git a/src/eligibility_signposting_api/audit/audit_context.py b/src/eligibility_signposting_api/audit/audit_context.py index a72a6c6a4..3f6f0817c 100644 --- a/src/eligibility_signposting_api/audit/audit_context.py +++ b/src/eligibility_signposting_api/audit/audit_context.py @@ -20,7 +20,6 @@ from eligibility_signposting_api.audit.audit_service import AuditService from eligibility_signposting_api.model.eligibility_status import ( BestIterationResult, - CohortGroupResult, ConditionName, IterationResult, MatchedActionDetail, @@ -64,12 +63,12 @@ def append_audit_condition( condition_name: ConditionName, best_iteration_result: BestIterationResult, action_detail: MatchedActionDetail, - cohort_results: list[CohortGroupResult], ) -> None: audit_eligibility_cohorts, audit_eligibility_cohort_groups, audit_actions = [], [], [] best_active_iteration = best_iteration_result.active_iteration best_candidate = best_iteration_result.iteration_result best_cohort_results = best_iteration_result.cohort_results + filter_audit_rules, suitability_audit_rules = [], [] if best_cohort_results: for cohort_label, result in sorted(best_cohort_results.items(), key=lambda item: item[1].cohort_code): @@ -84,13 +83,12 @@ def append_audit_condition( ) ) - filter_audit_rules, suitability_audit_rules = [], [] - for result in cohort_results: - for rule in result.audit_rules: - if rule.rule_type == RuleType.filter: - filter_audit_rules.append(rule) - if rule.rule_type == RuleType.suppression: - suitability_audit_rules.append(rule) + for result in best_cohort_results.values(): + for rule in result.audit_rules: + if rule.rule_type == RuleType.filter: + filter_audit_rules.append(rule) + if rule.rule_type == RuleType.suppression: + suitability_audit_rules.append(rule) audit_filter_rule = AuditContext.create_audit_filter_rule(filter_audit_rules) audit_suitability_rule = AuditContext.create_audit_suitability_rule(suitability_audit_rules) diff --git a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py index ab3122953..7ff8c18f8 100644 --- a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py +++ b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py @@ -117,7 +117,6 @@ def get_eligibility_status(self, include_actions: str, conditions: list[str], ca condition_name, best_iteration_result, matched_action_detail, - condition_results[condition_name].cohort_results, ) # Consolidate all the results and return diff --git a/tests/unit/audit/test_audit_context.py b/tests/unit/audit/test_audit_context.py index e08143295..fcd998d9f 100644 --- a/tests/unit/audit/test_audit_context.py +++ b/tests/unit/audit/test_audit_context.py @@ -150,9 +150,7 @@ def test_append_audit_condition_adds_condition_to_audit_log_on_g_for_actionable_ with app.app_context(): g.audit_log = AuditEvent() - AuditContext.append_audit_condition( - condition_name, best_iteration_results, matched_action_detail, [cohort_group_result] - ) + AuditContext.append_audit_condition(condition_name, best_iteration_results, matched_action_detail) expected_audit_action = [ AuditAction( @@ -226,9 +224,7 @@ def test_should_append_audit_suppression_rules_for_actionable_status(app): with app.app_context(): g.audit_log = AuditEvent() - AuditContext.append_audit_condition( - condition_name, best_iteration_results, MatchedActionDetail(), [cohort_group_result] - ) + AuditContext.append_audit_condition(condition_name, best_iteration_results, MatchedActionDetail()) assert g.audit_log.response.condition, condition_name cond = g.audit_log.response.condition[0] @@ -283,9 +279,7 @@ def test_should_append_audit_filter_rules_for_not_actionable_status(app): with app.app_context(): g.audit_log = AuditEvent() - AuditContext.append_audit_condition( - condition_name, best_iteration_results, MatchedActionDetail(), [cohort_group_result] - ) + AuditContext.append_audit_condition(condition_name, best_iteration_results, MatchedActionDetail()) assert g.audit_log.response.condition, condition_name cond = g.audit_log.response.condition[0] diff --git a/tests/unit/services/calculators/test_eligibility_calculator.py b/tests/unit/services/calculators/test_eligibility_calculator.py index 74c6fb1d5..2bbd0e560 100644 --- a/tests/unit/services/calculators/test_eligibility_calculator.py +++ b/tests/unit/services/calculators/test_eligibility_calculator.py @@ -756,6 +756,116 @@ def test_eligibility_status_replaces_tokens_with_attribute_data(faker: Faker): assert audit_condition.actions[0].action_url_label == "Your GP practice code is ." +def test_regardless_of_final_status_audit_all_types_of_cohort_status_rules(faker: Faker): + # Given + nhs_number = NHSNumber(faker.nhs_number()) + date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=85, maximum_age=85)) + + person_rows = person_rows_builder( + nhs_number, + date_of_birth=date_of_birth, + cohorts=[ + "rsv_eli_376_cohort_1", + "rsv_eli_376_cohort_2", + "rsv_eli_376_cohort_3", + "rsv_eli_376_cohort_4", + "rsv_eli_376_cohort_5", + ], + icb="ABC", + ) + + available_action = AvailableAction( + ActionType="ButtonAuthLink", + ExternalRoutingCode="BookNBS", + ActionDescription="## Get vaccinated at your GP surgery in [[PERSON.ICB]].", + UrlLink=HttpUrl("https://www.nhs.uk/book-rsv"), + UrlLabel="Your GP practice code is [[PERSON.GP_PRACTICE]].", + ) + + campaign_configs = [ + rule_builder.CampaignConfigFactory.build( + target="RSV", + iterations=[ + rule_builder.IterationFactory.build( + default_comms_routing="TOKEN_TEST", + default_not_actionable_routing="TOKEN_TEST", + default_not_eligible_routing="TOKEN_TEST", + iteration_cohorts=[ + rule_builder.IterationCohortFactory.build( + cohort_label="rsv_eli_376_cohort_1", cohort_group="rsv_eli_376_cohort_group", priority=0 + ), + rule_builder.IterationCohortFactory.build( + cohort_label="rsv_eli_376_cohort_2", cohort_group="rsv_eli_376_cohort_group", priority=1 + ), + rule_builder.IterationCohortFactory.build( + cohort_label="rsv_eli_376_cohort_3", cohort_group="rsv_eli_376_cohort_group", priority=2 + ), + rule_builder.IterationCohortFactory.build( + cohort_label="rsv_eli_376_cohort_4", + cohort_group="rsv_eli_376_cohort_group_other", + priority=3, + ), + rule_builder.IterationCohortFactory.build( + cohort_label="rsv_eli_376_cohort_5", + cohort_group="rsv_eli_376_cohort_group_another", + priority=4, + ), + ], + iteration_rules=[ + rule_builder.PersonAgeSuppressionRuleFactory.build( + type=RuleType.filter, + name=RuleName("NotEligible Reason 1"), + description=RuleDescription("NotEligible Description 1"), + priority=RulePriority("100"), + operator=RuleOperator.year_lte, + attribute_level=RuleAttributeLevel.PERSON, + attribute_name=RuleAttributeName("DATE_OF_BIRTH"), + comparator=RuleComparator("-80"), + cohort_label=CohortLabel("rsv_eli_376_cohort_1"), + ), + rule_builder.ICBRedirectRuleFactory.build( + operator=RuleOperator.ne, comparator=RuleComparator("ABC") + ), + rule_builder.PersonAgeSuppressionRuleFactory.build( + type=RuleType.suppression, + name=RuleName("NotActionable Reason 1"), + description=RuleDescription("NotActionable Description 1"), + priority=RulePriority("110"), + operator=RuleOperator.year_lte, + attribute_level=RuleAttributeLevel.PERSON, + attribute_name=RuleAttributeName("DATE_OF_BIRTH"), + comparator=RuleComparator("-80"), + cohort_label=CohortLabel("rsv_eli_376_cohort_5"), + ), + ], + actions_mapper=rule_builder.ActionsMapperFactory.build(root={"TOKEN_TEST": available_action}), + ) + ], + ) + ] + + calculator = EligibilityCalculator(person_rows, campaign_configs) + + # When + actual = calculator.get_eligibility_status("Y", ["ALL"], "ALL") + + # Then + assert_that( + actual, + is_eligibility_status().with_conditions( + has_item(is_condition().with_condition_name(ConditionName("RSV")).and_status(Status.actionable)) + ), + ) + + assert len(g.audit_log.response.condition[0].filter_rules) == 1 + assert g.audit_log.response.condition[0].filter_rules[0].rule_name == "NotEligible Reason 1" + assert g.audit_log.response.condition[0].filter_rules[0].rule_priority == "100" + assert len(g.audit_log.response.condition[0].suitability_rules) == 1 + assert g.audit_log.response.condition[0].suitability_rules[0].rule_name == "NotActionable Reason 1" + assert g.audit_log.response.condition[0].suitability_rules[0].rule_message == "NotActionable Description 1" + assert g.audit_log.response.condition[0].suitability_rules[0].rule_priority == "110" + + def test_eligibility_status_with_invalid_tokens_raises_attribute_error(faker: Faker): # Given nhs_number = NHSNumber(faker.nhs_number()) From 6305fe45ba9aec5801da76569a6a2b76a223538e Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Thu, 28 Aug 2025 17:49:31 +0100 Subject: [PATCH 2/2] ELI-405: Fixes Cognitive Complexity --- .../audit/audit_context.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/eligibility_signposting_api/audit/audit_context.py b/src/eligibility_signposting_api/audit/audit_context.py index 3f6f0817c..8cb56e5aa 100644 --- a/src/eligibility_signposting_api/audit/audit_context.py +++ b/src/eligibility_signposting_api/audit/audit_context.py @@ -20,6 +20,7 @@ from eligibility_signposting_api.audit.audit_service import AuditService from eligibility_signposting_api.model.eligibility_status import ( BestIterationResult, + CohortGroupResult, ConditionName, IterationResult, MatchedActionDetail, @@ -83,12 +84,7 @@ def append_audit_condition( ) ) - for result in best_cohort_results.values(): - for rule in result.audit_rules: - if rule.rule_type == RuleType.filter: - filter_audit_rules.append(rule) - if rule.rule_type == RuleType.suppression: - suitability_audit_rules.append(rule) + AuditContext.get_audit_rules(filter_audit_rules, suitability_audit_rules, result) audit_filter_rule = AuditContext.create_audit_filter_rule(filter_audit_rules) audit_suitability_rule = AuditContext.create_audit_suitability_rule(suitability_audit_rules) @@ -114,6 +110,14 @@ def append_audit_condition( g.audit_log.response.condition.append(audit_condition) + @staticmethod + def get_audit_rules(filter_audit_rules: list, suitability_audit_rules: list, result: CohortGroupResult) -> None: + for rule in result.audit_rules: + if rule.rule_type == RuleType.filter: + filter_audit_rules.append(rule) + if rule.rule_type == RuleType.suppression: + suitability_audit_rules.append(rule) + @staticmethod def add_rule_name_and_priority_to_audit( best_candidate: IterationResult | None,