From 8a141f50bd78f69011b2ee0dbdd68a65015b94cf Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Thu, 16 May 2024 09:43:40 +0100 Subject: [PATCH 1/8] A minor improvement to the logic --- .../android/ui/dashboard/data/ObserveSiteOrdersState.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt index 46de65908d2..aad08a42f2e 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt @@ -48,8 +48,7 @@ class ObserveSiteOrdersState @Inject constructor( orderStore.getOrderStatusOptionsForSite(selectedSite.get()) .filter { it.statusKey != "checkout-draft" } .takeIf { it.isNotEmpty() } - ?.sumOf { it.statusCount } - ?.let { it != 0 } + ?.any { it.statusCount > 0 } } private suspend fun fetchHasOrdersFromApi(): Boolean { From aabdf6d9aca69cef90a1969c76c62d0662f799b0 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Thu, 16 May 2024 11:03:58 +0100 Subject: [PATCH 2/8] Always use order status options to infer the number of processing orders This is done by updating the logic to fetch the order status options instead of the `/wc-analytics/reports/orders/totals` --- .../ui/main/ObserveProcessingOrdersCount.kt | 80 +++++++++++-------- 1 file changed, 47 insertions(+), 33 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt index 663954ee89d..aa20a0b9b2b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt @@ -1,38 +1,50 @@ package com.woocommerce.android.ui.main -import com.woocommerce.android.analytics.AnalyticsEvent -import com.woocommerce.android.analytics.AnalyticsTracker +import com.woocommerce.android.OnChangedException import com.woocommerce.android.extensions.NotificationReceivedEvent import com.woocommerce.android.notifications.NotificationChannelType import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.util.CoroutineDispatchers import com.woocommerce.android.util.WooLog +import com.woocommerce.android.util.dispatchAndAwait import com.woocommerce.android.util.observeEvents import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.transformLatest +import kotlinx.coroutines.withContext import org.greenrobot.eventbus.EventBus import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.action.WCOrderAction +import org.wordpress.android.fluxc.generated.WCOrderActionBuilder import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.network.rest.wpcom.wc.order.CoreOrderStatus import org.wordpress.android.fluxc.store.WCOrderStore +import org.wordpress.android.fluxc.store.WCOrderStore.FetchOrderStatusOptionsPayload import org.wordpress.android.fluxc.store.WCOrderStore.OnOrderChanged import org.wordpress.android.fluxc.store.WCOrderStore.OnOrderStatusOptionsChanged -import org.wordpress.android.fluxc.store.WCOrderStore.OrdersCountResult.Failure -import org.wordpress.android.fluxc.store.WCOrderStore.OrdersCountResult.Success import javax.inject.Inject class ObserveProcessingOrdersCount @Inject constructor( private val dispatcher: Dispatcher, private val wcOrderStore: WCOrderStore, - private val selectedSite: SelectedSite + private val selectedSite: SelectedSite, + private val dispatchers: CoroutineDispatchers ) { - @OptIn(ExperimentalCoroutinesApi::class) + companion object { + // A debounce duration to avoid fetching the value multiple times when there are multiple simultaneous events + private const val DEBOUNCE_DURATION_MS = 200L + } + + @OptIn(ExperimentalCoroutinesApi::class, FlowPreview::class) operator fun invoke(): Flow = selectedSite.observe().transformLatest { site -> if (site == null) { emit(null) @@ -42,18 +54,13 @@ class ObserveProcessingOrdersCount @Inject constructor( // Start with the cached value emit(getCachedValue(site)) - // emit updated value - fetchOrdersCount(site)?.let { emit(it) } + // Fetch value from API + fetchOrderStatusOptions(site) // Observe value changes coroutineScope { - dispatcher.observeEvents() - .onEach { - emit(getCachedValue(site)) - } - .launchIn(this) - merge( + wcOrderStore.observeOrderCountForSite(site), dispatcher.observeEvents() .filter { @Suppress("DEPRECATION") @@ -70,35 +77,42 @@ class ObserveProcessingOrdersCount @Inject constructor( WooLog.d(WooLog.T.ORDERS, "New order notification received, re-check unfilled orders count") } ) + .debounce(DEBOUNCE_DURATION_MS) .onEach { - fetchOrdersCount(site)?.let { emit(it) } + // Fetch value from API, the value will be emitted when OnOrderStatusOptionsChanged event + // is received below + fetchOrderStatusOptions(site) } .launchIn(this) + + emitAll( + dispatcher.observeEvents() + .map { getCachedValue(site) } + ) } } - private fun getCachedValue(site: SiteModel): Int? = + private suspend fun getCachedValue(site: SiteModel): Int? = withContext(dispatchers.io) { wcOrderStore.getOrderStatusForSiteAndKey(site, CoreOrderStatus.PROCESSING.value)?.statusCount + } - private suspend fun fetchOrdersCount(site: SiteModel): Int? { - return wcOrderStore.fetchOrdersCount(site, CoreOrderStatus.PROCESSING.value).let { - when (it) { - is Success -> { - AnalyticsTracker.track( - AnalyticsEvent.UNFULFILLED_ORDERS_LOADED, - mapOf(AnalyticsTracker.KEY_HAS_UNFULFILLED_ORDERS to it.count) - ) - it.count - } + private suspend fun fetchOrderStatusOptions(site: SiteModel): Result { + val event: OnOrderStatusOptionsChanged = dispatcher.dispatchAndAwait( + WCOrderActionBuilder.newFetchOrderStatusOptionsAction( + FetchOrderStatusOptionsPayload(site) + ) + ) - is Failure -> { - WooLog.e( - WooLog.T.ORDERS, - "Error fetching a count of orders waiting to be fulfilled: ${it.error.message}" - ) - null - } + return when { + event.isError -> { + WooLog.e( + WooLog.T.ORDERS, + "Error fetching order status options: ${event.error.message}" + ) + Result.failure(OnChangedException(event.error)) } + + else -> Result.success(Unit) } } } From f7af83ea6e2c7e2f9ecf93bf258839d6153075b1 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Thu, 16 May 2024 11:10:11 +0100 Subject: [PATCH 3/8] Update logic of ObserveSiteOrderState to observe status options changes --- .../dashboard/data/ObserveSiteOrdersState.kt | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt index aad08a42f2e..9004f037359 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt @@ -4,22 +4,27 @@ import com.woocommerce.android.R import com.woocommerce.android.model.DashboardWidget import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.util.CoroutineDispatchers +import com.woocommerce.android.util.observeEvents import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.transformLatest -import kotlinx.coroutines.withContext +import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.store.WCOrderStore import org.wordpress.android.fluxc.store.WCOrderStore.HasOrdersResult +import org.wordpress.android.fluxc.store.WCOrderStore.OnOrderStatusOptionsChanged import javax.inject.Inject @OptIn(ExperimentalCoroutinesApi::class) class ObserveSiteOrdersState @Inject constructor( private val selectedSite: SelectedSite, private val orderStore: WCOrderStore, - private val coroutineDispatchers: CoroutineDispatchers + private val coroutineDispatchers: CoroutineDispatchers, + private val dispatcher: Dispatcher ) { operator fun invoke() = selectedSite.observe() .filterNotNull() @@ -30,7 +35,12 @@ class ObserveSiteOrdersState @Inject constructor( if (!hasOrders) { // This means either the store doesn't have orders, or no orders are cached yet // Use other approaches to determine if the store has orders - emit(getHasOrdersFromOrderStatusOptions() ?: fetchHasOrdersFromApi()) + + emitAll( + observeValueFromOrdersStatusOptions().map { + it ?: fetchHasOrdersFromApi() + } + ) } else { emit(true) } @@ -44,12 +54,15 @@ class ObserveSiteOrdersState @Inject constructor( } } - private suspend fun getHasOrdersFromOrderStatusOptions() = withContext(coroutineDispatchers.io) { - orderStore.getOrderStatusOptionsForSite(selectedSite.get()) - .filter { it.statusKey != "checkout-draft" } - .takeIf { it.isNotEmpty() } - ?.any { it.statusCount > 0 } - } + private fun observeValueFromOrdersStatusOptions() = dispatcher.observeEvents() + .map { + orderStore.getOrderStatusOptionsForSite(selectedSite.get()) + .filter { it.statusKey != "checkout-draft" } + .takeIf { it.isNotEmpty() } + ?.any { it.statusCount > 0 } + } + .distinctUntilChanged() + .flowOn(coroutineDispatchers.io) private suspend fun fetchHasOrdersFromApi(): Boolean { return orderStore.fetchHasOrders(selectedSite.get(), null).let { From cf38b03abba129d6ec335f303629012978c87290 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Thu, 16 May 2024 11:32:08 +0100 Subject: [PATCH 4/8] Remove redundant fetch We already do a fetch when the app starts in the SiteObserver, so here we should just use the cached value initially, and then observe changes --- .../android/ui/main/ObserveProcessingOrdersCount.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt index aa20a0b9b2b..5815da806b7 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt @@ -54,9 +54,6 @@ class ObserveProcessingOrdersCount @Inject constructor( // Start with the cached value emit(getCachedValue(site)) - // Fetch value from API - fetchOrderStatusOptions(site) - // Observe value changes coroutineScope { merge( From 2e8a72f7f58b6de42c8392719f0bfa69935ce8eb Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Thu, 16 May 2024 11:47:57 +0100 Subject: [PATCH 5/8] Improve FakeDispatcher to allow executing a callback when dispatching --- .../test/kotlin/com/woocommerce/android/FakeDispatcher.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/FakeDispatcher.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/FakeDispatcher.kt index e7b13cee733..b6eee604d9d 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/FakeDispatcher.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/FakeDispatcher.kt @@ -4,7 +4,7 @@ import org.greenrobot.eventbus.Subscribe import org.wordpress.android.fluxc.Dispatcher import org.wordpress.android.fluxc.annotations.action.Action -class FakeDispatcher : Dispatcher() { +class FakeDispatcher(private val dispatchCallback: Dispatcher.(action: Action<*>) -> Unit = {}) : Dispatcher() { private val listeners = mutableListOf() @Synchronized @@ -31,7 +31,7 @@ class FakeDispatcher : Dispatcher() { } } - override fun dispatch(action: Action<*>?) { - // NO-OP + override fun dispatch(action: Action<*>) { + dispatchCallback(action) } } From 4be19aba3ea878a7fcb2130a1c4eea5ef5ab53b0 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Thu, 16 May 2024 11:52:40 +0100 Subject: [PATCH 6/8] Fix unit tests --- .../ui/main/ObserveProcessingOrdersCount.kt | 4 +- .../main/ObserveProcessingOrdersCountTest.kt | 102 ++++++++++++------ 2 files changed, 74 insertions(+), 32 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt index 5815da806b7..91f3c5b8d41 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.launchIn @@ -57,7 +58,8 @@ class ObserveProcessingOrdersCount @Inject constructor( // Observe value changes coroutineScope { merge( - wcOrderStore.observeOrderCountForSite(site), + wcOrderStore.observeOrderCountForSite(site) + .distinctUntilChanged(), dispatcher.observeEvents() .filter { @Suppress("DEPRECATION") diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCountTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCountTest.kt index 9372ffb4d1d..3f1c27146e1 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCountTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCountTest.kt @@ -8,9 +8,12 @@ import com.woocommerce.android.viewmodel.BaseUnitTest import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.async import kotlinx.coroutines.coroutineScope -import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.advanceUntilIdle import org.assertj.core.api.Assertions.assertThat import org.greenrobot.eventbus.EventBus import org.junit.Test @@ -24,13 +27,20 @@ import org.wordpress.android.fluxc.network.rest.wpcom.wc.order.CoreOrderStatus import org.wordpress.android.fluxc.store.WCOrderStore import org.wordpress.android.fluxc.store.WCOrderStore.OnOrderChanged import org.wordpress.android.fluxc.store.WCOrderStore.OnOrderStatusOptionsChanged -import org.wordpress.android.fluxc.store.WCOrderStore.OrdersCountResult @OptIn(ExperimentalCoroutinesApi::class) class ObserveProcessingOrdersCountTest : BaseUnitTest() { - private val dispatcher = FakeDispatcher() - private val orderStore: WCOrderStore = mock() + private val dispatcher = FakeDispatcher { action -> + if (action.type == WCOrderAction.FETCH_ORDER_STATUS_OPTIONS) { + emitChange( + OnOrderStatusOptionsChanged(0) + ) + } + } private val site = SiteModel() + private val orderStore: WCOrderStore = mock { + on { observeOrderCountForSite(site) } doReturn emptyFlow() + } private val selectedSite: SelectedSite = mock { on { observe() } doReturn flowOf(site) } @@ -38,7 +48,8 @@ class ObserveProcessingOrdersCountTest : BaseUnitTest() { private val sut = ObserveProcessingOrdersCount( dispatcher = dispatcher, wcOrderStore = orderStore, - selectedSite = selectedSite + selectedSite = selectedSite, + dispatchers = coroutinesTestRule.testDispatchers ) @Test @@ -52,9 +63,6 @@ class ObserveProcessingOrdersCountTest : BaseUnitTest() { @Test fun `when observation begins, then emit the cached count`() = testBlocking { - whenever( - orderStore.fetchOrdersCount(site, CoreOrderStatus.PROCESSING.value) - ).thenReturn(OrdersCountResult.Success(2)) whenever( orderStore.getOrderStatusForSiteAndKey(site, CoreOrderStatus.PROCESSING.value) ).thenReturn( @@ -68,23 +76,8 @@ class ObserveProcessingOrdersCountTest : BaseUnitTest() { assertThat(count).isEqualTo(1) } - @Test - fun `when observation begins, then fetch orders count from API`() = testBlocking { - whenever( - orderStore.fetchOrdersCount(site, CoreOrderStatus.PROCESSING.value) - ).thenReturn(OrdersCountResult.Success(1)) - - val count = sut.invoke().drop(1).first() - - assertThat(count).isEqualTo(1) - } - @Test fun `when order statuses are fetched, then re-emit new count`() = testBlocking { - whenever( - orderStore.fetchOrdersCount(site, CoreOrderStatus.PROCESSING.value) - ).thenReturn(OrdersCountResult.Success(1)) - whenever( orderStore.getOrderStatusForSiteAndKey(site, CoreOrderStatus.PROCESSING.value) ).thenReturn( @@ -105,10 +98,18 @@ class ObserveProcessingOrdersCountTest : BaseUnitTest() { } @Test - fun `when push notification is received, then re-fetch orders count`() = testBlocking { - whenever(orderStore.fetchOrdersCount(site, CoreOrderStatus.PROCESSING.value)) - .thenReturn(OrdersCountResult.Success(1)) - .thenReturn(OrdersCountResult.Success(2)) + fun `when push notification is received, then re-fetch orders status options`() = testBlocking { + whenever( + orderStore.getOrderStatusForSiteAndKey(site, CoreOrderStatus.PROCESSING.value) + ).thenReturn( + WCOrderStatusModel(0).apply { + statusCount = 1 + } + ).thenReturn( + WCOrderStatusModel(0).apply { + statusCount = 2 + } + ) val count = runAndReturnLastValue { EventBus.getDefault().post( @@ -117,20 +118,59 @@ class ObserveProcessingOrdersCountTest : BaseUnitTest() { channel = NotificationChannelType.NEW_ORDER ) ) + advanceUntilIdle() } assertThat(count).isEqualTo(2) } @Test - fun `when an order status is updated, then re-fetch orders count`() = testBlocking { - whenever(orderStore.fetchOrdersCount(site, CoreOrderStatus.PROCESSING.value)) - .thenReturn(OrdersCountResult.Success(1)) - .thenReturn(OrdersCountResult.Success(2)) + fun `when an order status is updated, then re-fetch orders status options`() = testBlocking { + whenever( + orderStore.getOrderStatusForSiteAndKey(site, CoreOrderStatus.PROCESSING.value) + ).thenReturn( + WCOrderStatusModel(0).apply { + statusCount = 1 + } + ).thenReturn( + WCOrderStatusModel(0).apply { + statusCount = 2 + } + ) val count = runAndReturnLastValue { @Suppress("DEPRECATION") dispatcher.emitChange(OnOrderChanged(causeOfChange = WCOrderAction.UPDATE_ORDER_STATUS)) + advanceUntilIdle() + } + + assertThat(count).isEqualTo(2) + } + + @Test + fun `when orders count change, then re-fetch orders status options`() = testBlocking { + whenever(orderStore.observeOrderCountForSite(site)) + .thenReturn( + flow { + emit(1) + delay(1000) + emit(2) + } + ) + whenever( + orderStore.getOrderStatusForSiteAndKey(site, CoreOrderStatus.PROCESSING.value) + ).thenReturn( + WCOrderStatusModel(0).apply { + statusCount = 1 + } + ).thenReturn( + WCOrderStatusModel(0).apply { + statusCount = 2 + } + ) + + val count = runAndReturnLastValue { + advanceUntilIdle() } assertThat(count).isEqualTo(2) From bdac5969237ad156aed7eb1219c787ec45efb720 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Fri, 17 May 2024 09:59:53 +0100 Subject: [PATCH 7/8] A minor improvement to avoid redundant API call We skip the initial orders count, with this we don't make a redundant API call to fetch the order status options, something that SiteObserver already handles. --- .../woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt index 91f3c5b8d41..a9d69210708 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/main/ObserveProcessingOrdersCount.kt @@ -14,6 +14,7 @@ import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.launchIn @@ -59,6 +60,7 @@ class ObserveProcessingOrdersCount @Inject constructor( coroutineScope { merge( wcOrderStore.observeOrderCountForSite(site) + .drop(1) .distinctUntilChanged(), dispatcher.observeEvents() .filter { From 73e7b31772f4563a1bba097257f39c237edb88eb Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Fri, 17 May 2024 11:20:22 +0100 Subject: [PATCH 8/8] Remove usage of `observeOrdersCount` for detecting if store has orders The function checks only the local DB, and given how pagination works, we don't remove trashed orders from the API, so when a site has trashed orders, we keep thinking that they have valid orders, and we keep showing the stats cards. --- .../dashboard/data/ObserveSiteOrdersState.kt | 47 ++++++++----------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt index 9004f037359..abb1b5163f8 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/dashboard/data/ObserveSiteOrdersState.kt @@ -7,13 +7,13 @@ import com.woocommerce.android.util.CoroutineDispatchers import com.woocommerce.android.util.observeEvents import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map -import kotlinx.coroutines.flow.transformLatest +import kotlinx.coroutines.flow.onStart import org.wordpress.android.fluxc.Dispatcher +import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.store.WCOrderStore import org.wordpress.android.fluxc.store.WCOrderStore.HasOrdersResult import org.wordpress.android.fluxc.store.WCOrderStore.OnOrderStatusOptionsChanged @@ -28,23 +28,12 @@ class ObserveSiteOrdersState @Inject constructor( ) { operator fun invoke() = selectedSite.observe() .filterNotNull() - .flatMapLatest { orderStore.observeOrderCountForSite(it) } - .map { count -> count != 0 } - .distinctUntilChanged() - .transformLatest { hasOrders -> - if (!hasOrders) { - // This means either the store doesn't have orders, or no orders are cached yet - // Use other approaches to determine if the store has orders - - emitAll( - observeValueFromOrdersStatusOptions().map { - it ?: fetchHasOrdersFromApi() - } - ) - } else { - emit(true) - } - }.map { hasOrders -> + .flatMapLatest { observeHasOrdersFromOrdersStatusOptions(it) } + .map { + // Fallback to fetching from API if we can't infer the value from the order status options + it ?: fetchHasOrdersFromApi() + } + .map { hasOrders -> if (hasOrders) { DashboardWidget.Status.Available } else { @@ -54,15 +43,17 @@ class ObserveSiteOrdersState @Inject constructor( } } - private fun observeValueFromOrdersStatusOptions() = dispatcher.observeEvents() - .map { - orderStore.getOrderStatusOptionsForSite(selectedSite.get()) - .filter { it.statusKey != "checkout-draft" } - .takeIf { it.isNotEmpty() } - ?.any { it.statusCount > 0 } - } - .distinctUntilChanged() - .flowOn(coroutineDispatchers.io) + private fun observeHasOrdersFromOrdersStatusOptions(site: SiteModel) = + dispatcher.observeEvents() + .onStart { emit(OnOrderStatusOptionsChanged(0)) } + .map { + orderStore.getOrderStatusOptionsForSite(site) + .filter { it.statusKey != "checkout-draft" } + .takeIf { it.isNotEmpty() } + ?.any { it.statusCount > 0 } + } + .distinctUntilChanged() + .flowOn(coroutineDispatchers.io) private suspend fun fetchHasOrdersFromApi(): Boolean { return orderStore.fetchHasOrders(selectedSite.get(), null).let {