Skip to content

Commit

Permalink
Merge pull request #4110 from open-formulieren/issue/4009-cleanup
Browse files Browse the repository at this point in the history
[#4009] Clean up usage of merged data
  • Loading branch information
sergei-maertens authored Apr 4, 2024
2 parents 300fa10 + d6c54da commit ad25a10
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 166 deletions.
21 changes: 12 additions & 9 deletions src/openforms/formio/formatters/tests/test_kitchensink.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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.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


Expand Down
2 changes: 1 addition & 1 deletion src/openforms/registrations/contrib/camunda/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
1 change: 0 additions & 1 deletion src/openforms/submissions/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
4 changes: 2 additions & 2 deletions src/openforms/submissions/mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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()

Expand Down
57 changes: 23 additions & 34 deletions src/openforms/submissions/models/submission.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

import logging
import uuid
from collections import OrderedDict
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
Expand Down Expand Up @@ -52,7 +51,7 @@
@dataclass
class SubmissionState:
form_steps: list[FormStep]
submission_steps: list["SubmissionStep"]
submission_steps: list[SubmissionStep]

def _get_step_offset(self):
completed_steps = sorted(
Expand All @@ -66,7 +65,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.
Expand All @@ -86,7 +85,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
Expand All @@ -96,7 +95,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)


Expand Down Expand Up @@ -467,7 +466,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

Expand Down Expand Up @@ -593,36 +592,18 @@ 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.
"""
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",
Expand All @@ -633,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):
Expand All @@ -655,29 +636,37 @@ 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 ""
if not (co_signer := self.co_sign_data.get("representation", "")):
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,6 @@

{% block after_field_sets %}
<fieldset class="module aligned">
<div class="form-row field-merged_data">
<div>
<label>{% trans "Merged Data:" %}</label>
<div class="readonly">
<ul>
{% for key, info in data.items %}
{% with component=info.0 value=info.1 %}
{% if component.type == 'unknown component' %}
<li>{% trans "Unknown component" %}</li>
{% elif component.type in image_components and value %}
<li>{{ key }}: <img class='signature-image' src='{{ value }}' alt='{{ key }}'></li>
{% else %}
<li>{{ key }}: {{ value }}</li>
{% endif %}
{% endwith %}
{% endfor %}
</ul>
</div>
</div>
</div>
<div class="form-row field-attachments">
<div>
<label>{% trans "Attachments:" %}</label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
46 changes: 1 addition & 45 deletions src/openforms/submissions/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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 = """
<ul>
<li>adres: Voorburg</li>
<li>voornaam: shea</li>
<li>familienaam: meyers</li>
<li>geboortedatum: None</li>
<li>signature: None</li>
</ul>
"""

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,
"<li>signature: <img class='signature-image' src='data:image/png;base64,iVBOR' alt='signature'></li>",
html=True,
)

def test_viewing_submission_details_in_admin_creates_log(self):
self.app.get(
reverse(
Expand Down
6 changes: 3 additions & 3 deletions src/openforms/submissions/tests/test_factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -94,7 +94,7 @@ def test_from_data__simple(self):
}
)

actual = submission.get_merged_data()
actual = submission.data
expected = {
"foo": 1,
"bar": 2,
Expand All @@ -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,
Expand Down
Loading

0 comments on commit ad25a10

Please sign in to comment.