From 41de9c915b6fabed164e7be31cb9dd515348bf9b Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Wed, 8 Jan 2025 16:10:40 +0100 Subject: [PATCH 1/3] Make sure metadata value is read correctly --- .../java/com/woocommerce/android/wear/model/Refund.kt | 2 +- .../kotlin/com/woocommerce/android/model/Refund.kt | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/WooCommerce-Wear/src/main/java/com/woocommerce/android/wear/model/Refund.kt b/WooCommerce-Wear/src/main/java/com/woocommerce/android/wear/model/Refund.kt index 941493e8ce5..c66f79e0d10 100644 --- a/WooCommerce-Wear/src/main/java/com/woocommerce/android/wear/model/Refund.kt +++ b/WooCommerce-Wear/src/main/java/com/woocommerce/android/wear/model/Refund.kt @@ -55,7 +55,7 @@ fun WCRefundModel.WCRefundItem.toAppModel(): Refund.Item { -totalTax, // WCRefundItem.totalTax is NEGATIVE sku ?: "", price ?: BigDecimal.ZERO, - metaData?.get(0)?.value?.toString()?.toLongOrNull() ?: -1 + metaData?.get(0)?.value?.stringValue?.toLongOrNull() ?: -1 ) } 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 1ffa8fda548..a3ca940f13f 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Refund.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/model/Refund.kt @@ -5,7 +5,9 @@ import com.woocommerce.android.ui.products.ProductHelper import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize import org.wordpress.android.fluxc.model.refunds.WCRefundModel -import org.wordpress.android.fluxc.model.refunds.WCRefundModel.* +import org.wordpress.android.fluxc.model.refunds.WCRefundModel.WCRefundFeeLine +import org.wordpress.android.fluxc.model.refunds.WCRefundModel.WCRefundItem +import org.wordpress.android.fluxc.model.refunds.WCRefundModel.WCRefundShippingLine import java.math.BigDecimal import java.math.RoundingMode.HALF_UP import java.util.Date @@ -94,13 +96,13 @@ fun WCRefundItem.toAppModel(): Refund.Item { -totalTax, // WCRefundItem.totalTax is NEGATIVE sku ?: "", price ?: BigDecimal.ZERO, - metaData?.get(0)?.value?.toString()?.toLongOrNull() ?: -1 + metaData?.get(0)?.value?.stringValue?.toLongOrNull() ?: -1 ) } fun WCRefundShippingLine.toAppModel(): Refund.ShippingLine { return Refund.ShippingLine( - itemId = metaData?.get(0)?.value?.toString()?.toLongOrNull() ?: -1, + itemId = metaData?.get(0)?.value?.stringValue?.toLongOrNull() ?: -1, methodId = methodId ?: "", methodTitle = methodTitle ?: "", totalTax = -totalTax, // WCRefundShippineLine.totalTax is NEGATIVE @@ -110,7 +112,7 @@ fun WCRefundShippingLine.toAppModel(): Refund.ShippingLine { fun WCRefundFeeLine.toAppModel(): Refund.FeeLine { return Refund.FeeLine( - id = metaData?.get(0)?.value?.toString()?.toLongOrNull() ?: -1, + id = metaData?.get(0)?.value?.stringValue?.toLongOrNull() ?: -1, name = name, totalTax = -totalTax, // WCRefundFeeLine.totalTax is NEGATIVE total = (total), // WCRefundFeeLine.total is NEGATIVE From d35e3213f02187ad87aa7935e223c28dc6b7dadc Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Wed, 8 Jan 2025 16:20:26 +0100 Subject: [PATCH 2/3] Add a release note --- RELEASE-NOTES.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 92357692686..8fde520fef2 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -20,6 +20,7 @@ - [**] Improved UX of card reader payment flow in Point of Sale, allowing faster payment collection [https://github.com/woocommerce/woocommerce-android/pull/12977] - [*] Fixed crash when trying to open barcode scanner on device without camera [https://github.com/woocommerce/woocommerce-android/pull/13172] - [Internal] Woo POS now accepts cash payments and there is a functionality of sending receipts [https://github.com/woocommerce/woocommerce-android/pull/13157] +- [**] Fixed a bug that caused the app to not show the list of refunded products in the order details screen [https://github.com/woocommerce/woocommerce-android/pull/13261] ----- 21.2 From 0fed58ca683b20a6b8a210665ee16241aa97bb21 Mon Sep 17 00:00:00 2001 From: Hicham Boushaba Date: Thu, 9 Jan 2025 10:07:26 +0100 Subject: [PATCH 3/3] Add unit tests --- .../android/model/RefundMapperTest.kt | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 WooCommerce/src/test/kotlin/com/woocommerce/android/model/RefundMapperTest.kt diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/model/RefundMapperTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/model/RefundMapperTest.kt new file mode 100644 index 00000000000..9d3993df715 --- /dev/null +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/model/RefundMapperTest.kt @@ -0,0 +1,99 @@ +package com.woocommerce.android.model + +import org.junit.Assert.assertEquals +import org.junit.Test +import org.wordpress.android.fluxc.model.metadata.WCMetaData +import org.wordpress.android.fluxc.model.metadata.WCMetaDataValue +import org.wordpress.android.fluxc.model.refunds.WCRefundModel +import java.math.BigDecimal + +class RefundMapperTest { + @Test + fun `when mapping refund item from domain model, then extract its properties correctly`() { + val domainModel = WCRefundModel.WCRefundItem( + productId = 1, + quantity = -1, + itemId = 2, + name = "name", + variationId = null, + subtotal = BigDecimal.TEN, + total = BigDecimal.TEN, + totalTax = BigDecimal.TEN, + sku = "sku", + price = BigDecimal.TEN, + metaData = listOf(WCMetaData(id = 0, key = "_refunded_item_id", value = WCMetaDataValue(10))) + ) + + val appModel = domainModel.toAppModel() + + assertEquals(1, appModel.productId) + assertEquals(1, appModel.quantity) + assertEquals(2, appModel.id) + assertEquals("name", appModel.name) + assertEquals(-1, appModel.variationId) + assertEquals(BigDecimal.TEN.negate(), appModel.subtotal) + assertEquals(BigDecimal.TEN.negate(), appModel.total) + assertEquals(BigDecimal.TEN.negate(), appModel.totalTax) + assertEquals("sku", appModel.sku) + assertEquals(BigDecimal.TEN, appModel.price) + assertEquals(10, appModel.orderItemId) + } + + @Test + fun `given item ID is wrapped as String, when mapping refund item from domain model, then extract it correctly`() { + val domainModel = WCRefundModel.WCRefundItem( + productId = 1, + quantity = -1, + itemId = 2, + name = "name", + variationId = null, + subtotal = BigDecimal.TEN, + total = BigDecimal.TEN, + totalTax = BigDecimal.TEN, + sku = "sku", + price = BigDecimal.TEN, + metaData = listOf(WCMetaData(id = 0, key = "_refunded_item_id", value = WCMetaDataValue("10"))) + ) + + val appModel = domainModel.toAppModel() + + assertEquals(10, appModel.orderItemId) + } + + @Test + fun `when mapping shipping lines from domain model, then extract its properties correctly`() { + val domainModel = WCRefundModel.WCRefundShippingLine( + id = 1, + methodId = "methodId", + methodTitle = "methodTitle", + totalTax = BigDecimal.TEN, + total = BigDecimal.TEN, + metaData = listOf(WCMetaData(id = 0, key = "_refunded_item_id", value = WCMetaDataValue(10))) + ) + + val appModel = domainModel.toAppModel() + + // Intentionally not checking all properties, we have an inconsistency in the function where + // we negate the totalTax but not the total, and there is no explanation for this. + // See https://github.com/woocommerce/woocommerce-android/pull/5517#discussion_r774573314 + // We should update the test when we have a clear understanding of the expected behavior. + // Note: these properties are not used in the app currently. + assertEquals(10, appModel.itemId) + } + + @Test + fun `when mapping fee lines from domain model, then extract its properties correctly`() { + val domainModel = WCRefundModel.WCRefundFeeLine( + id = 1, + name = "name", + totalTax = BigDecimal.TEN, + total = BigDecimal.TEN, + metaData = listOf(WCMetaData(id = 0, key = "_refunded_item_id", value = WCMetaDataValue(10))) + ) + + val appModel = domainModel.toAppModel() + + // See comment in the previous test. + assertEquals(10, appModel.id) + } +}