Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix another validation mismatch #4078

Merged
merged 5 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Comment on lines +95 to +96
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably at some point if we manage to define components as dataclass-likes structure, this will be easier to handle (via a method/property)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, this patch definitely had me longing for such a thing again :)

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")
Loading