OUT-3909: integration tests for invoice.paid webhook event - #260
Conversation
Cover every reachable branch of the invoice.paid handler: happy path, idempotent re-delivery, invoice not synced, invoice without a linked customer, missing invoice.created log, PENDING invoice.created log, and QB createPayment failure. Add a reusable seedInvoiceCreatedLog helper and a TEST_QB_PAYMENT_ID constant wired into the shared Intuit mock's createPayment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds 7 integration tests for the
Confidence Score: 5/5Safe to merge — the production change is a straightforward error-message split with no behavioral impact, and all seven integration tests are correctly wired to the real route and a live DB. The only change to production code is separating one combined conditional into two sequential guards with distinct error strings. Every affected branch is now covered by a dedicated integration test that drives the real route handler and asserts DB state. No logic, schema, or data-flow changes are present. No files require special attention. The minor suggestion on happyPath.test.ts is about making an existing assertion more precise, not a correctness issue. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant W as WebhookService
participant C as claimWebhookEvent
participant I as InvoiceService
participant DB as Postgres
W->>C: claim(copilotId, PAID)
alt already claimed
C-->>W: "claimed=false"
W-->>W: return (idempotency)
else first delivery
C-->>W: "claimed=true"
W->>I: webhookInvoicePaid(payload)
I->>DB: getInvoiceByNumber()
alt invoice missing
I-->>W: throw NOT_FOUND
W->>DB: updateOrCreateQBSyncLog(FAILED)
else invoice found
I->>DB: getOneByCopilotIdAndEventType(PAID)
alt SUCCESS log exists
I-->>I: return (idempotency guard)
end
I->>DB: check customerId
alt customerId null
I-->>W: throw APIError
W->>DB: updateOrCreateQBSyncLog(FAILED)
else customerId ok
I->>DB: getOneByCopilotIdAndEventType(CREATED)
alt log missing
I-->>W: throw Invoice sync log not found
W->>DB: updateOrCreateQBSyncLog(FAILED)
else log PENDING
I-->>W: throw Invoice sync log still pending
W->>DB: updateOrCreateQBSyncLog(FAILED)
else log SUCCESS
I->>I: createPayment(intuitApi)
alt QB rejects
I-->>W: throw Error
W->>DB: updateOrCreateQBSyncLog(FAILED)
else QB accepts
I->>DB: updateOrCreateQBSyncLog(SUCCESS)
I->>DB: "update QBInvoiceSync status=PAID"
end
end
end
end
end
%%{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 WebhookService
participant C as claimWebhookEvent
participant I as InvoiceService
participant DB as Postgres
W->>C: claim(copilotId, PAID)
alt already claimed
C-->>W: "claimed=false"
W-->>W: return (idempotency)
else first delivery
C-->>W: "claimed=true"
W->>I: webhookInvoicePaid(payload)
I->>DB: getInvoiceByNumber()
alt invoice missing
I-->>W: throw NOT_FOUND
W->>DB: updateOrCreateQBSyncLog(FAILED)
else invoice found
I->>DB: getOneByCopilotIdAndEventType(PAID)
alt SUCCESS log exists
I-->>I: return (idempotency guard)
end
I->>DB: check customerId
alt customerId null
I-->>W: throw APIError
W->>DB: updateOrCreateQBSyncLog(FAILED)
else customerId ok
I->>DB: getOneByCopilotIdAndEventType(CREATED)
alt log missing
I-->>W: throw Invoice sync log not found
W->>DB: updateOrCreateQBSyncLog(FAILED)
else log PENDING
I-->>W: throw Invoice sync log still pending
W->>DB: updateOrCreateQBSyncLog(FAILED)
else log SUCCESS
I->>I: createPayment(intuitApi)
alt QB rejects
I-->>W: throw Error
W->>DB: updateOrCreateQBSyncLog(FAILED)
else QB accepts
I->>DB: updateOrCreateQBSyncLog(SUCCESS)
I->>DB: "update QBInvoiceSync status=PAID"
end
end
end
end
end
Reviews (2): Last reviewed commit: "test(OUT-3909): assert taxAmount flows t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| expect(paidLogs).toHaveLength(1) | ||
| expect(paidLogs[0]).toMatchObject({ | ||
| entityType: EntityType.INVOICE, | ||
| eventType: EventType.PAID, |
There was a problem hiding this comment.
Both createdLogMissing and createdLogPending drive different branches of the handler (!invoiceLog vs invoiceLog.status === LogStatus.PENDING), but the production code throws the same error for both: 'Invoice sync log not found or still pending'. As a result, createdLogMissing asserts .toContain('Invoice sync log not found') and createdLogPending asserts .toContain('still pending') — both of which always match that single string. If the handler were ever refactored to collapse the two branches or swap their error text, neither test would catch the regression. Consider asserting 'Invoice sync log not found or still pending' verbatim in both tests, or — better — lobby to split the production error messages so the two failure modes can be distinguished end-to-end.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| eventType: EventType.PAID, | ||
| status: LogStatus.FAILED, | ||
| }) | ||
| expect(paidLogs[0].errorMessage).toContain('still pending') |
There was a problem hiding this comment.
'Invoice sync log not found or still pending'), checking .toContain('still pending') doesn't distinguish this branch from the missing-log case. Asserting on the full message makes the intent explicit and will catch the regression if the production error text is ever changed.
| expect(paidLogs[0].errorMessage).toContain('still pending') | |
| expect(paidLogs[0].errorMessage).toContain( | |
| 'Invoice sync log not found or still pending', | |
| ) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| expect(paidLogs[0]).toMatchObject({ | ||
| portalId: TEST_PORTAL_ID, | ||
| entityType: EntityType.INVOICE, | ||
| eventType: EventType.PAID, | ||
| status: LogStatus.SUCCESS, | ||
| copilotId: TEST_COPILOT_INVOICE_ID, | ||
| quickbooksId: TEST_QB_PAYMENT_ID, | ||
| invoiceNumber: TEST_INVOICE_NUMBER, | ||
| amount: '60000.00', | ||
| }) |
There was a problem hiding this comment.
taxAmount to the toMatchObject call so a regression that silently drops or mis-scales the tax column is caught alongside the amount assertion.
| expect(paidLogs[0]).toMatchObject({ | |
| portalId: TEST_PORTAL_ID, | |
| entityType: EntityType.INVOICE, | |
| eventType: EventType.PAID, | |
| status: LogStatus.SUCCESS, | |
| copilotId: TEST_COPILOT_INVOICE_ID, | |
| quickbooksId: TEST_QB_PAYMENT_ID, | |
| invoiceNumber: TEST_INVOICE_NUMBER, | |
| amount: '60000.00', | |
| }) | |
| expect(paidLogs[0]).toMatchObject({ | |
| portalId: TEST_PORTAL_ID, | |
| entityType: EntityType.INVOICE, | |
| eventType: EventType.PAID, | |
| status: LogStatus.SUCCESS, | |
| copilotId: TEST_COPILOT_INVOICE_ID, | |
| quickbooksId: TEST_QB_PAYMENT_ID, | |
| invoiceNumber: TEST_INVOICE_NUMBER, | |
| amount: '60000.00', | |
| taxAmount: '0.00', | |
| }) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The created-log guard threw one shared message for two distinct failure modes (missing CREATED log vs PENDING CREATED log), so a FAILED PAID log couldn't tell the resync cron which case it hit. Split into 'Invoice sync log not found' and 'Invoice sync log still pending' so the two branches are distinguishable end-to-end, and tighten createdLogPending to assert the full message. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Seed a non-zero, non-round tax on the CREATED log and assert it on the PAID log so a dropped or mis-scaled tax column is caught alongside amount. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@greptileai re-review |
Condense the multi-line explanatory comments added in this branch to concise one/two-liners. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| console.error( | ||
| 'InvoiceService#webhookInvoicePaid | Invoice sync log still pending', | ||
| ) | ||
| throw Error('Invoice sync log still pending') |
| const [row] = await db | ||
| .insert(QBSyncLog) | ||
| .values({ ...baseInvoiceCreatedLog, ...overrides }) | ||
| .returning() |
There was a problem hiding this comment.
I hate drizzle for these reasons. 😠
2 participants
Summary
Adds integration tests for the
invoice.paidQuickBooks webhook event, covering every reachable branch of the handler. Mirrors the existingpaymentSucceededsuite conventions. Closes OUT-3909.The tests drive the real route via
next-test-api-route-handleragainst a testcontainers Postgres, asserting DB state (qb_sync_logs,qb_invoice_sync) and Intuit/Copilot mock calls.Branches covered
happyPathidempotencyinvoiceNotFoundmissingCustomerIdcreatedLogMissinginvoice.createdlog to read the amount from → FAILED logcreatedLogPendinginvoice.createdlog still PENDING → FAILED logqbCreatePaymentFailsHelpers
seedInvoiceCreatedLog— reusable seeder for the prerequisite CREATED log (amount in cents).TEST_QB_PAYMENT_ID— new constant, wired into the shared Intuit mock'screatePaymentso the happy-path assertion is compile-linked to the mock.Notes
quickbooks_idforINVOICE/PAIDstores the QBO Payment id (polymorphic column) — pinned inhappyPath.qb_invoice_sync.customerIdmakes an orphaned customer impossible, leaving only the narrow soft-delete path.Verification
invoicePaidsuite: 7/7 passlint:check(0 errors) +prettier:checkclean🤖 Generated with Claude Code