OUT-3814: one item per product, driven by product.created

[SandipBajracharya]

What & why

Part of OUT-3787 (simplified product mapping). Previously each Assembly product price created a separate QB Item with auto-suffixed names ("Name (1)", "(2)"), cluttering the QB item list. This maps one Assembly product → one QB Item, with invoice lines carrying their own price.

Scope of this PR: backend + tests only. The mapping-UI changes ship in a follow-up PR (OUT-3816); the DB migration/index ships in OUT-3815.

Changes

• One item per product. Mapping lookup is product-keyed (getMappingByProductId, earliest-created canonical). getOne and getAllByProductId filter deletedAt.
• Item creation driven by product.created (webhookProductCreated) instead of price.created; the price.created event/handler is removed. Items are created at UnitPrice: 0.
• Invoice line UnitPrice comes from the line amount (lineItem.amount / 100); removed handleItemAmount and the copilotUnitPrice round-trip. Net: no invoice amount changes (lines already passed line-level price).
• Lazy invoice-time item creation aligned: no price fetch, one un-suffixed item per product, product-only upsert.
• handleProductMap upserts per product; ProductChangedItemReferenceSchema drops priceId/numericPrice.
• Resync (sync.service.processProductCreate) reconstructs from the product.

Tests

• price.created integration suite reworked into product.created (happy path, flag-off, idempotency, QB failure, soft-deleted-mapping).
• New: invoice line drives UnitPrice; lazy item creation makes one un-suffixed item; product/map product-keyed upsert.
• Full suite green (286), tsc clean, lint/prettier clean.

Testing Criteria

https://www.loom.com/share/c01657c34e08491ea67ff25e42f64756

Notes for review

• qb_product_sync price columns (price_id, unit_price, copilot_unit_price) are intentionally left in the schema; old rows keep their data, new rows leave them null. The partial unique index + one-time cleanup land in OUT-3815.
• Follow-up OUT-3816 carries the flatten DTO narrowing + getProductsForMapping price removal + the settings-table UI.

🤖 Generated with Claude Code

[linear-code]

OUT-3814

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
quickbooks-sync (dev)	Ready	Preview, Comment	Jun 5, 2026 3:03pm

[greptile-apps]

Greptile Summary

This PR migrates QuickBooks item creation from a price-keyed model (one QB item per Assembly product price) to a product-keyed model (one QB item per Assembly product), driven by product.created instead of price.created. Invoice line UnitPrice is now sourced from lineItem.amount / 100 rather than the stored mapping price, removing the handleItemAmount / copilotUnitPrice round-trip entirely.

• Webhook pivot: WebhookEvents.PRICE_CREATED replaced by PRODUCT_CREATED; webhookProductCreated no longer needs a Copilot API product fetch — name/description come directly from the webhook payload; QB item created at UnitPrice: 0.
• Mapping lookup: getMappingByProductId replaces getMappingByProductPriceId; getOne and getAllByProductId now filter deletedAt IS NULL; handleProductMap upserts per productId only.
• Sync/resync: processProductCreate fetches the product (not the price) for retries — backward-compatible because old FAILED logs already stored the product ID in copilotId.

Confidence Score: 4/5

Safe to merge for portals on the new product-keyed flow; existing portals with legacy multi-price mapping rows may have subtle inconsistencies until OUT-3815 cleans them up.

The behavioral changes are well-scoped, all test paths are covered, and the sync service retry path is backward-compatible. The three findings are migration-window quality concerns: the excluded-mapping short-circuit in the idempotency check, the divergent sort orders in getOne vs getMappingByProductId, and the non-atomic CREATED log upsert key.

src/app/api/quickbooks/product/product.service.ts — the idempotency check, sort-order divergence, and logSync key are all concentrated here.

Important Files Changed

Filename	Overview
src/app/api/quickbooks/product/product.service.ts	Core logic rewrite: replaces price-keyed mapping with product-keyed; adds deletedAt filters to getOne/getAllByProductId; logSync CREATED key now (copilotId, eventType). Minor ordering inconsistency between getOne (desc) and getMappingByProductId (asc).
src/app/api/quickbooks/invoice/invoice.service.ts	Removes handleItemAmount and copilotUnitPrice round-trip; lazy item creation now creates QB item at UnitPrice=0; invoice line UnitPrice/Amount driven by lineItem.amount/100.
src/app/api/quickbooks/webhook/webhook.service.ts	handlePriceCreated renamed to handleProductCreated; FAILED log payload updated from copilotPriceId/productPrice to copilotId/productName.
src/app/api/quickbooks/sync/sync.service.ts	processProductCreate now fetches by copilotId (product) instead of copilotPriceId (price); backward-compatible since old FAILED logs stored copilotId as the product ID.
src/type/dto/webhook.dto.ts	PriceCreatedResponseSchema removed; ProductCreatedResponseSchema added matching the existing ProductUpdatedResponseSchema contract.
src/db/schema/qbProductSync.ts	ProductChangedItemReferenceSchema drops numericPrice and priceId; price columns remain in the DB schema for old rows, cleaned up in OUT-3815.

Sequence Diagram

sequenceDiagram
    participant CW as Copilot Webhook
    participant WS as WebhookService
    participant PS as ProductService
    participant QB as QuickBooks API
    participant DB as qb_product_sync

    Note over CW,DB: product.created flow (new)
    CW->>WS: POST /webhook eventType product.created
    WS->>WS: parse ProductCreatedResponseSchema
    WS->>PS: webhookProductCreated(resource)
    PS->>DB: getAllByProductId(productId) filters deletedAt IS NULL
    alt already mapped
        PS-->>WS: return idempotent
    else not mapped
        PS->>QB: getAnItem(name)
        alt item exists in QB
            QB-->>PS: existing item
        else item not found
            PS->>QB: createItem UnitPrice 0
            QB-->>PS: new item
        end
        PS->>DB: createQBProduct productId qbItemId
        PS->>DB: logSync CREATED SUCCESS
        PS-->>WS: done
    end

    Note over CW,DB: invoice.created lazy item creation
    CW->>WS: POST /webhook eventType invoice.created
    WS->>PS: ensureProductExistsAndSyncToken(productId)
    PS->>DB: getMappingByProductId(productId) asc createdAt
    alt no mapping found
        PS->>QB: getAnItem or createItem UnitPrice 0
        QB-->>PS: item
        PS->>DB: updateOrCreateQBProduct
        PS->>DB: updateOrCreateQBSyncLog CREATED SUCCESS
    end
    Note right of WS: UnitPrice = lineItem.amount divided by 100

Loading

Comments Outside Diff (2)

1. src/app/api/quickbooks/product/product.service.ts, line 372-375 (link)

   Idempotency check includes excluded mappings with no QB item

   getAllByProductId returns any live (non-deleted) mapping row, including rows where isExcluded=true and qbItemId=null. If a product was previously excluded via the mapping UI, a subsequent product.created (e.g., triggered by processProductCreate resync) will see the excluded row and return early — permanently preventing a QB item from being created even if the exclusion is later lifted through a means other than handleProductMap. The comment "already mapped to a QB item" doesn't match the actual predicate; consider filtering on qbItemId IS NOT NULL or documenting that excluded products are intentionally skipped here.

2. src/app/api/quickbooks/product/product.service.ts, line 87-101 (link)

   getOne sort order diverges from getMappingByProductId

   getOne sorts desc(createdAt) (newest row first) while getMappingByProductId sorts asc(createdAt) (oldest row first). In practice, updateQBProduct applies its WHERE clause to all matching rows, so the row chosen by getOne only governs the INSERT-vs-UPDATE branch of updateOrCreateQBProduct — correctness is preserved today. However, portals that still carry multiple live rows from the old multi-price era (until OUT-3815 cleanup) will have handleProductMap and invoice lookup silently disagree about the "canonical" row.

   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!

_{Reviews (1): Last reviewed commit: "test(OUT-3814): cover one-item-per-produ..." | Re-trigger Greptile}

[SandipBajracharya]

Re: P2 — logSync CREATED upsert key not unique in the DB

Thanks — verified, with one correction to the mechanism:

updateOrCreateQBSyncLog doesn't use an unordered findFirst. It goes through SyncLogService.getOne, which orders by createdAt (default desc):

async getOne(conditions, orderByDirection: 'asc' | 'desc' = 'desc') {
  return this.db.query.QBSyncLog.findFirst({
    where: conditions,
    orderBy: [orderMap[orderByDirection](QBSyncLog.createdAt)],
  })
}

So when multiple PRODUCT/CREATED rows match (portalId, copilotId, eventType), it deterministically updates the most recent one — not a random row, so no row goes stale non-deterministically.

The residual point is fair: multiple PRODUCT/CREATED rows can coexist (legacy data, where old logs were keyed per copilotPriceId, or the webhookProductCreated ↔ lazy-creation race), and only the newest is reconciled. Impact is low, though — a new product produces exactly one CREATED log; legacy products don't re-fire product.created, so their existing rows aren't touched; and this is an audit/retry-log artifact, not QB-facing data.

One correction on the mitigation: OUT-3815 is the qb_product_sync index/cleanup and does not touch qb_sync_logs, so the CREATED-log dedup isn't actually covered there. The existing uq_qb_sync_logs_oneshot_active partial index covers invoice/payment one-shots, not PRODUCT/CREATED.

Treating this as non-blocking given the determinism above and the low impact. If we want DB-enforced dedup, I'll file a separate ticket for a partial unique index on qb_sync_logs (portal_id, copilot_id, event_type) WHERE entity_type = 'product' AND event_type = 'created'.

[priosshrsth]

@SandipBajracharya The PR looks good to me. I have added some comments and suggestions. But all of them are opinionated and minor. Let me know what you think about them

[priosshrsth]

lgtm