Skip to content

Comments

[CIAB: POS] Payment successful screen not displayed after booking cash payment#15407

Draft
samiuelson wants to merge 4 commits intotrunkfrom
wooprd-2472-pos-bookings-payment-successful-screen-not-displayed-after
Draft

[CIAB: POS] Payment successful screen not displayed after booking cash payment#15407
samiuelson wants to merge 4 commits intotrunkfrom
wooprd-2472-pos-bookings-payment-successful-screen-not-displayed-after

Conversation

@samiuelson
Copy link
Contributor

@samiuelson samiuelson commented Feb 20, 2026

Description

Adds a dedicated POS “Payment successful” screen and routes both card and cash (Bookings) flows to it, addressing cases where the success UI wasn’t shown after completing a booking cash payment.

Changes:

  • Introduces a new Payment Success destination (screen + navigation route) and wires it into the main POS graph and navigation handler.
  • Updates cash/card payment ViewModels to emit a new OpenPaymentSuccess navigation event (and removes the old in-place card success state/UI).

Test Steps

  1. Go to POS / Bookings
  2. Collect cash payment
  3. Verify the success screen is shown and it redirects back to Bookings

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.

Extract payment success UI into a standalone navigation destination
that can be reused by both card and cash payment flows.
Navigate to the shared success screen on payment success instead of
showing an inline composable. Remove PaymentSuccess state, the
CardPaymentSuccess composable, and onDoneClicked/onEmailReceiptClicked
from the card payment ViewModel.
Navigate to the shared success screen after a successful cash payment
for bookings instead of immediately navigating back.
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a dedicated POS “Payment successful” screen and routes both card and cash (Bookings) flows to it, addressing cases where the success UI wasn’t shown after completing a booking cash payment.

Changes:

  • Introduces a new Payment Success destination (screen + navigation route) and wires it into the main POS graph and navigation handler.
  • Updates cash/card payment ViewModels to emit a new OpenPaymentSuccess navigation event (and removes the old in-place card success state/UI).
  • Updates unit tests to assert the new navigation behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/paymentsuccess/WooPosPaymentSuccessScreen.kt New Compose UI for the unified payment success screen and its “Done/Email receipt” actions.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/paymentsuccess/WooPosPaymentSuccessNavigation.kt New navigation route + args encoding/decoding + PaymentSuccessSource.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/root/navigation/WooPosNavigationEvent.kt Adds OpenPaymentSuccess navigation event.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/root/navigation/WooPosNavigationEventHandler.kt Handles OpenPaymentSuccess by navigating to the new destination.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/root/navigation/WooPosMainFlowGraph.kt Registers the new paymentSuccessScreen in the graph.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/cashpayment/WooPosCashPaymentViewModel.kt For BOOKINGS cash success, emits OpenPaymentSuccess (instead of navigating back immediately).
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/cardpayment/WooPosCardPaymentViewModel.kt On card payment success, emits OpenPaymentSuccess instead of transitioning to an in-screen success state.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/cardpayment/WooPosCardPaymentState.kt Removes the PaymentSuccess state (success is now a separate screen).
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/cardpayment/WooPosCardPaymentScreen.kt Removes the embedded “CardPaymentSuccess” UI and related callbacks.
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/cashpayment/WooPosCashPaymentViewModelTest.kt Updates BOOKINGS success test to expect OpenPaymentSuccess.
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/cardpayment/WooPosCardPaymentViewModelTest.kt Updates/expands tests to assert OpenPaymentSuccess emission and receipt message passing.
Comments suppressed due to low confidence (3)

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/paymentsuccess/WooPosPaymentSuccessNavigation.kt:47

  • navigateToPaymentSuccessScreen() always includes the receiptSentMessage query parameter, and when receiptSentMessage is null it gets replaced with an empty string. Because the param is present, navArgument(... defaultValue = null, nullable = true) won’t apply and the screen receives "" instead of null, which can break the intended semantics. Consider building the route so the query parameter is omitted entirely when receiptSentMessage == null (or using a sentinel value and mapping it back to null).
    val route = PAYMENT_SUCCESS_ROUTE
        .replace("{$PAYMENT_SUCCESS_ORDER_ID_KEY}", orderId.toString())
        .replace("{$PAYMENT_SUCCESS_ORDER_TOTAL_TEXT_KEY}", orderTotalText.encodeForRoute())
        .replace("{$PAYMENT_SUCCESS_SOURCE_KEY}", source.name)
        .replace(
            "{$PAYMENT_SUCCESS_RECEIPT_SENT_MESSAGE_KEY}",
            receiptSentMessage?.encodeForRoute().orEmpty()
        )
    navigateOnce(route)

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/paymentsuccess/WooPosPaymentSuccessScreen.kt:74

  • The Email Receipt button now only emits OpenEmailReceipt(orderId) and no longer tracks the EmailReceiptTapped analytics event (previously tracked from WooPosCardPaymentViewModel.onEmailReceiptClicked()). If this metric is still needed, consider adding analytics tracking back in this flow (e.g., via a small PaymentSuccess ViewModel, or by plumbing a callback that tracks before emitting the navigation event).
            }
        },
        onEmailReceiptClicked = {
            onNavigationEvent(WooPosNavigationEvent.OpenEmailReceipt(orderId))
        },

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/cashpayment/WooPosCashPaymentViewModel.kt:146

  • In the BOOKINGS success path you fetch the order again via repository.getOrderById(orderId) just to build orderTotalText, even though the ViewModel already has the total in stateBeforeCompleting.total. This adds an extra IO call and introduces an avoidable null fallback; consider using the existing state value to format the success text instead of reloading the order.
                    CashPaymentSource.BOOKINGS -> {
                        val order = repository.getOrderById(orderId)
                        val orderTotalText = if (order != null) {
                            resourceProvider.getString(
                                R.string.woopos_totals_success_payment_cash,
                                priceFormat(order.total)
                            )
                        } else {
                            resourceProvider.getString(R.string.woopos_payment_successful_label)
                        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wpmobilebot
Copy link
Collaborator

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

App NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Build Number728
Version24.1
Application IDcom.woocommerce.android.prealpha
Commit9346fdc
Installation URL3u5j0ouhl6c0o
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 26.37363% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.29%. Comparing base (1ed28ca) to head (9346fdc).

Files with missing lines Patch % Lines
...s/paymentsuccess/WooPosPaymentSuccessNavigation.kt 5.35% 53 Missing ⚠️
...os/root/navigation/WooPosNavigationEventHandler.kt 0.00% 6 Missing ⚠️
...i/woopos/cashpayment/WooPosCashPaymentViewModel.kt 54.54% 1 Missing and 4 partials ⚠️
...i/woopos/cardpayment/WooPosCardPaymentViewModel.kt 81.81% 0 Missing and 2 partials ⚠️
...d/ui/woopos/root/navigation/WooPosMainFlowGraph.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #15407      +/-   ##
============================================
- Coverage     39.31%   39.29%   -0.03%     
+ Complexity    10990    10988       -2     
============================================
  Files          2239     2240       +1     
  Lines        127647   127709      +62     
  Branches      17832    17844      +12     
============================================
- Hits          50187    50184       -3     
- Misses        72331    72394      +63     
- Partials       5129     5131       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants