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

LocalDraftUploadStarter: Upload local draft pages #9902

Merged
merged 11 commits into from May 24, 2019

Conversation

@shiki
Copy link
Member

commented May 21, 2019

The first part of #9851 and has a corresponding FluxC PR at wordpress-mobile/WordPress-FluxC-Android#1271.

This adds automatically uploading local draft pages in LocalDraftUploadStarter. The same event triggers are used.

Event Before After
App started ✖️ ✔️
App placed in foreground ✖️ ✔️
Internet connection is restored while Page List is open ✖️ ✔️
Internet connection is restored while in other Activities ✖️ ✔️
The Page List is opened ✔️ ✔️
Swipe to refresh on Post List ✔️ ✔️

Outstanding Issues

There is an issue with Pages not automatically updating the list if an upload happens in the background. I'm currently working on fixing this as part of #9851. But I believe it's okay to get this PR merged before that is fixed.

The issue can be reproduced by these steps:

  1. Use this PR's branch
  2. Go offline
  3. Open Pages
  4. Create a draft
  5. Go online
  6. Wait a few seconds until you believe the upload should be finished. Notice the draft is still marked as Local Draft.
  7. Close Pages
  8. Go offline
  9. Reopen Pages. You should see that the draft is now shown as uploaded.

Also, snackbars do not seem to be shown when uploads finish. I'll create a separate issue for that.

Testing

  1. Go offline
  2. Create a local draft
  3. Trigger one of the events above. Validate that the local drafts were uploaded.

Release Notes

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

shiki added some commits May 13, 2019

LocalDraftUploadStarter: Fix ambiguous context
The IDE was complaining about an ambiguous coroutineContext on the `async` calls. It’s most probably because in `upload`, you can’t tell which context is being used, the `LocalDraftUploadStarter` or something else that’s calling `upload`.

@shiki shiki requested review from malinajirka and maxme May 22, 2019

@shiki shiki added this to the 12.6 milestone May 22, 2019

@shiki shiki marked this pull request as ready for review May 22, 2019

@shiki shiki changed the title Issue/9851 upload local draft pages LocalDraftUploadStarter: Upload local draft pages May 22, 2019

Fix incorrect merge in build.gradle
┻━┻ ︵ヽ(`Д´)ノ︵ ┻━┻

@malinajirka malinajirka self-assigned this May 24, 2019

@malinajirka
Copy link
Contributor

left a comment

Thanks @shiki! I've reviewed the code and tested the app and everything works as described. Good job!

Feel free to merge it when the corresponding FluxC's PR is merged, hash in WPAndroid is updated and the conflicts are resolved.

@shiki shiki merged commit f7dea65 into develop May 24, 2019

4 checks passed

Peril All green. Yay.
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: strings-check Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details

@shiki shiki added this to Done (PRs) in Offline Support: Posting [Android] via automation May 24, 2019

@shiki shiki deleted the issue/9851-upload-local-draft-pages branch May 24, 2019

@shiki shiki referenced this pull request May 24, 2019
2 of 2 tasks complete
@maxme maxme referenced this pull request May 24, 2019
5 of 7 tasks complete
@shiki shiki referenced this pull request Jul 29, 2019
0 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.