Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/components/Search/SearchBulkActionsButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ function SearchBulkActionsButton({queryJSON}: SearchBulkActionsButtonProps) {
pendingPaymentAdditionalDataRef.current = data;
},
currentUserAccountID: currentUserPersonalDetails.accountID,
isOffline,
})
}
variant={CONST.BUTTON_VARIANT.SUCCESS}
Expand Down Expand Up @@ -255,6 +256,7 @@ function SearchBulkActionsButton({queryJSON}: SearchBulkActionsButtonProps) {
pendingPaymentAdditionalDataRef.current = data;
},
currentUserAccountID: currentUserPersonalDetails.accountID,
isOffline,
})
}
isSplitButton={false}
Expand Down
9 changes: 7 additions & 2 deletions src/hooks/useSearchBulkActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
} from '@libs/actions/Search';
import initSplitExpense from '@libs/actions/SplitExpenses';
import {setNameValuePair} from '@libs/actions/User';
import deferModalPresentationAfterPopoverDismiss from '@libs/deferModalPresentationAfterPopoverDismiss';
import {getExpensifyCardStatementParamsFromFeed, getExpensifyCardStatementSelection} from '@libs/ExpensifyCardStatementUtils';
import type {ExpensifyCardStatementParams} from '@libs/ExpensifyCardStatementUtils';
import Log from '@libs/Log';
Expand Down Expand Up @@ -1165,7 +1166,9 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) {
return;
}
if (isOffline) {
setIsOfflineModalVisible(true);
// On iOS, presenting the offline modal while the payment popover is still dismissing freezes the app,
// so defer it until the popover transition completes (matching the delegate/locked-account guards).
deferModalPresentationAfterPopoverDismiss(() => setIsOfflineModalVisible(true));
return;
}

Expand Down Expand Up @@ -2108,7 +2111,9 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) {
}
const {shouldEnableBulkPayOption} = getPayOption(selectedReports, selectedTransactions, lastPaymentMethods, selectedReportIDs, personalPolicyID);

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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard offline Pay submenu selections before KYC

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 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MelvinBot I agree with this comment. Could you guard all payment methods behind an offline guard in advance?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MelvinBot I agree with this comment. Could you guard all payment methods behind an offline guard in advance?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, confirmPaymentonBulkPaySelected 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 testhandleActionButtonPressTest.ts (13 passed) and useSearchBulkActionsPayTest.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.


if (shouldShowPayOption) {
const shouldShowPaySubmenu = !!bulkPayButtonOptions?.length;
Expand Down
10 changes: 10 additions & 0 deletions src/libs/actions/Search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1877,6 +1877,7 @@ function handleBulkPayItemSelected(params: {
confirmPayment?: (paymentType: PaymentMethodType | undefined, additionalData?: BulkPaySelectionData) => void;
setPendingPaymentAdditionalData?: (data: BulkPaySelectionData | undefined) => void;
currentUserAccountID: number;
isOffline: boolean;
}) {
const {
item,
Expand All @@ -1896,13 +1897,22 @@ function handleBulkPayItemSelected(params: {
ownerBillingGracePeriodEnd,
setPendingPaymentAdditionalData,
currentUserAccountID,
isOffline,
} = params;
const {paymentType, policyFromPaymentMethod, policyFromContext, shouldSelectPaymentMethod} = getActivePaymentType(item.key, activeAdminPolicies, businessBankAccountOptions, policy?.id);
// Early return if item is not a valid payment method and not a policy-based payment option
if (!isValidBulkPayOption(item) && !policyFromPaymentMethod) {
return;
}

// While offline, route every payment method straight to the offline modal via confirmPayment (onBulkPaySelected),
// before any branch below can navigate the user into a KYC / account-verification / add-bank-account / restricted-action flow.
if (isOffline) {
Log.info('[BulkPay] Blocking bulk pay: offline, deferring to the offline modal');
confirmPayment?.(paymentType as PaymentMethodType, item?.additionalData as BulkPaySelectionData | undefined);
return;
}

if (isDelegateAccessRestricted) {
Log.info('[BulkPay] Blocking bulk pay: delegate access is restricted');
deferModalPresentationAfterPopoverDismiss(showDelegateNoAccessModal);
Expand Down
26 changes: 26 additions & 0 deletions tests/unit/Search/handleActionButtonPressTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ describe('handleBulkPayItemSelected', () => {
bankAccountList: undefined,
ownerBillingGracePeriodEnd: undefined,
currentUserAccountID: ownerAccountID,
isOffline: false,
};

beforeEach(async () => {
Expand Down Expand Up @@ -647,4 +648,29 @@ describe('handleBulkPayItemSelected', () => {
expect(baseParams.triggerKYCFlow).toHaveBeenCalled();
expect(baseParams.confirmPayment).not.toHaveBeenCalled();
});

it('should defer to confirmPayment (offline modal) and never navigate to KYC/verify-account when offline, even for a bank-funded payment type', async () => {
const policy = {
...createRandomPolicy(Number(policyID)),
id: policyID,
ownerAccountID,
} as Policy;

await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, policy);

handleBulkPayItemSelected({
...baseParams,
policy,
amountOwed: 0,
// VBBA + unvalidated user would normally route to account verification / KYC; offline must short-circuit that.
isUserValidated: false,
isOffline: true,
item: {key: CONST.IOU.PAYMENT_TYPE.VBBA, text: 'Pay with bank account', icon: () => null},
});

expect(baseParams.triggerKYCFlow).not.toHaveBeenCalled();
expect(Navigation.navigate).not.toHaveBeenCalledWith(createDynamicRoute(DYNAMIC_ROUTES.VERIFY_ACCOUNT.path));
// confirmPayment (onBulkPaySelected) is what surfaces the offline modal; the exact paymentType is not important here.
expect(baseParams.confirmPayment).toHaveBeenCalled();
});
});
Loading
Loading