Improve the code that creates submission attachments from files in submission data #2713
Comments
|
Refinement: We don't know how to make it better but it should be. @Viicos can write down his proposal. Special attention is needed with editgrids. |
|
Took a look at that part of the code-base: it seems closely related to #3154, in fact tackling this issue probably requires tackling the meta-issue. I'm not sure how the |
9 tasks
sergei-maertens
added a commit
that referenced
this issue
Jan 29, 2025
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.
10 tasks
sergei-maertens
added a commit
that referenced
this issue
Jan 30, 2025
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.
sergei-maertens
added a commit
that referenced
this issue
Jan 31, 2025
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.
sergei-maertens
added a commit
that referenced
this issue
Jan 31, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Thema / Theme
API
Omschrijving / Description
The bandaid fix for this issue #2699 showed that the code for creating submission attachments by looking through the submission data and extracting any file is quite fragile.
This should be refactored into a more robust solution.
Aspects to take into account:
formio/rendering/default.py-> there's a lot of configuration/data path mangling/processing - this would benefit from normalization so we have a more simple interfacepath/json_renderer_path/configuration_pathcomplexity insrc/openforms/formio/rendering/nodes.pyThe text was updated successfully, but these errors were encountered: