Product details: Keep Shipping card with One Time Shipping available when changing from Subscription to Non-Subscription Type Product

RELEASE-NOTES.txt

@@ -5,6 +5,7 @@
 -----
 - [*] "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]
+- [*] Fixed a bug related to incorrect Shipping settings in Product details when converting from Subscriptions to Simple product. [https://github.com/woocommerce/woocommerce-android/pull/13035]
 - [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]
 

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel.kt

@@ -1229,13 +1229,22 @@ class ProductDetailViewModel @Inject constructor(
         updateProductDraft(type = productType.value, isVirtual = isVirtual)
 
         viewState.productAggregateDraft?.let { productAggregateDraft ->
-            if (productType == ProductType.SUBSCRIPTION && productAggregateDraft.subscription == null) {
-                viewState = viewState.copy(
-                    subscriptionDraft = ProductHelper.getDefaultSubscriptionDetails().copy(
-                        price = productAggregateDraft.product.regularPrice
-                    )
-                )
-            }
+            viewState = viewState.copy(
+                subscriptionDraft = when {
+                    // If converting to subscription product, set the default subscription details
+                    productType == ProductType.SUBSCRIPTION && productAggregateDraft.subscription == null ->
+                        ProductHelper.getDefaultSubscriptionDetails().copy(
+                            price = productAggregateDraft.product.regularPrice
+                        )
+
+                    // If converting to non-subscription products, reset subscription data that might have existed
+                    // (e.g: if the original product is of subscription type).
+                    // This avoids any Product Details card conflicts that can happen after conversion.
+                    productType !in setOf(ProductType.SUBSCRIPTION, ProductType.VARIABLE_SUBSCRIPTION) -> null
+
+                    else -> viewState.subscriptionDraft
+                }
+            )
         }
     }
 
@@ -2731,7 +2740,7 @@ class ProductDetailViewModel @Inject constructor(
             }
         )
 
