From 47bbdc7c3a8118095d5e8d1ed026573f012fef41 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Tue, 12 Aug 2025 14:51:57 +0100 Subject: [PATCH 1/3] ELI-405: Update audit rules for filter and suppression regardless of status --- .../audit/audit_context.py | 32 ++-- tests/unit/audit/test_audit_context.py | 150 +++++++++++++++++- 2 files changed, 154 insertions(+), 28 deletions(-) diff --git a/src/eligibility_signposting_api/audit/audit_context.py b/src/eligibility_signposting_api/audit/audit_context.py index 58bb49578..a72a6c6a4 100644 --- a/src/eligibility_signposting_api/audit/audit_context.py +++ b/src/eligibility_signposting_api/audit/audit_context.py @@ -25,7 +25,7 @@ IterationResult, MatchedActionDetail, Reason, - Status, + RuleType, SuggestedAction, ) @@ -86,14 +86,14 @@ def append_audit_condition( filter_audit_rules, suitability_audit_rules = [], [] for result in cohort_results: - if result.status.name == Status.not_eligible.name: - filter_audit_rules.extend(result.audit_rules) - if result.status.name == Status.not_actionable.name: - suitability_audit_rules.extend(result.audit_rules) + 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) - audit_action_rule = AuditContext.add_rule_name_and_priority_to_audit(best_candidate, action_detail) audit_actions = AuditContext.create_audit_actions(action_detail.actions) @@ -186,20 +186,8 @@ def create_audit_filter_rule(reasons: list[Reason]) -> list[AuditFilterRule] | N @staticmethod def deduplicate_reasons(reasons: list[Reason]) -> list[Reason]: - unique_rule_codes = set() - deduplicated_reasons = [] - + deduped = {} for reason in 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( - reason.rule_type, - reason.rule_name, - reason.rule_priority, - reason.rule_description, - reason.matcher_matched, - ) - ) - - return deduplicated_reasons + key = (reason.rule_type, reason.rule_priority) + deduped.setdefault(key, reason) + return list(deduped.values()) diff --git a/tests/unit/audit/test_audit_context.py b/tests/unit/audit/test_audit_context.py index 05d78bd8e..350e66f9c 100644 --- a/tests/unit/audit/test_audit_context.py +++ b/tests/unit/audit/test_audit_context.py @@ -9,7 +9,8 @@ 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, RuleType +from eligibility_signposting_api.model import campaign_config +from eligibility_signposting_api.model.campaign_config import CampaignID, CampaignVersion, CohortLabel, RuleType from eligibility_signposting_api.model.eligibility_status import ( ActionCode, ActionDescription, @@ -82,7 +83,7 @@ def test_add_request_details_when_headers_are_empty_sets_audit_log_on_g(app): assert isinstance(audit_req.request_timestamp, datetime) -def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): +def test_append_audit_condition_adds_condition_to_audit_log_on_g_for_actionable_status(app): suggested_actions: list[SuggestedAction] | None condition_name: ConditionName campaign_details: tuple[CampaignID | None, CampaignVersion | None] @@ -101,13 +102,27 @@ def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): condition_name = ConditionName("Condition1") iteration = IterationFactory.build(version=12345) audit_rules = [ + Reason( + rule_type=RuleType.redirect, + rule_name=RuleName("RedirectRuleName1"), + rule_description=RuleDescription("RedirectRuleDescription1"), + matcher_matched=True, + rule_priority=RulePriority("1"), + ), Reason( rule_type=RuleType.filter, rule_name=RuleName("FilterRuleName1"), rule_description=RuleDescription("FilterRuleDescription1"), matcher_matched=True, rule_priority=RulePriority("1"), - ) + ), + Reason( + rule_type=RuleType.suppression, + rule_name=RuleName("SuppressionRuleName1"), + rule_description=RuleDescription("SuppressionRuleDescription1"), + matcher_matched=True, + rule_priority=RulePriority("1"), + ), ] cohort_group_result = CohortGroupResult( status=Status.actionable, @@ -120,10 +135,16 @@ def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): status=Status.actionable, cohort_results=[cohort_group_result], actions=suggested_actions ) campaign_details = (CampaignID("CampaignID1"), CampaignVersion(123)) - matched_action_detail = MatchedActionDetail(RuleName("RedirectRuleName1"), RulePriority("1"), suggested_actions) + matched_action_detail = MatchedActionDetail( + campaign_config.RuleName("RedirectRuleName1"), campaign_config.RulePriority(1), suggested_actions + ) best_iteration_results = BestIterationResult( - iteration_result, iteration, campaign_details[0], campaign_details[1], {"CohortCode1": cohort_group_result} + iteration_result, + iteration, + campaign_details[0], + campaign_details[1], + {CohortLabel("CohortCode1"): cohort_group_result}, ) with app.app_context(): @@ -156,7 +177,68 @@ def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): assert cond.actions == expected_audit_action assert cond.action_rule.rule_priority == "1" assert cond.action_rule.rule_name == "RedirectRuleName1" - assert cond.suitability_rules is None + assert len(cond.suitability_rules) == 1 + assert cond.suitability_rules[0].rule_priority == "1" + assert cond.suitability_rules[0].rule_name == "SuppressionRuleName1" + assert cond.suitability_rules[0].rule_message == "SuppressionRuleDescription1" + assert cond.filter_rules[0].rule_priority == "1" + assert cond.filter_rules[0].rule_name == "FilterRuleName1" + assert cond.eligibility_cohorts[0].cohort_code == "CohortCode1" + assert cond.eligibility_cohorts[0].cohort_status == "actionable" + assert cond.eligibility_cohort_groups[0].cohort_code == "CohortCode1" + assert cond.eligibility_cohort_groups[0].cohort_status == "actionable" + assert cond.eligibility_cohort_groups[0].cohort_text == "CohortDescription1" + + +def test_should_append_audit_suppression_rules_for_actionable_status(app): + condition_name: ConditionName + campaign_details: tuple[CampaignID | None, CampaignVersion | None] + + condition_name = ConditionName("Condition1") + iteration = IterationFactory.build() + audit_rules = [ + Reason( + rule_type=RuleType.suppression, + rule_name=RuleName("SuppressionRuleName1"), + rule_description=RuleDescription("SuppressionRuleDescription1"), + matcher_matched=True, + rule_priority=RulePriority("1"), + ) + ] + cohort_group_result = CohortGroupResult( + status=Status.actionable, + cohort_code="CohortCode1", + description="CohortDescription1", + audit_rules=audit_rules, + reasons=audit_rules, + ) + iteration_result = IterationResult(status=Status.actionable, cohort_results=[cohort_group_result], actions=[]) + campaign_details = (CampaignID("CampaignID1"), CampaignVersion(123)) + + best_iteration_results = BestIterationResult( + iteration_result, + iteration, + campaign_details[0], + campaign_details[1], + {CohortLabel("CohortCode1"): cohort_group_result}, + ) + + with app.app_context(): + g.audit_log = AuditEvent() + + AuditContext.append_audit_condition( + condition_name, best_iteration_results, MatchedActionDetail(), [cohort_group_result] + ) + + assert g.audit_log.response.condition, condition_name + cond = g.audit_log.response.condition[0] + assert cond.status == "actionable" + assert cond.status_text == "You should have the Condition1 vaccine" + assert cond.actions is None + assert cond.action_rule is None + assert cond.suitability_rules[0].rule_priority == "1" + assert cond.suitability_rules[0].rule_name == "SuppressionRuleName1" + assert cond.suitability_rules[0].rule_message == "SuppressionRuleDescription1" assert cond.filter_rules is None assert cond.eligibility_cohorts[0].cohort_code == "CohortCode1" assert cond.eligibility_cohorts[0].cohort_status == "actionable" @@ -165,6 +247,62 @@ def test_append_audit_condition_adds_condition_to_audit_log_on_g(app): assert cond.eligibility_cohort_groups[0].cohort_text == "CohortDescription1" +def test_should_append_audit_filter_rules_for_not_actionable_status(app): + condition_name: ConditionName + campaign_details: tuple[CampaignID | None, CampaignVersion | None] + + condition_name = ConditionName("Condition1") + iteration = IterationFactory.build() + audit_rules = [ + Reason( + rule_type=RuleType.filter, + rule_name=RuleName("FilterRuleName1"), + rule_description=RuleDescription("FilterRuleDescription1"), + matcher_matched=True, + rule_priority=RulePriority("1"), + ) + ] + cohort_group_result = CohortGroupResult( + status=Status.not_actionable, + cohort_code="CohortCode1", + description="CohortDescription1", + audit_rules=audit_rules, + reasons=audit_rules, + ) + iteration_result = IterationResult(status=Status.not_actionable, cohort_results=[cohort_group_result], actions=[]) + campaign_details = (CampaignID("CampaignID1"), CampaignVersion(123)) + + best_iteration_results = BestIterationResult( + iteration_result, + iteration, + campaign_details[0], + campaign_details[1], + {CohortLabel("CohortCode1"): cohort_group_result}, + ) + + with app.app_context(): + g.audit_log = AuditEvent() + + AuditContext.append_audit_condition( + condition_name, best_iteration_results, MatchedActionDetail(), [cohort_group_result] + ) + + assert g.audit_log.response.condition, condition_name + cond = g.audit_log.response.condition[0] + assert cond.status == "not_actionable" + assert cond.status_text == "You should have the Condition1 vaccine" + assert cond.actions is None + assert cond.action_rule is None + assert cond.filter_rules[0].rule_priority == "1" + assert cond.filter_rules[0].rule_name == "FilterRuleName1" + assert cond.suitability_rules is None + assert cond.eligibility_cohorts[0].cohort_code == "CohortCode1" + assert cond.eligibility_cohorts[0].cohort_status == "not_actionable" + assert cond.eligibility_cohort_groups[0].cohort_code == "CohortCode1" + assert cond.eligibility_cohort_groups[0].cohort_status == "not_actionable" + assert cond.eligibility_cohort_groups[0].cohort_text == "CohortDescription1" + + def test_add_response_details_adds_to_audit_log_on_g(app): response_id = uuid.uuid4() last_updated = datetime(2023, 1, 1, 0, 0, tzinfo=UTC) From ef7fc503c3087c104f74d02fdd4d349fe46b9296 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Wed, 13 Aug 2025 09:50:52 +0100 Subject: [PATCH 2/3] ELI-405: Updates tests --- tests/unit/audit/test_audit_context.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/tests/unit/audit/test_audit_context.py b/tests/unit/audit/test_audit_context.py index 350e66f9c..14f82376f 100644 --- a/tests/unit/audit/test_audit_context.py +++ b/tests/unit/audit/test_audit_context.py @@ -345,7 +345,7 @@ def test_no_duplicates_returns_same_list(): def test_duplicates_are_removed(): reasons = [ Reason(RuleType("F"), RuleName("code1"), RulePriority("1"), RuleDescription("desc1"), matcher_matched=True), - Reason(RuleType("S"), RuleName("code1"), RulePriority("2"), RuleDescription("desc2"), matcher_matched=False), + Reason(RuleType("F"), RuleName("code2"), RulePriority("1"), RuleDescription("desc2"), matcher_matched=False), Reason(RuleType("R"), RuleName("code3"), RulePriority("3"), RuleDescription("desc3"), matcher_matched=True), ] expected = [ @@ -360,15 +360,3 @@ def test_empty_list_returns_empty_list(): expected = [] assert AuditContext.deduplicate_reasons(reasons) == expected - -def test_reasons_with_no_description_are_filtered_out(): - reasons = [ - Reason(RuleType("F"), RuleName("code1"), RulePriority("1"), RuleDescription("desc1"), matcher_matched=True), - Reason(RuleType("S"), RuleName("code2"), RulePriority("2"), None, matcher_matched=False), - Reason(RuleType("R"), RuleName("code3"), RulePriority("3"), RuleDescription("desc3"), matcher_matched=True), - ] - expected = [ - Reason(RuleType("F"), RuleName("code1"), RulePriority("1"), RuleDescription("desc1"), matcher_matched=True), - Reason(RuleType("R"), RuleName("code3"), RulePriority("3"), RuleDescription("desc3"), matcher_matched=True), - ] - assert AuditContext.deduplicate_reasons(reasons) == expected From 0d094a32f59ba8d0fff280a7acbb56e68288dd44 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Wed, 13 Aug 2025 09:54:21 +0100 Subject: [PATCH 3/3] ELI-405: Fix formatting --- tests/unit/audit/test_audit_context.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/audit/test_audit_context.py b/tests/unit/audit/test_audit_context.py index 14f82376f..e08143295 100644 --- a/tests/unit/audit/test_audit_context.py +++ b/tests/unit/audit/test_audit_context.py @@ -359,4 +359,3 @@ def test_empty_list_returns_empty_list(): reasons = [] expected = [] assert AuditContext.deduplicate_reasons(reasons) == expected -