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

AutoUpload only publishable local drafts #9962

Merged
merged 1 commit into from
May 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<ConnectionStatus>
) : CoroutineScope {
private val job = Job()
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -75,6 +76,7 @@ class LocalDraftUploadStarterConcurrentTest {
bgDispatcher = Dispatchers.Default,
ioDispatcher = Dispatchers.IO,
networkUtilsWrapper = createMockedNetworkUtilsWrapper(),
postUtilsWrapper = createMockedPostUtilsWrapper(),
connectionStatus = mock(),
uploadServiceFacade = uploadServiceFacade
)
Expand All @@ -87,5 +89,9 @@ class LocalDraftUploadStarterConcurrentTest {
fun createMockedUploadServiceFacade() = mock<UploadServiceFacade> {
on { isPostUploadingOrQueued(any()) } doReturn false
}

fun createMockedPostUtilsWrapper() = mock<PostUtilsWrapper> {
on { isPublishable(any()) } doReturn true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<PostUtilsWrapper> {
on { isPublishable(any()) } doAnswer {
// return isPublishable = false on the first post of the site
it.getArgument<PostModel>(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
Expand Down Expand Up @@ -226,7 +258,8 @@ class LocalDraftUploadStarterTest {
@UseExperimental(ExperimentalCoroutinesApi::class)
private fun createLocalDraftUploadStarter(
connectionStatus: LiveData<ConnectionStatus>,
uploadServiceFacade: UploadServiceFacade
uploadServiceFacade: UploadServiceFacade,
postUtilsWrapper: PostUtilsWrapper = createMockedPostUtilsWrapper()
) = LocalDraftUploadStarter(
context = mock(),
postStore = postStore,
Expand All @@ -235,6 +268,7 @@ class LocalDraftUploadStarterTest {
bgDispatcher = Dispatchers.Unconfined,
ioDispatcher = Dispatchers.Unconfined,
networkUtilsWrapper = createMockedNetworkUtilsWrapper(),
postUtilsWrapper = postUtilsWrapper,
connectionStatus = connectionStatus,
uploadServiceFacade = uploadServiceFacade
)
Expand All @@ -250,6 +284,10 @@ class LocalDraftUploadStarterTest {
}
}

fun createMockedPostUtilsWrapper() = mock<PostUtilsWrapper> {
on { isPublishable(any()) } doReturn true
}

fun createMockedUploadServiceFacade() = mock<UploadServiceFacade> {
on { isPostUploadingOrQueued(any()) } doReturn false
}
Expand Down