Eli 440 magic cohort redefined with virtual (#344)

* eli 440 : virtual flag for magic cohort

* added more cases for virtual cohorts

* cohort labels with virtual cohorts

* validation for virtual with more test

* fix virtual - normalize_virtual

* updated TODOS

* eli-440 small invalid test addition

* eli-440 updated tests, lint, format

* ELI-440: Fixing format

---------

Co-authored-by: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com>
Co-authored-by: Shweta <216860557+shweta-nhs@users.noreply.github.com>

Author: 3 people

src/eligibility_signposting_api/config/contants.py

@@ -1,6 +1,5 @@
 from typing import Literal
 
-MAGIC_COHORT_LABEL = "elid_all_people"
 RULE_STOP_DEFAULT = False
 NHS_NUMBER_HEADER = "nhs-login-nhs-number"
 ALLOWED_CONDITIONS = Literal["COVID", "FLU", "MMR", "RSV"]

src/eligibility_signposting_api/model/campaign_config.py

@@ -11,7 +11,7 @@
 
 from pydantic import BaseModel, Field, HttpUrl, RootModel, field_serializer, field_validator, model_validator
 
-from eligibility_signposting_api.config.contants import ALLOWED_CONDITIONS, MAGIC_COHORT_LABEL, RULE_STOP_DEFAULT
+from eligibility_signposting_api.config.contants import ALLOWED_CONDITIONS, RULE_STOP_DEFAULT
 
 if typing.TYPE_CHECKING:  # pragma: no cover
     from pydantic import SerializationInfo
@@ -90,18 +90,38 @@ class RuleAttributeLevel(StrEnum):
     COHORT = "COHORT"
 
 
+class Virtual(StrEnum):
+    YES = "Y"
+    NO = "N"
+
+
 class IterationCohort(BaseModel):
     cohort_label: CohortLabel = Field(alias="CohortLabel")
     cohort_group: CohortGroup = Field(alias="CohortGroup")
     positive_description: Description | None = Field(None, alias="PositiveDescription")
     negative_description: Description | None = Field(None, alias="NegativeDescription")
     priority: int | None = Field(None, alias="Priority")
+    virtual: Virtual = Field(default=Virtual.NO, alias="Virtual")
 
     model_config = {"populate_by_name": True, "extra": "ignore"}
 
     @cached_property
     def is_magic_cohort(self) -> bool:
-        return self.cohort_label.upper() == MAGIC_COHORT_LABEL.upper()
+        return self.virtual == Virtual.YES
+
+    @field_validator("virtual", mode="before")
+    @classmethod
+    def normalize_virtual(cls, value: str) -> Virtual:
+        if value is None:
+            return Virtual.NO
+        if isinstance(value, str):
+            value = value.strip().upper()
+        if value == "Y":
+            return Virtual.YES
+        if value == "N":
+            return Virtual.NO
+        msg = f"Invalid value for Virtual: {value!r}"
+        raise ValueError(msg)
 
 
 class IterationRule(BaseModel):

src/rules_validation_api/validators/iteration_validator.py

@@ -28,6 +28,21 @@ def validate_iteration_rules(cls, iteration_rules: list[IterationRule]) -> list[
     @field_validator("iteration_cohorts")
     @classmethod
     def validate_iteration_cohorts(cls, iteration_cohorts: list[IterationCohort]) -> list[IterationCohortValidation]:
+        seen_labels = set()
+        errors = []
+        for cohort in iteration_cohorts:
+            label = cohort.cohort_label
+            if label in seen_labels:
+                error = InitErrorDetails(
+                    type="value_error",
+                    loc=("iteration_cohort",),
+                    input=label,
+                    ctx={"error": f"Duplicate iteration_cohort: {label}"},
+                )
+                errors.append(error)
+            seen_labels.add(label)
+        if errors:
+            raise ValidationError.from_exception_data(title="IterationValidation", line_errors=errors)
         return [IterationCohortValidation(**i.model_dump()) for i in iteration_cohorts]
 
     @field_validator("actions_mapper", mode="after")

tests/fixtures/builders/model/rule.py

@@ -25,6 +25,7 @@
     RulePriority,
     RuleType,
     StatusText,
+    Virtual,
 )
 
 
@@ -38,6 +39,7 @@ def future_date(days_ahead: int = 365) -> date:
 
 class IterationCohortFactory(ModelFactory[IterationCohort]):
     priority = RulePriority(0)
+    virtual = Virtual.NO
 
 
 class IterationRuleFactory(ModelFactory[IterationRule]):
@@ -114,11 +116,12 @@ def fix_iteration_date_invariants(iterations: list[Iteration], start_date: date)
 
 
 # Iteration cohort factories
-class MagicCohortFactory(IterationCohortFactory):
-    cohort_label = CohortLabel("elid_all_people")
-    cohort_group = CohortGroup("magic cohort group")
-    positive_description = Description("magic positive description")
-    negative_description = Description("magic negative description")
+class VirtualCohortFactory(IterationCohortFactory):
+    cohort_label = CohortLabel("virtual cohort label")
+    cohort_group = CohortGroup("virtual cohort group")
+    positive_description = Description("virtual positive description")
+    negative_description = Description("virtual negative description")
+    virtual = Virtual.YES
     priority = 1
 
 
@@ -127,6 +130,7 @@ class Rsv75RollingCohortFactory(IterationCohortFactory):
     cohort_group = CohortGroup("rsv_age_range")
     positive_description = Description("rsv_age_range positive description")
     negative_description = Description("rsv_age_range negative description")
+    virtual = Virtual.NO
     priority = 2
 
 
@@ -135,6 +139,7 @@ class Rsv75to79CohortFactory(IterationCohortFactory):
     cohort_group = CohortGroup("rsv_age_range")
     positive_description = Description("rsv_age_range positive description")
     negative_description = Description("rsv_age_range negative description")
+    virtual = Virtual.NO
     priority = 3
 
 
@@ -143,6 +148,7 @@ class RsvPretendClinicalCohortFactory(IterationCohortFactory):
     cohort_group = CohortGroup("rsv_clinical_cohort")
     positive_description = Description("rsv_clinical_cohort positive description")
     negative_description = Description("rsv_clinical_cohort negative description")
+    virtual = Virtual.NO
     priority = 4
 
 

tests/integration/conftest.py

@@ -788,7 +788,7 @@ def multiple_campaign_configs(s3_client: BaseClient, rules_bucket: BucketName) -
 
 
 @pytest.fixture(scope="class")
-def campaign_config_with_magic_cohort(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[CampaignConfig]:
+def campaign_config_with_virtual_cohort(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[CampaignConfig]:
     campaign: CampaignConfig = rule.CampaignConfigFactory.build(
         target="COVID",
         iterations=[
@@ -797,7 +797,7 @@ def campaign_config_with_magic_cohort(s3_client: BaseClient, rules_bucket: Bucke
                     rule.PostcodeSuppressionRuleFactory.build(type=RuleType.filter),
                     rule.PersonAgeSuppressionRuleFactory.build(),
                 ],
-                iteration_cohorts=[rule.MagicCohortFactory.build(cohort_label="elid_all_people")],
+                iteration_cohorts=[rule.VirtualCohortFactory.build(cohort_label="virtual_cohort")],
                 status_text=None,
             )
         ],

tests/integration/in_process/test_eligibility_endpoint.py

@@ -289,12 +289,12 @@ def test_actionable_with_and_rule(
         )
 
 
-class TestMagicCohortResponse:
-    def test_not_eligible_by_rule_when_only_magic_cohort_is_present(
+class TestVirtualCohortResponse:
+    def test_not_eligible_by_rule_when_only_virtual_cohort_is_present(
         self,
         client: FlaskClient,
         persisted_person_pc_sw19: NHSNumber,
-        campaign_config_with_magic_cohort: CampaignConfig,  # noqa: ARG002
+        campaign_config_with_virtual_cohort: CampaignConfig,  # noqa: ARG002
     ):
         # Given
 
@@ -317,9 +317,9 @@ def test_not_eligible_by_rule_when_only_magic_cohort_is_present(
                                     "status": "NotEligible",
                                     "eligibilityCohorts": [
                                         {
-                                            "cohortCode": "magic cohort group",
+                                            "cohortCode": "virtual cohort group",
                                             "cohortStatus": "NotEligible",
-                                            "cohortText": "magic negative description",
+                                            "cohortText": "virtual negative description",
                                         }
                                     ],
                                     "actions": [],
@@ -333,11 +333,11 @@ def test_not_eligible_by_rule_when_only_magic_cohort_is_present(
             ),
         )
 
-    def test_not_actionable_when_only_magic_cohort_is_present(
+    def test_not_actionable_when_only_virtual_cohort_is_present(
         self,
         client: FlaskClient,
         persisted_person: NHSNumber,
-        campaign_config_with_magic_cohort: CampaignConfig,  # noqa: ARG002
+        campaign_config_with_virtual_cohort: CampaignConfig,  # noqa: ARG002
     ):
         # Given
 
@@ -360,9 +360,9 @@ def test_not_actionable_when_only_magic_cohort_is_present(
                                     "status": "NotActionable",
                                     "eligibilityCohorts": [
                                         {
-                                            "cohortCode": "magic cohort group",
+                                            "cohortCode": "virtual cohort group",
                                             "cohortStatus": "NotActionable",
-                                            "cohortText": "magic positive description",
+                                            "cohortText": "virtual positive description",
                                         }
                                     ],
                                     "actions": [],
@@ -382,11 +382,11 @@ def test_not_actionable_when_only_magic_cohort_is_present(
             ),
         )
 
-    def test_actionable_when_only_magic_cohort_is_present(
+    def test_actionable_when_only_virtual_cohort_is_present(
         self,
         client: FlaskClient,
         persisted_77yo_person: NHSNumber,
-        campaign_config_with_magic_cohort: CampaignConfig,  # noqa: ARG002
+        campaign_config_with_virtual_cohort: CampaignConfig,  # noqa: ARG002
     ):
         # Given
 
@@ -409,9 +409,9 @@ def test_actionable_when_only_magic_cohort_is_present(
                                     "status": "Actionable",
                                     "eligibilityCohorts": [
                                         {
-                                            "cohortCode": "magic cohort group",
+                                            "cohortCode": "virtual cohort group",
                                             "cohortStatus": "Actionable",
-                                            "cohortText": "magic positive description",
+                                            "cohortText": "virtual positive description",
                                         }
                                     ],
                                     "actions": [