Skip to content

Conversation

@hafizrahman
Copy link
Contributor

@hafizrahman hafizrahman commented Nov 28, 2024

Closes: #12785

Description

This PR deals with the "One time shipping" feature in Subscription Product and the way its state is displayed in Product Details screen.

The documentation states this:

Note: For shipping to be charged on initial order, the subscription must not have a free trial or be synchronized to a date in the future. The One-Time Shipping option will be disabled if your product has a free trial or is synchronized.

This PR updates the Android behavior to match iOS:

  1. If free trial/future sync is enabled, then we should not display the "One time shipping" description label in Product details, as it is disabled automatically
  2. If there is no free trial/future sync and the feature is enabled inside "Shipping", then we want to display the "One time shipping: enabled" description label. If the feature is disabled, we should not display the "One time shipping" description label.
  3. Changes in the free trial and toggles should be reflected in the Product Details area correclty.

Steps to reproduce

  1. Start creation for Simple Subscription product in the app,
  2. Fill in the name, price, then tap Add More Details and select Shipping
  3. Fill in just the Weight value, then press back,
  4. Ensure the Shipping card in Product Details does not mention "One time shipping",
  5. Go back to Shipping and enable One time shipping, then press back,
  6. Ensure the Shipping card in Product Details says "One time shipping: Enabled",
  7. Tap Subscription free trial and enter a random number (e.g: 3 days), then press back,
  8. Check Shipping card and ensure it does not say anything about "One time shipping",
  9. Tap Shipping and ensure One Time Shipping toggle is now disabled (this is an existing behavior and not part of this PR, but good to check)

Optionally, also test this with an existing Product. The behavior should be the same.

Testing information

I tested this in an emulator with both new simple subscription product and existing product.

The tests that have been performed

Everything in the list above.

Images/gif

Expected states:

When there is free trial When no free trial and toggle enabled When no free trial and toggle disabled
❌ label ✅ label ❌ label
Screenshot_20241128_154311 Screenshot_20241128_154340 Screenshot_20241128_154356
Screen_recording_20241128_153843.mp4
  • 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.

@hafizrahman hafizrahman added type: bug A confirmed bug. feature: product details Related to adding or editing products, includes product settings. labels Nov 28, 2024
@hafizrahman hafizrahman added this to the 21.3 milestone Nov 28, 2024
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 28, 2024

📲 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
FlavorJalapeno
Build TypeDebug
Commitf109bb9
Direct Downloadwoocommerce-wear-prototype-build-pr13021-f109bb9.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 28, 2024

📲 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
FlavorJalapeno
Build TypeDebug
Commitf109bb9
Direct Downloadwoocommerce-prototype-build-pr13021-f109bb9.apk

@codecov-commenter
Copy link

codecov-commenter commented Nov 28, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 40.13%. Comparing base (4833cd3) to head (f109bb9).
Report is 10 commits behind head on trunk.

Files with missing lines Patch % Lines
...id/ui/products/details/ProductDetailCardBuilder.kt 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #13021   +/-   ##
=========================================
  Coverage     40.13%   40.13%           
- Complexity     6119     6120    +1     
=========================================
  Files          1280     1280           
  Lines         74006    74008    +2     
  Branches      10123    10122    -1     
=========================================
+ Hits          29701    29703    +2     
  Misses        41725    41725           
  Partials       2580     2580           

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

@hafizrahman hafizrahman marked this pull request as ready for review November 28, 2024 08:41
@irfano irfano self-assigned this Nov 28, 2024
Copy link
Contributor

@irfano irfano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for clearly explaining the rationale behind the change. LGTM and works as expected. 👍🏻 I’m approving but not merging to give you a chance to consider my suggestion regarding code style.
I also noticed another issue with the "One time shipping" label: #13027. It's reproducable on trunk and outside the scope of this PR.

@hafizrahman hafizrahman merged commit 4d873d6 into trunk Nov 29, 2024
15 checks passed
@hafizrahman hafizrahman deleted the issue/12785-subs-one-time-shipping-state branch November 29, 2024 05:35
@hafizrahman
Copy link
Contributor Author

I also noticed another issue with the "One time shipping" label: #13027. It's reproducable on trunk and outside the scope of this PR.

Thanks! I'll take a look at that issue next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: product details Related to adding or editing products, includes product settings. type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent Shipping details in product detail screen for subscription products

5 participants