🐛 [#4689] Fix uploads in repeating groups not being processed correctly

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.

We must look up the uploads in the url map and replace only the
file component nodes in the item values rather than the whole
repeating group. This also needs to recurse, since in theory
the repeating group can have another repeating group inside
it.

Backport-of: #5059

setup.cfg

@@ -63,3 +63,4 @@ exclude_also =
     \.\.\.
     pass$
     if settings.DEBUG:
+    assert_never\(.*\)

src/openforms/registrations/contrib/objects_api/handlers/v2.py

@@ -5,11 +5,14 @@
 from collections.abc import Sequence
 from dataclasses import dataclass
 from datetime import date, datetime
+from typing import assert_never, cast
 
 from glom import Assign, Path, glom
 
 from openforms.api.utils import underscore_to_camel
-from openforms.formio.typing import Component
+from openforms.formio.service import FormioData
+from openforms.formio.typing import Component, EditGridComponent
+from openforms.formio.typing.vanilla import FileComponent
 from openforms.typing import JSONObject, JSONValue
 
 from ..typing import ObjecttypeVariableMapping
@@ -56,7 +59,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.
+    """
+    variable_key = mapping["variable_key"]
     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
@@ -95,14 +122,11 @@ 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)
-
-        # single file - return only one element
         case {"type": "file"}:
-            assert isinstance(value, list)
-            value = value[0] if value else ""
+            assert attachment_urls is not None
+            value = _transform_file_value(
+                cast(FileComponent, component), attachment_urls
+            )
 
         case {"type": "map"}:
             # Currently we only support Point coordinates
@@ -113,9 +137,90 @@ def process_mapped_variable(
                 "coordinates": [value[1], value[0]],
             }
 
+        case {"type": "editgrid"} if attachment_urls is not None:
+            assert isinstance(value, list)
+            value = _transform_editgrid_value(
+                cast(EditGridComponent, component),
+                cast(list[JSONObject], value),
+                attachment_urls=attachment_urls,
+                key_prefix=variable_key,
+            )
         # not a component or standard behaviour where no transformation is necessary
         case None | _:
             pass
 
     assert target_path is not None
     return AssignmentSpec(destination=target_path, value=value)
+
+
+def _transform_file_value(
+    component: FileComponent,
+    attachment_urls: dict[str, list[str]],
+    key_prefix: str = "",
+) -> str | list[str]:
+    """
+    Transform a single file component value according to the component configuration.
+    """
+    key = component["key"]
+    multiple = component.get("multiple", False)
+
+    # it's possible keys are missing because there are no uploads at all for the
+    # component.
+    data_path = f"{key_prefix}.{key}" if key_prefix else key
+    upload_urls = attachment_urls.get(data_path, [])
+
+    match upload_urls:
+        # if there are no uploads and it's a single component -> normalize to empty string
+        case [] if not multiple:
+            return ""
+
+        # if there's an upload and it's a single component -> return the single URL string
+        case list() if upload_urls and not multiple:
+            return upload_urls[0]
+
+        # otherwise just return the list of upload URLs
+        case list():
+            assert multiple
+            return upload_urls
+
+        case _:
+            assert_never(upload_urls)
+
+
+def _transform_editgrid_value(
+    component: EditGridComponent,
+    value: list[JSONObject],
+    attachment_urls: dict[str, list[str]],
+    key_prefix: str,
+) -> list[JSONObject]:
+    nested_components = component["components"]
+
+    items: list[JSONObject] = []
+
+    # process file uploads inside (nested) repeating groups
+    for index, item in enumerate(value):
+        item_values = FormioData(item)
+
+        for nested_component in nested_components:
+            key = nested_component["key"]
+
+            match nested_component:
+                case {"type": "file"}:
+                    item_values[key] = _transform_file_value(
+                        cast(FileComponent, nested_component),
+                        attachment_urls=attachment_urls,
+                        key_prefix=f"{key_prefix}.{index}",
+                    )
+                case {"type": "editgrid"}:
+                    nested_items = item_values[key]
+                    assert isinstance(nested_items, list)
+                    item_values[key] = _transform_editgrid_value(
+                        cast(EditGridComponent, nested_component),
+                        value=cast(list[JSONObject], nested_items),
+                        attachment_urls=attachment_urls,
+                        key_prefix=f"{key_prefix}.{index}.{key}",
+                    )
+
+        items.append(item_values.data)
+
+    return items

src/openforms/registrations/contrib/objects_api/submission_registration.py

@@ -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 (
@@ -488,11 +490,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
@@ -538,28 +548,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.
@@ -572,7 +575,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)

src/openforms/registrations/contrib/objects_api/tests/files/vcr_cassettes/ObjectsAPIBackendV2Tests/ObjectsAPIBackendV2Tests.test_addressNl_legacy_before_of_30.yaml

@@ -20,7 +20,7 @@ interactions:
         everything","namePlural":"Accepts everything","description":"","dataClassification":"open","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2024-07-22","modifiedAt":"2024-07-22","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/8faed0fa-7864-4409-aa6d-533a37616a9e/versions/1"]},{"url":"http://objecttypes-web:8000/api/v2/objecttypes/644ab597-e88c-43c0-8321-f12113510b0e","uuid":"644ab597-e88c-43c0-8321-f12113510b0e","name":"Fieldset
         component","namePlural":"Fieldset component","description":"","dataClassification":"confidential","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2024-02-08","modifiedAt":"2024-02-08","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/644ab597-e88c-43c0-8321-f12113510b0e/versions/1"]},{"url":"http://objecttypes-web:8000/api/v2/objecttypes/f1dde4fe-b7f9-46dc-84ae-429ae49e3705","uuid":"f1dde4fe-b7f9-46dc-84ae-429ae49e3705","name":"Geo
         in data","namePlural":"Geo in data","description":"","dataClassification":"confidential","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2024-02-08","modifiedAt":"2024-02-08","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/f1dde4fe-b7f9-46dc-84ae-429ae49e3705/versions/1"]},{"url":"http://objecttypes-web:8000/api/v2/objecttypes/527b8408-7421-4808-a744-43ccb7bdaaa2","uuid":"527b8408-7421-4808-a744-43ccb7bdaaa2","name":"File
-        Uploads","namePlural":"File Uploads","description":"","dataClassification":"confidential","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2024-02-08","modifiedAt":"2024-02-08","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/527b8408-7421-4808-a744-43ccb7bdaaa2/versions/1"]},{"url":"http://objecttypes-web:8000/api/v2/objecttypes/3edfdaf7-f469-470b-a391-bb7ea015bd6f","uuid":"3edfdaf7-f469-470b-a391-bb7ea015bd6f","name":"Tree","namePlural":"Trees","description":"","dataClassification":"confidential","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2024-02-08","modifiedAt":"2024-02-08","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/3edfdaf7-f469-470b-a391-bb7ea015bd6f/versions/1"]},{"url":"http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48","uuid":"8e46e0a5-b1b4-449b-b9e9-fa3cea655f48","name":"Person","namePlural":"Persons","description":"","dataClassification":"open","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2023-10-24","modifiedAt":"2024-11-25","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48/versions/1","http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48/versions/2","http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48/versions/3","http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48/versions/4"]}]}'
+        Uploads","namePlural":"File Uploads","description":"","dataClassification":"confidential","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2024-02-08","modifiedAt":"2024-02-08","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/527b8408-7421-4808-a744-43ccb7bdaaa2/versions/1"]},{"url":"http://objecttypes-web:8000/api/v2/objecttypes/3edfdaf7-f469-470b-a391-bb7ea015bd6f","uuid":"3edfdaf7-f469-470b-a391-bb7ea015bd6f","name":"Tree","namePlural":"Trees","description":"","dataClassification":"confidential","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2024-02-08","modifiedAt":"2024-02-08","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/3edfdaf7-f469-470b-a391-bb7ea015bd6f/versions/1"]},{"url":"http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48","uuid":"8e46e0a5-b1b4-449b-b9e9-fa3cea655f48","name":"Person","namePlural":"Persons","description":"","dataClassification":"open","maintainerOrganization":"","maintainerDepartment":"","contactPerson":"","contactEmail":"","source":"","updateFrequency":"unknown","providerOrganization":"","documentationUrl":"","labels":{},"createdAt":"2023-10-24","modifiedAt":"2024-11-25","allowGeometry":true,"versions":["http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48/versions/2","http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48/versions/3","http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48/versions/1","http://objecttypes-web:8000/api/v2/objecttypes/8e46e0a5-b1b4-449b-b9e9-fa3cea655f48/versions/4"]}]}'
     headers:
       Allow:
       - GET, POST, HEAD, OPTIONS
@@ -33,11 +33,11 @@ interactions:
       Cross-Origin-Opener-Policy:
       - same-origin
       Date:
-      - Thu, 19 Dec 2024 08:19:29 GMT
+      - Wed, 29 Jan 2025 17:01:16 GMT
       Referrer-Policy:
       - same-origin
       Server:
-      - nginx/1.27.0
+      - nginx/1.27.3
       Vary:
       - origin
       X-Content-Type-Options:
@@ -77,11 +77,11 @@ interactions:
       Cross-Origin-Opener-Policy:
       - same-origin
       Date:
-      - Thu, 19 Dec 2024 08:19:29 GMT
+      - Wed, 29 Jan 2025 17:01:16 GMT
       Referrer-Policy:
       - same-origin
       Server:
-      - nginx/1.27.0
+      - nginx/1.27.3
       Vary:
       - origin
       X-Content-Type-Options: