Skip to content

Commit

Permalink
♻️ [#4689] Move upload processing for Objects API v2
Browse files Browse the repository at this point in the history
Moved the file component key/value processing to the mapped variable
processing instead of special casing this in the registration handler,
since we will need access to the component types to handle editgrids
which have file uploads inside, as these have different data paths
*and* will require recursion as well since there can be editgrids
inside editgrids that have this problem.

The alternative is special casing repeating groups too, which breaks
the mechanism to do component-specific post-processing in the
dedicated function.

This also updates the query for the submission variables so that if we
have a more exact data path for an upload (inside a repeating group)
that we use that instead of messing with the container editgrid which
incorrectly gets replaced now. For uploads *not* in repeating groups,
this data path is empty because it's identical to the variable key,
so we can coalesce there at the DB level to calculate this in a
unified way.

See also #2713 that highlights the difficulties with how file uploads
are now processed, which requires some proper re-structuring.
  • Loading branch information
sergei-maertens committed Jan 31, 2025
1 parent 66a594b commit e3b3634
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 33 deletions.
45 changes: 38 additions & 7 deletions src/openforms/registrations/contrib/objects_api/handlers/v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from collections.abc import Sequence
from dataclasses import dataclass
from datetime import date, datetime
from typing import cast

from glom import Assign, Path, glom

Expand Down Expand Up @@ -56,7 +57,31 @@ def process_mapped_variable(
JSONValue | date | datetime
), # can't narrow it down yet, as the type depends on the component type
component: Component | None = None,
attachment_urls: dict[str, list[str]] | None = None,
) -> AssignmentSpec | Sequence[AssignmentSpec]:
"""
Apply post-processing to a mapped variable.
A mapped variable may have additional options that specify the behaviour of how the
values are translated before they end up in the Objects API record. Often, these
transformations are dependent on the component type being processed.
:arg mapping: The mapping of form variable to destination path, including possible
component-specific configuration options that influence the mapping behaviour.
:arg value: The raw value of the form variable for the submission being processed.
The type/shape of the value depends on the variable/component data type being
processed and even the component configuration (such as multiple True/False).
:arg component: If the variable corresponds to a Formio component, the component
definition is provided, otherwise ``None``.
:arg attachment_urls: The registration plugin uploads attachments to a Documents API
and provides the API resource URL for each attachment. Keys are the data paths in
the (Formio) submission data, e.g. `myAttachment` or ``repeatingGroups.2.file``.
:returns: A single assignment spec or collection of assignment specs that specify
which value needs to be written to which "object path" for the record data, for
possible deep assignments.
"""

target_path = Path(*bits) if (bits := mapping.get("target_path")) else None

# normalize non-primitive date/datetime values so that they're ready for JSON
Expand Down Expand Up @@ -95,14 +120,20 @@ def process_mapped_variable(
assert target_path is not None
return AssignmentSpec(destination=target_path, value=value)

# multiple files - return an array
case {"type": "file", "multiple": True}:
assert isinstance(value, list)
case {"type": "file", **rest}:
assert attachment_urls is not None
multiple = rest.get("multiple", False)
upload_urls = attachment_urls[mapping["variable_key"]]

# single file - return only one element
case {"type": "file"}:
assert isinstance(value, list)
value = value[0] if value else ""
transformed_value = str | list[str]

# for multiple uploads, replace the Formio file dicts with our upload URLs
if multiple:
transformed_value = upload_urls
# single file - return only one element *if* there are uploads
else:
transformed_value = upload_urls[0] if upload_urls else ""
value = cast(JSONValue, transformed_value)

case {"type": "map"}:
assert isinstance(value, dict)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
override,
)

from django.db.models import F
from django.db import models
from django.db.models import F, Value
from django.db.models.functions import Coalesce, NullIf

from openforms.authentication.service import AuthAttribute
from openforms.contrib.objects_api.clients import (
Expand Down Expand Up @@ -478,11 +480,19 @@ def get_attachment_urls_by_key(submission: Submission) -> dict[str, list[str]]:
attachments = ObjectsAPISubmissionAttachment.objects.filter(
submission_file_attachment__submission_variable__submission=submission
).annotate(
variable_key=F("submission_file_attachment__submission_variable__key")
data_path=Coalesce(
NullIf(
F("submission_file_attachment___component_data_path"),
Value(""),
),
# fall back to variable/component key if no explicit data path is set
F("submission_file_attachment__submission_variable__key"),
output_field=models.TextField(),
),
)
for attachment_meta in attachments:
key: str = (
attachment_meta.variable_key # pyright: ignore[reportAttributeAccessIssue]
attachment_meta.data_path # pyright: ignore[reportAttributeAccessIssue]
)
urls_map[key].append(attachment_meta.document_url)
return urls_map
Expand Down Expand Up @@ -528,28 +538,21 @@ def get_record_data(
variable = None

value: JSONValue | date | datetime
# special casing documents - we transform the formio file upload data into
# the api resource URLs for the uploaded documents in the Documens API.
# Normalizing to string/array of strings is done later via
# process_mapped_variable which receives the component configuration.
if key in urls_map:
value = urls_map[key] # pyright: ignore[reportAssignmentType]
else:
try:
value = all_values[key]
except KeyError:
logger.info(
"Expected key %s to be present in the submission (%s) variables, "
"but it wasn't. Ignoring it.",
key,
submission.uuid,
extra={
"submission": submission.uuid,
"key": key,
"mapping_config": mapping,
},
)
continue
try:
value = all_values[key]
except KeyError:
logger.info(
"Expected key %s to be present in the submission (%s) variables, "
"but it wasn't. Ignoring it.",
key,
submission.uuid,
extra={
"submission": submission.uuid,
"key": key,
"mapping_config": mapping,
},
)
continue

# Look up if the key points to a form component that provides additional
# context for how to process the value.
Expand All @@ -562,7 +565,10 @@ def get_record_data(

# process the value so that we can assign it to the record data as requested
assignment_spec = process_mapped_variable(
mapping=mapping, value=value, component=component
mapping=mapping,
value=value,
component=component,
attachment_urls=urls_map,
)
if isinstance(assignment_spec, AssignmentSpec):
assignment_specs.append(assignment_spec)
Expand Down

0 comments on commit e3b3634

Please sign in to comment.