Skip to content

OUT-3951: default sync-log entityType when paying without a prior created log - #69

Merged
SandipBajracharya merged 1 commit into
mainfrom
OUT-3951
Jul 10, 2026
Merged

OUT-3951: default sync-log entityType when paying without a prior created log#69
SandipBajracharya merged 1 commit into
mainfrom
OUT-3951

Conversation

@SandipBajracharya

Copy link
Copy Markdown
Collaborator

What

Fixes a latent defect in syncPaidInvoiceToXero (invoice.paid flow) that can duplicate a payment in Xero. Follow-up to the merged PR #67; ref OUT-3951.

The bug

The success-path sync log was built by spreading prevSyncLog (from getInvoiceCreatedSyncLog) without hardcoding the NOT NULL entityType column:

await syncLogsService.createSyncLog({
  ...prevSyncLog,          // undefined when no invoice.created log exists
  eventType: SyncEventType.PAID,
  status: SyncStatus.SUCCESS,
  syncDate: new Date(),
})

When no prior invoice.created log exists, prevSyncLog is undefined, so entityType is missing and the INSERT fails inside the transaction. The rollback discards the synced_invoices status update and the synced_payments write — but markInvoicePaid already ran outside the transaction, so the payment is real in Xero. The handleEvent catch then records a failed_syncs retry row. On retry, getPaymentForInvoiceId finds no local record (rolled back), so markInvoicePaid runs again → duplicate payment in Xero.

The catch block already hardcodes entityType; the success path did not.

The fix

One line: add entityType: SyncEntityType.INVOICE to the success-log payload, matching the catch block. It's a no-op when prevSyncLog is present (getInvoiceCreatedSyncLog always filters entityType = INVOICE).

Test

invoicePaid/noPriorCreatedLog.test.ts — seeds a synced invoice with no prior created log, posts the paid webhook, and asserts 200, a committed synced_payments row, a PAID success log with entityType, and empty failed_syncs. Verified failing (500) before the fix.

Verification

  • pnpm test run test/integration/webhook/invoicePaid/ → 7 files / 8 tests pass
  • pnpm typecheck and pnpm lint clean

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Jul 9, 2026

Copy link
Copy Markdown

OUT-3951

@vercel

vercel Bot commented Jul 9, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
xero-integration Ready Ready Preview, Comment Jul 9, 2026 11:29am

Request Review

@supabase

supabase Bot commented Jul 9, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project pkdwtcdqcefmlgxmcwmc because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

…r created log

syncPaidInvoiceToXero built its success-log payload by spreading prevSyncLog
without setting the NOT-NULL entityType. When no invoice.created log exists the
log INSERT fails inside the transaction, rolling back the synced_invoices status
update and the synced_payments write — but markInvoicePaid already ran outside
the transaction. The resulting failed_syncs retry re-pays because the local
payment record was rolled back, so getPaymentForInvoiceId finds nothing and
markInvoicePaid runs again, duplicating the payment in Xero. Hardcode entityType
like the catch block already does, and cover it with a regression test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@SandipBajracharya SandipBajracharya changed the title fix(OUT-3951): default sync-log entityType when paying without a prior created log OUT-3951: default sync-log entityType when paying without a prior created log Jul 9, 2026
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown

Greptile Summary

Fixes a latent duplicate-payment defect in syncPaidInvoiceToXero: the success-path createSyncLog call was missing entityType: SyncEntityType.INVOICE, which would cause a NOT NULL constraint violation (and rolled-back transaction) when no prior invoice.created log existed — leaving markInvoicePaid already executed in Xero but no local payment record, triggering a re-pay on retry. The fix is a single-line addition matching the existing catch-block pattern, accompanied by a full integration test suite for the invoice.paid webhook path.

  • Core fix (SyncedInvoices.service.ts): adds entityType: SyncEntityType.INVOICE to the success-path createSyncLog payload in syncPaidInvoiceToXero.
  • Regression test (noPriorCreatedLog.test.ts): seeds an invoice with no prior created log and asserts a committed payment row, correct entityType, and empty failed_syncs.
  • New test suite (6 files): covers happy path (US/AU), idempotency, invoice not found, missing Xero invoice recovery, sync disabled, and Xero failure.

Confidence Score: 4/5

Safe to merge — the one-line fix is correct, well-tested, and resolves a real data-integrity defect without touching any other code paths.

The voidInvoice and deleteInvoice methods in the same file carry the identical pattern (missing entityType in both their success-path createSyncLog calls and their catch-block failedSyncLogPayload), meaning those flows can still hit the same NOT NULL failure when no prior created log exists. This PR does not introduce that gap, but it also does not close it.

src/features/invoice-sync/lib/SyncedInvoices.service.ts — the voidInvoice and deleteInvoice methods are unchanged and retain the same missing-entityType pattern in both success and error paths.

Important Files Changed

Filename Overview
src/features/invoice-sync/lib/SyncedInvoices.service.ts Adds entityType: SyncEntityType.INVOICE to the success-path createSyncLog call in syncPaidInvoiceToXero, directly fixing the duplicate-payment bug. The sibling voidInvoice and deleteInvoice methods have the same missing entityType pattern and are unchanged.
test/integration/webhook/invoicePaid/noPriorCreatedLog.test.ts New regression test covering the exact bug scenario: no prior created log, asserts 200, committed synced_payments row, PAID SUCCESS log with entityType=INVOICE, and empty failed_syncs.
test/integration/webhook/invoicePaid/happyPath.test.ts New parameterized integration test covering US and AU regions for the happy path.
test/integration/webhook/invoicePaid/xeroMarkPaidFails.test.ts New test verifying Xero API failure results in a FAILED sync log, failed_syncs retry row, and 500 response.
test/helpers/seed.ts Adds seedSyncedPayment and seedSyncLog helper functions for the new invoice.paid integration tests.
test/helpers/mocks.ts Extends the default XeroAPI mock with getInvoiceById and markInvoicePaid stubs.
test/fixtures/paidInvoice.webhook.ts New test fixture factory for invoice.paid webhook payloads.
test/helpers/constants.ts Adds TEST_XERO_INVOICE.total and TEST_XERO_PAYMENT constants for the invoice.paid test suite.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant W as Webhook Handler
    participant S as SyncedInvoicesService
    participant X as Xero API
    participant DB as Database

    W->>S: syncPaidInvoiceToXero(invoiceId)
    S->>DB: getInvoiceCreatedSyncLog() - prevSyncLog may be undefined
    S->>DB: getPaymentForInvoiceId() - idempotency check
    S->>X: markInvoicePaid() - outside transaction
    Note over X: Payment created in Xero (irreversible)
    S->>DB: BEGIN TRANSACTION
    DB->>DB: "update syncedInvoices status=success"
    DB->>DB: insert syncedPayments
    DB->>DB: insert syncLogs with entityType hardcoded to INVOICE
    Note over DB: Fix ensures entityType is always set
    S->>DB: COMMIT
    S-->>W: payment result
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant W as Webhook Handler
    participant S as SyncedInvoicesService
    participant X as Xero API
    participant DB as Database

    W->>S: syncPaidInvoiceToXero(invoiceId)
    S->>DB: getInvoiceCreatedSyncLog() - prevSyncLog may be undefined
    S->>DB: getPaymentForInvoiceId() - idempotency check
    S->>X: markInvoicePaid() - outside transaction
    Note over X: Payment created in Xero (irreversible)
    S->>DB: BEGIN TRANSACTION
    DB->>DB: "update syncedInvoices status=success"
    DB->>DB: insert syncedPayments
    DB->>DB: insert syncLogs with entityType hardcoded to INVOICE
    Note over DB: Fix ensures entityType is always set
    S->>DB: COMMIT
    S-->>W: payment result
Loading

Comments Outside Diff (1)

  1. src/features/invoice-sync/lib/SyncedInvoices.service.ts, line 357-362 (link)

    P2 Same missing entityType in voidInvoice and deleteInvoice

    Both voidInvoice (line 357) and deleteInvoice (line 406) spread prevSyncLog without hardcoding entityType in their success-path createSyncLog calls — identical to the bug fixed here for paid. When no prior invoice.created log exists, prevSyncLog is undefined, so the spread contributes nothing, and the NOT NULL entityType column is absent from the INSERT. The VOIDED/DELETED Xero action already ran outside the transaction, so the failure leaves state inconsistent. Notably, the catch block's failedSyncLogPayload for both methods also omits entityType, meaning the error-path log write would fail too, swallowing the error record entirely.

Reviews (1): Last reviewed commit: "fix(OUT-3951): default sync-log entityTy..." | Re-trigger Greptile

@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 94caf2d into main Jul 10, 2026
6 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