Skip to content

Commit

Permalink
🚑 Hotfix for more validation mismatches
Browse files Browse the repository at this point in the history
* Fixed required edit grid inside hidden fieldset throwing validation
  errors
* Fixed validation being applied to content components (which will
  never have values)
* Fixed formio dotted paths leading to containers and respective nested
  serializers

Backport-of: #4078
  • Loading branch information
sergei-maertens committed Mar 29, 2024
1 parent 83d75f7 commit d466b69
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 4 deletions.
5 changes: 4 additions & 1 deletion src/openforms/formio/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ def build_serializer_field(self, component: ComponentT) -> serializers.Field:
DeprecationWarning,
)

if is_layout_component(component):
# not considered a layout component (because it doesn't have children)
if component["type"] == "content":
required = False
elif is_layout_component(component):
required = False # they do not hold data, they can never be required
else:
required = (
Expand Down
21 changes: 21 additions & 0 deletions src/openforms/formio/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from __future__ import annotations

import logging
from typing import TYPE_CHECKING, TypeAlias

from glom import assign, glom
Expand Down Expand Up @@ -80,6 +81,26 @@ def _remove_validations_from_field(self, field: serializers.Field) -> None:
case serializers.CharField():
field.allow_blank = True

case serializers.ListField():
field.allow_null = True
field.allow_empty = True
field.min_length = None
field.max_length = None

def _get_required(self) -> bool:
return any(field.required for field in self.fields.values())

def _set_required(self, value: bool) -> None:
# we need a setter because the serializers.Field.__init__ sets the initival
# value, but we actually derive the value via :meth:`_get_required` above
# dynamically based on the children, so we just ignore it.
logging.debug(
"Setting the serializer required property has no effect. "
"This is deliberate"
)

required = property(_get_required, _set_required) # type:ignore


def dict_to_serializer(
fields: dict[str, FieldOrNestedFields], **kwargs
Expand Down
19 changes: 19 additions & 0 deletions src/openforms/formio/tests/validation/test_content.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from django.test import SimpleTestCase

from ...typing import ContentComponent
from .helpers import validate_formio_data


class ContentValidationTests(SimpleTestCase):
def test_content_ignored(self):
component: ContentComponent = {
"type": "content",
"key": "Helaas-5.36",
"label": "Nope",
"validate": {"required": False},
"html": "<p>Nope</p>",
}

is_valid, _ = validate_formio_data(component, {})

self.assertTrue(is_valid)
34 changes: 32 additions & 2 deletions src/openforms/formio/tests/validation/test_editgrid.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from openforms.typing import JSONObject

from ...service import build_serializer
from ...typing import EditGridComponent
from ...typing import EditGridComponent, FieldsetComponent
from .helpers import validate_formio_data


Expand Down Expand Up @@ -183,9 +183,39 @@ def test_optional_editgrid(self):
],
}

is_valid, errors = validate_formio_data(
is_valid, _ = validate_formio_data(
component,
{"optionalRepeatingGroup": []},
)

self.assertTrue(is_valid)

def test_required_but_empty_editgrid(self):
editgrid: EditGridComponent = {
"type": "editgrid",
"key": "requiredRepeatingGroup",
"label": "Required repeating group",
"validate": {"required": True},
"components": [
{
"type": "textfield",
"key": "optionalTextfield",
"label": "Optional Text field",
"validate": {"required": False},
}
],
}
component: FieldsetComponent = {
"type": "fieldset",
"key": "fieldset",
"label": "Hidden fieldset",
"hidden": True,
"components": [editgrid],
}

is_valid, _ = validate_formio_data(
component,
{"requiredRepeatingGroup": []},
)

self.assertTrue(is_valid)
16 changes: 15 additions & 1 deletion src/openforms/formio/tests/validation/test_generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from openforms.typing import JSONValue

from ...typing import Component
from .helpers import validate_formio_data
from .helpers import extract_error, validate_formio_data


class FallbackBehaviourTests(SimpleTestCase):
Expand All @@ -21,3 +21,17 @@ def test_unknown_component_passthrough(self):
is_valid, _ = validate_formio_data(component, data)

self.assertTrue(is_valid)

def test_nested_keys_and_fields_being_required(self):
component: Component = {
"type": "textfield",
"key": "nested.field",
"label": "Nested data",
"validate": {"required": True},
}

is_valid, errors = validate_formio_data(component, {})

self.assertFalse(is_valid)
error = extract_error(errors, "nested")
self.assertEqual(error.code, "required")

0 comments on commit d466b69

Please sign in to comment.