♻️ [#1068] Refactor submission data retrieval and formatting

src/openforms/emails/templatetags/form_summary.py

@@ -33,7 +33,7 @@ def filter_data_to_show_in_email(context: dict) -> dict:
         data_to_show_in_email += keys
 
     filtered_data = submission.get_printable_data(
-        limit_keys_to=data_to_show_in_email, use_merged_data_fallback=True
+        limit_keys_to=data_to_show_in_email,
     )
     return {"submitted_data": filtered_data}
 

src/openforms/formio/formatters/default.py

@@ -1,10 +1,24 @@
+from datetime import datetime
 from typing import Any, Dict
 
+from django.template.defaultfilters import date as fmt_date, time as fmt_time
+from django.utils.dateparse import parse_date, parse_time
+from django.utils.translation import gettext_lazy as _
+
 from openforms.plugins.plugin import AbstractBasePlugin
+from openforms.submissions.models import _join_mapped
 
 from .registry import register
 
 
+def _get_value_label(possible_values: list, value: str) -> str:
+    for possible_value in possible_values:
+        if possible_value["value"] == value:
+            return possible_value["label"]
+    # TODO what if value is not a string? shouldn't it be passed through something to covert for display?
+    return value
+
+
 class FormioFormatter(AbstractBasePlugin):
     def __call__(self, component: dict, value: Any) -> str:
         return self.format(component, value)
@@ -16,6 +30,15 @@ def format(self, component: dict, value: Any) -> str:
 @register("default")
 class DefaultFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
+        # printable_value = value
+        # if info.get("values"):
+        #     # Case in which each value has a label (for example select and radio components)
+        #     printable_value = self._get_value_label(info["values"], value)
+
+        # # more here? like getComponentValue() in the SDK?
+        # printable_data[label] = printable_value
+        if isinstance(value, list):
+            return value
         return str(value)
 
 
@@ -34,25 +57,31 @@ def format(self, component: Dict, value: str) -> str:
 @register("date")
 class DateFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        return _join_mapped(lambda v: fmt_date(parse_date(v)), value)
 
 
 @register("time")
 class TimeFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        # strip off the seconds
+        return _join_mapped(lambda v: fmt_time(parse_time(v)), value)
 
 
 @register("phoneNumber")
 class PhoneNumberFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        # TODO custom formatting?
+        raise str(value)
 
 
 @register("file")
 class FileFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        if value:
+            formatted = ", ".join(file["originalName"] for file in value)
+        else:
+            formatted = _("empty")
+        return formatted
 
 
 @register("textarea")
@@ -70,13 +99,14 @@ def format(self, component: Dict, value: str) -> str:
 @register("password")
 class PasswordFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        return "".join("\u25CF" for _ in value)
 
 
 @register("checkbox")
 class CheckboxFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        # TODO custom formatting?
+        raise str(value)
 
 
 @register("selectboxes")
@@ -91,22 +121,29 @@ def format(self, component: Dict, value: str) -> str:
 @register("select")
 class SelectFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        if component.get("appointments", {}).get("showDates", False):
+            return _join_mapped(lambda v: fmt_date(parse_date(v)), value)
+        elif component.get("appointments", {}).get("showTimes", False):
+            # strip off the seconds
+            return _join_mapped(lambda v: fmt_time(datetime.fromisoformat(v)), value)
+        return _get_value_label(component["data"]["values"], value)
 
 
 @register("currency")
 class CurrencyFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        # TODO custom formatting?
+        raise str(value)
 
 
 @register("radio")
 class RadioFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
-        raise NotImplementedError
+        return _get_value_label(component["values"], value)
 
 
 @register("signature")
 class SignatureFormatter(FormioFormatter):
     def format(self, component: Dict, value: str) -> str:
+        # TODO custom formatting?
         raise NotImplementedError

src/openforms/submissions/models.py

@@ -3,18 +3,16 @@
 import uuid
 from collections import OrderedDict, defaultdict
 from dataclasses import dataclass
-from datetime import date, datetime, timedelta
+from datetime import date, timedelta
 from typing import Any, Dict, List, Mapping, Optional, Tuple, Union
 
 from django.contrib.postgres.fields import JSONField
 from django.core.files.base import ContentFile, File
 from django.db import models, transaction
 from django.template import Context, Template
-from django.template.defaultfilters import date as fmt_date, time as fmt_time
 from django.template.loader import render_to_string
 from django.urls import resolve
 from django.utils import timezone
-from django.utils.dateparse import parse_date, parse_time
 from django.utils.translation import gettext, gettext_lazy as _
 
 from celery.result import AsyncResult
@@ -26,6 +24,7 @@
 
 from openforms.config.models import GlobalConfiguration
 from openforms.emails.utils import sanitize_content
+from openforms.formio.formatters.registry import register
 from openforms.forms.models import FormStep
 from openforms.payments.constants import PaymentStatus
 from openforms.utils.validators import (
@@ -454,26 +453,22 @@ def get_ordered_data_with_component_type(self) -> OrderedDict:
         for component in self.form.iter_components(recursive=True):
             key = component["key"]
             if key in merged_data:
-                ordered_data[key] = {
-                    "type": component["type"],
-                    "value": merged_data[key],
-                    "label": component.get("label", key),
-                    "multiple": component.get("multiple", False),
-                    # The select component has the values/labels nested in a 'data' field
-                    "values": component.get("values")
-                    or component.get("data", {}).get("values"),
-                    # appointments stuff...
-                    "appointments": component.get("appointments", {}),
-                }
+                component.setdefault("label", key)
+                ordered_data[key] = (
+                    component,
+                    merged_data[key],
+                )
 
         # now append remaining data that doesn't have a matching component
         for key, value in merged_data.items():
             if key not in ordered_data:
-                ordered_data[key] = {
-                    "type": "unknown component",
-                    "value": merged_data[key],
-                    "label": key,
-                }
+                ordered_data[key] = (
+                    {
+                        "type": "unknown component",
+                        "label": key,
+                    },
+                    merged_data[key],
+                )
 
         return ordered_data
 
@@ -526,87 +521,25 @@ def get_merged_data(self) -> dict:
 
     data = property(get_merged_data)
 
-    @staticmethod
-    def _get_value_label(possible_values: list, value: str) -> str:
-        for possible_value in possible_values:
-            if possible_value["value"] == value:
-                return possible_value["label"]
-        # TODO what if value is not a string? shouldn't it be passed through something to covert for display?
-        return value
-
     def get_printable_data(
-        self, limit_keys_to: Optional[List[str]] = None, use_merged_data_fallback=False
+        self,
+        limit_keys_to: Optional[List[str]] = None,
     ) -> Dict[str, str]:
         printable_data = OrderedDict()
-        attachment_data = self.get_merged_attachments()
-        merged_data = self.get_merged_data() if use_merged_data_fallback else {}
 
-        for key, info in self.get_ordered_data_with_component_type().items():
+        for key, (info, value) in self.get_ordered_data_with_component_type().items():
             if limit_keys_to and key not in limit_keys_to:
                 continue
 
-            label = info["label"]
-
-            if info["type"] == "file":
-                files = attachment_data.get(key)
-                if files:
-                    printable_data[label] = _("attachment: %s") % (
-                        ", ".join(file.get_display_name() for file in files)
-                    )
-                # FIXME: ugly workaround to patch the demo, this should be fixed properly
-                elif use_merged_data_fallback:
-                    printable_data[label] = merged_data.get(key)
-                else:
-                    printable_data[label] = _("empty")
-
-            elif info["type"] == "date" or (
-                info.get("appointments", {}).get("showDates", False)
-            ):
-                printable_data[label] = _join_mapped(
-                    lambda v: fmt_date(parse_date(v)), info["value"]
-                )
-
-            elif info["type"] == "time" or (
-                appointment_time := info.get("appointments", {}).get("showTimes", False)
-            ):
-                _parse_time = datetime.fromisoformat if appointment_time else parse_time
-                # strip off the seconds
-                printable_data[label] = _join_mapped(
-                    lambda v: fmt_time(_parse_time(v)), info["value"]
-                )
-
-            elif info["type"] == "selectboxes":
-                selected_values: Dict[str, bool] = info["value"]
-                selected_labels = [
-                    entry["label"]
-                    for entry in info["values"]
-                    if selected_values.get(entry["value"])
-                ]
-                printable_data[label] = ", ".join(selected_labels)
-
-            elif type(info["value"]) is dict:
-                printable_value = info["value"]
-                if "name" in printable_value:
-                    printable_data[label] = printable_value["name"]
-                else:
-                    printable_value = list(printable_value.values())[0]
-                    logger.warning(
-                        'Key "%s" is a dict with unknown values (%s). The first value "%s" was stored.',
-                        key,
-                        info["value"],
-                        printable_value,
-                    )
-                    printable_data[label] = printable_value["name"]
-            else:
-                printable_value = info["value"]
-                if info.get("values"):
-                    # Case in which each value has a label (for example select and radio components)
-                    printable_value = self._get_value_label(
-                        info["values"], info["value"]
-                    )
+            formatter = (
+                register[info["type"]]
+                if info["type"] in register
+                else register["default"]
+            )
 
-                # more here? like getComponentValue() in the SDK?
-                printable_data[label] = printable_value
+            label = info["label"]
+            formatted = formatter(info, value)
+            printable_data[label] = formatted
 
         # finally, check if we have co-sign information to append
         if self.co_sign_data:

src/openforms/submissions/templates/admin/submissions/submission/change_form.html

@@ -8,14 +8,16 @@
                 <label>{% trans "Merged Data:" %}</label>
                 <div class="readonly">
                     <ul>
-                        {% for key, value in data.items %}
-                            {% if value.type == 'unknown component' %}
+                        {% for key, info in data.items %}
+                            {% with component=info.0 value=info.1 %}
+                            {% if component.type == 'unknown component' %}
                                 <li>{% trans "Unknown component" %}</li>
-                            {% elif value.type in image_components %}
-                                <li>{{ key }}: <img class='signature-image' src='{{ value.value }}' alt='{{ key }}'></li>
+                            {% elif component.type in image_components %}
+                                <li>{{ key }}: <img class='signature-image' src='{{ value }}' alt='{{ key }}'></li>
                             {% else %}
-                                <li>{{ key }}: {{ value.value }}</li>
+                                <li>{{ key }}: {{ value }}</li>
                             {% endif %}
+                            {% endwith %}
                         {% endfor %}
                     </ul>
                 </div>