From 0093992c90003c75e350f550f692038c2ca10653 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Thu, 10 Jul 2025 10:44:26 +0100 Subject: [PATCH 1/2] ELI-138: Adds cohortLabel in audit.eligibilityCohorts instead of cohortGroup --- .../audit/audit_context.py | 21 +++++++++---------- tests/integration/conftest.py | 4 ++-- .../lambda/test_app_running_as_lambda.py | 8 +++---- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/eligibility_signposting_api/audit/audit_context.py b/src/eligibility_signposting_api/audit/audit_context.py index 09094d95b..424b2cea2 100644 --- a/src/eligibility_signposting_api/audit/audit_context.py +++ b/src/eligibility_signposting_api/audit/audit_context.py @@ -1,6 +1,5 @@ import logging from datetime import UTC, datetime -from operator import attrgetter from uuid import UUID from flask import Request, g @@ -73,29 +72,29 @@ def append_audit_condition( best_cohort_results = best_results[2] if best_cohort_results: - for value in sorted(best_cohort_results.values(), key=attrgetter("cohort_code")): - cohort_status_name = value.status.name if value.status else None + for cohort_label, result in sorted(best_cohort_results.items(), key=lambda item: item[1].cohort_code): + cohort_status_name = result.status.name if result.status else None audit_eligibility_cohorts.append( - AuditEligibilityCohorts(cohort_code=value.cohort_code, cohort_status=cohort_status_name) + AuditEligibilityCohorts(cohort_code=cohort_label, cohort_status=cohort_status_name) ) audit_eligibility_cohort_groups.append( AuditEligibilityCohortGroups( - cohort_code=value.cohort_code, cohort_status=cohort_status_name, cohort_text=value.description + cohort_code=result.cohort_code, cohort_status=cohort_status_name, cohort_text=result.description ) ) - if value.audit_rules and best_candidate: + if result.audit_rules and best_candidate: if best_candidate.status and best_candidate.status.name == Status.not_eligible.name: audit_filter_rule = AuditFilterRule( - rule_priority=value.audit_rules[0].rule_priority, - rule_name=value.audit_rules[0].rule_name, + rule_priority=result.audit_rules[0].rule_priority, + rule_name=result.audit_rules[0].rule_name, ) if best_candidate.status and best_candidate.status.name == Status.not_actionable.name: audit_suitability_rule = AuditSuitabilityRule( - rule_priority=value.audit_rules[0].rule_priority, - rule_name=value.audit_rules[0].rule_name, - rule_message=value.audit_rules[0].rule_description, + rule_priority=result.audit_rules[0].rule_priority, + rule_name=result.audit_rules[0].rule_name, + rule_message=result.audit_rules[0].rule_description, ) if best_candidate and best_candidate.status and best_candidate.status.name == Status.actionable.name: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 44ffeda08..eaeb3251a 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -376,7 +376,7 @@ def persisted_person_all_cohorts(person_table: Any, faker: Faker) -> Generator[e nhs_number, date_of_birth=date_of_birth, postcode="hp1", - cohorts=["cohort1", "cohort2", "cohort3"], + cohorts=["cohort_label1", "cohort_label2", "cohort_label3"], icb="QE1", ) ): @@ -501,7 +501,7 @@ def multiple_campaign_configs(s3_client: BaseClient, rules_bucket: BucketName) - iteration_rules=target_rules_map.get(targets[i]), iteration_cohorts=[ rule.IterationCohortFactory.build( - cohort_label="cohort1", + cohort_label=f"cohort_label{i + 1}", cohort_group=f"cohort_group{i + 1}", positive_description=f"positive_desc_{i + 1}", negative_description=f"negative_desc_{i + 1}", diff --git a/tests/integration/lambda/test_app_running_as_lambda.py b/tests/integration/lambda/test_app_running_as_lambda.py index 8253e9a1a..be536877e 100644 --- a/tests/integration/lambda/test_app_running_as_lambda.py +++ b/tests/integration/lambda/test_app_running_as_lambda.py @@ -216,7 +216,7 @@ def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers_and_check_i "conditionName": campaign_config.target, "status": "not_actionable", "statusText": "not_actionable", - "eligibilityCohorts": [{"cohortCode": "cohort_group1", "cohortStatus": "not_actionable"}], + "eligibilityCohorts": [{"cohortCode": "cohort1", "cohortStatus": "not_actionable"}], "eligibilityCohortGroups": [ { "cohortCode": "cohort_group1", @@ -320,7 +320,7 @@ def test_given_person_has_unique_status_for_different_conditions_with_audit( # "conditionName": rsv_campaign.target, "status": "not_eligible", "statusText": "not_eligible", - "eligibilityCohorts": [{"cohortCode": "cohort_group1", "cohortStatus": "not_eligible"}], + "eligibilityCohorts": [{"cohortCode": "cohort_label1", "cohortStatus": "not_eligible"}], "eligibilityCohortGroups": [ { "cohortCode": "cohort_group1", @@ -341,7 +341,7 @@ def test_given_person_has_unique_status_for_different_conditions_with_audit( # "conditionName": covid_campaign.target, "status": "not_actionable", "statusText": "not_actionable", - "eligibilityCohorts": [{"cohortCode": "cohort_group2", "cohortStatus": "not_actionable"}], + "eligibilityCohorts": [{"cohortCode": "cohort_label2", "cohortStatus": "not_actionable"}], "eligibilityCohortGroups": [ { "cohortCode": "cohort_group2", @@ -366,7 +366,7 @@ def test_given_person_has_unique_status_for_different_conditions_with_audit( # "conditionName": flu_campaign.target, "status": "actionable", "statusText": "actionable", - "eligibilityCohorts": [{"cohortCode": "cohort_group3", "cohortStatus": "actionable"}], + "eligibilityCohorts": [{"cohortCode": "cohort_label3", "cohortStatus": "actionable"}], "eligibilityCohortGroups": [ { "cohortCode": "cohort_group3", From 956ecad1ca245bd495c84dd731acb8efd595bbcf Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Thu, 10 Jul 2025 11:01:56 +0100 Subject: [PATCH 2/2] ELI-138: Refactored audit function to reduce its Cognitive Complexity from 35 to the 15 allowed. --- .../audit/audit_context.py | 72 ++++++++++++------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/src/eligibility_signposting_api/audit/audit_context.py b/src/eligibility_signposting_api/audit/audit_context.py index 424b2cea2..b52edbe9b 100644 --- a/src/eligibility_signposting_api/audit/audit_context.py +++ b/src/eligibility_signposting_api/audit/audit_context.py @@ -65,7 +65,7 @@ def append_audit_condition( campaign_details: tuple[CampaignID | None, CampaignVersion | None], redirect_rule_details: tuple[RulePriority | None, RuleName | None], ) -> None: - audit_eligibility_cohorts, audit_eligibility_cohort_groups, audit_actions = [], [], [] + audit_eligibility_cohorts, audit_eligibility_cohort_groups = [], [] audit_filter_rule, audit_suitability_rule, audit_redirect_rule = None, None, None best_active_iteration = best_results[0] best_candidate = best_results[1] @@ -85,37 +85,15 @@ def append_audit_condition( ) if result.audit_rules and best_candidate: - if best_candidate.status and best_candidate.status.name == Status.not_eligible.name: - audit_filter_rule = AuditFilterRule( - rule_priority=result.audit_rules[0].rule_priority, - rule_name=result.audit_rules[0].rule_name, - ) - if best_candidate.status and best_candidate.status.name == Status.not_actionable.name: - audit_suitability_rule = AuditSuitabilityRule( - rule_priority=result.audit_rules[0].rule_priority, - rule_name=result.audit_rules[0].rule_name, - rule_message=result.audit_rules[0].rule_description, - ) + audit_filter_rule = AuditContext.create_audit_filter_rule(best_candidate, result) + audit_suitability_rule = AuditContext.create_audit_suitability_rule(best_candidate, result) if best_candidate and best_candidate.status and best_candidate.status.name == Status.actionable.name: audit_redirect_rule = AuditRedirectRule( rule_priority=str(redirect_rule_details[0]), rule_name=redirect_rule_details[1] ) - if suggested_actions is None: - audit_actions = None - elif len(suggested_actions) > 0: - for action in suggested_actions: - audit_actions.append( - AuditAction( - internal_action_code=action.internal_action_code, - action_code=action.action_code, - action_type=action.action_type, - action_description=action.action_description, - action_url=str(action.url_link) if action.url_link else None, - action_url_label=action.url_label, - ) - ) + audit_actions = AuditContext.create_audit_actions(suggested_actions) audit_condition = AuditCondition( campaign_id=campaign_details[0], @@ -143,3 +121,45 @@ def add_response_details(response_id: UUID, last_updated: datetime) -> None: @staticmethod def write_to_firehose(service: AuditService) -> None: service.audit(g.audit_log.model_dump(by_alias=True)) + + @staticmethod + def create_audit_actions(suggested_actions: list[SuggestedAction] | None) -> list[AuditAction] | None: + audit_actions = [] + if suggested_actions is None: + audit_actions = None + elif len(suggested_actions) > 0: + for action in suggested_actions: + audit_actions.append( + AuditAction( + internal_action_code=action.internal_action_code, + action_code=action.action_code, + action_type=action.action_type, + action_description=action.action_description, + action_url=str(action.url_link) if action.url_link else None, + action_url_label=action.url_label, + ) + ) + return audit_actions + + @staticmethod + def create_audit_suitability_rule( + best_candidate: IterationResult, result: CohortGroupResult + ) -> AuditSuitabilityRule | None: + audit_suitability_rule = None + if best_candidate.status and best_candidate.status.name == Status.not_actionable.name: + audit_suitability_rule = AuditSuitabilityRule( + rule_priority=result.audit_rules[0].rule_priority, + rule_name=result.audit_rules[0].rule_name, + rule_message=result.audit_rules[0].rule_description, + ) + return audit_suitability_rule + + @staticmethod + def create_audit_filter_rule(best_candidate: IterationResult, result: CohortGroupResult) -> AuditFilterRule | None: + audit_filter_rule = None + if best_candidate.status and best_candidate.status.name == Status.not_eligible.name: + audit_filter_rule = AuditFilterRule( + rule_priority=result.audit_rules[0].rule_priority, + rule_name=result.audit_rules[0].rule_name, + ) + return audit_filter_rule