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

Handle stories media uploading post-publish #16445

Merged
merged 7 commits into from
May 11, 2021
Merged

Conversation

bjtitus
Copy link
Contributor

@bjtitus bjtitus commented May 7, 2021

Fixes #16054

Related PRs

Summary

Media Uploading

Changes the media upload process for stories to add media to the post through the existing addMedia method. Upon saving (and publishing) the post, the media references are updated if upload has completed or observed and then updated once complete.

This prevents the case where media may not be uploaded by the time the Publish button was tapped, leading to a case where post status wasn't changed (due to an issue with media observers clashing).

Kanvas Loading States

The Kanvas updates improve the loading state handling for multiple export. The loading indicator should appear during export and hide once export is complete -- about the same time the Prepublishing Sheet is shown.

Upload.from.GitHub.for.iOS.MOV

Testing

Note: There is a known issue with text positioning which will be fixed separately.

  • Create a new Story
  • Add multiple media items
  • Tap the Arrow button in the upper right
  • Quickly tap the Publish button without adding a title
  • You should see the media uploading in the Drafts list and it should move to Published when complete

Regression Notes

  1. Potential unintended areas of impact
  • Any post publishing (both stories and blog posts or pages)
  • Any post editing (both stories and blog posts or pages)
  1. What I did to test those areas of impact (or what existing automated tests I relied on)
  • Tested publishing a new story and editing an existing story
  • Tested publishing a new post and editing an existing post (with media)
  • Tested publishing a new page and editing an existing page
  1. What automated tests I added (or what prevented me from doing so)

I'm not sure where to add automated tests for this.

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.

- Calls `mediaCoordinator.addMedia` instead of `uploadMedia` + observers. The `prepareToSave` method will handle all of this instead.
- Removes all media upload references from the story specific APIs
This handles cases where media was added but uploaded before preparing to save. The `addMedia` method will immediately upload the post and references may need to be uploaded.
@bjtitus bjtitus self-assigned this May 7, 2021
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 7, 2021

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 7, 2021

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

@bjtitus bjtitus changed the title Moves stories media upload to post-publish Handle stories media uploading post-publish May 7, 2021
@bjtitus bjtitus requested a review from aerych May 7, 2021 19:05
@bjtitus bjtitus added this to the 17.4 milestone May 7, 2021
Copy link
Member

@aerych aerych left a comment

Choose a reason for hiding this comment

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

Hola @bjtitus 👋
Tested as described. I was able to select multiple media files, publish without a title, and see the uploading media status while viewing the drafts list.
There seems to some lag in several places: between selecting the media and rendering in the story editor and between responding to a tap of the arrow button and showing the spinner, but I don't see how that would be related to these changes.
:shipit: from me.

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.

Stories: Publishing a Story too quickly fails to change status
2 participants