From 4efe59a8170feab296ac6f5365ee8c545168763c Mon Sep 17 00:00:00 2001 From: SandipBajracharya Date: Thu, 7 May 2026 11:54:18 +0545 Subject: [PATCH 1/2] revert(OUT-3700): remove find-or-map invoice logic from webhook handlers The find-or-map fallback (1ed7b57) lazily inserted qb_invoice_sync rows from inside live webhook handlers when QBO already had the invoice. Drop that path from webhookInvoiceCreated/Paid/Voided and from the QBO-present branch of handleInvoiceDeleted; restore the original behavior where missing local mapping throws an APIError so the webhook-level catch records a FAILED row and the resync cron retries once a CREATED resync establishes the mapping. Kept intentionally: - checkIfInvoiceExistsInQBO body unchanged; serves the syncMissedInvoices cron path where mapping-on-find is desirable. - findOrMapInvoiceFromQBO retained as a private helper (only caller is checkIfInvoiceExistsInQBO now); dropped its unused qbInvoice param. - handleInvoiceDeleted's QBO-absent branch (soft-delete prior sync logs + record pre-soft-deleted DELETED log) stays. - OUT-3644 onConflictDoNothing race guard and OUT-3655 PENDING-status checks on CREATED log lookups are independent and unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../api/quickbooks/invoice/invoice.service.ts | 106 ++++-------------- 1 file changed, 21 insertions(+), 85 deletions(-) diff --git a/src/app/api/quickbooks/invoice/invoice.service.ts b/src/app/api/quickbooks/invoice/invoice.service.ts index 00b3460..a1e7524 100644 --- a/src/app/api/quickbooks/invoice/invoice.service.ts +++ b/src/app/api/quickbooks/invoice/invoice.service.ts @@ -572,24 +572,6 @@ export class InvoiceService extends BaseService { } const intuitApiService = new IntuitAPI(qbTokenInfo) - - // Check if invoice already exists in QBO (e.g. manually created) and map it if so - const mappedFromQBO = await this.findOrMapInvoiceFromQBO({ - invoiceNumber: invoiceResource.number, - copilotInvoiceId: invoiceResource.id, - clientId: invoiceResource.clientId, - companyId: invoiceResource.companyId, - status: invoiceResource.status, - total: invoiceResource.total, - taxAmount: invoiceResource.taxAmount, - intuitApi: intuitApiService, - }) - if (mappedFromQBO) { - console.info( - 'InvoiceService#webhookInvoiceCreated | Invoice found in QBO and mapped. Skipping creation.', - ) - return - } const incomeAccRef = await this.handleIncomeAccountRef( qbTokenInfo, intuitApiService, @@ -864,7 +846,7 @@ export class InvoiceService extends BaseService { invoiceNumber: payload.data.number, }) // 1. check if the status of invoice is already paid in sync table - let invoiceSync = await this.getInvoiceByNumber(payload.data.number, [ + const invoiceSync = await this.getInvoiceByNumber(payload.data.number, [ 'id', 'qbInvoiceId', 'status', @@ -872,27 +854,12 @@ export class InvoiceService extends BaseService { ]) if (!invoiceSync) { - console.info( - 'InvoiceService#webhookInvoicePaid | Invoice not found in sync table. Attempting find-or-map from QBO...', + // Throw so the webhook-level catch writes a FAILED PAID log; the + // resync cron will retry once a CREATED resync establishes the mapping. + throw new APIError( + httpStatus.NOT_FOUND, + `Invoice not found in sync table for paid event. Invoice number: ${payload.data.number}. Likely preceded by a failed CREATE sync.`, ) - const intuitApi = new IntuitAPI(qbTokenInfo) - const mappedInvoice = await this.findOrMapInvoiceFromQBO({ - invoiceNumber: payload.data.number, - copilotInvoiceId: payload.data.id, - clientId: payload.data.clientId, - companyId: payload.data.companyId, - status: payload.data.status, - total: payload.data.total, - taxAmount: payload.data.taxAmount, - intuitApi, - }) - if (!mappedInvoice) { - throw new APIError( - httpStatus.NOT_FOUND, - `Invoice not found in sync table or QBO for paid event. Invoice number: ${payload.data.number}. Likely preceded by a failed CREATE sync.`, - ) - } - invoiceSync = mappedInvoice } // check if the entity invoice has successful event paid @@ -1010,7 +977,7 @@ export class InvoiceService extends BaseService { invoiceNumber: payload.number, }) // 1. check if the status of invoice is already paid in sync table - let invoiceSync = await this.getInvoiceByNumber(payload.number, [ + const invoiceSync = await this.getInvoiceByNumber(payload.number, [ 'id', 'qbInvoiceId', 'status', @@ -1019,26 +986,12 @@ export class InvoiceService extends BaseService { ]) if (!invoiceSync) { - console.info( - 'InvoiceService#webhookInvoiceVoided | Invoice not found in sync table. Attempting find-or-map from QBO...', + // Throw so the webhook-level catch writes a FAILED VOIDED log; the + // resync cron will retry once a CREATED resync establishes the mapping. + throw new APIError( + httpStatus.NOT_FOUND, + `Invoice not found in sync table for void event. Invoice number: ${payload.number}. Likely preceded by a failed CREATE sync.`, ) - const intuitApi = new IntuitAPI(qbTokenInfo) - const mappedInvoice = await this.findOrMapInvoiceFromQBO({ - invoiceNumber: payload.number, - copilotInvoiceId: payload.id, - clientId: payload.clientId, - companyId: payload.companyId, - status: InvoiceStatus.OPEN, - total: payload.total, - intuitApi, - }) - if (!mappedInvoice) { - throw new APIError( - httpStatus.NOT_FOUND, - `Invoice not found in sync table or QBO for void event. Invoice number: ${payload.number}. Likely preceded by a failed CREATE sync.`, - ) - } - invoiceSync = mappedInvoice } if (invoiceSync.status !== InvoiceStatus.OPEN) { @@ -1116,7 +1069,7 @@ export class InvoiceService extends BaseService { invoiceNumber: payload.number, }) - let syncedInvoice = await this.getInvoiceByNumber(payload.number, [ + const syncedInvoice = await this.getInvoiceByNumber(payload.number, [ 'id', 'qbInvoiceId', 'status', @@ -1172,28 +1125,14 @@ export class InvoiceService extends BaseService { return } - // QBO has the invoice. Ensure we have a local mapping before deleting. + // QBO has the invoice but we have no local mapping. Throw so the + // webhook-level catch writes a FAILED DELETED log; the resync cron will + // retry once a CREATED resync establishes the mapping. if (!syncedInvoice) { - console.info( - 'InvoiceService#handleInvoiceDeleted | Invoice in QBO but not in sync table. Mapping before delete.', + throw new APIError( + httpStatus.NOT_FOUND, + `Invoice not found in sync table for delete event. Invoice number: ${payload.number}. Likely preceded by a failed CREATE sync.`, ) - const mappedInvoice = await this.findOrMapInvoiceFromQBO({ - invoiceNumber: payload.number, - copilotInvoiceId: payload.id, - clientId: payload.clientId, - companyId: payload.companyId, - status: InvoiceStatus.VOID, - total: payload.total, - intuitApi, - qbInvoice, - }) - if (!mappedInvoice) { - throw new APIError( - httpStatus.INTERNAL_SERVER_ERROR, - `Failed to map QBO invoice for delete. Invoice number: ${payload.number}`, - ) - } - syncedInvoice = mappedInvoice } // Copilot doesn't allow to delete invoice that are not voided. So, just log an error about possible edge cases without returning an error @@ -1340,8 +1279,6 @@ export class InvoiceService extends BaseService { total?: number taxAmount?: number | null intuitApi: IntuitAPI - // Pre-fetched QBO invoice; when provided, skips the internal getInvoice lookup. - qbInvoice?: Awaited> }) { const { invoiceNumber, @@ -1354,9 +1291,8 @@ export class InvoiceService extends BaseService { intuitApi, } = params - // 1. Query QBO for the invoice by DocNumber (unless caller already fetched it) - const qbInvoice = - params.qbInvoice ?? (await intuitApi.getInvoice(invoiceNumber)) + // 1. Query QBO for the invoice by DocNumber + const qbInvoice = await intuitApi.getInvoice(invoiceNumber) if (!qbInvoice) { console.info( 'InvoiceService#findOrMapInvoiceFromQBO | Invoice not found in QBO', From 308fa23981aecff16d6bae1487a3cf6ca375c2dc Mon Sep 17 00:00:00 2001 From: SandipBajracharya Date: Thu, 7 May 2026 12:14:01 +0545 Subject: [PATCH 2/2] docs(OUT-3700): explain why findOrMapInvoiceFromQBO is webhook-unsafe Note the DocNumber-collision failure mode that motivates the revert and points future readers at the surviving syncMissedInvoices cron caller. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../api/quickbooks/invoice/invoice.service.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/app/api/quickbooks/invoice/invoice.service.ts b/src/app/api/quickbooks/invoice/invoice.service.ts index a1e7524..bfd073b 100644 --- a/src/app/api/quickbooks/invoice/invoice.service.ts +++ b/src/app/api/quickbooks/invoice/invoice.service.ts @@ -1270,6 +1270,24 @@ export class InvoiceService extends BaseService { return { exists: mapping !== null } } + /** + * Looks up an invoice in QBO by DocNumber and, if present, lazily writes + * the local `qb_invoice_sync` mapping plus a CREATED sync log so downstream + * code can treat the invoice as already synced. + * + * Deliberately NOT called from the invoice created/paid/voided/deleted + * webhook handlers. An operator can create an unrelated invoice directly + * in QBO that happens to share a DocNumber with a Copilot invoice; if this + * helper ran on the webhook path it would bind the Copilot invoice to that + * unrelated QBO record, producing an inaccurate mapping that corrupts + * every subsequent paid/voided/deleted event for the same number. The + * webhook handlers therefore throw on missing mapping and let the resync + * cron retry once a CREATED resync establishes the correct mapping. + * + * Sole caller is `checkIfInvoiceExistsInQBO`, used by the + * `syncMissedInvoices` cron — a deliberate batch reconciliation where + * mapping-on-find is the explicit goal. + */ async findOrMapInvoiceFromQBO(params: { invoiceNumber: string copilotInvoiceId: string