fix(review): adopt latest server_rev on PUT; retry once on 409
Review was POSTing price/sku to /review before each save; that endpoint persists those overrides and bumps server_rev, so the following PUT sent a stale client_rev and hit 409 — Continue to Publish stayed disabled. Stop sending price/sku on review probes (PUT already persists them) and have updateSlab refetch + retry once on revision_conflict.
This commit is contained in:
@@ -45,6 +45,9 @@ export interface Slab {
|
|||||||
inference_status: string | null;
|
inference_status: string | null;
|
||||||
calibration_photo_id: string | null;
|
calibration_photo_id: string | null;
|
||||||
woo_product_id: number | 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;
|
server_rev: number;
|
||||||
created_at: string;
|
created_at: string;
|
||||||
updated_at: string;
|
updated_at: string;
|
||||||
@@ -135,7 +138,7 @@ export function getSlab(id: string): Promise<Slab> {
|
|||||||
return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`).then((r) => parse<Slab>(r));
|
return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`).then((r) => parse<Slab>(r));
|
||||||
}
|
}
|
||||||
|
|
||||||
export function updateSlab(id: string, patch: SlabInput & { client_rev: number }): Promise<Slab> {
|
function putSlab(id: string, patch: SlabInput & { client_rev: number }): Promise<Slab> {
|
||||||
return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`, {
|
return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`, {
|
||||||
method: 'PUT',
|
method: 'PUT',
|
||||||
headers: { 'Content-Type': 'application/json' },
|
headers: { 'Content-Type': 'application/json' },
|
||||||
@@ -143,6 +146,37 @@ export function updateSlab(id: string, patch: SlabInput & { client_rev: number }
|
|||||||
}).then((r) => parse<Slab>(r));
|
}).then((r) => parse<Slab>(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<Slab> {
|
||||||
|
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<void> {
|
export function deleteSlab(id: string): Promise<void> {
|
||||||
return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`, { method: 'DELETE' }).then((r) =>
|
return fetch(`/api/v1/slabs/${encodeURIComponent(id)}`, { method: 'DELETE' }).then((r) =>
|
||||||
parse<void>(r),
|
parse<void>(r),
|
||||||
|
|||||||
@@ -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;
|
||||||
|
},
|
||||||
|
);
|
||||||
|
});
|
||||||
@@ -379,10 +379,10 @@
|
|||||||
try {
|
try {
|
||||||
// Run the deterministic review FIRST (reads the current metadata) to get the
|
// Run the deterministic review FIRST (reads the current metadata) to get the
|
||||||
// title/short_description build before we persist.
|
// 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, {
|
const probe = await reviewSlab(slabId, {
|
||||||
primary_wood_category_id: primaryCatId === '' ? undefined : Number(primaryCatId),
|
primary_wood_category_id: primaryCatId === '' ? undefined : Number(primaryCatId),
|
||||||
price: price === '' ? undefined : Number(price),
|
|
||||||
sku: sku || undefined,
|
|
||||||
});
|
});
|
||||||
if (myToken !== commitToken) return;
|
if (myToken !== commitToken) return;
|
||||||
// If the owner hasn't typed a title/short-desc, adopt the deterministic build
|
// 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
|
if (myToken !== commitToken) return; // superseded; don't clobber the latest
|
||||||
slab = fresh;
|
slab = fresh;
|
||||||
// Re-run the review against the just-persisted row (authoritative ready/missing).
|
// 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, {
|
review = await reviewSlab(slabId, {
|
||||||
primary_wood_category_id: primaryCatId === '' ? undefined : Number(primaryCatId),
|
primary_wood_category_id: primaryCatId === '' ? undefined : Number(primaryCatId),
|
||||||
price: price === '' ? undefined : Number(price),
|
|
||||||
sku: sku || undefined,
|
|
||||||
});
|
});
|
||||||
if (myToken !== commitToken) return;
|
if (myToken !== commitToken) return;
|
||||||
if (review.primary_wood_category_id != null) primaryCatId = review.primary_wood_category_id;
|
if (review.primary_wood_category_id != null) primaryCatId = review.primary_wood_category_id;
|
||||||
|
|||||||
Reference in New Issue
Block a user