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

Use tmp folder to store temp uploaded logos in project admin form #2092

Draft
wants to merge 43 commits into
base: develop
Choose a base branch
from

Conversation

luistoptal
Copy link
Collaborator

@luistoptal luistoptal commented Nov 15, 2024

Pull request for issue: #376

IMPORTANT: this PR depends on this other PR: #2070 , which should be merged first

This is a pull request for the following functionalities:

The logo upload process involves:

  • uploading a "temp" logo to the server via the Uppy uploader. Before this PR, the logo was uploaded to a "temp" folder in a S3 bucket
  • confirming the temp logo once the project is created / updated
  • the logo is copied to the storage (a S3 bucket)

This PR simplifies the process by saving the logo in the /tmp folder of the server machine and is suppposed to rely in whatever auto clean up config is setup in the server. The rest of the steps are unchanged

How to test?

You will need to re-run ./up.sh and follow the instructions in #2070 to setup the storage to work local dev. Then, if you follow the same instructions to test in the mentioned PR, nothing should change from a frontend perspective.

Also, the playwright tests should pass when you run:

cd playwright
npm install
npm run test:single admin-project-logo-upload.spec.js

How have functionalities been implemented?

  • the idea is to expose the /tmp folder of the server to the application (server) container so a volume - /tmp:/var/www/datasetfiles was added. The same volume exists in the web (frontend) container and this is necessary because the frontend needs to be able to access and display the temp logos
  • the deleteTempLogo method was removed since we rely on the /tmp autoremoval config on the server
  • added a writeTmpLogoFromFile to write the temp logo uplloaded by the uppy loader to the /tmp folder
  • updated the writeLogoFromUrl function to take into account that the incoming file is in the server

Any issues with implementation?

  • I do not have access to testing on a staging environment on a real server, this PR should be tested in staging before merging

Any changes to automated tests?


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant