From 9a38261c5a8357c2d676029414e56e60de9df2be Mon Sep 17 00:00:00 2001 From: Wildan Muhlis Date: Tue, 10 Nov 2026 04:42:08 +0700 Subject: [PATCH 1/3] Block offline workspace delete when Consolidated Travel Billing is enabled The offline pre-flight guard only checked the Expensify Cards blocker, so a workspace blocked by Consolidated Travel Billing was optimistically deleted offline and later rejected with the Expensify Cards wording. Treat either blocker as a known rejection while offline and surface the matching error. --- src/libs/actions/Policy/Policy.ts | 12 +++++++----- .../deleteWorkspace/DeleteWorkspaceFlow.tsx | 16 +++++++++------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/libs/actions/Policy/Policy.ts b/src/libs/actions/Policy/Policy.ts index 0443dc177917..9f889fe76c8a 100644 --- a/src/libs/actions/Policy/Policy.ts +++ b/src/libs/actions/Policy/Policy.ts @@ -127,6 +127,7 @@ import type {OnboardingCompanySize, OnboardingPurpose} from '@userActions/Welcom import CONST from '@src/CONST'; import type {OnboardingAccounting} from '@src/CONST'; +import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import type {PolicySelector} from '@src/selectors/Policy'; import type { @@ -429,7 +430,8 @@ type DeleteWorkspaceActionParams = { reimbursementAccountError: Errors | undefined; lastUsedPaymentMethods?: LastPaymentMethod; localeCompare: LocaleContextProps['localeCompare']; - hasDeleteWorkspaceExpensifyCardsError?: boolean; + /** Translation key of the blocker error to surface when the delete is already known to be rejected while offline */ + deleteWorkspaceBlockedErrorKey?: TranslationPaths; currentUserAccountID: number; accountIDToLogin: Record; }; @@ -453,7 +455,7 @@ function deleteWorkspace(params: DeleteWorkspaceActionParams) { lastUsedPaymentMethods, localeCompare, personalPolicyID, - hasDeleteWorkspaceExpensifyCardsError, + deleteWorkspaceBlockedErrorKey, currentUserAccountID, accountIDToLogin, } = params; @@ -462,10 +464,10 @@ function deleteWorkspace(params: DeleteWorkspaceActionParams) { const filteredPolicies = Object.values(policies ?? {}).filter((p): p is Policy => p?.id !== policyID); const workspaceAccountID = policy?.policyAccountID; - // Offline pre-flight guard: we already know locally the workspace has active Expensify Cards, so surface the error instead of queuing a delete that the backend will reject on reconnect. - if (hasDeleteWorkspaceExpensifyCardsError) { + // Offline pre-flight guard: we already know locally the workspace has active Expensify Cards or Consolidated Travel Billing, so surface the error instead of queuing a delete that the backend will reject on reconnect. + if (deleteWorkspaceBlockedErrorKey) { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { - errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('workspace.common.deleteOpenExpensifyCardsError'), + errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey(deleteWorkspaceBlockedErrorKey), }); return; } diff --git a/src/pages/workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx b/src/pages/workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx index 1a95804e1b22..0b8307d4a606 100644 --- a/src/pages/workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx +++ b/src/pages/workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx @@ -116,8 +116,10 @@ function DeleteWorkspaceFlow({policyID, onDismiss, onDeleteComplete}: DeleteWork const hasTravelCards = Object.values(travelCardsList ?? {}).some(isCard); const isBlockedByExpensifyCards = hasExpensifyCardsEnabledOnWorkspace && hasExpensifyCards; const isBlockedByTravelBilling = hasTravelBillingEnabledOnWorkspace && hasTravelCards; - // While offline we can't get the real rejection reason from the backend, so if we already know locally that the workspace has active Expensify Cards, block the delete up front instead of queuing one that will fail on reconnect. - const hasDeleteWorkspaceExpensifyCardsError = isBlockedByExpensifyCards && !!isOffline; + // Expensify Cards take priority when both blockers apply. + const deleteWorkspaceBlockedErrorKey = isBlockedByExpensifyCards ? 'workspace.common.deleteOpenExpensifyCardsError' : 'workspace.common.deleteTravelBillingError'; + // While offline we can't get the real rejection reason from the backend, so if we already know locally that the workspace has active Expensify Cards or Consolidated Travel Billing, block the delete up front instead of queuing one that will fail on reconnect. + const hasDeleteWorkspaceBlockedError = (isBlockedByExpensifyCards || isBlockedByTravelBilling) && !!isOffline; const policyLatestErrorMessage = getLatestErrorMessage(policy); const isPendingDelete = isPendingDeletePolicy(policy); @@ -146,8 +148,8 @@ function DeleteWorkspaceFlow({policyID, onDismiss, onDeleteComplete}: DeleteWork prompt: ( { closeModal(); dismissDeleteWorkspaceFlow(); @@ -205,7 +207,7 @@ function DeleteWorkspaceFlow({policyID, onDismiss, onDeleteComplete}: DeleteWork confirmText: translate('common.delete'), cancelText: translate('common.cancel'), buttonVariant: CONST.BUTTON_VARIANT.DANGER, - ...(hasDeleteWorkspaceExpensifyCardsError ? {} : {isConfirmLoading: isPendingDelete}), + ...(hasDeleteWorkspaceBlockedError ? {} : {isConfirmLoading: isPendingDelete}), }).then((result) => { if (!policyName || result.action !== ModalActions.CONFIRM) { onDismiss(); @@ -227,12 +229,12 @@ function DeleteWorkspaceFlow({policyID, onDismiss, onDeleteComplete}: DeleteWork lastUsedPaymentMethods: lastPaymentMethod, localeCompare, personalPolicyID, - hasDeleteWorkspaceExpensifyCardsError, + deleteWorkspaceBlockedErrorKey: hasDeleteWorkspaceBlockedError ? deleteWorkspaceBlockedErrorKey : undefined, currentUserAccountID: session?.accountID ?? CONST.DEFAULT_NUMBER_ID, accountIDToLogin: accountIDToLogin ?? {}, }); - if (hasDeleteWorkspaceExpensifyCardsError) { + if (hasDeleteWorkspaceBlockedError) { showDeleteWorkspaceErrorModal(); } else if (isOffline) { closeModal(); From 7ec8644d1d00c00a2bbe7b4fd870e82a2ee9f15c Mon Sep 17 00:00:00 2001 From: Wildan Muhlis Date: Fri, 13 Nov 2026 06:09:36 +0700 Subject: [PATCH 2/3] Check the offline delete blocker before scanning the policies The offline guard only needs the policy ID and the error key, so it now returns before the policy list is filtered. The delete flow also shares one blocked condition between the offline guard and the warning shown after an online rejection. --- src/libs/actions/Policy/Policy.ts | 8 ++++---- .../workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx | 11 +++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/Policy/Policy.ts b/src/libs/actions/Policy/Policy.ts index e47145fb86d2..cc4e6baa7bfe 100644 --- a/src/libs/actions/Policy/Policy.ts +++ b/src/libs/actions/Policy/Policy.ts @@ -462,10 +462,6 @@ function deleteWorkspace(params: DeleteWorkspaceActionParams) { accountIDToLogin, } = params; - const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`]; - const filteredPolicies = Object.values(policies ?? {}).filter((p): p is Policy => p?.id !== policyID); - const workspaceAccountID = policy?.policyAccountID; - // Offline pre-flight guard: we already know locally the workspace has active Expensify Cards or Consolidated Travel Billing, so surface the error instead of queuing a delete that the backend will reject on reconnect. if (deleteWorkspaceBlockedErrorKey) { Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, { @@ -474,6 +470,10 @@ function deleteWorkspace(params: DeleteWorkspaceActionParams) { return; } + const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`]; + const filteredPolicies = Object.values(policies ?? {}).filter((p): p is Policy => p?.id !== policyID); + const workspaceAccountID = policy?.policyAccountID; + const optimisticData: Array< OnyxUpdate< | typeof ONYXKEYS.COLLECTION.POLICY diff --git a/src/pages/workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx b/src/pages/workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx index 0b8307d4a606..b8802d74e651 100644 --- a/src/pages/workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx +++ b/src/pages/workspace/deleteWorkspace/DeleteWorkspaceFlow.tsx @@ -116,10 +116,11 @@ function DeleteWorkspaceFlow({policyID, onDismiss, onDeleteComplete}: DeleteWork const hasTravelCards = Object.values(travelCardsList ?? {}).some(isCard); const isBlockedByExpensifyCards = hasExpensifyCardsEnabledOnWorkspace && hasExpensifyCards; const isBlockedByTravelBilling = hasTravelBillingEnabledOnWorkspace && hasTravelCards; - // Expensify Cards take priority when both blockers apply. + const isDeleteBlocked = isBlockedByExpensifyCards || isBlockedByTravelBilling; + // Only used while one of the blockers applies, and Expensify Cards take priority when both do. const deleteWorkspaceBlockedErrorKey = isBlockedByExpensifyCards ? 'workspace.common.deleteOpenExpensifyCardsError' : 'workspace.common.deleteTravelBillingError'; // While offline we can't get the real rejection reason from the backend, so if we already know locally that the workspace has active Expensify Cards or Consolidated Travel Billing, block the delete up front instead of queuing one that will fail on reconnect. - const hasDeleteWorkspaceBlockedError = (isBlockedByExpensifyCards || isBlockedByTravelBilling) && !!isOffline; + const hasDeleteWorkspaceBlockedError = isDeleteBlocked && !!isOffline; const policyLatestErrorMessage = getLatestErrorMessage(policy); const isPendingDelete = isPendingDeletePolicy(policy); @@ -148,7 +149,6 @@ function DeleteWorkspaceFlow({policyID, onDismiss, onDeleteComplete}: DeleteWork prompt: ( { closeModal(); @@ -291,7 +291,7 @@ function DeleteWorkspaceFlow({policyID, onDismiss, onDeleteComplete}: DeleteWork } closeVisibleModal(() => { - if (isBlockedByExpensifyCards || isBlockedByTravelBilling) { + if (isDeleteBlocked) { showDeleteWorkspaceErrorModal(); return; } @@ -303,8 +303,7 @@ function DeleteWorkspaceFlow({policyID, onDismiss, onDeleteComplete}: DeleteWork isPendingDelete, prevIsPendingDelete, policyLatestErrorMessage, - isBlockedByExpensifyCards, - isBlockedByTravelBilling, + isDeleteBlocked, closeModal, onDeleteComplete, onDismiss, From 2a763d0daad2385f266500b80c08540f50857b86 Mon Sep 17 00:00:00 2001 From: Wildan Muhlis Date: Fri, 13 Nov 2026 06:09:36 +0700 Subject: [PATCH 3/3] Add tests for the offline workspace delete guard --- tests/actions/PolicyTest.ts | 39 ++++++ tests/ui/DeleteWorkspaceFlowTest.tsx | 194 +++++++++++++++++++++++++++ 2 files changed, 233 insertions(+) create mode 100644 tests/ui/DeleteWorkspaceFlowTest.tsx diff --git a/tests/actions/PolicyTest.ts b/tests/actions/PolicyTest.ts index 06f13e202bc0..6c28b044864a 100644 --- a/tests/actions/PolicyTest.ts +++ b/tests/actions/PolicyTest.ts @@ -5354,6 +5354,45 @@ describe('actions/Policy', () => { const message = closedAction && 'message' in closedAction && Array.isArray(closedAction.message) ? closedAction.message.at(0) : undefined; expect(message?.text).toBe(''); }); + + it.each(['workspace.common.deleteOpenExpensifyCardsError', 'workspace.common.deleteTravelBillingError'] as const)( + 'should show %s on the workspace without queuing the delete when the delete is known to be blocked', + async (blockedErrorKey) => { + // Given a workspace whose delete the backend is known to reject + const fakePolicy = createRandomPolicy(0); + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`, fakePolicy); + const apiWriteSpy = jest.spyOn(APIModule, 'write').mockImplementation(() => Promise.resolve()); + + // When the delete flow passes the blocker's error key, as it does while offline + Policy.deleteWorkspace({ + policies: {[`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`]: fakePolicy}, + policyID: fakePolicy.id, + personalPolicyID: undefined, + activePolicyID: undefined, + policyName: fakePolicy.name, + lastAccessedWorkspacePolicyID: undefined, + policyCardFeeds: undefined, + lastSelectedFeed: undefined, + lastSelectedExpensifyCardFeed: undefined, + reportsToArchive: [], + transactionViolations: undefined, + reimbursementAccountError: undefined, + lastUsedPaymentMethods: undefined, + localeCompare: TestHelper.localeCompare, + deleteWorkspaceBlockedErrorKey: blockedErrorKey, + currentUserAccountID: ESH_ACCOUNT_ID, + accountIDToLogin: {}, + }); + await waitForBatchedUpdates(); + + // Then the workspace shows that blocker's error and stays, since a queued delete would only fail on reconnect + const policy = await getOnyxValue(`${ONYXKEYS.COLLECTION.POLICY}${fakePolicy.id}`); + expect(Object.values(policy?.errors ?? {})).toEqual([TestHelper.translateLocal(blockedErrorKey)]); + expect(policy?.pendingAction).not.toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE); + expect(apiWriteSpy).not.toHaveBeenCalled(); + apiWriteSpy.mockRestore(); + }, + ); }); describe('leaveWorkspace', () => { diff --git a/tests/ui/DeleteWorkspaceFlowTest.tsx b/tests/ui/DeleteWorkspaceFlowTest.tsx new file mode 100644 index 000000000000..56f03786fba1 --- /dev/null +++ b/tests/ui/DeleteWorkspaceFlowTest.tsx @@ -0,0 +1,194 @@ +import {act, render, waitFor} from '@testing-library/react-native'; + +import {LocaleContextProvider} from '@components/LocaleContextProvider'; +import type {ModalStateChangePayload} from '@components/Modal/Global/ModalContext'; +import {ModalActions} from '@components/Modal/Global/ModalContext'; + +import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils'; +import {getTravelBillingCardSettingsKey, getTravelBillingFeedID} from '@libs/TravelBillingUtils'; + +import DeleteWorkspaceFlow from '@pages/workspace/deleteWorkspace/DeleteWorkspaceFlow'; + +import CONST from '@src/CONST'; +import IntlStore from '@src/languages/IntlStore'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import type * as ReactNavigation from '@react-navigation/native'; + +import React from 'react'; +import Onyx from 'react-native-onyx'; + +import createRandomCard from '../utils/collections/card'; +import {translateLocal} from '../utils/TestHelper'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +type ConfirmModalCall = { + prompt?: React.ReactNode; + shouldShowCancelButton?: boolean; +}; + +const mockDeleteWorkspace = jest.fn(); +const mockShowConfirmModal = jest.fn, [ConfirmModalCall]>(); +const mockUseNetwork = jest.fn<{isOffline: boolean}, []>(); + +jest.mock('@libs/actions/Policy/Policy', () => ({ + calculateBillNewDot: jest.fn(), + deleteWorkspace: (...args: unknown[]) => { + mockDeleteWorkspace(...args); + }, + dismissWorkspaceError: jest.fn(), +})); + +jest.mock('@hooks/useConfirmModal', () => + jest.fn().mockImplementation(() => ({ + showConfirmModal: mockShowConfirmModal, + closeModal: jest.fn(), + })), +); + +jest.mock('@hooks/useNetwork', () => () => mockUseNetwork()); + +jest.mock('@hooks/useScreenBoundDynamicRoute', () => () => (suffix: string) => suffix); + +jest.mock('@libs/actions/Modal', () => ({ + close: jest.fn().mockImplementation((callback?: () => void) => callback?.()), +})); + +jest.mock('@react-navigation/native', () => ({ + ...jest.requireActual('@react-navigation/native'), + useIsFocused: () => true, +})); + +const POLICY_ID = 'ABCDEF1234567890'; +const WORKSPACE_ACCOUNT_ID = 1001; +const DOMAIN_FUND_ID = 2002; +const USER_ACCOUNT_ID = 1; +const USER_EMAIL = 'admin@example.com'; + +const renderFlow = (onDeleteComplete = jest.fn()) => + render( + + + , + ); + +async function enableTravelBilling(fundID: number, linkedPolicyIDs?: string[]) { + const travelCard = createRandomCard(fundID, {bank: CONST.EXPENSIFY_CARD.BANK, state: CONST.EXPENSIFY_CARD.STATE.OPEN}); + await Onyx.set(getTravelBillingCardSettingsKey(fundID), {[CONST.TRAVEL.PROGRAM_TRAVEL_US]: {isEnabled: true, linkedPolicyIDs}}); + await Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${getTravelBillingFeedID(fundID)}`, {[travelCard.cardID]: travelCard}); +} + +async function issueExpensifyCard() { + const expensifyCard = createRandomCard(1, {bank: CONST.EXPENSIFY_CARD.BANK, state: CONST.EXPENSIFY_CARD.STATE.OPEN}); + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, {areExpensifyCardsEnabled: true}); + await Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${WORKSPACE_ACCOUNT_ID}_${CONST.EXPENSIFY_CARD.BANK}`, {[expensifyCard.cardID]: expensifyCard}); +} + +function getErrorModalHTML() { + const prompt = mockShowConfirmModal.mock.calls.at(1)?.[0].prompt; + if (!React.isValidElement<{children?: React.ReactNode}>(prompt)) { + return undefined; + } + const renderHTML = prompt.props.children; + return React.isValidElement<{html?: string}>(renderHTML) ? renderHTML.props.html : undefined; +} + +describe('DeleteWorkspaceFlow', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + return IntlStore.load(CONST.LOCALES.EN); + }); + + beforeEach(async () => { + mockDeleteWorkspace.mockReset(); + mockShowConfirmModal.mockReset(); + mockShowConfirmModal.mockResolvedValue({action: ModalActions.CONFIRM}); + mockUseNetwork.mockReturnValue({isOffline: true}); + await act(async () => { + await Onyx.clear(); + await Onyx.set(ONYXKEYS.SESSION, {email: USER_EMAIL, accountID: USER_ACCOUNT_ID}); + await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, {id: POLICY_ID, name: 'Test', role: CONST.POLICY.ROLE.ADMIN, policyAccountID: WORKSPACE_ACCOUNT_ID}); + }); + }); + + it('shows the Consolidated Travel Billing error instead of deleting the workspace while offline', async () => { + // Given an offline admin whose workspace pays for travel through its own Consolidated Travel Billing, which the backend refuses to delete + await act(async () => { + await enableTravelBilling(WORKSPACE_ACCOUNT_ID); + }); + + // When the admin confirms the delete + renderFlow(); + await waitForBatchedUpdatesWithAct(); + + // Then the delete is not queued and the travel billing warning is shown, because the real rejection reason only arrives after reconnecting + await waitFor(() => expect(mockShowConfirmModal).toHaveBeenCalledTimes(2)); + expect(mockDeleteWorkspace).toHaveBeenCalledWith(expect.objectContaining({deleteWorkspaceBlockedErrorKey: 'workspace.common.deleteTravelBillingError'})); + expect(mockShowConfirmModal).toHaveBeenLastCalledWith(expect.objectContaining({shouldShowCancelButton: false})); + expect(getErrorModalHTML()).toBe(translateLocal('workspace.common.deleteTravelBillingError')); + }); + + it('shows the Expensify Cards error while offline when the workspace has both blockers', async () => { + // Given an offline admin whose workspace has active Expensify Cards as well as Consolidated Travel Billing + await act(async () => { + await enableTravelBilling(WORKSPACE_ACCOUNT_ID); + await issueExpensifyCard(); + }); + + // When the admin confirms the delete + renderFlow(); + await waitForBatchedUpdatesWithAct(); + + // Then the Expensify Cards warning wins, matching the warning shown when the same delete is rejected online + await waitFor(() => expect(mockShowConfirmModal).toHaveBeenCalledTimes(2)); + expect(mockDeleteWorkspace).toHaveBeenCalledWith(expect.objectContaining({deleteWorkspaceBlockedErrorKey: 'workspace.common.deleteOpenExpensifyCardsError'})); + expect(getErrorModalHTML()).toBe(translateLocal('workspace.common.deleteOpenExpensifyCardsError')); + }); + + it('still deletes the workspace offline when its Consolidated Travel Billing is funded by a domain', async () => { + // Given an offline admin whose workspace is billed for travel by a domain, which the backend lets the workspace be deleted with + const onDeleteComplete = jest.fn(); + await act(async () => { + await enableTravelBilling(DOMAIN_FUND_ID, [POLICY_ID]); + }); + + // When the admin confirms the delete + renderFlow(onDeleteComplete); + await waitForBatchedUpdatesWithAct(); + + // Then the delete goes ahead optimistically without a warning + await waitFor(() => expect(onDeleteComplete).toHaveBeenCalled()); + expect(mockDeleteWorkspace).toHaveBeenCalledWith(expect.objectContaining({deleteWorkspaceBlockedErrorKey: undefined})); + expect(mockShowConfirmModal).toHaveBeenCalledTimes(1); + }); + + it('sends the delete while online and shows the Consolidated Travel Billing warning when the backend rejects it', async () => { + // Given an online admin whose workspace has Consolidated Travel Billing + mockUseNetwork.mockReturnValue({isOffline: false}); + mockDeleteWorkspace.mockImplementation(() => Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, {pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE})); + await act(async () => { + await enableTravelBilling(WORKSPACE_ACCOUNT_ID); + }); + + // When the admin confirms the delete and the backend rejects it with its Expensify Cards wording + renderFlow(); + await waitForBatchedUpdatesWithAct(); + await waitFor(() => expect(mockDeleteWorkspace).toHaveBeenCalled()); + await act(async () => { + await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${POLICY_ID}`, { + pendingAction: null, + errors: getMicroSecondOnyxErrorWithTranslationKey('workspace.common.deleteOpenExpensifyCardsError'), + }); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the delete is sent without a pre-flight warning, and the rejection shows the travel billing warning in place of the backend's wording + expect(mockDeleteWorkspace).toHaveBeenCalledWith(expect.objectContaining({deleteWorkspaceBlockedErrorKey: undefined})); + await waitFor(() => expect(mockShowConfirmModal).toHaveBeenCalledTimes(2)); + expect(getErrorModalHTML()).toBe(translateLocal('workspace.common.deleteTravelBillingError')); + }); +});