Auto-upload/save all posts/pages with local changes #10319

malinajirka · 2019-08-01T10:18:16Z

This PR changes more than I initially planned. I think it'd be worth considering doing a pair/triple-review with me so I can explain the changes. Please let me know if that's what you prefer to do;).

Main changes

PostModel.changesConfirmedContentHashcode is a field which we use to mark that the current set of changes has been explicitly confirmed by the user. Whenever the user explicitly confirms the chagnes - clicks on Publish/Update/Schedule/Submit/Save/Sync/... button, we calculate a hashcode of the content of the post and save its value into the changesConfirmedContentHashcode field. When we are about to upload the post we calculate the hashcode again and if it matches the value stored in changesConfirmedContentHashcode we are sure the post hasn't been changed since the user confirmed the changes and we can safely upload it.
PostUploadHandler is the place where we decide whether the post will be uploaded, uploaded as draft or remote auto saved.

Uploaded are posts with changes which have been explicitly confirmed by the user -> the user clicked on Publish/Update/Schedule/Submit/Save/Sync... buttons.
Uploaded as drafts are local drafts (PostModel.isLocalDraft == true) with changes which haven't been confirmed by the user. We can't remote auto save them as they don't exist in the remote.
Remote-auto-saved are all other changes. This feature isn't supported on self-hosted sites, therefore we don't sync the changes with the remote on self-hosted sites unless the user explicitly confirms them.

UploadStarter now automatically syncs all changes with the remote - PostUploadHandler decides whether the changes should be uploaded, uploaded as draft or remote auto saved. This PR also adds several safe-guards so we don't automatically upload something we didn't want to upload.

To test:
This PR touches

manual publishing/saving/updating/...
automatic publishing/saving/updating/...
remote previews
both posts and pages

We decided we'll test these changes when we implement #10207

Update release notes:

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

Notes:

This PR should be reviewed by more than one person since the changes are touching auto-uploading, remote-auto-saving and even uploading initiated by the user.
I haven't tested Pages at all yet
This PR targets master-auto-upload which was created from master-remote-preview.

…-changes-confirmed-at # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java # WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt # build.gradle

…pdate-changes-confirmed-at # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java # WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt # build.gradle

…-changes-confirmed-at # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/PageAndPostListPreviewStateHelper.kt

…firmed-at # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt # WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java # build.gradle

WordPress/src/main/java/org/wordpress/android/ui/posts/PreviewStateHelper.kt

…firmed-at

peril-wordpress-mobile · 2019-08-01T12:42:31Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

shiki

I looked at this today. I have some comments and this question:

Prevent auto-uploading posts which are in conflict with remote

Why?

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt

shiki · 2019-08-02T16:32:10Z

This PR targets master-auto-upload which was created from master-remote-preview.

Which one should be merged first?

maxme

We have to make sure things work well for self hosted sites before merging this.

