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

Fixes #10108: Add auto save conflict dialog #10426

Merged
merged 12 commits into from
Aug 28, 2019
7 changes: 0 additions & 7 deletions .idea/inspectionProfiles/profiles_settings.xml

This file was deleted.

1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ XX.X
* Add Remote Preview support for posts and pages.
* Post List: Trashed post must now be restored before edit or preview.
* All changes to posts and pages will be automatically synced with the server.
* Post List: Unhandled conflict with auto saves are now detected and visible. On post opening, the app will let you choose which version you prefer.
* Clicking on "Publish" on a private post sometimes published the post as public

13.2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class PostActionHandler(
private val postStore: PostStore,
private val postListDialogHelper: PostListDialogHelper,
private val doesPostHaveUnhandledConflict: (PostModel) -> Boolean,
private val hasUnhandledAutoSave: (PostModel) -> Boolean,
private val triggerPostListAction: (PostListAction) -> Unit,
private val triggerPostUploadAction: (PostUploadAction) -> Unit,
private val invalidateList: () -> Unit,
Expand Down Expand Up @@ -170,12 +171,18 @@ class PostActionHandler(
}

private fun editPostButtonAction(site: SiteModel, post: PostModel) {
// first of all, check whether this post is in Conflicted state.
// first of all, check whether this post is in Conflicted state with a more recent remote version
if (doesPostHaveUnhandledConflict.invoke(post)) {
postListDialogHelper.showConflictedPostResolutionDialog(post)
return
}

// Then check if it's in conflicted state with a remote auto-save
malinajirka marked this conversation as resolved.
Show resolved Hide resolved
if (hasUnhandledAutoSave.invoke(post)) {
postListDialogHelper.showAutoSaveConflictedPostResolutionDialog(post)
return
}

editPost(site, post)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ class PostConflictResolver(
return !isFetchingConflictedPost && PostUtils.isPostInConflictWithRemote(post)
}

fun hasUnhandledAutoSave(post: PostModel): Boolean {
return PostUtils.isPostInConflictWithAutoSave(post)
malinajirka marked this conversation as resolved.
Show resolved Hide resolved
}

fun onPostSuccessfullyUpdated() {
originalPostCopyForConflictUndo?.id?.let {
val updatedPost = getPostByLocalPostId.invoke(it)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ private const val CONFIRM_DELETE_POST_DIALOG_TAG = "CONFIRM_DELETE_POST_DIALOG_T
private const val CONFIRM_PUBLISH_POST_DIALOG_TAG = "CONFIRM_PUBLISH_POST_DIALOG_TAG"
private const val CONFIRM_TRASH_POST_WITH_LOCAL_CHANGES_DIALOG_TAG = "CONFIRM_TRASH_POST_WITH_LOCAL_CHANGES_DIALOG_TAG"
private const val CONFIRM_ON_CONFLICT_LOAD_REMOTE_POST_DIALOG_TAG = "CONFIRM_ON_CONFLICT_LOAD_REMOTE_POST_DIALOG_TAG"
private const val CONFIRM_ON_AUTOSAVE_CONFLICT_DIALOG_TAG = "CONFIRM_ON_AUTOSAVE_CONFLICT_DIALOG_TAG"

/**
* This is a temporary class to make the PostListViewModel more manageable. Please feel free to refactor it any way
Expand All @@ -24,6 +25,7 @@ class PostListDialogHelper(
private var localPostIdForPublishDialog: Int? = null
private var localPostIdForTrashPostWithLocalChangesDialog: Int? = null
private var localPostIdForConflictResolutionDialog: Int? = null
private var localPostIdForAutoSaveConflictResolutionDialog: Int? = null

fun showDeletePostConfirmationDialog(post: PostModel) {
// We need network connection to delete a remote post, but not a local draft
Expand Down Expand Up @@ -84,12 +86,25 @@ class PostListDialogHelper(
showDialog.invoke(dialogHolder)
}

fun showAutoSaveConflictedPostResolutionDialog(post: PostModel) {
val dialogHolder = DialogHolder(
tag = CONFIRM_ON_AUTOSAVE_CONFLICT_DIALOG_TAG,
title = UiStringRes(R.string.dialog_confirm_autosave_title),
message = UiStringRes(R.string.dialog_confirm_autosave_body),
positiveButton = UiStringRes(R.string.dialog_confirm_autosave_restore_button),
negativeButton = UiStringRes(R.string.dialog_confirm_autosave_dont_restore_button)
)
localPostIdForAutoSaveConflictResolutionDialog = post.id
showDialog.invoke(dialogHolder)
}

fun onPositiveClickedForBasicDialog(
instanceTag: String,
trashPostWithLocalChanges: (Int) -> Unit,
deletePost: (Int) -> Unit,
publishPost: (Int) -> Unit,
updateConflictedPostWithRemoteVersion: (Int) -> Unit
updateConflictedPostWithRemoteVersion: (Int) -> Unit,
editRestoredAutoSavePost: (Int) -> Unit
) {
when (instanceTag) {
CONFIRM_DELETE_POST_DIALOG_TAG -> localPostIdForDeleteDialog?.let {
Expand All @@ -109,13 +124,19 @@ class PostListDialogHelper(
localPostIdForTrashPostWithLocalChangesDialog = null
trashPostWithLocalChanges(it)
}
CONFIRM_ON_AUTOSAVE_CONFLICT_DIALOG_TAG -> localPostIdForAutoSaveConflictResolutionDialog?.let {
// open the editor with the restored auto save
localPostIdForAutoSaveConflictResolutionDialog = null
editRestoredAutoSavePost(it)
}
else -> throw IllegalArgumentException("Dialog's positive button click is not handled: $instanceTag")
}
}

fun onNegativeClickedForBasicDialog(
instanceTag: String,
updateConflictedPostWithLocalVersion: (Int) -> Unit
updateConflictedPostWithLocalVersion: (Int) -> Unit,
editLocalPost: (Int) -> Unit
) {
when (instanceTag) {
CONFIRM_DELETE_POST_DIALOG_TAG -> localPostIdForDeleteDialog = null
Expand All @@ -124,20 +145,27 @@ class PostListDialogHelper(
CONFIRM_ON_CONFLICT_LOAD_REMOTE_POST_DIALOG_TAG -> localPostIdForConflictResolutionDialog?.let {
updateConflictedPostWithLocalVersion(it)
}
CONFIRM_ON_AUTOSAVE_CONFLICT_DIALOG_TAG -> localPostIdForAutoSaveConflictResolutionDialog?.let {
// open the editor with the local post (don't use the auto save version)
editLocalPost(it)
}
else -> throw IllegalArgumentException("Dialog's negative button click is not handled: $instanceTag")
}
}

fun onDismissByOutsideTouchForBasicDialog(
instanceTag: String,
updateConflictedPostWithLocalVersion: (Int) -> Unit
updateConflictedPostWithLocalVersion: (Int) -> Unit,
editLocalPost: (Int) -> Unit
) {
// Cancel and outside touch dismiss works the same way for all, except for conflict resolution dialog,
// Cancel and outside touch dismiss works the same way for all, except for conflict resolution dialogs,
// for which tapping outside and actively tapping the "edit local" have different meanings
if (instanceTag != CONFIRM_ON_CONFLICT_LOAD_REMOTE_POST_DIALOG_TAG) {
if (instanceTag != CONFIRM_ON_CONFLICT_LOAD_REMOTE_POST_DIALOG_TAG &&
instanceTag != CONFIRM_ON_AUTOSAVE_CONFLICT_DIALOG_TAG) {
onNegativeClickedForBasicDialog(
instanceTag = instanceTag,
updateConflictedPostWithLocalVersion = updateConflictedPostWithLocalVersion
updateConflictedPostWithLocalVersion = updateConflictedPostWithLocalVersion,
editLocalPost = editLocalPost
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import org.wordpress.android.analytics.AnalyticsTracker.Stat.POST_LIST_SEARCH_AC
import org.wordpress.android.analytics.AnalyticsTracker.Stat.POST_LIST_TAB_CHANGED
import org.wordpress.android.fluxc.Dispatcher
import org.wordpress.android.fluxc.generated.ListActionBuilder
import org.wordpress.android.fluxc.generated.PostActionBuilder
import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.model.list.PostListDescriptor
Expand Down Expand Up @@ -168,6 +169,7 @@ class PostListMainViewModel @Inject constructor(
postStore = postStore,
postListDialogHelper = postListDialogHelper,
doesPostHaveUnhandledConflict = postConflictResolver::doesPostHaveUnhandledConflict,
hasUnhandledAutoSave = postConflictResolver::hasUnhandledAutoSave,
triggerPostListAction = { _postListAction.postValue(it) },
triggerPostUploadAction = { _postUploadAction.postValue(it) },
invalidateList = this::invalidateAllLists,
Expand Down Expand Up @@ -269,6 +271,7 @@ class PostListMainViewModel @Inject constructor(
postActionHandler = postActionHandler,
getUploadStatus = uploadStatusTracker::getUploadStatus,
doesPostHaveUnhandledConflict = postConflictResolver::doesPostHaveUnhandledConflict,
hasAutoSave = postConflictResolver::hasUnhandledAutoSave,
postFetcher = postFetcher,
getFeaturedImageUrl = featuredImageTracker::getFeaturedImageUrl
)
Expand Down Expand Up @@ -358,6 +361,28 @@ class PostListMainViewModel @Inject constructor(
postActionHandler.handleEditPostResult(data)
}

private fun editRestoredAutoSavePost(localPostId: Int) {
val post = postStore.getPostByLocalPostId(localPostId)
if (post != null) {
post.title = post.autoSaveTitle ?: post.title
post.content = post.autoSaveContent ?: post.content
post.excerpt = post.autoSaveExcerpt ?: post.excerpt
dispatcher.dispatch(PostActionBuilder.newUpdatePostAction(post))
_postListAction.postValue(PostListAction.EditPost(site, post))
} else {
_snackBarMessage.value = SnackbarMessageHolder(R.string.error_post_does_not_exist)
}
}

private fun editLocalPost(localPostId: Int) {
val post = postStore.getPostByLocalPostId(localPostId)
if (post != null) {
_postListAction.postValue(PostListAction.EditPost(site, post))
} else {
_snackBarMessage.value = SnackbarMessageHolder(R.string.error_post_does_not_exist)
}
}

// BasicFragmentDialog Events

fun onPositiveClickedForBasicDialog(instanceTag: String) {
Expand All @@ -366,21 +391,24 @@ class PostListMainViewModel @Inject constructor(
trashPostWithLocalChanges = postActionHandler::trashPostWithLocalChanges,
deletePost = postActionHandler::deletePost,
publishPost = postActionHandler::publishPost,
updateConflictedPostWithRemoteVersion = postConflictResolver::updateConflictedPostWithRemoteVersion
updateConflictedPostWithRemoteVersion = postConflictResolver::updateConflictedPostWithRemoteVersion,
editRestoredAutoSavePost = this::editRestoredAutoSavePost
)
}

fun onNegativeClickedForBasicDialog(instanceTag: String) {
postListDialogHelper.onNegativeClickedForBasicDialog(
instanceTag = instanceTag,
updateConflictedPostWithLocalVersion = postConflictResolver::updateConflictedPostWithLocalVersion
updateConflictedPostWithLocalVersion = postConflictResolver::updateConflictedPostWithLocalVersion,
editLocalPost = this::editLocalPost
)
}

fun onDismissByOutsideTouchForBasicDialog(instanceTag: String) {
postListDialogHelper.onDismissByOutsideTouchForBasicDialog(
instanceTag = instanceTag,
updateConflictedPostWithLocalVersion = postConflictResolver::updateConflictedPostWithLocalVersion
updateConflictedPostWithLocalVersion = postConflictResolver::updateConflictedPostWithLocalVersion,
editLocalPost = this::editLocalPost
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,17 @@ public static boolean isPostInConflictWithRemote(PostModel post) {
return !post.getLastModified().equals(post.getRemoteLastModified()) && post.isLocallyChanged();
}

public static boolean isPostInConflictWithAutoSave(PostModel post) {
// TODO: would be great to check if title, content and excerpt are different,
// but we currently don't have them when we fetch the post list

// Ignore auto-saves in case the post is locally changed.
// This might be changed in the future to show a better conflict UX.
return !post.isLocallyChanged()
// has auto-save
&& post.hasUnpublishedRevision();
}

public static String getConflictedPostCustomStringForDialog(PostModel post) {
Context context = WordPress.getContext();
String firstPart = context.getString(R.string.dialog_confirm_load_remote_post_body);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class PostListItemUiStateHelper @Inject constructor(private val appPrefsWrapper:
post: PostModel,
uploadStatus: PostListItemUploadStatus,
unhandledConflicts: Boolean,
hasAutoSave: Boolean,
capabilitiesToPublish: Boolean,
statsSupported: Boolean,
featuredImageUrl: String?,
Expand Down Expand Up @@ -102,14 +103,16 @@ class PostListItemUiStateHelper @Inject constructor(private val appPrefsWrapper:
isLocalDraft = post.isLocalDraft,
isLocallyChanged = post.isLocallyChanged,
uploadUiState = uploadUiState,
hasUnhandledConflicts = unhandledConflicts
hasUnhandledConflicts = unhandledConflicts,
hasAutoSave = hasAutoSave
)
val statusesColor = getStatusesColor(
postStatus = postStatus,
isLocalDraft = post.isLocalDraft,
isLocallyChanged = post.isLocallyChanged,
uploadUiState = uploadUiState,
hasUnhandledConflicts = unhandledConflicts
hasUnhandledConflicts = unhandledConflicts,
hasAutoSave = hasAutoSave
)
val statusesDelimeter = UiStringRes(R.string.multiple_status_label_delimiter)
val onSelected = {
Expand Down Expand Up @@ -204,7 +207,8 @@ class PostListItemUiStateHelper @Inject constructor(private val appPrefsWrapper:
isLocalDraft: Boolean,
isLocallyChanged: Boolean,
uploadUiState: PostUploadUiState,
hasUnhandledConflicts: Boolean
hasUnhandledConflicts: Boolean,
hasAutoSave: Boolean
): List<UiString> {
val labels: MutableList<UiString> = ArrayList()
when {
Expand Down Expand Up @@ -233,6 +237,7 @@ class PostListItemUiStateHelper @Inject constructor(private val appPrefsWrapper:
}
}
hasUnhandledConflicts -> labels.add(UiStringRes(R.string.local_post_is_conflicted))
hasAutoSave -> labels.add(UiStringRes(R.string.local_post_autosave_conflict))
}

// we want to show either single error/progress label or 0-n info labels.
Expand Down Expand Up @@ -282,9 +287,10 @@ class PostListItemUiStateHelper @Inject constructor(private val appPrefsWrapper:
isLocalDraft: Boolean,
isLocallyChanged: Boolean,
uploadUiState: PostUploadUiState,
hasUnhandledConflicts: Boolean
hasUnhandledConflicts: Boolean,
hasAutoSave: Boolean
): Int? {
val isError = uploadUiState is PostUploadUiState.UploadFailed || hasUnhandledConflicts
val isError = uploadUiState is PostUploadUiState.UploadFailed || hasUnhandledConflicts || hasAutoSave
malinajirka marked this conversation as resolved.
Show resolved Hide resolved
val isProgressInfo = uploadUiState is UploadingPost || uploadUiState is UploadingMedia ||
uploadUiState is UploadQueued
val isStateInfo = isLocalDraft || isLocallyChanged || postStatus == PRIVATE || postStatus == PENDING ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ class PostListViewModel @Inject constructor(
post = post,
uploadStatus = connector.getUploadStatus(post, connector.site),
unhandledConflicts = connector.doesPostHaveUnhandledConflict(post),
hasAutoSave = connector.hasAutoSave(post),
capabilitiesToPublish = uploadUtilsWrapper.userCanPublish(connector.site),
statsSupported = isStatsSupported,
featuredImageUrl =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class PostListViewModelConnector(
val postActionHandler: PostActionHandler,
val getUploadStatus: (PostModel, SiteModel) -> PostListItemUploadStatus,
val doesPostHaveUnhandledConflict: (PostModel) -> Boolean,
val hasAutoSave: (PostModel) -> Boolean,
val postFetcher: PostFetcher,
private val getFeaturedImageUrl: (site: SiteModel, featuredImageId: Long) -> String?
) {
Expand Down
9 changes: 8 additions & 1 deletion WordPress/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,12 @@
<string name="snackbar_conflict_web_version_discarded">Web version discarded</string>
<string name="snackbar_conflict_undo">Undo</string>

<!-- post autosave conflict dialog -->
<string name="dialog_confirm_autosave_title">More Recent Version Conflict</string>
malinajirka marked this conversation as resolved.
Show resolved Hide resolved
<string name="dialog_confirm_autosave_body">A more recent version exists. Edit current version or most recent version?\n\n</string>
<string name="dialog_confirm_autosave_restore_button">More recent version</string>
<string name="dialog_confirm_autosave_dont_restore_button">Current version</string>

<!-- trash post with local changes dialog -->
<string name="dialog_confirm_trash_losing_local_changes_title">Local changes</string>
<string name="dialog_confirm_trash_losing_local_changes_message">Trashing this post will discard local changes, are you sure you want to continue?</string>
Expand Down Expand Up @@ -1412,6 +1418,7 @@

<!-- Remote Post has conflicts with local post -->
<string name="local_post_is_conflicted">Version conflict</string>
<string name="local_post_autosave_conflict">Unhandled Auto Save</string>

<!-- Post Remote Preview -->
<string name="post_preview_saving_draft">Saving&#8230;</string>
Expand All @@ -1421,7 +1428,7 @@
<string name="error_preview_empty_post">Can\'t preview an empty post</string>
<string name="error_preview_empty_page">Can\'t preview an empty page</string>
<string name="error_preview_empty_draft">Can\'t preview an empty draft</string>

<!-- message on post preview explaining links are disabled -->
<string name="preview_screen_links_disabled">Links are disabled on the preview screen</string>

Expand Down
Loading