Skip to content

Commit 7d63860

Browse files
authored
ELI-399: Fixing Future Iteration.StartDate Resulting in 500 Error (#282)
* ELI-399: Fixing Future Iteration.StartDate Resulting in 500 Error * ELI-399: Adds empty rules to fix flakiness
1 parent 3ba0f4a commit 7d63860

2 files changed

Lines changed: 110 additions & 27 deletions

File tree

src/eligibility_signposting_api/services/calculators/eligibility_calculator.py

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import logging
34
from collections import defaultdict
45
from dataclasses import dataclass, field
56
from typing import TYPE_CHECKING
@@ -31,6 +32,8 @@
3132
)
3233
from eligibility_signposting_api.model.person import Person
3334

35+
logger = logging.getLogger(__name__)
36+
3437

3538
@service
3639
class EligibilityCalculatorFactory:
@@ -84,6 +87,9 @@ def get_eligibility_status(self, include_actions: str, conditions: list[str], ca
8487
for condition_name, campaign_group in requested_grouped_campaigns:
8588
best_iteration_result = self.get_best_iteration_result(campaign_group)
8689

90+
if best_iteration_result is None:
91+
continue
92+
8793
matched_action_detail = self.action_rule_handler.get_actions(
8894
self.person,
8995
best_iteration_result.active_iteration,
@@ -107,28 +113,31 @@ def get_eligibility_status(self, include_actions: str, conditions: list[str], ca
107113
# Consolidate all the results and return
108114
return eligibility_status.EligibilityStatus(conditions=final_result)
109115

110-
def get_best_iteration_result(self, campaign_group: list[CampaignConfig]) -> BestIterationResult:
116+
def get_best_iteration_result(self, campaign_group: list[CampaignConfig]) -> BestIterationResult | None:
111117
iteration_results = self.get_iteration_results(campaign_group)
112118

113-
if iteration_results:
114-
(best_iteration_name, best_iteration_result) = max(
115-
iteration_results.items(),
116-
key=lambda item: next(iter(item[1].cohort_results.values())).status.value
117-
# Below handles the case where there are no cohort results
118-
if item[1].cohort_results
119-
else -1,
120-
)
121-
else:
122-
iteration_result = IterationResult(eligibility_status.Status.not_eligible, [], [])
123-
best_iteration_result = BestIterationResult(iteration_result, None, None, None, {})
119+
if not iteration_results:
120+
return None
121+
122+
(best_iteration_name, best_iteration_result) = max(
123+
iteration_results.items(),
124+
key=lambda item: next(iter(item[1].cohort_results.values())).status.value
125+
# Below handles the case where there are no cohort results
126+
if item[1].cohort_results
127+
else -1,
128+
)
124129

125130
return best_iteration_result
126131

127132
def get_iteration_results(self, campaign_group: list[CampaignConfig]) -> dict[IterationName, BestIterationResult]:
128133
iteration_results: dict[IterationName, BestIterationResult] = {}
129134

130135
for cc in campaign_group:
131-
active_iteration = cc.current_iteration
136+
try:
137+
active_iteration = cc.current_iteration
138+
except StopIteration:
139+
logger.info("Skipping campaign ID %s as no active iteration was found.", cc.id)
140+
continue
132141
cohort_results: dict[CohortLabel, CohortGroupResult] = self.rule_processor.get_cohort_group_results(
133142
self.person, active_iteration
134143
)

tests/unit/services/calculators/test_eligibility_calculator.py

Lines changed: 88 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
import datetime
2+
import logging
23
from typing import Any
34

45
import pytest
56
from faker import Faker
67
from flask import Flask
78
from freezegun import freeze_time
89
from hamcrest import assert_that, contains_exactly, contains_inanyorder, has_item, has_items, is_, is_in
9-
from pydantic import HttpUrl
1010

1111
from eligibility_signposting_api.model import campaign_config as rules_model
1212
from eligibility_signposting_api.model import eligibility_status
1313
from eligibility_signposting_api.model.campaign_config import (
14-
AvailableAction,
1514
CohortLabel,
1615
Description,
1716
RuleAttributeLevel,
@@ -585,19 +584,94 @@ def test_cohort_group_descriptions_are_selected_based_on_priority_when_cohorts_h
585584
)
586585

587586

588-
book_nbs_comms = AvailableAction(
589-
ActionType="ButtonAuthLink",
590-
ExternalRoutingCode="BookNBS",
591-
ActionDescription="Action description",
592-
UrlLink=HttpUrl("https://www.nhs.uk/book-rsv"),
593-
UrlLabel="Continue to booking",
594-
)
587+
@freeze_time("2025-04-25")
588+
def test_no_active_iteration_returns_empty_conditions_with_single_active_campaign(faker: Faker):
589+
# Given
590+
person_rows = person_rows_builder(NHSNumber(faker.nhs_number()))
591+
campaign_configs = [
592+
rule_builder.CampaignConfigFactory.build(
593+
target="RSV",
594+
iterations=[
595+
rule_builder.IterationFactory.build(
596+
name="inactive iteration",
597+
iteration_rules=[],
598+
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
599+
)
600+
],
601+
)
602+
]
603+
# Need to set the iteration date to override CampaignConfigFactory.fix_iteration_date_invariants behavior
604+
campaign_configs[0].iterations[0].iteration_date = datetime.date(2025, 5, 10)
595605

596-
default_comms_detail = AvailableAction(
597-
ActionType="CareCardWithText",
598-
ExternalRoutingCode="BookLocal",
599-
ActionDescription="You can get an RSV vaccination at your GP surgery",
600-
)
606+
calculator = EligibilityCalculator(person_rows, campaign_configs)
607+
608+
# When
609+
actual = calculator.get_eligibility_status("Y", ["ALL"], "ALL")
610+
611+
# Then
612+
assert_that(actual, is_eligibility_status().with_conditions([]))
613+
614+
615+
@pytest.mark.usefixtures("caplog")
616+
@freeze_time("2025-04-25")
617+
def test_returns_no_condition_data_for_campaign_without_active_iteration(faker: Faker, caplog):
618+
# Given
619+
person_rows = person_rows_builder(NHSNumber(faker.nhs_number()))
620+
campaign_configs = [
621+
rule_builder.CampaignConfigFactory.build(
622+
target="RSV",
623+
iterations=[
624+
rule_builder.IterationFactory.build(
625+
name="inactive iteration",
626+
iteration_rules=[],
627+
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
628+
)
629+
],
630+
),
631+
rule_builder.CampaignConfigFactory.build(
632+
target="COVID",
633+
iterations=[
634+
rule_builder.IterationFactory.build(
635+
name="active iteration",
636+
iteration_rules=[],
637+
iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")],
638+
)
639+
],
640+
),
641+
]
642+
# Need to set the iteration date to override CampaignConfigFactory.fix_iteration_date_invariants behavior
643+
rsv_campaign = campaign_configs[0]
644+
rsv_campaign.iterations[0].iteration_date = datetime.date(2025, 5, 10)
645+
646+
calculator = EligibilityCalculator(person_rows, campaign_configs)
647+
648+
# When
649+
with caplog.at_level(logging.INFO):
650+
actual = calculator.get_eligibility_status("Y", ["ALL"], "ALL")
651+
652+
# Then
653+
condition_names = [condition.condition_name for condition in actual.conditions]
654+
655+
assert ConditionName("RSV") not in condition_names
656+
assert ConditionName("COVID") in condition_names
657+
assert f"Skipping campaign ID {rsv_campaign.id} as no active iteration was found." in caplog.text
658+
659+
660+
@freeze_time("2025-04-25")
661+
def test_no_active_campaign(faker: Faker):
662+
# Given
663+
person_rows = person_rows_builder(NHSNumber(faker.nhs_number()))
664+
campaign_configs = [rule_builder.CampaignConfigFactory.build()]
665+
# Need to set the campaign dates to override CampaignConfigFactory.fix_iteration_date_invariants behavior
666+
campaign_configs[0].start_date = datetime.date(2025, 5, 10)
667+
668+
calculator = EligibilityCalculator(person_rows, campaign_configs)
669+
670+
# When
671+
actual = calculator.get_eligibility_status("Y", ["ALL"], "ALL")
672+
673+
# Then
674+
assert_that(actual, is_eligibility_status().with_conditions([]))
601675

602676

603677
class TestEligibilityResultBuilder:

0 commit comments

Comments
 (0)