Skip to content

OUT-3539: extract findOrMapInvoiceFromQBO helper for invoice sync resilience - #216

Merged
SandipBajracharya merged 3 commits into
masterfrom
OUT-3539
Apr 24, 2026
Merged

OUT-3539: extract findOrMapInvoiceFromQBO helper for invoice sync resilience#216
SandipBajracharya merged 3 commits into
masterfrom
OUT-3539

Conversation

@SandipBajracharya

@SandipBajracharya SandipBajracharya commented Apr 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds findOrMapInvoiceFromQBO() helper that queries QBO by invoice number, resolves/creates the customer mapping, and inserts a qb_invoice_sync row when found
  • Integrates the helper into all 4 webhook handlers (created, paid, voided, deleted) as a fallback when no local mapping exists
  • Refactors checkIfInvoiceExistsInQBO() to delegate to the new helper, so existing callers (syncMissedInvoices, backfillTimedOutInvoices) automatically gain mapping creation

Problem

When an invoice is manually created in QBO, our app tries to create the same invoice and fails. Subsequent invoice.paid, invoice.void, and invoice.delete events also silently skip because no qb_invoice_sync mapping row exists in our database.

Test plan

  • Invoice exists in QBO but not in local DB → created webhook maps it and skips creation
  • paid / voided / deleted webhooks with unmapped invoice → maps from QBO and proceeds
  • Invoice already mapped locally → handlers continue as before (no extra QBO API call)
  • Invoice not in QBO at all → handlers silently return as before
  • Calling findOrMapInvoiceFromQBO twice for the same invoice does not create duplicates

🤖 Generated with Claude Code

@linear

linear Bot commented Apr 9, 2026

Copy link
Copy Markdown

@vercel

vercel Bot commented Apr 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
quickbooks-sync (dev) Ready Ready Preview, Comment Apr 23, 2026 0:40am
quickbooks-sync Ready Ready Preview, Comment Apr 23, 2026 0:40am

Request Review

@SandipBajracharya SandipBajracharya changed the title feat(OUT-3539): extract findOrMapInvoiceFromQBO helper for invoice sync resilience OUT-3539: extract findOrMapInvoiceFromQBO helper for invoice sync resilience Apr 16, 2026
SandipBajracharya and others added 2 commits April 23, 2026 18:11
…nc resilience

When an invoice is manually created in QBO, subsequent webhook events
(paid, void, delete) silently failed because no qb_invoice_sync mapping
existed. The new findOrMapInvoiceFromQBO helper queries QBO by invoice
number, resolves/creates the customer mapping, and inserts the
qb_invoice_sync row so downstream handlers can proceed normally.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Previously, if invoice.created failed to sync to QBO, a FAILED CREATED row
landed in qb_sync_logs but subsequent invoice.paid/voided/deleted webhooks
for the same invoice returned silently, so the re-sync cron never saw them.

- webhookInvoicePaid/Voided now throw APIError when the invoice is absent
  from both the sync table and QBO, so the webhook-level catch records a
  FAILED PAID/VOIDED row for re-sync.
- handleInvoiceDeleted now queries QBO up front and branches on presence.
  When QBO doesn't have the invoice (never synced or manually deleted
  there), soft-delete prior sync logs, mark the local qb_invoice_sync row
  as DELETED, and record a pre-soft-deleted SUCCESS DELETED for audit.
  When QBO has it, ensure the local mapping and proceed with the existing
  delete flow, reusing the prefetched QBO invoice via a new optional
  qbInvoice param on findOrMapInvoiceFromQBO.
