[POS] Simultaneous cash and card payment handling #13277

samiuelson · 2025-01-09T17:05:23Z

Closes: #13278 13278

Description

This PR adds improvements to the cash payment collection flow:

When "Mark payment as complete" is tapped in the Cash payment screen, the card reader payment intent is canceled.
If the card payment is initiated, the Cash payment screen should be interrupted/closed automatically, showing the card payment processing state.

Steps to reproduce

Verify the following on trunk:

Payment intent cancelation

Open POS (menu more > Point of Sale)
Create order and make sure the reader is connected
Click "Cash payment"
Complete order by clicking the button, and observe that card reader payment action is not canceled

Cash payment screen not closed when card payment initiated:

Open POS (menu more > Point of Sale)
Create order and make sure the reader is connected
Click "Cash payment"
Tap card to the reader and observe Cash payment screen is not closed automatically

Testing information

It should be verified that the above scenarios are fixed.

The tests that have been performed

Samsung S9 Tab, Android 14
Verified that payment intent is canceled properly when order is marked completed (with cash)
Verified that Cash payment screen gets closed automatically when user taps card to the reader

Images/gif

Payment intent gets canceled when order marked as paid with cash

Screen_recording_20250109_173942.mp4

Cash payment is closed when card is tapped to the reader

Screen_recording_20250109_173912.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.

wpmobilebot · 2025-01-09T17:10:21Z

📲 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	`da47b85`
	Direct Download	`woocommerce-wear-prototype-build-pr13277-da47b85.apk`

wpmobilebot · 2025-01-10T11:14:55Z

📲 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	`da47b85`
	Direct Download	`woocommerce-prototype-build-pr13277-da47b85.apk`

codecov-commenter · 2025-01-10T11:20:46Z

Codecov Report

Attention: Patch coverage is 22.22222% with 21 lines in your changes missing coverage. Please review.

Project coverage is 41.09%. Comparing base (3222339) to head (da47b85).
Report is 31 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...ndroid/ui/woopos/root/navigation/WooPosRootHost.kt	0.00%	12 Missing ⚠️
...rce/android/ui/woopos/home/WooPosHomeNavigation.kt	0.00%	5 Missing ⚠️
...oid/ui/woopos/home/totals/WooPosTotalsViewModel.kt	83.33%	0 Missing and 1 partial ⚠️
...d/ui/woopos/root/navigation/WooPosMainFlowGraph.kt	0.00%	1 Missing ⚠️
...ui/woopos/root/navigation/WooPosNavigationEvent.kt	0.00%	1 Missing ⚠️
...os/root/navigation/WooPosNavigationEventHandler.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #13277      +/-   ##
============================================
- Coverage     41.10%   41.09%   -0.01%     
+ Complexity     6421     6420       -1     
============================================
  Files          1321     1321              
  Lines         77177    77200      +23     
  Branches      10643    10650       +7     
============================================
+ Hits          31722    31727       +5     
- Misses        42646    42663      +17     
- Partials       2809     2810       +1

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

kidinov · 2025-01-13T10:07:37Z

...src/main/kotlin/com/woocommerce/android/ui/woopos/cashpayment/WooPosCashPaymentNavigation.kt

@@ -49,6 +53,18 @@ fun NavGraphBuilder.cashPaymentScreen(
            }
        },
    ) { backStackEntry ->
+        val homeViewModel = hiltViewModel<WooPosHomeViewModel>()
+        LaunchedEffect(homeViewModel) {


Do you know if we need to have this code at all? Don't we handle the event in WooPosHomeScreen already?

The LaunchedEffect coroutine in WooPosHomeScreen is not active when we navigate to CashPayment screen. That's why I subscribed to the events in the CashPayment screen here again.

An alternative approach could be to move the subscription to navigation events up in the compose navigation hierarchy, to theWooPosRootHost, in order to avoid duplicate subscriptions – 5f9d68e. Let me know what you think, @kidinov !

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeViewModel.kt

...c/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeChildToParentCommunication.kt

kidinov · 2025-01-13T10:16:25Z

...merce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsViewModel.kt

-                    is ParentToChildrenEvent.OrderSuccessfullyPaid -> showSuccessfulPaymentState(
-                        event.paymentMethod
-                    )
+                    is ParentToChildrenEvent.OrderSuccessfullyPaid -> {


Just checking that it's ok that cancelation will be called also in the card reader payments cases

The flow works correctly, but I switched to canceling payment intent only in case the payment success comes from cash payment to avoid redundant logic – 60ef6e2

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeNavigation.kt

kidinov

Looks and works fine! I just left a few code improvements suggestions, please take a look!

…otHost`

samiuelson · 2025-01-14T17:00:57Z

Looks and works fine! I just left a few code improvements suggestions, please take a look!

Thanks for the review, @kidinov! I've addressed comments and pushed improvements. Let me know what you think.

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/root/navigation/WooPosRootHost.kt

kidinov

Looks and tests good!

samiuelson added 2 commits January 8, 2025 18:11

Cancel payment intent when "Mark ... as complete" tapped

9deb05e

Return to Home from Cash if card payment initiated

33dc451

Merge branch 'trunk' into pos-cash-card-payment-collection-alignment

4a506d8

samiuelson marked this pull request as ready for review January 10, 2025 08:18

samiuelson added 3 commits January 10, 2025 11:16

Merge branch 'trunk' into pos-cash-card-payment-collection-alignment

5e5ec14

Rename event

a87b61d

Rename event

749b5f8

samiuelson requested a review from kidinov January 10, 2025 10:53

samiuelson added the feature: point of sale POS project label Jan 10, 2025

samiuelson added this to the 21.5 milestone Jan 10, 2025

Reorg imports

f61a258

kidinov self-assigned this Jan 10, 2025

kidinov reviewed Jan 13, 2025

View reviewed changes

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeViewModel.kt Outdated Show resolved Hide resolved

kidinov reviewed Jan 13, 2025

View reviewed changes

...c/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeChildToParentCommunication.kt Outdated Show resolved Hide resolved

kidinov reviewed Jan 13, 2025

View reviewed changes

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/WooPosHomeNavigation.kt Show resolved Hide resolved

kidinov self-requested a review January 13, 2025 10:21

kidinov reviewed Jan 13, 2025

View reviewed changes

samiuelson added 7 commits January 14, 2025 12:06

Merge branch 'trunk' into pos-cash-card-payment-collection-alignment

629c416

Move WooPosHomeViewModel.navigationEvent collection up to `WooPosRo…

5f9d68e

…otHost`

Remove unused imports

7ce4eea

Rename nav event

929c42f

Cancel payment intent only if payment success comes from cash screen

60ef6e2

Simplify passing nav event from Totals

6259911

Satisfy detekt's complaints

cb990ff

Merge branch 'trunk' into pos-cash-card-payment-collection-alignment

da47b85

samiuelson requested a review from kidinov January 14, 2025 17:01

samiuelson commented Jan 15, 2025

View reviewed changes

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/root/navigation/WooPosRootHost.kt Show resolved Hide resolved

kidinov approved these changes Jan 15, 2025

View reviewed changes

kidinov merged commit 53e4403 into trunk Jan 15, 2025
15 checks passed

kidinov deleted the pos-cash-card-payment-collection-alignment branch January 15, 2025 14:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[POS] Simultaneous cash and card payment handling #13277

[POS] Simultaneous cash and card payment handling #13277

samiuelson commented Jan 9, 2025 •

edited by kidinov

Loading

wpmobilebot commented Jan 9, 2025 •

edited

Loading

wpmobilebot commented Jan 10, 2025 •

edited

Loading

codecov-commenter commented Jan 10, 2025 •

edited

Loading

kidinov Jan 13, 2025

samiuelson Jan 14, 2025

kidinov Jan 13, 2025

samiuelson Jan 14, 2025

kidinov left a comment

samiuelson commented Jan 14, 2025

kidinov left a comment

[POS] Simultaneous cash and card payment handling #13277

[POS] Simultaneous cash and card payment handling #13277

Conversation

samiuelson commented Jan 9, 2025 • edited by kidinov Loading

Description

Steps to reproduce

Payment intent cancelation

Cash payment screen not closed when card payment initiated:

Testing information

The tests that have been performed

Images/gif

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

wpmobilebot commented Jan 9, 2025 • edited Loading

wpmobilebot commented Jan 10, 2025 • edited Loading

codecov-commenter commented Jan 10, 2025 • edited Loading

Codecov Report

kidinov Jan 13, 2025

Choose a reason for hiding this comment

samiuelson Jan 14, 2025

Choose a reason for hiding this comment

kidinov Jan 13, 2025

Choose a reason for hiding this comment

samiuelson Jan 14, 2025

Choose a reason for hiding this comment

kidinov left a comment

Choose a reason for hiding this comment

samiuelson commented Jan 14, 2025

kidinov left a comment

Choose a reason for hiding this comment

samiuelson commented Jan 9, 2025 •

edited by kidinov

Loading

wpmobilebot commented Jan 9, 2025 •

edited

Loading

wpmobilebot commented Jan 10, 2025 •

edited

Loading

codecov-commenter commented Jan 10, 2025 •

edited

Loading