From 5f6ef58bbb6baaffecc6c83aabdad66a7966b5e4 Mon Sep 17 00:00:00 2001 From: Sambit Kumar Mishra Date: Fri, 20 Mar 2026 15:41:33 +0000 Subject: [PATCH 1/7] Changes for ELI-595 --- .../validators/iteration_validator.py | 21 +++++++++++++++++++ .../validation/test_iteration_validator.py | 16 ++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index d4934fd25..936f56a80 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -97,6 +97,27 @@ def transform_actions_mapper(cls, action_mapper: ActionsMapper) -> ActionsMapper action_mapper.root = new_root return action_mapper + @model_validator(mode="after") + def validate_rule_cohort_labels_against_iteration_cohorts(self) -> typing.Self: + allowed_labels = {cohort.cohort_label for cohort in self.iteration_cohorts} + print(allowed_labels) + + for idx, rule in enumerate(self.iteration_rules): + if rule.cohort_label is None: + continue + print("parsed cohort labels from rules:") + print(rule.parsed_cohort_labels) + if not all(label in allowed_labels for label in rule.parsed_cohort_labels): + allowed_str = ", ".join(sorted(allowed_labels)) + msg = ( + f"Invalid cohort_label value: {rule.cohort_label}. " + f"Allowed values: {allowed_str}. " + f"Rule index: {idx}" + ) + raise ValueError(msg) + + return self + @model_validator(mode="after") def action_mapper_validation(self) -> typing.Self: all_errors = [] diff --git a/tests/unit/validation/test_iteration_validator.py b/tests/unit/validation/test_iteration_validator.py index df63cfc11..31492f2f2 100644 --- a/tests/unit/validation/test_iteration_validator.py +++ b/tests/unit/validation/test_iteration_validator.py @@ -570,3 +570,19 @@ def test_iteration_full_datetime_validation( # noqa : PLR0913 f"Failed! Input: {iteration_time_input}, Default: {default_time_iteration_input}. " f"Expected {expected_date_time} but got {result}" ) + + def test_iteration_rules_having_invalid_cohort_labels_throws_error( + self, valid_iteration_with_only_mandatory_fields, + valid_iteration_rule_with_only_mandatory_fields, + valid_iteration_cohorts, + ): + data = valid_iteration_with_only_mandatory_fields.copy() + data["iteration_rules"] = [valid_iteration_rule_with_only_mandatory_fields] + data["iteration_cohorts"] = [valid_iteration_cohorts] + data["iteration_rules"][0]["CohortLabel"] = "label_2" + IterationValidation(**(data)) + # try: + + # IterationValidation(**(data)) + # except ValidationError as e: + # pytest.fail(f"Unexpected error during model instantiation: {e}") From 5a7d720f59c737e8eb4c3a4d1f4dc13248f930ff Mon Sep 17 00:00:00 2001 From: karthikeyannhs <174426205+Karthikeyannhs@users.noreply.github.com> Date: Mon, 23 Mar 2026 10:28:16 +0000 Subject: [PATCH 2/7] ELI-595 | KT | updated the names of fields used in testcases (they are pascal case) --- .../validators/iteration_validator.py | 7 +----- .../test_campaign_config_validator.py | 5 +--- .../test_iteration_rules_validator.py | 5 +--- .../validation/test_iteration_validator.py | 25 ++++++++----------- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 936f56a80..27ed9102b 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -100,19 +100,14 @@ def transform_actions_mapper(cls, action_mapper: ActionsMapper) -> ActionsMapper @model_validator(mode="after") def validate_rule_cohort_labels_against_iteration_cohorts(self) -> typing.Self: allowed_labels = {cohort.cohort_label for cohort in self.iteration_cohorts} - print(allowed_labels) for idx, rule in enumerate(self.iteration_rules): if rule.cohort_label is None: continue - print("parsed cohort labels from rules:") - print(rule.parsed_cohort_labels) if not all(label in allowed_labels for label in rule.parsed_cohort_labels): allowed_str = ", ".join(sorted(allowed_labels)) msg = ( - f"Invalid cohort_label value: {rule.cohort_label}. " - f"Allowed values: {allowed_str}. " - f"Rule index: {idx}" + f"Invalid cohort_label value: {rule.cohort_label}. Allowed values: {allowed_str}. Rule index: {idx}" ) raise ValueError(msg) diff --git a/tests/unit/validation/test_campaign_config_validator.py b/tests/unit/validation/test_campaign_config_validator.py index c0e47e981..7782621b1 100644 --- a/tests/unit/validation/test_campaign_config_validator.py +++ b/tests/unit/validation/test_campaign_config_validator.py @@ -13,10 +13,7 @@ class TestMandatoryFieldsSchemaValidations: def test_campaign_config_with_only_mandatory_fields_configuration( self, valid_campaign_config_with_only_mandatory_fields ): - try: - CampaignConfigValidation(**valid_campaign_config_with_only_mandatory_fields) - except ValidationError as e: - pytest.fail(f"Unexpected error during model instantiation: {e}") + CampaignConfigValidation(**valid_campaign_config_with_only_mandatory_fields) @pytest.mark.parametrize( "mandatory_field", diff --git a/tests/unit/validation/test_iteration_rules_validator.py b/tests/unit/validation/test_iteration_rules_validator.py index 1c33f4623..da2e92f43 100644 --- a/tests/unit/validation/test_iteration_rules_validator.py +++ b/tests/unit/validation/test_iteration_rules_validator.py @@ -9,10 +9,7 @@ class TestMandatoryFieldsSchemaValidations: def test_campaign_config_with_only_mandatory_fields_configuration( self, valid_iteration_rule_with_only_mandatory_fields ): - try: - IterationRuleValidation(**valid_iteration_rule_with_only_mandatory_fields) - except ValidationError as e: - pytest.fail(f"Unexpected error during model instantiation: {e}") + IterationRuleValidation(**valid_iteration_rule_with_only_mandatory_fields) @pytest.mark.parametrize( "mandatory_field", diff --git a/tests/unit/validation/test_iteration_validator.py b/tests/unit/validation/test_iteration_validator.py index 31492f2f2..b091c054d 100644 --- a/tests/unit/validation/test_iteration_validator.py +++ b/tests/unit/validation/test_iteration_validator.py @@ -14,10 +14,7 @@ class TestMandatoryFieldsSchemaValidations: def test_campaign_config_with_only_mandatory_fields_configuration( self, valid_campaign_config_with_only_mandatory_fields ): - try: - IterationValidation(**(valid_campaign_config_with_only_mandatory_fields["Iterations"][0])) - except ValidationError as e: - pytest.fail(f"Unexpected error during model instantiation: {e}") + IterationValidation(**(valid_campaign_config_with_only_mandatory_fields["Iterations"][0])) @pytest.mark.parametrize( "mandatory_field", @@ -556,7 +553,7 @@ def test_iteration_full_datetime_validation( # noqa : PLR0913 data = valid_campaign_config_with_only_mandatory_fields.copy() if default_time_iteration_input: - data["iteration_time"] = default_time_iteration_input + data["IterationTime"] = default_time_iteration_input data["Iterations"] = [iteration_data] @@ -572,17 +569,15 @@ def test_iteration_full_datetime_validation( # noqa : PLR0913 ) def test_iteration_rules_having_invalid_cohort_labels_throws_error( - self, valid_iteration_with_only_mandatory_fields, + self, + valid_iteration_with_only_mandatory_fields, valid_iteration_rule_with_only_mandatory_fields, valid_iteration_cohorts, ): data = valid_iteration_with_only_mandatory_fields.copy() - data["iteration_rules"] = [valid_iteration_rule_with_only_mandatory_fields] - data["iteration_cohorts"] = [valid_iteration_cohorts] - data["iteration_rules"][0]["CohortLabel"] = "label_2" - IterationValidation(**(data)) - # try: - - # IterationValidation(**(data)) - # except ValidationError as e: - # pytest.fail(f"Unexpected error during model instantiation: {e}") + data["IterationRules"] = [valid_iteration_rule_with_only_mandatory_fields] + data["IterationCohorts"] = [valid_iteration_cohorts] + data["IterationRules"][0]["CohortLabel"] = "label_2" + + with pytest.raises(ValidationError): + IterationValidation(**(data)) From 0bb1fb629d022e871dc413b2f5dc85591c96a6d1 Mon Sep 17 00:00:00 2001 From: Sambit Kumar Mishra Date: Mon, 23 Mar 2026 11:20:17 +0000 Subject: [PATCH 3/7] Added Test case --- tests/unit/validation/test_iteration_validator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/validation/test_iteration_validator.py b/tests/unit/validation/test_iteration_validator.py index b091c054d..3003611ef 100644 --- a/tests/unit/validation/test_iteration_validator.py +++ b/tests/unit/validation/test_iteration_validator.py @@ -576,7 +576,7 @@ def test_iteration_rules_having_invalid_cohort_labels_throws_error( ): data = valid_iteration_with_only_mandatory_fields.copy() data["IterationRules"] = [valid_iteration_rule_with_only_mandatory_fields] - data["IterationCohorts"] = [valid_iteration_cohorts] + data["IterationCohorts"] = [valid_iteration_cohorts()] data["IterationRules"][0]["CohortLabel"] = "label_2" with pytest.raises(ValidationError): From 59734d1244390ef6fb193a94e02dae75d9dedc37 Mon Sep 17 00:00:00 2001 From: Sambit Kumar Mishra Date: Tue, 24 Mar 2026 09:58:20 +0000 Subject: [PATCH 4/7] Resolving copilot code smells --- .../validators/iteration_validator.py | 22 ++++++++++++++----- .../validation/test_iteration_validator.py | 11 ++++++++-- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 27ed9102b..57fd5071e 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -100,16 +100,26 @@ def transform_actions_mapper(cls, action_mapper: ActionsMapper) -> ActionsMapper @model_validator(mode="after") def validate_rule_cohort_labels_against_iteration_cohorts(self) -> typing.Self: allowed_labels = {cohort.cohort_label for cohort in self.iteration_cohorts} + line_errors: list[InitErrorDetails] = [] for idx, rule in enumerate(self.iteration_rules): if rule.cohort_label is None: continue - if not all(label in allowed_labels for label in rule.parsed_cohort_labels): - allowed_str = ", ".join(sorted(allowed_labels)) - msg = ( - f"Invalid cohort_label value: {rule.cohort_label}. Allowed values: {allowed_str}. Rule index: {idx}" - ) - raise ValueError(msg) + + for label in rule.parsed_cohort_labels: + if label not in allowed_labels: + allowed_str = ", ".join(sorted(allowed_labels)) + error = InitErrorDetails( + type="value_error", + loc=("iteration_rules", idx, "cohort_label"), + input=rule.cohort_label, + ctx={ + "error": f"Invalid cohort_label value '{label}'. Allowed values: {allowed_str}." + }, + ) + line_errors.append(error) + if line_errors: + raise ValidationError.from_exception_data(title="IterationValidation", line_errors=line_errors) return self diff --git a/tests/unit/validation/test_iteration_validator.py b/tests/unit/validation/test_iteration_validator.py index 3003611ef..0d6c3002d 100644 --- a/tests/unit/validation/test_iteration_validator.py +++ b/tests/unit/validation/test_iteration_validator.py @@ -579,5 +579,12 @@ def test_iteration_rules_having_invalid_cohort_labels_throws_error( data["IterationCohorts"] = [valid_iteration_cohorts()] data["IterationRules"][0]["CohortLabel"] = "label_2" - with pytest.raises(ValidationError): - IterationValidation(**(data)) + with pytest.raises(ValidationError) as exc_info: + IterationValidation(**data) + + errors = exc_info.value.errors() + # Ensure at least one error is specifically about the invalid CohortLabel in IterationRules[0] + assert any( + err.get("loc", [])[:3] == ("iteration_rules", 0, "cohort_label") + for err in errors + ) From cfc0ec7b954dd65b27370b38a7a0de8c90aa2dae Mon Sep 17 00:00:00 2001 From: Sambit Kumar Mishra Date: Tue, 24 Mar 2026 10:30:42 +0000 Subject: [PATCH 5/7] Resolving copilot code smells --- .../validators/iteration_validator.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 57fd5071e..6aa485632 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -108,14 +108,21 @@ def validate_rule_cohort_labels_against_iteration_cohorts(self) -> typing.Self: for label in rule.parsed_cohort_labels: if label not in allowed_labels: - allowed_str = ", ".join(sorted(allowed_labels)) + if allowed_labels: + allowed_str = ", ".join(sorted(allowed_labels)) + error_message = ( + f"Invalid cohort_label value '{label}'. Allowed values: {allowed_str}." + ) + else: + error_message = ( + f"Invalid cohort_label value '{label}'. " + "No iteration cohorts are defined, so no cohort labels are allowed." + ) error = InitErrorDetails( type="value_error", loc=("iteration_rules", idx, "cohort_label"), input=rule.cohort_label, - ctx={ - "error": f"Invalid cohort_label value '{label}'. Allowed values: {allowed_str}." - }, + ctx={"error": error_message}, ) line_errors.append(error) if line_errors: From a394d350a137916e24ddb93484a5305224b07705 Mon Sep 17 00:00:00 2001 From: Sambit Kumar Mishra Date: Tue, 24 Mar 2026 14:59:35 +0000 Subject: [PATCH 6/7] Lint fix --- .../validators/iteration_validator.py | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 6aa485632..4bebdf197 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -99,34 +99,45 @@ def transform_actions_mapper(cls, action_mapper: ActionsMapper) -> ActionsMapper @model_validator(mode="after") def validate_rule_cohort_labels_against_iteration_cohorts(self) -> typing.Self: - allowed_labels = {cohort.cohort_label for cohort in self.iteration_cohorts} + allowed_labels = {c.cohort_label for c in self.iteration_cohorts} line_errors: list[InitErrorDetails] = [] + # Pre‑compute allowed label string once + allowed_str = ", ".join(sorted(allowed_labels)) if allowed_labels else None + for idx, rule in enumerate(self.iteration_rules): - if rule.cohort_label is None: + if not rule.cohort_label: continue for label in rule.parsed_cohort_labels: - if label not in allowed_labels: - if allowed_labels: - allowed_str = ", ".join(sorted(allowed_labels)) - error_message = ( - f"Invalid cohort_label value '{label}'. Allowed values: {allowed_str}." - ) - else: - error_message = ( - f"Invalid cohort_label value '{label}'. " - "No iteration cohorts are defined, so no cohort labels are allowed." - ) - error = InitErrorDetails( + if label in allowed_labels: + continue + + # Build error message + error_message = ( + f"Invalid cohort_label value '{label}'. " + f"Allowed values: {allowed_str}." + if allowed_str + else ( + f"Invalid cohort_label value '{label}'. " + "No iteration cohorts are defined, so no labels are allowed." + ) + ) + + line_errors.append( + InitErrorDetails( type="value_error", loc=("iteration_rules", idx, "cohort_label"), input=rule.cohort_label, ctx={"error": error_message}, ) - line_errors.append(error) + ) + if line_errors: - raise ValidationError.from_exception_data(title="IterationValidation", line_errors=line_errors) + raise ValidationError.from_exception_data( + title="IterationValidation", + line_errors=line_errors + ) return self From 115f2a3ddb994aa03b779fdc357b40bf95b6b55d Mon Sep 17 00:00:00 2001 From: Sambit Kumar Mishra Date: Tue, 24 Mar 2026 15:07:14 +0000 Subject: [PATCH 7/7] removed - in comment for lint --- .../validators/iteration_validator.py | 10 +++------- tests/unit/validation/test_iteration_validator.py | 5 +---- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/rules_validation_api/validators/iteration_validator.py b/src/rules_validation_api/validators/iteration_validator.py index 4bebdf197..1c0f713fc 100644 --- a/src/rules_validation_api/validators/iteration_validator.py +++ b/src/rules_validation_api/validators/iteration_validator.py @@ -102,7 +102,7 @@ def validate_rule_cohort_labels_against_iteration_cohorts(self) -> typing.Self: allowed_labels = {c.cohort_label for c in self.iteration_cohorts} line_errors: list[InitErrorDetails] = [] - # Pre‑compute allowed label string once + # Pre compute allowed label string once allowed_str = ", ".join(sorted(allowed_labels)) if allowed_labels else None for idx, rule in enumerate(self.iteration_rules): @@ -115,8 +115,7 @@ def validate_rule_cohort_labels_against_iteration_cohorts(self) -> typing.Self: # Build error message error_message = ( - f"Invalid cohort_label value '{label}'. " - f"Allowed values: {allowed_str}." + f"Invalid cohort_label value '{label}'. Allowed values: {allowed_str}." if allowed_str else ( f"Invalid cohort_label value '{label}'. " @@ -134,10 +133,7 @@ def validate_rule_cohort_labels_against_iteration_cohorts(self) -> typing.Self: ) if line_errors: - raise ValidationError.from_exception_data( - title="IterationValidation", - line_errors=line_errors - ) + raise ValidationError.from_exception_data(title="IterationValidation", line_errors=line_errors) return self diff --git a/tests/unit/validation/test_iteration_validator.py b/tests/unit/validation/test_iteration_validator.py index 0d6c3002d..a9ad48e91 100644 --- a/tests/unit/validation/test_iteration_validator.py +++ b/tests/unit/validation/test_iteration_validator.py @@ -584,7 +584,4 @@ def test_iteration_rules_having_invalid_cohort_labels_throws_error( errors = exc_info.value.errors() # Ensure at least one error is specifically about the invalid CohortLabel in IterationRules[0] - assert any( - err.get("loc", [])[:3] == ("iteration_rules", 0, "cohort_label") - for err in errors - ) + assert any(err.get("loc", [])[:3] == ("iteration_rules", 0, "cohort_label") for err in errors)