Skip to content

Commit 1dbf91a

Browse files
KarthikeyannhsKarthikeyannhs
committed
added unit test for rules stop
1 parent 5716598 commit 1dbf91a

File tree

2 files changed

+107
-98
lines changed

2 files changed

+107
-98
lines changed

‎src/eligibility_signposting_api/model/rules.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,10 @@ class IterationRule(BaseModel):
102102
rule_stop: bool = Field(default=False, alias="RuleStop")
103103

104104
@field_validator("rule_stop", mode="before")
105-
def parse_yn_to_bool(cls, v: str | bool) -> bool: # noqa: N805
105+
def parse_yn_to_bool(cls, v: str) -> bool: # noqa: N805
106106
if isinstance(v, str):
107107
return v.upper() == "Y"
108-
return bool(v)
108+
return False
109109

110110
model_config = {"populate_by_name": True, "extra": "ignore"}
111111

‎tests/unit/services/calculators/test_eligibility_calculator.py‎

Lines changed: 105 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from eligibility_signposting_api.model import rules
99
from eligibility_signposting_api.model import rules as rules_model
1010
from eligibility_signposting_api.model.eligibility import ConditionName, DateOfBirth, NHSNumber, Postcode, Status
11+
from eligibility_signposting_api.model.rules import IterationRule
1112
from eligibility_signposting_api.services.calculators.eligibility_calculator import EligibilityCalculator
1213
from tests.fixtures.builders.model import rule as rule_builder
1314
from tests.fixtures.builders.repos.person import person_rows_builder
@@ -336,48 +337,48 @@ def test_multiple_rule_types_cause_correct_status(faker: Faker):
336337
("test_comment", "rule1", "rule2", "expected_status"),
337338
[
338339
(
339-
"two rules, both exclude, same priority, should exclude",
340-
rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
341-
rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
342-
Status.not_actionable,
340+
"two rules, both exclude, same priority, should exclude",
341+
rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
342+
rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
343+
Status.not_actionable,
343344
),
344345
(
345-
"two rules, rule 1 excludes, same priority, should allow",
346-
rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
347-
rule_builder.PostcodeSuppressionRuleFactory.build(
348-
priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("NW1")
349-
),
350-
Status.actionable,
346+
"two rules, rule 1 excludes, same priority, should allow",
347+
rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
348+
rule_builder.PostcodeSuppressionRuleFactory.build(
349+
priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("NW1")
350+
),
351+
Status.actionable,
351352
),
352353
(
353-
"two rules, rule 2 excludes, same priority, should allow",
354-
rule_builder.PersonAgeSuppressionRuleFactory.build(
355-
priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("-65")
356-
),
357-
rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
358-
Status.actionable,
354+
"two rules, rule 2 excludes, same priority, should allow",
355+
rule_builder.PersonAgeSuppressionRuleFactory.build(
356+
priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("-65")
357+
),
358+
rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
359+
Status.actionable,
359360
),
360361
(
361-
"two rules, rule 1 excludes, different priority, should exclude",
362-
rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
363-
rule_builder.PostcodeSuppressionRuleFactory.build(
364-
priority=rules_model.RulePriority(10), comparator=rules_model.RuleComparator("NW1")
365-
),
366-
Status.not_actionable,
362+
"two rules, rule 1 excludes, different priority, should exclude",
363+
rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
364+
rule_builder.PostcodeSuppressionRuleFactory.build(
365+
priority=rules_model.RulePriority(10), comparator=rules_model.RuleComparator("NW1")
366+
),
367+
Status.not_actionable,
367368
),
368369
(
369-
"two rules, rule 2 excludes, different priority, should exclude",
370-
rule_builder.PersonAgeSuppressionRuleFactory.build(
371-
priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("-65")
372-
),
373-
rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(10)),
374-
Status.not_actionable,
370+
"two rules, rule 2 excludes, different priority, should exclude",
371+
rule_builder.PersonAgeSuppressionRuleFactory.build(
372+
priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("-65")
373+
),
374+
rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(10)),
375+
Status.not_actionable,
375376
),
376377
(
377-
"two rules, both excludes, different priority, should exclude",
378-
rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
379-
rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(10)),
380-
Status.not_actionable,
378+
"two rules, both excludes, different priority, should exclude",
379+
rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)),
380+
rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(10)),
381+
Status.not_actionable,
381382
),
382383
],
383384
)
@@ -524,46 +525,46 @@ def test_multiple_conditions_where_all_give_unique_statuses(faker: Faker):
524525
("test_comment", "campaign1", "campaign2"),
525526
[
526527
(
527-
"1st campaign allows, 2nd excludes",
528-
rule_builder.CampaignConfigFactory.build(
529-
target="RSV",
530-
iterations=[
531-
rule_builder.IterationFactory.build(
532-
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
533-
iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build()],
534-
)
535-
],
536-
),
537-
rule_builder.CampaignConfigFactory.build(
538-
target="RSV",
539-
iterations=[
540-
rule_builder.IterationFactory.build(
541-
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
542-
iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build(comparator="-85")],
543-
)
544-
],
545-
),
528+
"1st campaign allows, 2nd excludes",
529+
rule_builder.CampaignConfigFactory.build(
530+
target="RSV",
531+
iterations=[
532+
rule_builder.IterationFactory.build(
533+
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
534+
iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build()],
535+
)
536+
],
537+
),
538+
rule_builder.CampaignConfigFactory.build(
539+
target="RSV",
540+
iterations=[
541+
rule_builder.IterationFactory.build(
542+
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
543+
iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build(comparator="-85")],
544+
)
545+
],
546+
),
546547
),
547548
(
548-
"1st campaign excludes, 2nd allows",
549-
rule_builder.CampaignConfigFactory.build(
550-
target="RSV",
551-
iterations=[
552-
rule_builder.IterationFactory.build(
553-
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
554-
iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build(comparator="-85")],
555-
)
556-
],
557-
),
558-
rule_builder.CampaignConfigFactory.build(
559-
target="RSV",
560-
iterations=[
561-
rule_builder.IterationFactory.build(
562-
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
563-
iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build()],
564-
)
565-
],
566-
),
549+
"1st campaign excludes, 2nd allows",
550+
rule_builder.CampaignConfigFactory.build(
551+
target="RSV",
552+
iterations=[
553+
rule_builder.IterationFactory.build(
554+
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
555+
iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build(comparator="-85")],
556+
)
557+
],
558+
),
559+
rule_builder.CampaignConfigFactory.build(
560+
target="RSV",
561+
iterations=[
562+
rule_builder.IterationFactory.build(
563+
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
564+
iteration_rules=[rule_builder.PersonAgeSuppressionRuleFactory.build()],
565+
)
566+
],
567+
),
567568
),
568569
],
569570
)
@@ -814,45 +815,53 @@ def test_status_if_iteration_rules_contains_cohort_label_field(
814815

815816

816817
@pytest.mark.parametrize(
817-
("rule_stop", "expected_status", "test_comment"),
818+
"rule_stop, expected_status, test_comment",
818819
[
819820
("Y", Status.not_actionable, "Stops at the first rule"),
820821
("N", Status.not_eligible, "Both the rules are executed"),
821822
("", Status.not_eligible, "Both the rules are executed"),
822823
(None, Status.not_eligible, "Both the rules are executed"),
823824
],
824825
)
825-
def test_rules_stop(rule_stop: str, expected_status: Status, test_comment: str, faker: Faker):
826+
def test_rules_stop_behavior(rule_stop: str | None, expected_status: Status, test_comment: str, faker: Faker) -> None:
827+
826828
# Given
827829
nhs_number = NHSNumber(faker.nhs_number())
828830
date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=18, maximum_age=74))
829-
830831
person_rows = person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"])
831-
campaign_configs = [
832-
rule_builder.CampaignConfigFactory.build(
833-
target="RSV",
834-
iterations=[
835-
rule_builder.IterationFactory.build(
836-
iteration_rules=[
837-
rule_builder.PersonAgeSuppressionRuleFactory.build(
838-
priority=rules_model.RulePriority(10),
839-
type=rules_model.RuleType.suppression,
840-
rule_stop=rule_stop,
841-
),
842-
rule_builder.PersonAgeSuppressionRuleFactory.build(
843-
priority=rules_model.RulePriority(10), type=rules_model.RuleType.suppression
844-
),
845-
rule_builder.PersonAgeSuppressionRuleFactory.build(
846-
priority=rules_model.RulePriority(15), type=rules_model.RuleType.filter
847-
),
848-
],
849-
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
850-
)
851-
],
852-
)
832+
833+
# Base rule template
834+
# Not using model factory to create Iteration rules since it sets boolean values for "Y"/"N"
835+
simple_age_data = {
836+
"Name": "Exclude too young less than 75",
837+
"Description": "Exclude too young less than 75",
838+
"AttributeLevel": "PERSON",
839+
"AttributeName": "DATE_OF_BIRTH",
840+
"Operator": "Y>",
841+
"Comparator": "-75",
842+
}
843+
844+
# Build rule variations
845+
rule_variants = [
846+
{"Type": "S", "Priority": 10, "RuleStop": rule_stop},
847+
{"Type": "S", "Priority": 10},
848+
{"Type": "F", "Priority": 15},
853849
]
854850

855-
calculator = EligibilityCalculator(person_rows, campaign_configs)
851+
iteration_rules = [IterationRule.model_validate({**simple_age_data, **variant}) for variant in rule_variants]
852+
853+
# Build campaign configuration
854+
campaign_config = rule_builder.CampaignConfigFactory.build(
855+
target="RSV",
856+
iterations=[
857+
rule_builder.IterationFactory.build(
858+
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
859+
)
860+
],
861+
)
862+
campaign_config.iterations[0].iteration_rules.extend(iteration_rules)
863+
864+
calculator = EligibilityCalculator(person_rows, [campaign_config])
856865

857866
# When
858867
actual = calculator.evaluate_eligibility()

0 commit comments

Comments
 (0)