From cac33860bb79c24a011bd35be9f031b744fe30ea Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 14 Oct 2026 20:48:45 +0000 Subject: [PATCH 1/2] feat: disable QBO bank reconciliation beta (OUT-4262) - Add hard-off flag in isPortalInBankDepositABTest to disable the feature for all portals regardless of AB_FEATURE_TESTING_PORTALS env var - Remove bank deposit toggle and bank account picker from Invoice Details UI - Remove bank deposit warning modal and related state from useInvoiceDetailSettings - Remove unused props from InvoiceDetail component and SettingAccordion - Update unit tests to reflect the hard-off behavior The feature code is preserved and can be re-enabled by setting BANK_DEPOSIT_FEATURE_ENABLED to true in src/utils/abTesting.ts. This changes the semantics of AB_FEATURE_TESTING_PORTALS: previously empty/unset meant enabled for all portals; now the hard-off flag takes precedence, so empty/unset means disabled for everyone. Co-authored-by: Adam Schwartz --- .../dashboard/settings/SettingAccordion.tsx | 17 ------ .../sections/invoice/InvoiceDetail.tsx | 53 +---------------- src/hook/useSettings.ts | 58 ++----------------- src/utils/abTesting.ts | 18 ++++-- test/unit/setting/setting.controller.test.ts | 14 +++-- test/unit/utils/abTesting.test.ts | 28 ++++----- 6 files changed, 37 insertions(+), 151 deletions(-) diff --git a/src/components/dashboard/settings/SettingAccordion.tsx b/src/components/dashboard/settings/SettingAccordion.tsx index 2d05810a..a570944a 100644 --- a/src/components/dashboard/settings/SettingAccordion.tsx +++ b/src/components/dashboard/settings/SettingAccordion.tsx @@ -3,7 +3,6 @@ import InvoiceDetail from '@/components/dashboard/settings/sections/invoice/Invo import AccountMapping from '@/components/dashboard/settings/sections/account/AccountMapping' import ProductMapping from '@/components/dashboard/settings/sections/product/ProductMapping' import Accordion from '@/components/ui/Accordion' -import ConfirmModal from '@/components/ui/ConfirmModal' import Divider from '@/components/ui/Divider' import { useInvoiceDetailSettings, @@ -44,14 +43,8 @@ export default function SettingAccordion({ isLoading, changeSettings, showButton: showInvoiceButton, - bankDepositEnabled, - bankAccountOptions, - bankAccountsError, canSave, - showBankDepositWarning, requestInvoiceSettingsSave, - confirmBankDepositChange, - cancelBankDepositChange, } = useInvoiceDetailSettings() const { @@ -94,9 +87,6 @@ export default function SettingAccordion({ settingState={settingState} changeSettings={changeSettings} isLoading={isLoading} - bankDepositEnabled={bankDepositEnabled} - bankAccountOptions={bankAccountOptions} - bankAccountsError={bankAccountsError} /> ), }, @@ -208,13 +198,6 @@ export default function SettingAccordion({ ) })} - ) } diff --git a/src/components/dashboard/settings/sections/invoice/InvoiceDetail.tsx b/src/components/dashboard/settings/sections/invoice/InvoiceDetail.tsx index 8f705050..31bf100f 100644 --- a/src/components/dashboard/settings/sections/invoice/InvoiceDetail.tsx +++ b/src/components/dashboard/settings/sections/invoice/InvoiceDetail.tsx @@ -1,6 +1,5 @@ import { useApp } from '@/app/context/AppContext' -import AccountSelect from '@/components/dashboard/settings/sections/account/AccountSelect' -import { AccountOption, InvoiceSettingType } from '@/type/common' +import { InvoiceSettingType } from '@/type/common' import { getWorkspaceLabel } from '@/utils/workspace' import { Checkbox, Spinner } from 'copilot-design-system' @@ -11,18 +10,12 @@ type InvoiceDetailProps = { value: InvoiceSettingType[K], ) => void isLoading: boolean - bankDepositEnabled: boolean - bankAccountOptions: AccountOption[] | undefined - bankAccountsError: unknown } export default function InvoiceDetail({ settingState, changeSettings, isLoading, - bankDepositEnabled, - bankAccountOptions, - bankAccountsError, }: InvoiceDetailProps) { const { workspace } = useApp() @@ -43,50 +36,6 @@ export default function InvoiceDetail({ } /> - {/* 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. -

