diff --git a/src/app/api/core/types/notification.ts b/src/app/api/core/types/notification.ts index fee09918..81984753 100644 --- a/src/app/api/core/types/notification.ts +++ b/src/app/api/core/types/notification.ts @@ -31,6 +31,9 @@ export interface NotificationContext { // Comma-joined invoice numbers for a multi-invoice failure (mixed payout), // where the single invoiceNumber above can't hold them all. invoiceNumbers?: string | null + // Subset of invoiceNumbers whose absorbed fee is already recorded in QBO, so a + // mixed-payout body can tell IUs which fees not to record a second time. + invoiceNumbersWithFee?: string | null customerName?: string | null productName?: string | null qbItemName?: string | null diff --git a/src/app/api/notification/notification.helper.ts b/src/app/api/notification/notification.helper.ts index 19da981b..eb2f34ce 100644 --- a/src/app/api/notification/notification.helper.ts +++ b/src/app/api/notification/notification.helper.ts @@ -249,14 +249,20 @@ export const NotificationCopy: Record< const forInvoices = ctx?.invoiceNumbers ? ` for invoices ${ctx.invoiceNumbers}` : '' - return `A Stripe payout${ref} could not be recorded in QuickBooks because it mixes invoices set to batch into a bank deposit with invoices that are not — this happens when the bank-deposit setting changed between a payment and its payout. No deposit was created${forInvoices}, so nothing was double-booked. Record this payout's deposit manually in QuickBooks. This will not be retried automatically.` + const recordedFees = ctx?.invoiceNumbersWithFee + ? ` The Stripe fees for ${ctx.invoiceNumbersWithFee} are already recorded as expenses in QuickBooks, so do not record those fees again.` + : '' + return `A Stripe payout${ref} could not be recorded in QuickBooks because it mixes invoices set to batch into a bank deposit with invoices that are not — this happens when the bank-deposit setting changed between a payment and its payout. No deposit was created${forInvoices}. The payments are already recorded in QuickBooks; they just haven't been grouped into a bank deposit.${recordedFees} Record this payout's deposit manually in QuickBooks. This will not be retried automatically.` }, emailSubject: 'QuickBooks sync failed: payout needs manual reconciliation', emailBody: (ref, ctx) => { const forInvoices = ctx?.invoiceNumbers ? ` for invoices ${ctx.invoiceNumbers}` : '' - return `A Stripe payout${ref} could not be recorded in QuickBooks because it mixes invoices set to batch into a bank deposit with invoices that are not. This happens when the bank-deposit setting changed between a payment and its payout. No deposit was created${forInvoices}, so nothing was double-booked. Record this payout's deposit manually in QuickBooks. This payout will not be retried automatically.` + const recordedFees = ctx?.invoiceNumbersWithFee + ? ` The Stripe fees for ${ctx.invoiceNumbersWithFee} are already recorded as expenses in QuickBooks, so do not record those fees again.` + : '' + return `A Stripe payout${ref} could not be recorded in QuickBooks because it mixes invoices set to batch into a bank deposit with invoices that are not. This happens when the bank-deposit setting changed between a payment and its payout. No deposit was created${forInvoices}. The payments are already recorded in QuickBooks; they just haven't been grouped into a bank deposit.${recordedFees} Record this payout's deposit manually in QuickBooks. This payout will not be retried automatically.` }, }, diff --git a/src/app/api/quickbooks/syncLog/syncErrorNotifier.ts b/src/app/api/quickbooks/syncLog/syncErrorNotifier.ts index 46c1008e..2861e425 100644 --- a/src/app/api/quickbooks/syncLog/syncErrorNotifier.ts +++ b/src/app/api/quickbooks/syncLog/syncErrorNotifier.ts @@ -7,10 +7,13 @@ import { import { NotificationService } from '@/app/api/notification/notification.service' import { AppActionableErrorCodes, + MIXED_INTENT_INVOICE_DELIMITER, UserActionableErrorCodes, } from '@/constant/intuitErrorCode' import { QBSyncLogSelectSchemaType } from '@/db/schema/qbSyncLogs' import { getPortalConnection } from '@/db/service/token.service' +import { getInvoiceNumbersWithRecordedFee } from '@/db/service/syncLog.service' +import CustomLogger from '@/utils/logger' /** * Looks up the user-actionable notification action for a given QBO error code. @@ -43,6 +46,13 @@ export function getEntityKey(log: QBSyncLogSelectSchemaType): string { ) } +type MixedPayoutInvoices = { + // Display-joined affected invoice numbers (from the log `remark`). + affectedInvoiceNumbers?: string + // Subset whose absorbed fee is already recorded in QBO. + invoiceNumbersWithFee?: string +} + export class SyncErrorNotifier extends BaseService { /** * Dispatches an IU notification for a freshly written FAILED sync log row @@ -69,6 +79,15 @@ export class SyncErrorNotifier extends BaseService { return } + // Only mixed-payout rows carry an affected-invoice list to resolve. + const { + affectedInvoiceNumbers, + invoiceNumbersWithFee, + }: MixedPayoutInvoices = + action !== NotificationActions.QB_PAYOUT_MIXED_INTENT + ? {} + : await this.resolveMixedPayoutInvoices(log.remark) + const context: NotificationContext = { entityType: log.entityType, eventType: log.eventType, @@ -78,12 +97,8 @@ export class SyncErrorNotifier extends BaseService { productName: log.productName, qbItemName: log.qbItemName, errorMessage: log.errorMessage, - // Mixed-payout rows stash the affected invoice numbers in `remark`; surface - // them for the body while copilotId stays the ref. - invoiceNumbers: - action === NotificationActions.QB_PAYOUT_MIXED_INTENT - ? log.remark - : undefined, + invoiceNumbers: affectedInvoiceNumbers, + invoiceNumbersWithFee, } const portal = await getPortalConnection(this.user.workspaceId) @@ -97,4 +112,34 @@ export class SyncErrorNotifier extends BaseService { context, ) } + + // Resolve a mixed-payout `remark` into its affected invoices and the subset + // with a recorded fee; a lookup blip drops that detail, not the notification. + private async resolveMixedPayoutInvoices( + remark: string | null, + ): Promise { + if (!remark) return {} + const affected = remark + .split(MIXED_INTENT_INVOICE_DELIMITER) + .filter(Boolean) + let invoiceNumbersWithFee: string | undefined + try { + const withFee = await getInvoiceNumbersWithRecordedFee( + this.user.workspaceId, + affected, + ) + const recorded = affected.filter((invoiceNumber) => + withFee.has(invoiceNumber), + ) + if (recorded.length) + invoiceNumbersWithFee = recorded.join(MIXED_INTENT_INVOICE_DELIMITER) + } catch (error) { + CustomLogger.error({ + message: + 'SyncErrorNotifier | recorded-fee lookup failed; notifying without it', + obj: error, + }) + } + return { affectedInvoiceNumbers: remark, invoiceNumbersWithFee } + } } diff --git a/src/app/api/quickbooks/webhook/webhook.service.ts b/src/app/api/quickbooks/webhook/webhook.service.ts index 257b1766..3d5df6a3 100644 --- a/src/app/api/quickbooks/webhook/webhook.service.ts +++ b/src/app/api/quickbooks/webhook/webhook.service.ts @@ -40,7 +40,10 @@ import { getCategory, getShouldRetryForCategory } from '@/utils/synclog' import { addSyncBreadcrumb } from '@/utils/sentry' import { and, eq } from 'drizzle-orm' import httpStatus from 'http-status' -import { PAYOUT_MIXED_INTENT_CODE } from '@/constant/intuitErrorCode' +import { + MIXED_INTENT_INVOICE_DELIMITER, + PAYOUT_MIXED_INTENT_CODE, +} from '@/constant/intuitErrorCode' export class WebhookService extends BaseService { async handleWebhookEvent( @@ -723,7 +726,7 @@ export class WebhookService extends BaseService { const affectedInvoiceNumbers = copilotInvoiceIds .map((id) => paymentIdByInvoice.get(id)?.invoiceNumber) .filter(Boolean) - .join(', ') + .join(MIXED_INTENT_INVOICE_DELIMITER) // Single FAILED-log write. Mixed intent gets the routable sentinel so // SyncErrorNotifier alerts IUs; everything else keeps its derived code. // No qbItemName — it would outrank copilotId (the payout id) in the diff --git a/src/constant/intuitErrorCode.ts b/src/constant/intuitErrorCode.ts index 8b2cc262..f164400a 100644 --- a/src/constant/intuitErrorCode.ts +++ b/src/constant/intuitErrorCode.ts @@ -63,6 +63,10 @@ export const UserActionableErrorCodes: Record = { // above. Written to qb_sync_logs.error_code so SyncErrorNotifier routes it. export const PAYOUT_MIXED_INTENT_CODE = 'payout_mixed_intent' +// Packs the affected invoice numbers into a mixed-payout log's `remark`. Shared +// so the writer's join and the notifier's split can't drift. +export const MIXED_INTENT_INVOICE_DELIMITER = ', ' + // App-level (non-QBO) sentinel codes routed to IU notifications, consulted by // getActionForErrorCode alongside UserActionableErrorCodes. export const AppActionableErrorCodes: Record = { diff --git a/src/db/service/syncLog.service.ts b/src/db/service/syncLog.service.ts new file mode 100644 index 00000000..927b0b75 --- /dev/null +++ b/src/db/service/syncLog.service.ts @@ -0,0 +1,36 @@ +'use server' +import { EntityType, EventType, LogStatus } from '@/app/api/core/types/log' +import { db } from '@/db' +import { QBSyncLog } from '@/db/schema/qbSyncLogs' +import { and, eq, inArray, isNull } from 'drizzle-orm' + +// Which of these invoices already have a recorded absorbed-fee expense in QBO. +// A SUCCESS PAYMENT/SUCCEEDED row exists only if the fee Purchase was created. +export const getInvoiceNumbersWithRecordedFee = async ( + portalId: string, + invoiceNumbers: string[], +): Promise> => { + if (invoiceNumbers.length === 0) return new Set() + + const rows = await db + .select({ invoiceNumber: QBSyncLog.invoiceNumber }) + .from(QBSyncLog) + .where( + and( + eq(QBSyncLog.portalId, portalId), + eq(QBSyncLog.entityType, EntityType.PAYMENT), + eq(QBSyncLog.eventType, EventType.SUCCEEDED), + eq(QBSyncLog.status, LogStatus.SUCCESS), + inArray(QBSyncLog.invoiceNumber, invoiceNumbers), + isNull(QBSyncLog.deletedAt), + ), + ) + + return new Set( + rows + .map((row) => row.invoiceNumber) + .filter((invoiceNumber): invoiceNumber is string => + Boolean(invoiceNumber), + ), + ) +} diff --git a/test/integration/quickbooks/syncLog/getInvoiceNumbersWithRecordedFee.test.ts b/test/integration/quickbooks/syncLog/getInvoiceNumbersWithRecordedFee.test.ts new file mode 100644 index 00000000..a95af66e --- /dev/null +++ b/test/integration/quickbooks/syncLog/getInvoiceNumbersWithRecordedFee.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it, beforeEach } from 'vitest' + +import { EntityType, EventType, LogStatus } from '@/app/api/core/types/log' +import { db } from '@/db' +import { QBSyncLog } from '@/db/schema/qbSyncLogs' +import { getInvoiceNumbersWithRecordedFee } from '@/db/service/syncLog.service' +import { TEST_PORTAL_ID } from '@test/helpers/seed' +import { truncateAllTestTables } from '@test/helpers/testDb' + +const OTHER_PORTAL_ID = 'portal-other-0001' + +type LogSeed = { + invoiceNumber: string + portalId?: string + entityType?: EntityType + eventType?: EventType + status?: LogStatus + deletedAt?: Date | null +} + +const seedLog = (seed: LogSeed) => + db.insert(QBSyncLog).values({ + portalId: seed.portalId ?? TEST_PORTAL_ID, + copilotId: `pay_${seed.invoiceNumber}`, + entityType: seed.entityType ?? EntityType.PAYMENT, + eventType: seed.eventType ?? EventType.SUCCEEDED, + status: seed.status ?? LogStatus.SUCCESS, + invoiceNumber: seed.invoiceNumber, + deletedAt: seed.deletedAt ?? null, + }) + +describe('getInvoiceNumbersWithRecordedFee', () => { + beforeEach(async () => { + await truncateAllTestTables() + }) + + it('returns invoices that have a SUCCESS PAYMENT/SUCCEEDED log', async () => { + await seedLog({ invoiceNumber: 'INV-A' }) + await seedLog({ invoiceNumber: 'INV-B' }) + + const recorded = await getInvoiceNumbersWithRecordedFee(TEST_PORTAL_ID, [ + 'INV-A', + 'INV-B', + ]) + + expect(recorded).toEqual(new Set(['INV-A', 'INV-B'])) + }) + + it('only counts the recorded ones, ignoring the rest of the requested list', async () => { + await seedLog({ invoiceNumber: 'INV-A' }) + + const recorded = await getInvoiceNumbersWithRecordedFee(TEST_PORTAL_ID, [ + 'INV-A', + 'INV-B', + ]) + + expect(recorded).toEqual(new Set(['INV-A'])) + }) + + it('excludes non-SUCCESS, wrong entity/event, soft-deleted, and other-portal rows', async () => { + await seedLog({ invoiceNumber: 'INV-OK' }) + await seedLog({ invoiceNumber: 'INV-FAILED', status: LogStatus.FAILED }) + await seedLog({ + invoiceNumber: 'INV-WRONG-EVENT', + eventType: EventType.CREATED, + }) + await seedLog({ + invoiceNumber: 'INV-WRONG-ENTITY', + entityType: EntityType.INVOICE, + }) + await seedLog({ invoiceNumber: 'INV-DELETED', deletedAt: new Date() }) + await seedLog({ invoiceNumber: 'INV-OTHER', portalId: OTHER_PORTAL_ID }) + + const recorded = await getInvoiceNumbersWithRecordedFee(TEST_PORTAL_ID, [ + 'INV-OK', + 'INV-FAILED', + 'INV-WRONG-EVENT', + 'INV-WRONG-ENTITY', + 'INV-DELETED', + 'INV-OTHER', + ]) + + expect(recorded).toEqual(new Set(['INV-OK'])) + }) + + it('returns an empty set for empty input', async () => { + await seedLog({ invoiceNumber: 'INV-A' }) + + const recorded = await getInvoiceNumbersWithRecordedFee(TEST_PORTAL_ID, []) + + expect(recorded).toEqual(new Set()) + }) +}) diff --git a/test/unit/notification/notification.helper.test.ts b/test/unit/notification/notification.helper.test.ts index 1e41d68e..04f8b7f6 100644 --- a/test/unit/notification/notification.helper.test.ts +++ b/test/unit/notification/notification.helper.test.ts @@ -110,8 +110,31 @@ describe('getInProductNotificationDetail', () => { NotificationActions.QB_PAYOUT_MIXED_INTENT, ctx, ) - expect(detail.body).toContain('No deposit was created, so nothing') + expect(detail.body).toContain( + 'No deposit was created. The payments are already recorded', + ) expect(detail.body).not.toContain('for invoices') + expect(detail.body).not.toContain('already recorded as expenses') + }) + + it('warns which invoice fees are already recorded so they are not booked twice', () => { + const ctx: NotificationContext = { + entityType: 'payout', + eventType: 'settled', + entityKey: 'po_test_1', + invoiceNumbers: 'INV-A, INV-B', + invoiceNumbersWithFee: 'INV-A', + } + const detail = getInProductNotificationDetail( + NotificationActions.QB_PAYOUT_MIXED_INTENT, + ctx, + ) + expect(detail.body).toContain( + 'No deposit was created for invoices INV-A, INV-B', + ) + expect(detail.body).toContain( + 'The Stripe fees for INV-A are already recorded as expenses in QuickBooks, so do not record those fees again', + ) }) it('5010 (invoice-only after suppression) warns that the failure is final', () => { diff --git a/test/unit/quickbooks/syncErrorNotifier.test.ts b/test/unit/quickbooks/syncErrorNotifier.test.ts index 6ed6b000..bd74fef7 100644 --- a/test/unit/quickbooks/syncErrorNotifier.test.ts +++ b/test/unit/quickbooks/syncErrorNotifier.test.ts @@ -36,6 +36,16 @@ vi.mock('@/db/service/token.service', () => ({ getPortalConnection: () => getPortalConnectionMock(), })) +const getInvoiceNumbersWithRecordedFeeMock = vi + .fn() + .mockResolvedValue(new Set()) +vi.mock('@/db/service/syncLog.service', () => ({ + getInvoiceNumbersWithRecordedFee: ( + portalId: string, + invoiceNumbers: string[], + ) => getInvoiceNumbersWithRecordedFeeMock(portalId, invoiceNumbers), +})) + import { SyncErrorNotifier, getActionForErrorCode, @@ -189,6 +199,8 @@ describe('SyncErrorNotifier#notify', () => { beforeEach(() => { sendNotificationToIU.mockReset() + getInvoiceNumbersWithRecordedFeeMock.mockReset() + getInvoiceNumbersWithRecordedFeeMock.mockResolvedValue(new Set()) }) it('skips when status is not FAILED', async () => { @@ -264,6 +276,8 @@ describe('SyncErrorNotifier#notify', () => { entityKey: 'po_test_1', invoiceNumbers: 'INV-A, INV-B', }) + // No recorded-fee rows this run, so the warning field stays absent. + expect(ctx.invoiceNumbersWithFee).toBeUndefined() // Close the seam: the ctx extracted from `remark` must render the invoice // list in the real copy (both channels), with the payout id as the ref. @@ -275,6 +289,88 @@ describe('SyncErrorNotifier#notify', () => { } }) + it('flags the invoices whose fees are already recorded so IUs do not book them twice', async () => { + // Only INV-A has a recorded absorbed-fee expense; INV-B was deferred. + getInvoiceNumbersWithRecordedFeeMock.mockResolvedValueOnce( + new Set(['INV-A']), + ) + const notifier = new SyncErrorNotifier(user) + + await notifier.notify({ + ...baseLog, + entityType: 'payout' as never, + eventType: 'settled' as never, + errorCode: PAYOUT_MIXED_INTENT_CODE, + quickbooksId: null, + invoiceNumber: null, + copilotId: 'po_test_1', + remark: 'INV-A, INV-B', + errorMessage: + 'Payout po_test_1 mixes batched and non-batched invoices; unsupported', + }) + + expect(getInvoiceNumbersWithRecordedFeeMock).toHaveBeenCalledWith( + 'portal-1', + ['INV-A', 'INV-B'], + ) + const [, action, ctx] = sendNotificationToIU.mock.calls[0] + expect(ctx).toMatchObject({ + invoiceNumbers: 'INV-A, INV-B', + invoiceNumbersWithFee: 'INV-A', + }) + + const inProduct = getInProductNotificationDetail(action, ctx) + const email = getIEmailNotificationDetail(action, ctx) + for (const body of [inProduct.body, email.body]) { + expect(body).toContain( + 'The Stripe fees for INV-A are already recorded as expenses in QuickBooks, so do not record those fees again', + ) + } + }) + + it('lists recorded-fee invoices in remark order, not lookup order', async () => { + // Lookup returns them reversed; output must still follow the remark order. + getInvoiceNumbersWithRecordedFeeMock.mockResolvedValueOnce( + new Set(['INV-B', 'INV-A']), + ) + const notifier = new SyncErrorNotifier(user) + + await notifier.notify({ + ...baseLog, + entityType: 'payout' as never, + eventType: 'settled' as never, + errorCode: PAYOUT_MIXED_INTENT_CODE, + copilotId: 'po_test_1', + remark: 'INV-A, INV-B', + }) + + const [, , ctx] = sendNotificationToIU.mock.calls[0] + expect(ctx.invoiceNumbersWithFee).toBe('INV-A, INV-B') + }) + + it('still dispatches the mixed-payout notification when the recorded-fee lookup throws', async () => { + // A lookup blip must not swallow this terminal, never-retried notification. + getInvoiceNumbersWithRecordedFeeMock.mockRejectedValueOnce( + new Error('db blip'), + ) + const notifier = new SyncErrorNotifier(user) + + await notifier.notify({ + ...baseLog, + entityType: 'payout' as never, + eventType: 'settled' as never, + errorCode: PAYOUT_MIXED_INTENT_CODE, + copilotId: 'po_test_1', + remark: 'INV-A, INV-B', + }) + + expect(sendNotificationToIU).toHaveBeenCalledTimes(1) + const [, action, ctx] = sendNotificationToIU.mock.calls[0] + expect(action).toBe(NotificationActions.QB_PAYOUT_MIXED_INTENT) + expect(ctx).toMatchObject({ invoiceNumbers: 'INV-A, INV-B' }) + expect(ctx.invoiceNumbersWithFee).toBeUndefined() + }) + it('dispatches a notification for a FAILED row with a user-actionable code', async () => { const notifier = new SyncErrorNotifier(user)