Use correct card ID when returning back from report page#79702
Use correct card ID when returning back from report page#79702jasperhuangg merged 6 commits intoExpensify:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35fc95135b
ℹ️ 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".
| const domain = virtualCard?.domainName; | ||
| const domainCards = Object.values(cardList ?? {}).filter((card) => card?.domainName === domain); | ||
| const physicalCardFromDomain = domainCards.find((card) => !card?.nameValuePairs?.isVirtual); | ||
| const fallbackCardID = physicalCardFromDomain?.cardID ?? domainCards[0]?.cardID ?? cardID; |
There was a problem hiding this comment.
Prefer current virtual card when no physical card exists
The new fallback picks domainCards[0]?.cardID whenever no physical card is found. If a domain has multiple virtual cards but no physical card (e.g., virtual‑only setups), domainCards[0] can be a different card than the one being reported, so the back button will land on the wrong card detail page. This is a regression from the previous behavior that always returned to the current cardID. Consider falling back to cardID (or virtualCard?.cardID) when no physical card is available.
Useful? React with 👍 / 👎.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05295187dd
ℹ️ 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".
| const hasDetectedDomainFraud = cardsToShow?.some((card) => card?.fraud === CONST.EXPENSIFY_CARD.FRAUD_TYPES.DOMAIN); | ||
| const hasDetectedIndividualFraud = cardsToShow?.some((card) => card?.fraud === CONST.EXPENSIFY_CARD.FRAUD_TYPES.INDIVIDUAL); | ||
| const currentPhysicalCard = physicalCards?.find((card) => String(card?.cardID) === cardID); | ||
| const currentPhysicalCard = physicalCards?.find((card) => String(card?.cardID) === cardID) ?? physicalCards?.at(0); | ||
|
|
There was a problem hiding this comment.
Avoid arbitrary physical card fallback
When cardID doesn't match any physical card, this now falls back to physicalCards?.at(0). physicalCards is built from all active cards in the domain, so if a user has more than one physical card (e.g., a suspended old card plus a newly issued open card), the first element depends on object iteration order rather than the intended card. That means the physical card number and the report lost/activate navigation will target the wrong card ID, or the button can disappear because currentPhysicalCard.state is taken from the wrong card. Consider picking the physical card by state or otherwise matching the intended active card.
Useful? React with 👍 / 👎.
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Fix makes sense 👍
| const hasDetectedDomainFraud = cardsToShow?.some((card) => card?.fraud === CONST.EXPENSIFY_CARD.FRAUD_TYPES.DOMAIN); | ||
| const hasDetectedIndividualFraud = cardsToShow?.some((card) => card?.fraud === CONST.EXPENSIFY_CARD.FRAUD_TYPES.INDIVIDUAL); | ||
| const currentPhysicalCard = physicalCards?.find((card) => String(card?.cardID) === cardID); | ||
| const currentPhysicalCard = physicalCards?.find((card) => String(card?.cardID) === cardID) ?? physicalCards?.at(0); |
There was a problem hiding this comment.
Won't this select the incorrect card if there are multiple physical cards? I don't think this fixes the root cause, unless I'm missing something.
There was a problem hiding this comment.
As far as I know, there can be only 1 physical card. @joekaufmanexpensify Can you confirm that?
There was a problem hiding this comment.
New Expensify allows a user to be issued multiple physical cards. This was an upgrade over Expensify Classic where it was previously only possible for a user to have one physical card.
There was a problem hiding this comment.
Got it, I'll check and update
There was a problem hiding this comment.
@joekaufmanexpensify Can a domain have multiple physical cards?
There was a problem hiding this comment.
Got to know this: "A combo card is always 1 physical card that is tied to exactly 1 virtual card".
There was a problem hiding this comment.
@ShridharGoel Hmm, but in your video here there are 2 virtual cards and 1 physical card. Or do I misunderstand what a combo card is? 😄
There was a problem hiding this comment.
Yeah, that's because I had added cards via Onyx. But seems like that's not a valid combination (got to know this yesterday only).
There was a problem hiding this comment.
@ShridharGoel Looking at the code, physicalCards comes from cardsToShow, which would just return all domain cards for this flow.
App/src/pages/settings/Wallet/ExpensifyCardPage/index.tsx
Lines 100 to 105 in 08bc9e0
I think it's therefore possible that just blindly selecting the first physicalCard could result in a regression, unless it's guaranteed that a domain can only have one physical card (which I'm not sure is true).
|
@jjcoffee This is the experience when trying with different combinations. Screen.Recording.2026-02-08.at.3.38.35.PM.mov |
|
@ShridharGoel Could you share the Onyx testing data that you're using? |
This should be usefulAnd this |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-app-2026-02-16_17.18.07.mp4Android: mWeb Chromeandroid-chrome-2026-02-16_17.20.50.mp4iOS: HybridAppios-app-2026-02-16_17.12.15.mp4iOS: mWeb Safariios-safari-2026-02-16_17.10.34.mp4MacOS: Chrome / Safaridesktop-chrome-2026-02-16_16.48.47.mp4 |
|
@ShridharGoel Can you fix the failing checks? 🙏 |
This comment was marked as resolved.
This comment was marked as resolved.
|
@jjcoffee That happens often on the website when used in emulator. Can you try without the changes also to confirm? |
|
@ShridharGoel Thanks, yes it's also showing like that on main so no need to worry about that! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @jasperhuangg 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/jasperhuangg in version: 9.3.22-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.22-4 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.22-4 🚀
|
Explanation of Change
Use correct card ID when returning back from report page.
Fixed Issues
$ #74620
PROPOSAL: #74620 (comment)
Tests
Precondition: User has assigned a physical card but not activated yet
Offline tests
QA Steps
Same.
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))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
Screen.Recording.2026-01-15.at.10.28.56.PM.mov
Android: mWeb Chrome
Screen.Recording.2026-01-15.at.10.29.39.PM.mov
iOS: Native
Screen.Recording.2026-01-15.at.10.32.23.PM.mov
iOS: mWeb Safari
Screen.Recording.2026-01-15.at.10.31.18.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-01-15.at.10.25.46.PM.mov