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

🐛 [#4241] Fix validation of field with nested key in fieldset #4329

Merged
merged 2 commits into from
May 27, 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/rendering/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,10 @@ def __iter__(self) -> Iterator["ComponentNode"]:
return

# in export mode, only emit if the component is not a layout component
if self.mode != RenderModes.export or not is_layout_component(self.component):
if self.mode != RenderModes.export or (
not is_layout_component(self.component)
and self.component["type"] != "editgrid"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sergei-maertens I do have to add this check here explicitly after I added the editgrid check to is_layout_component

Copy link
Member

Choose a reason for hiding this comment

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

yeah we shoved editgrid in there in a weird way, I'm glad it's not worse than this

):
yield self

for child in self.get_children():
Expand Down
9 changes: 8 additions & 1 deletion src/openforms/formio/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from .datastructures import FormioConfigurationWrapper
from .typing import Component
from .utils import iter_components
from .utils import is_layout_component, iter_components

if TYPE_CHECKING:
from .registry import ComponentRegistry
Expand Down Expand Up @@ -62,6 +62,10 @@ def apply_hidden_state(
if is_visible:
continue

# Layout components do not have serializer fields associated with them
if is_layout_component(component):
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"])
Expand Down Expand Up @@ -149,6 +153,9 @@ def build_serializer(

config: JSONObject = {"components": components}
for component in iter_components(config, recurse_into_editgrid=False):
if is_layout_component(component):
sergei-maertens marked this conversation as resolved.
Show resolved Hide resolved
continue

field = register.build_serializer_field(component)
assign(obj=fields, path=component["key"], val=field, missing=dict)

Expand Down
20 changes: 20 additions & 0 deletions src/openforms/formio/tests/validation/test_fieldset.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,23 @@ def test_hidden_fieldset_with_nested_required_fields(self):
is_valid, _ = validate_formio_data(component, {"textfield": ""})

self.assertTrue(is_valid)

def test_validate_nested_field_with_nested_key(self):
component: FieldsetComponent = {
"type": "fieldset",
"key": "fieldset",
"label": "A field set",
"components": [
{
"type": "textfield",
"key": "fieldset.textfield",
"label": "Text field",
"validate": {"required": True},
},
],
"hidden": False,
}

is_valid, _ = validate_formio_data(component, {"fieldset": {"textfield": ""}})

self.assertFalse(is_valid)
4 changes: 4 additions & 0 deletions src/openforms/formio/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ def get_readable_path_from_configuration_path(
def is_layout_component(component: Component) -> bool:
# Adapted from isLayoutComponent util function in Formio
# https://github.com/formio/formio.js/blob/4.13.x/src/utils/formUtils.js#L25
# FIXME ideally there would be a cleaner fix for this
if component["type"] == "editgrid":
return False
Comment on lines +163 to +164
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a FIXME that this isn't the neatest way to handle this? 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added it


column = component.get("columns")
components = component.get("components")
rows = component.get("rows")
Expand Down
2 changes: 1 addition & 1 deletion src/openforms/forms/models/form_variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def create_for_formstep(self, form_step: "FormStep") -> list["FormVariable"]:
configuration=form_definition_configuration, recursive=True
):
if (
(is_layout_component(component) and not component["type"] == "editgrid")
is_layout_component(component)
or component["type"] == "content"
or component["key"] in existing_form_variables_keys
or component_in_editgrid(form_definition_configuration, component)
Expand Down
Loading