Use accounting integration bank accounts for reconciliation#87916
Use accounting integration bank accounts for reconciliation#87916
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
rlinoz
changed the title
melvin-bot
Bot
requested review from
heyjennahay and
lakchote
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a1d622000
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const programKey = getCardProgramKey(cardSettings); | ||
| const settings = getCardSettings(cardSettings, programKey); | ||
| const paymentBankAccountID = settings?.paymentBankAccountID; | ||
| const [reconciliationBankAccountID] = useOnyx(`${ONYXKEYS.COLLECTION.EXPENSIFY_CARD_RECONCILIATION_BANK_ACCOUNT_ID}${workspaceAccountID}`); |
There was a problem hiding this comment.
This screen reads/writes EXPENSIFY_CARD_RECONCILIATION_BANK_ACCOUNT_ID using workspaceAccountID, but CardReconciliationPage reads the same setting using effectiveDomainID (derived from the active card feed/domain). When those IDs differ (for example, domain-linked feeds where workspaceAccountID can be 0), the selected reconciliation account is stored under a different Onyx key than the one the parent page consumes, so the chosen account won’t appear selected/titled correctly after navigation and optimistic updates can be applied to the wrong entry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Seems like this could be a legit concern?
There was a problem hiding this comment.
Looking into this, will probably update tomorrow
blimpich
left a comment
blimpich
left a comment
There was a problem hiding this comment.
Looks good, though the bot comment does seem legit.
Also it looks like there are changes to Mobile-Expensify in this PR? probably accidental?
| const programKey = getCardProgramKey(cardSettings); | ||
| const settings = getCardSettings(cardSettings, programKey); | ||
| const paymentBankAccountID = settings?.paymentBankAccountID; | ||
| const [reconciliationBankAccountID] = useOnyx(`${ONYXKEYS.COLLECTION.EXPENSIFY_CARD_RECONCILIATION_BANK_ACCOUNT_ID}${workspaceAccountID}`); |
There was a problem hiding this comment.
Seems like this could be a legit concern?
rlinoz
changed the title
2 participants
Explanation of Change
ECard reconciliation actually depends on the bank accounts of the integration, and not from the user's wallet, this PR updates the code so it lists the available accounts in the integration and calls the correct command to set that.
Fixed Issues
https://github.com/Expensify/Expensify/issues/623838
PROPOSAL:
Tests
Offline tests
QA Steps
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
Screen.Recording.2026-04-15.at.12.55.58.mov
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari