Skip to content

Commit

Permalink
Merge pull request #4066 from open-formulieren/issue/4065-hidden-comp…
Browse files Browse the repository at this point in the history
…onent-validation

Fix hidden component validation blocking submission completion
  • Loading branch information
sergei-maertens authored Mar 27, 2024
2 parents 42f4f6a + 8529012 commit db0dbd3
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 8 deletions.
68 changes: 60 additions & 8 deletions src/openforms/formio/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -23,17 +26,67 @@
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.
Keys are the names of the serializer fields to use, while the values can be
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:
Expand All @@ -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.
Expand All @@ -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
13 changes: 13 additions & 0 deletions src/openforms/formio/tests/validation/test_textfield.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
92 changes: 92 additions & 0 deletions src/openforms/tests/e2e/test_input_validation_regressions.py
Original file line number Diff line number Diff line change
@@ -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()

0 comments on commit db0dbd3

Please sign in to comment.