diff --git a/.env.example b/.env.example index 1718660c..0cc84545 100644 --- a/.env.example +++ b/.env.example @@ -19,6 +19,9 @@ VERCEL_URL=localhost:3000 VERCEL_ENV=development CRON_SECRET=vercel cron secret +# Comma-separated portalIds the bank deposit feature is limited to. Empty/unset = all portals. +AB_FEATURE_TESTING_PORTALS= + SENTRY_ORG= SENTRY_PROJECT= NEXT_PUBLIC_SENTRY_DSN= diff --git a/src/app/api/quickbooks/invoice/invoice.service.ts b/src/app/api/quickbooks/invoice/invoice.service.ts index db1f9fcd..aa4fa517 100644 --- a/src/app/api/quickbooks/invoice/invoice.service.ts +++ b/src/app/api/quickbooks/invoice/invoice.service.ts @@ -49,6 +49,7 @@ import { InvoiceResponseType, InvoiceVoidedResponse, } from '@/type/dto/webhook.dto' +import { isPortalInBankDepositABTest } from '@/utils/abTesting' import { bottleneck } from '@/utils/bottleneck' import { CopilotAPI } from '@/utils/copilotAPI' import IntuitAPI, { IntuitAPITokensType } from '@/utils/intuitAPI' @@ -483,6 +484,8 @@ export class InvoiceService extends BaseService { // Reads the live batched-deposit setting. Called only at the freeze point // (row creation); everything else reads the frozen row value. private async readBankDepositFeeFlag(): Promise { + // AB gate: portals outside the allowlist never freeze as batched. + if (!isPortalInBankDepositABTest(this.user.workspaceId)) return false const settingService = new SettingService(this.user) const setting = await settingService.getOneByPortalId([ 'bankDepositFeeFlag', diff --git a/src/app/api/quickbooks/payout/payout.service.ts b/src/app/api/quickbooks/payout/payout.service.ts index 9054ced2..0cbf46a2 100644 --- a/src/app/api/quickbooks/payout/payout.service.ts +++ b/src/app/api/quickbooks/payout/payout.service.ts @@ -19,6 +19,7 @@ import IntuitAPI, { IntuitAPITokensType } from '@/utils/intuitAPI' import { AccountTypeObj } from '@/constant/qbConnection' import { validateAccessToken } from '@/utils/auth' import User from '@/app/api/core/models/User.model' +import { isPortalInBankDepositABTest } from '@/utils/abTesting' export class PayoutService extends BaseService { private syncLogService: SyncLogService @@ -84,6 +85,15 @@ export class PayoutService extends BaseService { qbTokenInfo: IntuitAPITokensType, opts: { runIdempotencyCheck: boolean }, ): Promise<{ depositId: string | null }> { + // AB gate: covers both callers (payout webhook + resync cron). Only fires + // for an explicitly excluded portal (empty allowlist = all portals). Logged + // rather than silent so a rare mid-flight exclusion is visible in Sentry. + if (!isPortalInBankDepositABTest(this.user.workspaceId)) { + console.info( + `PayoutService#reconcile | AB gate off for portal ${this.user.workspaceId}; skipping deposit for payout ${row.payoutId}`, + ) + return { depositId: null } + } validateAccessToken(qbTokenInfo) const payoutId = row.payoutId diff --git a/src/app/api/quickbooks/setting/setting.controller.ts b/src/app/api/quickbooks/setting/setting.controller.ts index 5c69f940..7dabdd1c 100644 --- a/src/app/api/quickbooks/setting/setting.controller.ts +++ b/src/app/api/quickbooks/setting/setting.controller.ts @@ -1,6 +1,7 @@ import authenticate from '@/app/api/core/utils/authenticate' import { SettingService } from '@/app/api/quickbooks/setting/setting.service' import { TokenService } from '@/app/api/quickbooks/token/token.service' +import { isPortalInBankDepositABTest } from '@/utils/abTesting' import { db } from '@/db' import { QBPortalConnection } from '@/db/schema/qbPortalConnections' import { QBSetting } from '@/db/schema/qbSettings' @@ -41,7 +42,12 @@ export async function getSettings(req: NextRequest) { ? (await getPortalConnection(user.workspaceId))?.bankAccountRef || null : null - return NextResponse.json({ setting, bankAccountRef }) + const bankDepositEnabled = + parsedType.success && parsedType.data === SettingType.INVOICE + ? isPortalInBankDepositABTest(user.workspaceId) + : false + + return NextResponse.json({ setting, bankAccountRef, bankDepositEnabled }) } export async function updateSettings(req: NextRequest) { @@ -54,17 +60,24 @@ export async function updateSettings(req: NextRequest) { const parsedType = z.nativeEnum(SettingType).parse(type) const parsed = SettingRequestSchema.parse(body) - const { bankAccountRef, ...settingFields } = parsed + const { bankAccountRef, bankDepositFeeFlag, ...settingFields } = parsed + + // Bank deposit fields are only honored for invoice settings on AB-test + // portals; everyone else has the flag and bank account stripped from writes. + const isBankDepositAB = + parsedType === SettingType.INVOICE && + isPortalInBankDepositABTest(user.workspaceId) const payload = { ...settingFields, + ...(isBankDepositAB && { bankDepositFeeFlag }), ...(parsedType === SettingType.INVOICE ? { initialInvoiceSettingMap: true } : { initialProductSettingMap: true }), } const writeBankAccountRef = - parsedType === SettingType.INVOICE && typeof bankAccountRef !== 'undefined' + isBankDepositAB && typeof bankAccountRef !== 'undefined' const setting = await db.transaction(async (tx) => { settingService.setTransaction(tx) diff --git a/src/components/dashboard/settings/SettingAccordion.tsx b/src/components/dashboard/settings/SettingAccordion.tsx index 64014baf..2d05810a 100644 --- a/src/components/dashboard/settings/SettingAccordion.tsx +++ b/src/components/dashboard/settings/SettingAccordion.tsx @@ -44,6 +44,7 @@ export default function SettingAccordion({ isLoading, changeSettings, showButton: showInvoiceButton, + bankDepositEnabled, bankAccountOptions, bankAccountsError, canSave, @@ -93,6 +94,7 @@ export default function SettingAccordion({ settingState={settingState} changeSettings={changeSettings} isLoading={isLoading} + bankDepositEnabled={bankDepositEnabled} bankAccountOptions={bankAccountOptions} bankAccountsError={bankAccountsError} /> diff --git a/src/components/dashboard/settings/sections/invoice/InvoiceDetail.tsx b/src/components/dashboard/settings/sections/invoice/InvoiceDetail.tsx index 606883d7..8f705050 100644 --- a/src/components/dashboard/settings/sections/invoice/InvoiceDetail.tsx +++ b/src/components/dashboard/settings/sections/invoice/InvoiceDetail.tsx @@ -11,6 +11,7 @@ type InvoiceDetailProps = { value: InvoiceSettingType[K], ) => void isLoading: boolean + bankDepositEnabled: boolean bankAccountOptions: AccountOption[] | undefined bankAccountsError: unknown } @@ -19,6 +20,7 @@ export default function InvoiceDetail({ settingState, changeSettings, isLoading, + bankDepositEnabled, bankAccountOptions, bankAccountsError, }: InvoiceDetailProps) { @@ -41,44 +43,49 @@ export default function InvoiceDetail({ } /> -
- - changeSettings( - 'bankDepositFeeFlag', - !settingState.bankDepositFeeFlag, - ) - } - /> -
- {settingState.bankDepositFeeFlag && ( -
- {bankAccountsError ? ( -

- Could not load bank accounts. Reload to retry. -

- ) : ( - <> - changeSettings('bankAccountRef', id)} - /> - {bankAccountOptions !== undefined && - !settingState.bankAccountRef && ( -

- Select a deposit bank account to enable bank deposits. -

- )} - + {/* Bank deposit UI is gated behind the AB rollout allowlist. */} + {bankDepositEnabled && ( + <> +
+ + changeSettings( + 'bankDepositFeeFlag', + !settingState.bankDepositFeeFlag, + ) + } + /> +
+ {settingState.bankDepositFeeFlag && ( +
+ {bankAccountsError ? ( +

+ Could not load bank accounts. Reload to retry. +

+ ) : ( + <> + changeSettings('bankAccountRef', id)} + /> + {bankAccountOptions !== undefined && + !settingState.bankAccountRef && ( +

+ Select a deposit bank account to enable bank deposits. +

+ )} + + )} +
)} -
+ )}
portalId.trim()) + .filter(Boolean) + // Supabase export const supabaseProjectUrl = process.env.NEXT_PUBLIC_SUPABASE_PROJECT_URL || '' diff --git a/src/hook/useSettings.ts b/src/hook/useSettings.ts index 5c2e4e9e..ce56727d 100644 --- a/src/hook/useSettings.ts +++ b/src/hook/useSettings.ts @@ -452,10 +452,13 @@ export const useInvoiceDetailSettings = () => { isLoading, } = useSwrHelper(`/api/quickbooks/setting?type=invoice&token=${token}`) + // AB gate from the settings GET; hides the bank deposit UI when off. + const bankDepositEnabled = setting?.bankDepositEnabled ?? false + const { data: bankAccountsData, error: bankAccountsError } = useSwrHelper<{ accounts: { Id: string; Name: string }[] }>( - isDisconnected + isDisconnected || !bankDepositEnabled ? null : `/api/quickbooks/setting/bank-account?token=${token}`, { suspense: false, revalidateOnMount: true }, @@ -550,6 +553,7 @@ export const useInvoiceDetailSettings = () => { error, isLoading, showButton, + bankDepositEnabled, bankAccountOptions, bankAccountsError, canSave, diff --git a/src/utils/abTesting.ts b/src/utils/abTesting.ts new file mode 100644 index 00000000..2a64c5cb --- /dev/null +++ b/src/utils/abTesting.ts @@ -0,0 +1,11 @@ +import { abFeatureTestingPortals } from '@/config' + +/** + * Whether a portal may use the bank deposit feature during its incremental + * rollout. An empty/unset allowlist means the feature is on for all portals; + * otherwise only listed portals get it. + */ +export function isPortalInBankDepositABTest(portalId: string): boolean { + if (abFeatureTestingPortals.length === 0) return true + return abFeatureTestingPortals.includes(portalId) +} diff --git a/test/helpers/abTestGate.ts b/test/helpers/abTestGate.ts new file mode 100644 index 00000000..5749f73b --- /dev/null +++ b/test/helpers/abTestGate.ts @@ -0,0 +1,17 @@ +// Drives the bank-deposit AB gate mocked in test/integration/setup.ts. The mock +// reads its allowlist from a globalThis-pinned holder (the real allowlist is +// env-parsed at module load and can't be varied per-test). `null` = feature on +// for all portals. Always reset in afterEach so state doesn't leak across files. +const AB_GATE_GLOBAL_KEY = '__qbsync_ab_test_gate__' +type ABGate = { allowlist: string[] | null } +const ref = globalThis as unknown as Record +ref[AB_GATE_GLOBAL_KEY] ??= { allowlist: null } + +export const abTestGate = { + setAllowlist(portalIds: string[] | null) { + ref[AB_GATE_GLOBAL_KEY]!.allowlist = portalIds + }, + reset() { + ref[AB_GATE_GLOBAL_KEY]!.allowlist = null + }, +} diff --git a/test/integration/quickbooks/invoiceCreated/abTestingGate.test.ts b/test/integration/quickbooks/invoiceCreated/abTestingGate.test.ts new file mode 100644 index 00000000..c02c8453 --- /dev/null +++ b/test/integration/quickbooks/invoiceCreated/abTestingGate.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect, afterEach } from 'vitest' +import { db } from '@/db' +import { QBInvoiceSync } from '@/db/schema/qbInvoiceSync' +import invoiceCreatedPayload from '@test/fixtures/invoiceCreated.webhook' +import { + seedHealthyPortal, + seedProductSync, + TEST_PORTAL_ID, +} from '@test/helpers/seed' +import { setupInvoiceCreatedTest } from '@test/helpers/invoiceCreatedTestSetup' +import { postWebhook } from '@test/helpers/webhook' +import { abTestGate } from '@test/helpers/abTestGate' + +// The freeze gate must win over the stored flag: a portal outside the AB +// allowlist freezes non-batched even with bankDepositFeeFlag=true, so the whole +// downstream payout/deposit path never engages for it. +describe('POST /api/quickbooks/webhook — invoice.created AB gate on batched intent', () => { + setupInvoiceCreatedTest() + + afterEach(() => { + abTestGate.reset() + }) + + it('freezes non-batched for a portal outside the allowlist despite the flag being on', async () => { + abTestGate.setAllowlist(['some-other-portal']) + await seedHealthyPortal({ setting: { bankDepositFeeFlag: true } }) + await seedProductSync() + + await postWebhook(invoiceCreatedPayload) + + const [row] = await db.select().from(QBInvoiceSync) + expect(row.isBatchedDeposit).toBe(false) + }) + + it('freezes batched for a portal on the allowlist with the flag on', async () => { + abTestGate.setAllowlist([TEST_PORTAL_ID]) + await seedHealthyPortal({ setting: { bankDepositFeeFlag: true } }) + await seedProductSync() + + await postWebhook(invoiceCreatedPayload) + + const [row] = await db.select().from(QBInvoiceSync) + expect(row.isBatchedDeposit).toBe(true) + }) +}) diff --git a/test/integration/setup.ts b/test/integration/setup.ts index e7bdd268..f50e5295 100644 --- a/test/integration/setup.ts +++ b/test/integration/setup.ts @@ -103,6 +103,23 @@ vi.mock('@/utils/sleep', () => ({ sleep: vi.fn().mockResolvedValue(undefined), })) +// AB gate for the bank deposit rollout. The real allowlist is parsed from env +// at `@/config` module load, so it can't be varied per-test once loaded. We +// mock the gate here (setupFiles runs before any app module binds it) and drive +// it via a globalThis-pinned allowlist. Default `null` = feature on for all +// portals, matching the empty-env behavior so existing tests are unaffected. +// A test opts in by setting `abTestGate.allowlist`; reset it in afterEach. +const AB_GATE_GLOBAL_KEY = '__qbsync_ab_test_gate__' +type ABGate = { allowlist: string[] | null } +const abGateRef = globalThis as unknown as Record +abGateRef[AB_GATE_GLOBAL_KEY] ??= { allowlist: null } +vi.mock('@/utils/abTesting', () => ({ + isPortalInBankDepositABTest: (portalId: string) => { + const gate = abGateRef[AB_GATE_GLOBAL_KEY]! + return gate.allowlist === null || gate.allowlist.includes(portalId) + }, +})) + // Importing modules that pull `next/server` corrupts NTARH's AsyncLocalStorage. // Shimming this entry point keeps the next/server import out of the graph. vi.mock('@/app/api/core/utils/afterIfAvailable', () => ({ diff --git a/test/unit/app/api/quickbooks/invoice/invoice.service.bankDepositGate.test.ts b/test/unit/app/api/quickbooks/invoice/invoice.service.bankDepositGate.test.ts new file mode 100644 index 00000000..4f7a1e03 --- /dev/null +++ b/test/unit/app/api/quickbooks/invoice/invoice.service.bankDepositGate.test.ts @@ -0,0 +1,90 @@ +/** + * Freeze-point coverage for InvoiceService#readBankDepositFeeFlag — the one + * place invoice creation decides batched intent. A portal outside the AB + * allowlist must freeze non-batched regardless of its stored setting, and must + * not even read the setting. readBankDepositFeeFlag is private, reached via a + * type cast (same approach as invoice.service.docNumber.test.ts). + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('@sentry/nextjs', () => ({ + withScope: vi.fn((cb: (scope: unknown) => void) => + cb({ setTag: vi.fn(), setExtra: vi.fn(), addEventProcessor: vi.fn() }), + ), + captureException: vi.fn(), + captureMessage: vi.fn(), + addBreadcrumb: vi.fn(), + init: vi.fn(), +})) +vi.mock('@/utils/logger', () => ({ + default: { info: vi.fn(), error: vi.fn() }, +})) +vi.mock('@/utils/copilotAPI', () => ({ CopilotAPI: vi.fn() })) +vi.mock('@/utils/intuitAPI', () => ({ + default: vi.fn(), + IntuitAPIErrorMessage: '#IntuitAPIErrorMessage#', +})) +// BaseService imports `@/db`, which initialises postgres at module load. +vi.mock('@/db', () => ({ db: {}, client: {} })) +vi.mock('@/utils/sentry', () => ({ + addSyncBreadcrumb: vi.fn(), + captureSyncError: vi.fn(), +})) +// SyncLogService is instantiated in the InvoiceService constructor. +vi.mock('@/app/api/quickbooks/syncLog/syncLog.service', () => ({ + SyncLogService: vi.fn(function () { + return {} + }), +})) + +const { getOneByPortalId, isPortalInBankDepositABTest } = vi.hoisted(() => ({ + getOneByPortalId: vi.fn(), + isPortalInBankDepositABTest: vi.fn(), +})) +vi.mock('@/app/api/quickbooks/setting/setting.service', () => ({ + SettingService: vi.fn(function () { + return { getOneByPortalId } + }), +})) +vi.mock('@/utils/abTesting', () => ({ isPortalInBankDepositABTest })) + +import { InvoiceService } from '@/app/api/quickbooks/invoice/invoice.service' +import User from '@/app/api/core/models/User.model' + +const stubUser = { + workspaceId: 'test-portal-00000001', + token: 'tkn', + qbConnection: undefined, +} as unknown as User + +type WithReadFlag = { readBankDepositFeeFlag: () => Promise } +const newSvc = () => new InvoiceService(stubUser) as unknown as WithReadFlag + +describe('InvoiceService#readBankDepositFeeFlag — AB freeze gate', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('freezes non-batched and skips the setting read for an excluded portal', async () => { + isPortalInBankDepositABTest.mockReturnValue(false) + getOneByPortalId.mockResolvedValue({ bankDepositFeeFlag: true }) + + expect(await newSvc().readBankDepositFeeFlag()).toBe(false) + expect(getOneByPortalId).not.toHaveBeenCalled() + }) + + it('honors the stored flag for an allowlisted portal', async () => { + isPortalInBankDepositABTest.mockReturnValue(true) + getOneByPortalId.mockResolvedValue({ bankDepositFeeFlag: true }) + + expect(await newSvc().readBankDepositFeeFlag()).toBe(true) + }) + + it('defaults to false when an allowlisted portal has no setting row', async () => { + isPortalInBankDepositABTest.mockReturnValue(true) + getOneByPortalId.mockResolvedValue(undefined) + + expect(await newSvc().readBankDepositFeeFlag()).toBe(false) + }) +}) diff --git a/test/unit/payout/payout.service.bankDepositGate.test.ts b/test/unit/payout/payout.service.bankDepositGate.test.ts new file mode 100644 index 00000000..2ee91d82 --- /dev/null +++ b/test/unit/payout/payout.service.bankDepositGate.test.ts @@ -0,0 +1,76 @@ +/** + * AB-gate coverage for PayoutService#reconcile — the deposit-creating step, + * shared by the payout webhook and the resync cron. An excluded portal must + * short-circuit to { depositId: null } before any token check or QBO call. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { QBPayoutSyncSelectSchemaType } from '@/db/schema/qbPayoutSync' +import type { IntuitAPITokensType } from '@/utils/intuitAPI' + +vi.mock('@sentry/nextjs', () => ({ + withScope: vi.fn(), + captureException: vi.fn(), + captureMessage: vi.fn(), + addBreadcrumb: vi.fn(), + init: vi.fn(), +})) +// BaseService imports `@/db`, which initialises postgres at module load. +vi.mock('@/db', () => ({ db: {}, client: {} })) +vi.mock('@/utils/copilotAPI', () => ({ CopilotAPI: vi.fn() })) +vi.mock('@/utils/intuitAPI', () => ({ default: vi.fn() })) +vi.mock('@/app/api/quickbooks/syncLog/syncLog.service', () => ({ + SyncLogService: vi.fn(function () { + return {} + }), +})) + +const { validateAccessToken, isPortalInBankDepositABTest } = vi.hoisted(() => ({ + validateAccessToken: vi.fn(), + isPortalInBankDepositABTest: vi.fn(), +})) +vi.mock('@/utils/auth', () => ({ validateAccessToken })) +vi.mock('@/utils/abTesting', () => ({ isPortalInBankDepositABTest })) + +import { PayoutService } from '@/app/api/quickbooks/payout/payout.service' +import User from '@/app/api/core/models/User.model' + +const stubUser = { workspaceId: 'test-portal-00000001' } as unknown as User +const stubRow = { + payoutId: 'po_123', +} as unknown as QBPayoutSyncSelectSchemaType +const stubTokens = {} as IntuitAPITokensType + +describe('PayoutService#reconcile — AB gate', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('short-circuits to no deposit for an excluded portal without checking the token', async () => { + isPortalInBankDepositABTest.mockReturnValue(false) + + const result = await new PayoutService(stubUser).reconcile( + stubRow, + stubTokens, + { runIdempotencyCheck: true }, + ) + + expect(result).toEqual({ depositId: null }) + expect(validateAccessToken).not.toHaveBeenCalled() + }) + + it('proceeds past the gate for an allowlisted portal', async () => { + isPortalInBankDepositABTest.mockReturnValue(true) + // Force a stop right after the gate so we assert only that it advanced. + validateAccessToken.mockImplementation(() => { + throw new Error('advanced past gate') + }) + + await expect( + new PayoutService(stubUser).reconcile(stubRow, stubTokens, { + runIdempotencyCheck: true, + }), + ).rejects.toThrow('advanced past gate') + expect(validateAccessToken).toHaveBeenCalledTimes(1) + }) +}) diff --git a/test/unit/setting/setting.controller.test.ts b/test/unit/setting/setting.controller.test.ts new file mode 100644 index 00000000..70092888 --- /dev/null +++ b/test/unit/setting/setting.controller.test.ts @@ -0,0 +1,168 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest' +import type { NextRequest } from 'next/server' + +// Spies are declared via vi.hoisted so the vi.mock factories below (which are +// hoisted above the imports) can reference them. +const { + updateQBSettings, + updateQBPortalConnection, + getOneByPortalId, + getPortalConnection, + isPortalInBankDepositABTest, + transaction, +} = vi.hoisted(() => ({ + updateQBSettings: vi.fn(), + updateQBPortalConnection: vi.fn(), + getOneByPortalId: vi.fn(), + getPortalConnection: vi.fn(), + isPortalInBankDepositABTest: vi.fn(), + transaction: vi.fn(), +})) + +vi.mock('@/db', () => ({ db: { transaction }, client: {} })) +vi.mock('@/db/service/token.service', () => ({ getPortalConnection })) +vi.mock('@/app/api/core/utils/authenticate', () => ({ + default: vi.fn(async () => ({ workspaceId: 'portal-1', token: 'token' })), +})) +vi.mock('@/app/api/quickbooks/setting/setting.service', () => ({ + SettingService: vi.fn(function () { + return { + setTransaction: vi.fn(), + unsetTransaction: vi.fn(), + updateQBSettings, + getOneByPortalId, + } + }), +})) +vi.mock('@/app/api/quickbooks/token/token.service', () => ({ + TokenService: vi.fn(function () { + return { + setTransaction: vi.fn(), + unsetTransaction: vi.fn(), + updateQBPortalConnection, + } + }), +})) +vi.mock('@/utils/abTesting', () => ({ isPortalInBankDepositABTest })) + +import { + getSettings, + updateSettings, +} from '@/app/api/quickbooks/setting/setting.controller' + +// Minimal request stub: the controller only reads the `type` search param and +// the JSON body. +function invoiceSettingsRequest(body: Record): NextRequest { + return { + nextUrl: { searchParams: new URLSearchParams({ type: 'invoice' }) }, + json: async () => ({ type: 'invoice', ...body }), + } as unknown as NextRequest +} + +function getSettingsRequest(type: string): NextRequest { + return { + nextUrl: { searchParams: new URLSearchParams({ type }) }, + } as unknown as NextRequest +} + +const baseInvoiceBody = { + absorbedFeeFlag: true, + useCompanyNameFlag: false, +} + +describe('updateSettings — bank deposit AB gate', () => { + beforeEach(() => { + vi.clearAllMocks() + updateQBSettings.mockImplementation(async (payload) => ({ + id: 'setting-1', + ...payload, + })) + transaction.mockImplementation(async (cb) => cb({})) + }) + + it('drops the bank deposit flag for a portal that is not in the AB test', async () => { + isPortalInBankDepositABTest.mockReturnValue(false) + + await updateSettings( + invoiceSettingsRequest({ + ...baseInvoiceBody, + bankDepositFeeFlag: true, + bankAccountRef: 'account-1', + }), + ) + + expect(updateQBSettings).toHaveBeenCalledTimes(1) + const savedPayload = updateQBSettings.mock.calls[0][0] + expect(savedPayload).not.toHaveProperty('bankDepositFeeFlag') + // Bank account ref is never written for a non-AB portal, even when supplied. + expect(updateQBPortalConnection).not.toHaveBeenCalled() + }) + + it('saves the flag and the bank account ref for an AB-test portal', async () => { + isPortalInBankDepositABTest.mockReturnValue(true) + + await updateSettings( + invoiceSettingsRequest({ + ...baseInvoiceBody, + bankDepositFeeFlag: true, + bankAccountRef: 'account-9', + }), + ) + + const savedPayload = updateQBSettings.mock.calls[0][0] + expect(savedPayload.bankDepositFeeFlag).toBe(true) + expect(updateQBPortalConnection).toHaveBeenCalledWith( + { bankAccountRef: 'account-9' }, + expect.anything(), + ) + }) + + it('lets an AB-test portal turn the flag off without a bank account', async () => { + isPortalInBankDepositABTest.mockReturnValue(true) + + await updateSettings( + invoiceSettingsRequest({ + ...baseInvoiceBody, + bankDepositFeeFlag: false, + }), + ) + + const savedPayload = updateQBSettings.mock.calls[0][0] + expect(savedPayload.bankDepositFeeFlag).toBe(false) + expect(updateQBPortalConnection).not.toHaveBeenCalled() + }) +}) + +describe('getSettings — bankDepositEnabled signal', () => { + beforeEach(() => { + vi.clearAllMocks() + getOneByPortalId.mockResolvedValue({ id: 'setting-1' }) + getPortalConnection.mockResolvedValue({ bankAccountRef: 'account-1' }) + }) + + it('reports the AB gate as the bankDepositEnabled flag for invoice settings', async () => { + isPortalInBankDepositABTest.mockReturnValue(true) + + const response = await getSettings(getSettingsRequest('invoice')) + + expect(await response.json()).toMatchObject({ bankDepositEnabled: true }) + }) + + it('reports bankDepositEnabled false for an excluded portal', async () => { + isPortalInBankDepositABTest.mockReturnValue(false) + + const response = await getSettings(getSettingsRequest('invoice')) + + expect(await response.json()).toMatchObject({ bankDepositEnabled: false }) + }) + + it('never enables the signal for non-invoice settings', async () => { + isPortalInBankDepositABTest.mockReturnValue(true) + + const response = await getSettings(getSettingsRequest('product')) + + expect(await response.json()).toMatchObject({ bankDepositEnabled: false }) + // The gate is not even consulted outside invoice settings. + expect(isPortalInBankDepositABTest).not.toHaveBeenCalled() + }) +}) diff --git a/test/unit/utils/abTesting.test.ts b/test/unit/utils/abTesting.test.ts new file mode 100644 index 00000000..057b08ac --- /dev/null +++ b/test/unit/utils/abTesting.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect, afterEach, vi } from 'vitest' + +// abFeatureTestingPortals is parsed from the env var at module load, so each +// case stubs the env, resets the module registry, and re-imports to pick up +// the fresh parse. +async function loadGate(envValue?: string) { + vi.resetModules() + if (envValue === undefined) { + vi.stubEnv('AB_FEATURE_TESTING_PORTALS', '') + } else { + vi.stubEnv('AB_FEATURE_TESTING_PORTALS', envValue) + } + const { isPortalInBankDepositABTest } = await import('@/utils/abTesting') + return isPortalInBankDepositABTest +} + +describe('isPortalInBankDepositABTest', () => { + afterEach(() => { + vi.unstubAllEnvs() + }) + + it('allows every portal when the allowlist is unset', async () => { + const isInTest = await loadGate(undefined) + expect(isInTest('portal-abc')).toBe(true) + }) + + it('allows every portal when the allowlist is empty', async () => { + const isInTest = await loadGate('') + expect(isInTest('portal-abc')).toBe(true) + }) + + it('allows a portal that is on the allowlist', async () => { + const isInTest = await loadGate('portal-abc,portal-def') + expect(isInTest('portal-abc')).toBe(true) + expect(isInTest('portal-def')).toBe(true) + }) + + it('blocks a portal that is not on the allowlist', async () => { + const isInTest = await loadGate('portal-abc,portal-def') + expect(isInTest('portal-xyz')).toBe(false) + }) + + it('ignores surrounding whitespace and empty entries in the allowlist', async () => { + const isInTest = await loadGate(' portal-abc , , portal-def ,') + expect(isInTest('portal-abc')).toBe(true) + expect(isInTest('portal-def')).toBe(true) + expect(isInTest('portal-xyz')).toBe(false) + }) +})