Skip to content

Create tempfile when Active Storage service doesn't implement path_for method#9362

Merged
ahukkanen merged 8 commits intodecidim:developfrom
mainio:fix/file_path_with_s3
Jun 14, 2022
Merged

Create tempfile when Active Storage service doesn't implement path_for method#9362
ahukkanen merged 8 commits intodecidim:developfrom
mainio:fix/file_path_with_s3

Conversation

@lahdeero
Copy link
Copy Markdown
Contributor

@lahdeero lahdeero commented May 25, 2022

🎩 What? Why?

When Decidim instance was using different active storage service than local it raised error when asked path_for (for a file). Here we download the file as a tempfile (i.e. download file from AWS S3 to server's temp folder) if the service doesn't implement path_for method.

📌 Related Issues

♥️ Thank you!

@lahdeero lahdeero marked this pull request as draft May 25, 2022 13:15
@lahdeero lahdeero marked this pull request as ready for review May 27, 2022 13:49
@andreslucena andreslucena added module: core type: fix PRs that implement a fix for a bug release: v0.27 Issues or PRs that need to be tackled for v0.27 labels May 31, 2022
@oriolgual
Copy link
Copy Markdown
Contributor

Could this be merged?

Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Tested this with an actual S3 bucket and it worked fine.

I have merged with develop myself and fixed the merge conflict.

@ahukkanen ahukkanen merged commit c75d594 into decidim:develop Jun 14, 2022
@ahukkanen ahukkanen deleted the fix/file_path_with_s3 branch June 14, 2022 17:28
andreslucena pushed a commit that referenced this pull request Jul 6, 2022
…r method (#9362)

* Create tempfile if file isnt available locally

* Update test names

* Make sure tempfile is deleted after use

* Undo importer changes

* Update readers and their specs

* Update importer test

* Process file in importer instead of reader

Co-authored-by: Eero Lahdenperä <eero.lahdenpera@mainiotech.fi>
Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>
eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
…r method (decidim#9362)

* Create tempfile if file isnt available locally

* Update test names

* Make sure tempfile is deleted after use

* Undo importer changes

* Update readers and their specs

* Update importer test

* Process file in importer instead of reader

Co-authored-by: Eero Lahdenperä <eero.lahdenpera@mainiotech.fi>
Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: core release: v0.27 Issues or PRs that need to be tackled for v0.27 type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dynamic attachment uploads do not work with cloud storage services

5 participants