Update mobile connect flow and UI #1008

jakub-rdx · 2024-06-21T12:46:09Z

Description

For some reason strings.xml are included in the PR content, making this PR look huge. Please disregard xml changes as strings will be added to crowdin once the ui screens will appear in Zeplin.

update the flow to match latest sargon implementation
add handling of deep link requests together with high priority screens & other dapp interactions. This is expected behavior, for now:
missing crowding copies, this will be added before merging.

when we have high priority screen opened and new deep link request comes in, we put it first into queue but display when high priority flow completes
when there is no high priority screen - incoming mobile connect request should interrupt other dapp interaction handling (but not reject it), so when mobile connect handling complete, we should start over handling of the request that was on the screen before mobile connect request came in.

update UI of link screen and success/failure dialogs

How to test

See description

micbakos-rdx · 2024-06-24T07:20:50Z

app/src/main/java/com/babylon/wallet/android/MainActivity.kt

@@ -61,7 +61,10 @@ class MainActivity : FragmentActivity() {
 super.onCreate(savedInstanceState)
 cloudBackupSyncExecutor.startPeriodicChecks(lifecycleOwner = this)

- intent.data?.let { viewModel.handleDeepLink(it) }
+ intent.data?.let {
+ intent.replaceExtras(Bundle())


Very nice that you added this here.

actually can someone explain me what does this line do? Because I see it in the line 103, too.
Does it "clear" the intent with an empty bundle?

it removes data from an intent, so deep link is not delivered 2nd time if you move app to background and open from recent

When the app is moved to background and re-opened from recent neither onCreate nor onNewIntent should get called in normal circumstances, unless the app task has been closed by the OS? Is that the case or there are simple repro steps for the issue that this line fixes? I'm really curious about what is happening.

micbakos-rdx · 2024-06-24T07:32:07Z

app/src/main/java/com/babylon/wallet/android/data/dapp/IncomingRequestRepository.kt

- fun getUnauthorizedRequest(requestId: String): IncomingRequest.UnauthorizedRequest?
-
- fun getTransactionWriteRequest(requestId: String): IncomingRequest.TransactionRequest?
-
- fun getAuthorizedRequest(requestId: String): IncomingRequest.AuthorizedRequest?


Very nice that you removed those 👍

micbakos-rdx

In general the flow seems nice 👍 .

Some comments regarding the experience:

I am not sure what will be the plan when the user doesn't have a profile. There are 2 paths here:
1. The user goes on to create a new profile. Will they be presented with the request?
2. The user just closes the app. Shouldn't we send a cancelation event to the dapp?
Even though I see that you clear the deep link when you first read it in MainActivity I have experienced this:
- The user doesn't have a profile available
- The dApp sends a request
- The user is presented with the modal that they don't have a profile
- I close the app with back or click ok on the dialog.
- Then I open the app from recents.
- I still get the dialog and I cannot continue.
As mentioned in the demo we need to cancel the request when we dismiss the verify screen.
For some reason I cannot dismiss a transaction while a dapp request is pending:
- Open the app and try a transfer between 2 accounts.
- Stay in the transaction review screen
- Then open a dapp and perform a connect
- This navigates back to wallet were the transaction screen is visible
- In this case now I cannot dismiss it. Both back and X button do not perform back.
After doing the above steps, without trying to dismiss the pending transfer, I complete the transfer. Then I am not presented with the dapp request.

app/src/main/java/com/babylon/wallet/android/domain/usecases/deeplink/ProcessDeepLinkUseCase.kt

app/src/main/java/com/babylon/wallet/android/presentation/main/MainViewModel.kt

app/src/main/java/com/babylon/wallet/android/presentation/navigation/NavigationHost.kt

app/src/main/java/com/babylon/wallet/android/presentation/navigation/PriorityRoutes.kt

giannis-rdx

tested and works flawlessly!

I left some comments only about code.

app/src/main/java/com/babylon/wallet/android/data/dapp/IncomingRequestRepository.kt

giannis-rdx · 2024-06-25T08:05:50Z

app/src/main/java/com/babylon/wallet/android/data/dapp/IncomingRequestRepository.kt

+ * - there is no high priority screen: request is added at the top of queue and if there other request currently handled,
+ * we send a dismiss event for it so that UI can react and dismiss handling, without removing it from the queue.


"there is no high priority screen: request is added at the top of queue and if there other request currently handled, ..."

here you mean that if wallet receives another high priority request while another non high priority is being handled then we dismiss it and so on... right?

Actually what I am thinking is that the term dismiss is not the right one to describe this particular business logic. "Dismiss" is like reject. We reject this request and we will never see it again.

But what if we use postpone which describes that we delay the handling of this request?
so it becomes override suspend fun requestPostponed and any other "dismiss" rename to "postpone"?

I would like to hear the opinion of others. Also any other word is welcome!

oh now I think defer is even better.
override suspend fun requestDeferred

yes, whenever this method is used, request added with it is supposed to go on screen while current one is deferred. This does not apply if we are in flows like creating accounts or other flows marked as high priority.

This does not apply if we are in flows like creating accounts or other flows marked as high priority.

wait I think I lost it here, you mean we really dismiss the request in this case?

no, we never dimiss when using addMobileConnectRequest(). It is just that we don't process it immediately. Example: when mobile connect request (MCR) comes in and we are in the middle of some other dApp login, dapp login is deferred and MCR takes over. But when we are in a wallet doing something, like account creation (high priority flow), then request is delivered after that particular screen/flow is complete.

nice thank you

…flow # Conflicts: # app/src/main/java/com/babylon/wallet/android/MainActivity.kt # app/src/main/java/com/babylon/wallet/android/presentation/navigation/NavigationHost.kt # app/src/main/res/values-en/strings.xml # gradle/libs.versions.toml

giannis-rdx

please rename the rest of the "dismiss" words in order to be consistent

app/src/main/java/com/babylon/wallet/android/utils/AppEventBusImpl.kt

app/src/test/java/com/babylon/wallet/android/data/dapp/IncomingRequestRepositoryTest.kt

app/src/main/java/com/babylon/wallet/android/data/dapp/IncomingRequestRepository.kt

micbakos-rdx · 2024-06-26T14:07:03Z

app/src/main/java/com/babylon/wallet/android/data/dapp/IncomingRequestRepository.kt

+ * we send a defer event for it so that UI can react and defer handling, without removing it from the queue.
+ * Deferred request will be handled again when top priority one handling completes
+ */
+ override suspend fun addMobileConnectRequest(incomingRequest: IncomingRequest) {


Maybe addDeferredRequest instead of naming it to mobile connect

this is not deferred request, mobile connect ones take priority over others. Will rename to addPriorityRequest

micbakos-rdx · 2024-06-26T14:08:37Z

app/src/main/java/com/babylon/wallet/android/data/dapp/IncomingRequestRepository.kt

+ }
+ }
+
+ override suspend fun requestDeferred(requestId: String) {


It is not actually deferred here? This is like a yield action right?

It is deferred, but this method works in tandem with addMobileConnectRequest. When you use the latter, new request is put in front, and calling requestDeferred is simply taking next request to handle without removing current from the queue. I couldn't do both those actions in one method as there are user interactions in between.

micbakos-rdx · 2024-06-26T14:11:47Z

app/src/main/java/com/babylon/wallet/android/domain/usecases/VerifyDAppUseCase.kt

+ request = request,
+ error = DappWalletInteractionErrorType.INVALID_REQUEST
+ )
+ return Result.failure(RadixWalletException.DappRequestException.WrongNetwork(networkId, request.metadata.networkId))


Can you please add the names of the parameters here? We had a bug in the past were the networkId and request.metadata.networkId were reversed.

micbakos-rdx · 2024-06-26T14:19:58Z

...java/com/babylon/wallet/android/presentation/transaction/submit/TransactionSubmitDelegate.kt

- _state.update {
- it.copy(isSubmitting = false)
+ ): Result<Unit> {
+ return runCatching {


You can inline this

sergiupuhalschi-rdx

Works very nice and smooth! Great job!

sergiupuhalschi-rdx · 2024-06-26T15:46:11Z

app/src/main/java/com/babylon/wallet/android/MainActivity.kt

@@ -61,7 +61,10 @@ class MainActivity : FragmentActivity() {
 super.onCreate(savedInstanceState)
 cloudBackupSyncExecutor.startPeriodicChecks(lifecycleOwner = this)

- intent.data?.let { viewModel.handleDeepLink(it) }
+ intent.data?.let {
+ intent.replaceExtras(Bundle())


When the app is moved to background and re-opened from recent neither onCreate nor onNewIntent should get called in normal circumstances, unless the app task has been closed by the OS? Is that the case or there are simple repro steps for the issue that this line fixes? I'm really curious about what is happening.

giannis-rdx · 2024-06-27T08:37:03Z

...in/java/com/babylon/wallet/android/data/repository/BufferedMobileConnectRequestRepository.kt

+ private var bufferedRequest: IncomingMessage.IncomingRequest? = null
+
+ fun setBufferedRequest(request: IncomingMessage.IncomingRequest) {
+ bufferedRequest = request
+ }
+
+ fun getBufferedRequest(): IncomingMessage.IncomingRequest? {
+ return bufferedRequest?.also { bufferedRequest = null }
+ }


actually why not to have this in the IncomingRequestRepository ? 🤷🏽‍♂️

I was thinking the same thing, more than that, this class just holds some data, so it looks more like a data source than a repository. The repository usually is the component abstracting the data sources, serving as an intermediary between the data sources and the business logic.

You are right, will make the change

sonarcloud · 2024-06-27T09:18:40Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarCloud

giannis-rdx · 2024-06-27T09:29:46Z

app/src/main/java/com/babylon/wallet/android/data/dapp/IncomingRequestRepository.kt

+ /**
+ * Request that can come in via deep link before wallet is setup.
+ */


update mobile connect flow and UI

bb0708d

jakub-rdx requested review from giannis-rdx, micbakos-rdx and sergiupuhalschi-rdx June 21, 2024 12:46

micbakos-rdx reviewed Jun 24, 2024

View reviewed changes

jakub-rdx added 3 commits June 24, 2024 12:13

update mobile connect requests handling

9508b28

buffer deep link when wallet is not set up

53ae79c

remove app link scheme

dfa3603

giannis-rdx reviewed Jun 25, 2024

View reviewed changes

jakub-rdx added 2 commits June 25, 2024 11:03

changes after merge with main

2e0da3b

jakub-rdx requested review from giannis-rdx and micbakos-rdx June 25, 2024 10:46

giannis-rdx approved these changes Jun 25, 2024

View reviewed changes

jakub-rdx added 5 commits June 25, 2024 13:12

downgrade okhttp to match the one in sargon

67b9c8a

update 'dismiss' request wording with defer

e6a77e1

dismiss -> defer wording in tests

d5536e9

update mobile connect request buffering and requests verification logic

e0aa548

push missing code

3d1af37

micbakos-rdx approved these changes Jun 26, 2024

View reviewed changes

sergiupuhalschi-rdx approved these changes Jun 27, 2024

View reviewed changes

pr review changes

b0c2275

giannis-rdx reviewed Jun 27, 2024

View reviewed changes

buffer request in incoming request repository

6b03334

giannis-rdx reviewed Jun 27, 2024

View reviewed changes

giannis-rdx approved these changes Jun 27, 2024

View reviewed changes

jakub-rdx merged commit 362f82a into feature/m2m-poc Jun 27, 2024
7 of 9 checks passed

jakub-rdx deleted the feature/mobile-connect-optimized-flow branch June 27, 2024 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update mobile connect flow and UI #1008

Update mobile connect flow and UI #1008

jakub-rdx commented Jun 21, 2024 •

edited

Loading

micbakos-rdx Jun 24, 2024

giannis-rdx Jun 25, 2024

jakub-rdx Jun 25, 2024

sergiupuhalschi-rdx Jun 26, 2024

micbakos-rdx Jun 24, 2024

micbakos-rdx left a comment

giannis-rdx left a comment

giannis-rdx Jun 25, 2024

giannis-rdx Jun 25, 2024

giannis-rdx Jun 25, 2024

jakub-rdx Jun 25, 2024

giannis-rdx Jun 25, 2024

jakub-rdx Jun 25, 2024

giannis-rdx Jun 25, 2024

giannis-rdx left a comment

micbakos-rdx Jun 26, 2024

jakub-rdx Jun 27, 2024 •

edited

Loading

micbakos-rdx Jun 26, 2024

jakub-rdx Jun 27, 2024

micbakos-rdx Jun 26, 2024

micbakos-rdx Jun 26, 2024

sergiupuhalschi-rdx left a comment

sergiupuhalschi-rdx Jun 26, 2024

giannis-rdx Jun 27, 2024

sergiupuhalschi-rdx Jun 27, 2024

jakub-rdx Jun 27, 2024

sonarcloud bot commented Jun 27, 2024

giannis-rdx Jun 27, 2024

		* - there is no high priority screen: request is added at the top of queue and if there other request currently handled,
		* we send a dismiss event for it so that UI can react and dismiss handling, without removing it from the queue.

Update mobile connect flow and UI #1008

Update mobile connect flow and UI #1008

Conversation

jakub-rdx commented Jun 21, 2024 • edited Loading

Description

How to test

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

micbakos-rdx left a comment

Choose a reason for hiding this comment

giannis-rdx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giannis-rdx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakub-rdx Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergiupuhalschi-rdx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jun 27, 2024

Quality Gate failed

Choose a reason for hiding this comment

jakub-rdx commented Jun 21, 2024 •

edited

Loading

jakub-rdx Jun 27, 2024 •

edited

Loading