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

[hafizrahman]

Closes: #13033

Description

This PR handles the edge case bug outlined in #13033: when converting from Simple Subscription or Variable Subscription to Simple Physical product, and the subscription product has "One time shipping" enabled but no other shipping values, then the Shipping card would disappear after the conversion, making it impossible to set Shipping.

Why does this happen?
The issue happens inside ProductDetailViewModel.onProductTypeChanged(). When it is called, updateProductDraft() does not take into account the existing subscriptionDraft that the product has from when it was originally a subscription product. When a Product becomes a simply physical but its subscriptionDraft value still exists, it causes issue with how cards are displayed.

With the issue in #13033 specifically, because subscriptionDraft still exists, this returns true:

woocommerce-android/WooCommerce/src/main/kotlin/com/woocommerce/android/model/ProductAggregate.kt

Lines 22 to 23 in 509b9c9

	val hasShipping: Boolean
	get() = product.hasShipping || subscription?.oneTimeShipping == true

which then makes the app try to show the Shipping card on

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

Lines 473 to 474 in 509b9c9

	private fun ProductAggregate.shipping(): ProductProperty? {
	return if (!this.product.isVirtual && hasShipping) {

However because there are no other values aside from One time shipping, and One time shipping display is gated behind Subscription product type check, nothing is shown for this card as there's nothing to show.

And again due to the hasShipping check, it becomes unavailable in "Add more details", too:

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

Lines 109 to 110 in 509b9c9

	private fun ProductAggregate.getShipping(): ProductDetailBottomSheetUiItem? {
	return if (!product.isVirtual && !hasShipping) {

Solution:
The fix in this PR is to reset the data inside subscriptionDraft whenever converting to a non simple/variable subscription product. That way hasShipping will properly return false and the checks above will behave correctly.

Steps to reproduce

1. Create a Simple Subscription product,
2. Go to Add More Details > Shipping,
3. Enable "One time shipping" but don't fill in another values, then press back,
4. Publish the product,
5. Once published, tap "Product Type" card and switch to "Simple Physical Product", confirm the change,
6. Confirm that "Shipping" is not listed there. But, if you tap "Add More Details", you should see the "Shipping" option there.

Also to test:

• Try the above also but from Variable Subscription product to simple physical product. The result should be the same.
• Try converting from Simple Subscription to Variable Subscription. The Shipping card should not be removed and the "One time shipping" label and setting should be preserved.
• Try the above also but with some values inside Shipping (weight, length, etc) in addition to "One time shipping". The Shipping card should still exist and showing the right values (minus "One time shipping") after converting to Simple physical product.

Testing information

I tested this in an emulator with API 35.

The tests that have been performed

All the tests listed above.

Images/gif

• I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

• The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
• Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
• Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	7f37411
	Direct Download	woocommerce-wear-prototype-build-pr13035-7f37411.apk

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	7f37411
	Direct Download	woocommerce-prototype-build-pr13035-7f37411.apk

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 40.24%. Comparing base (e5b5579) to head (7f37411).
Report is 137 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...roid/ui/products/details/ProductDetailViewModel.kt	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #13035      +/-   ##
============================================
+ Coverage     40.23%   40.24%   +0.01%     
- Complexity     6138     6141       +3     
============================================
  Files          1280     1280              
  Lines         74011    74014       +3     
  Branches      10124    10125       +1     
============================================
+ Hits          29779    29788       +9     
+ Misses        41645    41634      -11     
- Partials       2587     2592       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[irfano]

Thanks for adding these tests, @hafizrahman. I'm not approving or requesting changes at the moment, but I’ve asked some questions to initiate a discussion.

[irfano]

Thanks for the changes, Hafiz! LGTM! 👍🏻