-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[No QA] Automatically navigate away from 3DS challenge if transaction disappears #87293
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: main
Are you sure you want to change the base?
Changes from all commits
7ea9309
ee43091
2bad0b5
fddaef3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import type {SeverityLevel} from '@sentry/react-native'; | ||
| import * as Sentry from '@sentry/react-native'; | ||
| import React, {useCallback, useRef, useState} from 'react'; | ||
| import React, {useCallback, useEffect, useRef, useState} from 'react'; | ||
| import {View} from 'react-native'; | ||
| import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView'; | ||
| import HeaderWithBackButton from '@components/HeaderWithBackButton'; | ||
|
|
@@ -17,6 +17,7 @@ import useBeforeRemove from '@hooks/useBeforeRemove'; | |
| import useLocalize from '@hooks/useLocalize'; | ||
| import useNetworkWithOfflineStatus from '@hooks/useNetworkWithOfflineStatus'; | ||
| import useOnyx from '@hooks/useOnyx'; | ||
| import usePrevious from '@hooks/usePrevious'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import {denyTransaction, fireAndForgetDenyTransaction} from '@libs/actions/MultifactorAuthentication'; | ||
| import Navigation from '@libs/Navigation/Navigation'; | ||
|
|
@@ -118,18 +119,31 @@ function MultifactorAuthenticationScenarioAuthorizeTransactionPage({route}: Mult | |
| Navigation.closeRHPFlow(); | ||
| }; | ||
|
|
||
| // Automatically navigate away if transaction becomes nullish and we didn't deny it here | ||
| // User must have actioned it on a different device. | ||
| useEffect(() => { | ||
| if (transaction || isDenyingTransaction || denyOutcomeScreen) { | ||
| return; | ||
| } | ||
| addBreadcrumb('Transaction disappeared (actioned on another device)', {transactionID}, 'info'); | ||
| Navigation.closeRHPFlow(); | ||
| }, [denyOutcomeScreen, transaction, isDenyingTransaction, transactionID]); | ||
|
|
||
| // Keep track of the previous value of transaction to avoid a brief flash in the deny flow | ||
| // Onyx removes the transaction a moment before denyTransaction resolves | ||
| const previousTransaction = usePrevious(transaction); | ||
| const displayTransaction = transaction ?? previousTransaction; | ||
|
|
||
| if (denyOutcomeScreen) { | ||
| return <ScreenWrapper testID={MultifactorAuthenticationScenarioAuthorizeTransactionPage.displayName}>{denyOutcomeScreen}</ScreenWrapper>; | ||
| } | ||
|
|
||
| if (!transaction) { | ||
| // This case should not be possible to reach given the useEffect above, but we must satisfy the type system | ||
| if (!displayTransaction) { | ||
| addBreadcrumb('Transaction unavailable', {transactionID, isDenyingTransaction}, 'warning'); | ||
| // isDenyingTransaction is handled here because: | ||
| // When the transaction denial succeeds, the transaction gets removed from the queue slightly sooner than denyTransaction resolves. | ||
| // We handle this case specially here so that the user does not see a momentary flash of the AlreadyReviewedFailureScreen | ||
| return ( | ||
| <ScreenWrapper testID={MultifactorAuthenticationScenarioAuthorizeTransactionPage.displayName}> | ||
| {isDenyingTransaction ? <DeniedTransactionSuccessScreen /> : <AlreadyReviewedFailureScreen />} | ||
| <AlreadyReviewedFailureScreen /> | ||
|
Comment on lines
+141
to
+146
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may cause a flicker, where AlreadyReviewedFailureScreen briefly appears before the deny success page. I suggest removing this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can show a loading screen while waiting to navigate to the deny success screen to satisfy the type safe |
||
| </ScreenWrapper> | ||
| ); | ||
| } | ||
|
|
@@ -143,7 +157,7 @@ function MultifactorAuthenticationScenarioAuthorizeTransactionPage({route}: Mult | |
| /> | ||
| <FullPageOfflineBlockingView> | ||
| <View style={[styles.flex1, styles.flexColumn, styles.justifyContentBetween]}> | ||
| <MultifactorAuthenticationAuthorizeTransactionContent transaction={transaction} /> | ||
| <MultifactorAuthenticationAuthorizeTransactionContent transaction={displayTransaction} /> | ||
| <MultifactorAuthenticationAuthorizeTransactionActions | ||
| isLoading={isDenyingTransaction} | ||
| onAuthorize={onApproveTransaction} | ||
|
|
||
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.
I suggest adding an isFocused condition here—we should only call closeRHP if the current page is focused. This will avoid triggering closeRHPFlow after navigating to the outcome screen.