From 98aee2477bc94a6653a83a7a5597e802ac74eacf Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 11 Mar 2024 17:46:34 +0100 Subject: [PATCH 1/2] :recycle: [#3977] Let appointments use new formio validation mechanism Instead of the manual validation chain building, the serializer is now constructed and used for validation in the contact details step of appointment forms. This obsoletes the old approach. --- src/openforms/appointments/api/serializers.py | 16 ++++++++++------ .../contrib/jcc/tests/test_plugin.py | 8 +++++--- .../contrib/qmatic/tests/test_plugin.py | 8 +++++--- .../tests/test_api_appointment_create.py | 10 ++++++---- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/openforms/appointments/api/serializers.py b/src/openforms/appointments/api/serializers.py index 0de278a51a..72417c376f 100644 --- a/src/openforms/appointments/api/serializers.py +++ b/src/openforms/appointments/api/serializers.py @@ -7,7 +7,7 @@ from drf_spectacular.utils import extend_schema_field from rest_framework import serializers -from openforms.formio.service import validate_formio_data +from openforms.formio.service import build_serializer from openforms.forms.models import Form from openforms.submissions.api.fields import PrivacyPolicyAcceptedField from openforms.submissions.models import Submission @@ -274,12 +274,16 @@ def validate(self, attrs: dict) -> dict: {"datetime": _("The selected datetime is not available (anymore).")} ) - # 5. Validate contact details against product + # 5. Validate contact details against product(s) contact_details_meta = plugin.get_required_customer_fields(products) - try: - validate_formio_data(contact_details_meta, attrs["contact_details"]) - except serializers.ValidationError as errors: - raise serializers.ValidationError({"contactDetails": errors.detail}) + contact_details_serializer = build_serializer( + contact_details_meta, + data=attrs["contact_details"], + context=self.context, + ) + if not contact_details_serializer.is_valid(): + errors = contact_details_serializer.errors + raise serializers.ValidationError({"contact_details": errors}) # expose additional metadata attrs.update( diff --git a/src/openforms/appointments/contrib/jcc/tests/test_plugin.py b/src/openforms/appointments/contrib/jcc/tests/test_plugin.py index d473bc94a3..7cbe716338 100644 --- a/src/openforms/appointments/contrib/jcc/tests/test_plugin.py +++ b/src/openforms/appointments/contrib/jcc/tests/test_plugin.py @@ -8,7 +8,7 @@ import requests_mock from hypothesis import given, strategies as st -from openforms.formio.validation import build_validation_chain +from openforms.formio.service import build_serializer from openforms.utils.tests.logging import disable_logging from openforms.utils.xml import fromstring from soap.tests.factories import SoapServiceFactory @@ -696,13 +696,15 @@ def test_all_customer_fields_have_required_formio_properties(self): self.assertIn("key", component) self.assertIn("label", component) - def test_can_create_validation_chain_for_formio_fields(self): + def test_can_create_serializer_for_formio_fields(self): for component in FIELD_TO_FORMIO_COMPONENT.values(): assert "key" in component with self.subTest(component=component["key"]): try: - build_validation_chain(component) + serializer = build_serializer([component]) except Exception as exc: raise self.failureException( "Could not create validation chain" ) from exc + + self.assertIn(component["key"], serializer.fields) diff --git a/src/openforms/appointments/contrib/qmatic/tests/test_plugin.py b/src/openforms/appointments/contrib/qmatic/tests/test_plugin.py index fceb442c2f..cd865a166e 100644 --- a/src/openforms/appointments/contrib/qmatic/tests/test_plugin.py +++ b/src/openforms/appointments/contrib/qmatic/tests/test_plugin.py @@ -7,7 +7,7 @@ import requests_mock from hypothesis import given, strategies as st -from openforms.formio.validation import build_validation_chain +from openforms.formio.service import build_serializer from openforms.utils.date import TIMEZONE_AMS from openforms.utils.tests.logging import disable_logging @@ -609,13 +609,15 @@ def test_all_customer_fields_have_required_formio_properties(self): self.assertIn("key", component) self.assertIn("label", component) - def test_can_create_validation_chain_for_formio_fields(self): + def test_can_create_serializer_for_formio_fields(self): for component in FIELD_TO_FORMIO_COMPONENT.values(): assert "key" in component with self.subTest(component=component["key"]): try: - build_validation_chain(component) + serializer = build_serializer([component]) except Exception as exc: raise self.failureException( "Could not create validation chain" ) from exc + + self.assertIn(component["key"], serializer.fields) diff --git a/src/openforms/appointments/tests/test_api_appointment_create.py b/src/openforms/appointments/tests/test_api_appointment_create.py index 418684698f..0e9992be49 100644 --- a/src/openforms/appointments/tests/test_api_appointment_create.py +++ b/src/openforms/appointments/tests/test_api_appointment_create.py @@ -616,9 +616,10 @@ def test_invalid_contact_details(self): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) invalid_params = response.json()["invalidParams"] self.assertEqual(len(invalid_params), 2) - self.assertEqual(invalid_params[0]["name"], "contactDetails.0") + self.assertEqual(invalid_params[0]["name"], "contactDetails.lastName") self.assertEqual(invalid_params[0]["code"], "required") - self.assertEqual(invalid_params[1]["name"], "contactDetails.1") + + self.assertEqual(invalid_params[1]["name"], "contactDetails.email") self.assertEqual(invalid_params[1]["code"], "required") with self.subTest("value too long"): @@ -634,8 +635,9 @@ def test_invalid_contact_details(self): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) invalid_params = response.json()["invalidParams"] + self.assertEqual(len(invalid_params), 2) - self.assertEqual(invalid_params[0]["name"], "contactDetails.0") + self.assertEqual(invalid_params[0]["name"], "contactDetails.lastName") self.assertEqual(invalid_params[0]["code"], "max_length") - self.assertEqual(invalid_params[1]["name"], "contactDetails.1") + self.assertEqual(invalid_params[1]["name"], "contactDetails.email") self.assertEqual(invalid_params[1]["code"], "max_length") From ba34b6fa54e9daf896a761e89f2a9d21a66a0ba2 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 11 Mar 2024 17:47:17 +0100 Subject: [PATCH 2/2] :coffin: [#3977] Delete obsoleted manual formio validation code The serializer approach is superior, this code is no longer used or relevant. --- src/openforms/formio/service.py | 2 - src/openforms/formio/validation.py | 131 ----------------------------- 2 files changed, 133 deletions(-) delete mode 100644 src/openforms/formio/validation.py diff --git a/src/openforms/formio/service.py b/src/openforms/formio/service.py index e03733cf4f..8a753cd6d8 100644 --- a/src/openforms/formio/service.py +++ b/src/openforms/formio/service.py @@ -29,7 +29,6 @@ from .serializers import build_serializer as _build_serializer from .typing import Component from .utils import iter_components, iterate_data_with_components, recursive_apply -from .validation import validate_formio_data from .variables import inject_variables __all__ = [ @@ -41,7 +40,6 @@ "rewrite_formio_components_for_request", "FormioData", "iterate_data_with_components", - "validate_formio_data", "recursive_apply", "build_serializer", ] diff --git a/src/openforms/formio/validation.py b/src/openforms/formio/validation.py deleted file mode 100644 index 45e4b38a90..0000000000 --- a/src/openforms/formio/validation.py +++ /dev/null @@ -1,131 +0,0 @@ -""" -Server-side validation for Form.io data. -""" - -from typing import Any, Callable - -from django.core.exceptions import ValidationError as DjangoValidationError -from django.core.validators import ( - EmailValidator as _EmailValidator, - MaxLengthValidator as _MaxLengthValidator, -) -from django.utils.translation import gettext_lazy as _ - -from glom import assign -from rest_framework.fields import get_error_detail -from rest_framework.serializers import ValidationError - -from openforms.typing import JSONObject - -from .datastructures import FormioConfigurationWrapper, FormioData -from .typing import Component -from .utils import get_component_empty_value - -missing = object() - - -class RequiredValidator: - def __init__(self, empty_value): - self.empty_value = empty_value - - def __call__(self, value): - if value is missing or value == self.empty_value or value is None: - raise ValidationError( - _("You must provide a non-empty value."), code="required" - ) - return value - - -class ReturnValueMixin: - def __call__(self, value): - super().__call__(value) # type: ignore - return value - - -class EmailValidator(ReturnValueMixin, _EmailValidator): - pass - - -class MaxLengthValidator(ReturnValueMixin, _MaxLengthValidator): - pass - - -Validator = Callable[[Any], Any] - - -class ValidatorChain: - def __init__(self, validators: list[Validator]): - self.validators = validators - - def __call__(self, value: Any) -> Any: - for validator in self.validators: - value = validator(value) - return value - - -# XXX: if we start using this more broadly, see how we can leverage the component -# registry. -def build_validation_chain(component: Component) -> ValidatorChain: - assert "type" in component - if "validate" not in component: - return ValidatorChain([]) - - validators = [] - - # add required validator, generic for all component types - match component["validate"]: - case {"required": True}: - empty_value = get_component_empty_value(component) - validators.append(RequiredValidator(empty_value)) - - if component["type"] == "email": - validators.append(EmailValidator()) - - # text based validators - match component: - case { - "type": "textfield" | "email" | "phoneNumber" | "bsn", - "validate": {"maxLength": max_length}, - } if isinstance(max_length, int): - validators.append(MaxLengthValidator(max_length)) - - return ValidatorChain(validators) - - -def validate_formio_data(components, values: JSONObject) -> None: - """ - Minimal form.io validation. - - Supports: - - - required - - maxLength - - email - - """ - wrapper = FormioConfigurationWrapper({"components": components}) - paths = wrapper.reverse_flattened - data = FormioData(values) - - errors = {} - - for component in wrapper: - assert "key" in component - chain = build_validation_chain(component) - path = paths[component["key"]] - value = data.get(component["key"], default=missing) - try: - chain(value) - except (ValidationError, DjangoValidationError) as exc: - error_detail = ( - get_error_detail(exc) - if isinstance(exc, DjangoValidationError) - else exc.detail - ) - assert ( - len(error_detail) == 1 - ), "Expected only a single validation error for a component at a time" - assign(errors, path, error_detail[0], missing=dict) # type: ignore - - if errors: - raise ValidationError(errors["components"])