eli 372 - IR cohort labels can have multiple labels separated by ",". (#463)

* eli 372 - IR cohort labels can have multiple labels seperated by ",".

* eli 372 - added new tests for comma - separated cohort labels

* eli 372 - validation test fixed

* eli 372 - added more test to validation cohort label

* wip integration tests - test_regardless_of_final_status_audit_all_types_of_cohort_status_rules_based_on_cohort_labels

* Revert "wip integration tests - test_regardless_of_final_status_audit_all_types_of_cohort_status_rules_based_on_cohort_labels"

This reverts commit f373cd0.

* wip integration tests - test_multiple_comma_seperated_cohort_labels

---------

Co-authored-by: ayeshalshukri1-nhs <[email protected]>

Author: Karthikeyannhs

src/eligibility_signposting_api/model/campaign_config.py

@@ -138,12 +138,16 @@ def normalize_virtual(cls, value: str) -> Virtual:
 class IterationRule(BaseModel):
     type: RuleType = Field(..., alias="Type")
     name: RuleName = Field(..., alias="Name")
-    code: RuleCode | None = Field(None, alias="Code", description="use the `rule_code` property instead.")
-    description: RuleDescription = Field(..., alias="Description", description="use the `rule_text` property instead.")
+    code: RuleCode | None = Field(None, alias="Code", description="use `rule_code` property instead.")
+    description: RuleDescription = Field(..., alias="Description", description="use `rule_text` property instead.")
     priority: RulePriority = Field(..., alias="Priority")
     attribute_level: RuleAttributeLevel = Field(..., alias="AttributeLevel")
     attribute_name: RuleAttributeName | None = Field(None, alias="AttributeName")
-    cohort_label: CohortLabel | None = Field(None, alias="CohortLabel")
+    cohort_label: CohortLabel | None = Field(
+        None,
+        alias="CohortLabel",
+        description="Raw label input. Prefer using `parsed_cohort_labels` for normalized access.",
+    )
     operator: RuleOperator = Field(..., alias="Operator")
     comparator: RuleComparator = Field(..., alias="Comparator")
     attribute_target: RuleAttributeTarget | None = Field(None, alias="AttributeTarget")
@@ -197,6 +201,18 @@ def rule_text(self) -> str:
                     rule_text = rule_entry.rule_text
         return rule_text or self.description
 
+    @cached_property
+    def parsed_cohort_labels(self) -> list[str]:
+        """
+        Parses the cohort_label string into a list of individual labels.
+
+        Returns:
+            A list of cohort labels, split by comma. If no label is set, returns an empty list.
+        """
+        if not self.cohort_label:
+            return []
+        return [label.strip() for label in self.cohort_label.split(",") if label.strip()]
+
     def __str__(self) -> str:
         return json.dumps(self.model_dump(by_alias=True), indent=2)
 

src/eligibility_signposting_api/services/processors/rule_processor.py

@@ -123,8 +123,10 @@ def is_actionable(
 
     @staticmethod
     def _should_skip_rule_group(cohort: IterationCohort, group_rules: list[IterationRule]) -> bool:
-        cohort_specific_rules = [rule for rule in group_rules if rule.cohort_label is not None]
-        matching_specific_rules = [rule for rule in cohort_specific_rules if rule.cohort_label == cohort.cohort_label]
+        cohort_specific_rules = [rule for rule in group_rules if rule.parsed_cohort_labels]
+        matching_specific_rules = [
+            rule for rule in cohort_specific_rules if cohort.cohort_label in rule.parsed_cohort_labels
+        ]
         return bool(cohort_specific_rules and not matching_specific_rules)
 
     def evaluate_rules_priority_group(
@@ -148,13 +150,7 @@ def evaluate_rules_priority_group(
 
     @staticmethod
     def get_exclusion_rules(cohort: IterationCohort, rules: Iterable[IterationRule]) -> Iterator[IterationRule]:
-        return (
-            ir
-            for ir in rules
-            if ir.cohort_label is None
-            or cohort.cohort_label == ir.cohort_label
-            or (isinstance(ir.cohort_label, (list, set, tuple)) and cohort.cohort_label in ir.cohort_label)
-        )
+        return (ir for ir in rules if not ir.parsed_cohort_labels or cohort.cohort_label in ir.parsed_cohort_labels)
 
     def get_cohort_group_results(
         self, person: Person, active_iteration: Iteration

tests/unit/services/calculators/test_eligibility_calculator.py

@@ -382,6 +382,46 @@ def test_status_on_target_based_on_last_successful_date(
     )
 
 
+def test_multiple_comma_seperated_cohort_labels(faker: Faker):
+    # Given
+    nhs_number = NHSNumber(faker.nhs_number())
+    date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=66, maximum_age=74))
+
+    person_rows = person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort5", "cohort6"])
+    campaign_configs = [
+        rule_builder.CampaignConfigFactory.build(
+            target="RSV",
+            iterations=[
+                rule_builder.IterationFactory.build(
+                    iteration_cohorts=[
+                        rule_builder.IterationCohortFactory.build(cohort_label="cohort5", cohort_group="test1"),
+                        rule_builder.IterationCohortFactory.build(cohort_label="cohort6", cohort_group="test2"),
+                    ],
+                    iteration_rules=[
+                        rule_builder.PersonAgeSuppressionRuleFactory.build(cohort_label="cohort5, cohort6")
+                    ],
+                )
+            ],
+        )
+    ]
+
+    calculator = EligibilityCalculator(person_rows, campaign_configs)
+
+    # When
+    actual = calculator.get_eligibility_status("Y", ["ALL"], "ALL")
+
+    # Then
+    assert_that(
+        actual,
+        is_eligibility_status().with_conditions(
+            has_items(is_condition().with_condition_name(ConditionName("RSV")).and_status(Status.not_actionable))
+        ),
+    )
+
+    all_cohorts = [cohort.cohort_code for cohort in g.audit_log.response.condition[0].eligibility_cohorts]
+    assert_that(all_cohorts, contains_inanyorder("cohort6", "cohort5"))
+
+
 @pytest.mark.parametrize(
     ("person_cohorts", "expected_status", "test_comment"),
     [

tests/unit/services/processors/test_cohort_handler.py

@@ -3,6 +3,7 @@
 import pytest
 from hamcrest import assert_that, has_length, is_
 
+from eligibility_signposting_api.model.campaign_config import IterationCohort, IterationRule
 from eligibility_signposting_api.model.eligibility_status import CohortGroupResult, Status
 from eligibility_signposting_api.model.person import Person
 from eligibility_signposting_api.services.processors.cohort_handler import (
@@ -86,12 +87,22 @@ def test_filter_rule_handler_is_not_eligible(mock_rule_processor_for_handlers, m
     cohort = rule_builder.IterationCohortFactory.build(cohort_label="cohort1", negative_description="Not Eligible")
     cohort_results = {}
 
-    mock_rule_processor_for_handlers.is_eligible.side_effect = (
-        lambda p, c, cr, fr: cr.update(  # noqa: ARG005
-            {c.cohort_label: CohortGroupResult(c.cohort_group, Status.not_eligible, [], c.negative_description, [])}
+    def mark_not_eligible_side_effect(
+        person: Person,  # noqa : ARG001
+        context: IterationCohort,
+        results: dict[str, CohortGroupResult],
+        rules: list[IterationRule],  # noqa : ARG001
+    ) -> bool:
+        results.update(
+            {
+                context.cohort_label: CohortGroupResult(
+                    context.cohort_group, Status.not_eligible, [], context.negative_description, []
+                )
+            }
         )
-        or False
-    )
+        return False
+
+    mock_rule_processor_for_handlers.is_eligible.side_effect = mark_not_eligible_side_effect
 
     handler.handle(MOCK_PERSON, cohort, cohort_results, mock_rule_processor_for_handlers)
 
@@ -109,9 +120,21 @@ def test_suppression_rule_handler_is_actionable(mock_rule_processor_for_handlers
     cohort = rule_builder.IterationCohortFactory.build(cohort_label="cohort1", positive_description="Actionable")
     cohort_results = {}
 
-    mock_rule_processor_for_handlers.is_actionable.side_effect = lambda p, c, cr, sr: cr.update(  # noqa: ARG005
-        {c.cohort_label: CohortGroupResult(c.cohort_group, Status.actionable, [], c.positive_description, [])}
-    )
+    def mark_actionable_side_effect(
+        person: Person,  # noqa : ARG001
+        context: IterationCohort,
+        results: dict[str, CohortGroupResult],
+        rules: list[IterationRule],  # noqa : ARG001
+    ) -> None:
+        results.update(
+            {
+                context.cohort_label: CohortGroupResult(
+                    context.cohort_group, Status.actionable, [], context.positive_description, []
+                )
+            }
+        )
+
+    mock_rule_processor_for_handlers.is_actionable.side_effect = mark_actionable_side_effect
 
     handler.handle(MOCK_PERSON, cohort, cohort_results, mock_rule_processor_for_handlers)
 

tests/unit/services/processors/test_rule_processor.py

@@ -3,7 +3,7 @@
 import pytest
 from hamcrest import assert_that, empty, is_
 
-from eligibility_signposting_api.model.campaign_config import CohortLabel, RuleType
+from eligibility_signposting_api.model.campaign_config import CohortLabel, IterationCohort, RuleType
 from eligibility_signposting_api.model.eligibility_status import CohortGroupResult, Reason, RuleName, Status
 from eligibility_signposting_api.model.person import Person
 from eligibility_signposting_api.services.processors.person_data_reader import PersonDataReader
@@ -48,6 +48,14 @@ def test_get_exclusion_rules_matching_cohort_label():
     assert_that(result, is_([matching_rule]))
 
 
+def test_get_exclusion_rules_matching_cohort_label_when_it_contains_multiple_cohort_labels():
+    cohort = rule_builder.IterationCohortFactory.build(cohort_label="COHORT_A")
+    matching_rule = rule_builder.IterationRuleFactory.build(cohort_label="COHORT_A,COHORT_B")
+    rules_to_filter = [matching_rule]
+    result = list(RuleProcessor.get_exclusion_rules(cohort, rules_to_filter))
+    assert_that(result, is_([matching_rule]))
+
+
 def test_get_exclusion_rules_non_matching_cohort_label():
     cohort = rule_builder.IterationCohortFactory.build(cohort_label="COHORT_A")
     non_matching_rule = rule_builder.IterationRuleFactory.build(cohort_label="COHORT_B")
@@ -56,6 +64,14 @@ def test_get_exclusion_rules_non_matching_cohort_label():
     assert_that(result, is_([]))
 
 
+def test_get_exclusion_rules_non_matching_cohort_label_when_it_contains_multiple_cohort_labels():
+    cohort = rule_builder.IterationCohortFactory.build(cohort_label="COHORT_A")
+    non_matching_rule = rule_builder.IterationRuleFactory.build(cohort_label="COHORT_B,COHORT_C")
+    rules_to_filter = [non_matching_rule]
+    result = list(RuleProcessor.get_exclusion_rules(cohort, rules_to_filter))
+    assert_that(result, is_([]))
+
+
 def test_get_exclusion_rules_matching_from_list_cohort_label():
     cohort = rule_builder.IterationCohortFactory.build(cohort_label="COHORT_A")
     rule1 = rule_builder.IterationRuleFactory.build(cohort_label="COHORT_A")
@@ -65,6 +81,15 @@ def test_get_exclusion_rules_matching_from_list_cohort_label():
     assert_that(result, is_([rule1]))
 
 
+def test_get_exclusion_rules_matching_from_list_cohort_label_when_it_contains_multiple_cohort_labels():
+    cohort = rule_builder.IterationCohortFactory.build(cohort_label="COHORT_A")
+    rule1 = rule_builder.IterationRuleFactory.build(cohort_label="COHORT_A")
+    rule2 = rule_builder.IterationRuleFactory.build(cohort_label="COHORT_B,COHORT_C")
+    rules_to_filter = [rule1, rule2]
+    result = list(RuleProcessor.get_exclusion_rules(cohort, rules_to_filter))
+    assert_that(result, is_([rule1]))
+
+
 def test_get_exclusion_rules_mixed_rules():
     cohort = rule_builder.IterationCohortFactory.build(cohort_label="COHORT_A")
     no_cohort_label_rule = rule_builder.IterationRuleFactory.build(cohort_label=None, name="General")
@@ -76,6 +101,17 @@ def test_get_exclusion_rules_mixed_rules():
     assert_that({r.name for r in result}, is_({"General", "Matching"}))
 
 
+def test_get_exclusion_rules_mixed_rules_when_it_contains_multiple_cohort_labels():
+    cohort = rule_builder.IterationCohortFactory.build(cohort_label="COHORT_A")
+    no_cohort_label_rule = rule_builder.IterationRuleFactory.build(cohort_label=None, name="General")
+    matching_rule = rule_builder.IterationRuleFactory.build(cohort_label="COHORT_A,COHORT_C", name="Matching")
+    non_matching_rule = rule_builder.IterationRuleFactory.build(cohort_label="COHORT_B,COHORT_C", name="NonMatching")
+
+    rules_to_filter = [no_cohort_label_rule, matching_rule, non_matching_rule]
+    result = list(RuleProcessor.get_exclusion_rules(cohort, rules_to_filter))
+    assert_that({r.name for r in result}, is_({"General", "Matching"}))
+
+
 @patch("eligibility_signposting_api.services.processors.rule_processor.RuleCalculator")
 def test_evaluate_rules_priority_group_all_actionable(mock_rule_calculator_class, rule_processor):
     mock_rule_calculator_class.return_value.evaluate_exclusion.return_value = (
@@ -570,17 +606,22 @@ def test_get_cohort_group_results(
     suppression_rules = (rule_builder.IterationRuleFactory.build(type=RuleType.suppression),)
     mock_get_rules_by_type.return_value = (filter_rules, suppression_rules)
 
-    def mock_handle_side_effect(person, cohort, cohort_results_dict, rule_processor_instance):  # noqa: ARG001
+    def mock_handle_side_effect(
+        person: Person,  # noqa: ARG001
+        cohort: IterationCohort,
+        cohort_results: dict[CohortLabel, CohortGroupResult],
+        rule_processor_instance: RuleProcessor,  # noqa: ARG001
+    ):
         if cohort.cohort_label == CohortLabel("COHORT_A"):
-            cohort_results_dict[CohortLabel("COHORT_A")] = CohortGroupResult(
+            cohort_results[CohortLabel("COHORT_A")] = CohortGroupResult(
                 cohort_code=cohort.cohort_group,
                 status=Status.actionable,
                 reasons=[],
                 description="Cohort A Description",
                 audit_rules=[],
             )
         elif cohort.cohort_label == CohortLabel("COHORT_B"):
-            cohort_results_dict[CohortLabel("COHORT_B")] = CohortGroupResult(
+            cohort_results[CohortLabel("COHORT_B")] = CohortGroupResult(
                 cohort_code=cohort.cohort_group,
                 status=Status.not_eligible,
                 reasons=[],

tests/unit/validation/test_iteration_rules_validator.py

@@ -163,12 +163,26 @@ def test_invalid_attribute_name(self, attr_name, valid_iteration_rule_with_only_
             IterationRuleValidation(**data)
 
     # CohortLabel
-    @pytest.mark.parametrize("label", ["Cohort_A", "Segment_2025", None, ""])
-    def test_valid_cohort_label(self, label, valid_iteration_rule_with_only_mandatory_fields):
+    @pytest.mark.parametrize(
+        ("label", "expected_parsed_cohort_label"),
+        [
+            ("Cohort_A", ["Cohort_A"]),
+            ("Cohort_A,Cohort_B", ["Cohort_A", "Cohort_B"]),
+            ("Cohort_C,,,,", ["Cohort_C"]),
+            ("Cohort_D,,,,Cohort_E", ["Cohort_D", "Cohort_E"]),
+            (",,,,Cohort_E,,,,", ["Cohort_E"]),
+            (",,,,", []),
+            ("", []),
+            (None, []),
+        ],
+    )
+    def test_cohort_label_parsing(
+        self, label, expected_parsed_cohort_label, valid_iteration_rule_with_only_mandatory_fields
+    ):
         data = valid_iteration_rule_with_only_mandatory_fields.copy()
         data["CohortLabel"] = label
         result = IterationRuleValidation(**data)
-        assert result.cohort_label == label
+        assert result.parsed_cohort_labels == expected_parsed_cohort_label
 
     @pytest.mark.parametrize("label", [123, [], {}])
     def test_invalid_cohort_label(self, label, valid_iteration_rule_with_only_mandatory_fields):