-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add reconciliation #67970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add reconciliation #67970
Conversation
|
@ZhenjaHorbach 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] |
|
🚧 @luacmartins has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
1P rate limited the builds 😞 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid-hybrid.mp4Android: mWeb Chromeandroid-mweb.mp4iOS: HybridAppios-hybrid.mp4iOS: mWeb Safariios-mweb.mp4MacOS: Chrome / Safariweb.movMacOS: Desktopdesktop.mov |
|
🟡 Several languages use a
Why this matters: Inaccurate translations directly harm UX and trust (users think we’re referring to personal “reconciliation” instead of accounting reconciliation), it can also negatively impact search discoverability in non-English locales. // de.ts
reconciliation: 'Abgleich',
// fr.ts
reconciliation: 'Rapprochement',
// it.ts
reconciliation: 'Riconciliazione',
// nl.ts
reconciliation: 'Afstemming',
// pl.ts
reconciliation: 'Uzgodnienie',
// pt-BR.ts
reconciliation: 'Conciliação',
// es.ts
reconciliation: 'Conciliación',
// ja.ts
reconciliation: '照合',
// zh-hans.ts
reconciliation: '对账', // already correct |
|
Updated! |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
I addressed this one.
Do you have a video for this? My impression is that this would happen with other filters too, so maybe NAB here |
|
@luacmartins @JmillsExpensify Following the manual testing steps, while on the
Here's a video: Screen.Recording.2025-08-22.at.13.41.38.movLet me know if all these are expected, then I'll add the same video to the reviewer checklist, finish testing and Approve. |
Ah yea, that account doesn't have bank withdrawals, only Ecard ones. My bad!
Nice catch. This seems to happen for the Statements item too if we change posted. I pushed a fix.
I think this is expected. Those LHN should only be highlighted if the query is similar, which adding
I pushed a fix for all of them. Now we check the similarSearchHash, instead of the hash. |
|
Thanks for the fixes, I confirm that all the points from my previous comment were fixed / addressed ✅ |
ikevin127
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested on my own account since the ngrok seems to be down, the only thing is that on my account there were no results for either types, but other than that LGTM.
| const filterSet = new Set<string>(orderedQuery); | ||
|
|
||
| query.flatFilters | ||
| .map((filter) => { | ||
| similarSearchHashInput += filter.key; | ||
| filterSet.add(filter.key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NAB. Should we exclude dates so that their presence does not affect the similar hash value? Currently if you remove the withdrawn filter, the reconciliation LHN item is no longer highlighted (same for statements with posted filter)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure that we want to completely remove those, since those filters are still part of the suggested search. I think we leave it as is for now and maybe reconsider if we think this is disruptive to users.
JS00001
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, grouping will get added in a follow up, right?
Yes |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.99-0 🚀
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.1.99-11 🚀
|



Explanation of Change
Adds the
Reconciliationoption to theReportspage menuFixed Issues
$ #64576
$ #66974
Tests
Pre-requisite:
Reconciliationon the LHNtype:expense withdrawal-type:reimbursement withdrawn:last-monthwithdrawal-typetoexpensify-cardFiltersRHPwithdrawal-typefilterOffline tests
N/A
QA Steps
N/A
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
Screen.Recording.2025-08-21.at.1.53.51.PM.mov
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop