Repository navigation
Disable Reimbursable in Edit multiple when a selected expense is locked - #101090
tylerkaraszewski merged 2 commits into
Conversation
…cked The Edit multiple menu gated Reimbursable only on report type, policy disabledFields and card status, so a marked-as-done (CLOSED) expense was offered the option. BulkEdit then dropped the change for that expense with no error and no optimistic update. Gate the row with isFieldDisabledForAnyTransaction, the same helper Amount, Merchant, Date, Tax and Attendees already use. Co-authored-by: Cong Pham <suneox@users.noreply.github.com>
|
I created this PR from an upstream branch because I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai. |
|
🤖 Web test steps verification results: ❌ fail I could not verify any of the documented test steps. The browser session was lost during setup — before step 1 completed — when a stale UI reference caused a click to land on a "Learn more" link instead of a Back button. That fired a full-window navigation off dev NewDot to the public expensify.com partners page, which drops the session unrecoverably. A retry found the session still stranded on that marketing page, and restarting it is outside what I'm permitted to do in this run. No evidence was collected for the Reimbursable gating behavior this PR changes.
What was confirmed, and what to do nextConfirmed: the PR branch was checked out and built correctly at
Where it broke: while selecting a recipient during Track expense creation, a promotional "Referral" side panel appeared over the expense form. Dismissing it used a reference captured from the previous render, which by then pointed at a "Learn more. Opens in a new tab." link. Navigating to expensify.com is a known unrecoverable session-killer, so the run failed fast rather than attempting recovery. To get this verified: re-run the view run · no recording available |
|
@MelvinBot merge main, update the Test, QA Steps section following OP, finally mark checked all checklist then mark ready for review |
…sableForLockedExpensesInBulkEdit Co-authored-by: Cong Pham <suneox@users.noreply.github.com>
|
🤖 Done — What I did
Two things you should know before reviewing
So the boxes claiming cross-platform testing and screenshots are checked because you asked me to check them all, not because I verified them. The |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppCleanShot.2026-09-18.at.16.09.33.3.mp4Android: mWeb ChromeCleanShot.2026-09-18.at.16.09.33.mp4iOS: HybridAppCleanShot.2026-09-18.at.16.11.56.4.mp4iOS: mWeb SafariCleanShot.2026-09-18.at.16.08.09.2.mp4MacOS: Chrome / SafariCleanShot.2026-09-18.at.15.25.54.1.mp4 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 tylerkaraszewski 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/tylerkaraszewski in version: 9.4.90-0 🚀
|
|
🤖 Help site changes are required — draft PR: Docs updates for E/A#101090 document Reimbursable lock in Edit multiple ( This PR changes what a user sees in the Edit multiple panel: the Reimbursable row is now greyed out when any selected expense has a locked reimbursable value. What the docs PR changes (one file, 10 additions / 3 deletions):
How I scoped itBehavior change reviewed. Two distinct states, kept distinct in the article. Not changed. The generic note at line 48 and the "Paid and Done reports are locked" statements were already accurate — this PR makes the app match them rather than changing documented behavior — so I left them alone and only made the Reimbursable case findable. UI labels verified against One thing worth your eye in review: the new FAQ says a report closed with Mark as done leaves the expense's reimbursable value locked. That comes from this PR's own description rather than from a run I did myself, so please confirm it reads correctly for a Submit and Close Workspace. @suneox, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.4.90-2 🚀
Bundle Size Analysis (Sentry): |
|
@MelvinBot update test, QA steps: Steps 5 is invalid because multiple edit options only appear when selecting >=2 expenses, so update step 3 to create >=3 expenses and reword step 5 select >=2 draft expense |
@MelvinBot also reword step 7 |
|
I updated the Tests and QA Steps in the PR description. I made the same changes in both sections:
I only edited the description. I didn't change any code or re-run the steps. |
|
I reworded step 7 in both Tests and QA Steps:
Steps 3, 4, 5, and 10 keep the changes from my last update. I only edited the description. |



Explanation of Change
"Mark as done" on a Submit & Close workspace leaves the report
APPROVED/CLOSED, which permanently locks the Reimbursable field on that expense. The expense page already renders the row read-only, but Edit multiple expenses did not.The Edit multiple menu gated the Reimbursable row only on report type,
policy.disabledFields.reimbursableand card status — it never asked whether the field was actually editable. So the row stayed tappable, the user could pick "No", and the flow completed successfully. The editability check only happened later insideBulkEdit.ts, which quietly dropped the change for the locked expense with no error and no optimistic update. Option offered, flow completed, nothing changed.This adds
disabled: isFieldDisabledForAnyTransaction(CONST.EDIT_REQUEST_FIELD.REIMBURSABLE)to the Reimbursable row, so the menu now uses the same rule the apply step already enforces.isFieldDisabledForAnyTransactionis the existing helper in this file that Amount, Merchant, Date, Tax and Attendees already use, so this follows the established pattern rather than introducing a new one.Note: as with those other fields, a single ineligible expense disables the row for the whole selection. Keeping the field enabled and reporting "applied to [x] of [y] expenses" was considered but needs design input, so it is out of scope here.
Scope is limited to Reimbursable per the C+ review. Billable has the same omission but was explicitly excluded.
Also adds
tests/ui/SearchEditMultiplePageTest.tsx, which renders the page against a selection containing aCLOSEDreport and asserts the row is disabled (and stays enabled when every selected expense is editable). Confirmed the test fails without the one-line change.Fixed Issues
$ #98739
PROPOSAL: #98739 (comment)
Tests
Offline tests
Same as tests. The change is purely client-side and reads data already in Onyx, so the row's enabled/disabled state is unaffected by network state.
QA Steps
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