Skip to content

[HACK][Order Refund] Partial refund inconsistency #13212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
samiuelson merged 9 commits into from
Jan 8, 2025
Merged

[HACK][Order Refund] Partial refund inconsistency #13212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
73df5ac
Do not show items that's been refunded already
kidinov Dec 27, 2024
f277192
Fixed the quantaties. Added tests
kidinov Dec 27, 2024
f832bf6
Fixed formatting
kidinov Dec 27, 2024
c1b0c9a
Merge branch 'trunk' into 12964-mobile-payments-partial-refund-incons…
kidinov Jan 7, 2025
76220fb
Modify the method that calculates the max quantity for each item by s…
kidinov Jan 7, 2025
67934a5
Added a test. Fixed the usage of the adjusted method
kidinov Jan 7, 2025
09f75e4
Updated release notes
kidinov Jan 8, 2025
1790b0c
Fixed formatting
kidinov Jan 8, 2025
9248970
Removed unnecessary stubbings
kidinov Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
-----
Expand Down
21 changes: 15 additions & 6 deletions WooCommerce/src/main/kotlin/com/woocommerce/android/model/Refund.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,19 @@ fun List<Refund>.getNonRefundedProducts(
fun List<Refund>.getMaxRefundQuantities(
products: List<Order.Item>
): Map<Long, Float> {
val map = mutableMapOf<Long, Float>()
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()
}
4 changes: 2 additions & 2 deletions ...merce/src/main/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<ShippingLabel>()).whenever(orderDetailRepository).getOrderShippingLabels(any())
doReturn(emptyList<ShippingLabel>()).whenever(orderDetailRepository).fetchOrderShippingLabels(any())

viewModel.start()

Expand Down
290 changes: 289 additions & 1 deletion ...e/src/test/kotlin/com/woocommerce/android/ui/payments/refunds/IssueRefundViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 { "" }

Expand Down Expand Up @@ -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<Order.Item> {
on { quantity }.thenReturn(1.0F)
},
mock {
on { quantity }.thenReturn(1.0F)
},
mock {
on { quantity }.thenReturn(1.0F)
},
)
val order = mock<Order> {
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<OrderEntity>()
val orderMapper = mock<OrderMapper> {
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<Order.Item> {
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<Order> {
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<OrderEntity>()
val orderMapper = mock<OrderMapper> {
on { toAppModel(orderEntity) }.thenReturn(order)
}
val refundedItems = listOf(
mock<WCRefundItem> {
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<Order.Item> {
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<Order> {
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<OrderEntity>()
val orderMapper = mock<OrderMapper> {
on { toAppModel(orderEntity) }.thenReturn(order)
}
val refundedItems = listOf(
mock<WCRefundItem> {
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<WCRefundItem> {
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<Order.Item> {
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<Order> {
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<OrderEntity>()
val orderMapper = mock<OrderMapper> {
on { toAppModel(orderEntity) }.thenReturn(order)
}
val refundedItems = listOf(
mock<WCRefundItem> {
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<WCRefundItem> {
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<WCRefundItem> {
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)
}
}
Loading