[Woo POS] Start card reader connection flow and adapt to return result of connection to pos #11616

kidinov · 2024-05-29T15:03:56Z

Description

The PR adds some changes to reuse the card reader connection flow from the WooPOS screen

It doesn't handle "cancelation" of the connection flow properly, as the flow calls "navigate" on empty stack trace, and for some magical reason, instead of just going back from WooCardActivity to WooPosActivity it just restarts WooCardActivity. Please share if you have any ideas as to why this happens. As for M1 we aim in a happy flow I created a ticket on that.

~~I started to do "popBackStack" and then it works fine for cancelation of the flow too (looks a bit ugly code-wise though)~~

In the following PR I actually started to return result from the flow back and finish activity

I tried to extract the connection into a separate navigation graph but unfortunately, it requires a lot of changes in the current flows which is very risky at this point

Testing instructions

The most important is to validate that no IPP flow is affected by the changes

To validate that the flow works for POS, click on the "connect" button

Images/gif

05-30--11-14.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.

…on-flow-and-adapt-to-return-result-of-connection-to-pos

…of risk of breaking the current connection flow

…host

dangermattic · 2024-05-29T15:06:52Z

	6 Warnings
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️	Class `WooPosCardReaderEvent` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	Class `WooPosCardReaderFacade` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	Class `WooPosCardReaderMode` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	Class `WooPosCardReaderPaymentResult` is missing tests, but `unit-tests-exemption` label was set to ignore this.
⚠️	Class `WooPosCardReaderViewModel` is missing tests, but `unit-tests-exemption` label was set to ignore this.

	1 Message
📖	This PR contains changes to Tracks-related logic. Please ensure (author and reviewer) the following are completed: The tracks events must be validated in the Tracks system. Verify the internal Tracks spreadsheet has also been updated. Please consider registering any new events. The PR must be assigned the category: tracks label.

Generated by 🚫 Danger

wpmobilebot · 2024-05-29T15:28:32Z

📲 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
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`e0dbb5b`
	Direct Download	`woocommerce-prototype-build-pr11616-e0dbb5b.apk`

…d state

codecov-commenter · 2024-05-30T15:58:34Z

Codecov Report

Attention: Patch coverage is 2.94118% with 99 lines in your changes are missing coverage. Please review.

Project coverage is 39.23%. Comparing base (d238d06) to head (e0dbb5b).
Report is 9 commits behind head on trunk.

Files	Patch %	Lines
.../ui/woopos/cardreader/WooPosCardReaderViewModel.kt	0.00%	34 Missing ⚠️
...oid/ui/woopos/cardreader/WooPosCardReaderFacade.kt	0.00%	20 Missing ⚠️
...roid/ui/woopos/cardreader/WooPosCardReaderEvent.kt	0.00%	12 Missing ⚠️
...ts/methodselection/SelectPaymentMethodViewModel.kt	8.33%	10 Missing and 1 partial ⚠️
...s/cardreader/connect/CardReaderConnectViewModel.kt	20.00%	1 Missing and 3 partials ⚠️
...d/ui/woopos/root/navigation/WooPosMainFlowGraph.kt	0.00%	3 Missing ⚠️
...ndroid/ui/woopos/root/navigation/WooPosRootHost.kt	0.00%	3 Missing ⚠️
...dreader/onboarding/CardReaderOnboardingFragment.kt	33.33%	2 Missing ⚠️
...reader/onboarding/CardReaderOnboardingViewModel.kt	0.00%	2 Missing ⚠️
...droid/ui/woopos/cardreader/WooPosCardReaderMode.kt	0.00%	2 Missing ⚠️
... and 5 more

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #11616      +/-   ##
============================================
- Coverage     39.29%   39.23%   -0.06%     
  Complexity     5252     5252              
============================================
  Files          1104     1108       +4     
  Lines         64318    64410      +92     
  Branches       8868     8886      +18     
============================================
+ Hits          25273    25274       +1     
- Misses        36717    36804      +87     
- Partials       2328     2332       +4

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

backwardstruck · 2024-05-31T03:05:25Z

I got a crash, which I will was able to reproduce:

java.lang.RuntimeException: Unable to start activity ComponentInfo{com.woocommerce.android.dev/com.woocommerce.android.ui.woopos.cardreader.WooPosCardReaderActivity}: java.lang.IllegalStateException: WooPosCardReaderMode not found in savedStateHandle

I tapped on Connect to card reader, saw this screen, then tapped the back arrow on the dialog:

Full stack trace:

--------- beginning of main
--------- beginning of system
--------- beginning of crash
2024-05-30 23:02:27.036  5281-5281  AndroidRuntime          com.woocommerce.android.dev          E  FATAL EXCEPTION: main (Ask Gemini)
                                                                                                    Process: com.woocommerce.android.dev, PID: 5281
                                                                                                    java.lang.RuntimeException: Unable to start activity ComponentInfo{com.woocommerce.android.dev/com.woocommerce.android.ui.woopos.cardreader.WooPosCardReaderActivity}: java.lang.IllegalStateException: WooPosCardReaderMode not found in savedStateHandle
                                                                                                    	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3645)
                                                                                                    	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3782)
                                                                                                    	at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:101)
                                                                                                    	at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
                                                                                                    	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
                                                                                                    	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2307)
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:106)
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:201)
                                                                                                    	at android.os.Looper.loop(Looper.java:288)
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:7872)
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method)
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
                                                                                                    Caused by: java.lang.IllegalStateException: WooPosCardReaderMode not found in savedStateHandle
                                                                                                    	at com.woocommerce.android.ui.woopos.cardreader.WooPosCardReaderViewModel.<init>(WooPosCardReaderViewModel.kt:28)
                                                                                                    	at com.woocommerce.android.DaggerWooCommerceDebug_HiltComponents_SingletonC$ViewModelCImpl$SwitchingProvider.get1(DaggerWooCommerceDebug_HiltComponents_SingletonC.java:7381)
                                                                                                    	at com.woocommerce.android.DaggerWooCommerceDebug_HiltComponents_SingletonC$ViewModelCImpl$SwitchingProvider.get(DaggerWooCommerceDebug_HiltComponents_SingletonC.java:7460)
                                                                                                    	at dagger.hilt.android.internal.lifecycle.HiltViewModelFactory$2.createViewModel(HiltViewModelFactory.java:133)
                                                                                                    	at dagger.hilt.android.internal.lifecycle.HiltViewModelFactory$2.create(HiltViewModelFactory.java:104)
                                                                                                    	at dagger.hilt.android.internal.lifecycle.HiltViewModelFactory.create(HiltViewModelFactory.java:171)
                                                                                                    	at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.kt:187)
                                                                                                    	at androidx.lifecycle.ViewModelProvider.get(ViewModelProvider.kt:153)
                                                                                                    	at androidx.lifecycle.ViewModelLazy.getValue(ViewModelLazy.kt:53)
                                                                                                    	at androidx.lifecycle.ViewModelLazy.getValue(ViewModelLazy.kt:35)
                                                                                                    	at com.woocommerce.android.ui.woopos.cardreader.WooPosCardReaderActivity.getViewModel(WooPosCardReaderActivity.kt:15)
                                                                                                    	at com.woocommerce.android.ui.woopos.cardreader.WooPosCardReaderActivity.observeEvents(WooPosCardReaderActivity.kt:28)
                                                                                                    	at com.woocommerce.android.ui.woopos.cardreader.WooPosCardReaderActivity.onCreate(WooPosCardReaderActivity.kt:24)
                                                                                                    	at android.app.Activity.performCreate(Activity.java:8305)
                                                                                                    	at android.app.Activity.performCreate(Activity.java:8284)
                                                                                                    	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1417)
                                                                                                    	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3626)
                                                                                                    	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3782) 
                                                                                                    	at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:101) 
                                                                                                    	at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135) 
                                                                                                    	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95) 
                                                                                                    	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2307) 
                                                                                                    	at android.os.Handler.dispatchMessage(Handler.java:106) 
                                                                                                    	at android.os.Looper.loopOnce(Looper.java:201) 
                                                                                                    	at android.os.Looper.loop(Looper.java:288) 
                                                                                                    	at android.app.ActivityThread.main(ActivityThread.java:7872) 
                                                                                                    	at java.lang.reflect.Method.invoke(Native Method) 
                                                                                                    	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) 
                                                                                                    	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936) 
---------------------------- PROCESS STARTED (7685) for package com.woocommerce.android.dev ----------------------------

kidinov · 2024-05-31T10:41:46Z

@backwardstruck 👋

That's when you started the folow from the POS screen, right? Then this is expected, at least for M1, as we aiming in the "happy path" (it's debatable though if not fully completed onboarding is part of it or not, but in any case it's not part of this PR) More info on that

backwardstruck · 2024-05-31T12:16:32Z

@backwardstruck 👋

That's when you started the folow from the POS screen, right? Then this is expected, at least for M1, as we aiming in the "happy path" (it's debatable though if not fully completed onboarding is part of it or not, but in any case it's not part of this PR) More info on that

Yes, it was in the POS flow. Sounds good!

backwardstruck · 2024-05-31T20:22:48Z

Tested and seems to be working correctly:

√ The most important is to validate that no IPP flow is affected by the changes

√ To validate that the flow works for POS, click on the "connect" button

backwardstruck

Code change look good!

backwardstruck · 2024-05-31T20:26:22Z

FYI I didn't merge yet because the code freeze is still in progress.

…l-includes-connection-flow-and-adapt-to-return-result-of-connection-to-pos [Woo POS] Start card reader payment flow

…adapt-to-return-result-of-connection-to-pos # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/root/WooPosActivity.kt

samiuelson

Thanks for working on this, @kidinov!

LGTM. I briefly tested the app against regressions.

I also added a few code improvement ideas for you to consider, let me know what you think.

...n/kotlin/com/woocommerce/android/ui/payments/methodselection/SelectPaymentMethodViewModel.kt

...in/kotlin/com/woocommerce/android/ui/payments/methodselection/SelectPaymentMethodFragment.kt

...merce/src/main/kotlin/com/woocommerce/android/ui/woopos/cardreader/WooPosCardReaderFacade.kt

...rce/src/main/kotlin/com/woocommerce/android/ui/woopos/root/navigation/WooPosMainFlowGraph.kt

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

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/cart/WooPosCartNavigation.kt

Co-authored-by: Samuel Urbanowicz <samiuelson@gmail.com>

…n-flow-and-adapt-to-return-result-of-connection-to-pos' into 11608-woo-pos-start-cr-connection-flow-and-adapt-to-return-result-of-connection-to-pos

samiuelson

LGTM 🚀

kidinov and others added 17 commits May 27, 2024 08:40

Attempt to open acitvity with card reader

415f42f

Add redirect to card reader tutorial

a9f776b

Remove unnecessary navigate

14ea468

Fix start destination

f06dea9

Facade

01d931f

Merge branch 'pos-card-reader-2' into 11608-woo-pos-start-cr-connecti…

b469387

…on-flow-and-adapt-to-return-result-of-connection-to-pos

Removed unused imports

932e47c

Start card reader activity from facade

ab3485e

Intent to start activity in connection mode

cafa957

Fixed transparent theme

750b210

Extracted connection flow and start it via new activity

c1bef0e

Fixed compilation errors caused by extracting connection nav graph

07575e1

Renamed card reader connect flow id to exclude duplicate

c3d6853

Reverted extraction of the connection nav graph as that causes a lot …

714d050

…of risk of breaking the current connection flow

pass params from VM

8ae6ca5

pass params from VM

6575e38

Figuring out how if tehre is a way to send explicite result from nav …

0355768

…host

kidinov linked an issue May 29, 2024 that may be closed by this pull request

[Woo POS] Start CR connection flow and adapt to return result of connection to POS #11608

Closed

