Skip to content

test(OUT-3708): unit + integration tests for invoice.created webhook - #244

Merged
SandipBajracharya merged 25 commits into
masterfrom
OUT-3708
May 19, 2026
Merged

test(OUT-3708): unit + integration tests for invoice.created webhook#244
SandipBajracharya merged 25 commits into
masterfrom
OUT-3708

Conversation

@SandipBajracharya

Copy link
Copy Markdown
Collaborator

Summary

  • Adds 9 integration tests + 1 unit test pinning the production behavior of the invoice.created webhook handler.
  • Mirrors the existing priceCreated/ integration suite shape (testcontainers + module-mocked Copilot/Intuit).
  • Fixes a pre-existing flake in expiringSweep.test.ts surfaced by the higher integration-file count.

Linear: OUT-3708

What's covered

Integration (test/integration/quickbooks/invoiceCreated/):

  • happyPath — OPEN status, mapped product, new customer (asserts mapped-product path via Description from copilot.getProduct)
  • draftSkip — DRAFT gate sits before claim
  • idempotency — pre-existing CREATED log blocks second delivery
  • invoiceAlreadyExistsgetInvoiceByNumber short-circuits at the top of webhookInvoiceCreated, before customer resolution
  • qbCreateInvoiceFails — claim row flipped to FAILED with errorMessage
  • statusPaidCreatesPayment — PAID branch creates Payment with LinkedTxn linked to the just-created invoice
  • oneOffLineItem — line item without productId/priceId routes through the Assembly Service ref
  • createNewProductFlagOff — flag-off + unmapped line item falls back to one-off (no qb_product_sync row)
  • useCompanyNameFlag — company-only payload + flag-on creates customer with customerType='company'

Unit (test/unit/api/quickbooks/webhook/handleInvoiceCreated.test.ts):

  • 5 orchestration branches: parse-failure / DRAFT skip / already-claimed / happy path / service-throws → FAILED log

Side fix

expiringSweep.test.ts was using a per-file vi.mock('@/utils/intuit', …) factory. Under integration's pool:forks + fileParallelism:false + isolate:false config, that mock didn't survive across files once any earlier test transitively loaded the real @/utils/intuit (every webhook test does, via auth.service.ts). With master's 7 integration files this never landed in the bad slot; with this branch's 16, it landed ~50% of the time.

Fix: moved the module mock to test/integration/setup.ts and pinned the singleton on globalThis (Vitest re-evaluates the setupFiles factory more than once per session under these flags — a naive shared mock still produces multiple vi.fn() instances). Verified with 10 consecutive full-suite runs all green. Full rationale in commit 2664c98 and in the inline comment in setup.ts.

Out of scope (separate tickets)

  • INVOICE_UPDATED routing (uses the same handler with delayMs)
  • absorbedFeeFlag (gates PAYMENT_SUCCEEDED only, not invoice.created)
  • Tax math edge cases, fee-paid-by-client line item, sparse-customer-update, excluded-product

Test plan

  • CI green (test.yml + lint.yml)
  • Local yarn test — 22 files / 101 tests passing (verified 10/10 consecutive runs locally)
  • yarn lint:check — clean
  • yarn prettier:check — clean
  • priceCreated regression — 6/6 still green
  • Reviewer to spot-check the globalThis mock pin in test/integration/setup.ts is intelligible

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented May 8, 2026

Copy link
Copy Markdown

OUT-3708

@vercel

vercel Bot commented May 8, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
quickbooks-sync Ready Ready Preview, Comment May 19, 2026 8:23am
quickbooks-sync (dev) Ready Ready Preview, Comment May 19, 2026 8:23am

Request Review

Comment thread test/helpers/invoiceCreatedTestSetup.ts
SandipBajracharya and others added 21 commits May 19, 2026 10:53
Canary test verifying the shared infra (seeders, fixture, mocks, setup
helper) wires up correctly. Asserts QB customer + invoice creation,
mapping rows persisted, and a SUCCESS sync log written. Mock defaults
required no adjustment — flow is greener than expected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The per-file vi.mock factory in expiringSweep.test.ts didn't survive
Vitest's shared module registry under pool:forks + fileParallelism:false +
isolate:false. Once any earlier test transitively loaded @/utils/intuit
(via auth.service.ts), the per-file mock no longer applied.

Moves the module mock to test/integration/setup.ts so it's installed
before any test-file imports run, matching the existing pattern for
CopilotAPI/IntuitAPI/Sentry. expiringSweep.test.ts wires its per-test
behavior via vi.mocked(Intuit.getInstance) inside beforeEach.

The mock singleton is pinned on globalThis. Vitest's setupFiles get
evaluated more than once per run when separate test files spin up
fresh module-graph contexts under this config — a naive
`vi.mock(..., () => ({ default: { getInstance: vi.fn() } }))` produces
a different vi.fn() per factory invocation, so the test-file wiring
ends up on a different mock than the one tokenRefresh.ts closed over.
Storing the singleton on globalThis (single process, since
fileParallelism is false) makes every factory invocation return the
same getInstance mock, so beforeEach wiring is what the runtime sees.

Verified: 10 consecutive full integration-suite runs pass.

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

- happyPath: default getAnItem(undefined, id, true) mock returned undefined,
  collapsing the mapped-product path into the Assembly Service fallback.
  Both branches happened to yield Id '999' via createItem default. The
  ItemRef assertion was passing via the wrong path. Override getAnItem to
  return a real item when queried by id and pin the mapped-product branch
  via two new assertions: createItem is invoked exactly once for
  'Assembly Service' (handleServiceItem still runs because seedHealthyPortal
  does not set serviceItemRef) and the line Description comes from
  copilot.getProduct, which only happens on the mapped path.

- invoiceAlreadyExists: corrected the comment claiming the existence
  check runs after customer resolution. It runs first, before any service
  call. Added an explicit qb_customers count = 0 assertion to make the
  invariant load-bearing.

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

- Integration test for the QBO DocNumber collision flow: pre-check detects
  the duplicate, and a 6240 race after pre-check triggers a single re-walk.
- Unit test for resolveAvailableDocNumber covering base, suffix walk, and
  exhaustion (Sentry capture + rethrow).
- Default findInvoicesByDocNumberPrefix in the shared IntuitAPI mock so
  existing invoice.created tests no longer fall through undefined.
- Rewrite describe/it text across all invoice.created tests in plain
  English and strip ticket/internal-symbol references from comments.

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

greptile-apps Bot commented May 19, 2026

Copy link
Copy Markdown

Greptile Summary

Adds comprehensive test coverage (9 integration + 2 unit files) for the invoice.created webhook handler, mirroring the existing priceCreated suite shape. Also fixes a pre-existing intermittent failure in expiringSweep.test.ts by moving the @/utils/intuit module mock to the shared test/integration/setup.ts and pinning its singleton on globalThis to survive multiple factory evaluations under the shared-module-registry Vitest configuration.

  • Integration tests cover the full happy path plus 8 edge-case branches: DRAFT skip, idempotency guard, invoice-already-exists short-circuit, QB API failure → FAILED log, PAID branch creates Payment with LinkedTxn, one-off line items, createNewProductFlag: false fallback, and company-name customer type.
  • Unit tests pin the orchestration layer of WebhookService.handleWebhookEvent (parse failure, DRAFT skip, claim-already-taken, happy path, service-throws → FAILED) and InvoiceService#resolveAvailableDocNumber (clean slot, suffix walk, exhaustion → Sentry + rethrow).
  • Flake fix consolidates the @/utils/intuit mock into setup.ts with a globalThis guard so a single vi.fn() instance is shared across all integration files regardless of how many times Vitest re-evaluates the setup factory.

Confidence Score: 5/5

Safe to merge — all changes are test code and configuration; no production logic is modified.

The PR adds tests and fixes a flake; no production paths are touched. The globalThis singleton for the @/utils/intuit mock is unconventional but well-documented and addresses a real shared-module-registry constraint. All test logic is structurally sound: Once-queue mocks are correctly scoped inside optsFactory, the optsFactory pattern ensures fresh vi.fn() instances per beforeEach, and the idempotency/invoiceAlreadyExists/happyPath branches each target distinct code paths without overlapping DB state.

No files require special attention.

Important Files Changed

Filename Overview
test/integration/setup.ts Adds globalThis-singleton pattern for @/utils/intuit mock to prevent flake under shared module registry; well-documented and correct.
test/helpers/mocks.ts Extends createMockCopilotAPI and createMockIntuitAPI with sensible invoice.created defaults; no issues.
test/helpers/seed.ts Adds seedQBCustomer and seedQBInvoiceSync helpers with properly-typed base objects and override patterns consistent with existing seeders.
test/helpers/invoiceCreatedTestSetup.ts setupInvoiceCreatedTest helper correctly mirrors setupPriceCreatedTest; optsFactory pattern ensures fresh vi.fn() instances per beforeEach.
test/integration/quickbooks/invoiceCreated/qboDocNumberCollision.test.ts Both describe blocks correctly handle Once-queue drain: outer block uses mockResolvedValue (permanent), inner block builds mocks inside optsFactory for fresh queues each beforeEach.
test/integration/quickbooks/invoiceCreated/happyPath.test.ts Thorough happy-path assertions covering customer creation, invoice DB row, sync log, DocNumber collision pre-flight, and mapped-product Description sourcing from Copilot.
test/integration/quickbooks/refreshTokens/expiringSweep.test.ts Flake fixed by removing per-file vi.mock and wiring via vi.mocked(Intuit.getInstance) in beforeEach after clearAllMocks; correct approach for shared module registry.
test/integration/quickbooks/invoiceCreated/statusPaidCreatesPayment.test.ts Correctly asserts Payment creation with LinkedTxn; uses string literal 'paid' instead of InvoiceStatus.PAID enum (minor style inconsistency, not a bug).
test/unit/api/quickbooks/webhook/handleInvoiceCreated.test.ts Five orchestration branches correctly isolated with vi.fn() mocks hoisted before imports; covers all major control-flow paths.
test/unit/app/api/quickbooks/invoice/invoice.service.docNumber.test.ts Tests private resolveAvailableDocNumber via type cast with appropriate justification; covers clean slot, suffix walk, and exhaustion-to-Sentry paths.
eslint.config.mjs Adds no-restricted-imports rule preventing src/ from importing test helpers; good guardrail to enforce test/source boundary.
tsconfig.json Excludes test/ from root tsconfig; test/ gets its own test/tsconfig.json that extends root, keeping type-check scopes separate.

Reviews (2): Last reviewed commit: "test(OUT-3708): rebuild DocNumber-race m..." | Re-trigger Greptile

Comment thread test/integration/quickbooks/invoiceCreated/qboDocNumberCollision.test.ts Outdated
…mports

- Exclude test/ from the root tsconfig so the IDE stops suggesting test
  helpers when editing source files.
- Add test/tsconfig.json that extends root and re-includes test/** so test
  files still get full type-checking and the @/* + @test/* path aliases.
- Add a src/**-scoped ESLint no-restricted-imports rule that errors on any
  @test/* or relative ../test/** import from source code.

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

Move the findInvoicesByDocNumberPrefix and createInvoice mocks for the
6240-race block inside optsFactory so each beforeEach gets fresh vi.fn()s
with fresh Once-queues. clearAllMocks does not refill consumed Once
queues, so a watch-mode re-run or a second it in the block would
otherwise see undefined returns.

Also strip restatement-style comments that just narrate the next
assertion.

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

Copy link
Copy Markdown
Collaborator Author

@greptileai

@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.

This pr is a very good start. Well done.

Comment thread test/fixtures/invoiceCreated.webhook.json Outdated
…safety

- Replace test/fixtures/{invoiceCreated,priceCreated}.webhook.json with
  TypeScript fixtures typed against z.input<typeof Schema> so DTO drift
  surfaces as a compile-time error.
- Drop wire-only extras the schemas don't define; reference InvoiceStatus
  enum directly.
- Update 16 test files to import from the new .ts path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@SandipBajracharya
SandipBajracharya merged commit 33cec2f into master May 19, 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