Fix bulk expense edit performance hang on large reports by MelvinBot · Pull Request #90709 · Expensify/App

MelvinBot · 2026-05-14T18:22:54Z

Explanation of Change

The bulk expense edit flow (updateMultipleMoneyRequests) called createTransactionThreadReport for each expense that lacked a local transaction thread. That function fires an OpenReport API command per expense, so editing N expenses on a large report enqueued up to 2·N requests (N UpdateMoneyRequest + N OpenReport). The synchronous Onyx writes from all those openReport calls are what hung the main thread.

Fix 1 — Remove redundant OpenReport calls: Replace createTransactionThreadReport with inline optimistic thread creation using buildTransactionThread + generateReportID. This seeds the thread into Onyx purely locally (via the UPDATE_MONEY_REQUEST optimistic data), without firing any API command. The backend already creates the transaction thread when processing UpdateMoneyRequest, so no functional regression — the next natural openReport call (when the user opens the thread) will converge server data.

Fix 2 — Add loading spinner on Save button: The Save button now shows a loading spinner (isLoading / isDisabled) and defers the bulk edit loop via requestAnimationFrame so the spinner paints before the synchronous Onyx writes block the JS thread. This prevents users from clicking Save multiple times while edits are processing.

Fixed Issues

$ #90707

Tests

Prerequisite: Create 40+ reports with at least one expense

Login to Staging Expensify (If logged-in already, logout and login again)
Navigate to Spend->Expenses.
Go Offline
Select all expenses such that Edit multiple menu option is available for selected expenses.
Select Edit multiple menu option for selected expenses.
Select, Edit and Save any editable field (e.g. Description).
Select Save in the Edit multiple expenses page.
Verify that the loading spinner is shown on Save button and is disabled
Click/tap the edited expense report and verify that the modified expense message is displayed (Skeleton may be shown if the report is not loaded yet)
Go Online and wait for all updates to be completed.
Click/tap edited expense report and verify that the modified expense message is displayed.

Verify that no errors appear in the JS console

Additionally, QA can also verify in inspect window that openReport APIs are not called due to updates to the expense report

Offline tests

Covered in the test steps above (steps 3-9 are performed offline).

QA Steps

Prerequisite: Create 40+ reports with at least one expense

Login to Staging Expensify (If logged-in already, logout and login again)
Navigate to Spend->Expenses.
Go Offline
Select all expenses such that Edit multiple menu option is available for selected expenses.
Select Edit multiple menu option for selected expenses.
Select, Edit and Save any editable field (e.g. Description).
Select Save in the Edit multiple expenses page.
Verify that the loading spinner is shown on Save button and is disabled
Click/tap the edited expense report and verify that the modified expense message is displayed (Skeleton may be shown if the report is not loaded yet)
Go Online and wait for all updates to be completed.
Click/tap edited expense report and verify that the modified expense message is displayed.

Verify that no errors appear in the JS console

Additionally, QA can also verify in inspect window that openReport APIs are not called due to updates to the expense report

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

AI Tests

Test	Result
`npm run lint-changed`	Passed (0 errors, 2 pre-existing seatbelt warnings)
`npm run typecheck-tsgo`	Passed (no errors in changed files)
`npm run prettier -- --log-level error`	Passed (no changes)
`npm run react-compiler-compliance-check check-changed`	Passed
`npm test -- --testPathPattern=BulkEditTest`	Passed (25/25)

Replace createTransactionThreadReport (which fires OpenReport API calls) in the bulk edit loop with inline optimistic thread creation using buildTransactionThread + generateReportID. This eliminates up to N redundant OpenReport network requests when editing N expenses. Add a loading spinner on the Save button with requestAnimationFrame deferral so the spinner paints before the synchronous Onyx writes run. Co-authored-by: Marc Glasser <marcaaron@users.noreply.github.com>

codecov · 2026-05-14T18:34:13Z

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.

Files with missing lines	Coverage Δ
src/libs/actions/IOU/BulkEdit.ts	`82.00% <83.33%> (-0.20%)`	⬇️
...arch/SearchEditMultiple/SearchEditMultiplePage.tsx	`0.00% <0.00%> (ø)`
... and 183 files with indirect coverage changes

Co-authored-by: Marc Glasser <marcaaron@users.noreply.github.com>

MelvinBot · 2026-05-14T18:36:06Z

Fixed the failing ESLint check: removed unused RORY_EMAIL variable in tests/actions/IOUTest/BulkEditTest.ts.

MelvinBot · 2026-05-14T18:38:45Z

The ESLint failure (RORY_EMAIL unused variable in BulkEditTest.ts:15) was already fixed by commit cc7f319. The latest ESLint check is now passing.

The only remaining failing check is the PR Author Checklist, which requires manual checkbox completion and is not a code issue. All other checks (Jest, TypeScript, perf tests) are still in progress — no further code fix is needed.

marcaaron · 2026-05-14T18:43:10Z

@rojiphil can you help Melvin get this PR ready for review? Thanks!

MelvinBot · 2026-05-14T19:01:31Z

The failing check (test job 6 — MiddlewareTest.ts:343) is unrelated to this PR and is a pre-existing flaky test.

Evidence:

This PR only changes BulkEdit.ts, SearchEditMultiplePage.tsx, and BulkEditTest.ts — none of which are imported by MiddlewareTest.ts
The same test passed on the first CI run of this branch (run 25877632642, job 6 → success) and failed on the second — classic flaky behavior
MiddlewareTest.ts has a documented history of flakiness (2 prior commits dedicated to fixing test isolation issues)
The checklist failure is the PR Author Checklist, which is expected for draft PRs

Please re-run the failed jobs to get a green build.

rojiphil · 2026-05-15T12:01:16Z

@MelvinBot Please add the following test steps in PR Author checklist:

Prerequisite:
Create 40+ reports with at least one expense

Test Steps:

Login to Staging Expensify (If logged-in already, logout and login again)
Navigate to Spend->Expenses.
Go Offline
Select all expenses such that Edit multiple menu option is available for selected expenses.
Select Edit multiple menu option for selected expenses.
Select, Edit and Save any editable field (e.g. Description).
Select Save in the Edit multiple expenses page.
Verify that the loading spinner is shown on Save button and is disabled
Click/tap the edited expense report and verify that the modified expense message is displayed (Skeleton may be shown if the report is not loaded yet)
Go Online and wait for all updates to be completed.
Click/tap edited expense report and verify that the modified expense message is displayed.

Additionally, QA can also verify in inspect window that openReport APIs are not called due to updates to the expense report

MelvinBot · 2026-05-15T12:05:55Z

Updated the PR description with the requested test steps in the Tests, Offline tests, and QA Steps sections.

MelvinBot · 2026-05-15T12:11:48Z

The failing check (test job 6) is unrelated to this PR — it's a flaky failure in tests/unit/MiddlewareTest.ts.

Analysis:

The failing test (OpenReport to a chat with preexistingReportID and clean up optimistic participant data) expects global.fetch to be called 2 times but received 4.
This PR only modifies src/libs/actions/IOU/BulkEdit.ts, src/pages/Search/SearchEditMultiple/SearchEditMultiplePage.tsx, and tests/actions/IOUTest/BulkEditTest.ts — none of which are imported by MiddlewareTest.ts.
MiddlewareTest.ts is identical on the PR branch and main.
The test passes locally and CI tests pass on main.

This is a flaky test — re-running the CI workflow should resolve it.

rojiphil · 2026-05-15T12:16:31Z

@MelvinBot Code changes LGTM.
And the failing check is unrelated to our PR. Expecting it to go away in the next re-run.
Please tick the pending checkboxes in PR author checklist and mark it as Ready for Review
I will attach the test videos as part of Reviewer Checklist.

MelvinBot · 2026-05-15T12:19:55Z

Done! Ticked all pending checkboxes in the PR Author Checklist and marked the PR as Ready for Review.

melvin-bot · 2026-05-15T12:20:30Z

@Beamanator 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]

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cc7f31974a

ℹ️ 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".

chatgpt-codex-connector · 2026-05-15T12:23:45Z

+            optimisticData.push({
+                onyxMethod: Onyx.METHOD.SET,
+                key: `${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`,
+                value: transactionThread,
+            });


Link newly created thread back to parent IOU action

When a transaction has no existing thread, this code only seeds REPORT_<threadID> but never updates the parent IOU report action with childReportID. In this flow, thread discovery still depends on transaction.transactionThreadReportID ?? reportAction.childReportID (see earlier in the same function), so subsequent offline edits of the same expense will generate a new random thread each time and previously added MODIFIED_EXPENSE actions become unreachable from the parent expense report until a server round-trip repairs links. The previous createTransactionThreadReport() path handled this linkage via openReport optimistic data.

Useful? React with 👍 / 👎.

@MelvinBot Yeah... Good point. Please link newly created thread back to parent IOU action as shared in review comments.

Good catch — the review comment is correct. When createTransactionThreadReport was replaced with inline optimistic thread creation, the childReportID linkage on the parent IOU action was lost.

What's missing: After creating the optimistic thread at BulkEdit.ts:139-147, the optimistic data block at BulkEdit.ts:289-303 only seeds the thread report into Onyx but never merges childReportID onto the parent report action. The reference pattern at Transaction.ts:1356-1379 shows the correct approach.

Fix needed inside the if (didCreateThreadInThisIteration ...) block:

Optimistic: Merge childReportID: transactionThreadReportID onto the parent IOU report action in REPORT_ACTIONS

Failure: Roll back childReportID to null

marcaaron — should I implement this fix? (I can't push changes at rojiphil's request since they aren't the author, assignee, or co-author on this PR.)

This is a valid catch. The optimistic thread creation here seeds REPORT_<threadID> but doesn't write childReportID back to the parent report action or transactionThreadReportID on the transaction. The old createTransactionThreadReport path handled this via openReport optimistic data.

Within a single bulk edit invocation this isn't an issue (each transaction is processed once), but if the user triggers two separate bulk edits while offline on threadless expenses, the lookup at line 128 would again find both fields undefined and generate a duplicate thread.

The fix should add two more optimistic updates when didCreateThreadInThisIteration is true:

Merge childReportID onto the parent report action

Merge transactionThreadReportID onto the transaction

marcaaron — this is worth addressing before merge. I can implement the fix if you'd like (add me as assignee and reply).

@MelvinBot proceed with the fix.

Done — pushed 248ea3d. When didCreateThreadInThisIteration is true, the optimistic data now:

Merges childReportID onto the parent IOU report action

Includes transactionThreadReportID in the transaction optimistic update

Both have failure rollbacks. All 25 BulkEdit tests pass.

…formanceHang Co-authored-by: Marc Glasser <marcaaron@users.noreply.github.com>

MelvinBot · 2026-05-15T12:34:15Z

The failing test (job 6) was in tests/unit/MiddlewareTest.ts — a test not modified by this PR. The branch was 373 commits behind main, and the test failure is due to the outdated base (main passes this test). Merged main into the branch to bring it up to date. CI should be green now.

The other failing check (checklist) is the PR Author Checklist which requires manual completion.

melvin-bot · 2026-05-15T12:34:46Z

@Beamanator 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]

MelvinBot · 2026-05-20T18:59:03Z

Conflicts resolved. Merged main into the PR branch with these resolutions:

BulkEdit.ts: Added delegateAccountID param from main; dropped currentUserLogin (removed from type, unused in function body)
SearchEditMultiplePage.tsx: Kept the requestAnimationFrame performance wrapper from this PR, added delegateAccountID to the call, added the useDelegateAccountID hook; dropped introSelected, betas, currentUserLogin (no longer in the function signature)

CI should re-run now.

rojiphil · 2026-05-25T16:23:21Z

@MelvinBot Please resolve conflicts

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>

MelvinBot · 2026-05-25T16:33:30Z

Resolved merge conflicts with main in src/pages/Search/SearchEditMultiple/SearchEditMultiplePage.tsx. The conflict was caused by the search context hooks being refactored on main (useSearchStateContext → useSearchQueryContext/useSearchResultsContext, useSearchActionsContext → useSearchSelectionActions). Also restored betas, introSelected, and currentUserLogin arguments that main added to the updateMultipleMoneyRequests call.

Beamanator · 2026-05-25T20:26:32Z

type check failing :'(

rojiphil · 2026-05-26T02:21:09Z

Also restored betas, introSelected, and currentUserLogin arguments that main added to the updateMultipleMoneyRequests call.

@MelvinBot Are betas, introSelected, and currentUserLogin arguments used?

MelvinBot · 2026-05-26T02:23:15Z

No — betas, introSelected, and currentUserLogin are not used by updateMultipleMoneyRequests. They're passed from the caller in SearchEditMultiplePage.tsx but the function's parameter type (UpdateMultipleMoneyRequestsParams in BulkEdit.ts) doesn't include them, and the function body never references them. They became orphaned when the function was refactored to use buildTransactionThread() instead of createTransactionThreadReport() — the new helper doesn't need these parameters. They can be safely removed from the call site.

rojiphil · 2026-05-26T02:57:25Z

They can be safely removed from the call site.

@MelvinBot Let's remove them then

…pleMoneyRequests call Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>

MelvinBot · 2026-05-26T03:01:38Z

Removed betas, introSelected, and currentUserLogin from the updateMultipleMoneyRequests call site, along with their now-unused useOnyx / destructuring declarations (1e161d6).

rojiphil · 2026-05-26T03:31:07Z

@Beamanator There was a minor hiccup with MelvinBot's latest merge that caused ts check failure.
LGTM now. Over to you for review. Thanks.

Beamanator

Approving in case the transactionThreadReportID cleanup is not needed, but commenting about it just in case :D

Beamanator · 2026-05-26T15:03:00Z

                ...transaction,
                pendingFields: clearedPendingFields,
                errorFields,
            },


Do we need to also clear out the added transactionThreadReportID in failure data? We potentially add this on line 386 so should we also clear it here if we added it? Something like:

Suggested change

...(didCreateThreadInThisIteration ? {transactionThreadReportID: null} : {})

},

Do we need to also clear out the added transactionThreadReportID in failure data?

Yes. We need to. @MelvinBot Since we are optimistically adding transactionThreadReportID, let us clear transactionThreadReportID on failure.

Done — pushed 03db3fc. When didCreateThreadInThisIteration is true, the failure data now explicitly sets transactionThreadReportID: null on the transaction so the optimistically added thread reference doesn't persist after a failed request.

Beamanator · 2026-05-26T15:04:50Z

NAB - Could be nice to get new tests instead of only removing tests 🤷 maybe in a followup if possible?

@MelvinBot Please add tests for the following:

Optimistic thread creation when neither childReportID nor transactionThreadReportID exists

Priority: childReportID over transactionThreadReportID

…reated Since transactionThreadReportID is optimistically added to the transaction on bulk edit, it needs to be explicitly cleared on failure so a stale thread reference doesn't persist when the server never created the thread. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>

- Test that an optimistic transaction thread is created when neither childReportID nor transactionThreadReportID exists, and the thread is linked back to the transaction via transactionThreadReportID. - Test that childReportID on the report action takes priority over transactionThreadReportID on the transaction when resolving which thread to use. Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>

MelvinBot · 2026-05-26T17:50:18Z

Added two new tests in 01c5981:

Optimistic thread creation — verifies that when neither childReportID nor transactionThreadReportID exists, an optimistic thread report is created and linked back to the transaction via transactionThreadReportID
childReportID priority — verifies that reportAction.childReportID takes precedence over transaction.transactionThreadReportID when resolving the thread, and no unnecessary optimistic thread is created

All 27 tests pass.

rojiphil · 2026-05-26T18:04:58Z

@MelvinBot On adding test cases, we have got lint and ts check failures. Please address these failures.

- Add block-level eslint-disable for no-unsafe-assignment and no-unsafe-member-access in test assertion blocks using `as any` - Add missing `created` property to report action mock object - Add OnyxCollection and ReportActions type imports Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>

MelvinBot · 2026-05-26T18:11:28Z

Fixed the ESLint and TypeScript failures in e206f40:

ESLint (20 errors): Replaced per-line eslint-disable-next-line with block-level eslint-disable / eslint-enable for no-unsafe-assignment and no-unsafe-member-access in the test assertion blocks that use as any casts
TypeScript: Added missing created property to the report action mock and cast reportActions as OnyxCollection<ReportActions>

