Suitability results deduplication + eligibility cohorts build fix (#131)

* Response - add only cohort matching best-status

* Suitability rules deduplication

Author: Karthikeyannhs

src/eligibility_signposting_api/views/eligibility.py

@@ -69,26 +69,14 @@ def build_eligibility_response(eligibility_status: EligibilityStatus) -> eligibi
     )
 
 
-def build_suitability_results(condition: Condition) -> list[eligibility.SuitabilityRule]:
-    return [
-        eligibility.SuitabilityRule(
-            ruleType=eligibility.RuleType(reason.rule_type.value),
-            ruleCode=eligibility.RuleCode(reason.rule_name),
-            ruleText=eligibility.RuleText(reason.rule_result),
-        )
-        for cohort_result in condition.cohort_results
-        for reason in cohort_result.reasons
-        if condition.status == Status.not_actionable
-    ]
-
-
 def build_eligibility_cohorts(condition: Condition) -> list[eligibility.EligibilityCohort]:
     """Group Iteration cohorts and make only one entry per cohort group"""
 
     grouped_cohort_results = defaultdict(list)
 
     for cohort_result in condition.cohort_results:
-        grouped_cohort_results[cohort_result.cohort_code].append(cohort_result)
+        if condition.status == cohort_result.status:
+            grouped_cohort_results[cohort_result.cohort_code].append(cohort_result)
 
     return [
         eligibility.EligibilityCohort(
@@ -99,3 +87,26 @@ def build_eligibility_cohorts(condition: Condition) -> list[eligibility.Eligibil
         for cohort_group_code, cohort_group in grouped_cohort_results.items()
         if cohort_group
     ]
+
+
+def build_suitability_results(condition: Condition) -> list[eligibility.SuitabilityRule]:
+    if not condition.status.not_actionable:
+        return []
+
+    unique_rule_codes = set()
+    suitability_results = []
+
+    for cohort_result in condition.cohort_results:
+        if cohort_result.status.not_actionable:
+            for reason in cohort_result.reasons:
+                if reason.rule_name not in unique_rule_codes:
+                    unique_rule_codes.add(reason.rule_name)
+                    suitability_results.append(
+                        eligibility.SuitabilityRule(
+                            ruleType=eligibility.RuleType(reason.rule_type.value),
+                            ruleCode=eligibility.RuleCode(reason.rule_name),
+                            ruleText=eligibility.RuleText(reason.rule_result),
+                        )
+                    )
+
+    return suitability_results

tests/fixtures/builders/model/eligibility.py

@@ -4,7 +4,7 @@
 from polyfactory import Use
 from polyfactory.factories import DataclassFactory
 
-from eligibility_signposting_api.model.eligibility import Condition, EligibilityStatus
+from eligibility_signposting_api.model.eligibility import CohortResult, Condition, EligibilityStatus
 
 
 class ConditionFactory(DataclassFactory[Condition]): ...
@@ -14,5 +14,8 @@ class EligibilityStatusFactory(DataclassFactory[EligibilityStatus]):
     condition = Use(ConditionFactory.batch, size=2)
 
 
+class CohortResultFactory(DataclassFactory[CohortResult]): ...
+
+
 def random_str(length: int) -> str:
     return "".join(random.choice(string.ascii_lowercase + string.digits) for _ in range(length))

tests/fixtures/matchers/eligibility.py

@@ -1,7 +1,7 @@
 from hamcrest.core.matcher import Matcher
 
 from eligibility_signposting_api.model.eligibility import CohortResult, Condition, EligibilityStatus, Reason
-from eligibility_signposting_api.model.rules import IterationCohort
+from eligibility_signposting_api.views.response_model.eligibility import EligibilityCohort, SuitabilityRule
 
 from .meta import BaseAutoMatcher
 
@@ -18,7 +18,10 @@ class CohortResultMatcher(BaseAutoMatcher[CohortResult]): ...
 class ReasonMatcher(BaseAutoMatcher[Reason]): ...
 
 
-class IterationCohortMatcher(BaseAutoMatcher[IterationCohort]): ...
+class EligibilityCohortMatcher(BaseAutoMatcher[EligibilityCohort]): ...
+
+
+class SuitabilityRuleMatcher(BaseAutoMatcher[SuitabilityRule]): ...
 
 
 def is_eligibility_status() -> Matcher[EligibilityStatus]:
@@ -37,5 +40,9 @@ def is_reason() -> Matcher[Reason]:
     return ReasonMatcher()
 
 
-def is_iteration_cohort() -> Matcher[IterationCohort]:
-    return IterationCohortMatcher()
+def is_eligibility_cohort() -> Matcher[EligibilityCohort]:
+    return EligibilityCohortMatcher()
+
+
+def is_suitability_rule() -> Matcher[SuitabilityRule]:
+    return SuitabilityRuleMatcher()

tests/unit/views/test_eligibility.py

@@ -5,12 +5,26 @@
 from brunns.matchers.werkzeug import is_werkzeug_response as is_response
 from flask import Flask
 from flask.testing import FlaskClient
-from hamcrest import assert_that, contains_exactly, has_entries, has_key
+from hamcrest import assert_that, contains_exactly, has_entries, has_key, has_length
 from wireup.integration.flask import get_app_container
 
-from eligibility_signposting_api.model.eligibility import EligibilityStatus, NHSNumber
+from eligibility_signposting_api.model.eligibility import (
+    Condition,
+    EligibilityStatus,
+    NHSNumber,
+    Reason,
+    RuleName,
+    RuleResult,
+    RuleType,
+    Status,
+)
 from eligibility_signposting_api.services import EligibilityService, UnknownPersonError
-from tests.fixtures.builders.model.eligibility import EligibilityStatusFactory
+from eligibility_signposting_api.views.eligibility import (
+    build_eligibility_cohorts,
+    build_suitability_results,
+)
+from tests.fixtures.builders.model.eligibility import CohortResultFactory, ConditionFactory, EligibilityStatusFactory
+from tests.fixtures.matchers.eligibility import is_eligibility_cohort, is_suitability_rule
 
 logger = logging.getLogger(__name__)
 
@@ -95,3 +109,79 @@ def test_unexpected_error(app: Flask, client: FlaskClient):
                 )
             ),
         )
