From 7860aa44b7041e7800839876b14a513a8f37ee95 Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Mon, 11 Aug 2025 10:32:34 +0100 Subject: [PATCH 1/2] ELI-399: Fixing start date validation --- .../model/campaign_config.py | 15 ------ .../validators/campaign_config_validator.py | 20 +++++++- tests/integration/conftest.py | 47 ++++++++++++++++++- .../lambda/test_app_running_as_lambda.py | 38 +++++++++++++++ tests/unit/model/test_campaign_config.py | 14 ------ 5 files changed, 103 insertions(+), 31 deletions(-) diff --git a/src/eligibility_signposting_api/model/campaign_config.py b/src/eligibility_signposting_api/model/campaign_config.py index cc67fbc98..989f2e53d 100644 --- a/src/eligibility_signposting_api/model/campaign_config.py +++ b/src/eligibility_signposting_api/model/campaign_config.py @@ -228,21 +228,6 @@ def check_no_overlapping_iterations(self) -> typing.Self: raise ValueError(message) return self - @model_validator(mode="after") - def check_has_iteration_from_start(self) -> typing.Self: - iterations_by_date = sorted(self.iterations, key=attrgetter("iteration_date")) - if first_iteration := next(iter(iterations_by_date), None): - if first_iteration.iteration_date > self.start_date: - message = ( - f"campaign {self.id} starts on {self.start_date}, " - f"1st iteration starts later - {first_iteration.iteration_date}" - ) - raise ValueError(message) - return self - # Should never happen, since we are constraining self.iterations with a min_length of 1 - message = f"campaign {self.id} has no iterations." - raise ValueError(message) - @cached_property def campaign_live(self) -> bool: today = datetime.now(tz=UTC).date() diff --git a/src/rules_validation_api/validators/campaign_config_validator.py b/src/rules_validation_api/validators/campaign_config_validator.py index f25d740ff..61e7df691 100644 --- a/src/rules_validation_api/validators/campaign_config_validator.py +++ b/src/rules_validation_api/validators/campaign_config_validator.py @@ -1,4 +1,7 @@ -from pydantic import field_validator +import typing +from operator import attrgetter + +from pydantic import field_validator, model_validator from eligibility_signposting_api.model.campaign_config import CampaignConfig, Iteration from rules_validation_api.validators.iteration_validator import IterationValidation @@ -9,3 +12,18 @@ class CampaignConfigValidation(CampaignConfig): @field_validator("iterations") def validate_iterations(cls, iterations: list[Iteration]) -> list[IterationValidation]: return [IterationValidation(**i.model_dump()) for i in iterations] + + @model_validator(mode="after") + def check_has_iteration_from_start(self) -> typing.Self: + iterations_by_date = sorted(self.iterations, key=attrgetter("iteration_date")) + if first_iteration := next(iter(iterations_by_date), None): + if first_iteration.iteration_date > self.start_date: + message = ( + f"campaign {self.id} starts on {self.start_date}, " + f"1st iteration starts later - {first_iteration.iteration_date}" + ) + raise ValueError(message) + return self + # Should never happen, since we are constraining self.iterations with a min_length of 1 + message = f"campaign {self.id} has no iterations." + raise ValueError(message) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index f324245db..f2c7913e1 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -1,3 +1,4 @@ +import datetime import json import logging import os @@ -19,7 +20,9 @@ from eligibility_signposting_api.model import eligibility_status from eligibility_signposting_api.model.campaign_config import ( CampaignConfig, + EndDate, RuleType, + StartDate, ) from eligibility_signposting_api.repos.campaign_repo import BucketName from eligibility_signposting_api.repos.person_repo import TableName @@ -380,7 +383,7 @@ def persisted_person_all_cohorts(person_table: Any, faker: Faker) -> Generator[e nhs_number, date_of_birth=date_of_birth, postcode="SW19", - cohorts=["cohort_label1", "cohort_label2", "cohort_label3", "cohort_label4"], + cohorts=["cohort_label1", "cohort_label2", "cohort_label3", "cohort_label4", "cohort_label5"], icb="QE1", ).data ): @@ -484,6 +487,48 @@ def campaign_config(s3_client: BaseClient, rules_bucket: BucketName) -> Generato s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") +@pytest.fixture(scope="class") +def inactive_iteration_config(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[list[CampaignConfig]]: + campaigns, campaign_data_keys = [], [] + + target_iteration_dates = { + "start_date": ("RSV", datetime.date(2025, 1, 1)), # Active Iteration Date + "start_date_plus_one_day": ("COVID", datetime.date(2025, 1, 2)), # Active Iteration Date + "today": ("FLU", datetime.date(2025, 8, 8)), # Active Iteration Date + "tomorrow": ("MMR", datetime.date(2025, 8, 9)), # Inactive Iteration Date + } + + for target, data in target_iteration_dates.items(): + campaign = rule.CampaignConfigFactory.build( + id=f"campaign_{target}", + target=data[0], + type="V", + iterations=[ + rule.IterationFactory.build( + iteration_rules=[rule.PersonAgeSuppressionRuleFactory.build()], + iteration_cohorts=[rule.IterationCohortFactory.build(cohort_label="cohort_label1")], + ) + ], + ) + + campaign.start_date = StartDate(datetime.date(2025, 1, 1)) + campaign.end_date = EndDate(datetime.date(2026, 1, 1)) + campaign.iterations[0].iteration_date = data[1] + + campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} + key = f"{campaign.name}.json" + s3_client.put_object( + Bucket=rules_bucket, Key=key, Body=json.dumps(campaign_data), ContentType="application/json" + ) + campaigns.append(campaign) + campaign_data_keys.append(key) + + yield campaigns + + for key in campaign_data_keys: + s3_client.delete_object(Bucket=rules_bucket, Key=key) + + @pytest.fixture(scope="class") def multiple_campaign_configs(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[list[CampaignConfig]]: """Create and upload multiple campaign configs to S3, then clean up after tests.""" diff --git a/tests/integration/lambda/test_app_running_as_lambda.py b/tests/integration/lambda/test_app_running_as_lambda.py index b4cb61744..f4dcfc95e 100644 --- a/tests/integration/lambda/test_app_running_as_lambda.py +++ b/tests/integration/lambda/test_app_running_as_lambda.py @@ -10,6 +10,7 @@ from brunns.matchers.data import json_matching as is_json_that from brunns.matchers.response import is_response from faker import Faker +from freezegun import freeze_time from hamcrest import ( assert_that, contains_exactly, @@ -540,3 +541,40 @@ def test_given_person_has_unique_status_for_different_conditions_with_audit( # assert_that(audit_data["response"]["responseId"], is_not(equal_to(""))) assert_that(audit_data["response"]["lastUpdated"], is_not(equal_to(""))) assert_that(audit_data["response"]["condition"], contains_inanyorder(*expected_conditions)) + + +@freeze_time("2025-08-08") +def test_no_active_iteration_returns_empty_processed_suggestions( + lambda_client: BaseClient, # noqa:ARG001 + persisted_person_all_cohorts: NHSNumber, + inactive_iteration_config: list[CampaignConfig], # noqa:ARG001 + api_gateway_endpoint: URL, +): + invoke_url = f"{api_gateway_endpoint}/patient-check/{persisted_person_all_cohorts}" + response = httpx.get( + invoke_url, + headers={ + "nhs-login-nhs-number": str(persisted_person_all_cohorts), + "x_request_id": "x_request_id", + "x_correlation_id": "x_correlation_id", + "nhsd_end_user_organisation_ods": "nhsd_end_user_organisation_ods", + "nhsd_application_id": "nhsd_application_id", + }, + params={"includeActions": "Y", "category": "VACCINATIONS", "conditions": "COVID,FLU,RSV"}, + timeout=10, + ) + + assert_that( + response, + is_response().with_status_code(HTTPStatus.OK).and_body(is_json_that(has_key("processedSuggestions"))), + ) + + body = response.json() + assert_that( + body["processedSuggestions"], + contains_inanyorder( + has_entries("condition", "COVID"), + has_entries("condition", "RSV"), + has_entries("condition", "FLU"), + ), + ) diff --git a/tests/unit/model/test_campaign_config.py b/tests/unit/model/test_campaign_config.py index bab1aa102..dbeebc73f 100644 --- a/tests/unit/model/test_campaign_config.py +++ b/tests/unit/model/test_campaign_config.py @@ -52,20 +52,6 @@ def test_iteration_with_overlapping_start_dates_not_allowed(faker: Faker): RawCampaignConfigFactory.build(start_date=start_date, iterations=[iteration1, iteration2]) -def test_iteration_must_have_active_iteration_from_its_start(faker: Faker): - # Given - start_date = faker.date_object() - iteration = IterationFactory.build(iteration_date=start_date + relativedelta(days=1)) - - # When, Then - with pytest.raises( - ValueError, - match=r"1 validation error for CampaignConfig\n" - r".*1st iteration starts later", - ): - RawCampaignConfigFactory.build(start_date=start_date, iterations=[iteration]) - - @pytest.mark.parametrize( ("rule_stop", "expected"), [ From a0302ac77dd111b9c4e9049132a6c10c6fc0f38e Mon Sep 17 00:00:00 2001 From: Shweta <216860557+shweta-nhs@users.noreply.github.com> Date: Mon, 11 Aug 2025 10:47:56 +0100 Subject: [PATCH 2/2] ELI-399: Fixing annotation --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index e271afb36..59d696121 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -529,6 +529,7 @@ def inactive_iteration_config(s3_client: BaseClient, rules_bucket: BucketName) - s3_client.delete_object(Bucket=rules_bucket, Key=key) +@pytest.fixture(scope="class") def campaign_config_with_and_rule(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[CampaignConfig]: campaign: CampaignConfig = rule.CampaignConfigFactory.build( target="RSV", @@ -565,7 +566,6 @@ def campaign_config_with_and_rule(s3_client: BaseClient, rules_bucket: BucketNam s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") - @pytest.fixture(scope="class") def multiple_campaign_configs(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[list[CampaignConfig]]: """Create and upload multiple campaign configs to S3, then clean up after tests."""