PS: The "local draft" and "local changes" status on auto-saved draft/post/page feels a bit weird to me (I'm not expecting changes being available on the server when I see this label), it's not a big deal though.

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java

WordPress/src/main/java/org/wordpress/android/ui/posts/PostPreviewActivity.java

…mote-auto-save-when-leaving-editor Invoke remote-auto-save when exiting the editor

shiki · 2019-08-08T12:53:02Z

I wouldn't worry about it now. We can test the migration before we merge master-auto-upload.

Sounds good!

…hout-changes-remote-auto-saved Don't save post when it wasn't edited during the current session

shiki

This is going well. Almost there! I tested this with a self-hosted site today and found these.

Reverting a Published Post into a Draft
Preview error in the editor
Extra labels for canceled drafts

Reverting a Published Post into a Draft

There does not seem to be a way to revert a published post into a draft. Using develop (870511b), I can do this and it works:

Edit the published post.
Navigate to Post Settings. Change the Status to Draft.
Exit the editor by pressing the back button.
The post will be automatically uploaded and reverted into a draft.

With this change, I get a “Draft is uploading” but it always ends up in the Post List saying these in sequence:

Couldn't perform operation
Local changes

Here is a video showing this.

Preview error in the editor

I'm not sure if this is because of this PR or because of the preview changes.

Go offline.
Create a draft. Do not leave the editor.
Turn on the internet connection.

An upload will start in the background and an error related to previewing will be shown.

Extra labels for canceled drafts

This could probably use a design input. When canceling Private or Pending review local drafts, their status is still appended after the “Local draft” label.

Go offline.
Create a draft and set it to Private or Pending review.
Exit the editor.
In the Post List, press the Cancel button.

Please let me know if some of these should be out of scope.

shiki

Failed media uploads and publishing

I just wanted to confirm if this is expected. I believe you mentioned this a bit during the call. Currently, if a post has a failed media upload, it will always be uploaded as a draft even if the user confirmed publishing it. This video shows what I mean. Here are the steps:

Go offline.
Create a post and add an image. The media should fail to upload.
Press the Publish button. The error shown will not be the usual “Post will be published...” error.
Go online. The image will be uploaded and the post will be uploaded as a draft, not automatically published.

RFC on Failed media uploads and publishing

malinajirka · 2019-08-09T08:04:23Z

Thank you @shiki!

Reverting a Published Post into a Draft

Hmm, I don't see the "Couldn't perform operation" error. Just the "Local changes" label and it seems like an expected behavior. The user hasn't confirmed the changes, they need to manually click on "SAVE" (in the overflow menu).
I guess we could show "Save" button instead of "Publish" button on the Post List in these cases. Or another option would be to introduce a completely new label.

Preview error in the editor
An upload will start in the background

I'm not able to reproduce this one. I checked the places where we show this specific message and it shouldn't be shown unless the app thinks it's in "preparing" for preview state. @maxme Do you have any tips what might be causing this issue?

This could probably use a design input. When canceling Private or Pending review local drafts, their status is still appended after the “Local draft” label.

I'm not sure I follow what is the issue here. Are you suggesting we should hide the "Private/Pending review" labels if the post is still a local draft -> since when the connection returns we'll upload it as draft and clear the "Private/Pending review" status? If this is what you meant I'm not sure it's a good idea as if the user clicks on Publish again (after canceling the auto-upload) the post will be published as Private/Pending Review - so the label is imho still valid. Wdyt?

Btw I found another bug

Create a private draft in offline and publish it
Cancel the auto-upload
Click on publish on the post list
Enable internet connection
Notice the post gets published not published as private post
Private draft gets published as public post #10365

Currently, if a post has a failed media upload, it will always be uploaded as a draft even if the user confirmed publishing it.

I've fixed this issue in fe9e162. It's in the cancel button PR.

maxme · 2019-08-09T08:15:21Z

Preview error in the editor

I'm not sure if this is because of this PR or because of the preview changes.
1. Go offline.

2. Create a draft. Do not leave the editor.

3. Turn on the internet connection.
An upload will start in the background and an error related to previewing will be shown.

That's weird, I just tried this and the draft was uploaded while I was in the editor (I think you got an error, sometimes we get the "network status" event, but the network is not yet ready to do any request).

The message is confusing, I wonder if we're in a wrong state in EditPostActivity. I'm not sure what triggers this auto-upload, I'll check that first.

Note: I just noticed we should update the method name handleRemoteAutoSave to handleRemoteAutoSaveForPreview

WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java

Line 4162 in 0910bb0

private void handleRemoteAutoSave(boolean isError, PostModel post) {

maxme · 2019-08-09T09:27:50Z

The message is confusing, I wonder if we're in a wrong state in EditPostActivity. I'm not sure what triggers this auto-upload, I'll check that first.

Auto-upload is triggered by the UploadStarter, after the post is pushed, I get the mPostLoadingState == NONE and thus get the correct "draft uploaded" snackbar.

I tried to track how mPostLoadingState could be different from NONE in a non-preview operation and couldn't find any clue.

That makes me wonder if the snackbar you see comes from this line:

WordPress-Android/WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java

Line 4136 in 0910bb0

handleRemoteAutoSave(event.isError(), mPost);

but that would mean you tested on an old version of that PR.

@shiki can you try to repro this one and check the state of mPostLoadingState in EditPostAcitivty.handleRemoteAutoSave?

shiki · 2019-08-09T12:32:26Z

@malinajirka In response to #10319 (comment):

Reverting a Published Post into a Draft
I guess we could show "Save" button instead of "Publish" button on the Post List in these cases. Or another option would be to introduce a completely new label.

I didn't realize there was a Save button there. 🤦‍♂ Seeing that I myself was confused, I think we should change this. But not in this PR. Would you mind creating a separate issue for it? We can ask Chris for his input. The issue is probably a nice-to-have in the context of the Offline Posting project.

Create an issue for ☝️

Extra labels for canceled drafts
I'm not sure it's a good idea as if the user clicks on Publish again (after canceling the auto-upload) the post will be published as Private/Pending Review - so the label is imho still valid. Wdyt?

Yeah, you're right. Ignore me. 😁

shiki · 2019-08-09T12:58:05Z

Failed media uploads and publishing

@malinajirka This is fixed. Thank you. My remaining concern is that the error message in the Post List does not mention that the post will be published automatically. I'm okay with this being done in a separate PR so we can collect all the PRs in the master branch.

Fix or not fix this the error message? Or create an issue?

shiki · 2019-08-09T13:23:34Z

@maxme In response to #10319 (comment):

@shiki can you try to repro this one and check the state of mPostLoadingState in EditPostAcitivty.handleRemoteAutoSave?

Uhm, I can't reproduce it anymore. 😐 I guess we can close this out for now. Thank you for checking it!

…firmed-at

maxme · 2019-08-09T14:28:25Z

to the master branch.

shiki · 2019-08-09T22:00:05Z

Created issues for the remaining tasks:

malinajirka added 13 commits July 17, 2019 15:33

Initial commit introducing auto-upload/auto-save of modified posts

024436d

Fix auto-publish local draft logic

49cfe46

Merge branch 'feature/master-remote-previews' into issue/10174-update…

fe3a3d4

…-changes-confirmed-at # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/EditPostActivity.java # WordPress/src/main/java/org/wordpress/android/ui/posts/PostListMainViewModel.kt # build.gradle

Introduce auto-upload and remote-auto-save

d5f82a0

Fix UploadStarter concurrency issue

082556b

Fix UploadStarter tests

192aac4

Merge branch 'feature/master-remote-previews' into issue/10174-update…

c091e87

…-changes-confirmed-at # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/posts/PageAndPostListPreviewStateHelper.kt

Fix errors after merge

a1271b2

Minor refactoring in remote-autosave/upload

60215c5

Prevent auto-uploading posts which are in conflict with remote

3ea9b4d

Merge branch 'master-auto-upload' into issue/10174-update-changes-con…

aeded51

…firmed-at # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt # WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java # build.gradle

Update release-notes

acafc9e

malinajirka added [Type] Enhancement Posting/Editing labels Aug 1, 2019

malinajirka added this to the 13.1 milestone Aug 1, 2019

malinajirka requested review from maxme and shiki August 1, 2019 10:18

malinajirka commented Aug 1, 2019

View reviewed changes

WordPress/src/main/java/org/wordpress/android/ui/posts/PreviewStateHelper.kt Show resolved Hide resolved

malinajirka added 3 commits August 1, 2019 12:33

Remove unused code

08d1a6d

Merge branch 'master-auto-upload' into issue/10174-update-changes-con…

0422628

…firmed-at

Fix line length issue

fdc205d

malinajirka changed the title ~~Issue/10174 update changes confirmed at~~ Auto-upload/save all posts/pages with local changes Aug 1, 2019

Don't upload drafts but remote-auto-save instead when previewing

a916902

malinajirka added 2 commits August 1, 2019 16:08

Fix RemotePreview tests

a46ae1e

Remove unnecessary stubs

3ebae57

shiki reviewed Aug 2, 2019

View reviewed changes

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt Outdated Show resolved Hide resolved

WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadStarter.kt Show resolved Hide resolved

maxme suggested changes Aug 5, 2019

View reviewed changes

Merge pull request #10353 from wordpress-mobile/issue/10349-invoke-re…

3c254e6

…mote-auto-save-when-leaving-editor Invoke remote-auto-save when exiting the editor

malinajirka and others added 2 commits August 8, 2019 15:02

Fix uploading local draft on exiting the editor on self-hosted sites

0af1803

Merge pull request #10351 from wordpress-mobile/issue/10350-draft-wit…

0910bb0

…hout-changes-remote-auto-saved Don't save post when it wasn't edited during the current session

maxme approved these changes Aug 8, 2019

View reviewed changes

shiki reviewed Aug 8, 2019

View reviewed changes

shiki approved these changes Aug 9, 2019

View reviewed changes

maxme added 3 commits August 9, 2019 15:50

Merge branch 'master-auto-upload' into issue/10174-update-changes-con…

78e0a65

…firmed-at

Update FluxC hash (from updated branch in fluxc)

c6b025b

Merge branch 'master-auto-upload' into issue/10174-update-changes-con…

5ce1ff7

…firmed-at

maxme merged commit d03b198 into master-auto-upload Aug 9, 2019

This was referenced Aug 9, 2019

Possible UX issue when reverting a published post to a draft #10369

Closed

Unclear message if a media was not uploaded #10370

Closed

maxme deleted the issue/10174-update-changes-confirmed-at branch August 12, 2019 08:08

This was referenced Aug 15, 2019

Post changes are automatically published when image upload finishes #10090

Closed

Automatically upload posts with local changes #10174

Closed

malinajirka modified the milestones: 13.2, 13.3 Aug 26, 2019

malinajirka mentioned this pull request Aug 29, 2019

[Epic] Offline Publishing #10295

Closed

designsimply mentioned this pull request Jan 5, 2021

Post List: Sync posts automatically #8807

Closed

ParaskP7 mentioned this pull request May 19, 2023

Refactor concurrent uploading of local changes #18449

Closed

4 tasks

Auto-upload/save all posts/pages with local changes #10319

Auto-upload/save all posts/pages with local changes #10319

Uh oh!

Conversation

malinajirka commented Aug 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Aug 1, 2019

Uh oh!

shiki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shiki commented Aug 2, 2019

Uh oh!

maxme left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shiki commented Aug 8, 2019

Uh oh!

shiki left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Reverting a Published Post into a Draft

Preview error in the editor

Extra labels for canceled drafts

Uh oh!

shiki left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Failed media uploads and publishing

Uh oh!

malinajirka commented Aug 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxme commented Aug 9, 2019

Preview error in the editor

Uh oh!

maxme commented Aug 9, 2019

Uh oh!

shiki commented Aug 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shiki commented Aug 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shiki commented Aug 9, 2019

Uh oh!

maxme commented Aug 9, 2019

Uh oh!

shiki commented Aug 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

malinajirka commented Aug 1, 2019 •

edited

Loading

shiki left a comment •

edited

Loading

shiki left a comment •

edited

Loading

malinajirka commented Aug 9, 2019 •

edited

Loading

shiki commented Aug 9, 2019 •

edited

Loading

shiki commented Aug 9, 2019 •

edited

Loading