[Batch 4] Add dismiss action to DecisionModalWrapper and migrate domain close-account and company cards modals by MelvinBot · Pull Request #104531 · Expensify/App

MelvinBot · 2026-10-21T06:52:20Z

Explanation of Change

Batch 4 of the move to the global useDecisionModal. It covers the three modals listed in the issue.

Step 1: dismiss result in DecisionModalWrapper

Added ModalActions.DISMISS and an opt-in shouldReportDismiss prop on DecisionModalWrapper.
With the flag on, the backdrop, Escape, and Back resolve with DISMISS. Without it, they still resolve with CLOSE, so existing showDecisionModal callers behave the same.

Domain "Close account" modals (DomainMemberDetailsPage, DomainMembersPage)

Removed the local DecisionModal and the isModalVisible / shouldForceCloseAccount state. Each page now runs one async flow:
1. Await the decision (shouldReportDismiss: true).
2. On DISMISS, do nothing.
3. Otherwise, await the confirm modal. First option means force close, and the second means safe close.
4. On cancel, call the flow again so the decision modal comes back.
This avoids the loop described in the issue, where a dismiss counted as "Safe close".

Company cards "Offline" modal (WorkspaceCompanyCardsPage)

useAssignCard now takes an onOfflineAssign callback instead of the setShouldShowOfflineModal state setter.
The page passes a callback that shows the offline alert through showDecisionModal.

Tests

New tests/unit/components/DecisionModalWrapperTest.tsx: a default dismiss resolves CLOSE, an opt-in dismiss resolves DISMISS, and the buttons still resolve CONFIRM / CLOSE.
tests/ui/DomainMemberDetailsPageTest.tsx: dismiss does nothing, safe close and force close call closeUserAccount with the right flag, and cancelling the confirm modal reopens the decision modal.
tests/ui/WorkspaceCompanyCardsPageSelectionModeTest.tsx: onOfflineAssign shows the offline alert.
tests/unit/hooks/useAssignCard.test.ts: updated to the new onOfflineAssign prop.

AI Tests

✅ npm test on the 4 changed suites (34 tests) plus ModalContextCloseByIDTest, DecisionModalTest, useExportActionsTest, and useSearchBulkActionsExportTest (74 tests)
✅ npm run typecheck
✅ npm run lint-changed (no findings)
✅ npm run fmt (oxfmt)
✅ npm run react-compiler-compliance-check check <changed src files> (no errors). check-changed and spell-changed could not run in the sandbox because git merge-base failed.
⚠️ Not run locally: the full npm run lint, the full npm test, and the Storybook smoke test. CI will run them.

Fixed Issues

$ #102978
PROPOSAL:

Tests

Prerequisites

Account A is an admin of a verified private domain that has at least 3 members you can close. Use fresh test accounts with no pending approvals or active reimbursements, so "Close account safely" doesn't skip them.
Account A is an admin of a workspace with Company cards enabled and a direct (non-commercial) card feed that has at least one unassigned card.

Domain member details: Close account

Sign in as account A and open the domain > Members.
Click a member to open the member details page.
Click "Close account".
Verify a "Close account" modal opens with "Force close account" and "Close account safely" buttons.
Press Escape. Verify the modal closes and no "Are you sure? This action is permanent." confirm modal opens.
Click "Close account" again, then click the backdrop. Verify the modal closes and no confirm modal opens.
Web only: click "Close account" again, then press the browser Back button. Verify the modal closes and no confirm modal opens. Android: use the hardware Back button instead.
Click "Close account", then click "Close account safely". Verify the "Are you sure? This action is permanent." confirm modal opens.
Click "Cancel". Verify the "Close account" decision modal opens again.
Click "Force close account". Verify the confirm modal opens.
Click "Cancel". Verify the "Close account" decision modal opens again.
Click "Close account safely", then confirm. Verify the details page closes and the member is removed from the Members list.

