Skip to content

Conversation

@hugosolar
Copy link
Contributor

Description of the Change

We've seen some situations where users want to replace a PDF file with an updated version (CV for example)
This PR adds to the plugin the ability to replace pdf files (for now) using a new method to copy the blob and replace it with the uploaded file.
There's a new method called copy_media_to_blob_storage in helpers and also a new copy_blob method in the API client
this uses the Copy Blob operation described here: https://learn.microsoft.com/en-us/rest/api/storageservices/Copy-Blob

There's also a new button added in the media uploader that holds the workflow to replace the document

Open discussion for addressing images

We should have a discussion on how to approach image replacements since an image of a different orientation can be uploaded and image sizes of the image to be replaced won't match, so I was thinking on two paths

  • Should we force the same image sizes from the replaced image so we can have backward compatibility in case those images are being called directly?
  • Should we discard previous images and just replace thumbnails with the ones generated by the new image?

How to test the Change

To test this:

  • Go to the media screen in the admin dashboard (wp-admin/upload.php)
  • Upload a PDF file
  • Select the file and you'll see a "replace this media" button
  • Click the button and select the file to upload (the view is restricted to just upload a file)
  • Once it's uploaded click on the "Replace media" button below
  • That will trigger the replacement of the main file and the thumbnails stored as meta fields

Changelog Entry

Added - New feature to replace PDF files at the blob storage level
Fixed - Bug fix. Fixed a bug with the transient generated for displaying progress

Credits

@hugosolar

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@hugosolar hugosolar self-assigned this Mar 27, 2024
@jeffpaul jeffpaul requested review from NicoleGeissinger and removed request for dkotter and jeffpaul April 1, 2024 21:23
@jeffpaul jeffpaul added this to the 4.5.0 milestone Apr 1, 2024
@jeffpaul
Copy link
Member

jeffpaul commented Apr 1, 2024

@hugosolar looks like failing e2e tests, could you check on those please?

@hugosolar
Copy link
Contributor Author

@jeffpaul I think this is related to the constants passed to Cypress related to the account name & key

here https://github.com/hugosolar/windows-azure-storage/blob/022290b32e875986dc4b790be6891c56ad736c2d/tests/cypress/e2e/settings.test.js#L12

On a previous PR, I remember we faced the same situation and @Sidsector9 was able to fix it
let me know otherwise

@jeffpaul jeffpaul requested a review from Sidsector9 April 12, 2024 18:47
@rickalee
Copy link
Collaborator

@hugosolar Some feedback to address and need to address the E2E tests.

@hugosolar hugosolar requested a review from rickalee April 17, 2024 17:38
@jeffpaul jeffpaul merged commit b5fcb51 into 10up:develop Apr 23, 2024
@hugosolar hugosolar mentioned this pull request May 27, 2024
4 tasks
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.

3 participants