Skip to content

OUT-3816: collapse product mapping table to one row per product - #258

Merged
SandipBajracharya merged 8 commits into
OUT-3814from
OUT-3816
Jun 5, 2026
Merged

OUT-3816: collapse product mapping table to one row per product#258
SandipBajracharya merged 8 commits into
OUT-3814from
OUT-3816

Conversation

@SandipBajracharya

Copy link
Copy Markdown
Collaborator

What & why

Part of OUT-3787 (simplified product mapping). The mapping settings table previously rendered one row per price; this collapses it to one row per Assembly product and removes price from the table entirely (per product → one QB item; invoice lines carry their own price).

Stacked on #257 (OUT-3814). Base is OUT-3814; retarget to master once #257 merges.

Changes

  • Table is one row per product, matched by productId (dropped priceId predicates in useMapItem / useProductTableSetting / selectItem).
  • No prices in the table — removed the Assembly product price and the QB item price (subtitle + dropdown).
  • getProductsForMapping (was getProductsWithPrices) returns one row per product and no longer fetches Copilot prices; flatten DTO is name-only.
  • Dropped price validations from the mapping save: removed the unitPrice is required when isExcluded is false superRefine and numericPrice from QBItemSchema.
  • unitPrice is now always null through the UI mapping path and handleProductMap; removed dead price/numericPrice from QBItemDataType / formatQBItemForListing.

Regression review fixes (last commit)

  • webhookProductUpdated now sends sparse: true so a name/description update doesn't reset a mapped QB item's UnitPrice to 0 (mapped items store null unitPrice now).
  • handleProductMap calls unsetTransaction() before the initial-save early return (no closed-tx singleton leak).
  • Removed stale numericPrice from a test fixture; dropped the unused createdAt from the flatten DTO.

Verification

  • tsc clean · full suite 286 passing · lint/prettier clean.
  • Manual smoke test (dashboard: multi-price product shows one row; select/exclude persists after Save) still pending — needs a portal with a multi-price product.

🤖 Generated with Claude Code

@linear-code

linear-code Bot commented Jun 3, 2026

Copy link
Copy Markdown

OUT-3816

@vercel

vercel Bot commented Jun 3, 2026

Copy link
Copy Markdown

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

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

Request Review

@SandipBajracharya SandipBajracharya changed the title QuickBooks: collapse product mapping table to one row per product (OUT-3816) OUT-3816: collapse product mapping table to one row per product Jun 3, 2026
@greptile-apps

greptile-apps Bot commented Jun 3, 2026

Copy link
Copy Markdown

Greptile Summary

This PR collapses the QB product mapping table from one row per price to one row per Assembly product. Price fields are removed from the mapping table, the API, and the save flow — invoice lines carry their own UnitPrice going forward.

  • getProductsForMapping replaces getProductsWithPrices: no longer fetches Copilot prices, returns { id, name, description } only, eliminating the per-product price pagination loop.
  • Mapping upsert (handleProductMap) now keys on productId alone; unitPrice is always written as null; unsetTransaction() is called before the early-return on initial save to prevent a singleton transaction leak.
  • webhookProductUpdated gains sparse: true so a name/description update no longer resets a mapped item's UnitPrice to 0 in QuickBooks, and the prior comma-operator bug in mappedConditions (which silently dropped the qbItemId null-check) is replaced with a correct and() call.

Confidence Score: 5/5

Safe to merge once the pending smoke test with a multi-price product is complete; the regression fixes are correct and automated tests are clean.

The three regression fixes added in the last commit (and() for mappedConditions, unsetTransaction() before early return, sparse:true on the webhook update) are all correct and well-targeted. The core simplification — one row per product, no price in the mapping path — is consistent across the service, hooks, UI, DTO, and schema validation. No new defects are introduced; the orphaned DB columns (priceId, copilotUnitPrice) remain a cleanup item but don't cause runtime errors.

src/db/schema/qbProductSync.ts still declares priceId and copilotUnitPrice columns without a migration to drop them; a follow-up is warranted once the stacked PRs merge.

Important Files Changed

Filename Overview
src/app/api/quickbooks/product/product.service.ts Core service changes: drops price fetching, fixes mappedConditions comma-operator bug with and(), adds sparse:true to webhook update, adds unsetTransaction() before early return. Refactored logSync conditions are logically equivalent to before.
src/hook/useSettings.ts Removes price/priceId fields from ProductDataType, QBItemDataType, and all hook logic; simplifies selectItem deduplication to id-only; useMapItem return type narrowed to {name} only. Changes are consistent with the one-row-per-product model.
src/components/dashboard/settings/sections/product/ProductMappingTable.tsx Removes price subtitle display from Assembly and QB item columns; drops priceId prop from MapItemComponent; removes numericPrice/price from selectItem call. UI now shows name only, matching the simplified data model.
src/db/schema/qbProductSync.ts Removes numericPrice from QBItemSchema and the unitPrice superRefine validation. priceId and copilotUnitPrice columns remain in the Drizzle table definition without a companion migration (flagged in prior review).
src/type/dto/api.dto.ts ProductFlattenResponseSchema simplified to id/name/description — all price and interval fields removed, consistent with the new getProductsForMapping return shape.
src/app/api/quickbooks/product/product.controller.ts Simple rename: getProductsWithPrices → getProductsForMapping. No logic changes.
src/app/api/quickbooks/product/flatten/route.ts Route handler updated to export new getProductsForMapping controller. Trivial rename change only.
test/integration/quickbooks/productMap/productKeyed.test.ts Removes stale numericPrice: 1499 from the QB item fixture to align with the dropped field in QBItemSchema.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant FlattenRoute as /product/flatten
    participant MapRoute as /product/map
    participant ProductService
    participant CopilotAPI
    participant DB as DB (QBProductSync)
    participant QB as QuickBooks API

    Note over Browser,QB: Load mapping table (simplified flow)
    Browser->>FlattenRoute: GET /product/flatten
    FlattenRoute->>ProductService: getProductsForMapping()
    ProductService->>CopilotAPI: "getProducts(limit=1000)"
    CopilotAPI-->>ProductService: products[]
    ProductService-->>FlattenRoute: "{ id, name, description }[]"
    FlattenRoute-->>Browser: one row per product (no prices)

    Note over Browser,QB: Save mapping
    Browser->>MapRoute: POST /product/map
    MapRoute->>ProductService: handleProductMap(body)
    ProductService->>DB: "upsert WHERE productId (unitPrice=null always)"
    DB-->>ProductService: saved rows
    ProductService-->>MapRoute: updated mappings
    MapRoute-->>Browser: 200 OK

    Note over Browser,QB: Webhook: product name/description update
    QB->>ProductService: webhookProductUpdated(resource)
    ProductService->>DB: query mapped products (AND qbItemId NOT NULL, qbSyncToken NOT NULL)
    DB-->>ProductService: mapped rows
    ProductService->>QB: "itemFullUpdate({ sparse: true, Name, Description })"
    Note right of QB: sparse=true prevents UnitPrice reset to 0
    QB-->>ProductService: updated item
    ProductService->>DB: update qbSyncToken
Loading

Reviews (2): Last reviewed commit: "fix(OUT-3816): combine webhookProductUpd..." | Re-trigger Greptile

@SandipBajracharya

Copy link
Copy Markdown
Collaborator Author

@greptileai

SandipBajracharya and others added 5 commits June 3, 2026 15:49
- Show one row per Assembly product; match mappings by productId (drop priceId predicates in useMapItem / useProductTableSetting / selectItem).
- Remove all prices from the table (Assembly product price and QB item price subtitle/dropdown).
- getProductsForMapping (was getProductsWithPrices) returns one row per product and no longer fetches Copilot prices; flatten DTO is name-only.
- Drop priceId plumbing from MapItemComponent; controller + flatten route renamed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Remove the "unitPrice is required when isExcluded is false" superRefine from QBProductCreateArraySchema — items are created at price 0 now, so a mapped row legitimately has no unitPrice.
- Remove numericPrice from QBItemSchema (changedItemReference.qbItem); its only consumer was handleProductMap, which now stores unitPrice: null.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- selectItem and useProductTableSetting always set unitPrice: null — items are price-0 and price isn't tracked in the UI.
- Drop the now-dead price/numericPrice fields from QBItemDataType and formatQBItemForListing, and the numericPrice passed into selectItem.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- webhookProductUpdated now sends sparse:true so a name/description update no longer resets the QB item's UnitPrice to 0 (mapped items store null unitPrice now).
- handleProductMap calls unsetTransaction() before the initial-save early return, so the DB singleton isn't left on a closed tx.
- Drop the now-unused createdAt from the flatten DTO + getProductsForMapping.
- Remove stale numericPrice from the productKeyed test fixture; fix stale handleProductMap comment.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The mapped-products filter used the comma operator, so only the
qbSyncToken guard survived and the qbItemId guard was discarded — truly
unmapped rows (null qbItemId) could slip into the update loop. Use and()
to apply both guards. Also drop the stale "updates redis" comment.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread src/app/api/quickbooks/product/product.service.ts Outdated
Comment on lines +338 to +344
const formatted = (products?.data ?? []).map((product) => ({
id: product.id,
name: product.name,
description: convert(product.description),
}))

return { products: formatted }

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.

Suggested change
const formatted = (products?.data ?? []).map((product) => ({
id: product.id,
name: product.name,
description: convert(product.description),
}))
return { products: formatted }
reeturn (products?.data ?? []).map((product) => ({
id: product.id,
name: product.name,
description: convert(product.description),
}))

We are returning products here. No need to make it an object imo.

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.

Letting this be for now.

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.

Ok

Comment thread src/hook/useSettings.ts Outdated
Comment thread src/hook/useSettings.ts Outdated
Convert CopilotAPI._getProducts from positional (name, nextToken, limit)
optionals to a single options object and update all callers (product service +
backfill/sync cmd scripts). Add defensive optional chaining on products[index]
in the product mapping hook.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@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 and others added 2 commits June 5, 2026 20:43
… price columns

OUT-3815: add partial unique index uq_qb_product_sync_product_active on
(portal_id, product_id) WHERE deleted_at IS NULL.

OUT-3819: drop the vestigial price_id, unit_price, copilot_unit_price columns
and remove the legacy unitPrice read in webhookProductUpdated. Both schema
changes are folded into one migration (20260603102427). The one-time dedup SQL
is hand-run against prod before deploy (gitignored snippet).

Hardening in product.service.ts:
- guard the initial-save batch insert in handleProductMap with
  onConflictDoNothing scoped to the partial index, so a re-fired/concurrent
  save no-ops instead of 500-ing.
- wrap both db.transaction callbacks in try/finally so unsetTransaction()
  always restores the db singleton, even on a throw or early return.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The initial-save insert uses onConflictDoNothing, so RETURNING omitted rows
skipped on a re-fired/concurrent save and the client could receive []. Always
return getAll() instead; drop the now-unused returningFields param. Add a
regression test covering a repeated initial save.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@SandipBajracharya
SandipBajracharya merged commit b32470e into OUT-3814 Jun 5, 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