Skip to content

Commit

Permalink
Merge pull request #4752 from open-formulieren/issue/3629-bulk-export…
Browse files Browse the repository at this point in the history
…-crash

Ensure submissions tablib.Dataset generation doesn't crash
  • Loading branch information
sergei-maertens authored Oct 14, 2024
2 parents 97d32bc + 02c68e5 commit 43e397e
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 13 deletions.
12 changes: 12 additions & 0 deletions src/openforms/formio/rendering/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,16 @@ class EditGridNode(ContainerMixin, ComponentNode):
layout_modifier: str = "editgrid"
display_value: str = ""

@property
def is_layout(self) -> bool:
if self.mode == RenderModes.export:
return False
return True

@property
def value(self):
if self.mode == RenderModes.export:
return super().value
return None

@property
Expand Down Expand Up @@ -268,6 +276,10 @@ def get_children(self) -> Iterator["ComponentNode"]:
So we need to repeat the child nodes of the configuration and associate them with the data
provided by the user.
"""
# during exports, treat the entire repeating group as a single component/column
if self.mode == RenderModes.export:
return

repeats = len(self._value) if self._value else 0

for node_index in range(repeats):
Expand Down
5 changes: 1 addition & 4 deletions src/openforms/formio/rendering/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,7 @@ 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)
and self.component["type"] != "editgrid"
):
if self.mode != RenderModes.export or (not is_layout_component(self.component)):
yield self

for child in self.get_children():
Expand Down
13 changes: 5 additions & 8 deletions src/openforms/formio/rendering/tests/test_component_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ def test_export_always_emits_all_nodes(self):
)
nodelist += list(component_node)

self.assertEqual(len(nodelist), 19)
self.assertEqual(len(nodelist), 16)
labels = [node.label for node in nodelist]
# The fieldset/editgrid components have no labels
self.assertEqual(
Expand All @@ -427,13 +427,10 @@ def test_export_always_emits_all_nodes(self):
"input8",
"input9",
"input10",
# The editgrid components have 2 entries each in the data
"input11",
"input11",
"input12",
"input12",
"input13",
"input13",
# edit grid must be treated as a leaf node instead of a layout node
"visibleEditGridWithVisibleChildren",
"hiddenEditGridWithVisibleChildren",
"visibleEditGridWithHiddenChildren",
"input14",
"input15",
],
Expand Down
2 changes: 1 addition & 1 deletion src/openforms/formio/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def temporary_upload(obj, create, extracted, **kwargs):
else TemporaryFileUploadFactory.build
)
temporary_upload = create_temporary_upload(
file_name=obj["name"], file_size=obj["size"]
file_name=obj["name"], file_size=obj["size"], **kwargs
)
new_url = f"http://localhost/api/v2/submissions/files/{temporary_upload.uuid}"
obj["url"] = obj["data"]["url"] = new_url
2 changes: 2 additions & 0 deletions src/openforms/submissions/rendering/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class SubmissionStepNode(Node):

@property
def is_visible(self) -> bool:
if self.mode == RenderModes.export:
return True
# determine if the step as a whole is relevant or not. The stap may be not
# applicable because of form logic.
logic_evaluated = getattr(self.step, "_form_logic_evaluated", False)
Expand Down
121 changes: 121 additions & 0 deletions src/openforms/submissions/tests/test_exports.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
from django.utils import timezone

from freezegun import freeze_time
from privates.test import temp_private_root

from openforms.formio.tests.factories import SubmittedFileFactory
from openforms.forms.tests.factories import FormFactory, FormStepFactory
from openforms.variables.constants import FormVariableSources

Expand Down Expand Up @@ -266,3 +268,122 @@ def test_submissions_of_forms_with_translation_enabled_have_language_codes(self)
export = create_submission_export(Submission.objects.all())

self.assertIn(("Taalcode", "en"), zip(export.headers, export[0]))

@tag("gh-3629")
def test_different_number_of_items_in_repeating_groups(self):
form = FormFactory.create(
generate_minimal_setup=True,
formstep__form_definition__configuration={
"components": [
{
"type": "editgrid",
"key": "repeatingGroup",
"label": "Repeating group",
"components": [
{
"type": "textfield",
"key": "fullName",
"label": "Full name",
}
],
}
]
},
)
submission_1, submission_2 = SubmissionFactory.create_batch(
2, form=form, completed=True, completed_on=timezone.now()
)
SubmissionStepFactory.create(
submission=submission_1,
form_step=form.formstep_set.get(),
# no records at all, ensure first submission has less items than second
data={"repeatingGroup": []},
)
SubmissionStepFactory.create(
submission=submission_2,
form_step=form.formstep_set.get(),
# 1 record, more than the first submission
data={"repeatingGroup": [{"fullName": "Herman Brood"}]},
)

dataset = create_submission_export(Submission.objects.order_by("pk"))

self.assertEqual(len(dataset), 2)
self.assertEqual(len(dataset.headers), 3)
self.assertEqual(dataset.headers[2], "repeatingGroup")
self.assertIsNotNone(dataset[0][2])
self.assertIsNotNone(dataset[1][2])

@tag("gh-3629")
@temp_private_root()
def test_form_with_file_uploads(self):
form = FormFactory.create(
generate_minimal_setup=True,
formstep__form_definition__configuration={
"components": [
{
"type": "file",
"key": "attachments",
"label": "Attachments",
"multiple": True,
}
]
},
)
submission_1, submission_2 = SubmissionFactory.create_batch(
2, form=form, completed=True, completed_on=timezone.now()
)
SubmissionStepFactory.create(
submission=submission_1,
form_step=form.formstep_set.get(),
data={
"attachments": [
SubmittedFileFactory.create(
temporary_upload__submission=submission_1
)
]
},
)
files_2 = SubmittedFileFactory.create_batch(
2, temporary_upload__submission=submission_2
)
SubmissionStepFactory.create(
submission=submission_2,
form_step=form.formstep_set.get(),
# 1 record, more than the first submission
data={"attachments": files_2},
)

dataset = create_submission_export(Submission.objects.order_by("pk"))

self.assertEqual(len(dataset), 2)

@tag("gh-3629")
def test_submission_missing_submission_step(self):
form = FormFactory.create(
generate_minimal_setup=True,
formstep__form_definition__configuration={
"components": [
{
"type": "textfield",
"key": "fullName",
"label": "Full name",
}
]
},
)
submission_1, submission_2 = SubmissionFactory.create_batch(
2, form=form, completed=True, completed_on=timezone.now()
)
assert not submission_1.submissionstep_set.exists()
SubmissionStepFactory.create(
submission=submission_2,
form_step=form.formstep_set.get(),
data={"fullName": "Arie Kabaalstra"},
)

dataset = create_submission_export(Submission.objects.order_by("pk"))

self.assertEqual(len(dataset), 2)
self.assertEqual(len(dataset[0]), 3)
self.assertEqual(len(dataset[1]), 3)

0 comments on commit 43e397e

Please sign in to comment.