Domain members list: bulk Close accounts
13. On the Members page, select 2 members.
14. Open the selection dropdown and click "Close accounts".
15. Verify a "Close accounts" modal opens with "Force close accounts" and "Close accounts safely" buttons.
16. Press Escape. Verify the modal closes, no confirm modal opens, and both members stay selected.
17. Click "Close accounts" again, then click the backdrop. Verify the same result as step 16.
18. Click "Close accounts", then click "Force close accounts". Verify the confirm modal opens.
19. Click "Cancel". Verify the "Close accounts" decision modal opens again.
20. Click "Force close accounts", then confirm. Verify both members are removed from the list and the selection is cleared.

Regression: existing global decision modals
21. Open an expense report, go offline, and use the report header to download a CSV.
22. Verify the "You appear to be offline." modal opens. Press Escape or click the backdrop and verify it closes without errors.

Verify that no errors appear in the JS console

Offline tests

Company cards: assign while offline

As account A, open the workspace > Company cards and select the direct card feed.
Go offline (Troubleshoot > Force offline).
Click "Assign card" on an unassigned card.
Verify a "You appear to be offline." modal opens with the message "You can't take this action right now." and a "Got it" button.
Click "Got it". Verify the modal closes and the card stays unassigned.
Click "Assign card" again, then press Escape. Verify the modal closes.

Domain: close account while offline
7. Stay offline. Open the domain > Members and click a member.
8. Click "Close account", press Escape, and verify nothing else opens.
9. Click "Close account" > "Close account safely" > "Cancel". Verify the decision modal opens again.
10. Click "Close account safely" and confirm. Verify the member shows as pending deletion (greyed out) in the Members list.
11. Go back online. Verify the member is removed from the list.

QA Steps

Same as Tests and Offline tests.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

…ccount and company cards modals to useDecisionModal Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>

MelvinBot · 2026-10-21T06:52:35Z

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 push updates, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-migrateDecisionModalsBatch4
git checkout -b claude-migrateDecisionModalsBatch4 upstream/claude-migrateDecisionModalsBatch4
git push -u origin claude-migrateDecisionModalsBatch4

Then you can close this PR and open a new one from your fork.

codecov · 2026-10-21T06:58:22Z

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 Δ
...c/components/Modal/Global/DecisionModalWrapper.tsx	`95.83% <100.00%> (+95.83%)`	⬆️
src/components/Modal/Global/ModalContext.tsx	`95.38% <ø> (ø)`
src/hooks/useAssignCard.ts	`84.81% <100.00%> (ø)`
...rkspace/companyCards/WorkspaceCompanyCardsPage.tsx	`95.50% <100.00%> (-0.24%)`	⬇️
...c/pages/domain/Members/DomainMemberDetailsPage.tsx	`90.09% <85.24%> (+7.74%)`	⬆️
src/pages/domain/Members/DomainMembersPage.tsx	`0.00% <0.00%> (ø)`
... and 4 files with indirect coverage changes

MelvinBot · 2026-10-21T07:03:47Z

🤖 Android test steps verification results: ⚠️ partial

The PR's Tests section is still a TODO, so I tested against the acceptance criteria in the linked issue. The test account couldn't reach the domain close-account modals or the company card "Assign card" action. The app had no crashes or red-box errors in the flows I could reach.

Step	Status
Domain member details: "Close account" shows a decision modal with force close and safe close Blocked. The test account uses a public gmail.com address. Domains > New domain rejected it with "You have entered a public domain. To continue, please enter a private domain." No domain, no Domain Members page, and no member close-account modal could be reached.	⚠️
Back or backdrop dismiss on the decision modal does nothing Blocked. This step needs the modal from step 1.	⚠️
Safe close opens the confirm modal, and cancelling it brings the decision modal back Blocked. This step needs the modal from step 1.	⚠️
Force close opens the confirm modal, and cancelling it brings the decision modal back Blocked. This step needs the modal from step 1.	⚠️
Domain Members bulk "Close account" shows the same modal, and a dismiss does nothing Blocked. The Domain Members page can't be reached. The Workspace Members bulk menu has no close-account action, so it can't stand in for this step.	⚠️
Company cards: assigning a card while offline shows the "Offline" alert Blocked. Connecting a card feed needs a 6-digit code emailed to the test account, which I can't read. Without a feed, there is no "Assign card" button. With Force offline on, tapping "Add cards" showed only the offline banner. That button is not the one this PR changes, so this result doesn't confirm or rule out the fix.	⚠️
No red-box errors or crashes The app stayed in the foreground with no crash or error screens. I checked workspace creation, the new-domain form, the Members page, Troubleshoot settings, and the offline Company cards page.	✅

