Updated images/files are not persisted in case there is an error on the form

[ahukkanen]

Describe the bug

The file uploader does not send the updated files to the server in case there is an error on the form.

This causes the images not to save properly in case there are error on the forms.

Read through and try out the reproduction instructions as they pretty much explain the problem in more detail.

To Reproduce

1. Go to manage a process
2. Go to the "Attachments" seection
3. Click "New attachment" to Create a new attachment
4. Keep all fields blank but upload an image using the "Add file" button
5. Again, make sure all other fields are blank before submitting the form
6. Submit the form by clicking the "Create attachment" button at the bottom of the screen
7. See that the form was reloaded with some errors
8. Still, keep all the fields blank when there are errors
9. Re-submit the form by clicking "Create attachment"
10. See that the file was cleared

The reason is that when the form is reloaded without persisting the changes to the database, the file blob ID is not included in the form's payload parameters. Therefore, when the form is re-submitted the image information is lost and therefore not persisted.

The same way if you try to update an image and make an error on the form, the image is not updated during the re-submission of the form.

This bug applies to all forms that allow uploading file attachments, e.g. the participatory process form that allows defining an image for the process.

Expected behavior

I would expect that the images and files are always persisted when I create or update a record.

This would require including the file's blob ID on the form's payload parameters every time the form is submitted. We cannot map the image/file to the correct record before the record exists, so it has to happen after the record is created. And for further updates, it would be advisable not to change the record in case there were errors on the form which would also require keeping the blob ID in the form payload.

Screenshots

I am about to create a new attachment, this is the situation before the first submission:

The form errors out, displays the errors and the uploaded image is still visible in the view (but the payload parameter has been lost at this point):

I re-submit the form in this state and next time the form is loaded, the image is gone:

The situation is similar when the records are updated but the error is just a bit harder to notice in the update case.

Stacktrace

N/A

Extra data

• Device: (any)
• Device OS: (any)
• Browser: (any)
• Decidim Version: 0.27.0+ (after the dynamic uploads feature)
• Decidim installation: any instance running v0.27 or v0.28

Additional context

Uncertain, but this might be related to this bug that was fixed in the legacy CarrierWave implementation causing image quality reduction every time the form was updated: #6447.

When the fix is implemented, it should be ensured that the fix does not re-introduce this bug that was fixed in the legacy implementation.

I am unsure if this would even happen with ActiveStorage in the first place. At least with the representation URLs, ActiveStorage processes the image variant when it is displayed/requested for the first time. But unsure how it works for the "default" version of the image, i.e. when is that image processed exactly (I do not know).

I believe the bug was introduced at #8681 (just by looking at the 0.26 code which seemed to include the field value).

[ahukkanen]

Also just a sidenote after investigating this bug that it might be worth to consider a refactor on the image upload logic as a whole.

It seems a bit complex currently because we want to know the (a) the record the image is uploaded against, (b) the form that handles the validation logic and (c) the organization the image is uploaded to.

All these are relevant because:

• (a) The record can set specific upload validations, such as the allowed file formats, types, sizes, etc. for this specific image field. E.g. a "video uploader" can allow only specific formats while the organization could allow more formats.
• (b) The form can add extra validations for the image field (not sure if this is used anywhere but currently possible).
• (c) The organization can set certain constraints on the uploaded image, such as the allowed file formats and file sizes.

It just seems that we are using a lot of logic to achieve this and there is also some things that come from the history within these pieces of logic.

I believe that this could be re-architected to achieve a much simpler way of uploading the images. E.g. do we need to pass all this information to the upload or could we just define some kind of registry where we could define different upload formats, such as :participant_image, :participant_attachment, :admin_image, :admin_attachment, :admin_video, etc.

This way the only two information we would need for the validation would be:

1. The format of the upload (one of the examples above)
2. The organization where the image is uploaded to (available through the upload domain)

Currently it seems we want to read a lot of information from the record and the uploader itself to achieve the same and this causes a lot of seemingly unnecessary wrapping logic for the uploads.

In addition, I would see it extremely useful if we could define the system panel upload configurations for each of these formats individually. E.g. for uploading images, I would see 10 MB limit most of the times more than enough but the same limitations falls short when uploading videos, for instance. Similarly for some cases, I would see it enough for the participant images to be limited at 5 MB but could allow admins to upload attachments up to 20 MB. The latter case is already somewhat possible to achieve with the current configuration but not fully and not very granular.