From 170f3173f45846b4e6b3daedf4bf7dfa9bbb49ed Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 27 Mar 2024 11:06:55 +0100 Subject: [PATCH 1/2] :test_tube: [#4065] Add regression test with various cases of hidden components --- .../formio/tests/validation/test_textfield.py | 13 +++ .../e2e/test_input_validation_regressions.py | 92 +++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 src/openforms/tests/e2e/test_input_validation_regressions.py diff --git a/src/openforms/formio/tests/validation/test_textfield.py b/src/openforms/formio/tests/validation/test_textfield.py index 1f8409f670..5720bd7e0e 100644 --- a/src/openforms/formio/tests/validation/test_textfield.py +++ b/src/openforms/formio/tests/validation/test_textfield.py @@ -32,6 +32,19 @@ def test_textfield_required_validation(self): error = extract_error(errors, component["key"]) self.assertEqual(error.code, error_code) + def test_hidden_textfield_required_validation(self): + component: TextFieldComponent = { + "type": "textfield", + "key": "foo.bar", + "label": "Test", + "validate": {"required": True}, + "hidden": True, + } + + is_valid = validate_formio_data(component, {"foo": {"bar": ""}}) + + self.assertTrue(is_valid) + def test_textfield_max_length(self): component: TextFieldComponent = { "type": "textfield", diff --git a/src/openforms/tests/e2e/test_input_validation_regressions.py b/src/openforms/tests/e2e/test_input_validation_regressions.py new file mode 100644 index 0000000000..ba26792ac1 --- /dev/null +++ b/src/openforms/tests/e2e/test_input_validation_regressions.py @@ -0,0 +1,92 @@ +from django.test import override_settings, tag +from django.urls import reverse + +from asgiref.sync import sync_to_async +from furl import furl +from playwright.async_api import expect + +from openforms.forms.tests.factories import FormFactory + +from .base import E2ETestCase, browser_page + + +# allow all origins, since we don't know exactly the generated live server port number +@override_settings(CORS_ALLOW_ALL_ORIGINS=True) +class InputValidationRegressionTests(E2ETestCase): + + @tag("gh-4065") + async def test_hidden_components_validation(self): + + @sync_to_async + def setUpTestData(): + form = FormFactory.create( + slug="validation", + generate_minimal_setup=True, + registration_backend=None, + translation_enabled=False, # force Dutch + ask_privacy_consent=False, + ask_statement_of_truth=False, + formstep__next_text="Volgende", + formstep__form_definition__configuration={ + "components": [ + { + "type": "textfield", + "key": "textfield", + "label": "Visible text field", + "validate": {"required": True}, + }, + { + "type": "number", + "key": "number", + "label": "Hidden number, clearOnHide", + "validate": {"required": True}, + "hidden": True, + "clearOnHide": True, + }, + { + "type": "date", + "key": "date", + "label": "Hidden number, no clearOnHide", + "validate": {"required": True}, + "hidden": True, + "clearOnHide": False, + "defaultValue": "2024-03-27", + }, + { + "type": "bsn", + "key": "bsn", + "label": "Conditionally hidden", + "validate": {"required": True}, + "conditional": { + "show": True, + "when": "textfield", + "eq": "show me the bsn", + }, + "clearOnHide": False, + "defaultValue": "123456781", # invalid + }, + ] + }, + ) + return form + + form = await setUpTestData() + form_url = str( + furl(self.live_server_url) + / reverse("forms:form-detail", kwargs={"slug": form.slug}) + ) + + async with browser_page() as page: + await page.goto(form_url) + # Start the form + await page.get_by_role("button", name="Formulier starten").click() + + # Fill out the visible field + await page.get_by_label("Visible text field").fill("testing") + await page.get_by_role("button", name="Volgende").click() + + # Confirm and finish the form + await page.get_by_role("button", name="Verzenden").click() + await expect( + page.get_by_text("Een moment geduld", exact=False) + ).to_be_visible() From 85290120942d2a494ef81c4d008a5e9ca8d888ca Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 27 Mar 2024 12:10:59 +0100 Subject: [PATCH 2/2] :bug: [#4065] Skip validation in serializer fields when components are not visible This mimicks the validation logic from Formio itself, which forces a component to be valid when it's not visible or conditionally hidden. --- src/openforms/formio/serializers.py | 68 +++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/src/openforms/formio/serializers.py b/src/openforms/formio/serializers.py index 115f4ca654..253a206d1f 100644 --- a/src/openforms/formio/serializers.py +++ b/src/openforms/formio/serializers.py @@ -10,9 +10,12 @@ from typing import TYPE_CHECKING, TypeAlias -from glom import assign +from glom import assign, glom from rest_framework import serializers +from openforms.typing import DataMapping, JSONObject + +from .datastructures import FormioConfigurationWrapper from .typing import Component from .utils import iter_components @@ -23,9 +26,59 @@ FieldOrNestedFields: TypeAlias = serializers.Field | dict[str, "FieldOrNestedFields"] +class StepDataSerializer(serializers.Serializer): + + def apply_hidden_state( + self, configuration: JSONObject, fields: dict[str, FieldOrNestedFields] + ) -> None: + """ + Apply the hidden/visible state of the formio components to the serializer. + + Components may be hidden in a static fashing (through the 'hidden' property), + or be hidden dynamically based on the input data via the 'conditional' property. + + Hidden fields in Formio don't run *any* validation, see the + ``Component.shouldSkipValidation`` method for reference. + """ + config_wrapper = FormioConfigurationWrapper(configuration) + + # initial_data is only set if Serializer(data=...) was used, and we can't take + # (dynamic) hidden state into account without initial data. + if not hasattr(self, "initial_data"): + return + + # can't use FormioData yet because of is_visible_in_frontend + values: DataMapping = self.initial_data + + # loop over all components and delegate application to the registry + for component in iter_components(configuration, recurse_into_editgrid=False): + # XXX: is_visible_in_frontend does not understand editgrid at all yet, which + # is a broader issue, but also manifests here. + is_visible = config_wrapper.is_visible_in_frontend(component["key"], values) + + # we don't have to do anything when the component is visible, regular + # validation rules apply + if is_visible: + continue + + # when it's not visible, grab the field from the serializer and remove all + # the validators to match Formio's behaviour. + serializer_field = glom(fields, component["key"]) + self._remove_validations_from_field(serializer_field) + + def _remove_validations_from_field(self, field: serializers.Field) -> None: + field.required = False + # note that prefilled fields are validated separately when they're read-only + # to protect against tampering + field.allow_null = True + # reset the validators to the default validators, discarding any additional ones + # added based on component['validate'] + field.validators = field.get_validators() + + def dict_to_serializer( fields: dict[str, FieldOrNestedFields], **kwargs -) -> serializers.Serializer: +) -> StepDataSerializer: """ Recursively convert a mapping of field names to a serializer instance. @@ -33,7 +86,7 @@ def dict_to_serializer( serializer field instances or nested mappings. Nested mappings result in nested serializers. """ - serializer = serializers.Serializer(**kwargs) + serializer = StepDataSerializer(**kwargs) for bit, field in fields.items(): match field: @@ -51,7 +104,7 @@ def dict_to_serializer( def build_serializer( components: list[Component], register: ComponentRegistry, **kwargs -) -> serializers.Serializer: +) -> StepDataSerializer: """ Translate a sequence of Formio.js component definitions into a serializer. @@ -60,12 +113,11 @@ def build_serializer( """ fields: dict[str, FieldOrNestedFields] = {} - # TODO: check that editgrid nested components are not yielded here! - for component in iter_components( - {"components": components}, recurse_into_editgrid=False - ): + config: JSONObject = {"components": components} + for component in iter_components(config, recurse_into_editgrid=False): field = register.build_serializer_field(component) assign(obj=fields, path=component["key"], val=field, missing=dict) serializer = dict_to_serializer(fields, **kwargs) + serializer.apply_hidden_state(config, fields) return serializer