diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 5372a30bcd7a..caf5c800c23f 100644 --- a/RELEASE-NOTES.txt +++ b/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] diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel.kt index daaaef496d41..553c75c000ce 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModel.kt +++ b/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) ) diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt index 1eba481f348e..8f0fe0ef672d 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/products/details/ProductDetailViewModelTest.kt +++ b/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 }