Wordpress-android: Automatically upload posts with local changes

[shiki]

This is a subtask of #9846.

Currently, we only upload local drafts. If a draft has already been previously uploaded, we do not automatically upload it anymore. The same should be done for published posts.

[malinajirka]

I've increased the estimation from 3 to 8 as we realized it won't be as simple as we initially anticipated. We need to update the UploadService so only explicitly confirmed changes are published (visible to visitors of the site) and the other changes are remotely-auto-saved.

[malinajirka]

~Current plan for tackling this issue is following~
~- [ ] PR 1: Add publishedConfirmedAt field into FluxC~
~- [ ] PR 2: Start updating publishedConfirmedAt field in WPAndroid (we won't use it yet)~
~- [ ] PR 3: Start auto-uploading all posts with status Draft. We currently auto-upload only local drafts not posts with local changes. (If the user sets a published post to draft we'll unpublish it as soon as the device is online)~
~- [ ] PR 4: Start auto-uploading (published/scheduled/private/pending) post when publishedConfirmedAt == lastModified (we are sure we can upload them as the changes have been explicitly confirmed by the user)~
~- [ ] PR 5: Start auto-saving (published/scheduled/private/pending) posts when publishedConfirmedAt != lastModified. (We'll need to target FluxC's master-remote-auto-save)~

Update: See https://github.com/wordpress-mobile/WordPress-Android/issues/10174#issuecomment-511735103 for updated plan.

More info paCBwp-c7-p2

cc @maxme @shiki

[shiki]

@malinajirka Thank you for sharing your plans! It's looking good.

PR 3

For PR 3, where you said:

If the user sets a published post to draft we'll unpublish it as soon as the device is online

I'm not quite sure which issue this should be discussed on. I tried to look for one but I just got more confused. 😄 Anyways, when a user changes a previously PUBLISHED post to DRAFT, we should ask for explicit confirmation from the user. I believe that removing a post for the readers to see can still be damaging to a user's brand. The same thing with TRASHED status.

I would love to hear what you think too, @osullivanchris.

publishedConfirmedAt

With my arguments above, since we will include DRAFT and TRASHED in the confirmation, I think we should rename publishedConfirmedAt. Maybe userConfirmedIntentAt. Or something more grammatically correct. Or a clearer name. 😄

[malinajirka]

Anyways, when a user changes a previously PUBLISHED post to DRAFT, we should ask for explicit confirmation from the user.

I was on the edge when I was writing the proposal. I agree it can be damaging in some cases. The current proposal doesn't require introduction of remoteStatus - We currently can't tell whether a post with "Draft" status is "Published" in remote or not.

I was planning to filter out older posts in the UploadStarter, but it wouldn't solve the issue. If the user changes the post status in the Post settings, it'd still be automatically unpublished. We'll need to introduce remoteStatus I guess.

This leads me to a sub-issue. Any suggestions what to set to "remoteStatus" when we introduce the field? It'll be a bit tricky as we can't simply copy the current status field and we can't enforce fetching all the posts from the remote. ~Introducing a nullable field which shouldn't be nullable doesn't sound like the best idea either (but might be our only option).~ We need to support local drafts -> it needs to be nullable. Wdyt?

[malinajirka]

The plan got a bit more complicated, but it looks worse than it actually is.

Updated plan for tackling this issue is following

• [ ] PR 1: Add changesConfirmedAt and remoteStatus fields into FluxC. (Copy status into remoteStatus for all posts with hasLocalChanges == false. Set remoteStatus to Null for the rest.)

• [ ] PR 2: Start updating changesConfirmedAt field in WPAndroid (we won't use it yet)

• [ ] PR 2.5: Filter out posts with (remoteStatus || local status) == (Trashed || Unknown) from the UploadStarter. (we'll add support for offline trashing/restoring in #10205)

• [ ] PR 3: Start auto-uploading all posts with status == Draft && remoteStatus == Draft. We currently auto-upload only local drafts (remoteStatus == null) not drafts with local changes.

• [ ] PR 4: Start auto-uploading all posts which have changesConfirmedAt == lastModified (we are sure we can upload them as the changes have been explicitly confirmed by the user)

• [ ] PR 5: Start auto-saving all posts which have changesConfirmedAt != lastModified && remoteStatus != null. (We'll need to target FluxC's master-remote-auto-save. !!!Make sure it doesn't crash on self-hosted sites!!!)

• [ ] PR 6: Posts which have changesConfirmedAt != lastModified && remoteStatus == null needs to be auto-uploaded as drafts.

More info paCBwp-c7-p2

Note: The name changesConfirmedAt is still open for suggestions. I find it a bit more reusable and clear than userConfirmedIntentAt - but if it's just me, I'll be happy to update it.

[osullivanchris]

The idea of changing publishedConfirmedAt to something more general like changesConfirmedAt makes sense to me.

Anyways, when a user changes a previously PUBLISHED post to DRAFT, we should ask for explicit confirmation from the user. I believe that removing a post for the readers to see can still be damaging to a user's brand. The same thing with TRASHED status.

When we say we want confirmation. Does this mean literally in all cases where we want a confirmation, we plan on showing a dialogue (did you mean to do this?). OR do we only feel we need this when the UI is not clear that the user is going to make an important change? Because if that's the case it feels like some of the UI must not be doing the job right, if we always need to 'patch' it with a confirmation dialogue.

In the case of changing the status of a post in post settings, it seems for all big changes made from that feature the flow is not very clear. Its not obvious at which point the user is committing a change, and some of the changes are large.

I still think changesConfirmedAt is a good concept technically. I'm just wondering from a user point of view how often it means an explicit dialogue, and how often we are just checking "did the user at some point actually hit a button that said publish?"

[malinajirka]

I still think changesConfirmedAt is a good concept technically. I'm just wondering from a user point of view how often it means an explicit dialogue, and how often we are just checking "did the user at some point actually hit a button that said publish?"

My plan is to set changesConfirmedAt when the user explicitly hits Publish/Schedule/Submit/Save -> that's a confirmation for me. When the user simply changes the status in the post settings but they leave the editor without hitting "Publish/Schedule/Submit/Save" we won't publish the change.

Perhaps a better answer would be that we won't change the current behavior. We'll simply retry the upload when we re-gain connectivity.

• When you perform a certain action and the changes are published in online. The changes will pending upload if you perform the same action in offline.

I don't plan to add any new dialogs in this task.

[shiki]

Because if that's the case it feels like some of the UI must not be doing the job right, if we always need to 'patch' it with a confirmation dialogue.

I think you're on point on this, @osullivanchris. I remember there was a discussion about changing that UI somewhere. Maybe someday that should be fixed. I like @malinajirka's idea in https://github.com/wordpress-mobile/WordPress-Android/issues/10174#issuecomment-511814945 describing how changesConfirmedAt should be updated. We can go with that for now, it does not seem to require a lot of changes.

I don't plan to add any new dialogs in this task.

👌

[malinajirka]

I think the whole table and plan can be simplified to

Wdyt?

[shiki]

@malinajirka That sounds right for Published, Private, Scheduled, and Pending. For drafts, I think we don't need to check for changesConfirmedAt?

[malinajirka]

That sounds right for Published, Private, Scheduled, and Pending. For drafts, I think we don't need to check for changesConfirmedAt?

Sorry my bad, I read different discussions scattered all around the place and I forgot I didn't provide the context:D. The idea of this last proposal is to introduce just changesConfirmed field and do not introduce remoteStatus. Based on this discussion , we can never be sure what is the current status of the post in remote. Therefore unless we have explicit confirmation from the user to upload the changes we always only remote-auto-save them (even drafts, as we are not sure whether they are still drafts in remote).

[shiki]

@malinajirka Ah yes, on that context, it makes sense that we don't need remoteStatus.

[malinajirka]

Fixed in #10319