+
+
+def test_build_eligibility_cohorts_results_consider_only_cohorts_with_best_status():
+    condition: Condition = ConditionFactory.build(
+        status=Status.not_actionable,
+        cohort_results=[
+            CohortResultFactory.build(
+                cohort_code="cohort_group1",
+                status=Status.not_actionable,
+            ),
+            CohortResultFactory.build(
+                cohort_code="cohort_group2",
+                status=Status.not_eligible,
+            ),
+        ],
+    )
+
+    results = build_eligibility_cohorts(condition)
+
+    assert_that(
+        results,
+        contains_exactly(is_eligibility_cohort().with_cohort_code("cohort_group1").and_cohort_status("NotActionable")),
+    )
+
+
+def test_build_suitability_results_with_deduplication():
+    condition: Condition = ConditionFactory.build(
+        status=Status.not_actionable,
+        cohort_results=[
+            CohortResultFactory.build(
+                cohort_code="cohort_group1",
+                status=Status.not_actionable,
+                reasons=[
+                    Reason(
+                        rule_type=RuleType.suppression,
+                        rule_name=RuleName("Exclude too young less than 75"),
+                        rule_result=RuleResult("Age < 75"),
+                    ),
+                    Reason(
+                        rule_type=RuleType.suppression,
+                        rule_name=RuleName("Exclude more than 100"),
+                        rule_result=RuleResult("Age > 100"),
+                    ),
+                ],
+            ),
+            CohortResultFactory.build(
+                cohort_code="cohort_group2",
+                status=Status.not_actionable,
+                reasons=[
+                    Reason(
+                        rule_type=RuleType.suppression,
+                        rule_name=RuleName("Exclude too young less than 75"),
+                        rule_result=RuleResult("Age < 75"),
+                    )
+                ],
+            ),
+        ],
+    )
+
+    results = build_suitability_results(condition)
+
+    assert_that(
+        results,
+        contains_exactly(
+            is_suitability_rule().with_rule_code("Exclude too young less than 75").and_rule_text("Age < 75"),
+            is_suitability_rule().with_rule_code("Exclude more than 100").and_rule_text("Age > 100"),
+        ),
+    )
+
+
+def test_no_suitability_rules_for_actionable():
+    condition = ConditionFactory.build(status=Status.actionable, cohort_results=[])
+
+    results = build_suitability_results(condition)
+
+    assert_that(results, has_length(0))