From 2f1cdafce65f22fad3e8eb3e12c4f3e85e68ad62 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:07:43 +0200 Subject: [PATCH 1/6] [#4009] Remove forward annotations --- .../submissions/models/submission.py | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/openforms/submissions/models/submission.py b/src/openforms/submissions/models/submission.py index 76ebf0ddae..476d0881b5 100644 --- a/src/openforms/submissions/models/submission.py +++ b/src/openforms/submissions/models/submission.py @@ -52,7 +52,7 @@ @dataclass class SubmissionState: form_steps: list[FormStep] - submission_steps: list["SubmissionStep"] + submission_steps: list[SubmissionStep] def _get_step_offset(self): completed_steps = sorted( @@ -66,7 +66,7 @@ def _get_step_offset(self): ) return offset - def get_last_completed_step(self) -> "SubmissionStep" | None: + def get_last_completed_step(self) -> SubmissionStep | None: """ Determine the last step that was filled out. @@ -86,7 +86,7 @@ def get_last_completed_step(self) -> "SubmissionStep" | None: ) return next(candidates, None) - def get_submission_step(self, form_step_uuid: str) -> "SubmissionStep" | None: + def get_submission_step(self, form_step_uuid: str) -> SubmissionStep | None: return next( ( step @@ -96,7 +96,7 @@ def get_submission_step(self, form_step_uuid: str) -> "SubmissionStep" | None: None, ) - def resolve_step(self, form_step_uuid: str) -> "SubmissionStep": + def resolve_step(self, form_step_uuid: str) -> SubmissionStep: return self.get_submission_step(form_step_uuid=form_step_uuid) @@ -467,7 +467,7 @@ def remove_sensitive_data(self): @elasticapm.capture_span(span_type="app.data.loading") def load_submission_value_variables_state( self, refresh: bool = False - ) -> "SubmissionValueVariablesState": + ) -> SubmissionValueVariablesState: if hasattr(self, "_variables_state") and not refresh: return self._variables_state @@ -593,12 +593,12 @@ def render_summary_page(self) -> list[JSONObject]: return summary_data @property - def steps(self) -> list["SubmissionStep"]: + def steps(self) -> list[SubmissionStep]: # fetch the existing DB records for submitted form steps submission_state = self.load_execution_state() return submission_state.submission_steps - def get_last_completed_step(self) -> "SubmissionStep" | None: + def get_last_completed_step(self) -> SubmissionStep | None: """ Determine which is the next step for the current submission. """ @@ -620,7 +620,6 @@ def get_ordered_data_with_component_type(self) -> OrderedDict: component, value, ) - return ordered_data def get_merged_appointment_data(self) -> dict[str, dict[str, str | dict]]: @@ -668,16 +667,16 @@ def get_co_signer(self) -> str: logger.warning("Incomplete co-sign data for submission %s", self.uuid) return co_signer - def get_attachments(self) -> "SubmissionFileAttachmentQuerySet": + def get_attachments(self) -> SubmissionFileAttachmentQuerySet: from .submission_files import SubmissionFileAttachment return SubmissionFileAttachment.objects.for_submission(self) @property - def attachments(self) -> "SubmissionFileAttachmentQuerySet": + def attachments(self) -> SubmissionFileAttachmentQuerySet: return self.get_attachments() - def get_merged_attachments(self) -> Mapping[str, list["SubmissionFileAttachment"]]: + def get_merged_attachments(self) -> Mapping[str, list[SubmissionFileAttachment]]: if not hasattr(self, "_merged_attachments"): self._merged_attachments = self.get_attachments().as_form_dict() return self._merged_attachments From 17b1462f9976788c23d1e4ad831fe61b97ff61bf Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:23:40 +0200 Subject: [PATCH 2/6] [#4009] Remove merged data from admin form --- src/openforms/submissions/admin.py | 1 - .../submissions/submission/change_form.html | 20 ------------------- 2 files changed, 21 deletions(-) diff --git a/src/openforms/submissions/admin.py b/src/openforms/submissions/admin.py index edb9a0fff8..9618fa8855 100644 --- a/src/openforms/submissions/admin.py +++ b/src/openforms/submissions/admin.py @@ -269,7 +269,6 @@ def change_view(self, request, object_id, form_url="", extra_context=None): raise Http404(f"No {self.model._meta.object_name} matches the given query.") submission_details_view_admin(submission, request.user) extra_context = { - "data": submission.get_ordered_data_with_component_type(), "attachments": submission.get_merged_attachments(), "image_components": IMAGE_COMPONENTS, } diff --git a/src/openforms/submissions/templates/admin/submissions/submission/change_form.html b/src/openforms/submissions/templates/admin/submissions/submission/change_form.html index 57a6156948..e5cbaa922c 100644 --- a/src/openforms/submissions/templates/admin/submissions/submission/change_form.html +++ b/src/openforms/submissions/templates/admin/submissions/submission/change_form.html @@ -3,26 +3,6 @@ {% block after_field_sets %}
-
-
- -
-
    - {% for key, info in data.items %} - {% with component=info.0 value=info.1 %} - {% if component.type == 'unknown component' %} -
  • {% trans "Unknown component" %}
    • - {% elif component.type in image_components and value %} -
  • {{ key }}: {{ key }}
    • - {% else %} -
  • {{ key }}: {{ value }}
    • - {% endif %} - {% endwith %} - {% endfor %} -
-
-
-
From 3db3497b221231a56ed4068ceb1e988a94c03ef7 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:24:13 +0200 Subject: [PATCH 3/6] [#4009] Remove usage of `get_ordered_data_with_component_type` --- .../formatters/tests/test_kitchensink.py | 21 +++++---- .../submissions/models/submission.py | 18 ------- .../submissions/tests/test_models.py | 47 ------------------- 3 files changed, 12 insertions(+), 74 deletions(-) diff --git a/src/openforms/formio/formatters/tests/test_kitchensink.py b/src/openforms/formio/formatters/tests/test_kitchensink.py index d58ee6fe11..56a2d9df00 100644 --- a/src/openforms/formio/formatters/tests/test_kitchensink.py +++ b/src/openforms/formio/formatters/tests/test_kitchensink.py @@ -1,5 +1,8 @@ +from typing import Any + from django.utils.translation import gettext as _ +from openforms.submissions.models import Submission from openforms.submissions.tests.factories import ( SubmissionFactory, SubmissionFileAttachmentFactory, @@ -12,15 +15,15 @@ from .utils import load_json -def _get_printable_data(submission): - printable_data = [] - for key, ( - component, - value, - ) in submission.get_ordered_data_with_component_type().items(): - printable_data.append( - (component["label"], format_value(component, value, as_html=False)) - ) +def _get_printable_data(submission: Submission) -> list[tuple[str, Any]]: + printable_data: list[tuple[str, Any]] = [] + merged_data = submission.get_merged_data() + + for component in filter_printable(submission.form.iter_components(recursive=True)): + key = component["key"] + value = format_value(component, merged_data.get(key), as_html=False) + printable_data.append((component.get("label", key), value)) + return printable_data diff --git a/src/openforms/submissions/models/submission.py b/src/openforms/submissions/models/submission.py index 476d0881b5..50203fdcba 100644 --- a/src/openforms/submissions/models/submission.py +++ b/src/openforms/submissions/models/submission.py @@ -2,7 +2,6 @@ import logging import uuid -from collections import OrderedDict from dataclasses import dataclass from typing import TYPE_CHECKING, Mapping @@ -605,23 +604,6 @@ def get_last_completed_step(self) -> SubmissionStep | None: submission_state = self.load_execution_state() return submission_state.get_last_completed_step() - def get_ordered_data_with_component_type(self) -> OrderedDict: - from openforms.formio.formatters.printable import filter_printable - - ordered_data = OrderedDict() - merged_data = self.get_merged_data() - - # first collect data we have in the same order the components are defined in the form - for component in filter_printable(self.form.iter_components(recursive=True)): - key = component["key"] - value = merged_data.get(key, None) - component.setdefault("label", key) - ordered_data[key] = ( - component, - value, - ) - return ordered_data - def get_merged_appointment_data(self) -> dict[str, dict[str, str | dict]]: component_config_key_to_appointment_key = { "appointments.showProducts": "productIDAndName", diff --git a/src/openforms/submissions/tests/test_models.py b/src/openforms/submissions/tests/test_models.py index fe45c832d1..10988950de 100644 --- a/src/openforms/submissions/tests/test_models.py +++ b/src/openforms/submissions/tests/test_models.py @@ -39,53 +39,6 @@ def test_submission_str(self): str(submission), f"{submission.pk} - started on Nov. 26, 2021, 5 p.m." ) - def test_submission_data_with_selectboxes_formio_formatters(self): - form_definition = FormDefinitionFactory.create( - configuration={ - "display": "form", - "components": [ - { - "key": "testSelectBoxes", - "type": "selectboxes", - "label": "My Boxes", - "values": [ - {"value": "test1", "label": "test 1", "shortcut": ""}, - {"value": "test2", "label": "test 2", "shortcut": ""}, - {"value": "test3", "label": "test 3", "shortcut": ""}, - ], - }, - ], - } - ) - submission = SubmissionFactory.create() - SubmissionStepFactory.create( - submission=submission, - data={"testSelectBoxes": {"test1": True, "test2": True, "test3": False}}, - form_step=FormStepFactory.create( - form=submission.form, form_definition=form_definition - ), - ) - - ordered = submission.get_ordered_data_with_component_type() - self.assertEqual( - ordered, - { - "testSelectBoxes": ( - { - "key": "testSelectBoxes", - "type": "selectboxes", - "label": "My Boxes", - "values": [ - {"value": "test1", "label": "test 1", "shortcut": ""}, - {"value": "test2", "label": "test 2", "shortcut": ""}, - {"value": "test3", "label": "test 3", "shortcut": ""}, - ], - }, - {"test1": True, "test2": True, "test3": False}, - ) - }, - ) - def test_submission_remove_sensitive_data(self): form_definition = FormDefinitionFactory.create( configuration={ From 1e154833bcbe2beeda40da277ef62c6ddd611c9a Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:48:45 +0200 Subject: [PATCH 4/6] [#4009] Remove `get_merged_data` In the future, the `data` property could also be removed, to enforce people using the state wrapper instead. --- src/openforms/submissions/models/submission.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/openforms/submissions/models/submission.py b/src/openforms/submissions/models/submission.py index 50203fdcba..76e4b5bea0 100644 --- a/src/openforms/submissions/models/submission.py +++ b/src/openforms/submissions/models/submission.py @@ -3,7 +3,7 @@ import logging import uuid from dataclasses import dataclass -from typing import TYPE_CHECKING, Mapping +from typing import TYPE_CHECKING, Any, Mapping from django.conf import settings from django.db import models, transaction @@ -636,12 +636,20 @@ def get_merged_appointment_data(self) -> dict[str, dict[str, str | dict]]: return appointment_data - def get_merged_data(self) -> dict: + @property + def data(self) -> dict[str, Any]: + """The filled-in data of the submission. + + This is a mapping between variable keys and their corresponding values. + + .. note:: + + Keys containings dots (``.``) will be nested under another mapping. + Static variables values are *not* included. + """ values_state = self.load_submission_value_variables_state() return values_state.get_data() - data = property(get_merged_data) - def get_co_signer(self) -> str: if not self.co_sign_data: return "" From afebb3b61f9a045efb53ef1b8059c27b886c34dd Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:51:29 +0200 Subject: [PATCH 5/6] [#4009] Replace usage of `get_merged_data` --- src/openforms/formio/formatters/tests/test_kitchensink.py | 2 +- src/openforms/registrations/contrib/camunda/plugin.py | 2 +- .../registrations/contrib/microsoft_graph/plugin.py | 2 +- src/openforms/submissions/mapping.py | 4 ++-- src/openforms/submissions/models/submission.py | 2 +- .../submissions/tests/form_logic/test_submission_logic.py | 4 +--- src/openforms/submissions/tests/test_factories.py | 6 +++--- 7 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/openforms/formio/formatters/tests/test_kitchensink.py b/src/openforms/formio/formatters/tests/test_kitchensink.py index 56a2d9df00..82029675b1 100644 --- a/src/openforms/formio/formatters/tests/test_kitchensink.py +++ b/src/openforms/formio/formatters/tests/test_kitchensink.py @@ -17,7 +17,7 @@ def _get_printable_data(submission: Submission) -> list[tuple[str, Any]]: printable_data: list[tuple[str, Any]] = [] - merged_data = submission.get_merged_data() + merged_data = submission.data for component in filter_printable(submission.form.iter_components(recursive=True)): key = component["key"] diff --git a/src/openforms/registrations/contrib/camunda/plugin.py b/src/openforms/registrations/contrib/camunda/plugin.py index 415a9a3608..85b64dbf4c 100644 --- a/src/openforms/registrations/contrib/camunda/plugin.py +++ b/src/openforms/registrations/contrib/camunda/plugin.py @@ -48,7 +48,7 @@ def get_process_variables( ] ) - merged_data: dict[str, Any] = submission.get_merged_data() + merged_data = submission.data for component in submission.form.iter_components(recursive=True): if (key := component.get("key")) not in simple_mappings: continue diff --git a/src/openforms/registrations/contrib/microsoft_graph/plugin.py b/src/openforms/registrations/contrib/microsoft_graph/plugin.py index a2d4bec28c..48260e326a 100644 --- a/src/openforms/registrations/contrib/microsoft_graph/plugin.py +++ b/src/openforms/registrations/contrib/microsoft_graph/plugin.py @@ -64,7 +64,7 @@ def register_submission(self, submission: Submission, options: dict) -> None: submission_report.content, folder_name / "report.pdf" ) - data = submission.get_merged_data() + data = submission.data data["__metadata__"] = {"submission_language": submission.language_code} uploader.upload_json(data, folder_name / "data.json") diff --git a/src/openforms/submissions/mapping.py b/src/openforms/submissions/mapping.py index 7d610b8eaf..900ddcdfed 100644 --- a/src/openforms/submissions/mapping.py +++ b/src/openforms/submissions/mapping.py @@ -97,7 +97,7 @@ def apply_data_mapping( attr_key_lookup[attribute] = key # grab submitted data - data = submission.get_merged_data() + data = submission.data for target_path, conf in mapping_config.items(): if isinstance(conf, str): @@ -137,7 +137,7 @@ def get_unmapped_data( """ companion to apply_data_mapping() returns data not mapped to RegistrationAttributes """ - data = submission.get_merged_data() + data = submission.data attr_key_lookup = dict() diff --git a/src/openforms/submissions/models/submission.py b/src/openforms/submissions/models/submission.py index 76e4b5bea0..83a21b6c28 100644 --- a/src/openforms/submissions/models/submission.py +++ b/src/openforms/submissions/models/submission.py @@ -614,7 +614,7 @@ def get_merged_appointment_data(self) -> dict[str, dict[str, str | dict]]: "appointments.phoneNumber": "clientPhoneNumber", } - merged_data = self.get_merged_data() + merged_data = self.data appointment_data = {} for component in self.form.iter_components(recursive=True): diff --git a/src/openforms/submissions/tests/form_logic/test_submission_logic.py b/src/openforms/submissions/tests/form_logic/test_submission_logic.py index 39c5b0f402..567630e5db 100644 --- a/src/openforms/submissions/tests/form_logic/test_submission_logic.py +++ b/src/openforms/submissions/tests/form_logic/test_submission_logic.py @@ -424,9 +424,7 @@ def test_response_contains_submission(self): self._add_submission_to_session(submission) with freeze_time("2015-10-10"): - response = self.client.post( - endpoint, {"data": submission.get_merged_data()} - ) + response = self.client.post(endpoint, {"data": submission.data}) submission_details = response.json() diff --git a/src/openforms/submissions/tests/test_factories.py b/src/openforms/submissions/tests/test_factories.py index 417b774662..64d39fef33 100644 --- a/src/openforms/submissions/tests/test_factories.py +++ b/src/openforms/submissions/tests/test_factories.py @@ -63,7 +63,7 @@ def test_from_components__with_data(self): }, ) - actual = submission.get_merged_data() + actual = submission.data expected = {"foo": 1, "bar": 2} self.assertEqual(actual, expected) @@ -94,7 +94,7 @@ def test_from_data__simple(self): } ) - actual = submission.get_merged_data() + actual = submission.data expected = { "foo": 1, "bar": 2, @@ -111,7 +111,7 @@ def test_from_data__kwargs(self): kvk="111222333", ) - actual = submission.get_merged_data() + actual = submission.data expected = { "foo": 1, "bar": 2, From d6c54da382486ec96df62e0818894995c915b922 Mon Sep 17 00:00:00 2001 From: Viicos <65306057+Viicos@users.noreply.github.com> Date: Wed, 3 Apr 2024 16:22:33 +0200 Subject: [PATCH 6/6] [#4009] Remove tests related to merged data --- src/openforms/submissions/tests/test_admin.py | 46 +------------------ 1 file changed, 1 insertion(+), 45 deletions(-) diff --git a/src/openforms/submissions/tests/test_admin.py b/src/openforms/submissions/tests/test_admin.py index c6239978e2..9ce7238019 100644 --- a/src/openforms/submissions/tests/test_admin.py +++ b/src/openforms/submissions/tests/test_admin.py @@ -7,13 +7,12 @@ from maykin_2fa.test import disable_admin_mfa from openforms.accounts.tests.factories import UserFactory -from openforms.forms.models import FormVariable from openforms.logging.logevent import submission_start from openforms.logging.models import TimelineLogProxy from ...config.models import GlobalConfiguration from ..constants import PostSubmissionEvents, RegistrationStatuses -from .factories import SubmissionFactory, SubmissionValueVariableFactory +from .factories import SubmissionFactory @disable_admin_mfa() @@ -42,49 +41,6 @@ def setUp(self): super().setUp() self.user = UserFactory.create(is_superuser=True, is_staff=True) - def test_displaying_merged_data_formio_formatters(self): - response = self.app.get( - reverse( - "admin:submissions_submission_change", args=(self.submission_1.pk,) - ), - user=self.user, - ) - expected = """ -
    -
  • adres: Voorburg
    • -
  • voornaam: shea
    • -
  • familienaam: meyers
    • -
  • geboortedatum: None
    • -
  • signature: None
    • -
- """ - - self.assertContains(response, expected, html=True) - - def test_displaying_merged_data_displays_signature_as_image_formio_formatters(self): - form_variable = FormVariable.objects.get( - key="signature", form=self.submission_1.form - ) - SubmissionValueVariableFactory.create( - key="signature", - submission=self.submission_1, - value="data:image/png;base64,iVBOR", - form_variable=form_variable, - ) - - response = self.app.get( - reverse( - "admin:submissions_submission_change", args=(self.submission_1.pk,) - ), - user=self.user, - ) - - self.assertContains( - response, - "
  • signature: signature
    • ", - html=True, - ) - def test_viewing_submission_details_in_admin_creates_log(self): self.app.get( reverse(