Repository navigation
[Payment due @huult] Implement AddToDigitalWallet MFA scenario - #104179
chuckdries wants to merge 5 commits into
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Please tag me when it's ready for review. Thank you! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🚧 chuckdries has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
rafecolton
left a comment
There was a problem hiding this comment.
This should fix the bug that causes the MFA error screen to show before the native digital wallet prompt appears. I tested it with this ad-hoc build
From my agent:
Cause: Hermes V1 has a compiler bug. An async arrow function with destructured params doesn’t pause at await. The await resumes at once with undefined. Your scenario action is written that way, so await handleAddCardToWallet(...) returned undefined immediately. MFA then reported failure while the wallet flow kept running in the background.
Why it shows on ad hoc builds: ad hoc builds rebuild the JS with Re.Pack, which keeps async functions as native async. Metro bundles convert async into generators, so they don’t hit the bug.
Verified:
- I reproduced it locally with the same hermesc that ad hoc builds use, 250829098.0.14.
- An ad hoc build with this fix behaved correctly on device. The action waited for the wallet flow, and the failure screen showed only after Apple’s error.
Fix: destructure in the body instead of in the params.
diff --git a/src/components/MultifactorAuthentication/config/scenarios/AddToDigitalWallet.ts b/src/components/MultifactorAuthentication/config/scenarios/AddToDigitalWallet.ts
index e3ed008bbb0..103b1bcda73 100644
--- a/src/components/MultifactorAuthentication/config/scenarios/AddToDigitalWallet.ts
+++ b/src/components/MultifactorAuthentication/config/scenarios/AddToDigitalWallet.ts
@@ -19,7 +19,9 @@ type Payload = {
export default {
allowedAuthenticationMethods: [CONST.MULTIFACTOR_AUTHENTICATION.TYPE.BIOMETRICS_HSM],
- action: async ({card, cardHolderName, cardDescription, signedChallenge, authenticationMethod}, context) => {
+ // Hermes V1 does not suspend at await inside async arrows with destructured parameters, so destructure in the body.
+ action: async (params, context) => {
+ const {card, cardHolderName, cardDescription, signedChallenge, authenticationMethod} = params;
try {
const status = await handleAddCardToWallet(card, cardHolderName, cardDescription, {
authentication: {signedChallenge, authenticationMethod},It also noted:
Separate issue: Apple still rejects the card after our backend returns Marqeta’s payload, with
PKPassKitErrorDomain error 2. So the card add itself still fails.
|
@huult this is ready for review! I know you don't have a UK Expensify Card, so just focus on the code |
|
Thanks! I’ll start reviewing now. |
|
@puneetlath Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
🎯 @huult, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
Explanation of Change
Require UK/EU Expensify cardholders to authenticate through the existing native MFA flow before adding their card to Apple Pay or Google Wallet. Failed or canceled platform authentication stops the flow before native wallet provisioning starts. Other cardholders retain the existing flow.
UK/EU requests to
CreateDigitalWalletinclude a signed challenge and authentication method so the backend can route them internally.AddToDigitalWalletscenarioCreateDigitalWalletis pending prevents its eventual response from opening Google Wallet. This also covers offline cancellation.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/692372
PROPOSAL:
Tests
Add to wallet happy path Android
Add to wallet happy path iOS
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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.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
screen-20261020-084822-1792500452549.mp4
iOS: Native