Dynamic attachment uploads

[lahdeero]

🎩 What? Why?

Change attachments so that uploading happens in popup modal, where user can select or "drop" file(s) they want to upload. In this modal we show upload progress and after file has been uploaded (progress is 100%), we will validate the uploaded file. In case validations fail we give immediate feedback, so user can retry uploading after fixing errors (usually this means uploading different file), according to the error message(s).

There are two kind of attachments depending on whether the attachment is titled or not (technically form.attachment (titled) and form.upload (untitled)). They have slightly different UI's (see protos and screenshots below) and untitled attachments doesn't support multiple files! Untitled attachments can be removed inside a form or modal, but titled attachments can only be removed inside a modal.

Changes made inside the upload modal are not applied until user has pressed save button. For example, if a user wants to change the title of an attachment they need to press save before submitting the form or change won't be applied.

Untitled fields should not require any changes in the views. To update titled field in views

<%= form.file_field :add_documents, 
  multiple: true, 
  label: t("add_documents", scope: "decidim.proposals.proposals.edit") %>

changes to:

<%= form.attachment :documents,
  multiple: true,
  label: t("add_documents", scope: "decidim.proposals.proposals.edit"),
  button_label: t("decidim.proposals.proposals.edit.add_documents", scope: "decidim.proposals.proposals.edit"),
  button_edit_label: t("edit_documents", scope: "decidim.proposals.proposals.edit") %>

When updating form objects and commands, note that the frontend now gives a signed id (String), which you need to be able to find the ActiveStorage::Blob object.

📌 Related Issues

• Related to: Meta Decidim proposal
• Fixes: Native language for "Choose file" #6204
• Prototypes:
  • Figma attachments
  • Figma uploads

Testing

All file upload fields, for example:

1. Add image and documents to proposal
2. Change avatar
3. Change images in organization's appearance settings.
4. Import proposals

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

• ❓ CONSIDER adding a unit test if your PR resolves an issue.
• ✔️ DO check open PR's to avoid duplicates.
• ✔️ DO keep pull requests small so they can be easily reviewed.
• ✔️ DO build locally before pushing.
• ✔️ DO make sure tests pass.
• ✔️ DO make sure any new changes are documented in docs/.
• ✔️ DO add and modify seeds if necessary.
• ✔️ DO add CHANGELOG upgrade notes if required.
• ✔️ DO add to GraphQL API if there are new public fields.
• ✔️ DO add link to MetaDecidim if it's a new feature.
• ❌AVOID breaking the continuous integration build.
• ❌AVOID making significant changes to the overall architecture.

📷 Screenshots

♥️ Thank you!

[lahdeero]

@andreslucena I added a CHANGELOG entry. It’s hard to make it comprehensive (since there is so many different cases), but let me know if anything is missing.

[andreslucena]

👍🏽 👍🏽

[andreslucena]

@lahdeero I've just seen that we have a related to this PR flaky at develop. It seems like it's reproducible, at least I can do it locally and also in the last 3 runs of the pipeline always failed. Can you check it out please? https://github.com/decidim/decidim/runs/5409976604?check_suite_focus=true

[oriolgual]

This seems to have broken the integration with Amazon S3. When trying to upload a census to a Voting space using Amazon S3 as uploads, I get this error:

NoMethodError (undefined method `path_for' for #<ActiveStorage::Service::S3Service:0x000055fbb8327ad0 @client=#<Aws::S3::Resource:0x000055fbb8327850 @client=#<Aws::S3::Client>>, @bucket=#<Aws::S3::Bucket:0x000055fbb8595448 @name="decidim-staging", @data=nil, @client=#<Aws::S3::Client>, @waiter_block_warned=false, @resolved_region="eu-central-1", @arn=nil>, @multipart_upload_threshold=104857600, @public=false, @upload_options={}, @name=:amazon>):
decidim (b5eedf63462a) decidim-elections/app/commands/decidim/votings/census/admin/create_dataset.rb:88:in `file_path'
decidim (b5eedf63462a) decidim-elections/app/commands/decidim/votings/census/admin/create_dataset.rb:84:in `file_lines_count'
decidim (b5eedf63462a) decidim-elections/app/commands/decidim/votings/census/admin/create_dataset.rb:80:in `csv_row_count'
decidim (b5eedf63462a) decidim-elections/app/commands/decidim/votings/census/admin/create_dataset.rb:52:in `create_census_dataset!'
decidim (b5eedf63462a) decidim-elections/app/commands/decidim/votings/census/admin/create_dataset.rb:26:in `call'
decidim (b5eedf63462a) decidim-core/lib/decidim/command.rb:18:in `call'
decidim (b5eedf63462a) decidim-elections/app/controllers/decidim/votings/census/admin/census_controller.rb:28:in `create'
actionpack (6.1.6) lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action'
actionpack (6.1.6) lib/abstract_controller/base.rb:228:in `process_action'
actionpack (6.1.6) lib/action_controller/metal/rendering.rb:30:in `process_action'

Apparently the S3 adapter doesn't implement path_for

[ahukkanen]

Thanks for reporting this issue @oriolgual !

I've create a bug report at #9360 to track this.