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

Update synced media references when saving a post with ongoing uploads #17157

Merged
merged 5 commits into from
Sep 20, 2021

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented Sep 15, 2021

Fixes WordPress/gutenberg#33593

To test:

  1. Create a post.
  2. Add a gallery block and tap "Add Media".
  3. Select "Choose from device" option.
  4. Select multiple images from the device and confirm the selection.
  5. While images are uploading, leave the editor.
  6. Wait for uploads to complete while in the post list.
  7. Re-open the post with the gallery block.
  8. Tap on the post settings button (three dots button located at the top).
  9. Tap on "Switch to HTML mode"
  10. Observe that the src attributes of all img tags reference a remote URL and not a local version.
    • Remote URLs start with https://
    • Local URLs start with ///

Regression Notes

  1. Potential unintended areas of impact
    The fix is specific to the post save process when there are uploads in progress, so apart from this area, I don't expect other unintended areas of impact.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    I tested in the following scenarios for this area:

  • Save a post with completed and pending media uploads
  • Save a post with only pending media uploads
  • Save a post with no media uploads
  1. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Sep 15, 2021

You can test the changes on this Pull Request by downloading it from AppCenter here with build number: 56815. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Sep 15, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@mokagio
Copy link
Contributor

mokagio commented Sep 20, 2021

I'm going to bump this to the next release because we'll be code freezing 18.4 today and this hasn't been approved yet.

If this cannot wait two weeks and it's important that it makes it into this release, let me know and we'll organize a new beta once ready.

@mokagio mokagio modified the milestones: 18.3, 18.4 Sep 20, 2021
@fluiddot
Copy link
Contributor Author

I'm going to bump this to the next release because we'll be code freezing 18.4 today and this hasn't been approved yet.

If this cannot wait two weeks and it's important that it makes it into this release, let me know and we'll organize a new beta once ready.

Thanks @mokagio for bumping it to the next release 🙇 , this fix is not critical so it can wait to the next version.

Copy link
Contributor

@ceyhun ceyhun left a comment

Choose a reason for hiding this comment

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

I was able to reproduce the issue consistently following the instructions in WordPress/gutenberg#33593 (comment) and verified the fix and it LGTM 👍 We only need to update the release notes to be in the correct version.

@fluiddot
Copy link
Contributor Author

I was able to reproduce the issue consistently following the instructions in WordPress/gutenberg#33593 (comment) and verified the fix and it LGTM 👍 We only need to update the release notes to be in the correct version.

Thanks @ceyhun for the review ❤️! I've just updated the release notes in this commit, once the CI checks pass I'll merge it.

@fluiddot fluiddot merged commit 570b077 into develop Sep 20, 2021
@fluiddot fluiddot deleted the fix/media-local-urls-post-save branch September 20, 2021 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gallery block: Some images point to local URLs when closing the post with ongoing uploads
3 participants