Duplicate one report per tick when bulk duplicating reports - #99639
Conversation
bulkDuplicateReports built the entire selection in a single synchronous pass. Every eligible expense fires its own API.write, which applies its optimistic data synchronously, so nothing could paint, persist or drain from the request queue until the last expense of the last report had been created. Yield a macrotask between reports so each report's writes are applied and persisted before the next one starts.
Move the yield below the guards that skip a selected report, so entries with no reportID and entries with no matching report no longer cost a macrotask each for work that never happens. Handle the rejection at the call site as well. Now that the action is async a throw surfaces as a rejected promise rather than propagating out of the press handler, so log it instead of dropping it.
|
@FitseTLT pls ignore. @nkdengineer is going to review this. thanks |
![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: 4d3f1052d5
ℹ️ 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".
Yielding between reports lets the user sign out before the loop resumes. Signing out clears Onyx, so the reports, policies and identity captured when the loop started belong to an account that is no longer active, and the remaining duplicateReport() calls would write that data under the newly active session. Capture the account on entry and bail out after a yield if it changed.
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product.
|
@mukhrr Please merge main. |
|
@nkdengineer done |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-08-28.at.15.15.07.movAndroid: mWeb ChromeScreen.Recording.2026-08-28.at.15.12.13.moviOS: HybridAppScreen.Recording.2026-08-28.at.15.09.23.moviOS: mWeb SafariScreen.Recording.2026-08-28.at.15.06.16.movMacOS: Chrome / SafariScreen.Recording.2026-08-28.at.15.04.26.mov |
nkdengineer
left a comment
nkdengineer
left a comment
There was a problem hiding this comment.
@luacmartins All yours.
|
🚧 luacmartins has triggered a test Expensify/App build. You can view the workflow run here. |
|
Created this issue to add a bulk duplicate report command to the BE https://github.com/Expensify/Auth/issues/24120 |
|
✋ 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/luacmartins in version: 9.4.65-0 🚀
|
Help site review: docs update createdThe code change in this PR needs no help site update on its own — it's an internal performance fix (yield between reports, account-change guard, error logging) with no new feature, label, or flow. But reviewing the article that covers this feature surfaced three UI labels that don't match the product, so I created a draft PR to fix them. Draft PR: #99956 — labeled What was wrong in
|
| Article said | Product actually shows |
|---|---|
| Click Selected | N selected (button carries the count) |
| Choose Duplicate report (bulk flow) | Duplicate reports (plural for 2+) |
The bulk menu item is pluralized on selection count via useSearchBulkActions.ts:2682, resolving against en.ts:9366-9369 — Duplicate report for one, Duplicate reports for two or more. Both bulk sections tell the user to select two or more, so plural is correct there. The selection button label comes from workspace.common.selected in SearchBulkActionsButton.tsx:183.
I also added one sentence noting that reports are duplicated one at a time, so copies appear gradually and the page stays usable — the user-visible result of your change.
The single-report flow was left alone: it uses common.duplicateReport (singular) via useExpenseActions.ts:417, and More is correct.
Verification method and two caveats
Labels were verified against the source strings, not a live session. I drove a web session to capture the labels directly, but the test account had zero expense reports (No reports yet empty state), so the selection button and its menu could not be opened. I fell back to tracing the translation keys to their call sites, which is why the permalinks above are cited rather than screenshots. Please sanity-check the wording against a populated account when you review.
I could not assign the PR to you. GitHub returned 404 on the assignability check — Expensify/App only allows assigning users with push access, and your role there is read. The PR is unassigned; you'll become assignable once you comment on it, or an internal engineer can assign you. Expensify/pullerbear is requested as reviewer, and the draft is confirmed isDraft: true with the HelpDot label applied.
Nav path left unchanged. The tester observed the path as Spend tab → Expense reports → Reports secondary tab rather than a literal Spend > Reports breadcrumb, but that was from an empty-state account, and your own test steps use Spend > Reports. I didn't have confident enough evidence to change it.
@mukhrr, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.4.65-3 🚀
Bundle Size Analysis (Sentry): |
6 participants
Explanation of Change
bulkDuplicateReports()built the whole selection in one synchronous pass. Every eligible expense fires its ownAPI.write, which applies its optimistic Onyx data synchronously, so the JS thread stayed blocked until the last expense of the last report was created. Measured on dev web, that is around 120ms per duplicated expense, so 15 reports with 50 expenses each is roughly 90 seconds in a single tick and the tab dies.This makes the loop yield between reports, so each report's writes are applied and persisted before the next one starts.
A single report of 50 expenses still builds in one pass. The durable fix is a single
DuplicateReportscommand in the BE, and per this comment we keep usinguseBulkDuplicateReportActionuntil that lands.Fixed Issues
$ #98970
PROPOSAL: #98970 (comment)
Tests
Precondition: a workspace with at least 3 expense reports, each holding as many expenses as you can add. The freeze scales with the total number of expenses across the selection, so more expenses per report makes the difference easier to see.
Copy of <source report name>is created for each report you selected.Offline tests
Copy of ...report appears right away in the greyed-out offline style, and that the page stays responsive.QA Steps
Precondition: create at least 15 expense reports with 50+ expenses in each of them.
Copy of <source report name>is created for each report you selected.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
android_app.mp4
Android: mWeb Chrome
android_web.mp4
iOS: Native
ios_app.mp4
iOS: mWeb Safari
ios_web.mp4
MacOS: Chrome / Safari
web.mp4