Show Pay option in bulk actions dropdown while offline - #97942
Conversation
Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
I looked at the failing checks — there's nothing to fix by pushing code. The only two failures are human-gated checks, not code failures:
Every actual code check is green: ESLint, Oxfmt, typecheck, Bun tests, all 8 So no code fix is needed. To get this mergeable: complete the author checklist and get a reviewer approval. |
|
All checks have now finished. The only two failures are human-gated — there's no code fix to push:
Everything else is green (35 passing): ESLint, Oxfmt, typecheck, Bun tests, all 8 To make it mergeable: complete the author checklist and get a reviewer approval. |
|
@MelvinBot Mark the PR ready |
|
@DylanDylann 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] |
|
✅ Marked this PR as Ready for review. All meaningful CI checks are green — including the new unit test Note: the human co-author still needs to fill in the |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-08-10.at.15.40.51.movAndroid: mWeb ChromeScreen.Recording.2026-08-10.at.15.22.33.moviOS: HybridAppScreen.Recording.2026-08-10.at.16.11.52.moviOS: mWeb SafariScreen.Recording.2026-08-10.at.15.41.25.movMacOS: Chrome / SafariScreen.Recording.2026-08-10.at.15.15.11.mov |
![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: 02e71a0239
ℹ️ 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".
| const shouldShowPayOption = !isOffline && !isAnyTransactionOnHold && shouldEnableBulkPayOption && !!bulkPayButtonOptions?.length; | ||
| // Keep Pay visible while offline: selecting it is handled by onBulkPaySelected, which shows the offline modal | ||
| // rather than attempting a payment. Gating on !isOffline here would hide Pay entirely offline, which is wrong. | ||
| const shouldShowPayOption = !isAnyTransactionOnHold && shouldEnableBulkPayOption && !!bulkPayButtonOptions?.length; |
There was a problem hiding this comment.
When the user is offline, exposing Pay here still renders its payment-method submenu, but actual submenu clicks in SearchBulkActionsButton are routed through handleBulkPayItemSelected before confirmPayment is called. For non-Elsewhere items that require validation/KYC, add-bank-account invoice items, or locked/restricted-account cases, that helper navigates or opens another flow before the onBulkPaySelected offline guard can run, so selecting Pay offline does not consistently show the offline modal and can send users into verification/KYC/add-bank-account flows instead. Please add an offline guard to the submenu path or only expose offline Pay items that route directly to the offline modal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@MelvinBot I agree with this comment. Could you guard all payment methods behind an offline guard in advance?
There was a problem hiding this comment.
@MelvinBot I agree with this comment. Could you guard all payment methods behind an offline guard in advance?
There was a problem hiding this comment.
Done — pushed fb151a6. I moved the offline guard up so every payment method is short-circuited before any navigation can happen.
The gap was that submenu clicks route through handleBulkPayItemSelected in Search.ts, whose confirmPayment (the onBulkPaySelected offline guard) only runs at the very end — after the delegate/locked/restricted/KYC/verify-account branches could already navigate the user away. So I added an early offline check right after the item-validity check, before any of those branches:
if (isOffline) {
Log.info('[BulkPay] Blocking bulk pay: offline, deferring to the offline modal');
confirmPayment?.(paymentType as PaymentMethodType, item?.additionalData as BulkPaySelectionData | undefined);
return;
}isOffline is a new required param passed from both call sites in SearchBulkActionsButton.tsx (it was already available there via useNetwork()). When offline, confirmPayment → onBulkPaySelected just opens the offline modal, so no method can send the user into KYC / account-verification / add-bank-account / restricted-action flows anymore.
I also added a unit test in handleActionButtonPressTest.ts asserting that offline + an unvalidated user + a bank-funded (VBBA) item calls confirmPayment and never triggers triggerKYCFlow or the verify-account navigation.
Checks run locally
npm test—handleActionButtonPressTest.ts(13 passed) anduseSearchBulkActionsPayTest.ts(16 passed) ✅npm run typecheck(tsc) ✅- ESLint on changed files — 0 errors (only pre-existing grandfathered warnings) ✅
npm run fmt✅
The React Compiler compliance check couldn't run in this environment, but the only .tsx change adds an already-in-scope variable into an existing object literal inside pre-existing callbacks, so it introduces no new memoization boundary — CI runs the full check regardless.
|
There's nothing here I can fix by pushing code — every code check passes. The only two failing checks are process/human checks, not test or lint failures:
All actual CI is green: Bun tests, the So no commit is needed — just complete the author checklist/Tests/QA sections and get the peer approval, and both checks will go green. |
|
Still nothing to push a fix for — the state is unchanged from my previous comment. The only two failing checks are process/human checks, not code failures:
Every code check is green (Bun tests, |
|
@MelvinBot I agree with this comment. Could you guard all payment methods behind an offline guard in advance? |
|
Same as before — nothing here can be fixed by pushing code. All code checks (Bun tests, the
No commit is needed. Complete the author checklist / Tests / QA sections and get the peer approval, and both checks will go green. |
Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
@puneetlath 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] |
|
The offline-guard change I pushed earlier ( The only two failing checks are the same human-gated ones — not code failures, nothing I can push a fix for:
So the code is in good shape and nothing needs to be pushed — just complete the author checklist/Tests/QA and get a review approval, and both checks go green. |
|
Good news — this is already handled in the current PR head ( The guard is the first branch in
That directly addresses Coverage:
All test, typecheck, ESLint, Oxfmt, and React Compiler checks are green on |
|
Nothing has changed since my previous comment, and there's still nothing I can fix by pushing code. The only two failing checks remain process/human checks:
|
|
@MelvinBot BUG: On the iOS app, the app freezes when clicking 'Bulk Pay' while offline. Screen.Recording.2026-08-10.at.15.39.12.mov |
Presenting the offline DecisionModal synchronously while the payment popover is still dismissing freezes the app on iOS. Route the offline guard in onBulkPaySelected through deferModalPresentationAfterPopoverDismiss, matching the existing delegate/locked-account guards, so the modal presents only after the popover transition completes. Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
🤖 Fixed the iOS freeze on bulk Pay while offline and pushed to this branch ( Root cause: Selecting Pay offline calls Fix: Route the offline guard in Verification
|
|
@puneetlath Could you complete the author checklist? I can't edit the PR description created by Melvin. The test steps are already correct |
|
All code checks are green on the latest commit ( The only two failing checks are process/human checks, not code failures:
Both go green once the checklist/Tests/QA sections are completed and a reviewer approves — no commit needed. |
|
No code fix to push — all code checks are green on the latest commit (
Both clear once a human completes the checklist/Tests/QA and a reviewer approves — no push required. Re-running me won't change this since there's nothing in the code to fix. |
JmillsExpensify
left a comment
JmillsExpensify
left a comment
There was a problem hiding this comment.
Didn't test an ahoc, though the testing steps align with product spec
|
@DylanDylann the PR author checklist check is failing. |
|
|
The only failing check is PR Author Checklist (run) — and it's a stale result, not a code problem, so there's nothing to push. What happened: the checklist action evaluates the PR description body, not the code. It ran while the body still had the checklist empty ( Every other check (tests, typecheck, ESLint, oxfmt, build, perf, etc.) is passing. To clear it: re-run the failed PR Author Checklist job (Checks tab → PR Author Checklist → Re-run failed jobs). I attempted the re-run from here but don't have permission to trigger it in this environment. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 puneetlath 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/puneetlath in version: 9.4.53-0 🚀
|
Help site review: no changes requiredI reviewed this PR against the help articles in Why: This is an internal offline-behavior correctness fix. It makes the Pay option appear in the bulk-actions ("N selected") dropdown while offline and routes a click to the standard offline modal instead of hiding the option ( The relevant articles (Pay-Expenses.md, Understanding-Report-Statuses-and-Actions.md) describe how to pay reports; they do not document the granular offline visibility of the bulk-selection dropdown, and the help site doesn't document per-feature offline-modal behavior (it's standard app-wide UX). Nothing customer-facing in the docs is affected. @DylanDylann if you feel any customer-facing behavior here should be documented, let me know and I'll draft the help site PR. |
5 participants
Explanation of Change
On Search → Reports, when a user was offline and selected 2+ approved ("Ready to pay") reports, the Pay option was missing from the "N selected" bulk-actions dropdown. It rendered normally when online.
The bulk-action options array gated the Pay option behind
!isOfflinewhen it was built, so the item was never pushed into the dropdown while offline — even though the Pay click handler (onBulkPaySelected) already handles the offline case by showing the offline modal and attempting no payment. That offline guard was therefore dead code, because the option was removed before a user could ever click it.This change removes the
!isOfflineconjunct fromshouldShowPayOptioninsrc/hooks/useSearchBulkActions.tsso Pay renders while offline. Selecting it (directly, or via a submenu item that routes throughconfirmPayment) hits the existing offline guard, which opens the offline modal instead of attempting a payment — matching the expected result. The change is scoped to Pay only; the Approve and Hold/Unhold options intentionally keep their!isOfflinegate since they have no offline-modal handler.A unit test (
tests/unit/hooks/useSearchBulkActionsPayTest.ts) was added asserting the Pay option is present offline and that selecting it opens the offline modal rather than callingpayMoneyRequest.Fixed Issues
$ #97753
PROPOSAL: #97753 (comment)
Tests
Same as offline tests.
Offline tests
QA Steps
Same as offline tests.
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