Skip to content

Commit 4fb4711

Browse files
committed
extracting method for readability
1 parent a5f5f1e commit 4fb4711

2 files changed

Lines changed: 14 additions & 15 deletions

File tree

src/eligibility_signposting_api/services/processors/rule_processor.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,20 +82,11 @@ def is_actionable(
8282

8383
for _, rule_group in groupby(sorted_rules_by_priority, key=priority_getter):
8484
group_rules = list(rule_group)
85-
cohort_specific_rules = [rule for rule in group_rules if rule.cohort_label is not None]
86-
matching_specific_rules = [
87-
rule for rule in cohort_specific_rules if rule.cohort_label == cohort.cohort_label
88-
]
89-
if cohort_specific_rules and not matching_specific_rules:
90-
continue
91-
92-
applicable_rules = list(self.get_exclusion_rules(cohort, group_rules))
93-
94-
if not applicable_rules:
85+
if self._should_skip_rule_group(cohort, group_rules):
9586
continue
9687

9788
status, group_exclusion_reasons, rule_stop = self.evaluate_rules_priority_group(
98-
person, iter(applicable_rules)
89+
person, iter(group_rules)
9990
)
10091
if status.is_exclusion:
10192
is_actionable = False
@@ -118,6 +109,14 @@ def is_actionable(
118109
suppression_reasons,
119110
)
120111

112+
@staticmethod
113+
def _should_skip_rule_group(cohort: IterationCohort, group_rules: list[IterationRule]) -> bool:
114+
cohort_specific_rules = [rule for rule in group_rules if rule.cohort_label is not None]
115+
matching_specific_rules = [
116+
rule for rule in cohort_specific_rules if rule.cohort_label == cohort.cohort_label
117+
]
118+
return bool(cohort_specific_rules and not matching_specific_rules)
119+
121120
def evaluate_rules_priority_group(
122121
self, person: Person, rules_group: Iterator[IterationRule]
123122
) -> tuple[eligibility_status.Status, list[eligibility_status.Reason], bool]:

tests/unit/services/processors/test_rule_processor.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,13 @@ def test_general_rule_should_not_evaluate_in_isolation_without_matching_specific
153153
cohort = rule_builder.IterationCohortFactory.build(cohort_label="COHORT_B", positive_description="Eligible")
154154
cohort_results = {}
155155

156-
# Rule 1: cohort-specific to COHORT_A — should be filtered out
156+
# Rule 1: Non-matching rule cohort-specific to COHORT_A — should not be evaluated
157157
rule_specific = rule_builder.IterationRuleFactory.build(
158158
priority=510, type=RuleType.suppression, cohort_label="COHORT_A", name="SPECIFIC_RULE"
159159
)
160160

161-
# Rule 2: General rule
161+
# Rule 2: Matching general rule of the same priority as cohort-specific rule
162+
# - should also not be evaluated
162163
rule_general = rule_builder.IterationRuleFactory.build(
163164
priority=510, type=RuleType.suppression, cohort_label=None, name="GENERAL_RULE"
164165
)
@@ -168,9 +169,8 @@ def test_general_rule_should_not_evaluate_in_isolation_without_matching_specific
168169
# Act
169170
rule_processor.is_actionable(MOCK_PERSON_DATA, cohort, cohort_results, suppression_rules)
170171

171-
# ❌ BUG: General rule should not be evaluated in isolation.
172+
# None of the rules should be evaluated
172173
mock_evaluate_rules_priority_group.assert_not_called()
173-
174174
# Cohort remains actionable
175175
assert_that(cohort_results["COHORT_B"].status, is_(Status.actionable))
176176

0 commit comments

Comments
 (0)