To test the main flows, use an account that is an admin of a verified private domain and has a connected company card feed.

view run · view recording

MelvinBot · 2026-10-21T07:13:33Z

🤖 Web test steps verification results: ⚠️ partial

The PR's Tests section is still a TODO, so I tested against the acceptance criteria in the linked issue. I couldn't reach any of the three changed modals. The test account has no domain and no company card feed, and creating either one requires an emailed "Validate your account" code that I can't read.

Step	Status
Domain member details: "Close account" shows a decision modal with force close and safe close Blocked. The account has no domain. Creating one (melvintestdomain.com) opened a "Validate your account" screen that needs a 6-digit code sent by email.	⚠️
Escape, backdrop click, and browser Back dismiss the decision modal and do nothing else Blocked. This step needs the modal from step 1.	⚠️
Safe close opens the confirm modal, and cancelling it brings the decision modal back Blocked. This step needs the modal from step 1.	⚠️
Force close opens the confirm modal, and cancelling it brings the decision modal back Blocked. This step needs the modal from step 1.	⚠️
Domain Members bulk "Close account" shows the same modal, and a dismiss does nothing Blocked. There is no domain member list to select from.	⚠️
Company cards: assigning a card while offline shows the "Offline" alert Blocked. I turned on Force offline and opened Company cards. With no card feed, the only action is "Add cards", not "Assign card". "Add cards" opened the same "Validate your account" screen. Going back showed an "Add payment card" billing dialog. This result doesn't confirm or rule out the fix.	⚠️
No new JS console errors No error banners, crash screens, or broken UI appeared on any screen I reached. The test tool can't read the browser console on web, so this check covers visible errors only.	✅

To test the main flows, use an account that is an admin of a verified private domain and has a connected company card feed.

view run · view recording

ZhenjaHorbach · 2026-10-28T09:55:19Z

@MelvinBot
Complete the author checklist and add test cases

MelvinBot · 2026-10-28T09:59:15Z

I updated the PR description. It now has Tests, Offline tests, and QA steps, and I checked the author checklist items I can vouch for.

Test cases added

Domain member details: "Close account" dismiss (Escape, backdrop, Back), safe or force close, cancel returns to the decision modal, and confirm closes the account.
Domain members list: the same flow for bulk "Close accounts", including that the selection stays after a dismiss.
Regression: an existing global decision modal ("You appear to be offline." on CSV download) still dismisses normally.
Offline: "Assign card" on a direct feed shows the offline alert, and an offline close account shows as pending.

Still unchecked: you need to do these manually

Test offline and with a High Traffic account.
Run the tests on all 5 platforms and add screenshots or videos.
Check for console errors.
Test other components that use DecisionModalWrapper, and the generic-component item.
Test the domain member page through a deeplink.

The two earlier Melvin test runs couldn't reach these modals. The test account has no verified domain and no direct card feed. You'll need an account with both to run the steps.

Items checked as not applicable

Copy: no new text. The PR reuses existing strings.
New code pattern: shouldReportDismiss / ModalActions.DISMISS is the approach the linked issue asked for.
Assets, markdown, Storybook: not touched.
UI and accessibility: the modals look the same. Only their wiring changed.
main merged after review: no review yet.

view run

ZhenjaHorbach · 2026-10-28T10:25:14Z

@MelvinBot
Fix conflicts

…nModalsBatch4 Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com> # Conflicts: # src/pages/domain/Members/DomainMemberDetailsPage.tsx # src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx

MelvinBot · 2026-10-28T10:45:27Z

I fixed the merge conflicts. I merged main into the branch and pushed the merge commit (19769a5). The PR now shows as mergeable.

Both conflicts were in React imports:

DomainMemberDetailsPage.tsx: main removed useCallback, and this PR removed useState. The import is now import React from 'react';.
WorkspaceCompanyCardsPage.tsx: the same overlap. The import is now {useEffect, useEffectEvent, useRef}.

Checks after the merge

✅ The PR's 4 test suites pass (34 tests).
✅ Typecheck shows no errors in the PR's files. Errors in other files come from outdated local dependencies after the merge.
⚠️ ESLint flags some hook and selector rules in these two files, but main has the same errors. The merge adds no new ones. npm run lint-changed couldn't run locally, so CI will confirm.

view run

ZhenjaHorbach · 2026-10-28T10:50:54Z

@MelvinBot
Open the PR

MelvinBot · 2026-10-28T10:52:48Z

I marked this PR as ready for review.

The author checklist still has unchecked items that need manual testing, such as the all-platform runs, screenshots, and console checks. Complete them before review, or the PR Author Checklist check will fail.

view run

melvin-bot · 2026-10-28T10:52:49Z

@ZhenjaHorbach 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 · 2026-10-28T10:52:54Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-28T10:56:44.214485Z	`19769a5`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

trjExpensify · 2026-10-29T23:08:34Z

PR doesn’t need product input as a refactor PR. Unassigning and unsubscribing myself.

ZhenjaHorbach · 2026-10-30T12:52:32Z

@MelvinBot
Fix conflicts

…nModalsBatch4 Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com> # Conflicts: # src/pages/domain/Members/DomainMemberDetailsPage.tsx

…nModalsBatch4 Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>

melvin-bot · 2026-10-30T13:12:22Z

@roryabraham 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-10-30T13:12:41Z

Fixed the conflicts by merging main into this branch. The PR is mergeable again.

The only conflict was in src/pages/domain/Members/DomainMemberDetailsPage.tsx. I kept this PR's useDecisionModal migration and added main's changes:

the authTokenTypeSelector import
the false, session arguments to callFunctionIfActionIsAllowed on "Report suspicious activity"

Typecheck, lint, and the PR's 4 Jest suites (34 tests) pass locally.

view run

ZhenjaHorbach · 2026-10-30T13:19:02Z

Reviewer Checklist

Screenshots/Videos

Android: HybridApp

Screen.Recording.2026-10-30.at.14.48.50.mov

Android: mWeb Chrome

Screen.Recording.2026-10-30.at.14.48.50.mov

iOS: HybridApp

Screen.Recording.2026-10-30.at.14.40.39.mov

iOS: mWeb Safari

Screen.Recording.2026-10-30.at.14.37.59.mov

MacOS: Chrome / Safari

Screen.Recording.2026-10-30.at.14.19.12.mov

ZhenjaHorbach · 2026-10-30T13:52:08Z

LGTM!

github-actions · 2026-10-30T20:04:33Z

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

OSBotify · 2026-10-30T20:07:36Z

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

github-actions · 2026-10-30T20:09:08Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #104531 with the pinned Mobile-Expensify submodule.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-0c5a23e5-1f1fe5e-4e7e20599d0ad40037dcba50cce6687121e68efc/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-0c5a23e5-1f1fe5e-a4d8c893bdac1d44752f28184a76f9935926c9b7/index.html

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

👀 View the workflow run that generated this build 👀

OSBotify · 2026-10-31T07:20:50Z

🚀 Deployed to staging by https://github.com/roryabraham in version: 9.5.27-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

MelvinBot · 2026-10-31T07:22:42Z

No help site update needed. This PR moves the domain "Close account" and company cards "Offline" modals to the shared decision modal and fixes dismissing the close-account modal so it no longer counts as a safe close, with the same labels and options, so no help article sentence becomes incorrect.

view run

jponikarchuk · 2026-11-02T15:04:46Z

Deploy Blocker #105646 was identified to be related to this PR.

OSBotify · 2026-11-02T18:22:23Z

🚀 Deployed to production by https://github.com/blimpich in version: 9.5.27-2 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

Bundle Size Analysis (Sentry):

Android
iOS

Add dismiss action to DecisionModalWrapper and migrate domain close-a…

c820d95

…ccount and company cards modals to useDecisionModal Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>

MelvinBot assigned ZhenjaHorbach Oct 21, 2026

melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Oct 21, 2026

MelvinBot requested a review from a team October 21, 2026 06:52

MelvinBot mentioned this pull request Oct 21, 2026

[Due for payment 2026-11-09] [$175] [Batch 4] Add dismiss action to DecisionModalWrapper and migrate domain close-account and company cards modals (3 modals) #102978

Open

MelvinBot marked this pull request as ready for review October 28, 2026 10:52

MelvinBot requested a review from a team as a code owner October 28, 2026 10:52

melvin-bot Bot requested a review from ZhenjaHorbach October 28, 2026 10:52

melvin-bot Bot removed the request for review from a team October 28, 2026 10:52

melvin-bot Bot requested review from trjExpensify and removed request for a team October 28, 2026 10:52

trjExpensify removed their request for review October 29, 2026 23:08

MelvinBot and others added 2 commits October 30, 2026 13:09

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

0954456

…nModalsBatch4 Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com> # Conflicts: # src/pages/domain/Members/DomainMemberDetailsPage.tsx

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

a0eea7f

…nModalsBatch4 Co-authored-by: Yauheni Horbach <ZhenjaHorbach@users.noreply.github.com>

MelvinBot requested a review from a team as a code owner October 30, 2026 13:12

melvin-bot Bot requested review from roryabraham and removed request for a team October 30, 2026 13:12

ZhenjaHorbach approved these changes Oct 30, 2026

View reviewed changes

roryabraham approved these changes Oct 30, 2026

View reviewed changes

roryabraham merged commit 0c5a23e into main Oct 30, 2026
46 of 48 checks passed

roryabraham deleted the claude-migrateDecisionModalsBatch4 branch October 30, 2026 20:02

OSBotify mentioned this pull request Oct 31, 2026

Deploy Checklist: New Expensify 2026-10-31 #105544

Closed

77 tasks done

applause-bot Bot mentioned this pull request Nov 2, 2026

Domains-Browser returns to empty tab after clicking browser back button on close account modal #105646

Open

2 of 7 tasks

Conversation

MelvinBot commented Oct 21, 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

Uh oh!

MelvinBot commented Oct 21, 2026

Uh oh!

codecov Bot commented Oct 21, 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 Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

ZhenjaHorbach commented Oct 28, 2026

Uh oh!

MelvinBot commented Oct 28, 2026

Uh oh!

ZhenjaHorbach commented Oct 28, 2026

Uh oh!

MelvinBot commented Oct 28, 2026

Uh oh!

ZhenjaHorbach commented Oct 28, 2026

Uh oh!

MelvinBot commented Oct 28, 2026

Uh oh!

melvin-bot Bot commented Oct 28, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

trjExpensify commented Oct 29, 2026

Uh oh!

ZhenjaHorbach commented Oct 30, 2026

Uh oh!

melvin-bot Bot commented Oct 30, 2026

Uh oh!

MelvinBot commented Oct 30, 2026

Uh oh!

ZhenjaHorbach commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

ZhenjaHorbach commented Oct 30, 2026

Uh oh!

Uh oh!

github-actions Bot commented Oct 30, 2026

Uh oh!

OSBotify commented Oct 30, 2026

Uh oh!

github-actions Bot commented Oct 30, 2026

Uh oh!

OSBotify commented Oct 31, 2026

Uh oh!

MelvinBot commented Oct 31, 2026

Uh oh!

jponikarchuk commented Nov 2, 2026

Uh oh!

OSBotify commented Nov 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

MelvinBot commented Oct 21, 2026 •

edited

Loading

codecov Bot commented Oct 21, 2026 •

edited by github-actions Bot

Loading

MelvinBot commented Oct 21, 2026 •

edited

Loading

MelvinBot commented Oct 21, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 28, 2026 •

edited

Loading

ZhenjaHorbach commented Oct 30, 2026 •

edited

Loading