From 2127d34dfdcb0de15305ba7c5eacbb6a4f43a9ee Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 7 Feb 2020 15:34:44 +0100 Subject: [PATCH 01/12] Introduce offline related labels to page list items --- .../wordpress/android/ui/pages/PageItem.kt | 9 +- .../android/ui/pages/PageItemViewHolder.kt | 2 +- .../pages/CreatePageListItemLabelsUseCase.kt | 140 ++++++++++++++++++ .../pages/CreatePageUploadUiStateUseCase.kt | 64 ++++++++ .../pages/PageItemUploadProgressHelper.kt | 80 +--------- .../viewmodel/pages/PageListViewModel.kt | 83 ++++++----- .../viewmodel/pages/SearchListViewModel.kt | 30 ++-- .../posts/PostListItemUiStateHelper.kt | 2 +- 8 files changed, 285 insertions(+), 125 deletions(-) create mode 100644 WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageUploadUiStateUseCase.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItem.kt b/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItem.kt index 5eaa9e49a359..85a92000e519 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItem.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItem.kt @@ -12,6 +12,7 @@ import org.wordpress.android.ui.pages.PageItem.Action.VIEW_PAGE import org.wordpress.android.ui.pages.PageItem.Type.DIVIDER import org.wordpress.android.ui.pages.PageItem.Type.EMPTY import org.wordpress.android.ui.pages.PageItem.Type.PAGE +import org.wordpress.android.ui.utils.UiString import org.wordpress.android.viewmodel.uistate.ProgressBarUiState import java.util.Date @@ -20,7 +21,7 @@ sealed class PageItem(open val type: Type) { open val id: Long, open val title: String, open val date: Date, - open val labels: List, + open val labels: List, open var indent: Int, open var imageUrl: String?, open val actions: Set, @@ -34,7 +35,7 @@ sealed class PageItem(open val type: Type) { override val id: Long, override val title: String, override val date: Date, - override val labels: List = emptyList(), + override val labels: List = emptyList(), override var indent: Int = 0, override var imageUrl: String? = null, override var actionsEnabled: Boolean = true, @@ -58,7 +59,7 @@ sealed class PageItem(open val type: Type) { override val id: Long, override val title: String, override val date: Date, - override val labels: List = emptyList(), + override val labels: List = emptyList(), override var imageUrl: String? = null, override var actionsEnabled: Boolean = true, override val progressBarUiState: ProgressBarUiState, @@ -81,7 +82,7 @@ sealed class PageItem(open val type: Type) { override val id: Long, override val title: String, override val date: Date, - override val labels: List = emptyList(), + override val labels: List = emptyList(), override var imageUrl: String? = null, override var actionsEnabled: Boolean = true, override val progressBarUiState: ProgressBarUiState, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt b/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt index fb35a71fc5a5..e87d6d773183 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt @@ -82,7 +82,7 @@ sealed class PageItemViewHolder(internal val parent: ViewGroup, @LayoutRes layou time.text = DateTimeUtils.javaDateToTimeSpan(date, parent.context) .capitalizeWithLocaleWithoutLint(parent.context.currentLocale) - labels.text = page.labels.map { parent.context.getString(it) }.sorted() + labels.text = page.labels.map { uiHelper.getTextOfUiString(parent.context,it) }.sorted() .joinToString(separator = " · ") uiHelper.updateVisibility(labels, page.labels.isNotEmpty()) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt new file mode 100644 index 000000000000..0bf15b43c1ea --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt @@ -0,0 +1,140 @@ +package org.wordpress.android.viewmodel.pages + +import org.wordpress.android.BuildConfig +import org.wordpress.android.R +import org.wordpress.android.fluxc.model.PostModel +import org.wordpress.android.fluxc.model.post.PostStatus +import org.wordpress.android.fluxc.model.post.PostStatus.DRAFT +import org.wordpress.android.fluxc.model.post.PostStatus.PENDING +import org.wordpress.android.fluxc.model.post.PostStatus.PRIVATE +import org.wordpress.android.fluxc.model.post.PostStatus.PUBLISHED +import org.wordpress.android.fluxc.model.post.PostStatus.SCHEDULED +import org.wordpress.android.fluxc.model.post.PostStatus.TRASHED +import org.wordpress.android.fluxc.model.post.PostStatus.UNKNOWN +import org.wordpress.android.ui.uploads.UploadUtils +import org.wordpress.android.ui.utils.UiString +import org.wordpress.android.ui.utils.UiString.UiStringRes +import org.wordpress.android.util.AppLog +import org.wordpress.android.util.AppLog.T.POSTS +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadFailed +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadQueued +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadWaitingForConnection +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadingMedia +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadingPost +import javax.inject.Inject + +class CreatePageListItemLabelsUseCase @Inject constructor() { + fun createLabels(pagePostModel: PostModel, uploadUiState: PostUploadUiState): List { + return getLabels( + PostStatus.fromPost(pagePostModel), + pagePostModel.isLocalDraft, + pagePostModel.isLocallyChanged, + uploadUiState, + false, // TODO use conflict resolver + false // TODO use conflict resolver + ) + } + + private fun getLabels( + postStatus: PostStatus, + isLocalDraft: Boolean, + isLocallyChanged: Boolean, + uploadUiState: PostUploadUiState, + hasUnhandledConflicts: Boolean, + hasAutoSave: Boolean + ): List { + val labels: MutableList = ArrayList() + when { + uploadUiState is PostUploadUiState.UploadFailed -> { + getErrorLabel(uploadUiState, postStatus)?.let { labels.add(it) } + } + uploadUiState is UploadingPost -> if (uploadUiState.isDraft) { + labels.add(UiStringRes(R.string.post_uploading_draft)) + } else { + labels.add(UiStringRes(R.string.post_uploading)) + } + uploadUiState is UploadingMedia -> labels.add(UiStringRes(R.string.uploading_media)) + uploadUiState is UploadQueued -> labels.add(UiStringRes(R.string.post_queued)) + uploadUiState is UploadWaitingForConnection -> { + when (uploadUiState.postStatus) { + UNKNOWN, PUBLISHED -> labels.add(UiStringRes(R.string.post_waiting_for_connection_publish)) + PRIVATE -> labels.add(UiStringRes(R.string.post_waiting_for_connection_private)) + PENDING -> labels.add(UiStringRes(R.string.post_waiting_for_connection_pending)) + SCHEDULED -> labels.add(UiStringRes(R.string.post_waiting_for_connection_scheduled)) + DRAFT -> labels.add(UiStringRes(R.string.post_waiting_for_connection_draft)) + TRASHED -> AppLog.e( + POSTS, + "Developer error: This state shouldn't happen. Trashed post is in " + + "UploadWaitingForConnection state." + ) + } + } + hasUnhandledConflicts -> labels.add(UiStringRes(R.string.local_post_is_conflicted)) + hasAutoSave -> labels.add(UiStringRes(R.string.local_post_autosave_revision_available)) + } + + // we want to show either single error/progress label or 0-n info labels. + if (labels.isEmpty()) { + if (isLocalDraft) { + labels.add(UiStringRes(R.string.local_draft)) + } else if (isLocallyChanged) { + labels.add(UiStringRes(R.string.local_changes)) + } + if (postStatus == PRIVATE) { + labels.add(UiStringRes(R.string.post_status_post_private)) + } + if (postStatus == PENDING) { + labels.add(UiStringRes(R.string.post_status_pending_review)) + } + } + return labels + } + + private fun getErrorLabel(uploadUiState: UploadFailed, postStatus: PostStatus): UiString? { + return when { + uploadUiState.error.mediaError != null -> getMediaUploadErrorLabel( + uploadUiState, + postStatus + ) + uploadUiState.error.postError != null -> UploadUtils.getErrorMessageResIdFromPostError( + postStatus, + false, + uploadUiState.error.postError, + uploadUiState.isEligibleForAutoUpload + ) + else -> { + val errorMsg = "MediaError and postError are both null." + if (BuildConfig.DEBUG) { + throw IllegalStateException(errorMsg) + } else { + AppLog.e(POSTS, errorMsg) + } + UiStringRes(R.string.error_generic) + } + } + } + + private fun getMediaUploadErrorLabel( + uploadUiState: UploadFailed, + postStatus: PostStatus + ): UiStringRes { + return when { + uploadUiState.isEligibleForAutoUpload -> when (postStatus) { + PUBLISHED -> UiStringRes(R.string.error_media_recover_post_not_published_retrying) + PRIVATE -> UiStringRes(R.string.error_media_recover_post_not_published_retrying_private) + SCHEDULED -> UiStringRes(R.string.error_media_recover_post_not_scheduled_retrying) + PENDING -> UiStringRes(R.string.error_media_recover_post_not_submitted_retrying) + DRAFT, TRASHED, UNKNOWN -> UiStringRes(R.string.error_generic_error_retrying) + } + uploadUiState.retryWillPushChanges -> when (postStatus) { + PUBLISHED -> UiStringRes(R.string.error_media_recover_post_not_published) + PRIVATE -> UiStringRes(R.string.error_media_recover_post_not_published_private) + SCHEDULED -> UiStringRes(R.string.error_media_recover_post_not_scheduled) + PENDING -> UiStringRes(R.string.error_media_recover_post_not_submitted) + DRAFT, TRASHED, UNKNOWN -> UiStringRes(R.string.error_media_recover_post) + } + else -> UiStringRes(R.string.error_media_recover_post) + } + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageUploadUiStateUseCase.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageUploadUiStateUseCase.kt new file mode 100644 index 000000000000..aaa1c7fc71e3 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageUploadUiStateUseCase.kt @@ -0,0 +1,64 @@ +package org.wordpress.android.viewmodel.pages + +import org.wordpress.android.fluxc.model.PostModel +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.model.post.PostStatus +import org.wordpress.android.fluxc.model.post.PostStatus.DRAFT +import org.wordpress.android.fluxc.store.UploadStore.UploadError +import org.wordpress.android.ui.posts.PostModelUploadStatusTracker +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.NothingToUpload +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadFailed +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadQueued +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadWaitingForConnection +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadingMedia +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadingPost +import javax.inject.Inject + +class CreatePageUploadUiStateUseCase @Inject constructor(val uploadStatusTracker: PostModelUploadStatusTracker) { + /** + * Copied from PostListItemUiStateHelper since the behavior is similar for the Page List UI State. + */ + fun createUploadUiState( + post: PostModel, + site: SiteModel + ): PostUploadUiState { + val postStatus = PostStatus.fromPost(post) + val uploadStatus = uploadStatusTracker.getUploadStatus(post, site) + return when { + uploadStatus.hasInProgressMediaUpload -> UploadingMedia( + uploadStatus.mediaUploadProgress + ) + uploadStatus.isUploading -> UploadingPost( + postStatus == DRAFT + ) + // the upload error is not null on retry -> it needs to be evaluated after UploadingMedia and UploadingPost + uploadStatus.uploadError != null -> UploadFailed( + uploadStatus.uploadError, + uploadStatus.isEligibleForAutoUpload, + uploadStatus.uploadWillPushChanges + ) + uploadStatus.hasPendingMediaUpload || + uploadStatus.isQueued || + uploadStatus.isUploadingOrQueued -> UploadQueued + uploadStatus.isEligibleForAutoUpload -> UploadWaitingForConnection(postStatus) + else -> NothingToUpload + } + } + + /** + * Copied from PostListItemUiStateHelper since the behavior is similar for the Page List UI State. + */ + sealed class PostUploadUiState { + data class UploadingMedia(val progress: Int) : PostUploadUiState() + data class UploadingPost(val isDraft: Boolean) : PostUploadUiState() + data class UploadFailed( + val error: UploadError, + val isEligibleForAutoUpload: Boolean, + val retryWillPushChanges: Boolean + ) : PostUploadUiState() + + data class UploadWaitingForConnection(val postStatus: PostStatus) : PostUploadUiState() + object UploadQueued : PostUploadUiState() + object NothingToUpload : PostUploadUiState() + } +} diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageItemUploadProgressHelper.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageItemUploadProgressHelper.kt index 699203ccff0c..91f766101fe0 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageItemUploadProgressHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageItemUploadProgressHelper.kt @@ -1,43 +1,25 @@ package org.wordpress.android.viewmodel.pages -import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId import org.wordpress.android.fluxc.model.PostModel -import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.fluxc.model.post.PostStatus -import org.wordpress.android.fluxc.model.post.PostStatus.DRAFT -import org.wordpress.android.fluxc.store.PostStore -import org.wordpress.android.fluxc.store.UploadStore.UploadError -import org.wordpress.android.ui.posts.PostModelUploadStatusTracker import org.wordpress.android.ui.prefs.AppPrefsWrapper -import org.wordpress.android.viewmodel.pages.PageItemUploadProgressHelper.PostUploadUiState.NothingToUpload -import org.wordpress.android.viewmodel.pages.PageItemUploadProgressHelper.PostUploadUiState.UploadFailed -import org.wordpress.android.viewmodel.pages.PageItemUploadProgressHelper.PostUploadUiState.UploadQueued -import org.wordpress.android.viewmodel.pages.PageItemUploadProgressHelper.PostUploadUiState.UploadWaitingForConnection -import org.wordpress.android.viewmodel.pages.PageItemUploadProgressHelper.PostUploadUiState.UploadingMedia -import org.wordpress.android.viewmodel.pages.PageItemUploadProgressHelper.PostUploadUiState.UploadingPost +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadQueued +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadingMedia +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadingPost import org.wordpress.android.viewmodel.uistate.ProgressBarUiState -import org.wordpress.android.viewmodel.posts.PostListItemUploadStatus import javax.inject.Inject typealias ShouldShowOverlay = Boolean class PageItemUploadProgressHelper @Inject constructor( - private val appPrefsWrapper: AppPrefsWrapper, - private val postStore: PostStore, - val uploadStatusTracker: PostModelUploadStatusTracker + private val appPrefsWrapper: AppPrefsWrapper ) { fun getProgressStateForPage( - pageId: LocalId, - site: SiteModel + post: PostModel?, + uploadUiState: PostUploadUiState ): Pair { - val post = postStore.getPostByLocalPostId(pageId.value) - + // TODO ideally don't accept nullable PostModel post?.let { - val uploadStatus = uploadStatusTracker.getUploadStatus( - post, site - ) - val uploadUiState = createUploadUiState(uploadStatus, post) - val shouldShowOverlay = shouldShowOverlay(uploadUiState) return Pair(getProgressBarState(uploadUiState), shouldShowOverlay) } @@ -71,52 +53,6 @@ class PageItemUploadProgressHelper @Inject constructor( uploadUiState is UploadQueued } - /** - * Copied from PostListItemUiStateHelper since the behavior is similar for the Page List UI State. - */ - sealed class PostUploadUiState { - data class UploadingMedia(val progress: Int) : PostUploadUiState() - data class UploadingPost(val isDraft: Boolean) : PostUploadUiState() - data class UploadFailed( - val error: UploadError, - val isEligibleForAutoUpload: Boolean, - val retryWillPushChanges: Boolean - ) : PostUploadUiState() - - data class UploadWaitingForConnection(val postStatus: PostStatus) : PostUploadUiState() - object UploadQueued : PostUploadUiState() - object NothingToUpload : PostUploadUiState() - } - - /** - * Copied from PostListItemUiStateHelper since the behavior is similar for the Page List UI State. - */ - private fun createUploadUiState( - uploadStatus: PostListItemUploadStatus, - post: PostModel - ): PostUploadUiState { - val postStatus = PostStatus.fromPost(post) - return when { - uploadStatus.hasInProgressMediaUpload -> UploadingMedia( - uploadStatus.mediaUploadProgress - ) - uploadStatus.isUploading -> UploadingPost( - postStatus == DRAFT - ) - // the upload error is not null on retry -> it needs to be evaluated after UploadingMedia and UploadingPost - uploadStatus.uploadError != null -> UploadFailed( - uploadStatus.uploadError, - uploadStatus.isEligibleForAutoUpload, - uploadStatus.uploadWillPushChanges - ) - uploadStatus.hasPendingMediaUpload || - uploadStatus.isQueued || - uploadStatus.isUploadingOrQueued -> UploadQueued - uploadStatus.isEligibleForAutoUpload -> UploadWaitingForConnection(postStatus) - else -> NothingToUpload - } - } - private fun shouldShowOverlay(uploadUiState: PostUploadUiState): Boolean { // show overlay when post upload is in progress or (media upload is in progress and the user is not using Aztec) return (uploadUiState is UploadingPost || diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt index 32b49e8521a2..815a60aefda1 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/PageListViewModel.kt @@ -18,6 +18,7 @@ import org.wordpress.android.fluxc.model.page.PageStatus import org.wordpress.android.fluxc.store.MediaStore import org.wordpress.android.fluxc.store.MediaStore.MediaPayload import org.wordpress.android.fluxc.store.MediaStore.OnMediaChanged +import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.ui.pages.PageItem import org.wordpress.android.ui.pages.PageItem.Action @@ -28,6 +29,7 @@ import org.wordpress.android.ui.pages.PageItem.Page import org.wordpress.android.ui.pages.PageItem.PublishedPage import org.wordpress.android.ui.pages.PageItem.ScheduledPage import org.wordpress.android.ui.pages.PageItem.TrashedPage +import org.wordpress.android.ui.utils.UiString import org.wordpress.android.util.AppLog import org.wordpress.android.util.LocaleManagerWrapper import org.wordpress.android.util.SiteUtils @@ -39,6 +41,7 @@ import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListType.DRAF import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListType.PUBLISHED import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListType.SCHEDULED import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListType.TRASHED +import org.wordpress.android.viewmodel.uistate.ProgressBarUiState import javax.inject.Inject import javax.inject.Named @@ -46,7 +49,10 @@ private const val MAX_TOPOLOGICAL_PAGE_COUNT = 100 private const val DEFAULT_INDENT = 0 class PageListViewModel @Inject constructor( + private val createPageListItemLabelsUseCase: CreatePageListItemLabelsUseCase, + private val createPageUploadUiStateUseCase: CreatePageUploadUiStateUseCase, private val mediaStore: MediaStore, + private val postStore: PostStore, private val dispatcher: Dispatcher, private val localeManagerWrapper: LocaleManagerWrapper, @Named(BG_THREAD) private val coroutineDispatcher: CoroutineDispatcher, @@ -87,6 +93,7 @@ class PageListViewModel @Inject constructor( } } } + val title: Int get() = when (this) { PUBLISHED -> R.string.pages_published @@ -156,7 +163,7 @@ class PageListViewModel @Inject constructor( } private val uploadStatusObserver = Observer> { ids -> - progressHelper.uploadStatusTracker.invalidateUploadStatus(ids.map { localId -> localId.value }) + createPageUploadUiStateUseCase.uploadStatusTracker.invalidateUploadStatus(ids.map { localId -> localId.value }) } private fun loadPagesAsync(pages: List) = launch { @@ -237,26 +244,19 @@ class PageListViewModel @Inject constructor( } return sortedPages .map { - val labels = mutableListOf() - if (it.status == PageStatus.PRIVATE) - labels.add(R.string.pages_private) - if (it.hasLocalChanges) - labels.add(R.string.local_changes) - val pageItemIndent = if (shouldSortTopologically) { getPageItemIndent(it) } else { DEFAULT_INDENT } - val (progressBarUiState, showOverlay) = progressHelper.getProgressStateForPage(LocalId(it.pageId), - pagesViewModel.site) + val itemUiStateData = createItemUiStateData(it) PublishedPage( - it.remoteId, it.title, it.date, labels, pageItemIndent, + it.remoteId, it.title, it.date, itemUiStateData.labels, pageItemIndent, getFeaturedImageUrl(it.featuredImageId), actionsEnabled, - progressBarUiState, - showOverlay + itemUiStateData.progressBarUiState, + itemUiStateData.showOverlay ) } } @@ -269,20 +269,14 @@ class PageListViewModel @Inject constructor( .map { (date, results) -> listOf(Divider(date)) + results.map { - val labels = mutableListOf() - if (it.hasLocalChanges) - labels.add(R.string.local_changes) - - val (progressBarUiState, showOverlay) = progressHelper.getProgressStateForPage( - LocalId(it.pageId), - pagesViewModel.site) + val itemUiStateData = createItemUiStateData(it) ScheduledPage( - it.remoteId, it.title, it.date, labels, + it.remoteId, it.title, it.date, itemUiStateData.labels, getFeaturedImageUrl(it.featuredImageId), actionsEnabled, - progressBarUiState, - showOverlay + itemUiStateData.progressBarUiState, + itemUiStateData.showOverlay ) } } @@ -294,23 +288,16 @@ class PageListViewModel @Inject constructor( private fun prepareDraftPages(pages: List, actionsEnabled: Boolean): List { return pages.map { - val labels = mutableListOf() - if (it.status == PageStatus.PENDING) - labels.add(R.string.pages_pending) - if (it.hasLocalChanges) - labels.add(R.string.local_draft) - - val (progressBarUiState, showOverlay) = progressHelper.getProgressStateForPage(LocalId(it.pageId), - pagesViewModel.site) + val itemUiStateData = createItemUiStateData(it) DraftPage( it.remoteId, it.title, it.date, - labels, + itemUiStateData.labels, getFeaturedImageUrl(it.featuredImageId), actionsEnabled, - progressBarUiState, - showOverlay + itemUiStateData.progressBarUiState, + itemUiStateData.showOverlay ) } } @@ -320,16 +307,15 @@ class PageListViewModel @Inject constructor( actionsEnabled: Boolean ): List { return pages.map { - val (progressBarUiState, showOverlay) = progressHelper.getProgressStateForPage(LocalId(it.pageId), - pagesViewModel.site) + val itemUiStateData = createItemUiStateData(it) TrashedPage( it.remoteId, it.title, it.date, getFeaturedImageUrl(it.featuredImageId), actionsEnabled, - progressBarUiState, - showOverlay + itemUiStateData.progressBarUiState, + itemUiStateData.showOverlay ) } } @@ -369,4 +355,27 @@ class PageListViewModel @Inject constructor( invalidateFeaturedMedia(*event.mediaList.map { it.mediaId }.toLongArray()) } } + + private fun createItemUiStateData(pageModel: PageModel): ItemUiStateData { + // TODO don't load the post model from db during uistate creation as it can have significant performance impact + val postModel = postStore.getPostByLocalPostId(pageModel.pageId) + // TODO the postmodel is sometimes null, why? + val uploadUiState = createPageUploadUiStateUseCase.createUploadUiState( + postModel, + pagesViewModel.site + ) + val labels = createPageListItemLabelsUseCase.createLabels(postModel, uploadUiState) + + val (progressBarUiState, showOverlay) = progressHelper.getProgressStateForPage( + postModel, + uploadUiState + ) + return ItemUiStateData(labels, progressBarUiState, showOverlay) + } + + private data class ItemUiStateData( + val labels: List, + val progressBarUiState: ProgressBarUiState, + val showOverlay: Boolean + ) } diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/SearchListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/SearchListViewModel.kt index 579d9422796b..00cdfd32f2ad 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/SearchListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/SearchListViewModel.kt @@ -7,9 +7,9 @@ import androidx.lifecycle.ViewModel import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch import org.wordpress.android.R -import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId import org.wordpress.android.fluxc.model.page.PageModel import org.wordpress.android.fluxc.model.page.PageStatus +import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.modules.UI_SCOPE import org.wordpress.android.ui.pages.PageItem import org.wordpress.android.ui.pages.PageItem.Action @@ -28,6 +28,8 @@ import javax.inject.Named class SearchListViewModel @Inject constructor( + private val createPageUploadUiStateUseCase: CreatePageUploadUiStateUseCase, + private val postStore: PostStore, private val resourceProvider: ResourceProvider, @Named(UI_SCOPE) private val uiScope: CoroutineScope, private val progressHelper: PageItemUploadProgressHelper @@ -88,7 +90,15 @@ class SearchListViewModel } private fun PageModel.toPageItem(areActionsEnabled: Boolean): PageItem { - val progressState = progressHelper.getProgressStateForPage(LocalId(pageId), pagesViewModel.site) + // TODO don't load the post model from db during uistate creation + val postModel = postStore.getPostByLocalPostId(this.pageId) + val uploadUiState = createPageUploadUiStateUseCase.createUploadUiState( + postModel, + pagesViewModel.site + ) + // TODO any reason why we don't show labels in search? + val (progressBarUiState, showOverlay) = progressHelper.getProgressStateForPage(postModel, + uploadUiState) return when (status) { PageStatus.PUBLISHED, PageStatus.PRIVATE -> @@ -97,32 +107,32 @@ class SearchListViewModel title, date, actionsEnabled = areActionsEnabled, - progressBarUiState = progressState.first, - showOverlay = progressState.second + progressBarUiState = progressBarUiState, + showOverlay = showOverlay ) PageStatus.DRAFT, PageStatus.PENDING -> DraftPage( remoteId, title, date, actionsEnabled = areActionsEnabled, - progressBarUiState = progressState.first, - showOverlay = progressState.second + progressBarUiState = progressBarUiState, + showOverlay = showOverlay ) PageStatus.TRASHED -> TrashedPage( remoteId, title, date, actionsEnabled = areActionsEnabled, - progressBarUiState = progressState.first, - showOverlay = progressState.second + progressBarUiState = progressBarUiState, + showOverlay = showOverlay ) PageStatus.SCHEDULED -> ScheduledPage( remoteId, title, date, actionsEnabled = areActionsEnabled, - progressBarUiState = progressState.first, - showOverlay = progressState.second + progressBarUiState = progressBarUiState, + showOverlay = showOverlay ) } } diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt index 7f4e555beb59..c63418f59138 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt @@ -434,7 +434,7 @@ class PostListItemUiStateHelper @Inject constructor(private val appPrefsWrapper: return PostListItemAction.MoreItem(allItems, onButtonClicked) } - private sealed class PostUploadUiState { + private sealed class PostUploadUiState { data class UploadingMedia(val progress: Int) : PostUploadUiState() data class UploadingPost(val isDraft: Boolean) : PostUploadUiState() data class UploadFailed( From 8df63ebedfabdd4c46d9735ee90fa8a560dcf3ab Mon Sep 17 00:00:00 2001 From: malinajirka Date: Fri, 7 Feb 2020 16:37:18 +0100 Subject: [PATCH 02/12] Update strings resources for page list --- .../pages/CreatePageListItemLabelsUseCase.kt | 78 +++++++++---------- WordPress/src/main/res/values/strings.xml | 39 ++++++++-- 2 files changed, 73 insertions(+), 44 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt index 0bf15b43c1ea..84aa96fff67c 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt @@ -15,7 +15,7 @@ import org.wordpress.android.ui.uploads.UploadUtils import org.wordpress.android.ui.utils.UiString import org.wordpress.android.ui.utils.UiString.UiStringRes import org.wordpress.android.util.AppLog -import org.wordpress.android.util.AppLog.T.POSTS +import org.wordpress.android.util.AppLog.T.PAGES import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadFailed import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadQueued @@ -37,7 +37,7 @@ class CreatePageListItemLabelsUseCase @Inject constructor() { } private fun getLabels( - postStatus: PostStatus, + pagePostStatus: PostStatus, isLocalDraft: Boolean, isLocallyChanged: Boolean, uploadUiState: PostUploadUiState, @@ -46,59 +46,59 @@ class CreatePageListItemLabelsUseCase @Inject constructor() { ): List { val labels: MutableList = ArrayList() when { - uploadUiState is PostUploadUiState.UploadFailed -> { - getErrorLabel(uploadUiState, postStatus)?.let { labels.add(it) } + uploadUiState is UploadFailed -> { + getErrorLabel(uploadUiState, pagePostStatus)?.let { labels.add(it) } } uploadUiState is UploadingPost -> if (uploadUiState.isDraft) { - labels.add(UiStringRes(R.string.post_uploading_draft)) + labels.add(UiStringRes(R.string.page_uploading_draft)) } else { - labels.add(UiStringRes(R.string.post_uploading)) + labels.add(UiStringRes(R.string.page_uploading)) } uploadUiState is UploadingMedia -> labels.add(UiStringRes(R.string.uploading_media)) - uploadUiState is UploadQueued -> labels.add(UiStringRes(R.string.post_queued)) + uploadUiState is UploadQueued -> labels.add(UiStringRes(R.string.page_queued)) uploadUiState is UploadWaitingForConnection -> { when (uploadUiState.postStatus) { - UNKNOWN, PUBLISHED -> labels.add(UiStringRes(R.string.post_waiting_for_connection_publish)) - PRIVATE -> labels.add(UiStringRes(R.string.post_waiting_for_connection_private)) - PENDING -> labels.add(UiStringRes(R.string.post_waiting_for_connection_pending)) - SCHEDULED -> labels.add(UiStringRes(R.string.post_waiting_for_connection_scheduled)) - DRAFT -> labels.add(UiStringRes(R.string.post_waiting_for_connection_draft)) + UNKNOWN, PUBLISHED -> labels.add(UiStringRes(R.string.page_waiting_for_connection_publish)) + PRIVATE -> labels.add(UiStringRes(R.string.page_waiting_for_connection_private)) + PENDING -> labels.add(UiStringRes(R.string.page_waiting_for_connection_pending)) + SCHEDULED -> labels.add(UiStringRes(R.string.page_waiting_for_connection_scheduled)) + DRAFT -> labels.add(UiStringRes(R.string.page_waiting_for_connection_draft)) TRASHED -> AppLog.e( - POSTS, - "Developer error: This state shouldn't happen. Trashed post is in " + + PAGES, + "Developer error: This state shouldn't happen. Trashed pages is in " + "UploadWaitingForConnection state." ) } } - hasUnhandledConflicts -> labels.add(UiStringRes(R.string.local_post_is_conflicted)) - hasAutoSave -> labels.add(UiStringRes(R.string.local_post_autosave_revision_available)) + hasUnhandledConflicts -> labels.add(UiStringRes(R.string.local_page_is_conflicted)) + hasAutoSave -> labels.add(UiStringRes(R.string.local_page_autosave_revision_available)) } // we want to show either single error/progress label or 0-n info labels. if (labels.isEmpty()) { if (isLocalDraft) { - labels.add(UiStringRes(R.string.local_draft)) + labels.add(UiStringRes(R.string.page_local_draft)) } else if (isLocallyChanged) { - labels.add(UiStringRes(R.string.local_changes)) + labels.add(UiStringRes(R.string.page_local_changes)) } - if (postStatus == PRIVATE) { - labels.add(UiStringRes(R.string.post_status_post_private)) + if (pagePostStatus == PRIVATE) { + labels.add(UiStringRes(R.string.page_status_page_private)) } - if (postStatus == PENDING) { - labels.add(UiStringRes(R.string.post_status_pending_review)) + if (pagePostStatus == PENDING) { + labels.add(UiStringRes(R.string.page_status_pending_review)) } } return labels } - private fun getErrorLabel(uploadUiState: UploadFailed, postStatus: PostStatus): UiString? { + private fun getErrorLabel(uploadUiState: UploadFailed, pagePostStatus: PostStatus): UiString? { return when { uploadUiState.error.mediaError != null -> getMediaUploadErrorLabel( uploadUiState, - postStatus + pagePostStatus ) uploadUiState.error.postError != null -> UploadUtils.getErrorMessageResIdFromPostError( - postStatus, + pagePostStatus, false, uploadUiState.error.postError, uploadUiState.isEligibleForAutoUpload @@ -108,7 +108,7 @@ class CreatePageListItemLabelsUseCase @Inject constructor() { if (BuildConfig.DEBUG) { throw IllegalStateException(errorMsg) } else { - AppLog.e(POSTS, errorMsg) + AppLog.e(PAGES, errorMsg) } UiStringRes(R.string.error_generic) } @@ -117,24 +117,24 @@ class CreatePageListItemLabelsUseCase @Inject constructor() { private fun getMediaUploadErrorLabel( uploadUiState: UploadFailed, - postStatus: PostStatus + pagePostStatus: PostStatus ): UiStringRes { return when { - uploadUiState.isEligibleForAutoUpload -> when (postStatus) { - PUBLISHED -> UiStringRes(R.string.error_media_recover_post_not_published_retrying) - PRIVATE -> UiStringRes(R.string.error_media_recover_post_not_published_retrying_private) - SCHEDULED -> UiStringRes(R.string.error_media_recover_post_not_scheduled_retrying) - PENDING -> UiStringRes(R.string.error_media_recover_post_not_submitted_retrying) + uploadUiState.isEligibleForAutoUpload -> when (pagePostStatus) { + PUBLISHED -> UiStringRes(R.string.error_media_recover_page_not_published_retrying) + PRIVATE -> UiStringRes(R.string.error_media_recover_page_not_published_retrying_private) + SCHEDULED -> UiStringRes(R.string.error_media_recover_page_not_scheduled_retrying) + PENDING -> UiStringRes(R.string.error_media_recover_page_not_submitted_retrying) DRAFT, TRASHED, UNKNOWN -> UiStringRes(R.string.error_generic_error_retrying) } - uploadUiState.retryWillPushChanges -> when (postStatus) { - PUBLISHED -> UiStringRes(R.string.error_media_recover_post_not_published) - PRIVATE -> UiStringRes(R.string.error_media_recover_post_not_published_private) - SCHEDULED -> UiStringRes(R.string.error_media_recover_post_not_scheduled) - PENDING -> UiStringRes(R.string.error_media_recover_post_not_submitted) - DRAFT, TRASHED, UNKNOWN -> UiStringRes(R.string.error_media_recover_post) + uploadUiState.retryWillPushChanges -> when (pagePostStatus) { + PUBLISHED -> UiStringRes(R.string.error_media_recover_page_not_published) + PRIVATE -> UiStringRes(R.string.error_media_recover_page_not_published_private) + SCHEDULED -> UiStringRes(R.string.error_media_recover_page_not_scheduled) + PENDING -> UiStringRes(R.string.error_media_recover_page_not_submitted) + DRAFT, TRASHED, UNKNOWN -> UiStringRes(R.string.error_media_recover_page) } - else -> UiStringRes(R.string.error_media_recover_post) + else -> UiStringRes(R.string.error_media_recover_page) } } } diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index d01329ddbfe0..5f942e2ebefc 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -109,6 +109,8 @@ exit OK We cannot load the data for your site right now. Please try again later + Local draft + Local changes Not now @@ -246,7 +248,6 @@ We cannot open the posts right now. Please try again later Untitled (Untitled) - Local draft Uploading post Uploading draft Queued post @@ -1451,12 +1452,11 @@ Terms of Service Privacy policy - - Local changes - - + Version conflict You\'ve made unsaved changes to this post + @string/local_post_is_conflicted + You\'ve made unsaved changes to this page Saving… @@ -2569,6 +2569,35 @@ The selected page is not available + @string/post_status_pending_review + @string/post_status_draft + @string/post_status_post_private + + @string/local_draft + @string/local_changes + Uploading page + @string/post_uploading_draft + Queued page + + We\'ll publish the page when your device is back online. + We\'ll submit your page for review when your device is back online. + We\'ll schedule your page when your device is back online. + We\'ll publish your private page when your device is back online. + We\'ll save your draft when your device is back online + + We couldn\'t upload this media, and didn\'t publish the page. + We couldn\'t upload this media, and didn\'t publish this private page. + We couldn\'t upload this media, and didn\'t schedule this page. + We couldn\'t upload this media, and didn\'t submit this page for review. + + @string/error_page_not_published_retrying + @string/error_page_not_published_retrying_private + @string/error_page_not_scheduled_retrying + @string/error_page_not_submitted_retrying + + @string/error_media_recover_post + + Post author Me From 5988cfb78a4a92d57f8c2d9222ca339e19a833ed Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Feb 2020 09:10:02 +0100 Subject: [PATCH 03/12] Update strings.xml --- WordPress/src/main/res/values/strings.xml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 5f942e2ebefc..a222c3457b8e 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -2590,6 +2590,11 @@ We couldn\'t upload this media, and didn\'t schedule this page. We couldn\'t upload this media, and didn\'t submit this page for review. + We couldn\'t publish this page, but we\'ll try again later. + We couldn\'t publish this private page, but we\'ll try again later. + We couldn\'t schedule this page, but we\'ll try again later. + We couldn\'t submit this page for review, but we\'ll try again later. + @string/error_page_not_published_retrying @string/error_page_not_published_retrying_private @string/error_page_not_scheduled_retrying @@ -2597,7 +2602,6 @@ @string/error_media_recover_post - Post author Me From a79fa229c3cb4d24aad352599b1493236ac2bc40 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Feb 2020 09:27:47 +0100 Subject: [PATCH 04/12] Show unhandled auto-save label to page list items --- .../viewmodel/pages/CreatePageListItemLabelsUseCase.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt index 84aa96fff67c..327cee61be14 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt @@ -24,7 +24,10 @@ import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.Post import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState.UploadingPost import javax.inject.Inject -class CreatePageListItemLabelsUseCase @Inject constructor() { +/** + * Most of this code has been copied from PostListItemUIStateHelper. + */ +class CreatePageListItemLabelsUseCase @Inject constructor(private val pageConflictResolver: PageConflictResolver) { fun createLabels(pagePostModel: PostModel, uploadUiState: PostUploadUiState): List { return getLabels( PostStatus.fromPost(pagePostModel), @@ -32,7 +35,7 @@ class CreatePageListItemLabelsUseCase @Inject constructor() { pagePostModel.isLocallyChanged, uploadUiState, false, // TODO use conflict resolver - false // TODO use conflict resolver + pageConflictResolver.hasUnhandledAutoSave(pagePostModel) ) } From f0083d3e4358374659a4f955e6b12c9a485a1d68 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Feb 2020 10:20:04 +0100 Subject: [PATCH 05/12] Fix lint issues --- .../java/org/wordpress/android/ui/pages/PageItemViewHolder.kt | 2 +- .../android/viewmodel/posts/PostListItemUiStateHelper.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt b/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt index e87d6d773183..8e24c798a375 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt @@ -82,7 +82,7 @@ sealed class PageItemViewHolder(internal val parent: ViewGroup, @LayoutRes layou time.text = DateTimeUtils.javaDateToTimeSpan(date, parent.context) .capitalizeWithLocaleWithoutLint(parent.context.currentLocale) - labels.text = page.labels.map { uiHelper.getTextOfUiString(parent.context,it) }.sorted() + labels.text = page.labels.map { uiHelper.getTextOfUiString(parent.context, it) }.sorted() .joinToString(separator = " · ") uiHelper.updateVisibility(labels, page.labels.isNotEmpty()) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt index c63418f59138..7f4e555beb59 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListItemUiStateHelper.kt @@ -434,7 +434,7 @@ class PostListItemUiStateHelper @Inject constructor(private val appPrefsWrapper: return PostListItemAction.MoreItem(allItems, onButtonClicked) } - private sealed class PostUploadUiState { + private sealed class PostUploadUiState { data class UploadingMedia(val progress: Int) : PostUploadUiState() data class UploadingPost(val isDraft: Boolean) : PostUploadUiState() data class UploadFailed( From e155e7c8e6040cc44658836217c1d2b99b25b0f3 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Feb 2020 11:55:55 +0100 Subject: [PATCH 06/12] Fix unit tests --- .../viewmodel/pages/PageListViewModelTest.kt | 27 ++++++++++++++++--- .../pages/SearchListViewModelTest.kt | 12 ++++++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt index 7b4cf46e8b9d..98ef47a80502 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/PageListViewModelTest.kt @@ -2,25 +2,31 @@ package org.wordpress.android.viewmodel.pages import androidx.lifecycle.MutableLiveData import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.anyOrNull +import com.nhaarman.mockitokotlin2.argThat import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import kotlinx.coroutines.Dispatchers import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Test +import org.mockito.ArgumentMatchers.anyInt import org.mockito.Mock import org.wordpress.android.BaseUnitTest import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.model.LocalOrRemoteId.LocalId +import org.wordpress.android.fluxc.model.PostModel import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.model.page.PageModel import org.wordpress.android.fluxc.model.page.PageStatus import org.wordpress.android.fluxc.store.MediaStore +import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.ui.pages.PageItem import org.wordpress.android.ui.pages.PageItem.Divider import org.wordpress.android.ui.pages.PageItem.Page import org.wordpress.android.ui.pages.PageItem.PublishedPage import org.wordpress.android.util.LocaleManagerWrapper +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListState import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListType.PUBLISHED import org.wordpress.android.viewmodel.uistate.ProgressBarUiState @@ -31,10 +37,13 @@ private const val HOUR_IN_MILLISECONDS = 3600000L class PageListViewModelTest : BaseUnitTest() { @Mock lateinit var mediaStore: MediaStore + @Mock lateinit var postStore: PostStore @Mock lateinit var dispatcher: Dispatcher @Mock lateinit var pagesViewModel: PagesViewModel @Mock lateinit var localeManagerWrapper: LocaleManagerWrapper @Mock lateinit var progressHelper: PageItemUploadProgressHelper + @Mock lateinit var createUploadStateUseCase: CreatePageUploadUiStateUseCase + @Mock lateinit var createLabelsUseCase: CreatePageListItemLabelsUseCase private lateinit var viewModel: PageListViewModel private val site = SiteModel() @@ -42,7 +51,10 @@ class PageListViewModelTest : BaseUnitTest() { @Before fun setUp() { viewModel = PageListViewModel( + createLabelsUseCase, + createUploadStateUseCase, mediaStore, + postStore, dispatcher, localeManagerWrapper, Dispatchers.Unconfined, @@ -58,6 +70,10 @@ class PageListViewModelTest : BaseUnitTest() { whenever(pagesViewModel.site).thenReturn(site) whenever(pagesViewModel.invalidateUploadStatus).thenReturn(invalidateUploadStatus) whenever(localeManagerWrapper.getLocale()).thenReturn(Locale.getDefault()) + whenever(postStore.getPostByLocalPostId(anyInt())).thenReturn(PostModel()) + whenever(createUploadStateUseCase.createUploadUiState(any(), any())).thenReturn( + PostUploadUiState.NothingToUpload + ) site.id = 10 pageListState.value = PageListState.DONE } @@ -211,7 +227,7 @@ class PageListViewModelTest : BaseUnitTest() { val expectedShowOverlay = true val pages = MutableLiveData>() - whenever(progressHelper.getProgressStateForPage(LocalId(0), site)).thenReturn(Pair(mock(), + whenever(progressHelper.getProgressStateForPage(anyOrNull(), anyOrNull())).thenReturn(Pair(mock(), expectedShowOverlay)) whenever(pagesViewModel.pages).thenReturn(pages) @@ -232,7 +248,7 @@ class PageListViewModelTest : BaseUnitTest() { val expectedProgressBarUiState = ProgressBarUiState.Indeterminate val pages = MutableLiveData>() - whenever(progressHelper.getProgressStateForPage(LocalId(0), site)).thenReturn( + whenever(progressHelper.getProgressStateForPage(anyOrNull(), anyOrNull())).thenReturn( Pair( expectedProgressBarUiState, true @@ -255,14 +271,17 @@ class PageListViewModelTest : BaseUnitTest() { fun `progressState is specific to each page`() { // Arrange val pages = MutableLiveData>() - whenever(progressHelper.getProgressStateForPage(LocalId(0), site)).thenReturn( + whenever(postStore.getPostByLocalPostId(0)).thenReturn(PostModel().also { it.setId(0) }) + whenever(postStore.getPostByLocalPostId(1)).thenReturn(PostModel().also { it.setId(1) }) + + whenever(progressHelper.getProgressStateForPage(argThat { this.id == 0 }, any())).thenReturn( Pair( ProgressBarUiState.Indeterminate, true ) ) - whenever(progressHelper.getProgressStateForPage(LocalId(1), site)).thenReturn( + whenever(progressHelper.getProgressStateForPage(argThat { this.id == 1 }, any())).thenReturn( Pair( ProgressBarUiState.Hidden, false diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/SearchListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/SearchListViewModelTest.kt index 3508765ade80..a712d2af75e7 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/SearchListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/SearchListViewModelTest.kt @@ -10,14 +10,17 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers import org.mockito.Mock import org.mockito.junit.MockitoJUnitRunner import org.wordpress.android.R.string import org.wordpress.android.TEST_SCOPE +import org.wordpress.android.fluxc.model.PostModel import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.model.page.PageModel import org.wordpress.android.fluxc.model.page.PageStatus.DRAFT import org.wordpress.android.fluxc.model.page.PageStatus.PUBLISHED +import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.ui.pages.PageItem import org.wordpress.android.ui.pages.PageItem.Action.VIEW_PAGE import org.wordpress.android.ui.pages.PageItem.Divider @@ -25,6 +28,7 @@ import org.wordpress.android.ui.pages.PageItem.DraftPage import org.wordpress.android.ui.pages.PageItem.Empty import org.wordpress.android.ui.pages.PageItem.PublishedPage import org.wordpress.android.viewmodel.ResourceProvider +import org.wordpress.android.viewmodel.pages.CreatePageUploadUiStateUseCase.PostUploadUiState import org.wordpress.android.viewmodel.pages.PageListViewModel.PageListType import org.wordpress.android.viewmodel.uistate.ProgressBarUiState import java.util.Date @@ -39,6 +43,8 @@ class SearchListViewModelTest { @Mock lateinit var site: SiteModel @Mock lateinit var pagesViewModel: PagesViewModel @Mock lateinit var progressHelper: PageItemUploadProgressHelper + @Mock lateinit var createUploadStateUseCase: CreatePageUploadUiStateUseCase + @Mock lateinit var postStore: PostStore private lateinit var searchPages: MutableLiveData>> private lateinit var viewModel: SearchListViewModel @@ -48,7 +54,7 @@ class SearchListViewModelTest { @Before fun setUp() { page = PageModel(site, 1, "title", PUBLISHED, Date(), false, 11L, null, 0) - viewModel = SearchListViewModel(resourceProvider, TEST_SCOPE, progressHelper) + viewModel = SearchListViewModel(createUploadStateUseCase, postStore, resourceProvider, TEST_SCOPE, progressHelper) searchPages = MutableLiveData() whenever(progressHelper.getProgressStateForPage(any(), any())).thenReturn( @@ -59,6 +65,10 @@ class SearchListViewModelTest { ) whenever(pagesViewModel.searchPages).thenReturn(searchPages) whenever(pagesViewModel.site).thenReturn(site) + whenever(postStore.getPostByLocalPostId(ArgumentMatchers.anyInt())).thenReturn(PostModel()) + whenever(createUploadStateUseCase.createUploadUiState(any(), any())).thenReturn( + PostUploadUiState.NothingToUpload + ) viewModel.start(pagesViewModel) } From ffac0fc81d2e47897a4e8a86b42cc948ee5ef892 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Feb 2020 12:06:45 +0100 Subject: [PATCH 07/12] Fix lint issues --- WordPress/src/main/res/values/strings.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 8f1410262993..6dbfdd1072a3 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -2571,7 +2571,6 @@ @string/post_status_pending_review - @string/post_status_draft @string/post_status_post_private @string/local_draft From cfa967f81438f8ca75c28c41be4aa40dab3e991a Mon Sep 17 00:00:00 2001 From: malinajirka Date: Tue, 11 Feb 2020 12:12:12 +0100 Subject: [PATCH 08/12] Fix ktlint --- .../android/viewmodel/pages/SearchListViewModelTest.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/SearchListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/SearchListViewModelTest.kt index a712d2af75e7..1900a75220d8 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/SearchListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/pages/SearchListViewModelTest.kt @@ -54,7 +54,13 @@ class SearchListViewModelTest { @Before fun setUp() { page = PageModel(site, 1, "title", PUBLISHED, Date(), false, 11L, null, 0) - viewModel = SearchListViewModel(createUploadStateUseCase, postStore, resourceProvider, TEST_SCOPE, progressHelper) + viewModel = SearchListViewModel( + createUploadStateUseCase, + postStore, + resourceProvider, + TEST_SCOPE, + progressHelper + ) searchPages = MutableLiveData() whenever(progressHelper.getProgressStateForPage(any(), any())).thenReturn( From 0718d72038b4d9abefa004b2c875bb9c22b2953f Mon Sep 17 00:00:00 2001 From: malinajirka Date: Wed, 12 Feb 2020 11:49:42 +0100 Subject: [PATCH 09/12] Fix PostModel error messages for pages --- .../android/ui/uploads/UploadUtils.java | 24 ++++++++++++------- .../pages/CreatePageListItemLabelsUseCase.kt | 2 +- WordPress/src/main/res/values/strings.xml | 7 ++++++ 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java index d989e742b99d..a184563c54d5 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java @@ -85,7 +85,8 @@ UiString getErrorMessageResIdFromPostError(PostStatus postStatus, boolean isPage return isPage ? new UiStringRes(R.string.error_unknown_page) : new UiStringRes(R.string.error_unknown_post); case UNKNOWN_POST_TYPE: - return new UiStringRes(R.string.error_unknown_post_type); + return isPage ? new UiStringRes(R.string.error_unknown_page_type) + : new UiStringRes(R.string.error_unknown_post_type); case UNAUTHORIZED: return isPage ? new UiStringRes(R.string.error_refresh_unauthorized_pages) : new UiStringRes(R.string.error_refresh_unauthorized_posts); @@ -97,13 +98,14 @@ UiString getErrorMessageResIdFromPostError(PostStatus postStatus, boolean isPage if (eligibleForAutoUpload) { switch (postStatus) { case PRIVATE: - return new UiStringRes(R.string.error_post_not_published_retrying_private); + return isPage ? new UiStringRes(R.string.error_page_not_published_retrying_private) + : new UiStringRes(R.string.error_post_not_published_retrying_private); case PUBLISHED: - return new UiStringRes(R.string.error_post_not_published_retrying); + return isPage ? new UiStringRes(R.string.error_page_not_published_retrying) : new UiStringRes(R.string.error_post_not_published_retrying); case SCHEDULED: - return new UiStringRes(R.string.error_post_not_scheduled_retrying); + return isPage ? new UiStringRes(R.string.error_page_not_scheduled_retrying) : new UiStringRes(R.string.error_post_not_scheduled_retrying); case PENDING: - return new UiStringRes(R.string.error_post_not_submitted_retrying); + return isPage ? new UiStringRes(R.string.error_page_not_submitted_retrying) : new UiStringRes(R.string.error_post_not_submitted_retrying); case UNKNOWN: case DRAFT: case TRASHED: @@ -112,13 +114,17 @@ UiString getErrorMessageResIdFromPostError(PostStatus postStatus, boolean isPage } else { switch (postStatus) { case PRIVATE: - return new UiStringRes(R.string.error_post_not_published_private); + return isPage ? new UiStringRes(R.string.error_page_not_published_private) + : new UiStringRes(R.string.error_post_not_published_private); case PUBLISHED: - return new UiStringRes(R.string.error_post_not_published); + return isPage ? new UiStringRes(R.string.error_page_not_published) + : new UiStringRes(R.string.error_post_not_published); case SCHEDULED: - return new UiStringRes(R.string.error_post_not_scheduled); + return isPage ? new UiStringRes(R.string.error_page_not_scheduled) + : new UiStringRes(R.string.error_post_not_scheduled); case PENDING: - return new UiStringRes(R.string.error_post_not_submitted); + return isPage ? new UiStringRes(R.string.error_page_not_submitted) + : new UiStringRes(R.string.error_post_not_submitted); case UNKNOWN: case DRAFT: case TRASHED: diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt index 327cee61be14..b8aad14d087c 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt @@ -102,7 +102,7 @@ class CreatePageListItemLabelsUseCase @Inject constructor(private val pageConfli ) uploadUiState.error.postError != null -> UploadUtils.getErrorMessageResIdFromPostError( pagePostStatus, - false, + true, uploadUiState.error.postError, uploadUiState.isEligibleForAutoUpload ) diff --git a/WordPress/src/main/res/values/strings.xml b/WordPress/src/main/res/values/strings.xml index 6dbfdd1072a3..b203ed44dda8 100644 --- a/WordPress/src/main/res/values/strings.xml +++ b/WordPress/src/main/res/values/strings.xml @@ -2600,7 +2600,14 @@ @string/error_page_not_scheduled_retrying @string/error_page_not_submitted_retrying + + We couldn\'t complete this action, and didn\'t publish this page. + We couldn\'t complete this action, and didn\'t publish this private page. + We couldn\'t complete this action, and didn\'t schedule this page. + We couldn\'t complete this action, and didn\'t submit this page for review. + @string/error_media_recover_post + Unknown page format Post author From 127b0db81dc2eb9e934e50d1bc3f6b59f5c13120 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Wed, 12 Feb 2020 11:51:47 +0100 Subject: [PATCH 10/12] Rename some parameters in CreatePageListItemLabelsUseCase --- .../pages/CreatePageListItemLabelsUseCase.kt | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt index b8aad14d087c..b12614f5d9b9 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/pages/CreatePageListItemLabelsUseCase.kt @@ -28,19 +28,19 @@ import javax.inject.Inject * Most of this code has been copied from PostListItemUIStateHelper. */ class CreatePageListItemLabelsUseCase @Inject constructor(private val pageConflictResolver: PageConflictResolver) { - fun createLabels(pagePostModel: PostModel, uploadUiState: PostUploadUiState): List { + fun createLabels(postModel: PostModel, uploadUiState: PostUploadUiState): List { return getLabels( - PostStatus.fromPost(pagePostModel), - pagePostModel.isLocalDraft, - pagePostModel.isLocallyChanged, + PostStatus.fromPost(postModel), + postModel.isLocalDraft, + postModel.isLocallyChanged, uploadUiState, false, // TODO use conflict resolver - pageConflictResolver.hasUnhandledAutoSave(pagePostModel) + pageConflictResolver.hasUnhandledAutoSave(postModel) ) } private fun getLabels( - pagePostStatus: PostStatus, + postStatus: PostStatus, isLocalDraft: Boolean, isLocallyChanged: Boolean, uploadUiState: PostUploadUiState, @@ -50,7 +50,7 @@ class CreatePageListItemLabelsUseCase @Inject constructor(private val pageConfli val labels: MutableList = ArrayList() when { uploadUiState is UploadFailed -> { - getErrorLabel(uploadUiState, pagePostStatus)?.let { labels.add(it) } + getErrorLabel(uploadUiState, postStatus)?.let { labels.add(it) } } uploadUiState is UploadingPost -> if (uploadUiState.isDraft) { labels.add(UiStringRes(R.string.page_uploading_draft)) @@ -84,24 +84,24 @@ class CreatePageListItemLabelsUseCase @Inject constructor(private val pageConfli } else if (isLocallyChanged) { labels.add(UiStringRes(R.string.page_local_changes)) } - if (pagePostStatus == PRIVATE) { + if (postStatus == PRIVATE) { labels.add(UiStringRes(R.string.page_status_page_private)) } - if (pagePostStatus == PENDING) { + if (postStatus == PENDING) { labels.add(UiStringRes(R.string.page_status_pending_review)) } } return labels } - private fun getErrorLabel(uploadUiState: UploadFailed, pagePostStatus: PostStatus): UiString? { + private fun getErrorLabel(uploadUiState: UploadFailed, postStatus: PostStatus): UiString? { return when { uploadUiState.error.mediaError != null -> getMediaUploadErrorLabel( uploadUiState, - pagePostStatus + postStatus ) uploadUiState.error.postError != null -> UploadUtils.getErrorMessageResIdFromPostError( - pagePostStatus, + postStatus, true, uploadUiState.error.postError, uploadUiState.isEligibleForAutoUpload @@ -120,17 +120,17 @@ class CreatePageListItemLabelsUseCase @Inject constructor(private val pageConfli private fun getMediaUploadErrorLabel( uploadUiState: UploadFailed, - pagePostStatus: PostStatus + postStatus: PostStatus ): UiStringRes { return when { - uploadUiState.isEligibleForAutoUpload -> when (pagePostStatus) { + uploadUiState.isEligibleForAutoUpload -> when (postStatus) { PUBLISHED -> UiStringRes(R.string.error_media_recover_page_not_published_retrying) PRIVATE -> UiStringRes(R.string.error_media_recover_page_not_published_retrying_private) SCHEDULED -> UiStringRes(R.string.error_media_recover_page_not_scheduled_retrying) PENDING -> UiStringRes(R.string.error_media_recover_page_not_submitted_retrying) DRAFT, TRASHED, UNKNOWN -> UiStringRes(R.string.error_generic_error_retrying) } - uploadUiState.retryWillPushChanges -> when (pagePostStatus) { + uploadUiState.retryWillPushChanges -> when (postStatus) { PUBLISHED -> UiStringRes(R.string.error_media_recover_page_not_published) PRIVATE -> UiStringRes(R.string.error_media_recover_page_not_published_private) SCHEDULED -> UiStringRes(R.string.error_media_recover_page_not_scheduled) From 5fab47a42c30813e5c5d9b11fb6596f7ba6ba334 Mon Sep 17 00:00:00 2001 From: malinajirka Date: Wed, 12 Feb 2020 11:55:16 +0100 Subject: [PATCH 11/12] Fix lint issue --- .../org/wordpress/android/ui/uploads/UploadUtils.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java index a184563c54d5..981b1c76e869 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java +++ b/WordPress/src/main/java/org/wordpress/android/ui/uploads/UploadUtils.java @@ -101,11 +101,14 @@ UiString getErrorMessageResIdFromPostError(PostStatus postStatus, boolean isPage return isPage ? new UiStringRes(R.string.error_page_not_published_retrying_private) : new UiStringRes(R.string.error_post_not_published_retrying_private); case PUBLISHED: - return isPage ? new UiStringRes(R.string.error_page_not_published_retrying) : new UiStringRes(R.string.error_post_not_published_retrying); + return isPage ? new UiStringRes(R.string.error_page_not_published_retrying) + : new UiStringRes(R.string.error_post_not_published_retrying); case SCHEDULED: - return isPage ? new UiStringRes(R.string.error_page_not_scheduled_retrying) : new UiStringRes(R.string.error_post_not_scheduled_retrying); + return isPage ? new UiStringRes(R.string.error_page_not_scheduled_retrying) + : new UiStringRes(R.string.error_post_not_scheduled_retrying); case PENDING: - return isPage ? new UiStringRes(R.string.error_page_not_submitted_retrying) : new UiStringRes(R.string.error_post_not_submitted_retrying); + return isPage ? new UiStringRes(R.string.error_page_not_submitted_retrying) + : new UiStringRes(R.string.error_post_not_submitted_retrying); case UNKNOWN: case DRAFT: case TRASHED: From 2e933f6e22158fbf2c57eca539f834cbdc2fdb6d Mon Sep 17 00:00:00 2001 From: Joel Dean Date: Wed, 12 Feb 2020 17:28:34 -0500 Subject: [PATCH 12/12] formatted page label ui logic --- .../java/org/wordpress/android/ui/pages/PageItemViewHolder.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt b/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt index db90ef22cac7..9db8eb8b1075 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/pages/PageItemViewHolder.kt @@ -82,8 +82,8 @@ sealed class PageItemViewHolder(internal val parent: ViewGroup, @LayoutRes layou time.text = DateTimeUtils.javaDateToTimeSpan(date, parent.context) .capitalizeWithLocaleWithoutLint(parent.context.currentLocale) - labels.text = page.labels.map { uiHelper.getTextOfUiString(parent.context, it) }.sorted() - .joinToString(separator = " · ") + labels.text = page.labels.map { uiHelper.getTextOfUiString(parent.context, it) } + .sorted().joinToString(separator = " · ") uiHelper.updateVisibility(labels, page.labels.isNotEmpty()) itemView.setOnClickListener { onItemTapped(page) }