From 358764a0ae537329f0947ed7adf1ee64d950763b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Thu, 6 Nov 2025 14:32:26 +0100 Subject: [PATCH 1/7] Fetch order refunds if necessary --- .../com/woocommerce/android/model/Order.kt | 2 +- .../data/WooPosGetOrderRefundsByOrderId.kt | 21 --- .../common/data/WooPosRetrieveOrderRefunds.kt | 41 +++++ .../ui/woopos/orders/WooPosOrdersViewModel.kt | 6 +- .../WooPosGetOrderRefundsByOrderIdTest.kt | 102 ------------ .../data/WooPosRetrieveOrderRefundsTest.kt | 153 ++++++++++++++++++ .../WooPosLocalCatalogSyncWorkerTest.kt | 5 +- .../orders/WooPosOrdersViewModelTest.kt | 12 +- 8 files changed, 208 insertions(+), 134 deletions(-) delete mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetOrderRefundsByOrderId.kt create mode 100644 WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt delete mode 100644 WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetOrderRefundsByOrderIdTest.kt create mode 100644 WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt index 0ae3c11c95ea..651075821892 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt @@ -28,7 +28,7 @@ data class Order( val totalTax: BigDecimal, val shippingTotal: BigDecimal, val discountTotal: BigDecimal, - val refundTotal: BigDecimal, + var refundTotal: BigDecimal, val currency: String, val orderKey: String, val customerNote: String, diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetOrderRefundsByOrderId.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetOrderRefundsByOrderId.kt deleted file mode 100644 index a630e7989bb1..000000000000 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetOrderRefundsByOrderId.kt +++ /dev/null @@ -1,21 +0,0 @@ -package com.woocommerce.android.ui.woopos.common.data - -import com.woocommerce.android.model.Refund -import com.woocommerce.android.model.toAppModel -import com.woocommerce.android.tools.SelectedSite -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.withContext -import org.wordpress.android.fluxc.store.WCRefundStore -import javax.inject.Inject - -class WooPosGetOrderRefundsByOrderId @Inject constructor( - private val refundStore: WCRefundStore, - private val selectedSite: SelectedSite -) { - suspend operator fun invoke(orderId: Long): List { - return withContext(Dispatchers.IO) { - refundStore.getAllRefunds(selectedSite.get(), orderId) - .map { it.toAppModel() } - } - } -} diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt new file mode 100644 index 000000000000..314fda767df3 --- /dev/null +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt @@ -0,0 +1,41 @@ +package com.woocommerce.android.ui.woopos.common.data + +import com.woocommerce.android.model.Order +import com.woocommerce.android.model.Refund +import com.woocommerce.android.model.toAppModel +import com.woocommerce.android.tools.SelectedSite +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import org.wordpress.android.fluxc.store.WCRefundStore +import java.math.BigDecimal +import javax.inject.Inject + +/** + * Use case that retrieves all refunds for the given [Order]. + * + * - First attempts to load refunds from the local store. + * - If none are found, fetches them from the remote source. + * - If the order's [Order.refundTotal] is zero, returns an empty list without querying any source. + * + * This optimization prevents unnecessary store or network access for non-refunded orders. + */ +class WooPosRetrieveOrderRefunds @Inject constructor( + private val refundStore: WCRefundStore, + private val selectedSite: SelectedSite +) { + suspend operator fun invoke(order: Order): List = + withContext(Dispatchers.IO) { + if (order.refundTotal.compareTo(BigDecimal.ZERO) == 0) { + return@withContext emptyList() + } + + val site = selectedSite.get() + + var refundModels = refundStore.getAllRefunds(site, order.id) + if (refundModels.isEmpty()) { + refundModels = refundStore.fetchAllRefunds(site, order.id).model ?: emptyList() + } + + refundModels.map { it.toAppModel() } + } +} diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt index 94869534319a..90e53bf53de4 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt @@ -7,8 +7,8 @@ import com.woocommerce.android.R import com.woocommerce.android.model.Order import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosSearchInputState import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosSearchUIEvent -import com.woocommerce.android.ui.woopos.common.data.WooPosGetOrderRefundsByOrderId import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById +import com.woocommerce.android.ui.woopos.common.data.WooPosRetrieveOrderRefunds import com.woocommerce.android.ui.woopos.home.ChildToParentEvent.NavigationEvent.ToEmailReceipt import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender import com.woocommerce.android.ui.woopos.home.items.WooPosPaginationState @@ -42,7 +42,7 @@ class WooPosOrdersViewModel @Inject constructor( private val getProductById: WooPosGetProductById, private val childrenToParentEventSender: WooPosChildrenToParentEventSender, private val formatPrice: WooPosFormatPrice, - private val getOrderRefunds: WooPosGetOrderRefundsByOrderId, + private val retrieveOrderRefunds: WooPosRetrieveOrderRefunds, private val ordersAnalyticsTracker: WooPosOrdersAnalyticsTracker ) : ViewModel() { @@ -542,7 +542,7 @@ class WooPosOrdersViewModel @Inject constructor( val discountCode = order.couponLines.firstOrNull()?.code - val refundsDeferred = async { getOrderRefunds(order.id) } + val refundsDeferred = async { retrieveOrderRefunds(order) } val lineItems = lineItemsDeferred.awaitAll() val refunds = refundsDeferred.await() diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetOrderRefundsByOrderIdTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetOrderRefundsByOrderIdTest.kt deleted file mode 100644 index 54be743dfdc8..000000000000 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosGetOrderRefundsByOrderIdTest.kt +++ /dev/null @@ -1,102 +0,0 @@ -package com.woocommerce.android.ui.woopos.common.data - -import com.woocommerce.android.tools.SelectedSite -import kotlinx.coroutines.test.runTest -import org.assertj.core.api.Assertions.assertThat -import org.junit.Before -import org.junit.Test -import org.mockito.kotlin.any -import org.mockito.kotlin.mock -import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever -import org.wordpress.android.fluxc.model.SiteModel -import org.wordpress.android.fluxc.model.refunds.WCRefundModel -import org.wordpress.android.fluxc.store.WCRefundStore -import java.math.BigDecimal -import java.util.Date - -class WooPosGetOrderRefundsByOrderIdTest { - private lateinit var refundStore: WCRefundStore - private lateinit var selectedSite: SelectedSite - private lateinit var sut: WooPosGetOrderRefundsByOrderId - private lateinit var site: SiteModel - - @Before - fun setup() { - refundStore = mock() - selectedSite = mock() - site = mock() - - whenever(selectedSite.get()).thenReturn(site) - - sut = WooPosGetOrderRefundsByOrderId( - refundStore = refundStore, - selectedSite = selectedSite - ) - } - - @Test - fun `given refunds exist, when invoke called, then returns mapped refunds`() = runTest { - // GIVEN - val orderId = 123L - val fluxCRefunds = listOf( - WCRefundModel( - id = 1L, - dateCreated = Date(), - amount = BigDecimal.TEN, - reason = "Test refund", - automaticGatewayRefund = true, - items = emptyList(), - shippingLineItems = emptyList(), - feeLineItems = emptyList() - ), - WCRefundModel( - id = 2L, - dateCreated = Date(), - amount = BigDecimal.valueOf(5), - reason = "Another refund", - automaticGatewayRefund = false, - items = emptyList(), - shippingLineItems = emptyList(), - feeLineItems = emptyList() - ) - ) - whenever(refundStore.getAllRefunds(site, orderId)).thenReturn(fluxCRefunds) - - // WHEN - val result = sut.invoke(orderId) - - // THEN - assertThat(result).hasSize(2) - assertThat(result[0].id).isEqualTo(1L) - assertThat(result[0].amount).isEqualTo(BigDecimal.TEN) - assertThat(result[1].id).isEqualTo(2L) - assertThat(result[1].amount).isEqualTo(BigDecimal.valueOf(5)) - } - - @Test - fun `given no refunds exist, when invoke called, then returns empty list`() = runTest { - // GIVEN - val orderId = 123L - whenever(refundStore.getAllRefunds(site, orderId)).thenReturn(emptyList()) - - // WHEN - val result = sut.invoke(orderId) - - // THEN - assertThat(result).isEmpty() - } - - @Test - fun `given orderId provided, when invoke called, then passes correct orderId to store`() = runTest { - // GIVEN - val orderId = 456L - whenever(refundStore.getAllRefunds(any(), any())).thenReturn(emptyList()) - - // WHEN - sut.invoke(orderId) - - // THEN - verify(refundStore).getAllRefunds(site, orderId) - } -} diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt new file mode 100644 index 000000000000..c38cde2da31e --- /dev/null +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt @@ -0,0 +1,153 @@ +package com.woocommerce.android.ui.woopos.common.data + +import com.woocommerce.android.model.Order +import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.orders.OrderTestUtils +import kotlinx.coroutines.test.runTest +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.wordpress.android.fluxc.model.SiteModel +import org.wordpress.android.fluxc.model.refunds.WCRefundModel +import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooResult +import org.wordpress.android.fluxc.store.WCRefundStore +import java.math.BigDecimal +import java.util.Date + +class WooPosRetrieveOrderRefundsTest { + private lateinit var refundStore: WCRefundStore + private lateinit var selectedSite: SelectedSite + private lateinit var sut: WooPosRetrieveOrderRefunds + private lateinit var site: SiteModel + + @Before + fun setup() { + refundStore = mock() + selectedSite = mock() + site = mock() + + whenever(selectedSite.get()).thenReturn(site) + + sut = WooPosRetrieveOrderRefunds( + refundStore = refundStore, + selectedSite = selectedSite + ) + } + + @Test + fun `given refunds exist locally, when invoke called, then returns mapped refunds`() = runTest { + // GIVEN + val order = order(id = 123L, refundTotal = BigDecimal.TEN) + + val fluxCRefunds = listOf( + WCRefundModel( + id = 1L, + dateCreated = Date(), + amount = BigDecimal.TEN, + reason = "Test refund", + automaticGatewayRefund = true, + items = emptyList(), + shippingLineItems = emptyList(), + feeLineItems = emptyList() + ), + WCRefundModel( + id = 2L, + dateCreated = Date(), + amount = BigDecimal.valueOf(5), + reason = "Another refund", + automaticGatewayRefund = false, + items = emptyList(), + shippingLineItems = emptyList(), + feeLineItems = emptyList() + ) + ) + whenever(refundStore.getAllRefunds(site, order.id)).thenReturn(fluxCRefunds) + + // WHEN + val result = sut.invoke(order) + + // THEN + assertThat(result).hasSize(2) + assertThat(result[0].id).isEqualTo(1L) + assertThat(result[0].amount).isEqualTo(BigDecimal.TEN) + assertThat(result[1].id).isEqualTo(2L) + assertThat(result[1].amount).isEqualTo(BigDecimal.valueOf(5)) + } + + @Test + fun `given no refunds exist locally, when invoke called, then fetches and returns mapped refunds`() = runTest { + // GIVEN + val order = order(id = 123L, refundTotal = BigDecimal.TEN) + + whenever(refundStore.getAllRefunds(site, order.id)).thenReturn(emptyList()) + + val remoteRefunds = listOf( + WCRefundModel( + id = 10L, + dateCreated = Date(), + amount = BigDecimal.ONE, + reason = "Remote refund", + automaticGatewayRefund = false, + items = emptyList(), + shippingLineItems = emptyList(), + feeLineItems = emptyList() + ) + ) + whenever(refundStore.fetchAllRefunds(site, order.id)).thenReturn( + WooResult(model = remoteRefunds) + ) + + // WHEN + val result = sut.invoke(order) + + // THEN + assertThat(result).hasSize(1) + assertThat(result[0].id).isEqualTo(10L) + assertThat(result[0].amount).isEqualTo(BigDecimal.ONE) + verify(refundStore).getAllRefunds(site, order.id) + verify(refundStore).fetchAllRefunds(site, order.id) + } + + @Test + fun `given order refundTotal is zero, when invoke called, then returns empty list and does not hit store`() = runTest { + // GIVEN + val order = order(id = 999L, refundTotal = BigDecimal.ZERO) + + // WHEN + val result = sut.invoke(order) + + // THEN + assertThat(result).isEmpty() + // no calls to store + verify(refundStore, org.mockito.kotlin.never()).getAllRefunds(any(), any()) + } + + @Test + fun `given order provided, when invoke called, then passes correct orderId to store`() = runTest { + // GIVEN + val order = order(id = 456L, refundTotal = BigDecimal.ONE) + whenever(refundStore.getAllRefunds(any(), any())).thenReturn(emptyList()) + whenever(refundStore.fetchAllRefunds(any(), any(), any(), any())).thenReturn( + WooResult(emptyList()) + ) + + // WHEN + sut.invoke(order) + + // THEN + verify(refundStore).getAllRefunds(site, order.id) + } + + private fun order( + id: Long, + refundTotal: BigDecimal + ): Order { + val order = OrderTestUtils.generateTestOrder(orderId = id) + order.refundTotal = refundTotal + return order + } +} diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt index 6eabdd9622aa..c3d1a41536b0 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt @@ -333,8 +333,11 @@ class WooPosLocalCatalogSyncWorkerTest : BaseUnitTest() { @Test fun `given sync overdue, when validateSyncStatus is called, then proceeds with sync`() = testBlocking { // GIVEN + val lastSyncTimestamp = CURRENT_TIME_MILLIS - (8 * 24 * 60 * 60 * 1000L) // 8 days ago whenever(syncStatusChecker.checkSyncRequirement()) - .thenReturn(WooPosFullSyncRequirement.Overdue) + .thenReturn( + WooPosFullSyncRequirement.NonBlockingRequired(lastSyncTimestamp, isOverdue = true) + ) val worker = createWorker() diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModelTest.kt index f75171de2def..167d455c0d9a 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModelTest.kt @@ -6,8 +6,8 @@ import com.woocommerce.android.model.Refund import com.woocommerce.android.ui.orders.OrderTestUtils import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosSearchInputState import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosSearchUIEvent -import com.woocommerce.android.ui.woopos.common.data.WooPosGetOrderRefundsByOrderId import com.woocommerce.android.ui.woopos.common.data.WooPosGetProductById +import com.woocommerce.android.ui.woopos.common.data.WooPosRetrieveOrderRefunds import com.woocommerce.android.ui.woopos.home.ChildToParentEvent.NavigationEvent.ToEmailReceipt import com.woocommerce.android.ui.woopos.home.WooPosChildrenToParentEventSender import com.woocommerce.android.ui.woopos.home.items.WooPosPaginationState @@ -48,7 +48,7 @@ class WooPosOrdersViewModelTest { private val resourceProvider: ResourceProvider = mock() private val getProductById: WooPosGetProductById = mock() private val formatPrice: WooPosFormatPrice = mock() - private val getOrderRefunds: WooPosGetOrderRefundsByOrderId = mock() + private val retrieveOrderRefunds: WooPosRetrieveOrderRefunds = mock() private val providedLocale: Locale = Locale.US private val childrenToParentEventSender: WooPosChildrenToParentEventSender = mock() private val ordersAnalyticsTracker: WooPosOrdersAnalyticsTracker = mock() @@ -61,7 +61,7 @@ class WooPosOrdersViewModelTest { getProductById = getProductById, childrenToParentEventSender = childrenToParentEventSender, formatPrice = formatPrice, - getOrderRefunds = getOrderRefunds, + retrieveOrderRefunds = retrieveOrderRefunds, ordersAnalyticsTracker = ordersAnalyticsTracker ) } @@ -90,7 +90,7 @@ class WooPosOrdersViewModelTest { runBlocking { whenever(getProductById.invoke(any())).thenReturn(null) - whenever(getOrderRefunds.invoke(any())).thenReturn(emptyList()) + whenever(retrieveOrderRefunds.invoke(any())).thenReturn(emptyList()) } } @@ -648,7 +648,7 @@ class WooPosOrdersViewModelTest { flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1)))) } ) runBlocking { - whenever(getOrderRefunds.invoke(1L)).thenReturn(emptyList()) + whenever(retrieveOrderRefunds.invoke(order())).thenReturn(emptyList()) } // WHEN @@ -691,7 +691,7 @@ class WooPosOrdersViewModelTest { ) runBlocking { - whenever(getOrderRefunds.invoke(1L)).thenReturn(listOf(refund1, refund2)) + whenever(retrieveOrderRefunds.invoke(testOrder)).thenReturn(listOf(refund1, refund2)) whenever(formatPrice.invoke(java.math.BigDecimal("10.00"))).thenReturn("$10.00") whenever(formatPrice.invoke(java.math.BigDecimal("5.00"))).thenReturn("$5.00") } From 3f4a43fa4a9bb2d84d91d74505cfb562b50351d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Thu, 6 Nov 2025 14:47:03 +0100 Subject: [PATCH 2/7] Cleaner way to set refund total --- .../com/woocommerce/android/model/Order.kt | 2 +- .../android/ui/orders/OrderTestUtils.kt | 4 ++-- .../data/WooPosRetrieveOrderRefundsTest.kt | 19 ++++--------------- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt index 651075821892..0ae3c11c95ea 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Order.kt @@ -28,7 +28,7 @@ data class Order( val totalTax: BigDecimal, val shippingTotal: BigDecimal, val discountTotal: BigDecimal, - var refundTotal: BigDecimal, + val refundTotal: BigDecimal, val currency: String, val orderKey: String, val customerNote: String, diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderTestUtils.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderTestUtils.kt index a61b78f7285b..b4b1a7391b71 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderTestUtils.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderTestUtils.kt @@ -182,7 +182,7 @@ object OrderTestUtils { return result } - fun generateTestOrder(orderId: Long = 1): Order { + fun generateTestOrder(orderId: Long = 1, refundTotal: BigDecimal = -BigDecimal.TEN): Order { return Order.getEmptyOrder(Date(), Date()).copy( id = orderId, customer = Order.Customer( @@ -198,7 +198,7 @@ object OrderTestUtils { status = Order.Status.Pending, total = BigDecimal("106.00"), items = generateTestOrderItems(productId = 15), - refundTotal = -BigDecimal.TEN, + refundTotal = refundTotal, ) } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt index c38cde2da31e..d70ffad2ce23 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt @@ -1,6 +1,5 @@ package com.woocommerce.android.ui.woopos.common.data -import com.woocommerce.android.model.Order import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.orders.OrderTestUtils import kotlinx.coroutines.test.runTest @@ -41,7 +40,7 @@ class WooPosRetrieveOrderRefundsTest { @Test fun `given refunds exist locally, when invoke called, then returns mapped refunds`() = runTest { // GIVEN - val order = order(id = 123L, refundTotal = BigDecimal.TEN) + val order = OrderTestUtils.generateTestOrder(orderId = 123L) val fluxCRefunds = listOf( WCRefundModel( @@ -81,7 +80,7 @@ class WooPosRetrieveOrderRefundsTest { @Test fun `given no refunds exist locally, when invoke called, then fetches and returns mapped refunds`() = runTest { // GIVEN - val order = order(id = 123L, refundTotal = BigDecimal.TEN) + val order = OrderTestUtils.generateTestOrder(orderId = 123L) whenever(refundStore.getAllRefunds(site, order.id)).thenReturn(emptyList()) @@ -115,21 +114,20 @@ class WooPosRetrieveOrderRefundsTest { @Test fun `given order refundTotal is zero, when invoke called, then returns empty list and does not hit store`() = runTest { // GIVEN - val order = order(id = 999L, refundTotal = BigDecimal.ZERO) + val order = OrderTestUtils.generateTestOrder(orderId = 999L, refundTotal = BigDecimal.ZERO) // WHEN val result = sut.invoke(order) // THEN assertThat(result).isEmpty() - // no calls to store verify(refundStore, org.mockito.kotlin.never()).getAllRefunds(any(), any()) } @Test fun `given order provided, when invoke called, then passes correct orderId to store`() = runTest { // GIVEN - val order = order(id = 456L, refundTotal = BigDecimal.ONE) + val order = OrderTestUtils.generateTestOrder(orderId = 456L, refundTotal = BigDecimal.ONE) whenever(refundStore.getAllRefunds(any(), any())).thenReturn(emptyList()) whenever(refundStore.fetchAllRefunds(any(), any(), any(), any())).thenReturn( WooResult(emptyList()) @@ -141,13 +139,4 @@ class WooPosRetrieveOrderRefundsTest { // THEN verify(refundStore).getAllRefunds(site, order.id) } - - private fun order( - id: Long, - refundTotal: BigDecimal - ): Order { - val order = OrderTestUtils.generateTestOrder(orderId = id) - order.refundTotal = refundTotal - return order - } } From 77cc5e1845938ccc0a07502641e7db0ff557424e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Thu, 6 Nov 2025 14:48:26 +0100 Subject: [PATCH 3/7] Revert changes as it is done in trunk --- .../woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt index c3d1a41536b0..6eabdd9622aa 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosLocalCatalogSyncWorkerTest.kt @@ -333,11 +333,8 @@ class WooPosLocalCatalogSyncWorkerTest : BaseUnitTest() { @Test fun `given sync overdue, when validateSyncStatus is called, then proceeds with sync`() = testBlocking { // GIVEN - val lastSyncTimestamp = CURRENT_TIME_MILLIS - (8 * 24 * 60 * 60 * 1000L) // 8 days ago whenever(syncStatusChecker.checkSyncRequirement()) - .thenReturn( - WooPosFullSyncRequirement.NonBlockingRequired(lastSyncTimestamp, isOverdue = true) - ) + .thenReturn(WooPosFullSyncRequirement.Overdue) val worker = createWorker() From 59e0806f8bc19cd122c1f195af4ddbc84c662f6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 11 Nov 2025 15:19:29 +0100 Subject: [PATCH 4/7] Fetch refunds asynchronously --- .../common/data/WooPosRetrieveOrderRefunds.kt | 14 +- .../woopos/orders/WooPosOrdersDataSource.kt | 56 +++++-- .../ui/woopos/orders/WooPosOrdersState.kt | 3 +- .../ui/woopos/orders/WooPosOrdersViewModel.kt | 150 +++++++++++------ WooCommerce/src/main/res/values/strings.xml | 1 + .../data/WooPosRetrieveOrderRefundsTest.kt | 42 ++++- .../orders/WooPosOrdersDataSourceTest.kt | 47 ++++-- .../orders/WooPosOrdersViewModelTest.kt | 157 ++++++++++-------- 8 files changed, 318 insertions(+), 152 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt index 314fda767df3..cff71e169c50 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt @@ -23,19 +23,25 @@ class WooPosRetrieveOrderRefunds @Inject constructor( private val refundStore: WCRefundStore, private val selectedSite: SelectedSite ) { - suspend operator fun invoke(order: Order): List = + suspend operator fun invoke(order: Order): Result> = withContext(Dispatchers.IO) { if (order.refundTotal.compareTo(BigDecimal.ZERO) == 0) { - return@withContext emptyList() + return@withContext Result.success(emptyList()) } val site = selectedSite.get() var refundModels = refundStore.getAllRefunds(site, order.id) if (refundModels.isEmpty()) { - refundModels = refundStore.fetchAllRefunds(site, order.id).model ?: emptyList() + val fetchResult = refundStore.fetchAllRefunds(site, order.id) + if (fetchResult.isError) { + return@withContext Result.failure( + Exception("Failed to fetch refunds: ${fetchResult.error.message}") + ) + } + refundModels = fetchResult.model ?: emptyList() } - refundModels.map { it.toAppModel() } + Result.success(refundModels.map { it.toAppModel() }) } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSource.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSource.kt index 06267c69e44a..a9da1756bd83 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSource.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSource.kt @@ -2,8 +2,13 @@ package com.woocommerce.android.ui.woopos.orders import com.woocommerce.android.model.Order import com.woocommerce.android.model.OrderMapper +import com.woocommerce.android.model.Refund import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.woopos.common.data.WooPosRetrieveOrderRefunds import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.withContext @@ -16,21 +21,27 @@ import java.util.concurrent.atomic.AtomicInteger import javax.inject.Inject sealed class LoadOrdersResult { - data class SuccessCache(val orders: List) : LoadOrdersResult() - data class SuccessRemote(val orders: List) : LoadOrdersResult() + data class SuccessCache(val ordersWithRefunds: Map) : LoadOrdersResult() + data class SuccessRemote(val ordersWithRefunds: Map) : LoadOrdersResult() data class Error(val message: String) : LoadOrdersResult() } sealed class SearchOrdersResult { - data class Success(val orders: List) : SearchOrdersResult() + data class Success(val ordersWithRefunds: Map) : SearchOrdersResult() data class Error(val message: String) : SearchOrdersResult() } +sealed class RefundFetchResult { + data class Success(val refunds: List) : RefundFetchResult() + object Error : RefundFetchResult() +} + class WooPosOrdersDataSource @Inject constructor( private val restClient: OrderRestClient, private val selectedSite: SelectedSite, private val orderMapper: OrderMapper, - private val ordersCache: WooPosOrdersInMemoryCache + private val ordersCache: WooPosOrdersInMemoryCache, + private val retrieveOrderRefunds: WooPosRetrieveOrderRefunds ) { private val canLoadMore = AtomicBoolean(false) private val page = AtomicInteger(1) @@ -45,12 +56,16 @@ class WooPosOrdersDataSource @Inject constructor( fun loadOrders(): Flow = flow { val cached = ordersCache.getAll() - if (cached.isNotEmpty()) emit(LoadOrdersResult.SuccessCache(cached)) + if (cached.isNotEmpty()) { + val cachedWithRefunds = fetchRefundsForOrders(cached) + emit(LoadOrdersResult.SuccessCache(cachedWithRefunds)) + } val result = loadFirstPage() - result.onSuccess { - ordersCache.setAll(it) - emit(LoadOrdersResult.SuccessRemote(it)) + result.onSuccess { orders -> + ordersCache.setAll(orders) + val ordersWithRefunds = fetchRefundsForOrders(orders) + emit(LoadOrdersResult.SuccessRemote(ordersWithRefunds)) }.onFailure { emit(LoadOrdersResult.Error(it.message ?: UNKNOWN_ERROR)) } @@ -59,13 +74,20 @@ class WooPosOrdersDataSource @Inject constructor( suspend fun searchOrders(searchQuery: String): SearchOrdersResult { val result = loadFirstPage(searchQuery) return result.fold( - onSuccess = { SearchOrdersResult.Success(it) }, + onSuccess = { orders -> + val ordersWithRefunds = fetchRefundsForOrders(orders) + SearchOrdersResult.Success(ordersWithRefunds) + }, onFailure = { SearchOrdersResult.Error(it.message ?: UNKNOWN_ERROR) } ) } - suspend fun loadMore(searchQuery: String? = null): Result> = - withContext(Dispatchers.IO) { loadNextPage(searchQuery) } + suspend fun loadMore(searchQuery: String? = null): Result> = + withContext(Dispatchers.IO) { + loadNextPage(searchQuery).map { orders -> + fetchRefundsForOrders(orders) + } + } suspend fun refreshOrderById(orderId: Long): Result { val site = selectedSite.get() @@ -135,4 +157,16 @@ class WooPosOrdersDataSource @Inject constructor( private fun WCOrderStore.OrderError.toThrowable(): Throwable = Throwable("[$type] $message") + + private suspend fun fetchRefundsForOrders(orders: List): Map = + coroutineScope { + orders.map { order -> + async { + retrieveOrderRefunds(order).fold( + onSuccess = { refunds -> order to RefundFetchResult.Success(refunds) }, + onFailure = { order to RefundFetchResult.Error } + ) + } + }.awaitAll().toMap() + } } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersState.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersState.kt index 0b831df237fa..2ebe9653d98f 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersState.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersState.kt @@ -14,7 +14,8 @@ sealed class OrderDetailsViewState { @Immutable data class Lazy( override val orderId: Long, - val order: Order + val order: Order, + val refundResult: RefundFetchResult ) : OrderDetailsViewState() @Immutable diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt index 90e53bf53de4..2cee86b71a22 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt @@ -291,9 +291,14 @@ class WooPosOrdersViewModel @Inject constructor( val current = _state.value as? WooPosOrdersState.Content ?: return val loaded = current.items as? WooPosOrdersState.Content.Items.Loaded ?: return + val refundResult = retrieveOrderRefunds(updated).fold( + onSuccess = { refunds -> RefundFetchResult.Success(refunds) }, + onFailure = { RefundFetchResult.Error } + ) + val selectedId = loaded.items.keys.firstOrNull { it.isSelected }?.id val newItem = mapOrderItem(updated, selectedId) - val newDetailsViewState = mapOrderDetails(updated) + val newDetailsViewState = mapOrderDetails(updated, refundResult) val newDetails = OrderDetailsViewState.Computed( orderId = updated.id, details = newDetailsViewState @@ -353,7 +358,7 @@ class WooPosOrdersViewModel @Inject constructor( } is SearchOrdersResult.Success -> { - if (result.orders.isEmpty()) { + if (result.ordersWithRefunds.isEmpty()) { _state.value = WooPosOrdersState.Content( items = WooPosOrdersState.Content.Items.NothingFound( title = resourceProvider.getString(R.string.woopos_search_orders_empty_title), @@ -365,7 +370,7 @@ class WooPosOrdersViewModel @Inject constructor( paginationState = WooPosPaginationState.None ) } else { - replaceOrders(result.orders) + replaceOrders(result.ordersWithRefunds) } } } @@ -389,12 +394,12 @@ class WooPosOrdersViewModel @Inject constructor( } is LoadOrdersResult.SuccessCache -> { - if (result.orders.isEmpty()) { + if (result.ordersWithRefunds.isEmpty()) { _state.value = WooPosOrdersState.Loading( searchInputState = WooPosSearchInputState.Closed ) } else { - replaceOrders(result.orders) + replaceOrders(result.ordersWithRefunds) } } @@ -402,12 +407,12 @@ class WooPosOrdersViewModel @Inject constructor( val elapsedMs = mark.elapsedNow().inWholeMilliseconds ordersAnalyticsTracker.trackOrdersListFetched(elapsedMs) - if (result.orders.isEmpty()) { + if (result.ordersWithRefunds.isEmpty()) { _state.value = WooPosOrdersState.Empty( searchInputState = WooPosSearchInputState.Closed ) } else { - replaceOrders(result.orders) + replaceOrders(result.ordersWithRefunds) } } } @@ -429,17 +434,18 @@ class WooPosOrdersViewModel @Inject constructor( ?: error("Order $orderId not found in state") return when (orderDetails) { - is OrderDetailsViewState.Lazy -> mapOrderDetails(orderDetails.order) + is OrderDetailsViewState.Lazy -> mapOrderDetails(orderDetails.order, orderDetails.refundResult) is OrderDetailsViewState.Computed -> orderDetails.details } } private suspend fun replaceOrders( - orders: List, + ordersWithRefunds: Map, paginationState: WooPosPaginationState = WooPosPaginationState.None ) { + val orders = ordersWithRefunds.keys.toList() val newSelectedId = requireNotNull(orders.firstOrNull()?.id) { "Content requires at least one order" } - val items = buildItemsMap(orders, newSelectedId) + val items = buildItemsMap(ordersWithRefunds, newSelectedId) val selectedEntry = items.entries.first { (item, _) -> item.isSelected } val selectedDetails = when (val details = selectedEntry.value) { is OrderDetailsViewState.Computed -> details.details @@ -458,13 +464,13 @@ class WooPosOrdersViewModel @Inject constructor( } private suspend fun appendOrders( - orders: List, + ordersWithRefunds: Map, paginationState: WooPosPaginationState = WooPosPaginationState.None ) { val current = _state.value as WooPosOrdersState.Content val loadedItems = current.items as WooPosOrdersState.Content.Items.Loaded val currentSelectedId = loadedItems.items.entries.firstOrNull { it.key.isSelected }?.key?.id - val newItems = buildItemsMap(orders, currentSelectedId) + val newItems = buildItemsMap(ordersWithRefunds, currentSelectedId) val items = loadedItems.items + newItems _state.value = WooPosOrdersState.Content( @@ -479,17 +485,21 @@ class WooPosOrdersViewModel @Inject constructor( } private suspend fun buildItemsMap( - orders: List, + ordersWithRefunds: Map, selectedId: Long? ): Map = coroutineScope { - orders.map { order -> + ordersWithRefunds.map { (order, refundResult) -> async { val item = mapOrderItem(order, selectedId) val details: OrderDetailsViewState = if (order.id == selectedId) { - val fullDetails = mapOrderDetails(order) + val fullDetails = mapOrderDetails(order, refundResult) OrderDetailsViewState.Computed(orderId = order.id, details = fullDetails) } else { - OrderDetailsViewState.Lazy(orderId = order.id, order = order) + OrderDetailsViewState.Lazy( + orderId = order.id, + order = order, + refundResult = refundResult + ) } item to details @@ -518,17 +528,52 @@ class WooPosOrdersViewModel @Inject constructor( ) } - private suspend fun mapOrderDetails(order: Order): OrderDetailsViewState.Computed.Details = coroutineScope { - val statusText = order.status.localizedLabel(resourceProvider, locale) + // inside WooPosOrdersViewModel + + private suspend fun mapOrderDetails( + order: Order, + refundResult: RefundFetchResult + ): OrderDetailsViewState.Computed.Details = coroutineScope { + val status = mapOrderStatus(order) + val lineItems = buildLineItems(order) + val refundInfo = buildRefundInfo(order, refundResult) + val breakdown = buildTotalsBreakdown(order, refundInfo) + + OrderDetailsViewState.Computed.Details( + id = order.id, + number = "#${order.number}", + dateTime = order.dateCreated.formatToMMMddYYYYAtHHmm( + atWord = resourceProvider.getString(R.string.date_time_connector) + ), + customerEmail = order.customer?.email ?: order.billingAddress.email, + status = status, + lineItems = lineItems, + breakdown = breakdown, + total = formatPrice(order.total), + totalPaid = formatPrice(order.total), + paymentMethodTitle = order.paymentMethodTitle.takeIf { it.isNotBlank() } + ) + } - val status = PosOrderStatus( + private fun mapOrderStatus(order: Order): PosOrderStatus { + val statusText = order.status.localizedLabel(resourceProvider, locale) + return PosOrderStatus( text = statusText, colorKey = OrderStatusColorKey.fromStatus(order.status) ) + } - val lineItemsDeferred = order.items.map { item -> + private suspend fun buildLineItems( + order: Order + ): List = coroutineScope { + order.items.map { item -> async { - val unitPrice = if (item.quantity == 0f) item.total else item.total / item.quantity.toBigDecimal() + val unitPrice = + if (item.quantity == 0f) { + item.total + } else { + item.total / item.quantity.toBigDecimal() + } val product = getProductById(item.productId) OrderDetailsViewState.Computed.Details.LineItemRow( id = item.itemId, @@ -538,46 +583,57 @@ class WooPosOrdersViewModel @Inject constructor( imageUrl = product?.firstImageUrl ) } - } + }.awaitAll() + } - val discountCode = order.couponLines.firstOrNull()?.code + private data class RefundInfo( + val refundAmounts: List, + val totalRefunded: BigDecimal + ) - val refundsDeferred = async { retrieveOrderRefunds(order) } + private suspend fun buildRefundInfo( + order: Order, + refundResult: RefundFetchResult + ): RefundInfo { + return when (refundResult) { + is RefundFetchResult.Success -> { + val amounts = refundResult.refunds.map { "-${formatPrice(it.amount)}" } + val total = refundResult.refunds.sumOf { it.amount } + RefundInfo(amounts, total) + } + is RefundFetchResult.Error -> { + val amounts = + if (order.refundTotal > BigDecimal.ZERO) { + listOf(resourceProvider.getString(R.string.woopos_orders_details_refund_error)) + } else { + emptyList() + } + RefundInfo(amounts, BigDecimal.ZERO) + } + } + } - val lineItems = lineItemsDeferred.awaitAll() - val refunds = refundsDeferred.await() - val refundAmounts = refunds.map { "-${formatPrice(it.amount)}" } - val totalRefunded = refunds.sumOf { it.amount } - val netPayment = if (totalRefunded > BigDecimal.ZERO) { - formatPrice(order.total - totalRefunded) + private suspend fun buildTotalsBreakdown( + order: Order, + refundInfo: RefundInfo + ): OrderDetailsViewState.Computed.Details.TotalsBreakdown { + val netPayment = if (refundInfo.totalRefunded > BigDecimal.ZERO) { + formatPrice(order.total - refundInfo.totalRefunded) } else { null } - val breakdown = OrderDetailsViewState.Computed.Details.TotalsBreakdown( + val discountCode = order.couponLines.firstOrNull()?.code + + return OrderDetailsViewState.Computed.Details.TotalsBreakdown( products = formatPrice(order.productsTotal), discount = order.discountTotal.takeIf { it != BigDecimal.ZERO }?.let { "-${formatPrice(it)}" }, discountCode = discountCode, taxes = formatPrice(order.totalTax), shipping = order.shippingTotal.takeIf { it != BigDecimal.ZERO }?.let { formatPrice(it) }, - refunds = refundAmounts, + refunds = refundInfo.refundAmounts, netPayment = netPayment ) - - OrderDetailsViewState.Computed.Details( - id = order.id, - number = "#${order.number}", - dateTime = order.dateCreated.formatToMMMddYYYYAtHHmm( - atWord = resourceProvider.getString(R.string.date_time_connector) - ), - customerEmail = order.customer?.email ?: order.billingAddress.email, - status = status, - lineItems = lineItems, - breakdown = breakdown, - total = formatPrice(order.total), - totalPaid = formatPrice(order.total), - paymentMethodTitle = order.paymentMethodTitle.takeIf { it.isNotBlank() } - ) } } diff --git a/WooCommerce/src/main/res/values/strings.xml b/WooCommerce/src/main/res/values/strings.xml index bea8bc02b303..3d206312215a 100644 --- a/WooCommerce/src/main/res/values/strings.xml +++ b/WooCommerce/src/main/res/values/strings.xml @@ -3833,6 +3833,7 @@ Total Total paid Refunded + Unable to load refund Net Payment Order products list Order totals breakdown diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt index d70ffad2ce23..9026c4a5d5aa 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefundsTest.kt @@ -12,6 +12,9 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.model.refunds.WCRefundModel +import org.wordpress.android.fluxc.network.BaseRequest.GenericErrorType.UNKNOWN +import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooError +import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooErrorType.GENERIC_ERROR import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooResult import org.wordpress.android.fluxc.store.WCRefundStore import java.math.BigDecimal @@ -70,11 +73,13 @@ class WooPosRetrieveOrderRefundsTest { val result = sut.invoke(order) // THEN - assertThat(result).hasSize(2) - assertThat(result[0].id).isEqualTo(1L) - assertThat(result[0].amount).isEqualTo(BigDecimal.TEN) - assertThat(result[1].id).isEqualTo(2L) - assertThat(result[1].amount).isEqualTo(BigDecimal.valueOf(5)) + assertThat(result.isSuccess).isTrue() + val refunds = result.getOrThrow() + assertThat(refunds).hasSize(2) + assertThat(refunds[0].id).isEqualTo(1L) + assertThat(refunds[0].amount).isEqualTo(BigDecimal.TEN) + assertThat(refunds[1].id).isEqualTo(2L) + assertThat(refunds[1].amount).isEqualTo(BigDecimal.valueOf(5)) } @Test @@ -104,9 +109,11 @@ class WooPosRetrieveOrderRefundsTest { val result = sut.invoke(order) // THEN - assertThat(result).hasSize(1) - assertThat(result[0].id).isEqualTo(10L) - assertThat(result[0].amount).isEqualTo(BigDecimal.ONE) + assertThat(result.isSuccess).isTrue() + val refunds = result.getOrThrow() + assertThat(refunds).hasSize(1) + assertThat(refunds[0].id).isEqualTo(10L) + assertThat(refunds[0].amount).isEqualTo(BigDecimal.ONE) verify(refundStore).getAllRefunds(site, order.id) verify(refundStore).fetchAllRefunds(site, order.id) } @@ -120,7 +127,8 @@ class WooPosRetrieveOrderRefundsTest { val result = sut.invoke(order) // THEN - assertThat(result).isEmpty() + assertThat(result.isSuccess).isTrue() + assertThat(result.getOrThrow()).isEmpty() verify(refundStore, org.mockito.kotlin.never()).getAllRefunds(any(), any()) } @@ -139,4 +147,20 @@ class WooPosRetrieveOrderRefundsTest { // THEN verify(refundStore).getAllRefunds(site, order.id) } + + @Test + fun `given fetch refunds fails, when invoke called, then returns failure result`() = runTest { + // GIVEN + val order = OrderTestUtils.generateTestOrder(orderId = 789L, refundTotal = BigDecimal.ONE) + whenever(refundStore.getAllRefunds(site, order.id)).thenReturn(emptyList()) + whenever(refundStore.fetchAllRefunds(site, order.id)).thenReturn( + WooResult(WooError(GENERIC_ERROR, UNKNOWN)) + ) + + // WHEN + val result = sut.invoke(order) + + // THEN + assertThat(result.isFailure).isTrue() + } } diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt index 591c26a757bd..53c9d5c8ed2f 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt @@ -1,13 +1,17 @@ package com.woocommerce.android.ui.woopos.orders +import com.woocommerce.android.model.Order import com.woocommerce.android.model.OrderMapper import com.woocommerce.android.tools.SelectedSite import com.woocommerce.android.ui.orders.OrderTestUtils +import com.woocommerce.android.ui.woopos.common.data.WooPosRetrieveOrderRefunds import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.toList +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.assertj.core.api.Assertions.assertThat +import org.junit.Before import org.junit.Rule import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull @@ -37,24 +41,35 @@ class WooPosOrdersDataSourceTest { private val selectedSite: SelectedSite = mock { on { get() }.thenReturn(siteModel) } private val orderMapper: OrderMapper = mock() private val ordersCache: WooPosOrdersInMemoryCache = mock() + private val retrieveOrderRefunds: WooPosRetrieveOrderRefunds = mock() private val sut = WooPosOrdersDataSource( restClient = orderRestClient, selectedSite = selectedSite, orderMapper = orderMapper, - ordersCache = ordersCache + ordersCache = ordersCache, + retrieveOrderRefunds = retrieveOrderRefunds ) + @Before + fun setUp() { + runBlocking { + whenever(retrieveOrderRefunds.invoke(any())).thenReturn(Result.success(emptyList())) + } + } + @Test fun `when cache has data and fetch succeeds, then emit SuccessCache then SuccessRemote and store mapped in cache`() = runTest { - val cachedOrder = OrderTestUtils.generateTestOrder() + // GIVEN + val cachedOrder = OrderTestUtils.generateTestOrder(orderId = 1L) whenever(ordersCache.getAll()).thenReturn(listOf(cachedOrder)) - val e1 = OrderEntity(localSiteId = LocalOrRemoteId.LocalId(1), 11) - val e2 = OrderEntity(localSiteId = LocalOrRemoteId.LocalId(1), 22) + val e1 = OrderEntity(localSiteId = LocalOrRemoteId.LocalId(1), 11L) + val e2 = OrderEntity(localSiteId = LocalOrRemoteId.LocalId(1), 22L) val entities = listOf(e1 to emptyList(), e2 to emptyList()) - val mapped1 = OrderTestUtils.generateTestOrder() - val mapped2 = OrderTestUtils.generateTestOrder() + + val mapped1 = OrderTestUtils.generateTestOrder(orderId = 11L) + val mapped2 = OrderTestUtils.generateTestOrder(orderId = 22L) whenever(orderMapper.toAppModel(e1)).thenReturn(mapped1) whenever(orderMapper.toAppModel(e2)).thenReturn(mapped2) @@ -82,10 +97,10 @@ class WooPosOrdersDataSourceTest { assertThat(emissions).hasSize(2) val first = emissions[0] as LoadOrdersResult.SuccessCache - assertThat(first.orders).containsExactly(cachedOrder) + assertThat(first.ordersWithRefunds.keys).containsExactly(cachedOrder) val second = emissions[1] as LoadOrdersResult.SuccessRemote - assertThat(second.orders).containsExactly(mapped1, mapped2) + assertThat(second.ordersWithRefunds.keys).containsExactly(mapped1, mapped2) verify(selectedSite).get() verify(ordersCache).getAll() @@ -135,7 +150,7 @@ class WooPosOrdersDataSourceTest { assertThat(emissions).hasSize(2) val first = emissions[0] as LoadOrdersResult.SuccessCache - assertThat(first.orders).containsExactly(cachedOrder) + assertThat(first.ordersWithRefunds.keys).containsExactly(cachedOrder) val second = emissions[1] as LoadOrdersResult.Error assertThat(second.message).isEqualTo("[GENERIC_ERROR] generic error") @@ -180,7 +195,7 @@ class WooPosOrdersDataSourceTest { assertThat(emissions).hasSize(1) val first = emissions[0] as LoadOrdersResult.SuccessRemote - assertThat(first.orders).isEmpty() + assertThat(first.ordersWithRefunds).isEmpty() verify(selectedSite).get() verify(ordersCache).getAll() @@ -232,7 +247,7 @@ class WooPosOrdersDataSourceTest { // THEN assertThat(result).isInstanceOf(SearchOrdersResult.Success::class.java) val success = result as SearchOrdersResult.Success - assertThat(success.orders).containsExactly(mapped1, mapped2) + assertThat(success.ordersWithRefunds.keys).containsExactly(mapped1, mapped2) verify(orderRestClient).fetchOrders( site = siteModel, @@ -307,7 +322,7 @@ class WooPosOrdersDataSourceTest { // THEN assertThat(result).isInstanceOf(SearchOrdersResult.Success::class.java) val success = result as SearchOrdersResult.Success - assertThat(success.orders).isEmpty() + assertThat(success.ordersWithRefunds).isEmpty() } @Test @@ -329,7 +344,7 @@ class WooPosOrdersDataSourceTest { val emissions = sut.loadOrders().toList(mutableListOf()) val remote = emissions.last() as LoadOrdersResult.SuccessRemote - assertThat(remote.orders.map { it.id }).containsExactly(1L, 2L) + assertThat(remote.ordersWithRefunds.keys.map { it.id }).containsExactly(1L, 2L) assertThat(sut.hasMorePages).isTrue } @@ -362,7 +377,8 @@ class WooPosOrdersDataSourceTest { val result = sut.loadMore() assertThat(result.isSuccess).isTrue - assertThat(result.getOrThrow().map { it.id }).containsExactly(3L, 4L) + val ordersWithRefunds = result.getOrThrow() + assertThat(ordersWithRefunds.keys.map { it.id }).containsExactly(3L, 4L) assertThat(sut.hasMorePages).isFalse } @@ -456,7 +472,8 @@ class WooPosOrdersDataSourceTest { // THEN assertThat(result.isSuccess).isTrue - assertThat(result.getOrThrow().map { it.id }).containsExactly(33L, 44L) + val ordersWithRefunds = result.getOrThrow() + assertThat(ordersWithRefunds.keys.map { it.id }).containsExactly(33L, 44L) assertThat(sut.hasMorePages).isFalse verify(orderRestClient).fetchOrders( site = siteModel, diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModelTest.kt index 167d455c0d9a..247d6e87a9a2 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModelTest.kt @@ -16,6 +16,7 @@ import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule import com.woocommerce.android.ui.woopos.util.format.WooPosFormatPrice import com.woocommerce.android.viewmodel.ResourceProvider import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.flow import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.advanceUntilIdle @@ -24,9 +25,9 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.Before import org.junit.Rule import org.junit.Test -import org.mockito.Mockito.mock import org.mockito.kotlin.any import org.mockito.kotlin.eq +import org.mockito.kotlin.mock import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -41,10 +42,8 @@ class WooPosOrdersViewModelTest { val coroutineTestRule = WooPosCoroutineTestRule() private val dataSource: WooPosOrdersDataSource = mock() - private lateinit var viewModel: WooPosOrdersViewModel - private fun order(id: Long = 1L): Order = OrderTestUtils.generateTestOrder(orderId = id) private val resourceProvider: ResourceProvider = mock() private val getProductById: WooPosGetProductById = mock() private val formatPrice: WooPosFormatPrice = mock() @@ -53,6 +52,11 @@ class WooPosOrdersViewModelTest { private val childrenToParentEventSender: WooPosChildrenToParentEventSender = mock() private val ordersAnalyticsTracker: WooPosOrdersAnalyticsTracker = mock() + private fun order(id: Long = 1L): Order = OrderTestUtils.generateTestOrder(orderId = id) + + private fun ordersMap(vararg orders: Order): Map = + orders.associateWith { RefundFetchResult.Success(emptyList()) } + private fun createViewModel(): WooPosOrdersViewModel { return WooPosOrdersViewModel( ordersDataSource = dataSource, @@ -72,10 +76,14 @@ class WooPosOrdersViewModelTest { runBlocking { whenever(formatPrice.invoke(any())).thenReturn("$0.00") + whenever(getProductById.invoke(any())).thenReturn(null) + whenever(retrieveOrderRefunds.invoke(any())).thenReturn(Result.success(emptyList())) } whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessCache(listOf(order(1), order(2)))) } + flow { + emit(LoadOrdersResult.SuccessCache(ordersMap(order(1), order(2)))) + } ) whenever(resourceProvider.getString(R.string.woopos_orders_status_auto_draft)).thenReturn("Draft") @@ -87,11 +95,6 @@ class WooPosOrdersViewModelTest { whenever(resourceProvider.getString(R.string.woopos_orders_status_completed)).thenReturn("Completed") whenever(resourceProvider.getString(R.string.woopos_orders_status_refunded)).thenReturn("Refunded") whenever(resourceProvider.getString(R.string.woopos_search_orders)).thenReturn("Search orders") - - runBlocking { - whenever(getProductById.invoke(any())).thenReturn(null) - whenever(retrieveOrderRefunds.invoke(any())).thenReturn(emptyList()) - } } @Test @@ -101,8 +104,8 @@ class WooPosOrdersViewModelTest { val network = listOf(order(2), order(3)) whenever(dataSource.loadOrders()).thenReturn( flow { - emit(LoadOrdersResult.SuccessCache(cached)) - emit(LoadOrdersResult.SuccessRemote(network)) + emit(LoadOrdersResult.SuccessCache(ordersMap(*cached.toTypedArray()))) + emit(LoadOrdersResult.SuccessRemote(ordersMap(*network.toTypedArray()))) } ) @@ -128,8 +131,8 @@ class WooPosOrdersViewModelTest { val network = listOf(order(10)) whenever(dataSource.loadOrders()).thenReturn( flow { - emit(LoadOrdersResult.SuccessCache(emptyList())) - emit(LoadOrdersResult.SuccessRemote(network)) + emit(LoadOrdersResult.SuccessCache(emptyMap())) + emit(LoadOrdersResult.SuccessRemote(ordersMap(*network.toTypedArray()))) } ) @@ -139,11 +142,9 @@ class WooPosOrdersViewModelTest { // THEN val state = viewModel.state.value - assertThat(state).isInstanceOf(WooPosOrdersState.Content::class.java) val content = state as WooPosOrdersState.Content val loadedItems = content.items as WooPosOrdersState.Content.Items.Loaded assertThat(loadedItems.items.keys.map { it.id }).containsExactly(10L) - assertThat(content.pullToRefreshState).isEqualTo(WooPosPullToRefreshState.Enabled) assertThat(content.selectedDetails.id).isEqualTo(10L) } @@ -152,8 +153,8 @@ class WooPosOrdersViewModelTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( flow { - emit(LoadOrdersResult.SuccessCache(emptyList())) - emit(LoadOrdersResult.SuccessRemote(emptyList())) + emit(LoadOrdersResult.SuccessCache(emptyMap())) + emit(LoadOrdersResult.SuccessRemote(emptyMap())) } ) @@ -188,7 +189,7 @@ class WooPosOrdersViewModelTest { fun `given initial content, when refresh, then clear cache and update with network result`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1)))) } ) viewModel = createViewModel() advanceUntilIdle() @@ -198,8 +199,8 @@ class WooPosOrdersViewModelTest { whenever(dataSource.loadOrders()).thenReturn( flow { - emit(LoadOrdersResult.SuccessCache(emptyList())) - emit(LoadOrdersResult.SuccessRemote(listOf(order(5), order(6)))) + emit(LoadOrdersResult.SuccessCache(emptyMap())) + emit(LoadOrdersResult.SuccessRemote(ordersMap(order(5), order(6)))) } ) @@ -220,7 +221,7 @@ class WooPosOrdersViewModelTest { fun `given orders loaded, when selecting an order, then selected id, flags and details update`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1), order(2), order(3)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1), order(2), order(3)))) } ) // WHEN @@ -243,7 +244,7 @@ class WooPosOrdersViewModelTest { fun `given selection removed after reload, when refreshing, then first item is auto selected with details`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(100), order(200)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(100), order(200)))) } ) viewModel = createViewModel() advanceUntilIdle() @@ -252,8 +253,8 @@ class WooPosOrdersViewModelTest { whenever(dataSource.loadOrders()).thenReturn( flow { - emit(LoadOrdersResult.SuccessCache(emptyList())) - emit(LoadOrdersResult.SuccessRemote(listOf(order(300), order(400)))) + emit(LoadOrdersResult.SuccessCache(emptyMap())) + emit(LoadOrdersResult.SuccessRemote(ordersMap(order(300), order(400)))) } ) @@ -265,7 +266,6 @@ class WooPosOrdersViewModelTest { val state = viewModel.state.value as WooPosOrdersState.Content val loadedItems = state.items as WooPosOrdersState.Content.Items.Loaded assertThat(loadedItems.items.keys.map { it.id }).containsExactly(300L, 400L) - // first item should be auto-selected after reload assertThat(state.selectedDetails.id).isEqualTo(300L) } @@ -283,7 +283,6 @@ class WooPosOrdersViewModelTest { val state = viewModel.state.value as WooPosOrdersState.Content assertThat(state.searchInputState).isInstanceOf(WooPosSearchInputState.Open::class.java) val openState = state.searchInputState as WooPosSearchInputState.Open - assertThat(openState.input).isInstanceOf(WooPosSearchInputState.Open.Input.Hint::class.java) val hint = openState.input as WooPosSearchInputState.Open.Input.Hint assertThat(hint.hint).isEqualTo("Search orders") assertThat(openState.requestFocus).isTrue() @@ -295,7 +294,12 @@ class WooPosOrdersViewModelTest { // GIVEN val query = "test query" val searchResult = listOf(order(10), order(20)) - whenever(dataSource.searchOrders(query)).thenReturn(SearchOrdersResult.Success(searchResult)) + whenever(dataSource.searchOrders(query)).thenReturn( + SearchOrdersResult.Success(ordersMap(*searchResult.toTypedArray())) + ) + whenever(dataSource.loadOrders()).thenReturn( + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1)))) } + ) viewModel = createViewModel() advanceUntilIdle() @@ -337,6 +341,9 @@ class WooPosOrdersViewModelTest { .thenReturn("Unable to load orders") whenever(resourceProvider.getString(R.string.woopos_search_orders_error_description)) .thenReturn("Please try again.") + whenever(dataSource.loadOrders()).thenReturn( + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1)))) } + ) viewModel = createViewModel() advanceUntilIdle() @@ -363,10 +370,12 @@ class WooPosOrdersViewModelTest { fun `given more pages, when end reached and loadMore succeeds, then items append and pagination None`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1), order(2)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1), order(2)))) } ) whenever(dataSource.hasMorePages).thenReturn(true) - whenever(dataSource.loadMore()).thenReturn(Result.success(listOf(order(3), order(4)))) + whenever(dataSource.loadMore()).thenReturn( + Result.success(ordersMap(order(3), order(4))) + ) // WHEN viewModel = createViewModel() @@ -386,7 +395,7 @@ class WooPosOrdersViewModelTest { fun `given more pages, when end reached and loadMore fails, then keep items and show pagination error`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1), order(2)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1), order(2)))) } ) whenever(dataSource.hasMorePages).thenReturn(true) whenever(dataSource.loadMore()).thenReturn(Result.failure(RuntimeException("boom"))) @@ -409,8 +418,8 @@ class WooPosOrdersViewModelTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( flow { - emit(LoadOrdersResult.SuccessCache(listOf(order(1)))) - kotlinx.coroutines.delay(Long.MAX_VALUE) + emit(LoadOrdersResult.SuccessCache(ordersMap(order(1)))) + delay(Long.MAX_VALUE) } ) @@ -430,10 +439,12 @@ class WooPosOrdersViewModelTest { fun `given selected order, when appending next page, then selection and details are preserved`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(10), order(20)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(10), order(20)))) } ) whenever(dataSource.hasMorePages).thenReturn(true) - whenever(dataSource.loadMore()).thenReturn(Result.success(listOf(order(30), order(40)))) + whenever(dataSource.loadMore()).thenReturn( + Result.success(ordersMap(order(30), order(40))) + ) // WHEN viewModel = createViewModel() @@ -456,7 +467,7 @@ class WooPosOrdersViewModelTest { fun `given pagination error, when try again succeeds, then append next page and clear error`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1), order(2)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1), order(2)))) } ) whenever(dataSource.hasMorePages).thenReturn(true) whenever(dataSource.loadMore()).thenReturn(Result.failure(RuntimeException("boom"))) @@ -468,14 +479,14 @@ class WooPosOrdersViewModelTest { viewModel.onEndOfOrdersListReached() advanceUntilIdle() - // THEN (pagination shows error) + // THEN var content = viewModel.state.value as WooPosOrdersState.Content var loadedItems = content.items as WooPosOrdersState.Content.Items.Loaded assertThat(loadedItems.items.keys.map { it.id }).containsExactly(1L, 2L) assertThat(content.paginationState).isEqualTo(WooPosPaginationState.Error) - // GIVEN (next call will succeed) - whenever(dataSource.loadMore()).thenReturn(Result.success(listOf(order(3), order(4)))) + // GIVEN + whenever(dataSource.loadMore()).thenReturn(Result.success(ordersMap(order(3), order(4)))) // WHEN viewModel.onPaginationErrorTryAgain() @@ -496,7 +507,7 @@ class WooPosOrdersViewModelTest { val base = OrderTestUtils.generateTestOrder(orderId = 42L) val withOnHold = base.copy(status = Order.Status.OnHold) whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(withOnHold))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(withOnHold))) } ) // WHEN @@ -516,7 +527,7 @@ class WooPosOrdersViewModelTest { val base = OrderTestUtils.generateTestOrder(orderId = 77L) val custom = base.copy(status = Order.Status.Custom("awaiting-payment")) whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(custom))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(custom))) } ) // WHEN @@ -535,7 +546,7 @@ class WooPosOrdersViewModelTest { // GIVEN val one = OrderTestUtils.generateTestOrder(orderId = 11L) whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(one))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(one))) } ) // WHEN @@ -553,7 +564,7 @@ class WooPosOrdersViewModelTest { fun `given orders loaded, when selecting an order, then selectedOrderDetails is populated`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1), order(2)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1), order(2)))) } ) // WHEN @@ -565,7 +576,6 @@ class WooPosOrdersViewModelTest { // THEN val content = viewModel.state.value as WooPosOrdersState.Content val loadedItems = content.items as WooPosOrdersState.Content.Items.Loaded - // selected item should be 2, and selectedDetails should match val selectedItemId = loadedItems.items.keys.single { it.isSelected }.id assertThat(selectedItemId).isEqualTo(2L) assertThat(content.selectedDetails.id).isEqualTo(2L) @@ -577,10 +587,10 @@ class WooPosOrdersViewModelTest { fun `given selected order, when appending next page, then selectedOrderDetails content remains correct`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(10), order(20)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(10), order(20)))) } ) whenever(dataSource.hasMorePages).thenReturn(true) - whenever(dataSource.loadMore()).thenReturn(Result.success(listOf(order(30), order(40)))) + whenever(dataSource.loadMore()).thenReturn(Result.success(ordersMap(order(30), order(40)))) // WHEN viewModel = createViewModel() @@ -603,12 +613,12 @@ class WooPosOrdersViewModelTest { fun `given orders reloaded, when refreshing, then first order details are auto-populated`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(100), order(200)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(100), order(200)))) } ) viewModel = createViewModel() advanceUntilIdle() whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(300), order(400)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(300), order(400)))) } ) // WHEN @@ -627,7 +637,7 @@ class WooPosOrdersViewModelTest { fun `given content loaded, when onEmailReceiptButtonClicked called, then ToEmailReceipt is sent`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(123)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(123)))) } ) viewModel = createViewModel() advanceUntilIdle() @@ -645,10 +655,10 @@ class WooPosOrdersViewModelTest { fun `given order with no refunds, when mapped, then breakdown has empty refunds and null net payment`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1)))) } ) runBlocking { - whenever(retrieveOrderRefunds.invoke(order())).thenReturn(emptyList()) + whenever(retrieveOrderRefunds.invoke(order(1))).thenReturn(Result.success(emptyList())) } // WHEN @@ -665,9 +675,6 @@ class WooPosOrdersViewModelTest { fun `given order with refunds, when mapped, then breakdown includes refund amounts and net payment`() = runTest { // GIVEN val testOrder = order(1) - whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(testOrder))) } - ) val refund1 = Refund( id = 1, @@ -691,11 +698,22 @@ class WooPosOrdersViewModelTest { ) runBlocking { - whenever(retrieveOrderRefunds.invoke(testOrder)).thenReturn(listOf(refund1, refund2)) - whenever(formatPrice.invoke(java.math.BigDecimal("10.00"))).thenReturn("$10.00") - whenever(formatPrice.invoke(java.math.BigDecimal("5.00"))).thenReturn("$5.00") + whenever(formatPrice.invoke(BigDecimal("10.00"))).thenReturn("$10.00") + whenever(formatPrice.invoke(BigDecimal("5.00"))).thenReturn("$5.00") } + whenever(dataSource.loadOrders()).thenReturn( + flow { + emit( + LoadOrdersResult.SuccessRemote( + mapOf( + testOrder to RefundFetchResult.Success(listOf(refund1, refund2)) + ) + ) + ) + } + ) + // WHEN viewModel = createViewModel() advanceUntilIdle() @@ -710,7 +728,7 @@ class WooPosOrdersViewModelTest { fun `given selected order, when onBackFromSuccessfullySendingEmailReceipt succeeds, then refreshes details`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(100), order(200)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(100), order(200)))) } ) viewModel = createViewModel() advanceUntilIdle() @@ -726,7 +744,6 @@ class WooPosOrdersViewModelTest { // THEN val state = viewModel.state.value as WooPosOrdersState.Content - assertThat(state.selectedDetails.id).isEqualTo(200L) verify(dataSource).refreshOrderById(200L) } @@ -735,7 +752,7 @@ class WooPosOrdersViewModelTest { fun `given selected order, when onBackFromSuccessfullySendingEmailReceipt fails, then details remain unchanged`() = runTest { // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1), order(2)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1), order(2)))) } ) viewModel = createViewModel() advanceUntilIdle() @@ -753,22 +770,24 @@ class WooPosOrdersViewModelTest { // THEN val after = viewModel.state.value as WooPosOrdersState.Content - assertThat(after.selectedDetails).isEqualTo(beforeDetails) verify(dataSource).refreshOrderById(1L) } @Test fun `given orders loaded, when selecting an order, then tracks OrdersListRowTapped and OrderDetailsLoaded`() = runTest { + // GIVEN whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1), order(2), order(3)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1), order(2), order(3)))) } ) viewModel = createViewModel() advanceUntilIdle() + // WHEN viewModel.onOrderSelected(3L) advanceUntilIdle() + // THEN verify(ordersAnalyticsTracker).trackOrdersListRowTapped( orderId = eq(3L), orderStatus = any(), @@ -785,36 +804,44 @@ class WooPosOrdersViewModelTest { @Test fun `when remote load succeeds, then tracks OrdersListFetched`() = runTest { + // GIVEN val cached = listOf(order(1)) val remote = listOf(order(2), order(3)) whenever(dataSource.loadOrders()).thenReturn( flow { - emit(LoadOrdersResult.SuccessCache(cached)) - emit(LoadOrdersResult.SuccessRemote(remote)) + emit(LoadOrdersResult.SuccessCache(ordersMap(*cached.toTypedArray()))) + emit(LoadOrdersResult.SuccessRemote(ordersMap(*remote.toTypedArray()))) } ) + // WHEN viewModel = createViewModel() advanceUntilIdle() + // THEN verify(ordersAnalyticsTracker).trackOrdersListFetched(any()) } @Test fun `when search succeeds, then tracks OrdersListSearchResultsFetched`() = runTest { + // GIVEN val query = "abc" val searchResult = listOf(order(10), order(20)) whenever(dataSource.loadOrders()).thenReturn( - flow { emit(LoadOrdersResult.SuccessRemote(listOf(order(1)))) } + flow { emit(LoadOrdersResult.SuccessRemote(ordersMap(order(1)))) } + ) + whenever(dataSource.searchOrders(query)).thenReturn( + SearchOrdersResult.Success(ordersMap(*searchResult.toTypedArray())) ) - whenever(dataSource.searchOrders(query)).thenReturn(SearchOrdersResult.Success(searchResult)) viewModel = createViewModel() advanceUntilIdle() + // WHEN viewModel.onSearchEvent(WooPosSearchUIEvent.Search(query, query.length)) advanceUntilIdle() + // THEN verify(ordersAnalyticsTracker).trackOrdersListSearchResultsFetched(any()) } } From d6b20cd9c466512aa8862ec2bd0c23199c044f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 11 Nov 2025 15:27:59 +0100 Subject: [PATCH 5/7] Remove not so useful comment --- .../ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt | 9 --------- 1 file changed, 9 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt index cff71e169c50..3bb7c773368d 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosRetrieveOrderRefunds.kt @@ -10,15 +10,6 @@ import org.wordpress.android.fluxc.store.WCRefundStore import java.math.BigDecimal import javax.inject.Inject -/** - * Use case that retrieves all refunds for the given [Order]. - * - * - First attempts to load refunds from the local store. - * - If none are found, fetches them from the remote source. - * - If the order's [Order.refundTotal] is zero, returns an empty list without querying any source. - * - * This optimization prevents unnecessary store or network access for non-refunded orders. - */ class WooPosRetrieveOrderRefunds @Inject constructor( private val refundStore: WCRefundStore, private val selectedSite: SelectedSite From b98b56374a009092517a02bb6d8040846478064a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Tue, 11 Nov 2025 16:07:47 +0100 Subject: [PATCH 6/7] Attempt to fix test in CI --- .../android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt index 53c9d5c8ed2f..7f77a482eefb 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersDataSourceTest.kt @@ -219,8 +219,9 @@ class WooPosOrdersDataSourceTest { val e1 = OrderEntity(localSiteId = LocalOrRemoteId.LocalId(1), 11) val e2 = OrderEntity(localSiteId = LocalOrRemoteId.LocalId(1), 22) val entities = listOf(e1 to emptyList(), e2 to emptyList()) - val mapped1 = OrderTestUtils.generateTestOrder() - val mapped2 = OrderTestUtils.generateTestOrder() + + val mapped1 = OrderTestUtils.generateTestOrder(orderId = 11L) + val mapped2 = OrderTestUtils.generateTestOrder(orderId = 22L) whenever(orderMapper.toAppModel(e1)).thenReturn(mapped1) whenever(orderMapper.toAppModel(e2)).thenReturn(mapped2) From 76683f16df34396dd1ef71083d28c4087600db79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ce=CC=81sar=20Vargas=20Casaseca?= Date: Wed, 12 Nov 2025 11:15:30 +0100 Subject: [PATCH 7/7] Remove comment --- .../android/ui/woopos/orders/WooPosOrdersViewModel.kt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt index 2cee86b71a22..3696b6532637 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/WooPosOrdersViewModel.kt @@ -527,9 +527,7 @@ class WooPosOrdersViewModel @Inject constructor( createdAtMillis = order.dateCreated.time ) } - - // inside WooPosOrdersViewModel - + private suspend fun mapOrderDetails( order: Order, refundResult: RefundFetchResult