From 5496516ec9ce7207bcbef790dd7d76e2bfcbf2c5 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Tue, 25 Nov 2025 13:07:10 +0000 Subject: [PATCH 01/15] ELI 557 - Iteration Date must be >= Campaign Start Date AND <= Campaign End Date --- .../validators/campaign_config_validator.py | 16 ++++++++++------ .../validation/test_campaign_config_validator.py | 12 ++++++------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/rules_validation_api/validators/campaign_config_validator.py b/src/rules_validation_api/validators/campaign_config_validator.py index 94b007346..02f985ba7 100644 --- a/src/rules_validation_api/validators/campaign_config_validator.py +++ b/src/rules_validation_api/validators/campaign_config_validator.py @@ -14,15 +14,19 @@ def validate_iterations(cls, iterations: list[Iteration]) -> list[IterationValid return [IterationValidation(**i.model_dump()) for i in iterations] @model_validator(mode="after") - def check_has_iteration_from_start(self) -> typing.Self: + def validate_campaign_has_iteration_within_schedule(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}" + if first_iteration.iteration_date < self.start_date: + raise ValueError( + f"Iteration {first_iteration.id} starts before campaign {self.id} " + f"start date {self.start_date}." + ) + if first_iteration.iteration_date > self.end_date: + raise ValueError( + f"Iteration {first_iteration.id} starts after campaign {self.id} " + f"end date {self.end_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." diff --git a/tests/unit/validation/test_campaign_config_validator.py b/tests/unit/validation/test_campaign_config_validator.py index 61bb75ca7..a4d7c2f00 100644 --- a/tests/unit/validation/test_campaign_config_validator.py +++ b/tests/unit/validation/test_campaign_config_validator.py @@ -196,8 +196,8 @@ class TestBUCValidations: ("start_date", "end_date"), [ ("20250101", "20250331"), # valid range - ("20250601", "20250630"), # valid short range - ("20250101", "20250101"), # same day + ("20250201", "20250228"), # valid short range + ("20250202", "20250202"), # same day ], ) def test_valid_start_and_end_dates_and_iteration_dates_relation( @@ -206,14 +206,14 @@ def test_valid_start_and_end_dates_and_iteration_dates_relation( data = valid_campaign_config_with_only_mandatory_fields.copy() data["StartDate"] = start_date data["EndDate"] = end_date - data["Iterations"][0]["IterationDate"] = "20241231" + data["Iterations"][0]["IterationDate"] = "20250202" CampaignConfigValidation(**data) @pytest.mark.parametrize( ("start_date", "end_date"), [ - ("20241230", "20250101"), # campaign start date is after the iteration date - ("20250331", "20250101"), # end before start + ("20241230", "20250101"), # Campaign start date is after the iteration date + ("20240729", "20241228"), # Campaign ends date is before the iteration date ], ) def test_invalid_start_and_end_dates_and_iteration_dates_relation( @@ -222,7 +222,7 @@ def test_invalid_start_and_end_dates_and_iteration_dates_relation( data = valid_campaign_config_with_only_mandatory_fields.copy() data["StartDate"] = start_date data["EndDate"] = end_date - data["Iterations"][0]["IterationDate"] = "20241231" + data["Iterations"][0]["IterationDate"] = "20241229" with pytest.raises(ValidationError): CampaignConfigValidation(**data) From 825f8651c5389565d96a0471f431e3cde071c93a Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Tue, 25 Nov 2025 13:51:38 +0000 Subject: [PATCH 02/15] ELI 557 - Iteration id must be unique --- .../validators/campaign_config_validator.py | 11 ++++++++++ tests/unit/validation/conftest.py | 20 +++++++++++++++++++ .../test_campaign_config_validator.py | 14 +++++++++++++ 3 files changed, 45 insertions(+) diff --git a/src/rules_validation_api/validators/campaign_config_validator.py b/src/rules_validation_api/validators/campaign_config_validator.py index 02f985ba7..5cc95ecda 100644 --- a/src/rules_validation_api/validators/campaign_config_validator.py +++ b/src/rules_validation_api/validators/campaign_config_validator.py @@ -1,4 +1,5 @@ import typing +from collections import Counter from operator import attrgetter from pydantic import field_validator, model_validator @@ -13,6 +14,16 @@ class CampaignConfigValidation(CampaignConfig): def validate_iterations(cls, iterations: list[Iteration]) -> list[IterationValidation]: return [IterationValidation(**i.model_dump()) for i in iterations] + @model_validator(mode="after") + def validate_iterations_have_unique_id(self) -> typing.Self: + ids = [iteration.id for iteration in self.iterations] + duplicates = {i_id for i_id, count in Counter(ids).items() if count > 1} + if duplicates: + raise ValueError( + f"Iterations contain duplicate IDs: {', '.join(duplicates)}" + ) + return self + @model_validator(mode="after") def validate_campaign_has_iteration_within_schedule(self) -> typing.Self: iterations_by_date = sorted(self.iterations, key=attrgetter("iteration_date")) diff --git a/tests/unit/validation/conftest.py b/tests/unit/validation/conftest.py index 8b95bef44..2870b29a9 100644 --- a/tests/unit/validation/conftest.py +++ b/tests/unit/validation/conftest.py @@ -34,6 +34,26 @@ def valid_campaign_config_with_only_mandatory_fields(): } +@pytest.fixture +def valid_iteration_with_only_mandatory_fields(): + return { + "ID": "ITER001", + "Version": 1, + "Name": "Mid-January Push", + "IterationDate": "20250102", + "IterationNumber": 1, + "ApprovalMinimum": 10, + "ApprovalMaximum": 100, + "Type": "A", + "DefaultCommsRouting": "", + "DefaultNotEligibleRouting": "", + "DefaultNotActionableRouting": "", + "IterationCohorts": [], + "IterationRules": [], + "ActionsMapper": {}, + } + + @pytest.fixture def valid_iteration_rule_with_only_mandatory_fields(): return { diff --git a/tests/unit/validation/test_campaign_config_validator.py b/tests/unit/validation/test_campaign_config_validator.py index a4d7c2f00..fcf30bd52 100644 --- a/tests/unit/validation/test_campaign_config_validator.py +++ b/tests/unit/validation/test_campaign_config_validator.py @@ -233,3 +233,17 @@ def test_validate_iterations_non_empty(self, valid_campaign_config_with_only_man CampaignConfigValidation(**data) errors = error.value.errors() assert any(e["loc"][-1] == "Iterations" for e in errors), "Expected validation error on 'Iterations'" + + def test_unique_iteration_ids(self, valid_campaign_config_with_only_mandatory_fields, valid_iteration_with_only_mandatory_fields): + data = valid_campaign_config_with_only_mandatory_fields.copy() + data["Iterations"].append(valid_iteration_with_only_mandatory_fields.copy()) + data["Iterations"][1]["ID"] = data["Iterations"][0]["ID"] + with pytest.raises(ValidationError) as exc_info: + CampaignConfigValidation(**data) + + # Extract the error message + error_message = str(exc_info.value) + + # Assert that the duplicate ID appears in the message + duplicate_id = data["Iterations"][0]["ID"] + assert f"Iterations contain duplicate IDs: {duplicate_id}" in error_message From 336d2847fc52044d30a4bc1e027ba128c1778e32 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Wed, 26 Nov 2025 10:50:04 +0000 Subject: [PATCH 03/15] validate approval minimum and maximum --- .../validators/campaign_config_validator.py | 9 +++++++ .../test_campaign_config_validator.py | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/rules_validation_api/validators/campaign_config_validator.py b/src/rules_validation_api/validators/campaign_config_validator.py index 5cc95ecda..be3c80d39 100644 --- a/src/rules_validation_api/validators/campaign_config_validator.py +++ b/src/rules_validation_api/validators/campaign_config_validator.py @@ -9,11 +9,20 @@ class CampaignConfigValidation(CampaignConfig): + @field_validator("iterations") @classmethod def validate_iterations(cls, iterations: list[Iteration]) -> list[IterationValidation]: return [IterationValidation(**i.model_dump()) for i in iterations] + @model_validator(mode="after") + def validate_approval_minimum_is_less_than_or_equal_to_approval_maximum(self) -> typing.Self: + if self.approval_minimum > self.approval_maximum: + raise ValueError( + f"approval_minimum {self.approval_minimum} is greater than approval_maximum {self.approval_maximum}" + ) + return self + @model_validator(mode="after") def validate_iterations_have_unique_id(self) -> typing.Self: ids = [iteration.id for iteration in self.iterations] diff --git a/tests/unit/validation/test_campaign_config_validator.py b/tests/unit/validation/test_campaign_config_validator.py index fcf30bd52..29ed079a0 100644 --- a/tests/unit/validation/test_campaign_config_validator.py +++ b/tests/unit/validation/test_campaign_config_validator.py @@ -247,3 +247,29 @@ def test_unique_iteration_ids(self, valid_campaign_config_with_only_mandatory_fi # Assert that the duplicate ID appears in the message duplicate_id = data["Iterations"][0]["ID"] assert f"Iterations contain duplicate IDs: {duplicate_id}" in error_message + + + def test_error_approval_minimum_is_greater_than_approval_maximum(self, valid_campaign_config_with_only_mandatory_fields): + data = valid_campaign_config_with_only_mandatory_fields.copy() + data["ApprovalMinimum"] = 2 + data["ApprovalMaximum"] = 1 + with pytest.raises(ValidationError): + CampaignConfigValidation(**data) + + @pytest.mark.parametrize( + ("approval_min", "approval_max"), + [ + (1, 2), + (1, 1), + ] + ) + def test_approval_minimum_greater_than_approval_maximum_is_invalid( + self, valid_campaign_config_with_only_mandatory_fields, + approval_min, + approval_max, + ): + data = valid_campaign_config_with_only_mandatory_fields.copy() + data["ApprovalMinimum"] = approval_min + data["ApprovalMaximum"] = approval_max + CampaignConfigValidation(**data) + From cf482df33484d7256211ec9a3bbc3e8f12ba73c9 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Wed, 26 Nov 2025 17:17:12 +0000 Subject: [PATCH 04/15] lint fix --- .../validators/campaign_config_validator.py | 34 +++++++++---------- .../test_campaign_config_validator.py | 17 ++++++---- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/rules_validation_api/validators/campaign_config_validator.py b/src/rules_validation_api/validators/campaign_config_validator.py index be3c80d39..394ac14df 100644 --- a/src/rules_validation_api/validators/campaign_config_validator.py +++ b/src/rules_validation_api/validators/campaign_config_validator.py @@ -9,7 +9,6 @@ class CampaignConfigValidation(CampaignConfig): - @field_validator("iterations") @classmethod def validate_iterations(cls, iterations: list[Iteration]) -> list[IterationValidation]: @@ -17,10 +16,11 @@ def validate_iterations(cls, iterations: list[Iteration]) -> list[IterationValid @model_validator(mode="after") def validate_approval_minimum_is_less_than_or_equal_to_approval_maximum(self) -> typing.Self: - if self.approval_minimum > self.approval_maximum: - raise ValueError( - f"approval_minimum {self.approval_minimum} is greater than approval_maximum {self.approval_maximum}" - ) + if self.approval_minimum and self.approval_maximum: + if self.approval_minimum > self.approval_maximum: + msg = f"approval_minimum {self.approval_minimum} > approval_maximum {self.approval_maximum}" + raise ValueError(msg) + return self return self @model_validator(mode="after") @@ -28,9 +28,8 @@ def validate_iterations_have_unique_id(self) -> typing.Self: ids = [iteration.id for iteration in self.iterations] duplicates = {i_id for i_id, count in Counter(ids).items() if count > 1} if duplicates: - raise ValueError( - f"Iterations contain duplicate IDs: {', '.join(duplicates)}" - ) + msg = f"Iterations contain duplicate IDs: {', '.join(duplicates)}" + raise ValueError(msg) return self @model_validator(mode="after") @@ -38,16 +37,15 @@ def validate_campaign_has_iteration_within_schedule(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: - raise ValueError( - f"Iteration {first_iteration.id} starts before campaign {self.id} " - f"start date {self.start_date}." - ) + msg = f"Iteration {first_iteration.id} starts before campaign {self.id} start date {self.start_date}." + raise ValueError(msg) + if first_iteration.iteration_date > self.end_date: - raise ValueError( - f"Iteration {first_iteration.id} starts after campaign {self.id} " - f"end date {self.end_date}." - ) + msg = f"Iteration {first_iteration.id} starts after campaign {self.id} end date {self.end_date}." + raise ValueError(msg) + 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) + msg = f"campaign {self.id} has no iterations." + raise ValueError(msg) diff --git a/tests/unit/validation/test_campaign_config_validator.py b/tests/unit/validation/test_campaign_config_validator.py index 29ed079a0..c7321aef4 100644 --- a/tests/unit/validation/test_campaign_config_validator.py +++ b/tests/unit/validation/test_campaign_config_validator.py @@ -234,10 +234,12 @@ def test_validate_iterations_non_empty(self, valid_campaign_config_with_only_man errors = error.value.errors() assert any(e["loc"][-1] == "Iterations" for e in errors), "Expected validation error on 'Iterations'" - def test_unique_iteration_ids(self, valid_campaign_config_with_only_mandatory_fields, valid_iteration_with_only_mandatory_fields): + def test_unique_iteration_ids( + self, valid_campaign_config_with_only_mandatory_fields, valid_iteration_with_only_mandatory_fields + ): data = valid_campaign_config_with_only_mandatory_fields.copy() data["Iterations"].append(valid_iteration_with_only_mandatory_fields.copy()) - data["Iterations"][1]["ID"] = data["Iterations"][0]["ID"] + data["Iterations"][1]["ID"] = data["Iterations"][0]["ID"] with pytest.raises(ValidationError) as exc_info: CampaignConfigValidation(**data) @@ -248,8 +250,9 @@ def test_unique_iteration_ids(self, valid_campaign_config_with_only_mandatory_fi duplicate_id = data["Iterations"][0]["ID"] assert f"Iterations contain duplicate IDs: {duplicate_id}" in error_message - - def test_error_approval_minimum_is_greater_than_approval_maximum(self, valid_campaign_config_with_only_mandatory_fields): + def test_error_approval_minimum_is_greater_than_approval_maximum( + self, valid_campaign_config_with_only_mandatory_fields + ): data = valid_campaign_config_with_only_mandatory_fields.copy() data["ApprovalMinimum"] = 2 data["ApprovalMaximum"] = 1 @@ -261,10 +264,11 @@ def test_error_approval_minimum_is_greater_than_approval_maximum(self, valid_cam [ (1, 2), (1, 1), - ] + ], ) def test_approval_minimum_greater_than_approval_maximum_is_invalid( - self, valid_campaign_config_with_only_mandatory_fields, + self, + valid_campaign_config_with_only_mandatory_fields, approval_min, approval_max, ): @@ -272,4 +276,3 @@ def test_approval_minimum_greater_than_approval_maximum_is_invalid( data["ApprovalMinimum"] = approval_min data["ApprovalMaximum"] = approval_max CampaignConfigValidation(**data) - From ad28d051ba59e5e49f7962715265a4f0f571e326 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Thu, 27 Nov 2025 10:27:08 +0000 Subject: [PATCH 05/15] fix for approve minimum and maximum validator --- .../validators/campaign_config_validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules_validation_api/validators/campaign_config_validator.py b/src/rules_validation_api/validators/campaign_config_validator.py index 394ac14df..3b5cf509d 100644 --- a/src/rules_validation_api/validators/campaign_config_validator.py +++ b/src/rules_validation_api/validators/campaign_config_validator.py @@ -16,7 +16,7 @@ def validate_iterations(cls, iterations: list[Iteration]) -> list[IterationValid @model_validator(mode="after") def validate_approval_minimum_is_less_than_or_equal_to_approval_maximum(self) -> typing.Self: - if self.approval_minimum and self.approval_maximum: + if self.approval_minimum is not None and self.approval_maximum is not None: if self.approval_minimum > self.approval_maximum: msg = f"approval_minimum {self.approval_minimum} > approval_maximum {self.approval_maximum}" raise ValueError(msg) From 69473d1d461845dc327f36c6698ea845c41b1187 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Thu, 27 Nov 2025 16:29:13 +0000 Subject: [PATCH 06/15] date fix --- .../model/campaign_config.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/eligibility_signposting_api/model/campaign_config.py b/src/eligibility_signposting_api/model/campaign_config.py index 6c4eaeeea..a107d669b 100644 --- a/src/eligibility_signposting_api/model/campaign_config.py +++ b/src/eligibility_signposting_api/model/campaign_config.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import re import typing from collections import Counter from datetime import UTC, date, datetime @@ -273,7 +274,12 @@ class Iteration(BaseModel): def parse_dates(cls, v: str | date) -> date: if isinstance(v, date): return v - return datetime.strptime(v, "%Y%m%d").date() # noqa: DTZ007 + if not re.fullmatch(r"\d{8}", str(v)): + raise ValueError(f"Invalid format: {v}. Must be YYYYMMDD with 8 digits.") + try: + return datetime.strptime(str(v), "%Y%m%d").date() # noqa: DTZ007 + except ValueError: + raise ValueError(f"Invalid date value: {v}. Must be a valid calendar date in YYYYMMDD format.") @field_serializer("iteration_date", when_used="always") @staticmethod @@ -316,7 +322,12 @@ class CampaignConfig(BaseModel): def parse_dates(cls, v: str | date) -> date: if isinstance(v, date): return v - return datetime.strptime(v, "%Y%m%d").date() # noqa: DTZ007 + if not re.fullmatch(r"\d{8}", str(v)): + raise ValueError(f"Invalid format: {v}. Must be YYYYMMDD with 8 digits.") + try: + return datetime.strptime(str(v), "%Y%m%d").date() # noqa: DTZ007 + except ValueError: + raise ValueError(f"Invalid date value: {v}. Must be a valid calendar date in YYYYMMDD format.") @field_serializer("start_date", "end_date", when_used="always") @staticmethod From 2fd830d9dae82a1e687105631ddd992d7458c181 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Thu, 27 Nov 2025 16:44:30 +0000 Subject: [PATCH 07/15] attribute name is optional only for cohort attribte level --- .../model/campaign_config.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/eligibility_signposting_api/model/campaign_config.py b/src/eligibility_signposting_api/model/campaign_config.py index a107d669b..9ae26280f 100644 --- a/src/eligibility_signposting_api/model/campaign_config.py +++ b/src/eligibility_signposting_api/model/campaign_config.py @@ -163,6 +163,17 @@ def parse_yn_to_bool(cls, v: str | bool) -> bool: # noqa: N805 return v.upper() == "Y" return v + @model_validator( mode="after") + def validate_attribute_name_is_optional_only_for_cohort_attribute_level(self) -> typing.Self: # noqa: N805 + if self.attribute_name: + return self + else: + if self.attribute_level == RuleAttributeLevel.COHORT: + return self + else: + raise ValueError(f"AttributeName must be set for {self.attribute_level} AttributeLevel.") + + _parent: Iteration | None = PrivateAttr(default=None) def set_parent(self, parent: Iteration) -> None: From 2f5ae0a2a231b40e33ffbe9aad8d6a12f4329185 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Thu, 27 Nov 2025 16:59:04 +0000 Subject: [PATCH 08/15] attribute level and attribute target relation --- .../model/campaign_config.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/eligibility_signposting_api/model/campaign_config.py b/src/eligibility_signposting_api/model/campaign_config.py index 9ae26280f..fcb185ee3 100644 --- a/src/eligibility_signposting_api/model/campaign_config.py +++ b/src/eligibility_signposting_api/model/campaign_config.py @@ -171,7 +171,17 @@ def validate_attribute_name_is_optional_only_for_cohort_attribute_level(self) - if self.attribute_level == RuleAttributeLevel.COHORT: return self else: - raise ValueError(f"AttributeName must be set for {self.attribute_level} AttributeLevel.") + raise ValueError(f"AttributeName must be set where AttributeLevel is {self.attribute_level} .") + + @model_validator(mode="after") + def validate_attribute_target_is_mandatory_for_target_attribute_level(self) -> typing.Self: # noqa: N805 + if self.attribute_target: + return self + else: + if self.attribute_level != RuleAttributeLevel.TARGET: + return self + else: + raise ValueError(f"AttributeTarget is mandatory where AttributeLevel is {self.attribute_level}.") _parent: Iteration | None = PrivateAttr(default=None) From 9153de09e8ea37e30678b7d67627e6065cab40f9 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Thu, 27 Nov 2025 17:06:58 +0000 Subject: [PATCH 09/15] cohort label md --- src/rules_validation_api/validations.md | 116 ++++++++++++------------ 1 file changed, 58 insertions(+), 58 deletions(-) diff --git a/src/rules_validation_api/validations.md b/src/rules_validation_api/validations.md index a54dc9d6c..2fcddc633 100644 --- a/src/rules_validation_api/validations.md +++ b/src/rules_validation_api/validations.md @@ -1,60 +1,60 @@ # CampaignConfig Validation Rules -| Key | Validation | Data Type | Field Uniqueness | Enum/Allowed Values | Cross-field/Conditional | -|-----------------------------------------------------------------------------|------------|--------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------| -| CampaignConfig.ID | Mandatory | string | Unique | - | - | -| CampaignConfig.Version | Mandatory | int | - | - | - | -| CampaignConfig.Name | Mandatory | string | - | - | - | -| CampaignConfig.Type | Mandatory | string | - | V, S | - | -| CampaignConfig.Target | Mandatory | string | - | See [`config/constants.py`](../eligibility_signposting_api/config/constants.py) for the definition of `ALLOWED_CONDITIONS`. | - | -| CampaignConfig.Manager | Optional | list[string] | - | - | - | -| CampaignConfig.Approver | Optional | list[string] | - | - | - | -| CampaignConfig.Reviewer | Optional | list[string] | - | - | - | -| CampaignConfig.IterationFrequency | Mandatory | string | - | X, D, W, M, Q, A | - | -| CampaignConfig.IterationType | Mandatory | string | - | A, M, S, O | - | -| CampaignConfig.IterationTime | Optional | string | - | - | - | -| CampaignConfig.DefaultCommsRouting | Optional | string | - | - | - | -| CampaignConfig.StartDate | Mandatory | date | - | - | StartDate < EndDate | -| CampaignConfig.EndDate | Mandatory | date | - | - | EndDate > StartDate | -| CampaignConfig.ApprovalMinimum | Optional | int | - | - | ApprovalMinimum ≤ ApprovalMaximum | -| CampaignConfig.ApprovalMaximum | Optional | int | - | - | ApprovalMaximum ≥ ApprovalMinimum | -| CampaignConfig.Iterations[x].ID | Mandatory | string | Unique | - | - | -| CampaignConfig.Iterations[x].Version | Mandatory | int | - | - | - | -| CampaignConfig.Iterations[x].Name | Mandatory | string | - | - | - | -| CampaignConfig.Iterations[x].IterationDate | Mandatory | date | Unique | - | No overlapping dates | -| CampaignConfig.Iterations[x].IterationNumber | Optional | int | - | - | - | -| CampaignConfig.Iterations[x].ApprovalMinimum | Optional | int | - | - | - | -| CampaignConfig.Iterations[x].ApprovalMaximum | Optional | int | - | - | - | -| CampaignConfig.Iterations[x].Type | Mandatory | string | - | A, M, S, O | - | -| CampaignConfig.Iterations[x].DefaultCommsRouting | Mandatory | string | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].DefaultNotEligibleRouting | Mandatory | string | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].DefaultNotActionableRouting | Mandatory | string | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].StatusText.NotEligible | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].StatusText.NotActionable | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].StatusText.Actionable | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].RulesMapper.RULE_ENTRY.RuleNames | Optional | list[string] | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].RulesMapper.RULE_ENTRY.RuleCode | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].RulesMapper.RULE_ENTRY.RuleText | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | -| CampaignConfig.Iterations[x].IterationCohorts[y].CohortLabel | Mandatory | string | Unique in iteration | - | - | -| CampaignConfig.Iterations[x].IterationCohorts[y].CohortGroup | Mandatory | string | - | - | - | -| CampaignConfig.Iterations[x].IterationCohorts[y].PositiveDescription | Optional | string | - | - | - | -| CampaignConfig.Iterations[x].IterationCohorts[y].NegativeDescription | Optional | string | - | - | - | -| CampaignConfig.Iterations[x].IterationCohorts[y].Priority | Optional | int | - | - | - | -| CampaignConfig.Iterations[x].IterationCohorts[y].Virtual | Optional | string | - | Y, N | - | -| CampaignConfig.Iterations[x].IterationRules[z].Type | Mandatory | string | - | F, S, R, X, Y | - | -| CampaignConfig.Iterations[x].IterationRules[z].Name | Mandatory | string | - | - | - | -| CampaignConfig.Iterations[x].IterationRules[z].Description | Mandatory | string | - | - | - | -| CampaignConfig.Iterations[x].IterationRules[z].Operator | Mandatory | string | - | See [`model/campaign_config.py`](../eligibility_signposting_api/model/campaign_config.py) for the definition of `RuleOperator`. | - | -| CampaignConfig.Iterations[x].IterationRules[z].Comparator | Mandatory | string | - | - | - | -| CampaignConfig.Iterations[x].IterationRules[z].AttributeTarget | Optional | string | - | - | Required and non-null if AttributeLevel == TARGET | -| CampaignConfig.Iterations[x].IterationRules[z].AttributeLevel | Mandatory | string | - | PERSON, TARGET, COHORT | - | -| CampaignConfig.Iterations[x].IterationRules[z].AttributeName | Optional | string | - | - | If AttributeLevel is COHORT, AttributeName must be COHORT_LABEL or left blank. | -| CampaignConfig.Iterations[x].IterationRules[z].CohortLabel | Optional | string | - | - | Required if AttributeLevel == COHORT and CohortLabel only matters for Type 'F'/'S' | -| CampaignConfig.Iterations[x].IterationRules[z].Priority | Mandatory | int | - | - | - | -| CampaignConfig.Iterations[x].IterationRules[z].RuleStop | Optional | bool | - | - | - | -| CampaignConfig.Iterations[x].IterationRules[z].CommsRouting | Optional | string | - | - | - | -| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.ExternalRoutingCode | Optional | string | - | - | - | -| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.ActionDescription | Optional | string | - | Must be valid Markdown and follow style rules (headers, lists, blank lines) | - | -| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.ActionType | Optional | string | - | - | - | -| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.UrlLink | Optional | string | - | - | - | -| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.UrlLabel | Optional | string | - | - | - | +| Key | Validation | Data Type | Field Uniqueness | Enum/Allowed Values | Cross-field/Conditional | +|-----------------------------------------------------------------------------|------------|--------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------| +| CampaignConfig.ID | Mandatory | string | Unique | - | - | +| CampaignConfig.Version | Mandatory | int | - | - | - | +| CampaignConfig.Name | Mandatory | string | - | - | - | +| CampaignConfig.Type | Mandatory | string | - | V, S | - | +| CampaignConfig.Target | Mandatory | string | - | See [`config/constants.py`](../eligibility_signposting_api/config/constants.py) for the definition of `ALLOWED_CONDITIONS`. | - | +| CampaignConfig.Manager | Optional | list[string] | - | - | - | +| CampaignConfig.Approver | Optional | list[string] | - | - | - | +| CampaignConfig.Reviewer | Optional | list[string] | - | - | - | +| CampaignConfig.IterationFrequency | Mandatory | string | - | X, D, W, M, Q, A | - | +| CampaignConfig.IterationType | Mandatory | string | - | A, M, S, O | - | +| CampaignConfig.IterationTime | Optional | string | - | - | - | +| CampaignConfig.DefaultCommsRouting | Optional | string | - | - | - | +| CampaignConfig.StartDate | Mandatory | date | - | - | StartDate < EndDate | +| CampaignConfig.EndDate | Mandatory | date | - | - | EndDate > StartDate | +| CampaignConfig.ApprovalMinimum | Optional | int | - | - | ApprovalMinimum ≤ ApprovalMaximum | +| CampaignConfig.ApprovalMaximum | Optional | int | - | - | ApprovalMaximum ≥ ApprovalMinimum | +| CampaignConfig.Iterations[x].ID | Mandatory | string | Unique | - | - | +| CampaignConfig.Iterations[x].Version | Mandatory | int | - | - | - | +| CampaignConfig.Iterations[x].Name | Mandatory | string | - | - | - | +| CampaignConfig.Iterations[x].IterationDate | Mandatory | date | Unique | - | No overlapping dates | +| CampaignConfig.Iterations[x].IterationNumber | Optional | int | - | - | - | +| CampaignConfig.Iterations[x].ApprovalMinimum | Optional | int | - | - | - | +| CampaignConfig.Iterations[x].ApprovalMaximum | Optional | int | - | - | - | +| CampaignConfig.Iterations[x].Type | Mandatory | string | - | A, M, S, O | - | +| CampaignConfig.Iterations[x].DefaultCommsRouting | Mandatory | string | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].DefaultNotEligibleRouting | Mandatory | string | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].DefaultNotActionableRouting | Mandatory | string | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].StatusText.NotEligible | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].StatusText.NotActionable | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].StatusText.Actionable | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].RulesMapper.RULE_ENTRY.RuleNames | Optional | list[string] | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].RulesMapper.RULE_ENTRY.RuleCode | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].RulesMapper.RULE_ENTRY.RuleText | Optional | string | - | - | Each value in this field must match a key in ActionsMapper. | +| CampaignConfig.Iterations[x].IterationCohorts[y].CohortLabel | Mandatory | string | Unique in iteration | - | - | +| CampaignConfig.Iterations[x].IterationCohorts[y].CohortGroup | Mandatory | string | - | - | - | +| CampaignConfig.Iterations[x].IterationCohorts[y].PositiveDescription | Optional | string | - | - | - | +| CampaignConfig.Iterations[x].IterationCohorts[y].NegativeDescription | Optional | string | - | - | - | +| CampaignConfig.Iterations[x].IterationCohorts[y].Priority | Optional | int | - | - | - | +| CampaignConfig.Iterations[x].IterationCohorts[y].Virtual | Optional | string | - | Y, N | - | +| CampaignConfig.Iterations[x].IterationRules[z].Type | Mandatory | string | - | F, S, R, X, Y | - | +| CampaignConfig.Iterations[x].IterationRules[z].Name | Mandatory | string | - | - | - | +| CampaignConfig.Iterations[x].IterationRules[z].Description | Mandatory | string | - | - | - | +| CampaignConfig.Iterations[x].IterationRules[z].Operator | Mandatory | string | - | See [`model/campaign_config.py`](../eligibility_signposting_api/model/campaign_config.py) for the definition of `RuleOperator`. | - | +| CampaignConfig.Iterations[x].IterationRules[z].Comparator | Mandatory | string | - | - | - | +| CampaignConfig.Iterations[x].IterationRules[z].AttributeTarget | Optional | string | - | - | Required and non-null if AttributeLevel == TARGET | +| CampaignConfig.Iterations[x].IterationRules[z].AttributeLevel | Mandatory | string | - | PERSON, TARGET, COHORT | - | +| CampaignConfig.Iterations[x].IterationRules[z].AttributeName | Optional | string | - | - | If AttributeLevel is COHORT, AttributeName must be COHORT_LABEL or left blank. | +| CampaignConfig.Iterations[x].IterationRules[z].CohortLabel | Optional | string | - | - | CohortLabel only works for Type 'F'/'S' | +| CampaignConfig.Iterations[x].IterationRules[z].Priority | Mandatory | int | - | - | - | +| CampaignConfig.Iterations[x].IterationRules[z].RuleStop | Optional | bool | - | - | - | +| CampaignConfig.Iterations[x].IterationRules[z].CommsRouting | Optional | string | - | - | - | +| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.ExternalRoutingCode | Optional | string | - | - | - | +| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.ActionDescription | Optional | string | - | Must be valid Markdown and follow style rules (headers, lists, blank lines) | - | +| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.ActionType | Optional | string | - | - | - | +| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.UrlLink | Optional | string | - | - | - | +| CampaignConfig.Iterations[x].ActionsMapper.ACTION_ENTRY.UrlLabel | Optional | string | - | - | - | From 6d61ec8c147fb3ac6c9290f9637520eb047aaf77 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Fri, 28 Nov 2025 07:55:28 +0000 Subject: [PATCH 10/15] refactored --- .../model/campaign_config.py | 21 ------ .../validators/iteration_rules_validator.py | 22 +++++- .../test_iteration_rules_validator.py | 73 ++++++++++++++++++- 3 files changed, 88 insertions(+), 28 deletions(-) diff --git a/src/eligibility_signposting_api/model/campaign_config.py b/src/eligibility_signposting_api/model/campaign_config.py index fcb185ee3..a107d669b 100644 --- a/src/eligibility_signposting_api/model/campaign_config.py +++ b/src/eligibility_signposting_api/model/campaign_config.py @@ -163,27 +163,6 @@ def parse_yn_to_bool(cls, v: str | bool) -> bool: # noqa: N805 return v.upper() == "Y" return v - @model_validator( mode="after") - def validate_attribute_name_is_optional_only_for_cohort_attribute_level(self) -> typing.Self: # noqa: N805 - if self.attribute_name: - return self - else: - if self.attribute_level == RuleAttributeLevel.COHORT: - return self - else: - raise ValueError(f"AttributeName must be set where AttributeLevel is {self.attribute_level} .") - - @model_validator(mode="after") - def validate_attribute_target_is_mandatory_for_target_attribute_level(self) -> typing.Self: # noqa: N805 - if self.attribute_target: - return self - else: - if self.attribute_level != RuleAttributeLevel.TARGET: - return self - else: - raise ValueError(f"AttributeTarget is mandatory where AttributeLevel is {self.attribute_level}.") - - _parent: Iteration | None = PrivateAttr(default=None) def set_parent(self, parent: Iteration) -> None: diff --git a/src/rules_validation_api/validators/iteration_rules_validator.py b/src/rules_validation_api/validators/iteration_rules_validator.py index 74f6ce12d..2acd19ded 100644 --- a/src/rules_validation_api/validators/iteration_rules_validator.py +++ b/src/rules_validation_api/validators/iteration_rules_validator.py @@ -1,4 +1,4 @@ -from typing import Self +import typing from pydantic import model_validator @@ -12,7 +12,7 @@ class IterationRuleValidation(IterationRule): @model_validator(mode="after") - def check_cohort_attribute_name(self) -> Self: + def check_cohort_attribute_name(self) -> typing.Self: if ( self.attribute_level == RuleAttributeLevel.COHORT and self.attribute_name @@ -23,7 +23,7 @@ def check_cohort_attribute_name(self) -> Self: return self @model_validator(mode="after") - def check_cohort_label_for_non_f_and_s_types(self) -> Self: + def check_cohort_label_for_non_f_and_s_types(self) -> typing.Self: allowed_types = {RuleType("F"), RuleType("S")} if self.cohort_label is not None and self.type not in allowed_types: msg = ( @@ -32,3 +32,19 @@ def check_cohort_label_for_non_f_and_s_types(self) -> Self: ) raise ValueError(msg) return self + + @model_validator(mode="after") + def validate_attribute_name_is_optional_only_for_cohort_attribute_level(self) -> typing.Self: + if self.attribute_name: + return self + if self.attribute_level == RuleAttributeLevel.COHORT: + return self + raise ValueError(f"AttributeName must be set where AttributeLevel is {self.attribute_level} .") + + @model_validator(mode="after") + def validate_attribute_target_is_mandatory_for_target_attribute_level(self) -> typing.Self: + if self.attribute_target: + return self + if self.attribute_level != RuleAttributeLevel.TARGET: + return self + raise ValueError(f"AttributeTarget is mandatory where AttributeLevel is {self.attribute_level}.") diff --git a/tests/unit/validation/test_iteration_rules_validator.py b/tests/unit/validation/test_iteration_rules_validator.py index 2e6dcfd21..33c66abc7 100644 --- a/tests/unit/validation/test_iteration_rules_validator.py +++ b/tests/unit/validation/test_iteration_rules_validator.py @@ -1,6 +1,7 @@ import pytest from pydantic import ValidationError +from eligibility_signposting_api.model.campaign_config import RuleAttributeLevel from rules_validation_api.validators.iteration_validator import IterationRuleValidation @@ -80,8 +81,18 @@ def test_invalid_priority(self, priority_value, valid_iteration_rule_with_only_m with pytest.raises(ValidationError): IterationRuleValidation(**data) - @pytest.mark.parametrize("attribute_level", ["PERSON", "TARGET", "COHORT"]) - def test_valid_attribute_level(self, attribute_level, valid_iteration_rule_with_only_mandatory_fields): + @pytest.mark.parametrize("attribute_level", ["PERSON", "TARGET"]) + def test_valid_attribute_level_person_and_targer( + self, attribute_level, valid_iteration_rule_with_only_mandatory_fields + ): + data = valid_iteration_rule_with_only_mandatory_fields.copy() + data["AttributeLevel"] = attribute_level + data["AttributeName"] = "Something" # Ignoring the validation constraint btw AttributeLevel and AttributeName + result = IterationRuleValidation(**data) + assert result.attribute_level == attribute_level + + @pytest.mark.parametrize("attribute_level", ["COHORT"]) + def test_valid_attribute_level_cohort(self, attribute_level, valid_iteration_rule_with_only_mandatory_fields): data = valid_iteration_rule_with_only_mandatory_fields.copy() data["AttributeLevel"] = attribute_level data["AttributeName"] = None # Ignoring the validation constraint btw AttributeLevel and AttributeName @@ -148,7 +159,7 @@ def test_rule_stop_boolean_resolution( class TestOptionalFieldsSchemaValidations: # AttributeName - @pytest.mark.parametrize("attr_name", ["status", "user_type", None]) + @pytest.mark.parametrize("attr_name", ["status", "user_type"]) def test_valid_attribute_name(self, attr_name, valid_iteration_rule_with_only_mandatory_fields): data = valid_iteration_rule_with_only_mandatory_fields.copy() data["AttributeName"] = attr_name @@ -192,7 +203,7 @@ def test_invalid_cohort_label(self, label, valid_iteration_rule_with_only_mandat IterationRuleValidation(**data) # AttributeTarget - @pytest.mark.parametrize("target", ["target_value", None]) + @pytest.mark.parametrize("target", ["target_value"]) def test_valid_attribute_target(self, target, valid_iteration_rule_with_only_mandatory_fields): data = valid_iteration_rule_with_only_mandatory_fields.copy() data["AttributeTarget"] = target @@ -291,3 +302,57 @@ def test_valid_when_cohort_label_absent_for_non_f_s_types( data.pop("CohortLabel", None) result = IterationRuleValidation(**data) assert result.cohort_label is None + + @pytest.mark.parametrize( + ("attribute_level", "attribute_name", "is_valid"), + [ + (RuleAttributeLevel.COHORT, None, True), # Allowed: name optional for cohort + (RuleAttributeLevel.COHORT, "COHORT_LABEL", True), # Allowed: name provided + (RuleAttributeLevel.TARGET, "RSV", True), # Allowed: name provided for target + (RuleAttributeLevel.TARGET, None, False), # NOT allowed: missing for non-cohort + ], + ) + def test_attribute_name_optional_only_for_cohort( + self, attribute_level, attribute_name, is_valid, valid_iteration_rule_with_only_mandatory_fields + ): + data = valid_iteration_rule_with_only_mandatory_fields.copy() + data["AttributeLevel"] = attribute_level + data["AttributeName"] = attribute_name + + if is_valid: + # Should validate with no exceptions + IterationRuleValidation(**data) + else: + with pytest.raises(ValidationError) as exc: + IterationRuleValidation(**data) + + assert "AttributeName must be set" in str(exc.value) + + @pytest.mark.parametrize( + ("attribute_level", "attribute_target", "attribute_name", "is_valid"), + [ + (RuleAttributeLevel.TARGET, "RSV", "BOOKED_APPOINTMENT_DATE", True), # Valid: required for TARGET + (RuleAttributeLevel.TARGET, None, "BOOKED_APPOINTMENT_DATE", False), # Invalid: missing + (RuleAttributeLevel.COHORT, None, "COHORT_LABEL", True), # Valid: not required + ], + ) + def test_attribute_target_mandatory_for_target_level( + self, + attribute_level, + attribute_target, + attribute_name, + is_valid, + valid_iteration_rule_with_only_mandatory_fields, + ): + data = valid_iteration_rule_with_only_mandatory_fields.copy() + data["AttributeLevel"] = attribute_level + data["AttributeTarget"] = attribute_target + data["AttributeName"] = attribute_name + + if is_valid: + IterationRuleValidation(**data) + else: + with pytest.raises(ValidationError) as exc: + IterationRuleValidation(**data) + + assert "AttributeTarget is mandatory" in str(exc.value) From 39856a5a9f5a2ec9ac1381e997c68e832af1f060 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Fri, 28 Nov 2025 07:59:01 +0000 Subject: [PATCH 11/15] lint fix --- .../model/campaign_config.py | 32 +++++++++++++------ .../validators/iteration_rules_validator.py | 8 +++-- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/eligibility_signposting_api/model/campaign_config.py b/src/eligibility_signposting_api/model/campaign_config.py index a107d669b..492094dfc 100644 --- a/src/eligibility_signposting_api/model/campaign_config.py +++ b/src/eligibility_signposting_api/model/campaign_config.py @@ -274,12 +274,18 @@ class Iteration(BaseModel): def parse_dates(cls, v: str | date) -> date: if isinstance(v, date): return v - if not re.fullmatch(r"\d{8}", str(v)): - raise ValueError(f"Invalid format: {v}. Must be YYYYMMDD with 8 digits.") + + v_str = str(v) + + if not re.fullmatch(r"\d{8}", v_str): + msg = f"Invalid format: {v_str}. Must be YYYYMMDD with 8 digits." + raise ValueError(msg) + try: - return datetime.strptime(str(v), "%Y%m%d").date() # noqa: DTZ007 - except ValueError: - raise ValueError(f"Invalid date value: {v}. Must be a valid calendar date in YYYYMMDD format.") + return datetime.strptime(v_str, "%Y%m%d").date() # noqa: DTZ007 + except ValueError as err: + msg = f"Invalid date value: {v_str}. Must be a valid calendar date in YYYYMMDD format." + raise ValueError(msg) from err @field_serializer("iteration_date", when_used="always") @staticmethod @@ -322,12 +328,18 @@ class CampaignConfig(BaseModel): def parse_dates(cls, v: str | date) -> date: if isinstance(v, date): return v - if not re.fullmatch(r"\d{8}", str(v)): - raise ValueError(f"Invalid format: {v}. Must be YYYYMMDD with 8 digits.") + + v_str = str(v) + + if not re.fullmatch(r"\d{8}", v_str): + msg = f"Invalid format: {v_str}. Must be YYYYMMDD with 8 digits." + raise ValueError(msg) + try: - return datetime.strptime(str(v), "%Y%m%d").date() # noqa: DTZ007 - except ValueError: - raise ValueError(f"Invalid date value: {v}. Must be a valid calendar date in YYYYMMDD format.") + return datetime.strptime(v_str, "%Y%m%d").date() # noqa: DTZ007 + except ValueError as err: + msg = f"Invalid date value: {v_str}. Must be a valid calendar date in YYYYMMDD format." + raise ValueError(msg) from err @field_serializer("start_date", "end_date", when_used="always") @staticmethod diff --git a/src/rules_validation_api/validators/iteration_rules_validator.py b/src/rules_validation_api/validators/iteration_rules_validator.py index 2acd19ded..ccce344e8 100644 --- a/src/rules_validation_api/validators/iteration_rules_validator.py +++ b/src/rules_validation_api/validators/iteration_rules_validator.py @@ -27,7 +27,7 @@ def check_cohort_label_for_non_f_and_s_types(self) -> typing.Self: allowed_types = {RuleType("F"), RuleType("S")} if self.cohort_label is not None and self.type not in allowed_types: msg = ( - f"CohortLabel is only allowed for rule types F and S. " + "CohortLabel is only allowed for rule types F and S. " f"Found type: {self.type} with cohort_label: {self.cohort_label}" ) raise ValueError(msg) @@ -39,7 +39,8 @@ def validate_attribute_name_is_optional_only_for_cohort_attribute_level(self) -> return self if self.attribute_level == RuleAttributeLevel.COHORT: return self - raise ValueError(f"AttributeName must be set where AttributeLevel is {self.attribute_level} .") + msg = f"AttributeName must be set where AttributeLevel is {self.attribute_level}." + raise ValueError(msg) @model_validator(mode="after") def validate_attribute_target_is_mandatory_for_target_attribute_level(self) -> typing.Self: @@ -47,4 +48,5 @@ def validate_attribute_target_is_mandatory_for_target_attribute_level(self) -> t return self if self.attribute_level != RuleAttributeLevel.TARGET: return self - raise ValueError(f"AttributeTarget is mandatory where AttributeLevel is {self.attribute_level}.") + msg = f"AttributeTarget is mandatory where AttributeLevel is {self.attribute_level}." + raise ValueError(msg) From 6ff59e4ba6d3ac56d6884bda4a72be68b4dcb80d Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Fri, 28 Nov 2025 08:40:52 +0000 Subject: [PATCH 12/15] code cleanup --- .../validators/available_action_validator.py | 4 +--- src/rules_validation_api/validators/iteration_validator.py | 6 +----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/rules_validation_api/validators/available_action_validator.py b/src/rules_validation_api/validators/available_action_validator.py index 745fcd853..07e80c315 100644 --- a/src/rules_validation_api/validators/available_action_validator.py +++ b/src/rules_validation_api/validators/available_action_validator.py @@ -1,13 +1,11 @@ import re -from pydantic import Field, field_validator +from pydantic import field_validator from eligibility_signposting_api.model.campaign_config import AvailableAction class AvailableActionValidation(AvailableAction): - action_description: str | None = Field(None, alias="ActionDescription") - @field_validator("action_description") @classmethod def validate_description_style(cls, text: str) -> str: diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 0e9e46b9b..725065187 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -1,6 +1,6 @@ import typing -from pydantic import Field, ValidationError, field_validator, model_validator +from pydantic import ValidationError, field_validator, model_validator from pydantic_core import InitErrorDetails from eligibility_signposting_api.model.campaign_config import ( @@ -18,10 +18,6 @@ class IterationValidation(Iteration): - iteration_cohorts: list[IterationCohort] = Field(..., alias="IterationCohorts") - iteration_rules: list[IterationRule] = Field(..., alias="IterationRules") - actions_mapper: ActionsMapper = Field(..., alias="ActionsMapper") - @field_validator("iteration_rules") @classmethod def validate_iteration_rules(cls, iteration_rules: list[IterationRule]) -> list[IterationRuleValidation]: From b572f6c7479cd6eff75317279a97651361adbfd2 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Fri, 28 Nov 2025 11:28:21 +0000 Subject: [PATCH 13/15] cohort priority should be unique --- .../validators/iteration_validator.py | 40 +++++++++++++++---- tests/unit/validation/conftest.py | 4 +- .../validation/test_iteration_validator.py | 29 ++++++++++++++ 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 725065187..5f5f32143 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -25,22 +25,46 @@ def validate_iteration_rules(cls, iteration_rules: list[IterationRule]) -> list[ @field_validator("iteration_cohorts") @classmethod - def validate_iteration_cohorts(cls, iteration_cohorts: list[IterationCohort]) -> list[IterationCohortValidation]: + def validate_iteration_cohorts( + cls, iteration_cohorts: list[IterationCohort] + ) -> list[IterationCohortValidation]: seen_labels = set() + seen_priorities = set() errors = [] + for cohort in iteration_cohorts: label = cohort.cohort_label + priority = cohort.priority + + # Duplicate label check if label in seen_labels: - error = InitErrorDetails( - type="value_error", - loc=("iteration_cohort",), - input=label, - ctx={"error": f"Duplicate iteration_cohort: {label}"}, + errors.append( + InitErrorDetails( + type="value_error", + loc=("iteration_cohort", "cohort_label"), + input=label, + ctx={"error": f"Duplicate iteration_cohort label: {label}"}, + ) ) - errors.append(error) seen_labels.add(label) + + # Duplicate priority check + if priority in seen_priorities: + errors.append( + InitErrorDetails( + type="value_error", + loc=("iteration_cohort", "priority"), + input=priority, + ctx={"error": f"Duplicate iteration_cohort priority: {priority}"}, + ) + ) + seen_priorities.add(priority) + if errors: - raise ValidationError.from_exception_data(title="IterationValidation", line_errors=errors) + raise ValidationError.from_exception_data( + title="IterationValidation", line_errors=errors + ) + return [IterationCohortValidation(**i.model_dump()) for i in iteration_cohorts] @field_validator("actions_mapper", mode="after") diff --git a/tests/unit/validation/conftest.py b/tests/unit/validation/conftest.py index 2870b29a9..41f8014ed 100644 --- a/tests/unit/validation/conftest.py +++ b/tests/unit/validation/conftest.py @@ -82,13 +82,13 @@ def valid_available_action(): @pytest.fixture def valid_iteration_cohorts(): - def _cohort(label: str = "label_1", group: str = "group_1"): + def _cohort(label: str = "label_1", group: str = "group_1", priority: int = 0): return { "CohortLabel": label, "CohortGroup": group, "PositiveDescription": "are a member of eli_399_cohort_group [[PERSON.POSTCODE:DATE(%d %B %Y)]]", "NegativeDescription": "are not a member of eli_399_cohort_group [[PERSON.POSTCODE:DATE(%d %B %Y)]]", - "Priority": 0, + "Priority": priority, } return _cohort diff --git a/tests/unit/validation/test_iteration_validator.py b/tests/unit/validation/test_iteration_validator.py index 7475c60db..5c08bb4cc 100644 --- a/tests/unit/validation/test_iteration_validator.py +++ b/tests/unit/validation/test_iteration_validator.py @@ -441,3 +441,32 @@ def test_invalid_iteration_if_more_than_one_cohort_has_the_same_cohort_label( assert label_counts["label_2"] == expected_label_2_error_count, ( f"Expected {expected_label_2_error_count} error for label_2, got {label_counts['label_2']}" ) + + def test_invalid_iteration_if_more_than_one_cohort_has_the_same_priority( + self, valid_campaign_config_with_only_mandatory_fields, valid_iteration_cohorts + ): + iteration_data = { + **valid_campaign_config_with_only_mandatory_fields["Iterations"][0], + "IterationCohorts": [ + valid_iteration_cohorts(label="label_1", priority=1, group="group_1"), + valid_iteration_cohorts(label="label_2", priority=1, group="group_1"), + valid_iteration_cohorts(label="label_3", priority=1, group="group_1"), + ], + } + + with pytest.raises(ValidationError) as error: + IterationValidation(**iteration_data) + + errors = error.value.errors() + # Extract all cohort_label mentions from error inputs + label_mentions = [err["input"] for err in errors if err.get("ctx")] + + # Count occurrences + priority_counts = Counter(label_mentions) + + # Assert expected counts + expected_priority_1_error_count = 2 + + assert priority_counts[1] == expected_priority_1_error_count, ( + f"Expected {expected_priority_1_error_count} errors for priority '1' , got {priority_counts['label_1']}" + ) From 336f90da4244a8817da20b79b053c492e58b8541 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Fri, 28 Nov 2025 11:28:45 +0000 Subject: [PATCH 14/15] cohort priority should be unique --- .../validators/iteration_validator.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 5f5f32143..87950689d 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -25,9 +25,7 @@ def validate_iteration_rules(cls, iteration_rules: list[IterationRule]) -> list[ @field_validator("iteration_cohorts") @classmethod - def validate_iteration_cohorts( - cls, iteration_cohorts: list[IterationCohort] - ) -> list[IterationCohortValidation]: + def validate_iteration_cohorts(cls, iteration_cohorts: list[IterationCohort]) -> list[IterationCohortValidation]: seen_labels = set() seen_priorities = set() errors = [] @@ -61,9 +59,7 @@ def validate_iteration_cohorts( seen_priorities.add(priority) if errors: - raise ValidationError.from_exception_data( - title="IterationValidation", line_errors=errors - ) + raise ValidationError.from_exception_data(title="IterationValidation", line_errors=errors) return [IterationCohortValidation(**i.model_dump()) for i in iteration_cohorts] From d10dd4c8e5fb29557f9805c4a24c117cbaa39f67 Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Fri, 28 Nov 2025 18:23:16 +0000 Subject: [PATCH 15/15] config validator gives verbose result now --- index.html | 32 +++++++---- .../model/campaign_config.py | 12 ++--- src/rules_validation_api/app.py | 53 ++++++++++++++++--- .../decorators/__init__.py | 0 .../decorators/tracker.py | 28 ++++++++++ .../validators/actions_mapper_validator.py | 5 +- .../validators/available_action_validator.py | 2 + .../validators/campaign_config_validator.py | 2 + .../validators/iteration_cohort_validator.py | 2 + .../validators/iteration_rules_validator.py | 2 + .../validators/iteration_validator.py | 2 + .../validators/rules_validator.py | 2 + 12 files changed, 116 insertions(+), 26 deletions(-) create mode 100644 src/rules_validation_api/decorators/__init__.py create mode 100644 src/rules_validation_api/decorators/tracker.py diff --git a/index.html b/index.html index 111769777..153b7a695 100644 --- a/index.html +++ b/index.html @@ -187,18 +187,28 @@

Visualiser Output

const jsonInput = document.getElementById("jsonfile"); const runBtn = document.getElementById("run"); - function log(text) { - let cleanText = text.replace(/&/g, "&").replace(//g, ">"); - // Handle ANSI Colors for Pydantic output - cleanText = cleanText - .replace(/\x1b\[92m/g, '') - .replace(/\x1b\[93m/g, '') - .replace(/\x1b\[91m/g, '') - .replace(/\x1b\[0m/g, ''); +function log(text) { + let cleanText = text.replace(/&/g, "&").replace(//g, ">"); + // ANSI color replacements + cleanText = cleanText + .replace(/\x1b\[92m/g, '') // validator/method green + .replace(/\x1b\[93m/g, '') // general yellow + .replace(/\x1b\[34m/g, '') // blue + .replace(/\x1b\[35m/g, '') // magenta + .replace(/\x1b\[36m/g, '') // cyan + .replace(/\x1b\[94m/g, '') // light blue + .replace(/\x1b\[95m/g, '') // light magenta + .replace(/\x1b\[96m/g, '') // light cyan + .replace(/\x1b\[37m/g, '') // white/light grey + .replace(/\x1b\[33m/g, '') // colon yellow + .replace(/\x1b\[0m/g, ''); // reset - output.innerHTML += cleanText + "\n"; - output.scrollTop = output.scrollHeight; + if (cleanText.includes("Valid Config")) { + cleanText = cleanText.replace(/Valid Config/g, 'Valid Config'); } + output.innerHTML += cleanText + "\n"; + output.scrollTop = output.scrollHeight; +} function clearLog() { output.innerHTML = ""; } // Changed to innerHTML for spans @@ -217,6 +227,8 @@

Visualiser Output

"src/eligibility_signposting_api/model/campaign_config.py", "src/eligibility_signposting_api/config/__init__.py", "src/eligibility_signposting_api/config/constants.py", + "src/rules_validation_api/decorators/__init__.py", + "src/rules_validation_api/decorators/tracker.py", "src/rules_validation_api/__init__.py", "src/rules_validation_api/validators/__init__.py", "src/rules_validation_api/validators/rules_validator.py", diff --git a/src/eligibility_signposting_api/model/campaign_config.py b/src/eligibility_signposting_api/model/campaign_config.py index 492094dfc..321e97666 100644 --- a/src/eligibility_signposting_api/model/campaign_config.py +++ b/src/eligibility_signposting_api/model/campaign_config.py @@ -269,6 +269,12 @@ class Iteration(BaseModel): model_config = {"populate_by_name": True, "arbitrary_types_allowed": True, "extra": "ignore"} + def __init__(self, **data: dict[str, typing.Any]) -> None: + super().__init__(**data) + # Ensure each rule knows its parent iteration + for rule in self.iteration_rules: + rule.set_parent(self) + @field_validator("iteration_date", mode="before") @classmethod def parse_dates(cls, v: str | date) -> date: @@ -292,12 +298,6 @@ def parse_dates(cls, v: str | date) -> date: def serialize_dates(v: date, _info: SerializationInfo) -> str: return v.strftime("%Y%m%d") - @model_validator(mode="after") - def attach_rule_parents(self) -> Iteration: - for rule in self.iteration_rules: - rule.set_parent(self) - return self - def __str__(self) -> str: return json.dumps(self.model_dump(by_alias=True), indent=2) diff --git a/src/rules_validation_api/app.py b/src/rules_validation_api/app.py index 1ff67b782..5bd349066 100644 --- a/src/rules_validation_api/app.py +++ b/src/rules_validation_api/app.py @@ -2,23 +2,38 @@ import json import logging import sys +from collections import defaultdict from pathlib import Path +from rules_validation_api.decorators.tracker import VALIDATORS_CALLED from rules_validation_api.validators.rules_validator import RulesValidation logging.basicConfig( - level=logging.INFO, # or DEBUG for more detail + level=logging.INFO, format="%(asctime)s | %(levelname)s | %(name)s | %(message)s", force=True, ) -GREEN = "\033[92m" # pragma: no cover -RESET = "\033[0m" # pragma: no cover -YELLOW = "\033[93m" # pragma: no cover -RED = "\033[91m" # pragma: no cover +GREEN = "\033[92m" +RESET = "\033[0m" +YELLOW = "\033[93m" +RED = "\033[91m" +# ANSI color codes +LEFT_COLOR = "\033[34m" # Blue for class name +COLON_COLOR = "\033[33m" # Yellow for colon +RIGHT_COLOR = "\033[92m" # Milk green for validator +CLASS_COLORS = [ + "\033[34m", # blue + "\033[35m", # magenta + "\033[36m", # cyan + "\033[94m", # light blue + "\033[95m", # light magenta + "\033[96m", # light cyan + "\033[37m", # white/light grey +] -def main() -> None: # pragma: no cover +def main() -> None: parser = argparse.ArgumentParser(description="Validate campaign configuration.") parser.add_argument("--config_path", required=True, help="Path to the campaign config JSON file") args = parser.parse_args() @@ -28,9 +43,33 @@ def main() -> None: # pragma: no cover json_data = json.load(file) RulesValidation(**json_data) sys.stdout.write(f"{GREEN}Valid Config{RESET}\n") + + # Group by class + grouped = defaultdict(list) + for v in VALIDATORS_CALLED: + cls, method = v.split(":", 1) + grouped[cls].append(method.strip()) + + # Assign colors to classes + cls_color_map = {} + for i, cls_name in enumerate(sorted(grouped.keys(), reverse=True)): + cls_color_map[cls_name] = CLASS_COLORS[i % len(CLASS_COLORS)] + + # Print grouped + for cls_name in sorted(grouped.keys(), reverse=True): + methods = sorted(grouped[cls_name]) + # First method prints class name + first = methods[0] + colored = f"{cls_color_map[cls_name]}{cls_name}{RESET}{COLON_COLOR}:{RESET}{RIGHT_COLOR}{first}{RESET}" + print(colored) + # Rest methods indented + for method_name in methods[1:]: + colored = f"{' ' * len(cls_name)}{COLON_COLOR}:{RESET}{RIGHT_COLOR}{method_name}{RESET}" + print(colored) + except ValueError as e: sys.stderr.write(f"{YELLOW}Validation Error:{RESET} {RED}{e}{RESET}\n") -if __name__ == "__main__": # pragma: no cover +if __name__ == "__main__": main() diff --git a/src/rules_validation_api/decorators/__init__.py b/src/rules_validation_api/decorators/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/rules_validation_api/decorators/tracker.py b/src/rules_validation_api/decorators/tracker.py new file mode 100644 index 000000000..9db3cd298 --- /dev/null +++ b/src/rules_validation_api/decorators/tracker.py @@ -0,0 +1,28 @@ +from typing import Self + +from pydantic import model_validator + +VALIDATORS_CALLED: list[str] = [] + + +# --- Mixin and decorator to track validators --- +class TrackValidatorsMixin: + """ + Mixin to track all validator names in a Pydantic model. + """ + + @model_validator(mode="after") + def _track_validators(self) -> Self: + for name in dir(self): + if name.startswith(("validate_", "check_")) and callable(getattr(self, name)): + full_name = f"{self.__class__.__name__}:{name}" + if full_name not in VALIDATORS_CALLED: + VALIDATORS_CALLED.append(full_name) + return self + + +def track_validators(cls) -> type: # noqa:ANN001 + """ + Decorator to add the tracking mixin to a Pydantic model. + """ + return type(cls.__name__, (TrackValidatorsMixin, cls), {}) diff --git a/src/rules_validation_api/validators/actions_mapper_validator.py b/src/rules_validation_api/validators/actions_mapper_validator.py index 7efdd2452..4ac67bcff 100644 --- a/src/rules_validation_api/validators/actions_mapper_validator.py +++ b/src/rules_validation_api/validators/actions_mapper_validator.py @@ -1,9 +1,11 @@ from pydantic import ValidationError, model_validator from eligibility_signposting_api.model.campaign_config import ActionsMapper +from rules_validation_api.decorators.tracker import track_validators from rules_validation_api.validators.available_action_validator import AvailableActionValidation +@track_validators class ActionsMapperValidation(ActionsMapper): @model_validator(mode="after") def validate_keys(self) -> "ActionsMapperValidation": @@ -16,7 +18,6 @@ def validate_keys(self) -> "ActionsMapperValidation": @model_validator(mode="after") def validate_values(self) -> "ActionsMapperValidation": error_report = [] - for key, value in self.root.items(): try: AvailableActionValidation.model_validate(value.model_dump()) @@ -24,9 +25,7 @@ def validate_values(self) -> "ActionsMapperValidation": for err in e.errors(): msg = err.get("msg", "Unknown error").replace("Value error, ", "") error_report.append(f"\n Action '{key}': {msg}") - if error_report: final_msg = "Markdown Validation Issues:".join(error_report) raise ValueError(final_msg) - return self diff --git a/src/rules_validation_api/validators/available_action_validator.py b/src/rules_validation_api/validators/available_action_validator.py index 07e80c315..74d17e1df 100644 --- a/src/rules_validation_api/validators/available_action_validator.py +++ b/src/rules_validation_api/validators/available_action_validator.py @@ -3,8 +3,10 @@ from pydantic import field_validator from eligibility_signposting_api.model.campaign_config import AvailableAction +from rules_validation_api.decorators.tracker import track_validators +@track_validators class AvailableActionValidation(AvailableAction): @field_validator("action_description") @classmethod diff --git a/src/rules_validation_api/validators/campaign_config_validator.py b/src/rules_validation_api/validators/campaign_config_validator.py index 3b5cf509d..9f474e363 100644 --- a/src/rules_validation_api/validators/campaign_config_validator.py +++ b/src/rules_validation_api/validators/campaign_config_validator.py @@ -5,9 +5,11 @@ from pydantic import field_validator, model_validator from eligibility_signposting_api.model.campaign_config import CampaignConfig, Iteration +from rules_validation_api.decorators.tracker import track_validators from rules_validation_api.validators.iteration_validator import IterationValidation +@track_validators class CampaignConfigValidation(CampaignConfig): @field_validator("iterations") @classmethod diff --git a/src/rules_validation_api/validators/iteration_cohort_validator.py b/src/rules_validation_api/validators/iteration_cohort_validator.py index 32e1a4b3a..c0d8911d4 100644 --- a/src/rules_validation_api/validators/iteration_cohort_validator.py +++ b/src/rules_validation_api/validators/iteration_cohort_validator.py @@ -1,5 +1,7 @@ from eligibility_signposting_api.model.campaign_config import IterationCohort +from rules_validation_api.decorators.tracker import track_validators +@track_validators class IterationCohortValidation(IterationCohort): pass diff --git a/src/rules_validation_api/validators/iteration_rules_validator.py b/src/rules_validation_api/validators/iteration_rules_validator.py index ccce344e8..64d4c64c6 100644 --- a/src/rules_validation_api/validators/iteration_rules_validator.py +++ b/src/rules_validation_api/validators/iteration_rules_validator.py @@ -8,8 +8,10 @@ RuleAttributeName, RuleType, ) +from rules_validation_api.decorators.tracker import track_validators +@track_validators class IterationRuleValidation(IterationRule): @model_validator(mode="after") def check_cohort_attribute_name(self) -> typing.Self: diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 87950689d..5860ae757 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -11,12 +11,14 @@ IterationRule, RuleType, ) +from rules_validation_api.decorators.tracker import track_validators from rules_validation_api.validators.actions_mapper_validator import ActionsMapperValidation from rules_validation_api.validators.available_action_validator import AvailableActionValidation from rules_validation_api.validators.iteration_cohort_validator import IterationCohortValidation from rules_validation_api.validators.iteration_rules_validator import IterationRuleValidation +@track_validators class IterationValidation(Iteration): @field_validator("iteration_rules") @classmethod diff --git a/src/rules_validation_api/validators/rules_validator.py b/src/rules_validation_api/validators/rules_validator.py index cacb143d0..32f574871 100644 --- a/src/rules_validation_api/validators/rules_validator.py +++ b/src/rules_validation_api/validators/rules_validator.py @@ -1,9 +1,11 @@ from pydantic import field_validator from eligibility_signposting_api.model.campaign_config import CampaignConfig, Rules +from rules_validation_api.decorators.tracker import track_validators from rules_validation_api.validators.campaign_config_validator import CampaignConfigValidation +@track_validators class RulesValidation(Rules): @field_validator("campaign_config") @classmethod