-
Notifications
You must be signed in to change notification settings - Fork 131
[WOMOB-178][POS] Coupons Checkout Screen - Happy Path #13987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
[WOMOB-178][POS] Coupons Checkout Screen - Happy Path #13987
Conversation
…count-in-cart # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartViewModel.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the coupons checkout screen for the happy path by aligning the UI, tests, and event communication with the latest designs. Key changes include:
- Updating tests to verify proper coupon discount formatting and event handling.
- Refactoring naming from cartProductUpdater to cartItemsUpdater across multiple modules.
- Adjusting UI components and logging to support coupon discount displays and error reporting.
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
WooPosTotalsViewModelTest.kt | Added tests for coupon discount text correctness and handling of invalid coupons. |
WooPosCartViewModelTest.kt | Updated dependency and null handling for formatted coupon discounts in the cart. |
WooPosCartItemsUpdaterTest.kt | Revised tests to verify the updated coupons logic and reporting in cart items updater. |
WooPosTotalsViewModel.kt | Adjusted coupon discount formatting and mapping of coupon lines from orders. |
WooPosCartScreen.kt | Modified UI to conditionally style coupon items based on calculated discount. |
WooPosCartItemsUpdater.kt | Updated coupon processing to include logging and crash reporting when a coupon is missing. |
WooPosHomeViewModel.kt & Communication Files | Updated data structures and event communication for coupons. |
WooPosTheme.kt | Revised color definitions for success states in the design system. |
Files not reviewed (1)
- WooCommerce/src/main/res/values/strings.xml: Language not supported
Comments suppressed due to low confidence (2)
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartItemsUpdater.kt:87
- [nitpick] Consider including the coupon code (e.g., item.name) in the error message to improve debugging when the coupon cannot be found.
val message = "Coupon not found in the cart"
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt:490
- [nitpick] If coupon parsing failures are an expected occurrence, consider logging a warning instead of an error to prevent over-reporting.
wooLogWrapper.e(POS, "Parsing coupon failed, discount: ${it.discount}, code: ${it.code}, id: ${it.id}")
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #13987 +/- ##
============================================
+ Coverage 38.30% 38.32% +0.02%
- Complexity 9495 9503 +8
============================================
Files 2116 2116
Lines 116422 116494 +72
Branches 14919 14922 +3
============================================
+ Hits 44592 44644 +52
- Misses 67747 67760 +13
- Partials 4083 4090 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -65,6 +68,12 @@ sealed class ParentToChildrenEvent { | |||
val variationId: Long, | |||
) : ProductInfo(id, name, price, quantity) | |||
} | |||
|
|||
data class CouponLine( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np: I think we should call it either CouponInfo
or rename to ProductLine
for consistance
private suspend fun updateCouponsWithFormattedDiscount( | ||
updatedCoupons: List<ParentToChildrenEvent.OrderCreated.CouponLine>, | ||
item: WooPosCartItemViewState.Coupon, | ||
) = updatedCoupons.find { it.code == item.name }?.let { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this ok, that apparently the same is called "code" in one place and "name" in another?
if (isDiscountCalculated) { | ||
Spacer(modifier = Modifier.height(WooPosSpacing.XSmall.value.toAdaptivePadding())) | ||
WooPosText( | ||
text = requireNotNull(item.formattedDiscount) { "Can't be null" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np: seems redundant message as it doesn't provide anything extra
text = requireNotNull(item.formattedDiscount),
@@ -420,7 +423,7 @@ class WooPosTotalsViewModel @Inject constructor( | |||
.fold( | |||
onSuccess = { order -> handleCreatedOrder(order) }, | |||
onFailure = { error -> | |||
WooLog.e(T.POS, "Order creation failed - $error") | |||
WooLog.e(POS, "Order creation failed - $error") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np: if we inject wooLogWrapper
, then maybe to use it here too
code = it.code, | ||
discountAmount = BigDecimal(it.discount) | ||
) | ||
} catch (e: NumberFormatException) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np: e is unused
} catch (_: NumberFormatException) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I left a couple of NPs, please take a look!
Some general feedback:
- It's possible to checkout with just a coupon, with not products
- When the discount is shown on the coupon, we move from 2 lines to 3


And this creates a small jaggering effect. Maybe to add a quite, subtle animation the this state change?
- Also maybe consider adding animation on the color change from what it is to the green one
- When we go from the checout, we kind a"unapply" the coupon, but the reduced price of the coupons stay there


Closes: #WOOMOB-178
Description
This PR updates coupons on the checkout screen to match designs - happy path only.
The coupon in the cart is green and contains the discount after the order is created. The cart item is reverted if the user goes back to the product selection screen. Lastly, the totals section is re-ordered and rephrased.
This PR also updates the success/onSuccess colors to match the updated Design System.
Known issues:
There are changes in the cart
snackbar is always shown - will be fixed here.Testing information
No Coupon
Discount total
section is hidden and the flow works as expected.Coupon Added
Multiple coupons added
The tests that have been performed
I have verified the above.
Images/gif
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: