From c4ebb17c32c795b4808f1423a7dcd90fcd75f16a Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 5 Apr 2024 15:35:39 +0200 Subject: [PATCH] :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. Backport-of: #4125 --- src/openforms/formio/components/vanilla.py | 116 +++++++++- src/openforms/formio/serializers.py | 9 +- .../formio/tests/validation/test_editgrid.py | 219 +++++++++++++++++- 3 files changed, 327 insertions(+), 17 deletions(-) diff --git a/src/openforms/formio/components/vanilla.py b/src/openforms/formio/components/vanilla.py index fca819d548..77e9c4331e 100644 --- a/src/openforms/formio/components/vanilla.py +++ b/src/openforms/formio/components/vanilla.py @@ -5,14 +5,20 @@ adjacent custom.py module. """ +from collections.abc import Mapping from typing import TYPE_CHECKING -from django.core.validators import RegexValidator +from django.core.validators import ( + MaxLengthValidator, + MinLengthValidator, + 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 @@ -21,6 +27,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, @@ -388,24 +395,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.root.initial_data, 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 c9affb68ed..19190aa959 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 +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) @@ -297,3 +306,207 @@ 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, 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, errors = validate_formio_data(component, invalid_data) + + 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): + """ + 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)