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
4 changes: 3 additions & 1 deletion src/components/SettlementButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,9 @@ function SettlementButton({
return chatReport?.invoiceReceiver?.policyID;
}

if (canUseActivePolicy) {
// Currency only decides whether direct reimbursement is offered, so it must not gate reuse here:
// the setup flow we navigate to next is what brings the workspace to a supported currency.
if (hasActivePolicyAsAdmin) {
return activePolicy.id;
Comment on lines +423 to 424

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep bank setup currency aligned with the invoice

When the active admin workspace is EUR but the invoice and payer currency are USD—the exact scenario added to the regression test—this branch now opens bank setup for the EUR workspace. The non-USD setup fixes the account currency to policy.outputCurrency (NonUSDVerifiedBankAccountFlowPage.tsx lines 74–76), while SettlementButton only offers accounts whose bankCurrency matches the invoice currency via matchesCurrency() (PaymentUtils.ts lines 165–166). The newly created EUR account therefore never appears as a payment method for the USD invoice, leaving the payer unable to complete payment; reuse needs to route the workspace through a currency change first or otherwise preserve creation of a currency-compatible workspace.

Useful? React with 👍 / 👎.

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.

Verified this in the app — the mechanism is real. With a EUR active workspace the reuse branch does enter the non-USD flow, so the new account's currency follows the workspace rather than the invoice.

It isn't a regression though. In that same state (payer local currency IDR, USD invoice, EUR active workspace), main doesn't reuse, so it creates a workspace in the payer's local currency — IDR — which isn't globally reimbursable and dead-ends on the "set your workspace currency to USD" screen. And it does that on every press, which is the duplicate-workspace bug being fixed here. Neither path produces a payable account in that scenario; main just also litters workspaces.

main only lands on a compatible currency when the payer's local currency happens to match the invoice, since it comes from localCurrencyCode and is independent of the invoice currency. So bank-account currency vs invoice currency is a pre-existing gap rather than something this change introduces, and it's worth tracking separately — making reuse currency-aware here would mean reinstating the exact gate that causes the duplicates.

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.

Thanks for checking this @wildan-m. What do you think about the specific case where the invoice and the payer’s payment currency are USD, while the active workspace is EUR?
I believe this may differ from the IDR scenario mentioned in your response. Could the resulting EUR bank account be filtered out when returning to the USD invoice? I’m not completely certain, so perhaps it would be worth verifying this exact scenario. What do you think?

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.

@brunovjk Yes, it'd be filtered — matchesCurrency compares bankCurrency against the invoice currency. Whether that's a regression turns on the payer's localCurrencyCode rather than their payment currency: if it's non-USD (what I tested) main also ends up unpayable and spawns a workspace per press, so only a USD localCurrencyCode favours main — and that one I can't reproduce, since it's set at signup with no setting to change it.

}

Expand Down
155 changes: 154 additions & 1 deletion tests/ui/components/SettlementButtonTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import SettlementButton from '@components/SettlementButton';
import type SettlementButtonProps from '@components/SettlementButton/types';

import {createWorkspace} from '@libs/actions/Policy/Policy';
import {navigateToBankAccountRoute} from '@libs/actions/ReimbursementAccount';

import CONST from '@src/CONST';
import IntlStore from '@src/languages/IntlStore';
Expand Down Expand Up @@ -82,6 +83,34 @@ jest.mock('@libs/actions/Policy/Policy', () => ({
createWorkspace: jest.fn(() => ({policyID: 'mock-created-policy-id'})),
}));

jest.mock('@libs/actions/ReimbursementAccount', () => ({
...jest.requireActual<Record<string, unknown>>('@libs/actions/ReimbursementAccount'),
navigateToBankAccountRoute: jest.fn(),
}));

// Dropdown items defer onSelected until the popover finishes hiding, so the modal has to report that.
jest.mock('@components/Modal/ReanimatedModal', () => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const {useEffect, useRef}: {useEffect: typeof React.useEffect; useRef: typeof React.useRef} = require('react');

return function MockReanimatedModal({isVisible, onModalHide, children}: {isVisible: boolean; onModalHide?: () => void; children: React.ReactElement}) {
const wasVisible = useRef<boolean>(isVisible);

useEffect(() => {
if (wasVisible.current && !isVisible) {
onModalHide?.();
}
wasVisible.current = isVisible;
}, [isVisible, onModalHide]);

if (!isVisible) {
return null;
}

return children;
};
});

const mockVerifyAccountAndResume = jest.fn();

jest.mock('@hooks/useVerifyAccountAndResume', () => ({
Expand Down Expand Up @@ -201,13 +230,16 @@ type OnyxSetupParams = {
report?: Report;
chatReport?: Report;
policy?: Policy;
extraPolicies?: Policy[];
activePolicyID?: string;
personalPolicyID?: string;
bankAccountList?: BankAccountList;
lastPaymentMethod?: LastPaymentMethod;
userWallet?: {tierName?: ValueOf<typeof CONST.WALLET.TIER_NAME>};
betas?: Beta[];
};

async function setupOnyxState({report, chatReport, policy, bankAccountList, lastPaymentMethod, userWallet, betas}: OnyxSetupParams) {
async function setupOnyxState({report, chatReport, policy, extraPolicies, activePolicyID, personalPolicyID, bankAccountList, lastPaymentMethod, userWallet, betas}: OnyxSetupParams) {
await act(async () => {
await Onyx.merge(ONYXKEYS.SESSION, {accountID: ACCOUNT_ID, email: ACCOUNT_LOGIN});
await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {
Expand All @@ -223,6 +255,15 @@ async function setupOnyxState({report, chatReport, policy, bankAccountList, last
if (policy) {
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${policy.id}`, policy);
}
for (const extraPolicy of extraPolicies ?? []) {
await Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY}${extraPolicy.id}`, extraPolicy);
}
if (activePolicyID) {
await Onyx.merge(ONYXKEYS.NVP_ACTIVE_POLICY_ID, activePolicyID);
}
if (personalPolicyID) {
await Onyx.merge(ONYXKEYS.PERSONAL_POLICY_ID, personalPolicyID);
}
if (bankAccountList) {
await Onyx.merge(ONYXKEYS.BANK_ACCOUNT_LIST, bankAccountList);
}
Expand Down Expand Up @@ -1108,4 +1149,116 @@ describe('SettlementButton', () => {
expect(createWorkspaceMock).not.toHaveBeenCalled();
});
});

describe('invoice business add bank account workspace reuse', () => {
const ACTIVE_POLICY_ID = 'active-admin-policy-id';
const PERSONAL_POLICY_ID = 'personal-policy-id';

function pressPopoverMenuItem(text: string) {
const menuItem = screen.getByTestId(`PopoverMenuItem-${text}`);

// MenuItem only forwards the press when it receives an event, so a GestureResponderEvent-shaped one is required.
fireEvent.press(menuItem, {nativeEvent: {}, type: 'press', target: menuItem, currentTarget: menuItem});
}

function createPersonalPolicy(): Policy {
return createTestPolicy({
id: PERSONAL_POLICY_ID,
name: 'Personal Policy',
type: CONST.POLICY.TYPE.PERSONAL,
outputCurrency: CONST.CURRENCY.USD,
});
}

async function payAsBusinessAddBankAccount(params: Omit<OnyxSetupParams, 'report' | 'betas'>) {
const invoiceReport = createInvoiceReport();

await setupOnyxState({
report: invoiceReport,
betas: [CONST.BETAS.PAY_INVOICE_VIA_EXPENSIFY],
...params,
});

render(
<SettlementButtonWrapper>
<SettlementButton
{...defaultProps}
iouReport={invoiceReport}
/>
</SettlementButtonWrapper>,
);

await waitForBatchedUpdatesWithAct();

fireEvent.press(screen.getByText(translateLocal('iou.settlePayment', '$100.00')));
await waitForBatchedUpdatesWithAct();

pressPopoverMenuItem(translateLocal('bankAccount.addBankAccount'));
await waitForBatchedUpdatesWithAct();
}

const individualReceiverChat = () =>
createChatReport({
invoiceReceiver: {
type: CONST.REPORT.INVOICE_RECEIVER_TYPE.INDIVIDUAL,
accountID: ACCOUNT_ID,
},
});

it('should reuse the active admin workspace when its currency is not supported for direct reimbursement', async () => {
await payAsBusinessAddBankAccount({
chatReport: individualReceiverChat(),
policy: createTestPolicy({id: ACTIVE_POLICY_ID, outputCurrency: CONST.CURRENCY.EUR}),
extraPolicies: [createPersonalPolicy()],
activePolicyID: ACTIVE_POLICY_ID,
personalPolicyID: PERSONAL_POLICY_ID,
});

expect(createWorkspace).not.toHaveBeenCalled();
expect(navigateToBankAccountRoute).toHaveBeenCalledWith({policyID: ACTIVE_POLICY_ID});
});

it('should reuse the active admin workspace when its currency is supported for direct reimbursement', async () => {
await payAsBusinessAddBankAccount({
chatReport: individualReceiverChat(),
policy: createTestPolicy({id: ACTIVE_POLICY_ID, outputCurrency: CONST.CURRENCY.USD}),
extraPolicies: [createPersonalPolicy()],
activePolicyID: ACTIVE_POLICY_ID,
personalPolicyID: PERSONAL_POLICY_ID,
});

expect(createWorkspace).not.toHaveBeenCalled();
expect(navigateToBankAccountRoute).toHaveBeenCalledWith({policyID: ACTIVE_POLICY_ID});
});

it('should create a workspace when the active policy is not one the user administers', async () => {
await payAsBusinessAddBankAccount({
chatReport: individualReceiverChat(),
policy: createTestPolicy({id: ACTIVE_POLICY_ID, outputCurrency: CONST.CURRENCY.EUR, role: CONST.POLICY.ROLE.USER}),
extraPolicies: [createPersonalPolicy()],
activePolicyID: ACTIVE_POLICY_ID,
personalPolicyID: PERSONAL_POLICY_ID,
});

expect(createWorkspace).toHaveBeenCalled();
expect(navigateToBankAccountRoute).toHaveBeenCalledWith({policyID: 'mock-created-policy-id'});
});

it('should use the receiver workspace for a business invoice receiver', async () => {
await payAsBusinessAddBankAccount({
chatReport: createChatReport({
invoiceReceiver: {
type: CONST.REPORT.INVOICE_RECEIVER_TYPE.BUSINESS,
policyID: POLICY_ID,
},
}),
policy: createTestPolicy(),
extraPolicies: [createTestPolicy({id: ACTIVE_POLICY_ID, outputCurrency: CONST.CURRENCY.EUR})],
activePolicyID: ACTIVE_POLICY_ID,
});

expect(createWorkspace).not.toHaveBeenCalled();
expect(navigateToBankAccountRoute).toHaveBeenCalledWith({policyID: POLICY_ID});
});
});
});
Loading