Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to upload cloudinary file to specific folder with path variable #13919

Merged
merged 4 commits into from Feb 2, 2023

Conversation

angelbanderasudg
Copy link
Contributor

What does it do?

This makes possible to upload files to a specific folder for the upload-cloudinary provider, like the aws-s3 provider, taking the path variable as the folder

Why is it needed?

By using it, you can organize your files on cloudinary

How to test it?

Just set your cloudinary details for the plugin, open the /upload endpoint and send a request with the "files" and "path" variable

Related issue(s)/PR(s)


@alexandrebodin alexandrebodin added source: core:upload Source is core/upload package pr: enhancement This PR adds or updates some part of the codebase or features labels Aug 8, 2022
@SalahAdDin
Copy link

What does it do?

This makes it possible to upload files to a specific folder for the upload-cloudinary provider, like the aws-s3 provider, taking the path variable as the folder

Why is it needed?

By using it, you can organize your files on cloudinary

How to test it?

Just set your cloudinary details for the plugin, open the /upload endpoint and send a request with the "files" and "path" variable

Related issue(s)/PR(s)

Is it all? Just these few lines?

@angelbanderasudg Can you add documentation for this?

Also, what about setting the main default folder for it? Let's say, we have more projects connected to the same Cloudinary "bucket", so we want to set all media related to project A in projectA folder(all the Strapi folder within), and the media from project B in projectB, etc.

It would like to add a prefix for the file path with the name of the project(from an environment variable, for instance, or from another Strapi setting), and following by the folder path, which is already a property for any media content.

What do you think about this @derrickmehaffy and Strapi staff?

@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/media-library-folders-beta-is-live/19515/34

@alexandrebodin alexandrebodin requested review from Marc-Roig and removed request for petersg83 November 23, 2022 10:00
@diegovfeder
Copy link

Any updates here? Looking forward to this.

@Marc-Roig
Copy link
Contributor

Marc-Roig commented Jan 27, 2023

This looks good to me, could you fix the merge conflicts?
Also the branch should not be named master, but should follow the Contribution guidelines of fix/<fix-name>

@codecov
Copy link

codecov bot commented Feb 2, 2023

Codecov Report

Base: 59.11% // Head: 59.11% // No change to project coverage 👍

Coverage data is based on head (fdc362d) compared to base (b697228).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13919   +/-   ##
=======================================
  Coverage   59.11%   59.11%           
=======================================
  Files        1502     1502           
  Lines       38360    38360           
  Branches     7385     7385           
=======================================
  Hits        22677    22677           
  Misses      13411    13411           
  Partials     2272     2272           
Flag Coverage Δ
back 47.38% <ø> (ø)
front 67.15% <ø> (ø)
unit_back 47.38% <ø> (ø)
unit_front 67.15% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Marc-Roig
Copy link
Contributor

Fixed the conflicts :) Everything works on my side, I will merge when tests pass

@Marc-Roig Marc-Roig added this to the 4.6.1 milestone Feb 2, 2023
@Marc-Roig Marc-Roig merged commit 096d00a into strapi:main Feb 2, 2023
@SalahAdDin
Copy link

What does it do?

This makes possible to upload files to a specific folder for the upload-cloudinary provider, like the aws-s3 provider, taking the path variable as the folder

Why is it needed?

By using it, you can organize your files on cloudinary

How to test it?

Just set your cloudinary details for the plugin, open the /upload endpoint and send a request with the "files" and "path" variable

Related issue(s)/PR(s)

Can you document this?

@Marc-Roig
Copy link
Contributor

Marc-Roig commented Feb 6, 2023

@SalahAdDin For some reason I completely missed your message on this thread. Sorry.

About the documentation, I am looking for a way we could document providers. https://docs.strapi.io/developer-docs/latest/plugins/upload.html#examples Right now, in the docs, we only mention the user to look up the provider configuration.

But we should have a way to document this, I will take a look.

Also, what about setting the main default folder for it? Let's say, we have more projects connected to the same Cloudinary "bucket", so we want to set all media related to project A in projectA folder(all the Strapi folder within), and the media from project B in projectB, etc.

It would like to add a prefix for the file path with the name of the project(from an environment variable, for instance, or from another Strapi setting), and following by the folder path, which is already a property for any media content.

That is a good idea, but maybe it could fit in another PR so we can have a discussion for it. would you be open to work on it? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: enhancement This PR adds or updates some part of the codebase or features source: core:upload Source is core/upload package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants