Merge pull request #13032 from woocommerce/issue/13027-remove-one-tim…

…e-label-when-switching-to-simple Product details: Do not show "one time shipping" label on Simple Product
woocommerce · Dec 3, 2024 · 300721b · 300721b
2 parents 7507308 + e5b5579
commit 300721b
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 13 deletions.
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -4,10 +4,12 @@
 21.3
 -----
 - [*] "One time shipping" label in Product Subscriptions now matches its availability state correctly. [https://github.com/woocommerce/woocommerce-android/pull/13021]
+- [*] "One time shipping" label should not be shown in Simple product after conversion from Subscriptions product. [https://github.com/woocommerce/woocommerce-android/pull/13032]
 - [Internal] Refactored IPP Payment flow to allow customizing payment collection UI in POS [https://github.com/woocommerce/woocommerce-android/pull/13014]
 - [*] Blaze Campaign Intro screen now offers creating a new product if the site has no products yet [https://github.com/woocommerce/woocommerce-android/pull/13001]
 - [*] When entering a wrong WordPress.com account for login, retrying will bring the step back to the email input screen [https://github.com/woocommerce/woocommerce-android/pull/13024]
 
+
 -----
 21.2
 - [Internal] Changed a way how authenticated web view opened in the IPP flows [https://github.com/woocommerce/woocommerce-android/pull/12908]

diff --git a/...e/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailCardBuilder.kt b/...e/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailCardBuilder.kt
@@ -474,21 +474,26 @@ class ProductDetailCardBuilder(
 
     @Suppress("LongMethod")
     private fun ProductAggregate.shipping(): ProductProperty? {
-        return if (!this.product.isVirtual && hasShipping) {
+        val currentProduct = this.product
+        return if (!currentProduct.isVirtual && hasShipping) {
             val weightWithUnits = product.getWeightWithUnits(parameters.weightUnit)
             val sizeWithUnits = product.getSizeWithUnits(parameters.dimensionUnit)
-            val shippingGroup = mapOf(
-                Pair(resources.getString(string.product_weight), weightWithUnits),
-                Pair(resources.getString(string.product_dimensions), sizeWithUnits),
-                Pair(
+            val shippingGroup = buildMap {
+                put(resources.getString(string.product_weight), weightWithUnits)
+                put(resources.getString(string.product_dimensions), sizeWithUnits)
+                put(
                     resources.getString(string.product_shipping_class),
-                    viewModel.getShippingClassByRemoteShippingClassId(this.product.shippingClassId)
-                ),
-                Pair(
-                    resources.getString(string.subscription_one_time_shipping),
-                    buildOneTimeShippingDescription(subscription)
+                    viewModel.getShippingClassByRemoteShippingClassId(currentProduct.shippingClassId)
                 )
-            )
+
+                // Only add "One time shipping" info if product is Subscription types
+                if (currentProduct.productType == SUBSCRIPTION || currentProduct.productType == VARIABLE_SUBSCRIPTION) {
+                    put(
+                        resources.getString(string.subscription_one_time_shipping),
+                        buildOneTimeShippingDescription(subscription)
+                    )
+                }
+            }
 
             PropertyGroup(
                 string.product_shipping,

diff --git a/...c/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailCardBuilderTest.kt b/...c/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailCardBuilderTest.kt
@@ -6,6 +6,7 @@ import com.woocommerce.android.model.ProductAggregate
 import com.woocommerce.android.tools.SelectedSite
 import com.woocommerce.android.ui.blaze.IsBlazeEnabled
 import com.woocommerce.android.ui.customfields.CustomFieldsRepository
+import com.woocommerce.android.ui.products.ProductHelper
 import com.woocommerce.android.ui.products.ProductTestUtils
 import com.woocommerce.android.ui.products.ProductType
 import com.woocommerce.android.ui.products.addons.AddonRepository
@@ -37,6 +38,10 @@ class ProductDetailCardBuilderTest : BaseUnitTest() {
         onBlocking { hasDisplayableCustomFields(any()) } doReturn false
     }
 
+    private val resourceProvider: ResourceProvider = mock {
+        on { getString(any()) } doAnswer { it.getArgument<Any?>(0).toString() }
+    }
+
     @Before
     fun setUp() {
         val viewModel: ProductDetailViewModel = mock {
@@ -194,4 +199,112 @@ class ProductDetailCardBuilderTest : BaseUnitTest() {
         }
         Assertions.assertThat(customFieldsCard).isNull()
     }
+
+    @Test
+    fun `given subscription product with one time shipping enabled, when building cards, then shipping includes one-time shipping`() = testBlocking {
+        productStub = ProductTestUtils.generateProduct()
+            .copy(
+                isVirtual = false,
+                type = ProductType.SUBSCRIPTION.value,
+                weight = 1.5f,
+                length = 10f,
+                width = 20f,
+                height = 30f,
+                shippingClassId = 123
+            )
+
+        val subscriptionDetails = ProductHelper.getDefaultSubscriptionDetails().copy(
+            oneTimeShipping = true
+        )
+
+        val cards = sut.buildPropertyCards(
+            ProductAggregate(
+                product = productStub,
+                subscription = subscriptionDetails
+            ),
+            ""
+        )
+
+        val shippingGroup = cards.first { it.type == ProductPropertyCard.Type.SECONDARY }
+            .properties
+            .find {
+                it is ProductProperty.PropertyGroup &&
+                    it.title == R.string.product_shipping
+            } as ProductProperty.PropertyGroup
+
+        val propertyKeys = shippingGroup.properties.toList().map { it.first }
+        Assertions.assertThat(propertyKeys).hasSize(4) // Weight, Dimensions, Shipping class, One-time shipping
+        Assertions.assertThat(propertyKeys).contains(
+            resourceProvider.getString(R.string.subscription_one_time_shipping)
+        )
+    }
+
+    @Test
+    fun `given variable subscription product with one time shipping enabled, when building cards, then shipping includes one-time shipping`() = testBlocking {
+        productStub = ProductTestUtils.generateProduct()
+            .copy(
+                isVirtual = false,
+                type = ProductType.VARIABLE_SUBSCRIPTION.value,
+                weight = 1.5f,
+                length = 10f,
+                width = 20f,
+                height = 30f,
+                shippingClassId = 123
+            )
+
+        val subscriptionDetails = ProductHelper.getDefaultSubscriptionDetails().copy(
+            oneTimeShipping = true
+        )
+
+        val cards = sut.buildPropertyCards(
+            ProductAggregate(
+                product = productStub,
+                subscription = subscriptionDetails
+            ),
+            ""
+        )
+
+        val shippingGroup = cards.first { it.type == ProductPropertyCard.Type.SECONDARY }
+            .properties
+            .find {
+                it is ProductProperty.PropertyGroup &&
+                    it.title == R.string.product_shipping
+            } as ProductProperty.PropertyGroup
+
+        val propertyKeys = shippingGroup.properties.toList().map { it.first }
+        Assertions.assertThat(propertyKeys).hasSize(4) // Weight, Dimensions, Shipping class, One-time shipping
+        Assertions.assertThat(propertyKeys).contains(
+            resourceProvider.getString(R.string.subscription_one_time_shipping)
+        )
+    }
+
+    @Test
+    fun `given simple non-virtual product, when building cards, then shipping excludes one-time shipping`() = testBlocking {
+        productStub = ProductTestUtils.generateProduct()
+            .copy(
+                isVirtual = false,
+                type = ProductType.SIMPLE.value,
+                weight = 1.5f,
+                length = 10f,
+                width = 20f,
+                height = 30f,
+                shippingClassId = 123
+            )
+
+        val cards = sut.buildPropertyCards(ProductAggregate(productStub), "")
+
+        val shippingGroup = cards.first { it.type == ProductPropertyCard.Type.SECONDARY }
+            .properties
+            .find {
+                it is ProductProperty.PropertyGroup &&
+                    it.title == R.string.product_shipping
+            } as ProductProperty.PropertyGroup
+
+        val propertyKeys = shippingGroup.properties.toList().map { it.first }
+
+        Assertions.assertThat(propertyKeys).hasSize(3) // Weight, Dimensions, Shipping class
+        Assertions.assertThat(propertyKeys).doesNotContain(
+            resourceProvider.getString(R.string.subscription_one_time_shipping)
+        )
+    }
 }
diff --git a/...src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt b/...src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt
@@ -208,8 +208,7 @@ class ProductDetailViewModelTest : BaseUnitTest() {
                             resources.getString(R.string.product_dimensions),
                             productWithParameters.productDraft?.getSizeWithUnits(siteParams.dimensionUnit) ?: ""
                         ),
-                        Pair(resources.getString(R.string.product_shipping_class), ""),
-                        Pair(resources.getString(R.string.subscription_one_time_shipping), "")
+                        Pair(resources.getString(R.string.product_shipping_class), "")
                     ),
                     R.drawable.ic_gridicons_shipping,
                     true