-        fun copy(subscriptionDraft: SubscriptionDetails) = copy(
+        fun copy(subscriptionDraft: SubscriptionDetails?) = copy(
             productAggregateDraft = productAggregateDraft?.copy(subscription = subscriptionDraft)
         )
 

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt

@@ -17,8 +17,10 @@ import com.woocommerce.android.ui.blaze.IsBlazeEnabled
 import com.woocommerce.android.ui.customfields.CustomFieldsRepository
 import com.woocommerce.android.ui.media.MediaFileUploadHandler
 import com.woocommerce.android.ui.products.ParameterRepository
+import com.woocommerce.android.ui.products.ProductHelper
 import com.woocommerce.android.ui.products.ProductStatus
 import com.woocommerce.android.ui.products.ProductTestUtils
+import com.woocommerce.android.ui.products.ProductType
 import com.woocommerce.android.ui.products.addons.AddonRepository
 import com.woocommerce.android.ui.products.categories.ProductCategoriesRepository
 import com.woocommerce.android.ui.products.models.ProductProperty
@@ -977,7 +979,6 @@ class ProductDetailViewModelTest : BaseUnitTest() {
         doReturn(
             productAggregate.copy(product = productAggregate.product.copy(regularPrice = BigDecimal(99)))
         ).whenever(productRepository).getProductAggregate(any())
-        viewModel.productDetailViewStateData.observeForever { _, _ -> }
         viewModel.start()
 
         viewModel.updateProductDraft(sku = "E9999999")
@@ -990,7 +991,6 @@ class ProductDetailViewModelTest : BaseUnitTest() {
         doReturn(
             productAggregate.copy(product = productAggregate.product.copy(salePrice = BigDecimal(99)))
         ).whenever(productRepository).getProductAggregate(any())
-        viewModel.productDetailViewStateData.observeForever { _, _ -> }
         viewModel.start()
 
         viewModel.updateProductDraft(sku = "E9999999")
@@ -1003,7 +1003,6 @@ class ProductDetailViewModelTest : BaseUnitTest() {
         doReturn(
             productAggregate.copy(product = productAggregate.product.copy(regularPrice = BigDecimal(99)))
         ).whenever(productRepository).getProductAggregate(any())
-        viewModel.productDetailViewStateData.observeForever { _, _ -> }
         viewModel.start()
 
         viewModel.updateProductDraft(regularPrice = BigDecimal(0))
@@ -1016,7 +1015,6 @@ class ProductDetailViewModelTest : BaseUnitTest() {
         doReturn(
             productAggregate.copy(product = productAggregate.product.copy(regularPrice = BigDecimal(99)))
         ).whenever(productRepository).getProductAggregate(any())
-        viewModel.productDetailViewStateData.observeForever { _, _ -> }
         viewModel.start()
 
         viewModel.updateProductDraft(salePrice = BigDecimal(0))
@@ -1029,7 +1027,6 @@ class ProductDetailViewModelTest : BaseUnitTest() {
         doReturn(
             productAggregate.copy(product = productAggregate.product.copy(regularPrice = BigDecimal(99)))
         ).whenever(productRepository).getProductAggregate(any())
-        viewModel.productDetailViewStateData.observeForever { _, _ -> }
         viewModel.start()
 
         viewModel.updateProductDraft(regularPrice = null)
@@ -1042,7 +1039,6 @@ class ProductDetailViewModelTest : BaseUnitTest() {
         doReturn(
             productAggregate.copy(product = productAggregate.product.copy(regularPrice = BigDecimal(99)))
         ).whenever(productRepository).getProductAggregate(any())
-        viewModel.productDetailViewStateData.observeForever { _, _ -> }
         viewModel.start()
 
         viewModel.updateProductDraft(salePrice = null)
@@ -1243,6 +1239,78 @@ class ProductDetailViewModelTest : BaseUnitTest() {
             verify(productRepository, never()).fetchProductPassword(any())
         }
 
+    @Test
+    fun `When converting from subscription to simple product, subscription data is cleared`() = testBlocking {
+        // GIVEN
+        val subscriptionProduct = productAggregate.copy(
+            product = productAggregate.product.copy(
+                type = ProductType.SUBSCRIPTION.value
+            ),
+            subscription = ProductHelper.getDefaultSubscriptionDetails()
+        )
+        doReturn(subscriptionProduct).whenever(productRepository).getProductAggregate(any())
+        viewModel.start()
+
+        // Verify initial state has subscription data
+        Assertions.assertThat(viewModel.getProduct().subscriptionDraft).isNotNull()
+
+        // WHEN
+        viewModel.onProductTypeChanged(ProductType.SIMPLE, false)
+
+        // THEN
+        Assertions.assertThat(viewModel.getProduct().subscriptionDraft).isNull()
+    }
+
+    @Test
+    fun `When converting from simple to subscription product, default subscription data is added`() = testBlocking {
+        // GIVEN
+        val simpleProduct = productAggregate.copy(
+            product = productAggregate.product.copy(
+                type = ProductType.SIMPLE.value,
+                regularPrice = BigDecimal("10.00")
+            ),
+            subscription = null
+        )
+        doReturn(simpleProduct).whenever(productRepository).getProductAggregate(any())
+        viewModel.start()
+
+        // Verify initial state has no subscription data
+        Assertions.assertThat(viewModel.getProduct().subscriptionDraft).isNull()
+
+        // WHEN
+        viewModel.onProductTypeChanged(ProductType.SUBSCRIPTION, false)
+
+        // THEN
+        viewModel.getProduct().subscriptionDraft?.let {
+            Assertions.assertThat(it.price).isEqualTo(simpleProduct.product.regularPrice)
+            Assertions.assertThat(it.period)
+                .isEqualTo(ProductHelper.getDefaultSubscriptionDetails().period)
+            Assertions.assertThat(it.periodInterval)
+                .isEqualTo(ProductHelper.getDefaultSubscriptionDetails().periodInterval)
+        } ?: Assertions.fail("Subscription draft should not be null")
+    }
+
+    @Test
+    fun `When converting from simple subscription to variable subscription product, subscription data is preserved`() = testBlocking {
+        // GIVEN
+        val subscriptionProduct = productAggregate.copy(
+            product = productAggregate.product.copy(
+                type = ProductType.SUBSCRIPTION.value
+            ),
+            subscription = ProductHelper.getDefaultSubscriptionDetails()
+        )
+        doReturn(subscriptionProduct).whenever(productRepository).getProductAggregate(any())
+        viewModel.start()
+
+        val originalSubscription = viewModel.getProduct().subscriptionDraft
+
+        // WHEN
+        viewModel.onProductTypeChanged(ProductType.VARIABLE_SUBSCRIPTION, false)
+
+        // THEN
+        Assertions.assertThat(viewModel.getProduct().subscriptionDraft).isEqualTo(originalSubscription)
+    }
+
     private val productsDraft
         get() = viewModel.productDetailViewStateData.liveData.value?.productDraft
 }