Eli 615 : fix - multi campaign target collision (#593)

* ELI-615 | campaign having recent - active start_date supersedes the others sharing same best-status

* ELI-615 | more linting

* ELI-615 | revert commit

* ELI-615 | wip

* ELI-615 | wip

* ELI-615 | wip

* Bump werkzeug from 3.1.5 to 3.1.6

Bumps [werkzeug](https://github.com/pallets/werkzeug) from 3.1.5 to 3.1.6.
- [Release notes](https://github.com/pallets/werkzeug/releases)
- [Changelog](https://github.com/pallets/werkzeug/blob/main/CHANGES.rst)
- [Commits](pallets/werkzeug@3.1.5...3.1.6)

---
updated-dependencies:
- dependency-name: werkzeug
  dependency-version: 3.1.6
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

* Updated not_member_of operator to NotMemberOf (#594)

* Added vulture to workflows (#585)

* Added vulture to workflows

* Added new make commands and added to project

* Added updated lockfile

* Minimal config with no errors

* Corrected vulture commands

* Generating new lock file

* ELI-615 | modified iterations_result to iteration result

* ELI-615 | fix - naming issues | handle stop iter exception

* ELI-615 | campaign_configs - fixture updated | test case fixed

* ELI-615 | fix flaky tests do to fixture scope

* ELI-615 | fix flaky tests - removed best status test

* ELI-615 | used raw campagin config for tests using iteration dates

* ELI-615 | fix - campaign group is used correctly

* ELI-615 | fix test_campaigns_grouped_by_condition_name_filters_correctly

* ELI-615 | fix tests

* ELI-615 | linting

* ELI-615 | renamed best_iteration_result to iteration_result_summary

* ELI-615 | add more test cases - it tests

* ELI-615 | test commit - try git leaks ignore

* ELI-615 | incorporated review comments

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: oneeb-nhs <258801025+oneeb-nhs@users.noreply.github.com>
Co-authored-by: Robert Bailiff <rob.bailiff1@nhs.net>

Author: 4 people

.gitleaksignore

@@ -1,3 +1,5 @@
 # SEE: https://github.com/gitleaks/gitleaks/blob/master/README.md#gitleaksignore
 
 cd9c0efec38c5d63053dd865e5d4e207c0760d91:docs/guides/Perform_static_analysis.md:generic-api-key:37
+
+bf0c77098978c450d8570b38fb480fbb8d6a0628:.github/instructions/*.instructions.md:stripe-access-token:140

src/eligibility_signposting_api/audit/audit_context.py

@@ -19,10 +19,10 @@
 )
 from eligibility_signposting_api.audit.audit_service import AuditService
 from eligibility_signposting_api.model.eligibility_status import (
-    BestIterationResult,
     CohortGroupResult,
     ConditionName,
     IterationResult,
+    IterationResultSummary,
     MatchedActionDetail,
     Reason,
     RuleType,
@@ -63,13 +63,13 @@ def add_request_details(request: Request) -> None:
     @staticmethod
     def append_audit_condition(
         condition_name: ConditionName,
-        best_iteration_result: BestIterationResult,
+        iteration_result_summary: IterationResultSummary,
         action_detail: MatchedActionDetail,
     ) -> None:
         audit_eligibility_cohorts, audit_eligibility_cohort_groups, audit_actions = [], [], []
-        best_active_iteration = best_iteration_result.active_iteration
-        best_candidate = best_iteration_result.iteration_result
-        best_cohort_results = best_iteration_result.cohort_results
+        best_active_iteration = iteration_result_summary.active_iteration
+        best_candidate = iteration_result_summary.iteration_result
+        best_cohort_results = iteration_result_summary.cohort_results
         filter_audit_rules, suitability_audit_rules = [], []
 
         if best_cohort_results:
@@ -94,8 +94,8 @@ def append_audit_condition(
         audit_actions = AuditContext.create_audit_actions(action_detail.actions)
 
         audit_condition = AuditCondition(
-            campaign_id=best_iteration_result.campaign_id,
-            campaign_version=best_iteration_result.campaign_version,
+            campaign_id=iteration_result_summary.campaign_id,
+            campaign_version=iteration_result_summary.campaign_version,
             iteration_id=best_active_iteration.id if best_active_iteration else None,
             iteration_version=best_active_iteration.version if best_active_iteration else None,
             condition_name=condition_name,

src/eligibility_signposting_api/model/eligibility_status.py

@@ -138,7 +138,7 @@ class IterationResult:
 
 
 @dataclass
-class BestIterationResult:
+class IterationResultSummary:
     iteration_result: IterationResult
     active_iteration: Iteration | None = None
     campaign_id: CampaignID | None = None

src/eligibility_signposting_api/services/calculators/eligibility_calculator.py

@@ -11,12 +11,12 @@
 from eligibility_signposting_api.audit.audit_context import AuditContext
 from eligibility_signposting_api.model import campaign_config, eligibility_status
 from eligibility_signposting_api.model.eligibility_status import (
-    BestIterationResult,
     CohortGroupResult,
     Condition,
     ConditionName,
     EligibilityStatus,
     IterationResult,
+    IterationResultSummary,
     Reason,
     Status,
     StatusText,
@@ -32,7 +32,6 @@
     from eligibility_signposting_api.model.campaign_config import (
         CampaignConfig,
         CohortLabel,
-        IterationName,
     )
     from eligibility_signposting_api.model.person import Person
 
@@ -81,31 +80,32 @@ def get_the_best_cohort_memberships(
 
         return best_status, best_cohorts
 
-    def get_eligibility_status(self, include_actions: str, conditions: list[str], category: str) -> EligibilityStatus:
+    def get_eligibility_status(
+        self, include_actions: str, conditions: list[str], requested_category: str
+    ) -> EligibilityStatus:
         include_actions_flag = include_actions.upper() == "Y"
         condition_results: dict[ConditionName, IterationResult] = {}
         final_result = []
 
-        requested_grouped_campaigns = self.campaign_evaluator.get_requested_grouped_campaigns(
-            self.campaign_configs, conditions, category
+        requested_cc_with_active_iteration = (
+            self.campaign_evaluator.get_campaign_with_latest_active_iteration_per_target(
+                self.campaign_configs, conditions, requested_category
+            )
         )
-        for condition_name, campaign_group in requested_grouped_campaigns:
-            best_iteration_result = self.get_best_iteration_result(campaign_group)
-
-            if best_iteration_result is None:
-                continue
+        for condition_name, campaign in requested_cc_with_active_iteration:
+            iteration_result_summary = self.evaluate_iteration_result_summary(campaign)
 
             matched_action_detail = self.action_rule_handler.get_actions(
                 self.person,
-                best_iteration_result.active_iteration,
-                best_iteration_result.iteration_result,
+                iteration_result_summary.active_iteration,
+                iteration_result_summary.iteration_result,
                 include_actions_flag=include_actions_flag,
             )
 
-            best_iteration_result = TokenProcessor.find_and_replace_tokens(self.person, best_iteration_result)
+            iteration_result_summary = TokenProcessor.find_and_replace_tokens(self.person, iteration_result_summary)
             matched_action_detail = TokenProcessor.find_and_replace_tokens(self.person, matched_action_detail)
 
-            condition_results[condition_name] = best_iteration_result.iteration_result
+            condition_results[condition_name] = iteration_result_summary.iteration_result
             condition_results[condition_name].actions = matched_action_detail.actions
 
             condition: Condition = self.build_condition(
@@ -116,54 +116,34 @@ def get_eligibility_status(self, include_actions: str, conditions: list[str], ca
 
             AuditContext.append_audit_condition(
                 condition_name,
-                best_iteration_result,
+                iteration_result_summary,
                 matched_action_detail,
             )
 
         # Consolidate all the results and return
         return eligibility_status.EligibilityStatus(conditions=final_result)
 
-    def get_best_iteration_result(self, campaign_group: list[CampaignConfig]) -> BestIterationResult | None:
-        iteration_results = self.get_iteration_results(campaign_group)
-
-        if not iteration_results:
-            return None
-
-        (_best_iteration_name, best_iteration_result) = max(
-            iteration_results.items(),
-            key=lambda item: next(iter(item[1].cohort_results.values())).status.value
-            # Below handles the case where there are no cohort results
-            if item[1].cohort_results
-            else -1,
+    def evaluate_iteration_result_summary(
+        self, campaign_with_active_iteration: CampaignConfig
+    ) -> IterationResultSummary:
+        active_iteration = campaign_with_active_iteration.current_iteration
+        cohort_results: dict[CohortLabel, CohortGroupResult] = self.rule_processor.get_cohort_group_results(
+            self.person, active_iteration
         )
 
-        return best_iteration_result
-
-    def get_iteration_results(self, campaign_group: list[CampaignConfig]) -> dict[IterationName, BestIterationResult]:
-        iteration_results: dict[IterationName, BestIterationResult] = {}
-
-        for cc in campaign_group:
-            try:
-                active_iteration = cc.current_iteration
-            except StopIteration:
-                logger.info("Skipping campaign ID %s as no active iteration was found.", cc.id)
-                continue
-            cohort_results: dict[CohortLabel, CohortGroupResult] = self.rule_processor.get_cohort_group_results(
-                self.person, active_iteration
-            )
-
-            # Determine Result between cohorts - get the best
-            status, best_cohorts = self.get_the_best_cohort_memberships(cohort_results)
-            status_text = self.get_status_text(active_iteration.status_text, ConditionName(cc.target), status)
+        # Determine Result between cohorts - get the best
+        status, best_cohorts = self.get_the_best_cohort_memberships(cohort_results)
+        status_text = self.get_status_text(
+            active_iteration.status_text, ConditionName(campaign_with_active_iteration.target), status
+        )
 
-            iteration_results[active_iteration.name] = BestIterationResult(
-                IterationResult(status, status_text, best_cohorts, []),
-                active_iteration,
-                cc.id,
-                cc.version,
-                cohort_results,
-            )
-        return iteration_results
+        return IterationResultSummary(
+            IterationResult(status, status_text, best_cohorts, []),
+            active_iteration,
+            campaign_with_active_iteration.id,
+            campaign_with_active_iteration.version,
+            cohort_results,
+        )
 
     @staticmethod
     def get_status_text(

src/eligibility_signposting_api/services/processors/action_rule_handler.py

@@ -27,14 +27,14 @@ def get_actions(
         self,
         person: Person,
         active_iteration: Iteration | None,
-        best_iteration_result: IterationResult,
+        iteration_result: IterationResult,
         *,
         include_actions_flag: bool,
     ) -> MatchedActionDetail:
         action_detail = MatchedActionDetail()
 
         if active_iteration is not None and include_actions_flag:
-            rule_type = best_iteration_result.status.get_action_rule_type()
+            rule_type = iteration_result.status.get_action_rule_type()
             action_detail = self._handle(person, active_iteration, rule_type)
 
         return action_detail

src/eligibility_signposting_api/services/processors/campaign_evaluator.py

@@ -1,3 +1,4 @@
+import logging
 from collections.abc import Collection, Iterator
 from itertools import groupby
 from operator import attrgetter
@@ -7,6 +8,8 @@
 from eligibility_signposting_api.model import eligibility_status
 from eligibility_signposting_api.model.campaign_config import CampaignConfig
 
+logger = logging.getLogger(__name__)
+
 
 @service
 class CampaignEvaluator:
@@ -15,29 +18,68 @@ class CampaignEvaluator:
     def get_active_campaigns(self, campaign_configs: Collection[CampaignConfig]) -> list[CampaignConfig]:
         return [cc for cc in campaign_configs if cc.campaign_live]
 
-    def get_requested_grouped_campaigns(
-        self, campaign_configs: Collection[CampaignConfig], conditions: list[str], category: str
-    ) -> Iterator[tuple[eligibility_status.ConditionName, list[CampaignConfig]]]:
+    def get_campaign_with_latest_iteration(self, active_campaigns: list[CampaignConfig]) -> CampaignConfig | None:
+        """
+        Returns the campaign with the latest active iteration date.
+
+        1. Collect all campaigns with an active iteration.
+        2. Sort by iteration date (descending).
+        3. Extract the lead campaign, throwing an error if a tie for the latest date exists.
+        """
+
+        valid_items = []
+
+        for cc in active_campaigns:
+            try:
+                valid_items.append((cc.current_iteration.iteration_date, cc))
+            except StopIteration:
+                logger.info(
+                    "Skipping campaign ID %s as no active iteration was found.",
+                    cc.id,
+                )
+
+        if not valid_items:
+            latest_campaign = None
+        else:
+            max_date = max(item[0] for item in valid_items)
+            cc_with_max_iteration_date: list[CampaignConfig] = [item[1] for item in valid_items if item[0] == max_date]
+            if len(cc_with_max_iteration_date) > 1:
+                err_msg = (
+                    f"Ambiguous result: '{len(cc_with_max_iteration_date)}' active iterations "
+                    f"for target {cc_with_max_iteration_date[0].target} "
+                    f"found for date '{max_date}' "
+                    f"across campaign(s) {[cc.id for cc in cc_with_max_iteration_date]}"
+                )
+                raise ValueError(err_msg)
+
+            latest_campaign = cc_with_max_iteration_date[0]
+
+        return latest_campaign
+
+    def get_campaign_with_latest_active_iteration_per_target(
+        self, campaign_configs: Collection[CampaignConfig], conditions: list[str], requested_category: str
+    ) -> Iterator[tuple[eligibility_status.ConditionName, CampaignConfig]]:
         mapping = {
             "ALL": {"V", "S"},
             "VACCINATIONS": {"V"},
             "SCREENING": {"S"},
         }
 
-        allowed_types = mapping.get(category, set())
+        allowed_types = mapping.get(requested_category, set())
 
         filter_all_conditions = "ALL" in conditions
 
-        active_campaigns = self.get_active_campaigns(campaign_configs)
+        allowed_campaigns = [c for c in campaign_configs if c.type in allowed_types]
+        active_campaigns = self.get_active_campaigns(allowed_campaigns)
 
         for condition_name, campaign_group in groupby(
             sorted(active_campaigns, key=attrgetter("target")),
             key=attrgetter("target"),
         ):
-            campaigns = list(campaign_group)
-            if (
-                campaigns
-                and campaigns[0].type in allowed_types
-                and (filter_all_conditions or str(condition_name) in conditions)
-            ):
-                yield condition_name, campaigns
+            filtered_campaigns = [
+                c for c in campaign_group if filter_all_conditions or str(condition_name) in conditions
+            ]
+
+            campaign = self.get_campaign_with_latest_iteration(filtered_campaigns)
+            if campaign is not None:
+                yield (condition_name, campaign)