- Add SyncLogService.softDeleteLogsByCopilotId helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Apr 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces findOrMapInvoiceFromQBO, a helper that queries QBO by invoice number, resolves the customer mapping, and creates a qb_invoice_sync row when a locally unmapped invoice is found. It is wired into all four webhook handlers (created, paid, voided, deleted) as a fallback path and refactors checkIfInvoiceExistsInQBO to delegate to it.

  • P1 — wrong payment amount: findOrMapInvoiceFromQBO stores amount: total.toFixed(2) (raw dollars) in the CREATED sync log, but webhookInvoicePaid reads invoiceAmount = Number(amount) / 100. Every payment created via the new fallback path will be sent to QBO at 1/100th of the correct value (e.g. $1.00 for a $100 invoice). The fix is (total * 100).toFixed(2), matching the convention at line 790.
  • P2 — no idempotency inside the helper: findOrMapInvoiceFromQBO calls createQBInvoice without first checking for an existing row; concurrent webhook deliveries or retries will hit a unique-constraint failure rather than a graceful short-circuit.

Confidence Score: 3/5

Do not merge until the ×100 amount bug is fixed — payments created via the fallback path will be 100× too small in QBO.

A P1 financial bug causes QBO payments to be created at 1/100th of the correct amount whenever the new findOrMapInvoiceFromQBO helper is the first to record the CREATED sync log. This directly corrupts payment records in QBO and must be fixed before merging.

src/app/api/quickbooks/invoice/invoice.service.ts — specifically the amount and taxAmount fields in the logSync call inside findOrMapInvoiceFromQBO, and the DELETED-event log amount in handleInvoiceDeleted.

Important Files Changed

Filename Overview
src/app/api/quickbooks/invoice/invoice.service.ts Extracts findOrMapInvoiceFromQBO helper and integrates it into all 4 webhook handlers; P1 bug: amounts stored without ×100 factor, causing payments sent to QBO to be 100× too small when the helper creates the CREATED sync log.
src/app/api/quickbooks/syncLog/syncLog.service.ts Adds softDeleteLogsByCopilotId method; correctly scoped to workspace and uses isNull(deletedAt) guard to prevent double-deletion. No issues found.

Sequence Diagram

sequenceDiagram
    participant W as Webhook Handler
    participant IS as InvoiceService
    participant DB as Local DB
    participant QBO as QuickBooks Online

    W->>IS: webhookInvoicePaid / Voided / Deleted / Created
    IS->>DB: getInvoiceByNumber()
    alt mapping found
        DB-->>IS: invoiceSync row
        IS->>IS: proceed with normal flow
    else no mapping
        DB-->>IS: null
        IS->>IS: findOrMapInvoiceFromQBO()
        IS->>QBO: getInvoice(invoiceNumber)
        alt invoice in QBO
            QBO-->>IS: qbInvoice {Id, SyncToken}
            IS->>DB: createQBInvoice (mapping row)
            IS->>DB: logSync CREATED — amount=total.toFixed(2) missing x100
            IS-->>W: invoiceSync = mappedInvoice
            W->>W: paid path: amount divided by 100 → payment 100x too small
        else invoice not in QBO
            QBO-->>IS: null
            IS-->>W: null → throw or return
        end
    end
Loading

Reviews (1): Last reviewed commit: "fix(OUT-3539): log subsequent invoice ev..." | Re-trigger Greptile

Comment thread src/app/api/quickbooks/invoice/invoice.service.ts
Comment thread src/app/api/quickbooks/invoice/invoice.service.ts
Comment thread src/app/api/quickbooks/invoice/invoice.service.ts
…log writes

- SyncService#processInvoiceVoided/Deleted: qb_sync_logs.amount is already
  in cents, so parseFloat(record.amount) / 100 produced a value 100x smaller
  than intended when rebuilding the invoice payload for retry. Drop the
  division.
- WebhookService#pushFailedInvoiceToSyncLog: use updateOrCreateQBSyncLog
  instead of createQBSyncLog so repeated failures for the same (copilotId,
  eventType) update the existing row instead of inserting duplicates.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
id: record.copilotId,
number: invNumber,
total: record.amount ? parseFloat(record.amount) / 100 : 0, // assuming amount is in cents
total: record.amount ? parseFloat(record.amount) : 0,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Won't this cause regression?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This shouldn’t cause any regression. The change is only in the invoice void/delete function, and we’re not actually using the total amount for syncing anyway—it’s just there for logging.

@priosshrsth priosshrsth left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@SandipBajracharya
SandipBajracharya merged commit 1c62f26 into master Apr 24, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants