OUT-3868 | OUT-3870: Migrate synced_items to one item per product (drop priceId) - #59
Conversation
Move from one Xero Item per price to one per product: drop the price_id column and replace the (portal_id, product_id, price_id) unique index with (portal_id, tenant_id, product_id). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the sync/service layer from price-level to product-level now that synced_items is keyed on (portal_id, tenant_id, product_id): - Drop priceId from the Mappable type. - Rename getSyncedItemsMapByPriceIds -> getSyncedItemsMapByProductIds and key the map on productId. - Rename getPriceIdToXeroItem -> getProductIdToXeroItem; resolve invoice line items by productId (no longer requires a priceId). - Rename getCopilotProductAndPrice -> getCopilotProductAndXeroItem, drop the now-meaningless per-price fetch and the productPrice sync-log field on product map/unmap/update events. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collapse the product-mapping experience to one row per product to match the
product-level synced_items model:
- ProductMapping type -> { product, item } (drop price and item amount).
- getProductMappings iterates products instead of prices and keys synced
items by productId.
- updateMappedItems and updateSyncedItemsAction operate on { productId, itemId }.
- Mapping table renders one row per product; remove the per-price amount line
under the product name and the amount under the mapped Xero item.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Greptile SummaryThis PR migrates
Confidence Score: 4/5Safe to merge once the manual deduplication DELETE has run against production data; the three deferred issues in SyncedItems.service.ts are acknowledged and tracked but are not introduced by this PR. The schema, service, and UI changes are mutually consistent and the four-case update logic in updateMappedItems handles all mapping transitions correctly. The known open items — createSyncedItemsForPrices lacking an existence guard, and the return-vs-continue bugs in addSyncedItems/deleteSyncedItems — pre-date or are explicitly deferred from this PR, but they do represent real defects on paths this PR touches. src/features/items-sync/lib/SyncedItems.service.ts — the three deferred issues all live here and will need resolution before the price.created webhook path is fully safe under the new unique constraint. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as ProductMappingTable
participant Action as updateSyncedItemsAction
participant PMS as ProductMappingsService
participant SIS as SyncedItemsService
participant DB as synced_items (DB)
UI->>Action: updateSyncedItemsAction(token, productMappings)
Action->>PMS: "updateMappedItems([{productId, itemId}])"
PMS->>DB: "SELECT * WHERE portalId + tenantId"
DB-->>PMS: existing rows (keyed by productId)
PMS->>PMS: diff → deletedMappings / addedMappings
PMS->>SIS: deleteSyncedItems(deletedMappings)
SIS->>DB: DELETE WHERE productId + itemId
PMS->>SIS: addSyncedItems(addedMappings)
SIS->>DB: "INSERT {portalId, tenantId, productId, itemId}"
PMS->>PMS: getProductMappings()
PMS-->>Action: updated ProductMapping[]
Action-->>UI: ProductMapping[]
Reviews (2): Last reviewed commit: "feat(OUT-3868): product-level mapping UI..." | Re-trigger Greptile |
priosshrsth
left a comment
There was a problem hiding this comment.
@SandipBajracharya Solid pr. I have added some comments. But they are minor nitpicks only. I think one mapping per product makes more sense than the flow before.
| <tr key={item.product.id} className="transition-colors"> | ||
| {/* Assembly Products Column */} | ||
| <td className="py-2 pr-3 pl-4" id={`price-id-${item.price.id}`}> | ||
| <td className="py-2 pr-3 pl-4" id={`product-id-${item.product.id}`}> |
There was a problem hiding this comment.
Any reason for adding id attribute?
There was a problem hiding this comment.
Those are just a placeholder ids for easy debugging.
| item: item | ||
| ? { | ||
| itemID: item.itemID, | ||
| code: item.code, | ||
| name: item.name, | ||
| } | ||
| : null, | ||
| } |
There was a problem hiding this comment.
| item: item | |
| ? { | |
| itemID: item.itemID, | |
| code: item.code, | |
| name: item.name, | |
| } | |
| : null, | |
| } | |
| item: item || null, | |
| } |
Would this work?
There was a problem hiding this comment.
It works. We are basically returning the necessary columns to UI. I think I will let it be.
There was a problem hiding this comment.
@SandipBajracharya It's about code simplicity too. I think. When we add conditions that is not necessary and handle things that don't need handling, we add additional divergent point to things. Or at least that is my opinion.
- Remove the commented-out "create missing items on invoice" scaffolding from getProductIdToXeroItem. - Simplify the synced-item lookup in getProductMappings to guard on itemId directly instead of the empty-string fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t.created Auto-create the Xero item on product.created instead of price.created: - Replace PriceCreatedEvent/PriceCreatedWebhook schemas with ProductCreated equivalents (shared ProductEventSchema with product.updated) and swap the discriminated-union member. - ValidWebhookEvent keeps PriceCreated (marked legacy) for historical failed_syncs rows, but it is no longer in the WebhookEvent union or routed. - handlePriceCreated -> handleProductCreated; logs and returns early when the product is already mapped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Replace createSyncedItemsForPrices with createSyncedItemsForProducts: skip products that are already mapped, and create the Xero item with no salesDetails.unitPrice (invoice lines always supply the price). - createItems now takes a code -> productId map and uses onConflictDoNothing as race safety against the (portalId, tenantId, productId) unique index. - addSyncedItems/deleteSyncedItems: skip (continue) items missing an itemId instead of aborting the whole batch; correct the loop comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y price.created - failed_syncs_type enum: add product.created (additive migration), keep price.created so historical rows stay valid. - On retry, resolve legacy price.created records as product.created: look up the product by the payload's productId, dispatch product.created, and drop the legacy row (after the fetch, so transient failures keep it for retry). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two concurrent product.created events for the same product could each create a Xero item; onConflictDoNothing kept only one DB mapping, leaving the other Xero item orphaned and referenced by a stale sync log. Use returning() to detect which insert won, delete the losing request's Xero item, and skip its sync log. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Delete the legacy row only after a successful product.created dispatch (like every other event), so a failure in handleEvent can't lose the row. - Log when a legacy record is dropped because its product no longer exists or its payload has no productId. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Migrates
synced_itemsfrom one Xero Item per price to one per product, and updates all downstream code to match. Part of the OUT-3788 simplified-product-mapping epic.Commits, by scope:
09a8b24) — drop theprice_idcolumn, replace the(portal_id, product_id, price_id)unique index with(portal_id, tenant_id, product_id), and dedupe existing rows (earliestcreatedAtwins). (OUT-3868)433356a) — move services to product-level:MappabledropspriceId;getSyncedItemsMapByProductIds,getProductIdToXeroItem, andgetCopilotProductAndXeroItemreplace their price-keyed equivalents; invoice line items resolve byproductId. (OUT-3869)c633347) — one row per product:ProductMappingbecomes{ product, item },getProductMappingsiterates products, the action/service operate on{ productId, itemId }, and the table drops the per-price/amount lines. (OUT-3870)Verification
pnpm typecheck✅pnpm lint✅Deferred follow-ups (tracked on OUT-3869)
These are webhook/sync-scope and intentionally not in this PR:
price.created→createSyncedItemsForPriceshas no "already synced?" guard, so a second price for an already-synced product would create a duplicate Xero Item and hit the new unique constraint (caught →failed_syncs→ retried forever). Needs anonConflictDoNothing/existence guard.addSyncedItems/deleteSyncedItemsusereturninstead ofcontinuein the missing-itemIdguard, aborting the whole batch.Testing Criteria
https://www.loom.com/share/0d075d8b0ee9431ca9542c300306b169
🤖 Generated with Claude Code