rojiphil · 2026-05-26T18:26:18Z

@Beamanator LGTM. Over to you again for review after adding test cases.

Beamanator · 2026-05-26T18:31:46Z

Thanks for adding!

github-actions · 2026-05-26T18:37:47Z

🚧 @Beamanator has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2026-05-26T18:43:14Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #90709.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-abfab700aa6-73f6a85-be5531bed2d9a7a3ff072281cd4c27aaaca3d752/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-abfab70-73f6a85-535a466a6dfa2a8923e03e2a32a62bf0676dc7a7/index.html

Web 🕸️
https://90709.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-05-26T18:45:44Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

MelvinBot assigned marcaaron May 14, 2026

MelvinBot requested a review from a team May 14, 2026 18:22

MelvinBot mentioned this pull request May 14, 2026

[$250] Bulk expense edit hangs and crashes on large reports #90707

Open

Remove unused RORY_EMAIL variable in BulkEditTest

cc7f319

Co-authored-by: Marc Glasser <marcaaron@users.noreply.github.com>

marcaaron requested review from marcaaron and rojiphil May 14, 2026 18:42

MelvinBot marked this pull request as ready for review May 15, 2026 12:19

MelvinBot requested a review from a team as a code owner May 15, 2026 12:19

melvin-bot Bot requested review from Beamanator and trjExpensify and removed request for a team May 15, 2026 12:19

chatgpt-codex-connector Bot reviewed May 15, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into claude-fixBulkEditPer…

613f9f5

…formanceHang Co-authored-by: Marc Glasser <marcaaron@users.noreply.github.com>

MelvinBot requested a review from a team as a code owner May 15, 2026 12:34

melvin-bot Bot removed the request for review from a team May 15, 2026 12:34

Merge main into claude-fixBulkEditPerformanceHang to resolve conflicts

8078237

Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>

Remove unused betas, introSelected, currentUserLogin from updateMulti…

1e161d6

…pleMoneyRequests call Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>

Beamanator previously approved these changes May 26, 2026

View reviewed changes

MelvinBot dismissed Beamanator’s stale review via 03db3fc May 26, 2026 17:14

Beamanator previously approved these changes May 26, 2026

View reviewed changes

MelvinBot dismissed Beamanator’s stale review via 01c5981 May 26, 2026 17:50

Beamanator approved these changes May 26, 2026

View reviewed changes

Beamanator merged commit abfab70 into main May 26, 2026
34 checks passed

Beamanator deleted the claude-fixBulkEditPerformanceHang branch May 26, 2026 18:36

OSBotify mentioned this pull request May 26, 2026

Deploy Checklist: New Expensify 2026-05-26 #91756

Open

31 tasks


	...(didCreateThreadInThisIteration ? {transactionThreadReportID: null} : {})
	},

Conversation

MelvinBot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

AI Tests

Uh oh!

codecov Bot commented May 14, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

MelvinBot commented May 14, 2026

Uh oh!

MelvinBot commented May 14, 2026

Uh oh!

marcaaron commented May 14, 2026

Uh oh!

MelvinBot commented May 14, 2026

Uh oh!

rojiphil commented May 15, 2026

Uh oh!

MelvinBot commented May 15, 2026

Uh oh!

MelvinBot commented May 15, 2026

Uh oh!

rojiphil commented May 15, 2026

Uh oh!

MelvinBot commented May 15, 2026

Uh oh!

melvin-bot Bot commented May 15, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 15, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented May 15, 2026

Uh oh!

melvin-bot Bot commented May 15, 2026

Uh oh!

MelvinBot commented May 20, 2026

Uh oh!

rojiphil commented May 25, 2026

Uh oh!

MelvinBot commented May 25, 2026

Uh oh!

Beamanator commented May 25, 2026

Uh oh!

rojiphil commented May 26, 2026

Uh oh!

MelvinBot commented May 26, 2026

Uh oh!

rojiphil commented May 26, 2026

Uh oh!

MelvinBot commented May 26, 2026

Uh oh!

rojiphil commented May 26, 2026

Uh oh!

Beamanator left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MelvinBot commented May 14, 2026 •

edited

Loading

codecov Bot commented May 14, 2026 •

edited by github-actions Bot

Loading