Skip to content

Commit f9932c9

Browse files
shweta-nhsshweta-nhs
authored
ELI-138: Adds cohortLabel in audit.eligibilityCohorts instead of coho… (#224)
* ELI-138: Adds cohortLabel in audit.eligibilityCohorts instead of cohortGroup * ELI-138: Refactored audit function to reduce its Cognitive Complexity from 35 to the 15 allowed.
1 parent 00b7b85 commit f9932c9

3 files changed

Lines changed: 57 additions & 38 deletions

File tree

‎src/eligibility_signposting_api/audit/audit_context.py‎

Lines changed: 51 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import logging
22
from datetime import UTC, datetime
3-
from operator import attrgetter
43
from uuid import UUID
54

65
from flask import Request, g
@@ -66,57 +65,35 @@ def append_audit_condition(
6665
campaign_details: tuple[CampaignID | None, CampaignVersion | None],
6766
redirect_rule_details: tuple[RulePriority | None, RuleName | None],
6867
) -> None:
69-
audit_eligibility_cohorts, audit_eligibility_cohort_groups, audit_actions = [], [], []
68+
audit_eligibility_cohorts, audit_eligibility_cohort_groups = [], []
7069
audit_filter_rule, audit_suitability_rule, audit_redirect_rule = None, None, None
7170
best_active_iteration = best_results[0]
7271
best_candidate = best_results[1]
7372
best_cohort_results = best_results[2]
7473

7574
if best_cohort_results:
76-
for value in sorted(best_cohort_results.values(), key=attrgetter("cohort_code")):
77-
cohort_status_name = value.status.name if value.status else None
75+
for cohort_label, result in sorted(best_cohort_results.items(), key=lambda item: item[1].cohort_code):
76+
cohort_status_name = result.status.name if result.status else None
7877
audit_eligibility_cohorts.append(
79-
AuditEligibilityCohorts(cohort_code=value.cohort_code, cohort_status=cohort_status_name)
78+
AuditEligibilityCohorts(cohort_code=cohort_label, cohort_status=cohort_status_name)
8079
)
8180

8281
audit_eligibility_cohort_groups.append(
8382
AuditEligibilityCohortGroups(
84-
cohort_code=value.cohort_code, cohort_status=cohort_status_name, cohort_text=value.description
83+
cohort_code=result.cohort_code, cohort_status=cohort_status_name, cohort_text=result.description
8584
)
8685
)
8786

88-
if value.audit_rules and best_candidate:
89-
if best_candidate.status and best_candidate.status.name == Status.not_eligible.name:
90-
audit_filter_rule = AuditFilterRule(
91-
rule_priority=value.audit_rules[0].rule_priority,
92-
rule_name=value.audit_rules[0].rule_name,
93-
)
94-
if best_candidate.status and best_candidate.status.name == Status.not_actionable.name:
95-
audit_suitability_rule = AuditSuitabilityRule(
96-
rule_priority=value.audit_rules[0].rule_priority,
97-
rule_name=value.audit_rules[0].rule_name,
98-
rule_message=value.audit_rules[0].rule_description,
99-
)
87+
if result.audit_rules and best_candidate:
88+
audit_filter_rule = AuditContext.create_audit_filter_rule(best_candidate, result)
89+
audit_suitability_rule = AuditContext.create_audit_suitability_rule(best_candidate, result)
10090

10191
if best_candidate and best_candidate.status and best_candidate.status.name == Status.actionable.name:
10292
audit_redirect_rule = AuditRedirectRule(
10393
rule_priority=str(redirect_rule_details[0]), rule_name=redirect_rule_details[1]
10494
)
10595

106-
if suggested_actions is None:
107-
audit_actions = None
108-
elif len(suggested_actions) > 0:
109-
for action in suggested_actions:
110-
audit_actions.append(
111-
AuditAction(
112-
internal_action_code=action.internal_action_code,
113-
action_code=action.action_code,
114-
action_type=action.action_type,
115-
action_description=action.action_description,
116-
action_url=str(action.url_link) if action.url_link else None,
117-
action_url_label=action.url_label,
118-
)
119-
)
96+
audit_actions = AuditContext.create_audit_actions(suggested_actions)
12097

12198
audit_condition = AuditCondition(
12299
campaign_id=campaign_details[0],
@@ -144,3 +121,45 @@ def add_response_details(response_id: UUID, last_updated: datetime) -> None:
144121
@staticmethod
145122
def write_to_firehose(service: AuditService) -> None:
146123
service.audit(g.audit_log.model_dump(by_alias=True))
124+
125+
@staticmethod
126+
def create_audit_actions(suggested_actions: list[SuggestedAction] | None) -> list[AuditAction] | None:
127+
audit_actions = []
128+
if suggested_actions is None:
129+
audit_actions = None
130+
elif len(suggested_actions) > 0:
131+
for action in suggested_actions:
132+
audit_actions.append(
133+
AuditAction(
134+
internal_action_code=action.internal_action_code,
135+
action_code=action.action_code,
136+
action_type=action.action_type,
137+
action_description=action.action_description,
138+
action_url=str(action.url_link) if action.url_link else None,
139+
action_url_label=action.url_label,
140+
)
141+
)
142+
return audit_actions
143+
144+
@staticmethod
145+
def create_audit_suitability_rule(
146+
best_candidate: IterationResult, result: CohortGroupResult
147+
) -> AuditSuitabilityRule | None:
148+
audit_suitability_rule = None
149+
if best_candidate.status and best_candidate.status.name == Status.not_actionable.name:
150+
audit_suitability_rule = AuditSuitabilityRule(
151+
rule_priority=result.audit_rules[0].rule_priority,
152+
rule_name=result.audit_rules[0].rule_name,
153+
rule_message=result.audit_rules[0].rule_description,
154+
)
155+
return audit_suitability_rule
156+
157+
@staticmethod
158+
def create_audit_filter_rule(best_candidate: IterationResult, result: CohortGroupResult) -> AuditFilterRule | None:
159+
audit_filter_rule = None
160+
if best_candidate.status and best_candidate.status.name == Status.not_eligible.name:
161+
audit_filter_rule = AuditFilterRule(
162+
rule_priority=result.audit_rules[0].rule_priority,
163+
rule_name=result.audit_rules[0].rule_name,
164+
)
165+
return audit_filter_rule

‎tests/integration/conftest.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ def persisted_person_all_cohorts(person_table: Any, faker: Faker) -> Generator[e
376376
nhs_number,
377377
date_of_birth=date_of_birth,
378378
postcode="hp1",
379-
cohorts=["cohort1", "cohort2", "cohort3"],
379+
cohorts=["cohort_label1", "cohort_label2", "cohort_label3"],
380380
icb="QE1",
381381
)
382382
):
@@ -501,7 +501,7 @@ def multiple_campaign_configs(s3_client: BaseClient, rules_bucket: BucketName) -
501501
iteration_rules=target_rules_map.get(targets[i]),
502502
iteration_cohorts=[
503503
rule.IterationCohortFactory.build(
504-
cohort_label="cohort1",
504+
cohort_label=f"cohort_label{i + 1}",
505505
cohort_group=f"cohort_group{i + 1}",
506506
positive_description=f"positive_desc_{i + 1}",
507507
negative_description=f"negative_desc_{i + 1}",

‎tests/integration/lambda/test_app_running_as_lambda.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers_and_check_i
216216
"conditionName": campaign_config.target,
217217
"status": "not_actionable",
218218
"statusText": "not_actionable",
219-
"eligibilityCohorts": [{"cohortCode": "cohort_group1", "cohortStatus": "not_actionable"}],
219+
"eligibilityCohorts": [{"cohortCode": "cohort1", "cohortStatus": "not_actionable"}],
220220
"eligibilityCohortGroups": [
221221
{
222222
"cohortCode": "cohort_group1",
@@ -320,7 +320,7 @@ def test_given_person_has_unique_status_for_different_conditions_with_audit( #
320320
"conditionName": rsv_campaign.target,
321321
"status": "not_eligible",
322322
"statusText": "not_eligible",
323-
"eligibilityCohorts": [{"cohortCode": "cohort_group1", "cohortStatus": "not_eligible"}],
323+
"eligibilityCohorts": [{"cohortCode": "cohort_label1", "cohortStatus": "not_eligible"}],
324324
"eligibilityCohortGroups": [
325325
{
326326
"cohortCode": "cohort_group1",
@@ -341,7 +341,7 @@ def test_given_person_has_unique_status_for_different_conditions_with_audit( #
341341
"conditionName": covid_campaign.target,
342342
"status": "not_actionable",
343343
"statusText": "not_actionable",
344-
"eligibilityCohorts": [{"cohortCode": "cohort_group2", "cohortStatus": "not_actionable"}],
344+
"eligibilityCohorts": [{"cohortCode": "cohort_label2", "cohortStatus": "not_actionable"}],
345345
"eligibilityCohortGroups": [
346346
{
347347
"cohortCode": "cohort_group2",
@@ -366,7 +366,7 @@ def test_given_person_has_unique_status_for_different_conditions_with_audit( #
366366
"conditionName": flu_campaign.target,
367367
"status": "actionable",
368368
"statusText": "actionable",
369-
"eligibilityCohorts": [{"cohortCode": "cohort_group3", "cohortStatus": "actionable"}],
369+
"eligibilityCohorts": [{"cohortCode": "cohort_label3", "cohortStatus": "actionable"}],
370370
"eligibilityCohortGroups": [
371371
{
372372
"cohortCode": "cohort_group3",

0 commit comments

Comments
 (0)