[No QA] Automatically navigate away from 3DS challenge if transaction disappears#87293
[No QA] Automatically navigate away from 3DS challenge if transaction disappears#87293chuckdries wants to merge 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ea9309c32
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!transaction && !isDenyingTransaction && !denyOutcomeScreen) { | ||
| Navigation.closeRHPFlow(); |
There was a problem hiding this comment.
Wait for Onyx hydration before auto-closing 3DS screen
This effect now closes the flow whenever transaction is falsy, but useOnyx(ONYXKEYS.TRANSACTIONS_PENDING_3DS_REVIEW) can be temporarily undefined before hydration completes (notably on cold start or web refresh). In that loading window, the screen will close even for a still-pending challenge, so users can be kicked out of a valid authorization flow without taking action. Please gate this branch on Onyx metadata (or another explicit “queue loaded” signal) before calling Navigation.closeRHPFlow().
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Experimentally, this is not something that can happen, but even if it did, the 3ds navigation trigger would kick the user right back into the flow once it did hydrate
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
JakubKorytko
left a comment
There was a problem hiding this comment.
Two small comments, overall LGTM
…ndex.tsx Co-authored-by: Jakub Korytko <jakub@korytko.me>
dariusz-biela
left a comment
There was a problem hiding this comment.
I have no further comments 😄
|
Hey @DylanDylann would you mind checklisting this? I know you don't have a real way to test 3DS challenges - you'll have to hard-code the |
| // 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 /> |
There was a problem hiding this comment.
This may cause a flicker, where AlreadyReviewedFailureScreen briefly appears before the deny success page. I suggest removing this.
There was a problem hiding this comment.
I think we can show a loading screen while waiting to navigate to the deny success screen to satisfy the type safe
| // 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) { |
There was a problem hiding this comment.
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.
Reviewer Checklist
Screenshots/VideosScreen.Recording.2026-04-14.at.15.11.55.movAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
@chuckdries Some NITs ☝️ |
Explanation of Change
Adds an effect in AuthorizeTransactionPage which calls
closeRHPFlow()if the transaction disappears and we did not decline it on this device.I'm unsure of whether we want to adjust our breadcrumbs in this case
Fixed Issues
$ #86772
PROPOSAL:
Tests
deny.transaction.auto-navigate.ios-web.mp4
Offline tests
None
QA Steps
None - QA cannot test 3DS transactions
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari