From c98ed4ac9462a0adaa40bd74fe0587ddf350ebac Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 5 Apr 2024 15:35:39 +0200 Subject: [PATCH 1/3] :test_tube: [DH#673] Add regression test for conditional inside edit grid Components inside the edit grid are conditionally made visible, which affects whether they're required or not. --- .../formio/tests/validation/test_editgrid.py | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/openforms/formio/tests/validation/test_editgrid.py b/src/openforms/formio/tests/validation/test_editgrid.py index c9affb68ed..32c2470cb1 100644 --- a/src/openforms/formio/tests/validation/test_editgrid.py +++ b/src/openforms/formio/tests/validation/test_editgrid.py @@ -1,7 +1,7 @@ from django.test import SimpleTestCase, tag from openforms.submissions.tests.factories import SubmissionFactory -from openforms.typing import JSONObject +from openforms.typing import JSONObject, JSONValue from ...service import build_serializer from ...typing import EditGridComponent, FieldsetComponent @@ -297,3 +297,44 @@ def test_regression_dh_ooievaarspas(self): is_valid = serializer.is_valid() self.assertTrue(is_valid) + + @tag("dh-673") + def test_conditional_hidden_inside_editgrid(self): + component: EditGridComponent = { + "type": "editgrid", + "key": "editgrid", + "label": "Edit grid with nested conditional", + "components": [ + { + "type": "textfield", + "key": "textfield1", + "label": "text field 1", + "validate": {"required": True}, + }, + { + "type": "textfield", + "key": "textfield2", + "label": "text field 2", + "validate": {"required": True}, + "conditional": { # type: ignore + "eq": "SHOW_FIELD_2", + "show": True, + "when": "editgrid.textfield1", + }, + }, + ], + } + + with self.subTest("invalid"): + invalid_data: JSONValue = {"editgrid": [{"textfield1": "SHOW_FIELD_2"}]} + + is_valid, _ = validate_formio_data(component, invalid_data) + + self.assertFalse(is_valid) + + with self.subTest("valid"): + invalid_data: JSONValue = {"editgrid": [{"textfield1": "NO_SHOW_FIELD_2"}]} + + is_valid, _ = validate_formio_data(component, invalid_data) + + self.assertTrue(is_valid) From 5d5fe5d5e175754b3b150050adb95760efd39e8d Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 5 Apr 2024 23:05:22 +0200 Subject: [PATCH 2/3] :bug: [#4126] Rework editgrid serializer field setup We cannot simply use ListField, as that applies the same child serializer for every item inside the array of values, but the field configuration of the child actually changes depending on the values passed in (some fields may be hidden, and that disables their validations entirely). Note the disclaimer about rest_framework.fields.Field.__repr__, this method behaves unexpectedly because it doesn't report the current values of args/kwargs, but the values at the time of instance creation. --- src/openforms/formio/components/vanilla.py | 118 +++++++++++++++-- src/openforms/formio/serializers.py | 9 +- .../formio/tests/validation/test_editgrid.py | 122 +++++++++++++++++- 3 files changed, 231 insertions(+), 18 deletions(-) diff --git a/src/openforms/formio/components/vanilla.py b/src/openforms/formio/components/vanilla.py index c1a33daf91..cab5e5372a 100644 --- a/src/openforms/formio/components/vanilla.py +++ b/src/openforms/formio/components/vanilla.py @@ -6,15 +6,23 @@ """ import logging +from collections.abc import Mapping from datetime import time from typing import TYPE_CHECKING, Any -from django.core.validators import MaxValueValidator, MinValueValidator, RegexValidator +from django.core.validators import ( + MaxLengthValidator, + MaxValueValidator, + MinLengthValidator, + MinValueValidator, + RegexValidator, +) from django.utils.translation import gettext_lazy as _ from rest_framework import serializers from rest_framework.request import Request from rest_framework.reverse import reverse +from rest_framework.utils.formatting import lazy_format from csp_post_processor import post_process_html from openforms.config.constants import UploadFileType @@ -23,6 +31,7 @@ from openforms.utils.urls import build_absolute_uri from openforms.validations.service import PluginValidator +from ..datastructures import FormioData from ..dynamic_config.dynamic_options import add_options_to_config from ..formatters.formio import ( CheckboxFormatter, @@ -563,24 +572,109 @@ def rewrite_for_request(component: ContentComponent, request: Request): component["html"] = post_process_html(component["html"], request) -@register("editgrid") -class EditGrid(BasePlugin[EditGridComponent]): - def build_serializer_field( - self, component: EditGridComponent - ) -> serializers.ListField: - validate = component.get("validate", {}) - required = validate.get("required", False) - nested = build_serializer( - components=component.get("components", []), +class EditGridField(serializers.Field): + """ + A variant of :class:`serializers.ListField`. + + The same child serializer cannot be applied for each item in the list of values, + since the field validation parameters depend on the input data/conditionals. + + For each item, a dynamic serializer is set up on the fly to perform input + validation. + """ + + initial = [] + default_error_messages = { + "not_a_list": _('Expected a list of items but got type "{input_type}".'), + "empty": _("This list may not be empty."), + "min_length": _("Ensure this field has at least {min_length} elements."), + "max_length": _("Ensure this field has no more than {max_length} elements."), + } + + def __init__(self, **kwargs): + self.registry = kwargs.pop("registry") + self.components: list[Component] = kwargs.pop("components", []) + self.allow_empty = kwargs.pop("allow_empty", True) + self.max_length = kwargs.pop("max_length", None) + self.min_length = kwargs.pop("min_length", None) + super().__init__(**kwargs) + if self.max_length is not None: + message = lazy_format( + self.error_messages["max_length"], max_length=self.max_length + ) + self.validators.append(MaxLengthValidator(self.max_length, message=message)) + if self.min_length is not None: + message = lazy_format( + self.error_messages["min_length"], min_length=self.min_length + ) + self.validators.append(MinLengthValidator(self.min_length, message=message)) + + def get_value(self, dictionary): + # We don't bother with html or partial serializers + return dictionary.get(self.field_name, serializers.empty) + + def to_internal_value(self, data): + """ + List of dicts of native values <- List of dicts of primitive datatypes. + """ + if isinstance(data, (str, Mapping)) or not hasattr(data, "__iter__"): + self.fail("not_a_list", input_type=type(data).__name__) + if not self.allow_empty and len(data) == 0: + self.fail("empty") + return self.run_child_validation(data) + + def to_representation(self, value: list): + """ + List of object instances -> List of dicts of primitive datatypes. + """ + child = self._build_child() + return [ + child.to_representation(item) if item is not None else None + for item in value + ] + + def _build_child(self, **kwargs): + return build_serializer( + components=self.components, # XXX: check out type annotations here, there's some co/contra variance # in play register=self.registry, + **kwargs, ) + + def run_child_validation(self, data): + result = [] + errors = {} + + for idx, item in enumerate(data): + # given the local scope of data, build a nested serializer for the component + # configuration and apply the dynamic hide/visible logic to it. + # Note that we add the editgrid key as container so that the + # conditional.when values resolve, as these look like `editgridparent.child`. + data = FormioData({self.field_name: item}).data + nested_serializer = self._build_child(data=data) + try: + result.append(nested_serializer.run_validation(item)) + except serializers.ValidationError as e: + errors[idx] = e.detail + + if not errors: + return result + raise serializers.ValidationError(errors) + + +@register("editgrid") +class EditGrid(BasePlugin[EditGridComponent]): + def build_serializer_field(self, component: EditGridComponent) -> EditGridField: + validate = component.get("validate", {}) + required = validate.get("required", False) + components = component.get("components", []) kwargs = {} if (max_length := validate.get("maxLength")) is not None: kwargs["max_length"] = max_length - return serializers.ListField( - child=nested, + return EditGridField( + components=components, + registry=self.registry, required=required, allow_null=not required, allow_empty=not required, diff --git a/src/openforms/formio/serializers.py b/src/openforms/formio/serializers.py index a6be48e517..2569fae068 100644 --- a/src/openforms/formio/serializers.py +++ b/src/openforms/formio/serializers.py @@ -68,6 +68,12 @@ def apply_hidden_state( self._remove_validations_from_field(serializer_field) def _remove_validations_from_field(self, field: serializers.Field) -> None: + from .components.vanilla import EditGridField # circular import + + # Dynamically change the properties of the field to remove validations. CAREFUL + # when inspecting field instances with a debugger - ``str(field)`` or + # ``repr(field)`` does *not* reflect the current state of field, only how it + # was initialized. Mutations are not taken into account here (!!). field.required = False # note that prefilled fields are validated separately when they're read-only # to protect against tampering @@ -81,8 +87,7 @@ def _remove_validations_from_field(self, field: serializers.Field) -> None: case serializers.CharField(): field.allow_blank = True - case serializers.ListField(): - field.allow_null = True + case serializers.ListField() | EditGridField(): field.allow_empty = True field.min_length = None field.max_length = None diff --git a/src/openforms/formio/tests/validation/test_editgrid.py b/src/openforms/formio/tests/validation/test_editgrid.py index 32c2470cb1..ca456028df 100644 --- a/src/openforms/formio/tests/validation/test_editgrid.py +++ b/src/openforms/formio/tests/validation/test_editgrid.py @@ -1,11 +1,17 @@ +from collections import namedtuple + from django.test import SimpleTestCase, tag +from rest_framework import serializers + from openforms.submissions.tests.factories import SubmissionFactory from openforms.typing import JSONObject, JSONValue +from ...components.vanilla import EditGridField +from ...registry import register from ...service import build_serializer from ...typing import EditGridComponent, FieldsetComponent -from .helpers import validate_formio_data +from .helpers import extract_error, validate_formio_data class EditGridValidationTests(SimpleTestCase): @@ -161,7 +167,10 @@ def test_valid_configuration_nested_field_not_present_in_top_level_serializer(se self.assertNotIn("nested", top_level_fields) with self.subTest("nested fields"): - nested_fields = top_level_fields["toplevel"].child.fields # type: ignore + # implementation detail, but an important one - nested components may only + # appear in the child serializers used to validate each item in the value + # list. + nested_fields = top_level_fields["toplevel"]._build_child().fields # type: ignore self.assertIn("nested", nested_fields) self.assertNotIn("toplevel", nested_fields) @@ -328,13 +337,118 @@ def test_conditional_hidden_inside_editgrid(self): with self.subTest("invalid"): invalid_data: JSONValue = {"editgrid": [{"textfield1": "SHOW_FIELD_2"}]} - is_valid, _ = validate_formio_data(component, invalid_data) + is_valid, errors = validate_formio_data(component, invalid_data) self.assertFalse(is_valid) + error = extract_error(errors["editgrid"][0], "textfield2") + self.assertEqual(error.code, "required") with self.subTest("valid"): invalid_data: JSONValue = {"editgrid": [{"textfield1": "NO_SHOW_FIELD_2"}]} - is_valid, _ = validate_formio_data(component, invalid_data) + is_valid, errors = validate_formio_data(component, invalid_data) self.assertTrue(is_valid) + + +class EditGridFieldTests(SimpleTestCase): + """ + Unit tests for the custom ``ListField`` implementation. + """ + + def test_invalid_type_provided(self): + class Serializer(serializers.Serializer): + editgrid = EditGridField( + registry=register, + components=[ + {"type": "textfield", "key": "textfield", "label": "Text field"} + ], + ) + + invalid_values = (False, {}, 123, "foo") + for value in invalid_values: + with self.subTest(invalid_value=value): + serializer = Serializer(data={"editgrid": value}) + + is_valid = serializer.is_valid() + + self.assertFalse(is_valid) + error = extract_error(serializer.errors, "editgrid") + self.assertEqual(error.code, "not_a_list") + + def test_empty_disallowed(self): + class Serializer(serializers.Serializer): + editgrid = EditGridField( + registry=register, + components=[ + {"type": "textfield", "key": "textfield", "label": "Text field"} + ], + allow_empty=False, + ) + + serializer = Serializer(data={"editgrid": []}) + + is_valid = serializer.is_valid() + + self.assertFalse(is_valid) + error = extract_error(serializer.errors, "editgrid") + self.assertEqual(error.code, "empty") + + def test_min_length(self): + class Serializer(serializers.Serializer): + editgrid = EditGridField( + registry=register, + components=[ + {"type": "textfield", "key": "textfield", "label": "Text field"} + ], + allow_empty=False, + min_length=3, + ) + + serializer = Serializer(data={"editgrid": [{"textfield": "foo"}]}) + + is_valid = serializer.is_valid() + + self.assertFalse(is_valid) + error = extract_error(serializer.errors, "editgrid") + self.assertEqual(error.code, "min_length") + + def test_max_length(self): + class Serializer(serializers.Serializer): + editgrid = EditGridField( + registry=register, + components=[ + {"type": "textfield", "key": "textfield", "label": "Text field"} + ], + allow_empty=False, + max_length=3, + ) + + serializer = Serializer(data={"editgrid": [{"textfield": "foo"}] * 4}) + + is_valid = serializer.is_valid() + + self.assertFalse(is_valid) + error = extract_error(serializer.errors, "editgrid") + self.assertEqual(error.code, "max_length") + + def test_to_representation(self): + class Serializer(serializers.Serializer): + editgrid = EditGridField( + registry=register, + components=[{"type": "textfield", "key": "bar", "label": "Bar"}], + ) + + Foo = namedtuple("Foo", "bar") + foos = [Foo(bar="first"), Foo(bar="second")] + serializer = Serializer(instance={"editgrid": foos}) + + data = serializer.data + + expected = { + "editgrid": [ + {"bar": "first"}, + {"bar": "second"}, + ] + } + self.assertEqual(data, expected) From e51f543c5df09733ee1eab2c9c0b9c18437876c5 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 8 Apr 2024 17:14:37 +0200 Subject: [PATCH 3/3] :bug: [#4126] Conditions may refer to form fields outside the editgrid --- src/openforms/formio/components/vanilla.py | 2 +- .../formio/tests/validation/test_editgrid.py | 58 +++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/openforms/formio/components/vanilla.py b/src/openforms/formio/components/vanilla.py index cab5e5372a..68eee42570 100644 --- a/src/openforms/formio/components/vanilla.py +++ b/src/openforms/formio/components/vanilla.py @@ -651,7 +651,7 @@ def run_child_validation(self, data): # configuration and apply the dynamic hide/visible logic to it. # Note that we add the editgrid key as container so that the # conditional.when values resolve, as these look like `editgridparent.child`. - data = FormioData({self.field_name: item}).data + data = FormioData({**self.root.initial_data, self.field_name: item}).data nested_serializer = self._build_child(data=data) try: result.append(nested_serializer.run_validation(item)) diff --git a/src/openforms/formio/tests/validation/test_editgrid.py b/src/openforms/formio/tests/validation/test_editgrid.py index ca456028df..19190aa959 100644 --- a/src/openforms/formio/tests/validation/test_editgrid.py +++ b/src/openforms/formio/tests/validation/test_editgrid.py @@ -350,6 +350,64 @@ def test_conditional_hidden_inside_editgrid(self): self.assertTrue(is_valid) + @tag("dh-673") + def test_conditional_hidden_inside_editgrid_with_reference_to_outside_editgrid( + self, + ): + editgrid: EditGridComponent = { + "type": "editgrid", + "key": "editgrid", + "label": "Edit grid with nested conditional", + "components": [ + { + "type": "textfield", + "key": "nestedInEditgrid", + "label": "Nested in edit grid", + "validate": {"required": True}, + "conditional": { # type: ignore + "eq": "SHOW_NESTED", + "show": True, + "when": "outsideEditgrid", + }, + }, + ], + } + component: FieldsetComponent = { + "type": "fieldset", + "key": "fieldset", + "label": "Container", + "components": [ + { + "type": "textfield", + "key": "outsideEditgrid", + "label": "Textfield outside edit grid", + }, + editgrid, + ], + } + + with self.subTest("invalid"): + invalid_data: JSONValue = { + "outsideEditgrid": "SHOW_NESTED", + "editgrid": [{}], + } + + is_valid, errors = validate_formio_data(component, invalid_data) + + self.assertFalse(is_valid) + error = extract_error(errors["editgrid"][0], "nestedInEditgrid") + self.assertEqual(error.code, "required") + + with self.subTest("valid"): + invalid_data: JSONValue = { + "outsideEditgrid": "NO_SHOW_NESTED", + "editgrid": [{}], + } + + is_valid, errors = validate_formio_data(component, invalid_data) + + self.assertTrue(is_valid) + class EditGridFieldTests(SimpleTestCase): """