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

[#4009] Clean up usage of merged data #4110

Merged
merged 6 commits into from
Apr 4, 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
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
Loading