Skip to content

Commit 55bdb2d

Browse files
Karthikeyannhsdependabot[bot]oneeb-nhsrobbailiff2
committed
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>
1 parent 750a2c5 commit 55bdb2d

4 files changed

Lines changed: 109 additions & 11 deletions

File tree

‎src/eligibility_signposting_api/services/calculators/eligibility_calculator.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,6 @@ def get_eligibility_status(
9393
)
9494
)
9595
for condition_name, campaign in requested_cc_with_active_iteration:
96-
if campaign is None:
97-
continue # skipping as no active iteration was found.
98-
9996
iteration_result_summary = self.evaluate_iteration_result_summary(campaign)
10097

10198
matched_action_detail = self.action_rule_handler.get_actions(

‎src/eligibility_signposting_api/services/processors/campaign_evaluator.py‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ def get_campaign_with_latest_iteration(self, active_campaigns: list[CampaignConf
4545
cc_with_max_iteration_date: list[CampaignConfig] = [item[1] for item in valid_items if item[0] == max_date]
4646
if len(cc_with_max_iteration_date) > 1:
4747
err_msg = (
48-
f"Ambiguous result: {len(cc_with_max_iteration_date)} iterations "
49-
f"for target {cc_with_max_iteration_date[0].current_iteration.iteration_date}"
50-
f"found for date {max_date}"
48+
f"Ambiguous result: '{len(cc_with_max_iteration_date)}' active iterations "
49+
f"for target {cc_with_max_iteration_date[0].target} "
50+
f"found for date '{max_date}' "
51+
f"across campaign(s) {[cc.id for cc in cc_with_max_iteration_date]}"
5152
)
5253
raise ValueError(err_msg)
5354

@@ -57,7 +58,7 @@ def get_campaign_with_latest_iteration(self, active_campaigns: list[CampaignConf
5758

5859
def get_campaign_with_latest_active_iteration_per_target(
5960
self, campaign_configs: Collection[CampaignConfig], conditions: list[str], requested_category: str
60-
) -> Iterator[tuple[eligibility_status.ConditionName, CampaignConfig | None]]:
61+
) -> Iterator[tuple[eligibility_status.ConditionName, CampaignConfig]]:
6162
mapping = {
6263
"ALL": {"V", "S"},
6364
"VACCINATIONS": {"V"},
@@ -79,4 +80,6 @@ def get_campaign_with_latest_active_iteration_per_target(
7980
c for c in campaign_group if filter_all_conditions or str(condition_name) in conditions
8081
]
8182

82-
yield condition_name, self.get_campaign_with_latest_iteration(filtered_campaigns)
83+
campaign = self.get_campaign_with_latest_iteration(filtered_campaigns)
84+
if campaign is not None:
85+
yield (condition_name, campaign)

‎tests/integration/in_process/test_eligibility_endpoint.py‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,3 +1445,100 @@ def test_if_cc_with_latest_active_iteration_is_chosen_if_exists_multiple_campaig
14451445
assert_that(audit_data["response"]["condition"][0].get("campaignId"), equal_to(expected_campaign_id))
14461446
else:
14471447
assert_that(len(audit_data["response"]["condition"]), equal_to(0))
1448+
1449+
def test_if_multiple_active_iterations_with_same_iteration_datetime_for_the_same_target_throws_internal_error( # noqa: PLR0913
1450+
self,
1451+
client: FlaskClient,
1452+
persisted_person_pc_sw19: NHSNumber,
1453+
s3_client: BaseClient,
1454+
consumer_mapping_bucket: BucketName,
1455+
rules_bucket: BucketName,
1456+
secretsmanager_client: BaseClient, # noqa: ARG002
1457+
caplog,
1458+
):
1459+
# Given
1460+
consumer_id = "consumer-n3bs-jo4hn-ce4na"
1461+
headers = {"nhs-login-nhs-number": str(persisted_person_pc_sw19), UNIQUE_CONSUMER_HEADER: consumer_id}
1462+
1463+
# Consumer Mapping Data
1464+
s3_client.put_object(
1465+
Bucket=consumer_mapping_bucket,
1466+
Key="consumer_mapping_config.json",
1467+
Body=json.dumps(
1468+
{
1469+
consumer_id: [
1470+
{"CampaignConfigID": "RSV_campaign_id_1"},
1471+
{"CampaignConfigID": "RSV_campaign_id_2"},
1472+
],
1473+
}
1474+
),
1475+
ContentType="application/json",
1476+
)
1477+
previous_day = yesterday()
1478+
# Campaign configs
1479+
campaign_1 = rule.RawCampaignConfigFactory.build(
1480+
id="RSV_campaign_id_1",
1481+
target="RSV",
1482+
start_date=previous_day,
1483+
type="V",
1484+
iterations=[rule.IterationFactory.build(iteration_date=previous_day)],
1485+
)
1486+
1487+
campaign_2 = rule.RawCampaignConfigFactory.build(
1488+
id="RSV_campaign_id_2",
1489+
target="RSV",
1490+
start_date=previous_day,
1491+
type="V",
1492+
iterations=[rule.IterationFactory.build(iteration_date=previous_day)],
1493+
)
1494+
1495+
for campaign in [campaign_1, campaign_2]:
1496+
s3_client.put_object(
1497+
Bucket=rules_bucket,
1498+
Key=f"{campaign.id}.json",
1499+
Body=json.dumps({"CampaignConfig": campaign.model_dump(by_alias=True)}),
1500+
ContentType="application/json",
1501+
)
1502+
1503+
# When
1504+
response = client.get(f"/patient-check/{persisted_person_pc_sw19}", headers=headers)
1505+
1506+
assert_that(
1507+
response,
1508+
is_response()
1509+
.with_status_code(HTTPStatus.INTERNAL_SERVER_ERROR)
1510+
.with_headers(has_entries({"Content-Type": "application/fhir+json"}))
1511+
.and_text(
1512+
is_json_that(
1513+
has_entries(
1514+
resourceType="OperationOutcome",
1515+
issue=contains_exactly(
1516+
has_entries(
1517+
severity="error",
1518+
code="processing",
1519+
diagnostics="An unexpected error occurred.",
1520+
details={
1521+
"coding": [
1522+
{
1523+
"system": "https://fhir.nhs.uk/STU3/ValueSet/Spine-ErrorOrWarningCode-1",
1524+
"code": "INTERNAL_SERVER_ERROR",
1525+
"display": "An unexpected internal server error occurred.",
1526+
}
1527+
]
1528+
},
1529+
)
1530+
),
1531+
)
1532+
)
1533+
),
1534+
)
1535+
1536+
err_msg = (
1537+
"Ambiguous result: '2' active iterations "
1538+
"for target RSV "
1539+
f"found for date '{previous_day}' "
1540+
"across campaign(s) ['RSV_campaign_id_1', 'RSV_campaign_id_2']"
1541+
)
1542+
assert any(err_msg in message for message in caplog.messages), (
1543+
f"Expected log message not found. Logged messages: {caplog.messages}"
1544+
)

‎tests/unit/services/processors/test_campaign_evaluator.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,11 @@ def test_campaigns_grouped_by_condition_name_with_various_categories(
7979

8080
def test_campaigns_grouped_by_condition_name_with_empty_conditions_filter(campaign_evaluator):
8181
campaign = rule.CampaignConfigFactory.build(target="RSV", type="V")
82-
result = campaign_evaluator.get_campaign_with_latest_active_iteration_per_target([campaign], [], "VACCINATIONS")
82+
result = list(
83+
campaign_evaluator.get_campaign_with_latest_active_iteration_per_target([campaign], [], "VACCINATIONS")
84+
)
8385

84-
actual = next((name, camp) for name, camp in result)
85-
assert_that(actual, is_(("RSV", None)))
86+
assert_that(result, is_([]))
8687

8788

8889
def test_campaigns_grouped_by_condition_name_groups_multiple_campaigns_for_same_target(campaign_evaluator):

0 commit comments

Comments
 (0)