diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index bc01b74b94cb..33c8c05b6a0d 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -9,6 +9,7 @@ - [**] Fixed a crash when a shop manager was trying to install or activate plugin in the POS onboarding [https://github.com/woocommerce/woocommerce-android/pull/13203] - [*] Fixed a crash on the order details [https://github.com/woocommerce/woocommerce-android/pull/13191] - [**] Introduced fallback logic for the barcode scanner to use the front-facing camera when a back-facing camera is unavailable [https://github.com/woocommerce/woocommerce-android/pull/13230] +- [**] Fixed a bug when refunded items were displayed on the refund screen [https://github.com/woocommerce/woocommerce-android/pull/13212] 21.3 ----- diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Refund.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Refund.kt index 1ffa8fda5480..01f184e72121 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Refund.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Refund.kt @@ -147,10 +147,19 @@ fun List.getNonRefundedProducts( fun List.getMaxRefundQuantities( products: List ): Map { - val map = mutableMapOf() - val groupedRefunds = this.flatMap { it.items }.groupBy { it.orderItemId } - products.map { item -> - map[item.itemId] = item.quantity - (groupedRefunds[item.itemId]?.sumOf { it.quantity } ?: 0) - } - return map + val allRefundedItems = this.flatMap { it.items } + + return products.mapNotNull { product -> + val refundedQuantity = allRefundedItems + .filter { it.productId == product.productId } + .sumOf { it.quantity } + + val quantityLeftToRefund = product.quantity - refundedQuantity + + if (quantityLeftToRefund > 0) { + product.itemId to quantityLeftToRefund.toFloat() + } else { + null + } + }.toMap() } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModel.kt index 9f0be15d165c..03a3107d8440 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModel.kt @@ -284,8 +284,8 @@ class IssueRefundViewModel @Inject constructor( ) } - val items = order.items.map { - val maxQuantity = maxQuantities[it.itemId] ?: 0f + val items = order.items.mapNotNull { + val maxQuantity = maxQuantities[it.itemId] ?: return@mapNotNull null val selectedQuantity = min(selectedQuantities[it.itemId] ?: 0, maxQuantity.toInt()) ProductRefundListItem( orderItem = it, diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt index 88780dee50c8..bf0d06567d00 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt @@ -483,14 +483,11 @@ class OrderDetailViewModelTest : BaseUnitTest() { doReturn(true).whenever(orderDetailRepository).hasVirtualProductsOnly(listOf(3, 4)) doReturn(virtualOrder).whenever(orderDetailRepository).getOrderById(any()) - doReturn(virtualOrder).whenever(orderDetailRepository).fetchOrderById(any()) doReturn(testOrderRefunds).whenever(orderDetailRepository).getOrderRefunds(any()) - doReturn(true).whenever(orderDetailRepository).fetchOrderNotes(any()) doReturn(testOrderNotes).whenever(orderDetailRepository).getOrderNotes(any()) doReturn(emptyList()).whenever(orderDetailRepository).getOrderShippingLabels(any()) - doReturn(emptyList()).whenever(orderDetailRepository).fetchOrderShippingLabels(any()) viewModel.start() diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModelTest.kt index 7469797760f6..0371b91b6739 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModelTest.kt @@ -7,6 +7,7 @@ import com.woocommerce.android.analytics.AnalyticsTrackerWrapper import com.woocommerce.android.extensions.isEqualTo import com.woocommerce.android.model.AmbiguousLocation import com.woocommerce.android.model.Location +import com.woocommerce.android.model.Order import com.woocommerce.android.model.OrderMapper import com.woocommerce.android.tools.NetworkStatus import com.woocommerce.android.tools.SelectedSite @@ -26,9 +27,11 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever +import org.wordpress.android.fluxc.model.OrderEntity import org.wordpress.android.fluxc.model.SiteModel import org.wordpress.android.fluxc.model.metadata.WCMetaData import org.wordpress.android.fluxc.model.refunds.WCRefundModel +import org.wordpress.android.fluxc.model.refunds.WCRefundModel.WCRefundItem import org.wordpress.android.fluxc.network.BaseRequest import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooError import org.wordpress.android.fluxc.network.rest.wpcom.wc.WooErrorType @@ -78,7 +81,9 @@ class IssueRefundViewModelTest : BaseUnitTest() { private lateinit var viewModel: IssueRefundViewModel - private fun initViewModel() { + private fun initViewModel( + orderMapper: OrderMapper = this.orderMapper + ) { whenever(selectedSite.get()).thenReturn(SiteModel()) whenever(currencyFormatter.buildBigDecimalFormatter(any())).thenReturn { "" } @@ -948,4 +953,287 @@ class IssueRefundViewModelTest : BaseUnitTest() { ) } } + + @Test + fun `given order with zero refunded items, when vm init, then all items are shown`() = testBlocking { + // GIVEN + val items = listOf( + mock { + on { quantity }.thenReturn(1.0F) + }, + mock { + on { quantity }.thenReturn(1.0F) + }, + mock { + on { quantity }.thenReturn(1.0F) + }, + ) + val order = mock { + on { this.items }.thenReturn(items) + on { this.total }.thenReturn(BigDecimal.TEN) + on { this.refundTotal }.thenReturn(BigDecimal.ZERO) + on { this.currency }.thenReturn("USD") + on { this.shippingTotal }.thenReturn(BigDecimal.ZERO) + on { this.feesTotal }.thenReturn(BigDecimal.ZERO) + on { this.paymentMethod }.thenReturn("cod") + } + val orderEntity = mock() + val orderMapper = mock { + on { toAppModel(orderEntity) }.thenReturn(order) + } + whenever(orderStore.getOrderByIdAndSite(any(), any())).thenReturn(orderEntity) + whenever(refundStore.getAllRefunds(any(), any())).thenReturn(emptyList()) + + // WHEN + initViewModel(orderMapper) + + // THEN + assertThat(viewModel.refundItems.value).hasSize(3) + } + + @Test + fun `given order with 1 refunded items, when vm init, then all items minus one are shown`() = testBlocking { + // GIVEN + val items = listOf( + mock { + on { productId }.thenReturn(1L) + on { itemId }.thenReturn(1L) + on { quantity }.thenReturn(1.0F) + }, + mock { + on { productId }.thenReturn(2L) + on { itemId }.thenReturn(2L) + on { quantity }.thenReturn(1.0F) + }, + mock { + on { productId }.thenReturn(3L) + on { itemId }.thenReturn(3L) + on { quantity }.thenReturn(1.0F) + }, + ) + val order = mock { + on { this.items }.thenReturn(items) + on { this.total }.thenReturn(BigDecimal.TEN) + on { this.refundTotal }.thenReturn(BigDecimal.ZERO) + on { this.currency }.thenReturn("USD") + on { this.shippingTotal }.thenReturn(BigDecimal.ZERO) + on { this.feesTotal }.thenReturn(BigDecimal.ZERO) + on { this.paymentMethod }.thenReturn("cod") + } + val orderEntity = mock() + val orderMapper = mock { + on { toAppModel(orderEntity) }.thenReturn(order) + } + val refundedItems = listOf( + mock { + on { productId }.thenReturn(1L) + on { quantity }.thenReturn(-1) + on { subtotal }.thenReturn(BigDecimal.ZERO) + on { total }.thenReturn(BigDecimal.ZERO) + on { sku }.thenReturn("") + on { price }.thenReturn(BigDecimal.ZERO) + on { totalTax }.thenReturn(BigDecimal.ZERO) + on { metaData }.thenReturn(null) + } + ) + val refund = WCRefundModel( + id = 1L, + dateCreated = Date(), + amount = BigDecimal.ZERO, + reason = "", + automaticGatewayRefund = false, + items = refundedItems, + shippingLineItems = listOf(), + feeLineItems = listOf() + ) + whenever(orderStore.getOrderByIdAndSite(any(), any())).thenReturn(orderEntity) + whenever(refundStore.getAllRefunds(any(), any())).thenReturn(listOf(refund)) + + // WHEN + initViewModel(orderMapper) + + // THEN + assertThat(viewModel.refundItems.value).hasSize(2) + } + + @Test + @Suppress("LongMethod") + fun `given order with 6 product each and 2 fully refunded, when vm init, then 1 product with 2 items shown`() = + testBlocking { + // GIVEN + val items = listOf( + mock { + on { productId }.thenReturn(1L) + on { itemId }.thenReturn(1L) + on { quantity }.thenReturn(2.0F) + }, + mock { + on { productId }.thenReturn(2L) + on { itemId }.thenReturn(2L) + on { quantity }.thenReturn(2.0F) + }, + mock { + on { productId }.thenReturn(3L) + on { itemId }.thenReturn(3L) + on { quantity }.thenReturn(2.0F) + }, + ) + val order = mock { + on { this.items }.thenReturn(items) + on { this.total }.thenReturn(BigDecimal.TEN) + on { this.refundTotal }.thenReturn(BigDecimal.ZERO) + on { this.currency }.thenReturn("USD") + on { this.shippingTotal }.thenReturn(BigDecimal.ZERO) + on { this.feesTotal }.thenReturn(BigDecimal.ZERO) + on { this.paymentMethod }.thenReturn("cod") + } + val orderEntity = mock() + val orderMapper = mock { + on { toAppModel(orderEntity) }.thenReturn(order) + } + val refundedItems = listOf( + mock { + on { productId }.thenReturn(1L) + on { quantity }.thenReturn(-2) + on { subtotal }.thenReturn(BigDecimal.ZERO) + on { total }.thenReturn(BigDecimal.ZERO) + on { sku }.thenReturn("") + on { price }.thenReturn(BigDecimal.ZERO) + on { totalTax }.thenReturn(BigDecimal.ZERO) + on { metaData }.thenReturn(null) + }, + mock { + on { productId }.thenReturn(2L) + on { quantity }.thenReturn(-2) + on { subtotal }.thenReturn(BigDecimal.ZERO) + on { total }.thenReturn(BigDecimal.ZERO) + on { sku }.thenReturn("") + on { price }.thenReturn(BigDecimal.ZERO) + on { totalTax }.thenReturn(BigDecimal.ZERO) + on { metaData }.thenReturn(null) + }, + ) + val refund = WCRefundModel( + id = 1L, + dateCreated = Date(), + amount = BigDecimal.ZERO, + reason = "", + automaticGatewayRefund = false, + items = refundedItems, + shippingLineItems = listOf(), + feeLineItems = listOf() + ) + whenever(orderStore.getOrderByIdAndSite(any(), any())).thenReturn(orderEntity) + whenever(refundStore.getAllRefunds(any(), any())).thenReturn(listOf(refund)) + + // WHEN + initViewModel(orderMapper) + + // THEN + assertThat(viewModel.refundItems.value).hasSize(1) + assertThat(viewModel.refundItems.value!!.first().maxQuantity).isEqualTo(2.0F) + } + + @Test + @Suppress("LongMethod") + fun `given order with 5 product each and some fully refunded some partially, when vm init, then the rest are shown`() = + testBlocking { + // GIVEN + val items = listOf( + mock { + on { productId }.thenReturn(1L) + on { itemId }.thenReturn(1L) + on { quantity }.thenReturn(2.0F) + }, + mock { + on { productId }.thenReturn(2L) + on { itemId }.thenReturn(2L) + on { quantity }.thenReturn(2.0F) + }, + mock { + on { productId }.thenReturn(3L) + on { itemId }.thenReturn(3L) + on { quantity }.thenReturn(2.0F) + }, + mock { + on { productId }.thenReturn(4L) + on { itemId }.thenReturn(4L) + on { quantity }.thenReturn(2.0F) + }, + mock { + on { productId }.thenReturn(5L) + on { itemId }.thenReturn(5L) + on { quantity }.thenReturn(2.0F) + }, + ) + val order = mock { + on { this.items }.thenReturn(items) + on { this.total }.thenReturn(BigDecimal.TEN) + on { this.refundTotal }.thenReturn(BigDecimal.ZERO) + on { this.currency }.thenReturn("USD") + on { this.shippingTotal }.thenReturn(BigDecimal.ZERO) + on { this.feesTotal }.thenReturn(BigDecimal.ZERO) + on { this.paymentMethod }.thenReturn("cod") + } + val orderEntity = mock() + val orderMapper = mock { + on { toAppModel(orderEntity) }.thenReturn(order) + } + val refundedItems = listOf( + mock { + on { productId }.thenReturn(1L) + on { quantity }.thenReturn(-2) + on { subtotal }.thenReturn(BigDecimal.ZERO) + on { total }.thenReturn(BigDecimal.ZERO) + on { sku }.thenReturn("") + on { price }.thenReturn(BigDecimal.ZERO) + on { totalTax }.thenReturn(BigDecimal.ZERO) + on { metaData }.thenReturn(null) + }, + mock { + on { productId }.thenReturn(2L) + on { quantity }.thenReturn(-1) + on { subtotal }.thenReturn(BigDecimal.ZERO) + on { total }.thenReturn(BigDecimal.ZERO) + on { sku }.thenReturn("") + on { price }.thenReturn(BigDecimal.ZERO) + on { totalTax }.thenReturn(BigDecimal.ZERO) + on { metaData }.thenReturn(null) + }, + mock { + on { productId }.thenReturn(3L) + on { quantity }.thenReturn(-2) + on { subtotal }.thenReturn(BigDecimal.ZERO) + on { total }.thenReturn(BigDecimal.ZERO) + on { sku }.thenReturn("") + on { price }.thenReturn(BigDecimal.ZERO) + on { totalTax }.thenReturn(BigDecimal.ZERO) + on { metaData }.thenReturn(null) + }, + + ) + + val refund = WCRefundModel( + id = 1L, + dateCreated = Date(), + amount = BigDecimal.ZERO, + reason = "", + automaticGatewayRefund = false, + items = refundedItems, + shippingLineItems = listOf(), + feeLineItems = listOf() + ) + + whenever(orderStore.getOrderByIdAndSite(any(), any())).thenReturn(orderEntity) + whenever(refundStore.getAllRefunds(any(), any())).thenReturn(listOf(refund)) + + // WHEN + initViewModel(orderMapper) + + // THEN + assertThat(viewModel.refundItems.value).hasSize(3) + assertThat(viewModel.refundItems.value!![0].maxQuantity).isEqualTo(1.0F) + assertThat(viewModel.refundItems.value!![1].maxQuantity).isEqualTo(2.0F) + assertThat(viewModel.refundItems.value!![2].maxQuantity).isEqualTo(2.0F) + } }