Fix: show Search bulk export options without opening a report first#93297
Conversation
The Search bulk-select export options read the policy and report from live Onyx, which lacks policy.connections on a fresh load until a report is opened. Read from the Search snapshot first (which carries connections), matching the per-row export action logic, so the options show without opening a report. Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
Fixed the failing Root cause: This PR added a call to Fix: Added The only other failing check, |
| const reportPolicy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`]; | ||
| const completeReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`]; | ||
| const reportPolicy = currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`] ?? policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`]; | ||
| const completeReport = getReportFromSearchSnapshot(report.reportID, currentSearchResults?.data, allReports); |
There was a problem hiding this comment.
const completeReport = getReportFromSearchSnapshot(report.reportID, currentSearchResults?.data, allReports);
@MelvinBot This change alone has display the export to integration item, let's remove the the other changes.
There was a problem hiding this comment.
Done. Reverted the policy and reportPolicy lookups back to live Onyx and kept only the completeReport snapshot lookup. The diff is now a single behavioral change:
- const completeReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`];
+ const completeReport = getReportFromSearchSnapshot(report.reportID, currentSearchResults?.data, allReports);Verified locally:
npm test -- tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts→ 43 passedtypecheck-tsgo→ clean- React Compiler check → only the pre-existing
322:0errors remain (identical on the base, unrelated to this change)
Pushed as 8c10f07d3.
There was a problem hiding this comment.
Done — renamed completeReport to snapshotReport (all 3 references in canReportBeExported). Typecheck clean and the unit test still passes (43/43). Pushed as 8092398816c.
…rt snapshot lookup Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
There was a problem hiding this comment.
@MelvinBot fallback to the snapshot report here to be
const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];This will help when checking !report?.isExportedToIntegration below, since the snapshot report already has isExportedToIntegration.
There was a problem hiding this comment.
The onyx report can be an incomplete optimistic record here (e.g. exported offline before it was ever loaded will create incomplete optimistic report to insert pendingFields?.export), so it won't have reportName.
@MelvinBot Falling back to the Search snapshot report ensures the "export again" modal still shows the report name.
const reportName = report?.reportName ?? currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportName;There was a problem hiding this comment.
Done. Since the live Onyx record can win the ?? at line 1318 yet be an incomplete optimistic record without reportName, I fall the name back to the snapshot:
- if (report.reportName) {
- exportedReportNames.push(report.reportName);
+ const reportName = report.reportName ?? currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportName;
+ if (reportName) {
+ exportedReportNames.push(reportName);
}Verified locally: typecheck clean, useSearchBulkActionsDuplicateTest → 43 passed. Pushed as 24f30c6.
Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
|
@MelvinBot update Test steps and QA steps to be Preconditions
@MelvinBot update offline steps to be
|
|
@MelvinBot Mark this PR as ready for review. |
|
@MelvinBot You have the permission, please mark this PR as ready for review. |
melvin-bot
Bot
requested review from
JmillsExpensify and
ahmedGaber93
and removed request for
a team and
JmillsExpensify
|
@ahmedGaber93 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| for (const reportID of selectedReportIDs) { | ||
| const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; | ||
| const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? currentSearchResults?.data?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The helper function getReportFromSearchSnapshot already exists in src/libs/actions/Search.ts (and is imported and used at line 1281 in this same file) to look up a report from the search snapshot with a fallback to allReports. Here on line 1318, the same lookup is done inline but with reversed priority (allReports first, then snapshot), which is inconsistent with the helper's behavior (snapshot first, then allReports).
If the intent is to prefer the live Onyx report over the snapshot here, use getReportFromSearchSnapshot and swap the fallback order inside it, or create a clearly named variant. If the intent is the same as the helper, replace this line with:
const report = getReportFromSearchSnapshot(reportID, currentSearchResults?.data, allReports);Reviewed at: 24f30c6 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
We need live Onyx report first to check report.pendingFields.export more details here #83527 (comment)
|
@trjExpensify @Expensify/design We have an UI edge case when bulk export to integration and we have a big list of reports alreay exported again. 
I think we have two options to fix this:
Screen.Recording.2026-06-12.at.9.46.40.PM.mov
'The following reports have already been exported to QuickBooks Online:\n' +
exportedReportNames.join('\n') +
'\n' +
'Are you sure you want to export them again?'Screen.Recording.2026-06-12.at.9.23.32.PM.mov |
|
I'll defer to Design, but personally, I would go with the second option so the names of the reports can be discovered. |
|
Done — pushed
|
| }, | ||
|
|
||
| confirmModalPromptScrollable: { | ||
| maxHeight: 300, |
There was a problem hiding this comment.
@MelvinBot Add paddingRight: 8 to the scrollview style to prevent the scrollbar from hiding the text below it.
There was a problem hiding this comment.
Done — added paddingRight: 8 to confirmModalPromptScrollable so the scrollbar no longer overlaps the prompt text. Pushed as 1985ca1.
…verlap Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppa.movAndroid: mWeb ChromeScreen.Recording.2026-06-13.at.2.57.31.AM.moviOS: HybridAppScreen.Recording.2026-06-13.at.2.51.25.AM.moviOS: mWeb SafariScreen.Recording.2026-06-13.at.2.42.33.AM.movMacOS: Chrome / SafariScreen.Recording.2026-06-13.at.2.41.13.AM.mov |
|
@MelvinBot Fix the ESLint issue. |
Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
Fixed the failing ESLint check. The |
|
Can we add a regression unit test for the actual fix? |
|
@MelvinBot please implement #93297 (comment) and fix the conflict. |
…rtSnapshot Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com> # Conflicts: # src/components/ConfirmContent.tsx # src/components/ConfirmModal.tsx
…pshot Covers the fix where bulk export integration options appear without first opening a report, by resolving the report from the Search snapshot. Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
🤖 Done — added a regression unit test for the fix and resolved the merge conflict with Regression test: It exercises the actual fix in
Both tests pass locally. Conflict: resolved the conflicts in |
|
@MelvinBot Please fix the ts issue |
Use typed fixtures (createRandomReport, a SearchResults factory) and a Parameters-based mock signature to drop unsafe assertions, the forbidden import() type annotation, and the invalid spread argument. Co-authored-by: ahmedGaber93 <ahmedGaber93@users.noreply.github.com>
|
🤖 Fixed the failing
Verified locally: |
|
🚧 @youssef-lr 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/youssef-lr in version: 9.4.10-0 🚀
Bundle Size Analysis (Sentry): |
Help site review — no changes requiredI reviewed the changes in this PR against the help site articles under Why:
Since no help site changes are needed, I did not create a draft docs PR. If you believe a specific article should be updated to mention this flow, let me know and I'll draft it. @ahmedGaber93, please confirm whether the help site adequately reflects the current behavior, or point me to an article you'd like updated. |
|
🚀 Deployed to staging by https://github.com/youssef-lr in version: 9.4.12-0 🚀
Bundle Size Analysis (Sentry): |
📝 Help site review — no docs changes requiredI reviewed the changes in this PR against the help articles under Why:
No draft docs PR was created since there is nothing to document. If you believe a specific behavior here should be captured in a help article, let me know which workflow and I'll draft it. |
|
🚀 Deployed to staging by https://github.com/youssef-lr in version: 9.4.14-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.4.14-1 🚀
|
6 participants
Explanation of Change
On the Search > Reports page, the bulk-select export options ("Export to NetSuite" / "Mark as manually exported") did not appear on a fresh load (after clearing cache) until the user opened a report first.
Root cause: the bulk-export visibility gate in
useSearchBulkActionsreads the report from live Onyx (allReports).SearchBulkActionsButtonis rendered outsideSearchScopeProvider, so theuseOnyxwrapper does not redirect this read to the Search snapshot. On a fresh load the livereport_<id>is missing entirely, socanReportBeExportedbails early atif (!completeReport) return falseand neither export option is offered. Opening a report firesOpenReport, which populates the livereport_<id>— that is exactly the "open it first" workaround.Fix: resolve the report through the shared
getReportFromSearchSnapshothelper, which prefers the Search snapshot (currentSearchResults.data) and falls back to live Onyx. The snapshot already carries the full report on a fresh load — the same data the per-row Search export action relies on — so the bulk menu now shows the options without opening a report.Two related fallbacks were added so the existing "export again" flow keeps working with snapshot-sourced data:
report_<id>is missing.reportName.The policy reads remain on live Onyx (no snapshot lookup), matching the rest of the export-options logic.
Additionally, the "export again" confirmation modal now supports a scrollable prompt (new
shouldEnablePromptScrollprop onConfirmModal) so that a large list of already-exported reports scrolls within the modal instead of extending beyond the screen, and the "Are you sure you want to export them again?" question now precedes the report list.Fixed Issues
$ #92976
PROPOSAL: #92976 (comment)
Tests
Case 1
Preconditions
Case 2
Offline tests
QA Steps
Same as Test 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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari