[Download receipts] Add "Download receipts" to the Expenses and Reports search pages - #97671
Conversation
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.
|
|
I will update the videos in checklist once it is good to merge |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| const isExpenseSearch = queryJSON?.type === CONST.SEARCH.DATA_TYPES.EXPENSE || searchResults?.search.type === CONST.SEARCH.DATA_TYPES.EXPENSE; | ||
| if (isExpenseSearch && selectedTransactionsKeys.length > 0) { |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
This new options.push({...}) block for the "Download receipts" action is a near-verbatim copy of the block added directly above it for isExpenseReportSearch. Both push an option with the same icon, text, value, shouldCloseModalOnSelect, and an identical onSelected handler (offline guard + exportReceiptsToZip(...) + trackExport(exportID)). The only differences are the guard condition and the single argument passed to exportReceiptsToZip ({reportIDs} vs {transactionIDs}). Duplicating the whole option object increases maintenance overhead — any future change to the action (icon, copy, offline behavior) has to be made in two places.
Consolidate into a single block that computes the argument once, e.g.:
const receiptExportArgs = isExpenseReportSearch && selectedReportIDs.length > 0
? {reportIDs: selectedReportIDs}
: isExpenseSearch && selectedTransactionsKeys.length > 0
? {transactionIDs: selectedTransactionsKeys}
: undefined;
if (receiptExportArgs) {
options.push({
icon: expensifyIcons.Download,
text: translate('common.downloadReceipts'),
value: CONST.SEARCH.BULK_ACTION_TYPES.DOWNLOAD_RECEIPTS,
shouldCloseModalOnSelect: true,
onSelected: () => {
if (isOffline) {
setIsOfflineModalVisible(true);
return;
}
const exportID = exportReceiptsToZip(receiptExportArgs);
trackExport(exportID);
},
});
}Reviewed at: a1fd2d8 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![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: a1fd2d81dd
ℹ️ 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".
| setIsOfflineModalVisible(true); | ||
| return; | ||
| } | ||
| const exportID = exportReceiptsToZip({transactionIDs: selectedTransactionsKeys}); |
There was a problem hiding this comment.
On a grouped Expenses search, selecting a group before its children are fetched stores the selection under the group_... key (the CSV export path already detects this and avoids passing those keys as transaction IDs). This new action passes selectedTransactionsKeys directly to ExportReceiptsToZip, so in that grouped-selection state the request sends a group key instead of real transaction IDs and the receipts export fails or misses the selected group's receipts. Filter/expand grouped selections or disable this action for group-key selections.
Useful? React with 👍 / 👎.
| // transaction ID. Drop those keys so ExportReceiptsToZip only receives valid transaction IDs. | ||
| const transactionIDs = selectedTransactionsKeys.filter((key) => !key.startsWith(CONST.SEARCH.GROUP_PREFIX)); | ||
| if (transactionIDs.length === 0) { | ||
| return; |
There was a problem hiding this comment.
With shouldCloseModalOnSelect: true, the bare return means the menu just closes and nothing happens - no modal, no error, no console warning.
So i think we can call setIsDownloadErrorModalVisible(true); here
|
@truph01 could you please add unit tests to cover the logic changed in this PR? |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
@daledah Good catch! I think we don't need to show the option for no expense report. |
Resolved via a8df631 |
|
🚧 mollfpr has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
We're still waiting to fix the bug mentioned in this comment |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb Chromeandorid-mweb.moviOS: HybridAppi.moviOS: mWeb Safariim.movMacOS: Chrome / SafariScreen.Recording.2026-08-13.at.14.15.40.mov |
|
@truph01 please merge main to fix the failing check |
|
🚧 mollfpr has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mollfpr in version: 9.4.55-0 🚀
|
|
Yes — help site changes are required, and I've drafted them. This PR adds a Download receipts bulk action to the multi-select menu on the Spend > Expenses and Spend > Reports search pages. The most relevant help article — Export Expenses and Reports — documents that same bulk-action menu (including Download as PDF) but didn't mention receipt downloads, so it was out of date. Draft help site PR: #98819 What it updates in
The Download receipts label is taken verbatim from the merged translation string ( @truph01, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
Deploy Blocker #98827 was identified to be related to this PR. |
|
Deploy Blocker #98833 was identified to be related to this PR. |
7 participants
Explanation of Change
Fixed Issues
$ #95355
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include ""[No QA].""
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))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-08-11.at.18.41.23.mov
Android: mWeb Chrome
Screen.Recording.2026-08-11.at.18.38.21.mov
iOS: Native
Screen.Recording.2026-08-11.at.18.38.55.mov
iOS: mWeb Safari
Screen.Recording.2026-08-11.at.18.27.26.mov
MacOS: Chrome / Safari
Screen.Recording.2026-08-11.at.18.26.07.mov