Merge pull request #20552 from wordpress-mobile/issue/20531-post-conf…

…lict

Sync issues: Post conflict handling

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

@@ -73,7 +73,8 @@ class PostActionHandler(
     private val showSnackbar: (SnackbarMessageHolder) -> Unit,
     private val showToast: (ToastMessageHolder) -> Unit,
     private val triggerPreviewStateUpdate: (PostListRemotePreviewState, PostInfoType) -> Unit,
-    private val copyPost: (SiteModel, PostModel, Boolean) -> Unit
+    private val copyPost: (SiteModel, PostModel, Boolean) -> Unit,
+    private val syncPublishingFeatureUtils: SyncPublishingFeatureUtils
 ) {
     private val criticalPostActionTracker = CriticalPostActionTracker(onStateChanged = {
         invalidateList.invoke()
@@ -207,7 +208,9 @@ class PostActionHandler(
             return
         }
         post.setStatus(DRAFT.toString())
-        dispatcher.dispatch(PostActionBuilder.newPushPostAction(RemotePostPayload(post, site)))
+        dispatcher.dispatch(PostActionBuilder.newPushPostAction(
+            syncPublishingFeatureUtils.getRemotePostPayloadForPush(RemotePostPayload(post, site))
+        ))
 
         val localPostId = LocalId(post.id)
         criticalPostActionTracker.add(localPostId, MOVING_POST_TO_DRAFT)

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

@@ -5,7 +5,10 @@ import org.wordpress.android.fluxc.Dispatcher
 import org.wordpress.android.fluxc.generated.PostActionBuilder
 import org.wordpress.android.fluxc.model.PostModel
 import org.wordpress.android.fluxc.model.SiteModel
+import org.wordpress.android.fluxc.store.PostStore
+import org.wordpress.android.fluxc.store.PostStore.PostErrorType
 import org.wordpress.android.fluxc.store.PostStore.RemotePostPayload
+import org.wordpress.android.fluxc.store.UploadStore
 import org.wordpress.android.ui.pages.SnackbarMessageHolder
 import org.wordpress.android.ui.utils.UiString.UiStringRes
 import org.wordpress.android.util.ToastUtils.Duration
@@ -23,10 +26,11 @@ class PostConflictResolver(
     private val invalidateList: () -> Unit,
     private val checkNetworkConnection: () -> Boolean,
     private val showSnackbar: (SnackbarMessageHolder) -> Unit,
-    private val showToast: (ToastMessageHolder) -> Unit
+    private val showToast: (ToastMessageHolder) -> Unit,
+    private val uploadStore: UploadStore,
+    private val postStore: PostStore
 ) {
-    private var originalPostCopyForConflictUndo: PostModel? = null
-    private var localPostIdForFetchingRemoteVersionOfConflictedPost: Int? = null
+    private var originalPostId: Int? = null
 
     fun updateConflictedPostWithRemoteVersion(localPostId: Int) {
         // We need network connection to load a remote post
@@ -36,7 +40,11 @@ class PostConflictResolver(
 
         val post = getPostByLocalPostId.invoke(localPostId)
         if (post != null) {
-            originalPostCopyForConflictUndo = post.clone()
+            originalPostId = post.id
+            post.error = null
+            post.setIsLocallyChanged(false)
+            post.setAutoSaveExcerpt(null)
+            post.setAutoSaveRevisionId(0)
             dispatcher.dispatch(PostActionBuilder.newFetchPostAction(RemotePostPayload(post, site)))
             showToast.invoke(ToastMessageHolder(R.string.toast_conflict_updating_post, Duration.SHORT))
         }
@@ -48,75 +56,47 @@ class PostConflictResolver(
             return
         }
 
-        // Keep a reference to which post is being updated with the local version so we can avoid showing the conflicted
-        // label during the undo snackBar.
-        localPostIdForFetchingRemoteVersionOfConflictedPost = localPostId
         invalidateList.invoke()
 
         val post = getPostByLocalPostId.invoke(localPostId) ?: return
+        post.error = null
+        uploadStore.clearUploadErrorForPost(post)
 
-        // and now show a snackBar, acting as if the Post was pushed, but effectively push it after the snackbar is gone
-        var isUndoed = false
-        val undoAction = {
-            isUndoed = true
-
-            // Remove the reference for the post being updated and re-show the conflicted label on undo
-            localPostIdForFetchingRemoteVersionOfConflictedPost = null
-            invalidateList.invoke()
-        }
-
-        val onDismissAction = { _: Int ->
-            if (!isUndoed) {
-                localPostIdForFetchingRemoteVersionOfConflictedPost = null
-                PostUtils.trackSavePostAnalytics(post, site)
-                dispatcher.dispatch(PostActionBuilder.newPushPostAction(RemotePostPayload(post, site)))
-            }
-        }
         val snackBarHolder = SnackbarMessageHolder(
-            UiStringRes(R.string.snackbar_conflict_web_version_discarded),
-            UiStringRes(R.string.snackbar_conflict_undo),
-            undoAction,
-            onDismissAction
+            UiStringRes(R.string.snackbar_conflict_web_version_discarded)
         )
         showSnackbar.invoke(snackBarHolder)
-    }
 
-    fun doesPostHaveUnhandledConflict(post: PostModel): Boolean {
-        // If we are fetching the remote version of a conflicted post, it means it's already being handled
-        val isFetchingConflictedPost = localPostIdForFetchingRemoteVersionOfConflictedPost != null &&
-                localPostIdForFetchingRemoteVersionOfConflictedPost == post.id
-        return !isFetchingConflictedPost && PostUtils.isPostInConflictWithRemote(post)
+        PostUtils.trackSavePostAnalytics(post, site)
+        val remotePostPayload = RemotePostPayload(post, site)
+        remotePostPayload.shouldSkipConflictResolutionCheck = true
+        dispatcher.dispatch(PostActionBuilder.newPushPostAction(remotePostPayload))
     }
 
+    fun doesPostHaveUnhandledConflict(post: PostModel): Boolean =
+        uploadStore.getUploadErrorForPost(post)?.postError?.type == PostErrorType.OLD_REVISION ||
+                PostUtils.isPostInConflictWithRemote(post)
+
     fun hasUnhandledAutoSave(post: PostModel): Boolean {
         return PostUtils.hasAutoSave(post)
     }
 
     fun onPostSuccessfullyUpdated() {
-        originalPostCopyForConflictUndo?.id?.let {
-            val updatedPost = getPostByLocalPostId.invoke(it)
+        originalPostId?.let { id ->
+            val updatedPost = getPostByLocalPostId.invoke(id)
+            originalPostId = null
             // Conflicted post has been successfully updated with its remote version
+            uploadStore.clearUploadErrorForPost(updatedPost)
+            postStore.removeLocalRevision(updatedPost)
             if (!PostUtils.isPostInConflictWithRemote(updatedPost)) {
                 conflictedPostUpdatedWithRemoteVersion()
             }
         }
     }
 
     private fun conflictedPostUpdatedWithRemoteVersion() {
-        val undoAction = {
-            // here replace the post with whatever we had before, again
-            if (originalPostCopyForConflictUndo != null) {
-                dispatcher.dispatch(PostActionBuilder.newUpdatePostAction(originalPostCopyForConflictUndo))
-            }
-        }
-        val onDismissAction = { _: Int ->
-            originalPostCopyForConflictUndo = null
-        }
         val snackBarHolder = SnackbarMessageHolder(
-            UiStringRes(R.string.snackbar_conflict_local_version_discarded),
-            UiStringRes(R.string.snackbar_conflict_undo),
-            undoAction,
-            onDismissAction
+            UiStringRes(R.string.snackbar_conflict_local_version_discarded)
         )
         showSnackbar.invoke(snackBarHolder)
     }

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

@@ -185,16 +185,30 @@ class PostListEventListener(
     fun onPostUploaded(event: OnPostUploaded) {
         if (event.post != null && event.post.localSiteId == site.id) {
             if (!isRemotePreviewingFromPostsList.invoke() && !isRemotePreviewingFromEditor(event.post)) {
-                triggerPostUploadAction.invoke(
-                    PostUploadedSnackbar(
-                        dispatcher,
-                        site,
-                        event.post,
-                        event.isError,
-                        event.isFirstTimePublish,
-                        null
+                if (event.isError && event.error.type == PostStore.PostErrorType.OLD_REVISION) {
+                    triggerPostUploadAction.invoke(
+                        PostUploadedSnackbar(
+                            dispatcher,
+                            site,
+                            event.post,
+                            event.isError,
+                            event.isFirstTimePublish,
+                            event.error.message,
+                            showRetry = false
+                        )
                     )
-                )
+                } else {
+                    triggerPostUploadAction.invoke(
+                        PostUploadedSnackbar(
+                            dispatcher,
+                            site,
+                            event.post,
+                            event.isError,
+                            event.isFirstTimePublish,
+                            null
+                        )
+                    )
+                }
             }
 
             uploadStatusChanged(event.post.id)

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

@@ -82,7 +82,8 @@ class PostListMainViewModel @Inject constructor(
     private val savePostToDbUseCase: SavePostToDbUseCase,
     @Named(UI_THREAD) private val mainDispatcher: CoroutineDispatcher,
     @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher,
-    private val uploadStarter: UploadStarter
+    private val uploadStarter: UploadStarter,
+    private val syncPublishingFeatureUtils: SyncPublishingFeatureUtils
 ) : ViewModel(), CoroutineScope {
     private val lifecycleOwner = object : LifecycleOwner {
         val lifecycleRegistry = LifecycleRegistry(this)
@@ -162,7 +163,9 @@ class PostListMainViewModel @Inject constructor(
             invalidateList = this::invalidateAllLists,
             checkNetworkConnection = this::checkNetworkConnection,
             showSnackbar = { _snackBarMessage.postValue(it) },
-            showToast = { _toastMessage.postValue(it) }
+            showToast = { _toastMessage.postValue(it) },
+            uploadStore = uploadStore,
+            postStore = postStore
         )
     }
 
@@ -182,7 +185,8 @@ class PostListMainViewModel @Inject constructor(
             showSnackbar = { _snackBarMessage.postValue(it) },
             showToast = { _toastMessage.postValue(it) },
             triggerPreviewStateUpdate = this::updatePreviewAndDialogState,
-            copyPost = this::copyPost
+            copyPost = this::copyPost,
+            syncPublishingFeatureUtils = syncPublishingFeatureUtils
         )
     }
 

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

@@ -29,7 +29,8 @@ sealed class PostUploadAction {
         val post: PostModel,
         val isError: Boolean,
         val isFirstTimePublish: Boolean,
-        val errorMessage: String?
+        val errorMessage: String?,
+        val showRetry: Boolean = true
     ) : PostUploadAction()
 
     class MediaUploadedSnackbar(
@@ -89,7 +90,8 @@ fun handleUploadAction(
                 action.post,
                 action.errorMessage,
                 action.site,
-                onPublishingCallback
+                onPublishingCallback,
+                action.showRetry
             )
         }
         is PostUploadAction.MediaUploadedSnackbar -> {

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

@@ -0,0 +1,29 @@
+package org.wordpress.android.ui.posts
+
+import org.wordpress.android.fluxc.store.PostStore.RemotePostPayload
+import org.wordpress.android.util.config.SyncPublishingFeatureConfig
+import javax.inject.Inject
+
+class SyncPublishingFeatureUtils @Inject constructor(
+    private val syncPublishingFeatureConfig: SyncPublishingFeatureConfig
+) {
+    private fun isSyncPublishingEnabled(): Boolean {
+        return syncPublishingFeatureConfig.isEnabled()
+    }
+
+    /**
+     * This helper function aids in post-conflict resolution. When attempting to edit a post,
+     * sending the "if_not_modified_since" to the backend will trigger a 409 error if a newer version
+     * has already been uploaded from another device. This functionality should be encapsulated
+     * by the SYNC_PUBLISHING feature flag. The function is used to generate the final RemotePostPayload
+     * that is sent to the backend through PostActionBuilder.newPushPostAction(). By setting the
+     * shouldSkipConflictResolutionCheck = true, "if_not_modified_since" is not sent to server and the post overwrites
+     * the remote version.
+     */
+    fun getRemotePostPayloadForPush(payload: RemotePostPayload): RemotePostPayload {
+        if (isSyncPublishingEnabled().not()) {
+            payload.shouldSkipConflictResolutionCheck = true
+        }
+        return payload
+    }
+}

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

@@ -34,6 +34,7 @@
 import org.wordpress.android.fluxc.store.PostStore.RemotePostPayload;
 import org.wordpress.android.fluxc.store.SiteStore;
 import org.wordpress.android.ui.posts.PostUtils;
+import org.wordpress.android.ui.posts.SyncPublishingFeatureUtils;
 import org.wordpress.android.ui.prefs.AppPrefs;
 import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.FetchPostStatusFailed;
 import org.wordpress.android.ui.uploads.AutoSavePostIfNotDraftResult.PostAutoSaveFailed;
@@ -83,6 +84,7 @@ public class PostUploadHandler implements UploadHandler<PostModel>, OnAutoSavePo
     @Inject UploadActionUseCase mUploadActionUseCase;
     @Inject AutoSavePostIfNotDraftUseCase mAutoSavePostIfNotDraftUseCase;
     @Inject PostMediaHandler mPostMediaHandler;
+    @Inject SyncPublishingFeatureUtils mSyncPublishingFeatureUtils;
 
     PostUploadHandler(PostUploadNotifier postUploadNotifier) {
         ((WordPress) WordPress.getContext().getApplicationContext()).component().inject(this);
@@ -285,12 +287,18 @@ protected UploadPostTaskResult doInBackground(PostModel... posts) {
             switch (mUploadActionUseCase.getUploadAction(mPost)) {
                 case UPLOAD:
                     AppLog.d(T.POSTS, "PostUploadHandler - UPLOAD. Post: " + mPost.getTitle());
-                    mDispatcher.dispatch(PostActionBuilder.newPushPostAction(payload));
+                    mDispatcher.dispatch(
+                            PostActionBuilder.newPushPostAction(
+                                    mSyncPublishingFeatureUtils.getRemotePostPayloadForPush(payload)
+                            )
+                    );
                     break;
                 case UPLOAD_AS_DRAFT:
                     mPost.setStatus(PostStatus.DRAFT.toString());
                     AppLog.d(T.POSTS, "PostUploadHandler - UPLOAD_AS_DRAFT. Post: " + mPost.getTitle());
-                    mDispatcher.dispatch(PostActionBuilder.newPushPostAction(payload));
+                    mDispatcher.dispatch(PostActionBuilder.newPushPostAction(
+                            mSyncPublishingFeatureUtils.getRemotePostPayloadForPush(payload)
+                    ));
                     break;
                 case REMOTE_AUTO_SAVE:
                     AppLog.d(T.POSTS, "PostUploadHandler - REMOTE_AUTO_SAVE. Post: " + mPost.getTitle());
@@ -637,7 +645,9 @@ public void handleAutoSavePostIfNotDraftResult(@NonNull AutoSavePostIfNotDraftRe
              */
             post.setStatus(PostStatus.DRAFT.toString());
             SiteModel site = mSiteStore.getSiteByLocalId(post.getLocalSiteId());
-            mDispatcher.dispatch(PostActionBuilder.newPushPostAction(new RemotePostPayload(post, site)));
+            mDispatcher.dispatch(PostActionBuilder.newPushPostAction(
+                    mSyncPublishingFeatureUtils.getRemotePostPayloadForPush(new RemotePostPayload(post, site))
+            ));
         } else {
             throw new IllegalStateException("All AutoSavePostIfNotDraftResult types must be handled");
         }
@@ -659,15 +669,30 @@ public void onPostUploaded(OnPostUploaded event) {
         if (event.isError()) {
             AppLog.w(T.POSTS, "PostUploadHandler > Post upload failed. " + event.error.type + ": "
                               + event.error.message);
-            Context context = WordPress.getContext();
-            String errorMessage = mUiHelpers.getTextOfUiString(context,
-                    UploadUtils.getErrorMessageResIdFromPostError(PostStatus.fromPost(event.post), event.post.isPage(),
-                            event.error, mUploadActionUseCase.isEligibleForAutoUpload(site, event.post))).toString();
-            String notificationMessage = UploadUtils.getErrorMessage(context, event.post.isPage(), errorMessage, false);
-            mPostUploadNotifier.removePostInfoFromForegroundNotification(event.post,
-                    mMediaStore.getMediaForPost(event.post));
-            mPostUploadNotifier.incrementUploadedPostCountFromForegroundNotification(event.post);
-            mPostUploadNotifier.updateNotificationErrorForPost(event.post, site, notificationMessage, 0);
+
+            if (event.error.type != PostStore.PostErrorType.OLD_REVISION) {
+                Context context = WordPress.getContext();
+                String errorMessage = mUiHelpers.getTextOfUiString(
+                        context,
+                        UploadUtils.getErrorMessageResIdFromPostError(
+                                PostStatus.fromPost(event.post),
+                                event.post.isPage(),
+                                event.error,
+                                mUploadActionUseCase.isEligibleForAutoUpload(site, event.post)
+                        )
+                ).toString();
+                String notificationMessage = UploadUtils.getErrorMessage(
+                        context,
+                        event.post.isPage(),
+                        errorMessage,
+                        false
+                );
+                mPostUploadNotifier.removePostInfoFromForegroundNotification(event.post,
+                        mMediaStore.getMediaForPost(event.post));
+                mPostUploadNotifier.incrementUploadedPostCountFromForegroundNotification(event.post);
+                mPostUploadNotifier.updateNotificationErrorForPost(event.post, site, notificationMessage, 0);
+            }
+
             sFirstPublishPosts.remove(event.post.getId());
         } else {
             mPostUploadNotifier.incrementUploadedPostCountFromForegroundNotification(event.post);

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

@@ -3,6 +3,7 @@ package org.wordpress.android.ui.uploads
 import dagger.Reusable
 import org.wordpress.android.fluxc.model.PostImmutableModel
 import org.wordpress.android.fluxc.model.SiteModel
+import org.wordpress.android.fluxc.store.PostStore
 import org.wordpress.android.fluxc.store.UploadStore
 import org.wordpress.android.ui.posts.PostUtilsWrapper
 import org.wordpress.android.ui.uploads.UploadActionUseCase.UploadAction.DO_NOTHING
@@ -39,7 +40,8 @@ class UploadActionUseCase @Inject constructor(
         }
 
         // Do not auto-upload post which is in conflict with remote
-        if (postUtilsWrapper.isPostInConflictWithRemote(post)) {
+        if (uploadStore.getUploadErrorForPost(post)?.postError?.type == PostStore.PostErrorType.OLD_REVISION
+            || postUtilsWrapper.isPostInConflictWithRemote(post)) {
             return DO_NOTHING
         }