Fix getting in-wallet card status on iOS#61769
Conversation
|
|
|
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
|
@brunovjk could you do a quick test of the native apps? |
|
@mjasikowski 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] |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
It seems like getting card status doesn’t work on ad-hoc builds. The ad-hoc app probably has different properties, and Expensify Cards aren’t linked with it, and we can’t access it through Apple’s PassKit library
@mountiny, I think it's something that needs to be fixed by Marqeta, but it doesn't block us with this PR, since on dev builds, everything works |
|
Ok yeah I agree, @brunovjk would you be able to complete your testing on this PR ? |
|
Sure! Im on it now! |
|
FYI I checked the edge case where Android wasn't working and everything is fine now 🚀 but of course you'd better double check me @brunovjk 😅 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp - Emulator61769_android_native.movAndroid: HybridApp - Emulator not signed in61769_android_native_no_signed.movAndroid: mWeb Chrome61769_android_web.moviOS: HybridApp61769_ios_native.moviOS: mWeb Safari61769_ios_web.movMacOS: Chrome / Safari61769_web_chrome.movMacOS: Desktop61769_web-desktop.mov |
|
|
||
| /** Card's primary account identifier */ | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| expensifyCard_panReferenceID?: string; |
There was a problem hiding this comment.
@Skalakid is there still a need for the expensifyCard_panReferenceID field here? I noticed it’s not currently used anywhere outside of nameValuePairs. If there’s no plan to use it at the top level, should we consider removing it to keep the type clean?
There was a problem hiding this comment.
I think we need it. In iOS's isCardInWallet wallet function, we are checking either using this value or using the card suffix. The card suffix isn't unique, there can be a situation where 2 cards have the same last 4 numbers. So, checking the state using panReferenceID is better in most situations. However, it's not always present, especially when you add the card manually threw Apple Wallet, so that why we are using the suffix listing too
There was a problem hiding this comment.
Great! Thanks for clarifying 🚀 🚀 🚀
brunovjk
left a comment
There was a problem hiding this comment.
I’ve reviewed and tested the PR as much as possible:
-
On my Android emulator, I can get to the point where I see a not device allowed message, up to this point everything seems fine to me:

-
On my physical Android device (with the latest adhoc build), I couldn’t get as far—the button doesn’t even show up.
-
On all other platforms, I also don’t see the button, but the app runs normally in my tests.
Other than this comment, which I don't think would be a blocker, everything else in the code looks fine to me.
Yeah, I think we need to whitelist ad-hoc builds to be able to use the Google SDK. Right now every call returns |
|
@mountiny as the PR is approved by Bruno, can we go ahead with it? 😊 |
|
Snce this PR fixed mostly the iOS part, and the feature can only be tested on physical devices with signed app builds, I think we can continue testing iOS on prod (since it's behind the beta) |
mountiny
left a comment
There was a problem hiding this comment.
Thanks for testing and we can do more checks in staging/ production while this is behind beeta
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.47-0 🚀
|
|
@mountiny this is ready for QA. Checking it off for now though since it's behind beta |
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.1.47-6 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.1.47-6 🚀
|
|
🚀 Deployed to production by https://github.com/arosiclair in version: 9.1.47-6 🚀
|



Important
Please merge after #60270
Explanation of Change
Recently the Expensify Cards details were updated and now they are properly linked to the Expensify App. This PR changes the identifier that we are passing to the
react-native-walletlibrary so we can choose the correct card from the wallet and check its state. Previously the assumed that the card'sPrimary Account Identifieris just acard tokenreturned from the backend (PR). We couldn't test it, since cards weren't linked properly to the App and PassKit always returned an empty array when trying to list all cards. Now everything works, and after checking theexpensifyCard_panReferenceID, it is a correct Primary Account Identifier.Fixed Issues
$ #60112
PROPOSAL:
Tests
Important
This feature fully works only on signed builds on physical devices. Please test it on staging or production builds
SettingsWalletPageAdd Card to WalletbuttonCard is already in wallettext appearedOffline tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
N/A
Android: mWeb Chrome
N/A
iOS: Native
ios.MP4
iOS: mWeb Safari
N/A
MacOS: Chrome / Safari
N/A
MacOS: Desktop
N/A