kidinov marked this pull request as draft May 29, 2024 15:04

kidinov added 2 commits May 30, 2024 11:06

Removed attempts to handle cancelation

d5f30c5

Fixed detekt complains

b058315

kidinov requested a review from samiuelson May 30, 2024 09:12

kidinov added the feature: point of sale POS project label May 30, 2024

kidinov added this to the 19.0 milestone May 30, 2024

kidinov added the unit-tests-exemption label May 30, 2024

Pop back stack instead of going up to handle cancelation/not connecte…

26e18db

…d state

kidinov requested a review from backwardstruck May 30, 2024 09:54

kidinov marked this pull request as ready for review May 30, 2024 09:57

kidinov added 4 commits May 30, 2024 16:18

Collect paymnt button and test order creation

b6a99d5

Small refactoring of the events. Setting the correct graph

ccdc8b4

Hack to hide ui of select payment method fragment

d3cee29

Fixed detekt complains

62c7f6c

backwardstruck self-assigned this May 30, 2024

kidinov added 4 commits May 31, 2024 10:08

Pass result of the payment flow

7f3fd8e

Return result to the root activity via suspend function

fc88cbf

Crash if activity is null

30de778

Finish activity after the flow is finished

8dfeb55

backwardstruck approved these changes May 31, 2024

View reviewed changes

Merge pull request #11629 from woocommerce/11609-woo-pos-start-cr-ful…

352ef95

…l-includes-connection-flow-and-adapt-to-return-result-of-connection-to-pos [Woo POS] Start card reader payment flow

samiuelson self-assigned this Jun 3, 2024

Merge branch 'trunk' into 11608-woo-pos-start-cr-connection-flow-and-…

81ccc3a

…adapt-to-return-result-of-connection-to-pos # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/root/WooPosActivity.kt

kidinov added the category: tracks Related to analytics, including Tracks Events. label Jun 3, 2024

samiuelson reviewed Jun 3, 2024

View reviewed changes

kidinov and others added 6 commits June 3, 2024 16:16

Fixed naming and simplified the condition

b736ee9

View is not nullable

60a3f10

Co-authored-by: Samuel Urbanowicz <samiuelson@gmail.com>

Removed redundant super call

fe06a4e

Co-authored-by: Samuel Urbanowicz <samiuelson@gmail.com>

Reader status to the up pf the class

f56268a

Merge remote-tracking branch 'origin/11608-woo-pos-start-cr-connectio…

e23660a

…n-flow-and-adapt-to-return-result-of-connection-to-pos' into 11608-woo-pos-start-cr-connection-flow-and-adapt-to-return-result-of-connection-to-pos

Fixed broken back navigation from tutorial fragment

e0dbb5b

samiuelson approved these changes Jun 3, 2024

View reviewed changes

samiuelson merged commit ed8fa9b into trunk Jun 3, 2024
14 checks passed

samiuelson deleted the 11608-woo-pos-start-cr-connection-flow-and-adapt-to-return-result-of-connection-to-pos branch June 3, 2024 15:20

[Woo POS] Start card reader connection flow and adapt to return result of connection to pos #11616

[Woo POS] Start card reader connection flow and adapt to return result of connection to pos #11616

Uh oh!

Conversation

kidinov commented May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing instructions

Images/gif

Uh oh!

dangermattic commented May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented May 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

backwardstruck commented May 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kidinov commented May 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

backwardstruck commented May 31, 2024

Uh oh!

backwardstruck commented May 31, 2024

Uh oh!

backwardstruck left a comment

Choose a reason for hiding this comment

Uh oh!

backwardstruck commented May 31, 2024

Uh oh!

samiuelson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samiuelson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kidinov commented May 29, 2024 •

edited

Loading

dangermattic commented May 29, 2024 •

edited

Loading

wpmobilebot commented May 29, 2024 •

edited

Loading

codecov-commenter commented May 30, 2024 •

edited

Loading

backwardstruck commented May 31, 2024 •

edited

Loading

kidinov commented May 31, 2024 •

edited

Loading