Skip to content

Added onRetryUploadForMediaClicked interface for GB #9158

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.

Sign up for GitHub

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

Jump to bottom
Merged
mzorz merged 34 commits into from
Feb 1, 2019
Merged

Added onRetryUploadForMediaClicked interface for GB #9158

mzorz merged 34 commits into from
Feb 1, 2019

Conversation

@mzorz
Copy link
Contributor

@mzorz mzorz commented Jan 30, 2019 •
edited by marecar3
Loading

Introduces Retry/Retry all/Remove for failed media uploads in Gutenberg

gutenberg-mobile PR wordpress-mobile/gutenberg-mobile#528
gutenberg PR WordPress/gutenberg#13615

To test:

CASE A RETRY (single)

  1. start a new draft
  2. insert image block
  3. choose DEVICE LIBRARY
  4. choose an image (better a big one or throttle your connection to make it last)
  5. once it starts uploading, turn airplane mode ON
  6. observe it errors and displays Failed to insert media.\nPlease tap for options.
  7. turn airplane mode OFF
  8. tap on the image
  9. observe the dialog appears with options REMOVE/RETRY ALL/ RETRY
  10. tap on RETRY
  11. observe the image starts to upload again, and finishes successfully.

testa

CASE B RETRY (multiple)

  1. start a new draft
  2. insert image block
  3. insert another image block
  4. choose DEVICE LIBRARY on one of them
  5. choose an image (better a big one or throttle your connection to make it last - I used 2G as preferred network and turned wifi off to make it worse)
  6. once it starts uploading, tap DEVICE LIBRARY on the other block and do the same
  7. once you see both are slowly uploading, turn airplane mode ON.
  8. observe it errors and displays Failed to insert media.\nPlease tap for options.
  9. turn airplane mode OFF
  10. tap on the image
  11. observe the dialog appears with options REMOVE/RETRY ALL/ RETRY
  12. tap on RETRY ALL
  13. observe both images start to upload again

testb2

CASE C REMOVE

  1. repeat steps 1-9 of case A
  2. tap on REMOVE
  3. observe the block is reset as not containing any image.

testc

CASE D: cancel

  1. start a draft
  2. insert an image block
  3. choose an image from the device
  4. while it's uploading, tap on it and observe the "Stop uploading?" dialog appears.
  5. tap YES to cancel upload, or NO to dismiss the dialog.

testd

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@mzorz mzorz requested a review from marecar3 January 30, 2019 15:12
@mzorz mzorz added this to the 11.8 milestone Jan 30, 2019
marecar3 and others added 14 commits January 30, 2019 16:46
@mzorz
Copy link
Contributor Author

mzorz commented Jan 31, 2019

Just added the proper tests description to this PR and lifted the [Not ready for review] label from it @marecar3 . This should be ready for your review now, along with the gutenberg-mobile and gutenberg PRs. Thanks in advance!
cc @SergioEstevao

@mzorz
Copy link
Contributor Author

mzorz commented Jan 31, 2019

Just found an issue: realized that when an image fails to upload, then we exit the Editor, then enter again, we should show it still as failed.
Going to work on that 👌

@marecar3 marecar3 mentioned this pull request Jan 31, 2019
Merged
marecar3
marecar3 reviewed Jan 31, 2019
View reviewed changes
WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java Outdated
UploadService.cancelFinalNotification(this, mPost);
UploadService.cancelFinalNotificationForMedia(this, mSite);

ArrayList<MediaModel> failedMedia = new ArrayList<>();
Copy link
Contributor

@marecar3 marecar3 Jan 31, 2019

Choose a reason for hiding this comment

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

Maybe we can consider failedMedias or failedMediaList ?

Copy link
Contributor Author

@mzorz mzorz Jan 31, 2019

Choose a reason for hiding this comment

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

👍 addressed in ccfe884

@marecar3
Copy link
Contributor

marecar3 commented Jan 31, 2019

Nice work @mzorz!
Especially for detailed test scenarios!

@mzorz
Copy link
Contributor Author

mzorz commented Jan 31, 2019

realized that when an image fails to upload, then we exit the Editor, then enter again, we should show it still as failed.

This case is covered here now as well in commits 4c2d92d and 3f6143c

mzorz and others added 10 commits January 31, 2019 09:47
@mzorz
removed commented code
b7c626a
@mzorz
updated UploadService to only try to process the Post with Aztec if A…
a7f92c2 
…ztec enabled
@marecar3
Updated gutenberg mobile submodule
3a9c4cb
@marecar3
Updated gutenberg mobile submodule
532e8fa
@mzorz
make sure not to try processing Gutenberg posts with Aztec
69676f2
@mzorz
Merge branch 'gb/retry-failed-upload' of https://github.com/wordpress…
668e4d8 
…-mobile/WordPress-Android into gb/retry-failed-upload
@marecar3
Updated gutenberg mobile submodule
7a1d98c
@mzorz
updated gb-mobile hash
f086c5d
@mzorz
make sure to check content of post to see if processable with Aztec o…
159b412 
…r not, as the AppPrefs may have changed but the post has been edited before
@mzorz
Merge branch 'gb/retry-failed-upload' of https://github.com/wordpress…
c588a20 
…-mobile/WordPress-Android into gb/retry-failed-upload
marecar3
marecar3 approved these changes Jan 31, 2019
View reviewed changes
Copy link
Contributor

@marecar3 marecar3 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mzorz mzorz merged commit 89e2c68 into develop Feb 1, 2019
@mzorz mzorz deleted the gb/retry-failed-upload branch February 1, 2019 03:38
@mzorz mzorz mentioned this pull request Feb 1, 2019
Merged
1 task
@marecar3 marecar3 mentioned this pull request Feb 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@marecar3 marecar3 marecar3 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

11.8

Development

Successfully merging this pull request may close these issues.

3 participants

@mzorz @marecar3