From 83b1738e3727ced1a7621a90a2349c3de7b87bea Mon Sep 17 00:00:00 2001 From: malinajirka Date: Thu, 30 May 2019 15:25:59 +0200 Subject: [PATCH] AutoUpload only publishable local drafts --- .../android/ui/posts/PostUtilsWrapper.kt | 17 ++++++++ .../ui/uploads/LocalDraftUploadStarter.kt | 6 ++- .../LocalDraftUploadStarterConcurrentTest.kt | 6 +++ .../ui/uploads/LocalDraftUploadStarterTest.kt | 40 ++++++++++++++++++- 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt new file mode 100644 index 000000000000..a0296d8534fd --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostUtilsWrapper.kt @@ -0,0 +1,17 @@ +package org.wordpress.android.ui.posts + +import dagger.Reusable +import org.wordpress.android.fluxc.model.PostModel +import javax.inject.Inject + +/** + * Injectable wrapper around PostUtils. + * + * PostUtils interface is consisted of static methods, which make the client code difficult to test/mock. Main purpose + * of this wrapper is to make testing easier. + * + */ +@Reusable +class PostUtilsWrapper @Inject constructor() { + fun isPublishable(post: PostModel) = PostUtils.isPublishable(post) +} diff --git a/WordPress/src/main/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarter.kt b/WordPress/src/main/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarter.kt index 47160d97162e..b1c7a226e9fe 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarter.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarter.kt @@ -19,6 +19,7 @@ import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.fluxc.store.SiteStore import org.wordpress.android.modules.BG_THREAD import org.wordpress.android.modules.IO_THREAD +import org.wordpress.android.ui.posts.PostUtilsWrapper import org.wordpress.android.util.CrashLoggingUtils import org.wordpress.android.util.NetworkUtilsWrapper import org.wordpress.android.util.skip @@ -49,6 +50,7 @@ open class LocalDraftUploadStarter @Inject constructor( @Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher, private val uploadServiceFacade: UploadServiceFacade, private val networkUtilsWrapper: NetworkUtilsWrapper, + private val postUtilsWrapper: PostUtilsWrapper, private val connectionStatus: LiveData ) : CoroutineScope { private val job = Job() @@ -131,7 +133,9 @@ open class LocalDraftUploadStarter @Inject constructor( val postsAndPages = posts.await() + pages.await() - postsAndPages.filterNot { uploadServiceFacade.isPostUploadingOrQueued(it) } + postsAndPages + .filterNot { uploadServiceFacade.isPostUploadingOrQueued(it) } + .filter { postUtilsWrapper.isPublishable(it) } .forEach { localDraft -> uploadServiceFacade.uploadPost( context = context, diff --git a/WordPress/src/test/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarterConcurrentTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarterConcurrentTest.kt index c0b49378e145..9c13f7d9a6ca 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarterConcurrentTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarterConcurrentTest.kt @@ -17,6 +17,7 @@ import org.wordpress.android.fluxc.model.PostModel import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.PageStore import org.wordpress.android.fluxc.store.PostStore +import org.wordpress.android.ui.posts.PostUtilsWrapper import org.wordpress.android.util.NetworkUtilsWrapper /** @@ -75,6 +76,7 @@ class LocalDraftUploadStarterConcurrentTest { bgDispatcher = Dispatchers.Default, ioDispatcher = Dispatchers.IO, networkUtilsWrapper = createMockedNetworkUtilsWrapper(), + postUtilsWrapper = createMockedPostUtilsWrapper(), connectionStatus = mock(), uploadServiceFacade = uploadServiceFacade ) @@ -87,5 +89,9 @@ class LocalDraftUploadStarterConcurrentTest { fun createMockedUploadServiceFacade() = mock { on { isPostUploadingOrQueued(any()) } doReturn false } + + fun createMockedPostUtilsWrapper() = mock { + on { isPublishable(any()) } doReturn true + } } } diff --git a/WordPress/src/test/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarterTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarterTest.kt index 6568b9f6d717..c4097ee8f80b 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarterTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/uploads/LocalDraftUploadStarterTest.kt @@ -28,6 +28,7 @@ import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.PageStore import org.wordpress.android.fluxc.store.PostStore import org.wordpress.android.fluxc.store.SiteStore +import org.wordpress.android.ui.posts.PostUtilsWrapper import org.wordpress.android.util.NetworkUtilsWrapper import org.wordpress.android.viewmodel.helpers.ConnectionStatus import org.wordpress.android.viewmodel.helpers.ConnectionStatus.AVAILABLE @@ -174,6 +175,37 @@ class LocalDraftUploadStarterTest { ) } + @Test + fun `when uploading, it ignores local drafts that are not publishable`() { + // Given + val site: SiteModel = sites[1] + + val connectionStatus = createConnectionStatusLiveData(null) + val uploadServiceFacade = createMockedUploadServiceFacade() + val postUtilsWrapper = mock { + on { isPublishable(any()) } doAnswer { + // return isPublishable = false on the first post of the site + it.getArgument(0) != sitesAndPosts[site]?.get(0)!! + } + } + + val starter = createLocalDraftUploadStarter(connectionStatus, uploadServiceFacade, postUtilsWrapper) + + // When + starter.queueUploadFromSite(site) + + // Then + // subtract - 1 as we've returned isPublishable = false for the first post of the site + val expectedUploadPostExecutions = sitesAndPosts.getValue(site).size + sitesAndPages.getValue(site).size - 1 + verify(uploadServiceFacade, times(expectedUploadPostExecutions)).uploadPost( + context = any(), + post = any(), + trackAnalytics = any(), + publish = any(), + isRetry = eq(true) + ) + } + @Test fun `when uploading, it ignores local drafts that are already queued`() { // Given @@ -226,7 +258,8 @@ class LocalDraftUploadStarterTest { @UseExperimental(ExperimentalCoroutinesApi::class) private fun createLocalDraftUploadStarter( connectionStatus: LiveData, - uploadServiceFacade: UploadServiceFacade + uploadServiceFacade: UploadServiceFacade, + postUtilsWrapper: PostUtilsWrapper = createMockedPostUtilsWrapper() ) = LocalDraftUploadStarter( context = mock(), postStore = postStore, @@ -235,6 +268,7 @@ class LocalDraftUploadStarterTest { bgDispatcher = Dispatchers.Unconfined, ioDispatcher = Dispatchers.Unconfined, networkUtilsWrapper = createMockedNetworkUtilsWrapper(), + postUtilsWrapper = postUtilsWrapper, connectionStatus = connectionStatus, uploadServiceFacade = uploadServiceFacade ) @@ -250,6 +284,10 @@ class LocalDraftUploadStarterTest { } } + fun createMockedPostUtilsWrapper() = mock { + on { isPublishable(any()) } doReturn true + } + fun createMockedUploadServiceFacade() = mock { on { isPostUploadingOrQueued(any()) } doReturn false }