Post List / Editor: Race condition saving while refreshing the list can loose changes

[aerych]

Previously #404

There are two scenarios at play as far as I can see:

• Scenario 1:

• A user edits a post for longer than the post list refresh duration.

• The post is saved, kicking off the call to push changes to the server and dismiss the editor.

• The post list becomes the visible view controller and kicks off a call to sync posts.

• The call to sync posts completes before the call to update posts completes.
  The result is the changes are no longer saved in the app.

• Scenario 2:

• A user edits an existing post in wp-admin.

• The user returns to the app and opens the post list kicking off a call to sync posts.

• Before the sync completes the user taps to edit the post.
  Since the sync has not completed, the app does not have any of the changes.

• The sync completes while the user is viewing the editor and the post in core data is updated.

• Any change the user makes in the editor is going to overwrite what was just synced.
  The result is the changes are over written.

Reported in the forums: https://ios.forums.wordpress.org/topic/problems-saving-drafts

[jleandroperez]

This sounds exactly like the concurrency issue we've got in the Account Settings interface.

cc'ing @koke. We really need a generic solution here

[koke]

There's wordpress-mobile/WordPress-Editor-iOS#58, which was #404 before, and https://ios.trac.wordpress.org/ticket/1498 before that. Definitely not a new issue 😉

[koke]

It's a slightly different variant of the settings problem. In Account Settings you could block any refresh until any pending changes complete or fail, since when they fail we just revert to the previous version. In the case of posts, we keep the failed updates as local changes because you don't want to discard that content. A failed upload shouldn't prevent a posts refresh.

[diegoreymendez]

We should discuss the desired behavior in Scenario 2.

Settings is a bit different because each configuration can be considered atomic (or at least most of them). You can potentially update one without affecting the others... meaning merging remote and local changes is a valid option. Posts are a big entity which can't be merged partially. Either the remote one or the local one must prevail when synching.

I don't think we should try to update the post once the WPiOS editor has been opened because that could potentially overwrite local edits. At most, we should let the user know there are remote changes, and ask whether they want to keep the remote version or the local version (like many text editors do).

Also, scenario 1 and 2 should be different tickets imho.

[diegoreymendez]

Regarding scenario 1: I don't think that's possible any more (due to recent changes). Posts with changes are kept as revisions now, and any synchronization call is performed on the original post. The revision is only merged back to the original post once the upload completes - meaning that you'll only get to see the revision until the changes are posted.

[elibud]

@diegoreymendez can you all take a look at this one as part of the offline improvements?

[designsimply]

We are discussing the same issue for Android at wordpress-mobile/WordPress-Android#10292.

[designsimply]

Adding to Groundskeeping but noting this has also been proposed as project work because of the size.

I do feel this can fit into Conflict Resolution, if we want to push that as a project.
^{— wordpress-mobile/WordPress-Android#10292 (comment)}

I think it definitely fits within a proposed conflict resolution project.
^{— wordpress-mobile/WordPress-Android#10292 (comment)}

[mchowning]

Confirmed both scenarios are still reproducible.

[kean]

Any change the user makes in the editor is going to overwrite what was just synced.

Fixed. In the unlikely scenario this happens and there is a true data conflict, the app will kick off the "conflict resolution" dialog.