From a8c40bb6e9f95146790f6bccf7d871f3037d2190 Mon Sep 17 00:00:00 2001 From: Kegan Maher Date: Tue, 26 Nov 2024 22:37:12 +0000 Subject: [PATCH] fix(models): invert clean, catch and rethrow ValidationError TransitAgency cleans all of its associated EnrollmentFlows, to ensure that an agency doesn't go live with partially configured flows since TransitAgency won't have fields from EnrollmentFlow, any ValidationError that bubbles would raise a new error in Django Admin instead, catch and rethrow a ValidationError indicating the problem flow --- benefits/core/models.py | 8 ++++++-- tests/pytest/core/test_models.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/benefits/core/models.py b/benefits/core/models.py index 64a097587..25525d918 100644 --- a/benefits/core/models.py +++ b/benefits/core/models.py @@ -313,6 +313,12 @@ def clean(self): template_errors = [] if self.active: + for flow in self.enrollment_flows.all(): + try: + flow.clean() + except ValidationError: + raise ValidationError(f"Invalid EnrollmentFlow: {flow.label}") + message = "This field is required for active transit agencies." needed = dict( short_name=self.short_name, @@ -625,8 +631,6 @@ def clean(self): field_errors.update(reenrollment_error_template=ValidationError("Required when supports expiration is True.")) if self.transit_agency and self.transit_agency.active: - self.transit_agency.clean() - if self.claims_provider: message = "Required for claims verification." needed = dict( diff --git a/tests/pytest/core/test_models.py b/tests/pytest/core/test_models.py index 021e6dd33..679170414 100644 --- a/tests/pytest/core/test_models.py +++ b/tests/pytest/core/test_models.py @@ -630,6 +630,18 @@ def test_TransitAgency_clean_templates(model_TransitAgency_inactive, template_at model_TransitAgency_inactive.clean() +@pytest.mark.django_db +def test_TransitAgency_clean_dirty_flow(model_TransitAgency, model_EnrollmentFlow, model_ClaimsProvider): + # partially setup the EnrollmentFlow + # missing scope and claims + model_EnrollmentFlow.claims_provider = model_ClaimsProvider + model_EnrollmentFlow.transit_agency = model_TransitAgency + + # clean the agency, and expect an invalid EnrollmentFlow error + with pytest.raises(ValidationError, match=f"Invalid EnrollmentFlow: {model_EnrollmentFlow.label}"): + model_TransitAgency.clean() + + @pytest.mark.django_db def test_EnrollmentEvent_create(model_TransitAgency, model_EnrollmentFlow): ts = timezone.now()