diff --git a/frontend/src/api/slabs.ts b/frontend/src/api/slabs.ts index 478d726..302a8d3 100644 --- a/frontend/src/api/slabs.ts +++ b/frontend/src/api/slabs.ts @@ -45,6 +45,9 @@ export interface Slab { inference_status: string | null; calibration_photo_id: string | null; woo_product_id: number | null; + /** Create-time optimistic token; updates compare against server_rev, not this. */ + client_rev: number; + /** Optimistic concurrency token; send as client_rev on PUT. */ server_rev: number; created_at: string; updated_at: string; @@ -135,7 +138,7 @@ export function getSlab(id: string): Promise { return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`).then((r) => parse(r)); } -export function updateSlab(id: string, patch: SlabInput & { client_rev: number }): Promise { +function putSlab(id: string, patch: SlabInput & { client_rev: number }): Promise { return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`, { method: 'PUT', headers: { 'Content-Type': 'application/json' }, @@ -143,6 +146,37 @@ export function updateSlab(id: string, patch: SlabInput & { client_rev: number } }).then((r) => parse(r)); } +/** + * PUT a slab patch. `client_rev` must equal the slab's current `server_rev`. + * On 409 revision_conflict (stale rev — e.g. review POST bumped server_rev, or + * a concurrent save), refetch once, adopt the latest server_rev, and retry. + * A second 409 surfaces a clear reload message. + */ +export async function updateSlab( + id: string, + patch: SlabInput & { client_rev: number }, +): Promise { + try { + return await putSlab(id, patch); + } catch (e) { + if (!(e instanceof ApiError) || e.status !== 409) throw e; + const latest = await getSlab(id); + try { + return await putSlab(id, { ...patch, client_rev: latest.server_rev }); + } catch (e2) { + if (e2 instanceof ApiError && e2.status === 409) { + throw new ApiError( + 409, + e2.code ?? 'revision_conflict', + 'This slab changed elsewhere. Reload the page and try again.', + e2.detail, + ); + } + throw e2; + } + } +} + export function deleteSlab(id: string): Promise { return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`, { method: 'DELETE' }).then((r) => parse(r), diff --git a/frontend/src/api/slabs.update.test.ts b/frontend/src/api/slabs.update.test.ts new file mode 100644 index 0000000..d3c5b22 --- /dev/null +++ b/frontend/src/api/slabs.update.test.ts @@ -0,0 +1,146 @@ +import assert from 'node:assert/strict'; +import { afterEach, test } from 'node:test'; +import { ApiError, updateSlab, type Slab } from './slabs'; + +const SLAB_ID = 'slab-rev-1'; + +function baseSlab(server_rev: number): Slab { + return { + id: SLAB_ID, + sku: 'SLAB-01', + status: 'calibrated', + price: 100, + price_source: 'recommendation', + length_in: 48, + thickness_in: 2, + sqft: 2.4, + bdft: 4.8, + width_min_in: 6, + width_max_in: 9, + width_avg_in: 8, + species_id: 1, + wood_category_ids: [2], + edge_type_term_id: 3, + figure_term_ids: [], + grade_term_id: 4, + thickness_term_id: 5, + moisture_term_id: 6, + fig_tag_ids: [], + feat_tag_ids: [], + title: 'Maple Slab', + short_description: 'short', + description: null, + content_source: 'manual', + species_confidence: null, + figure_confidence: null, + grade_confidence: null, + inference_status: null, + calibration_photo_id: null, + woo_product_id: null, + client_rev: 1, + server_rev, + created_at: 't', + updated_at: 't', + }; +} + +type Call = { method: string; url: string; body: unknown }; +const calls: Call[] = []; +let handler: (method: string, url: string, body: unknown) => { status: number; body: unknown }; + +const origFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = origFetch; + calls.length = 0; +}); + +function mockFetch() { + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + const method = (init?.method ?? 'GET').toUpperCase(); + const body = init?.body ? JSON.parse(String(init.body)) : null; + calls.push({ method, url, body }); + const { status, body: resp } = handler(method, url, body); + return new Response(JSON.stringify(resp), { + status, + headers: { 'Content-Type': 'application/json' }, + }); + }) as typeof fetch; +} + +test('updateSlab succeeds on first PUT when client_rev matches', async () => { + mockFetch(); + handler = (method, _url, body) => { + assert.equal(method, 'PUT'); + const patch = body as { client_rev: number }; + assert.equal(patch.client_rev, 3); + return { status: 200, body: baseSlab(4) }; + }; + const out = await updateSlab(SLAB_ID, { client_rev: 3, title: 'Maple Slab' }); + assert.equal(out.server_rev, 4); + assert.equal(calls.length, 1); +}); + +test('updateSlab on 409 refetches, adopts server_rev, retries once', async () => { + mockFetch(); + let puts = 0; + handler = (method, url, body) => { + if (method === 'PUT') { + puts += 1; + const patch = body as { client_rev: number; title?: string }; + if (puts === 1) { + assert.equal(patch.client_rev, 9); // stale (e.g. review POST bumped) + return { + status: 409, + body: { + error: { + code: 'revision_conflict', + message: 'client_rev mismatch', + detail: { current_server_rev: 10 }, + }, + }, + }; + } + assert.equal(patch.client_rev, 10); // adopted from GET + assert.equal(patch.title, 'Maple Slab'); + return { status: 200, body: baseSlab(11) }; + } + if (method === 'GET' && url.includes(SLAB_ID)) { + return { status: 200, body: baseSlab(10) }; + } + throw new Error(`unexpected ${method} ${url}`); + }; + const out = await updateSlab(SLAB_ID, { client_rev: 9, title: 'Maple Slab' }); + assert.equal(out.server_rev, 11); + assert.equal(puts, 2); + assert.equal(calls.map((c) => c.method).join(','), 'PUT,GET,PUT'); +}); + +test('updateSlab second 409 surfaces a reload message', async () => { + mockFetch(); + handler = (method) => { + if (method === 'PUT') { + return { + status: 409, + body: { + error: { + code: 'revision_conflict', + message: 'client_rev mismatch', + detail: { current_server_rev: 99 }, + }, + }, + }; + } + return { status: 200, body: baseSlab(99) }; + }; + await assert.rejects( + () => updateSlab(SLAB_ID, { client_rev: 1, sku: 'X' }), + (e: unknown) => { + assert.ok(e instanceof ApiError); + assert.equal(e.status, 409); + assert.match(e.message, /Reload the page/i); + return true; + }, + ); +}); diff --git a/frontend/src/create/review/Review.svelte b/frontend/src/create/review/Review.svelte index ad6adfc..9d2e14f 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -379,10 +379,10 @@ try { // Run the deterministic review FIRST (reads the current metadata) to get the // title/short_description build before we persist. + // Do NOT send price/sku here: review POST persists those overrides and bumps + // server_rev (#80), which would stale the client_rev on the PUT below. const probe = await reviewSlab(slabId, { primary_wood_category_id: primaryCatId === '' ? undefined : Number(primaryCatId), - price: price === '' ? undefined : Number(price), - sku: sku || undefined, }); if (myToken !== commitToken) return; // If the owner hasn't typed a title/short-desc, adopt the deterministic build @@ -411,10 +411,10 @@ if (myToken !== commitToken) return; // superseded; don't clobber the latest slab = fresh; // Re-run the review against the just-persisted row (authoritative ready/missing). + // No price/sku in payload — already on the row from the PUT; sending them + // would bump server_rev again and stale the next edit's client_rev. review = await reviewSlab(slabId, { primary_wood_category_id: primaryCatId === '' ? undefined : Number(primaryCatId), - price: price === '' ? undefined : Number(price), - sku: sku || undefined, }); if (myToken !== commitToken) return; if (review.primary_wood_category_id != null) primaryCatId = review.primary_wood_category_id;