- )} - - )} -
- )} - - )}
{ useCompanyNameFlag: false, bankAccountRef: '', } - const { token, setAppParams, syncFlag, portalConnectionStatus } = useApp() - // Skip the /bank-account fetch when QB isn't connected or sync is off, same - // rationale as useAccountMapping's isDisconnected. - const isDisconnected = !syncFlag || !portalConnectionStatus + const { token, setAppParams } = useApp() const [settingState, setSettingState] = useState( initialInvoiceSetting, ) const [showButton, setShowButton] = useState(false) - const [showBankDepositWarning, setShowBankDepositWarning] = useState(false) const [intialSettingState, setIntialSettingState] = useState< InvoiceSettingType | undefined >() @@ -452,23 +448,6 @@ 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 || !bankDepositEnabled - ? null - : `/api/quickbooks/setting/bank-account?token=${token}`, - { suspense: false, revalidateOnMount: true }, - ) - const bankAccountOptions: AccountOption[] | undefined = - bankAccountsData?.accounts.map((account) => ({ - id: account.Id, - name: account.Name, - })) - const changeSettings = async ( flag: K, value: InvoiceSettingType[K], @@ -476,9 +455,8 @@ export const useInvoiceDetailSettings = () => { setSettingState((prev) => ({ ...prev, [flag]: value })) } - const canSave = !( - settingState.bankDepositFeeFlag && !settingState.bankAccountRef - ) + // Bank deposit feature disabled (OUT-4262), so canSave is always true. + const canSave = true useEffect(() => { if (!settingState || !intialSettingState) return @@ -516,7 +494,7 @@ export const useInvoiceDetailSettings = () => { ) mutate(`/api/quickbooks/setting?type=invoice&token=${token}`) } catch (err) { - setShowButton(true) // show the update settings button if error + setShowButton(true) console.error('Error submitting Invoice settings', err) } } @@ -526,26 +504,6 @@ export const useInvoiceDetailSettings = () => { setSettingState(intialSettingState || initialInvoiceSetting) } - // Warn only when the bank-deposit flag actually changed vs the saved value. - const bankDepositFlagChanged = - !!intialSettingState && - settingState.bankDepositFeeFlag !== intialSettingState.bankDepositFeeFlag - - const requestInvoiceSettingsSave = () => { - if (bankDepositFlagChanged) { - setShowBankDepositWarning(true) - return - } - submitInvoiceSettings() - } - - const confirmBankDepositChange = () => { - setShowBankDepositWarning(false) - submitInvoiceSettings() - } - - const cancelBankDepositChange = () => setShowBankDepositWarning(false) - return { settingState, changeSettings, @@ -553,14 +511,8 @@ export const useInvoiceDetailSettings = () => { error, isLoading, showButton, - bankDepositEnabled, - bankAccountOptions, - bankAccountsError, canSave, - showBankDepositWarning, - requestInvoiceSettingsSave, - confirmBankDepositChange, - cancelBankDepositChange, + requestInvoiceSettingsSave: submitInvoiceSettings, } } diff --git a/src/utils/abTesting.ts b/src/utils/abTesting.ts index 2a64c5cb..9b8d3c74 100644 --- a/src/utils/abTesting.ts +++ b/src/utils/abTesting.ts @@ -1,11 +1,19 @@ 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. + * Hard-off flag: the QBO automatic bank reconciliation beta has been wound down + * (OUT-4262). Set to `true` to re-enable the feature with the allowlist gate. */ -export function isPortalInBankDepositABTest(portalId: string): boolean { +const BANK_DEPOSIT_FEATURE_ENABLED = false + +/** + * Whether a portal may use the bank deposit feature. The feature is currently + * disabled for all portals (OUT-4262 wind-down). When BANK_DEPOSIT_FEATURE_ENABLED + * is true, an empty/unset allowlist means enabled for all; otherwise only + * listed portals get it. + */ +export function isPortalInBankDepositABTest(_portalId: string): boolean { + if (!BANK_DEPOSIT_FEATURE_ENABLED) return false if (abFeatureTestingPortals.length === 0) return true - return abFeatureTestingPortals.includes(portalId) + return abFeatureTestingPortals.includes(_portalId) } diff --git a/test/unit/setting/setting.controller.test.ts b/test/unit/setting/setting.controller.test.ts index 70092888..1910deea 100644 --- a/test/unit/setting/setting.controller.test.ts +++ b/test/unit/setting/setting.controller.test.ts @@ -80,7 +80,10 @@ describe('updateSettings — bank deposit AB gate', () => { transaction.mockImplementation(async (cb) => cb({})) }) - it('drops the bank deposit flag for a portal that is not in the AB test', async () => { + // OUT-4262: Bank deposit feature is disabled via hard-off flag. The AB gate + // always returns false, so the flag is always dropped regardless of portal. + + it('drops the bank deposit flag when feature is disabled (hard-off)', async () => { isPortalInBankDepositABTest.mockReturnValue(false) await updateSettings( @@ -94,11 +97,10 @@ describe('updateSettings — bank deposit AB gate', () => { 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 () => { + it('drops the flag even when gate mock returns true (defensive test for future re-enablement)', async () => { isPortalInBankDepositABTest.mockReturnValue(true) await updateSettings( @@ -117,8 +119,8 @@ describe('updateSettings — bank deposit AB gate', () => { ) }) - it('lets an AB-test portal turn the flag off without a bank account', async () => { - isPortalInBankDepositABTest.mockReturnValue(true) + it('saves absorbedFeeFlag without bankDepositFeeFlag when flag is off', async () => { + isPortalInBankDepositABTest.mockReturnValue(false) await updateSettings( invoiceSettingsRequest({ @@ -128,7 +130,7 @@ describe('updateSettings — bank deposit AB gate', () => { ) const savedPayload = updateQBSettings.mock.calls[0][0] - expect(savedPayload.bankDepositFeeFlag).toBe(false) + expect(savedPayload).not.toHaveProperty('bankDepositFeeFlag') expect(updateQBPortalConnection).not.toHaveBeenCalled() }) }) diff --git a/test/unit/utils/abTesting.test.ts b/test/unit/utils/abTesting.test.ts index 057b08ac..108a1672 100644 --- a/test/unit/utils/abTesting.test.ts +++ b/test/unit/utils/abTesting.test.ts @@ -19,31 +19,23 @@ describe('isPortalInBankDepositABTest', () => { vi.unstubAllEnvs() }) - it('allows every portal when the allowlist is unset', async () => { - const isInTest = await loadGate(undefined) - expect(isInTest('portal-abc')).toBe(true) - }) + // OUT-4262: Bank deposit feature is now disabled via hard-off flag. + // The following tests verify the hard-off behavior. When the feature is + // re-enabled, revert these to the original assertions. - it('allows every portal when the allowlist is empty', async () => { - const isInTest = await loadGate('') - expect(isInTest('portal-abc')).toBe(true) + it('blocks every portal when the hard-off flag is set (feature disabled)', async () => { + const isInTest = await loadGate(undefined) + expect(isInTest('portal-abc')).toBe(false) }) - it('allows a portal that is on the allowlist', async () => { + it('blocks every portal even with allowlist when hard-off is set', async () => { const isInTest = await loadGate('portal-abc,portal-def') - expect(isInTest('portal-abc')).toBe(true) - expect(isInTest('portal-def')).toBe(true) + expect(isInTest('portal-abc')).toBe(false) + expect(isInTest('portal-def')).toBe(false) }) - it('blocks a portal that is not on the allowlist', async () => { + it('blocks portals not on allowlist when hard-off is set', 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) - }) }) From 8a1a180877333abd4f2de0e725b689699c4f7ccc Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 15 Oct 2026 11:35:41 +0000 Subject: [PATCH 2/2] fix: allow payout reconciliation for existing batched invoices (OUT-4262) - Remove AB gate from PayoutService#reconcile() so existing invoices with isBatchedDeposit=true can still have their deposits created and fees booked - Keep the hard-off at the invoice freeze point (InvoiceService#readBankDepositFeeFlag) so new invoices freeze as non-batched - Update payout service unit tests to reflect the removal of AB gate - Rename misnamed test to 'saves the flag and bank account when the gate returns true' This ensures payments already in Undeposited Funds get properly deposited and Stripe fees are booked, while preventing new invoices from being batched. Co-authored-by: Adam Schwartz --- .../api/quickbooks/payout/payout.service.ts | 16 +++----- .../payout.service.bankDepositGate.test.ts | 39 +++++++------------ test/unit/setting/setting.controller.test.ts | 2 +- 3 files changed, 20 insertions(+), 37 deletions(-) diff --git a/src/app/api/quickbooks/payout/payout.service.ts b/src/app/api/quickbooks/payout/payout.service.ts index 0cbf46a2..bccf88bf 100644 --- a/src/app/api/quickbooks/payout/payout.service.ts +++ b/src/app/api/quickbooks/payout/payout.service.ts @@ -19,7 +19,6 @@ 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 @@ -80,20 +79,17 @@ export class PayoutService extends BaseService { // Checks the payout, finds its payments, then builds and creates the deposit. // Neither caller claims again. Returns { depositId: null } when there is // nothing to deposit. + // + // NOTE (OUT-4262): The AB gate was removed from this method so existing + // invoices with isBatchedDeposit=true can still have their deposits created + // and Stripe fees booked. New invoices freeze as non-batched (the freeze + // point in InvoiceService#readBankDepositFeeFlag returns false), so this + // method will naturally receive only non-batched payouts going forward. async reconcile( row: QBPayoutSyncSelectSchemaType, 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/test/unit/payout/payout.service.bankDepositGate.test.ts b/test/unit/payout/payout.service.bankDepositGate.test.ts index 7e7a69af..6adf52d3 100644 --- a/test/unit/payout/payout.service.bankDepositGate.test.ts +++ b/test/unit/payout/payout.service.bankDepositGate.test.ts @@ -1,7 +1,11 @@ /** - * 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. + * Coverage for PayoutService#reconcile — the deposit-creating step, shared by + * the payout webhook and the resync cron. + * + * OUT-4262: The AB gate was removed from this method so existing invoices with + * isBatchedDeposit=true can still have their deposits created and Stripe fees + * booked. The freeze point (InvoiceService#readBankDepositFeeFlag) blocks new + * invoices from being batched, so this method naturally winds down. */ import { describe, it, expect, vi, beforeEach } from 'vitest' @@ -15,7 +19,6 @@ vi.mock('@sentry/nextjs', () => ({ addBreadcrumb: vi.fn(), init: vi.fn(), })) -// BaseService imports `@/db`, which initialises postgres at module load. vi.mock('@/db', () => ({ db: {}, client: {} })) vi.mock('@/utils/assemblyAPI', () => ({ AssemblyAPI: vi.fn() })) vi.mock('@/utils/intuitAPI', () => ({ default: vi.fn() })) @@ -25,12 +28,10 @@ vi.mock('@/app/api/quickbooks/syncLog/syncLog.service', () => ({ }), })) -const { validateAccessToken, isPortalInBankDepositABTest } = vi.hoisted(() => ({ +const { validateAccessToken } = 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' @@ -41,36 +42,22 @@ const stubRow = { } as unknown as QBPayoutSyncSelectSchemaType const stubTokens = {} as IntuitAPITokensType -describe('PayoutService#reconcile — AB gate', () => { +describe('PayoutService#reconcile — no AB gate (OUT-4262)', () => { 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. + it('proceeds directly to token validation without an AB gate check', async () => { + // Force a stop at token validation to confirm the method proceeds. validateAccessToken.mockImplementation(() => { - throw new Error('advanced past gate') + throw new Error('advanced to token validation') }) await expect( new PayoutService(stubUser).reconcile(stubRow, stubTokens, { runIdempotencyCheck: true, }), - ).rejects.toThrow('advanced past gate') + ).rejects.toThrow('advanced to token validation') expect(validateAccessToken).toHaveBeenCalledTimes(1) }) }) diff --git a/test/unit/setting/setting.controller.test.ts b/test/unit/setting/setting.controller.test.ts index 1910deea..64050afe 100644 --- a/test/unit/setting/setting.controller.test.ts +++ b/test/unit/setting/setting.controller.test.ts @@ -100,7 +100,7 @@ describe('updateSettings — bank deposit AB gate', () => { expect(updateQBPortalConnection).not.toHaveBeenCalled() }) - it('drops the flag even when gate mock returns true (defensive test for future re-enablement)', async () => { + it('saves the flag and bank account when the gate returns true', async () => { isPortalInBankDepositABTest.mockReturnValue(true) await updateSettings(