[NoQA] Send searchKey in reconciliation CSV export so cash back rows are included - #99106
[NoQA] Send searchKey in reconciliation CSV export so cash back rows are included#99106ShridharGoel wants to merge 4 commits into
Conversation
The CSV export serialized the query without searchKey, so the backend treated reconciliation exports as a plain expense search and dropped the Expensify Card cash back arm. search() already sends searchKey; the export now matches that so the exported rows match the viewed rows.
|
@MelvinBot review, can this lead to regressions? |
|
Low regression risk. The change is a targeted, additive fix that mirrors an already-established pattern in the codebase, and it's effectively a no-op for the large majority of searches. Why it's safe:
One thing worth noting (not a regression from this PR): the template/group export path at Verification
I did not run a device test — this is a code-review question, and exercising the reconciliation cash-back path needs a workspace on the Expensify Card cash-back beta that the test account doesn't have, so a UI run wouldn't add signal beyond the static analysis above. |
Cash back is a group-only row (no nested transactions), so selecting it makes |
|
@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] |
|
I think it makes sense to send the searchKey, so that we're sure we're getting the right results from the search |
|
@JS00001 Thanks, can we merge it then? |
|
@ShridharGoel can you add videos please? |
|
@JS00001 Do you mean videos showing the export being done? Asking since screenshots of the exported CSV are already there |
…iation searchKey is preserved
fb682ac to
f6c314c
Compare
|
@JS00001 Added another change and also included a video for that |
|
Jest is failing |
|
Seem unrelated: |
|
Latest main is already merged |
|
The failing tests do seem related: This PR logic changed the logic used to calculate the hash |
|
Got it, shouldn't the tests derive the hash from the runtime suggested searches instead of hardcoding values? Like |
|
Yes, i agree |
|
Thanks, updated. |
Explanation of Change
This PR fixes two bugs that caused Expensify Card cash back rows to disappear from the bank reconciliation view.
Bug 1: CSV export dropped cash back rows
The CSV export built the backend query without
searchKey. Thesearch()call already sendssearchKey, and the backend uses it to pick the right result set. For the bank reconciliation view,searchKeyisreconciliation, and the backend uses it to add the Expensify Card cash back rows.Because the export did not send
searchKey, the backend treated the export as a plain expense search and dropped the cash back rows. This caused two symptoms:Fix: send
searchKeyin the export query, the same waysearch()does, so the exported rows match the rows the user sees.Bug 2: Feed or bank account filter hid cash back rows from the live view
The reconciliation tab identity (
currentSearchKey) is derived by matchingsimilarSearchHashagainst the suggested searches. When a user applied a feed or bank account filter, the filter key was added tosimilarSearchHash, which no longer matched the reconciliation suggested search. This madecurrentSearchKeyundefined, so thesearch()call sent nosearchKey, and the backend skipped the cash back arm entirely.Fix: add
FEEDandBANK_ACCOUNTtosimilarSearchIgnoredFiltersso these filters refine results without changing the tab identity or the backendsearchKey.Fixed Issues
For #94133
PROPOSAL:
Tests
Screen.Recording.2026-08-26.at.10.55.30.PM.mov
Offline tests
QA Steps
N/A - will be tested along with the Cashback feature.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari