fix(products): variant SKU collisions & save-flow error UX - #91
Merged
Conversation
Root cause (from production requestId aa6d6370): POST
/api/products/:id/variants crashed with a raw D1 UNIQUE constraint on
product_variants.sku → HTTP 500 → generic toast. No test anywhere
covered variant SKU uniqueness — mock tests cannot hit real SQL
constraints, and the prior e2e suites covered orders/stock/money only.
Server
- variants/queries.ts now wraps shared createVariant/updateVariant
with a SKU availability pre-check (and a defensive unique-violation
mapping for the check→write race). A collision returns 409
DUPLICATE_SKU carrying the offending SKU — the same friendly
treatment product creation already had.
- Regression-locked on real D1 (variants.sku-e2e): create-dup,
update-dup, self-rename OK, fresh SKU OK + opening-stock ledger
intact. The exact failing HTTP call now returns 409 with the SKU.
- variants/CONTEXT.md: "SKU collisions crash late" is no longer true.
Dashboard
- productErrorMessage surfaces real API detail instead of collapsing
everything to "Something went wrong": DUPLICATE_SKU names the SKU,
VALIDATION_FAILED lists every failing field/path, 500s carry the
requestId so support can correlate with server logs.
- ProductForm save flow: failures name the failing step ("Variant
أبيض / 128GB could not be saved — …") instead of one generic blob.
- Double-create retry trap closed: when a new product saves but a
variant/image step fails, the form redirects to the edit page with a
toast — retry becomes an update, never a duplicate product.
- Client validation hardened: price must be a whole number; duplicate
SKUs across variant rows are caught before any request is sent.
- 7 new locale keys × ar/en/fr (i18n parity guard passes).
Verified: cod-server 118 files / 1709 tests green + tsc clean;
cod-client-astro 142 tests + typecheck + i18n parity green; deployed
and smoke-verified (the original 500 endpoint now 409).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A merchant editing a product hit
POST /api/products/:id/variantsreturning 500 "An unexpected error occurred" (production requestIdaa6d6370). Root cause traced through the server log: a raw D1 UNIQUE constraint onproduct_variants.sku— the variant save carried a SKU that already existed. Products already had a friendly 409 for this; variants never did. This PR fixes the crash and the surrounding error-UX gaps in the product save flow.Why the tests missed it: no test anywhere covered variant SKU uniqueness — mock-based tests cannot hit real SQL constraints, and the previous real-D1 e2e suites covered orders/stock/money paths, not variant CRUD.
Fixes
Server — friendly 409 instead of raw 500
variants/queries.tswraps the sharedcreateVariant/updateVariantwith a SKU availability pre-check, plus a defensive unique-violation mapping for the check→write race409 DUPLICATE_SKUcarrying the offending SKU — the same treatment product creation already hadvariants.sku-e2e, 4 tests): create-dup → 409, update-rename-dup → 409, self-rename → OK, fresh SKU → OK with opening-stock ledger intact409with the SKU namedDashboard — errors that say what actually happened
SKU "X" is already used by another variantprice: must be an int · sku: required7 new locale keys × ar/en/fr (i18n parity guard passes).
Test plan
variants.sku-e2e(4 tests) — includes the exact failing endpoint shapetsc --noEmitcleanFollow-up from #90 still open (cosmetic): storefront rates map displays fees for disabled delivery types — server refuses those orders, theme01 shows the fee before rejection.