feat: stage new items as pending until an admin publishes them (#90)
SonarQube Analysis / sonarqube (pull_request) Failing after 38m41s
Tests / lint (pull_request) Successful in 8m37s
Tests / backend-unit (pull_request) Successful in 1m22s
Tests / frontend-e2e (pull_request) Failing after 30m44s

An item used to be live on the storefront the instant it was created. Now it starts pending, and a customer sees it only once it is published.

The migration changes the column default and nothing else. Backfilling would un-publish the entire live catalogue, which is the one thing it must not do.

Hiding a pending item took four separate changes, not one, and that is the part worth knowing. The storefront's item routes had no status filter at all — sold items are listed and rendered with a Sold badge deliberately — so pending could not be expressed as one more optional filter. GET /api/items now carries an exclusion the caller cannot opt out of; GET /api/items/:id carries the same, because hiding an item from the list while still serving it by id would leave it reachable to anyone who kept a link; and GET /api/filters excludes pending from both aggregates it computes. That last one is the least obvious: a pending item would have inflated its tags' counts, so a customer would read "Rare (1)", filter by it, and be told nothing matches — and its price would have stretched the slider to a range no visible item occupies.

The tag count is computed over the joined items rather than filtered with a WHERE. A WHERE would have dropped the row for a tag whose only item is pending, and the tag would have vanished from the drawer instead of showing zero. There is a test for exactly that, because the first version of this query had that bug.

parseItemFilters is shared by the storefront and admin routes, so 'pending' parses on both. The public route refuses it explicitly rather than answering with an empty list, which would read as "no items match" instead of "you may not ask that". The storefront's URL reader is deliberately left not accepting it either, with a comment saying so, since a request guaranteed to fail is not worth constructing.

Publishing is the existing mark-available: same transition, same UPDATE, so the admin UI labels that button "Publish" when the item is pending rather than adding a second endpoint that does the same thing. Unpublish is new and is not symmetrical — it is refused for a reserved item, which someone is holding in their cart right now, and for a sold one, which is a record of something that happened rather than a draft. Both refusals name their reason, and the buttons are hidden in those states so the refusal is not how you find out.

Changing a column default has reach, and it surfaced eight test fixtures that silently depended on it. Each is now explicit about the status it wants rather than inheriting one — better practice regardless, and immune to the next default change. Two tests also used 'pending' as their example of an *unknown* status; both would have quietly become tautologies, so they now use one that is genuinely unknown.

Verified: 98 unit, 160 integration and 94 end-to-end passing, the last on a freshly created container. One earlier run showed a single failure in favorites.spec.ts; it passes in isolation and on a clean container, and is the cross-spec interference already recorded against the suite rather than anything from this change.

Refs #90
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-21 12:40:08 -05:00
co-authored by Claude Opus 5
parent adf480faf6
commit ecc2219fa5
24 changed files with 495 additions and 26 deletions
+27 -3
View File
@@ -11,7 +11,7 @@ import '@uiw/react-md-editor/markdown-editor.css';
import '@uiw/react-markdown-preview/markdown.css';
import {
Item, Category, Tag as TagRecord,
fetchAdminItems, saveItem, deleteItem, deleteItemImage, markSold, markAvailable,
fetchAdminItems, saveItem, deleteItem, deleteItemImage, markSold, markAvailable, unpublishItem,
fetchAdminCategories, fetchAdminTags
} from '../api';
import { useThemeMode } from '../theme/ThemeContext';
@@ -29,7 +29,9 @@ const { Title } = Typography;
// Anything not sold or reserved is available, so green is the default rather
// than a third entry — a new status shows up green instead of crashing.
const STATUS_TAG_COLORS: Record<string, string> = { sold: 'red', reserved: 'orange' };
// Pending is grey rather than a colour: it is the absence of being published,
// not a state of its own worth drawing the eye to.
const STATUS_TAG_COLORS: Record<string, string> = { sold: 'red', reserved: 'orange', pending: 'default' };
function Inventory() {
const [items, setItems] = useState<Item[]>([]);
@@ -227,6 +229,20 @@ function Inventory() {
{item.status !== 'sold'
? <Button size="small" onClick={() => handleStatusChange(markSold, item.id, 'mark sold')}>Mark Sold</Button>
: <Button size="small" onClick={() => handleStatusChange(markAvailable, item.id, 'mark available')}>Mark Available</Button>}
{/* Publishing is mark-available under a name that says what it means
here. Unpublish is offered only from available — the server
refuses reserved and sold and says why, and hiding the button in
those states keeps the refusal from being the way you find out. */}
{item.status === 'pending' && (
<Button size="small" type="primary" onClick={() => handleStatusChange(markAvailable, item.id, 'publish')}>
Publish
</Button>
)}
{item.status === 'available' && (
<Button size="small" onClick={() => handleStatusChange(unpublishItem, item.id, 'unpublish')}>
Unpublish
</Button>
)}
</Space>
)
}
@@ -314,7 +330,15 @@ function Inventory() {
<div style={{ maxWidth: 340, margin: '0 auto' }}>
{/* onChanged never fires: every handler that would call it is
short-circuited by `preview`. */}
<ItemCard item={previewItem} onChanged={() => undefined} preview />
{/* A pending item is previewed as it will look once published.
No customer ever sees a pending item, so rendering that state
would answer a question nobody is asking — what is wanted here
is "how will this look when it is live". */}
<ItemCard
item={previewItem.status === 'pending' ? { ...previewItem, status: 'available' } : previewItem}
onChanged={() => undefined}
preview
/>
</div>
)}
</Drawer>
+1
View File
@@ -21,6 +21,7 @@ function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] {
}
const STATUS_OPTIONS: { value: ItemStatus; label: string }[] = [
{ value: 'pending', label: 'Pending' },
{ value: 'available', label: 'Available' },
{ value: 'reserved', label: 'Reserved' },
{ value: 'sold', label: 'Sold' }
+16 -1
View File
@@ -13,7 +13,7 @@ export interface Item {
description: string | null;
price_cents: number;
images: { id: number; image_path: string; sort_order: number }[];
status: 'available' | 'reserved' | 'sold';
status: 'pending' | 'available' | 'reserved' | 'sold';
category_id: number | null;
category_name: string | null;
tags: ItemTag[];
@@ -114,6 +114,10 @@ export async function markSold(id: number): Promise<Item> {
return res.json();
}
// Publishing a pending item is mark-available: it is the same transition and
// the same UPDATE, so the admin UI simply labels the button "Publish" when the
// item is pending rather than calling a second endpoint that does the same
// thing.
export async function markAvailable(id: number): Promise<Item> {
const res = await expectOk(
await fetch(`/api/admin/items/${id}/mark-available`, { method: 'POST' }),
@@ -122,6 +126,17 @@ export async function markAvailable(id: number): Promise<Item> {
return res.json();
}
// Not symmetrical with the above: the server refuses to unpublish a reserved or
// sold item and says which, so the message it returns is worth surfacing rather
// than replacing with a generic one.
export async function unpublishItem(id: number): Promise<Item> {
const res = await expectOk(
await fetch(`/api/admin/items/${id}/unpublish`, { method: 'POST' }),
'failed to unpublish'
);
return res.json();
}
// The admin endpoints return a JSON error body on 4xx; surfacing its message
// lets the UI say "that name is already used here" instead of a generic
// failure.
+8 -1
View File
@@ -1,6 +1,6 @@
import type { Category } from './api';
export type ItemStatus = 'available' | 'reserved' | 'sold';
export type ItemStatus = 'pending' | 'available' | 'reserved' | 'sold';
export interface ItemFilters {
categoryId: number | null;
@@ -51,6 +51,13 @@ export function filtersFromSearchParams(params: URLSearchParams): ItemFilters {
.map((part) => readInt(part))
.filter((id): id is number => id !== null && id > 0);
// Deliberately does NOT accept 'pending', even though it is a valid
// ItemStatus. This reader exists for the storefront's URL, where filtering by
// pending is not a thing a customer may ask for — the public API refuses it
// outright, so parsing it here would only produce a request guaranteed to
// fail. The admin's status filter holds its value in React state and never
// round-trips through this function, so it is unaffected. Do not "complete"
// this list to match the type.
const rawStatus = params.get('status');
const status = rawStatus === 'available' || rawStatus === 'reserved' || rawStatus === 'sold'
? rawStatus
@@ -96,6 +96,8 @@ test.describe('Disabling a customer account', () => {
multipart: { name: itemName, description: '', price: '40', category_id: '', tags: '[]' }
});
const itemId = (await created.json()).id as number;
// New items are pending, and a pending item cannot be added to a cart.
expect((await request.post(`/api/admin/items/${itemId}/mark-available`)).ok()).toBeTruthy();
await register(page, email);
expect((await page.request.post(`/api/cart/items/${itemId}`)).status()).toBe(201);
@@ -18,8 +18,10 @@ test.beforeAll(async ({ playwright }) => {
})).json()).id;
await api.post('/api/admin/tags', { data: { name: NAMES.tag } });
const item = (name: string, price: string, inCategory: boolean) =>
api.post('/api/admin/items', {
// Published after creation: new items are pending, and these fixtures stand
// in for ordinary stock rather than staged drafts.
const item = async (name: string, price: string, inCategory: boolean) => {
const res = await api.post('/api/admin/items', {
multipart: {
name,
description: '',
@@ -28,6 +30,8 @@ test.beforeAll(async ({ playwright }) => {
tags: JSON.stringify(inCategory ? [NAMES.tag] : [])
}
});
await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`);
};
await item(NAMES.cheap, '50', true);
await item(NAMES.mid, '150', true);
@@ -7,7 +7,12 @@ async function createItem(page: import('@playwright/test').Page, name: string, p
multipart: { name, description: 'A preview subject', price, category_id: '', tags: '[]' }
});
expect(created.ok()).toBeTruthy();
return (await created.json()).id as number;
const id = (await created.json()).id as number;
// New items are pending. Published here so the card renders the same states
// a customer would see; the pending case is covered in the pending spec.
const published = await page.request.post(`/api/admin/items/${id}/mark-available`);
expect(published.ok()).toBeTruthy();
return id;
}
test.describe('Admin item preview', () => {
@@ -12,6 +12,8 @@ async function reserveItem(page: import('@playwright/test').Page, itemName: stri
});
expect(created.ok()).toBeTruthy();
const itemId = (await created.json()).id as number;
// New items are pending, and a pending item cannot be reserved.
expect((await page.request.post(`/api/admin/items/${itemId}/mark-available`)).ok()).toBeTruthy();
await page.goto('/register');
await page.getByRole('textbox', { name: 'Email' }).fill(email);
@@ -21,6 +21,8 @@ test.beforeAll(async ({ playwright }) => {
multipart: { name, description: '', price, category_id: '', tags: '[]' }
});
expect(res.ok()).toBeTruthy();
// New items are pending; the storefront only lists published ones.
expect((await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`)).ok()).toBeTruthy();
}
await api.dispose();
});
+2
View File
@@ -12,6 +12,8 @@ test.beforeAll(async ({ playwright }) => {
multipart: { name: ITEM, description: '', price: '60', category_id: '', tags: '[]' }
});
expect(res.ok()).toBeTruthy();
// New items are pending; the storefront only lists published ones.
expect((await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`)).ok()).toBeTruthy();
await api.dispose();
});
+2
View File
@@ -49,6 +49,8 @@ async function createItem(
}
});
expect(res.ok()).toBeTruthy();
// New items are pending; the storefront only lists published ones.
expect((await api.post(`/api/admin/items/${(await res.json()).id}/mark-available`)).ok()).toBeTruthy();
}
test.beforeAll(async ({ playwright }) => {
@@ -0,0 +1,78 @@
import { test, expect } from './fixtures';
const suffix = () => `s${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
// Creates an item and leaves it as it arrives — pending. Deliberately does not
// publish, unlike the other specs' fixtures, because the staged state is what
// is being tested here.
async function createStagedItem(page: import('@playwright/test').Page, name: string) {
const created = await page.request.post('/api/admin/items', {
multipart: { name, description: '', price: '55', category_id: '', tags: '[]' }
});
expect(created.ok()).toBeTruthy();
const body = await created.json();
// The whole premise: creating something does not publish it.
expect(body.status).toBe('pending');
return body.id as number;
}
const rowFor = (page: import('@playwright/test').Page, name: string) =>
page.getByRole('row').filter({ hasText: name });
test.describe('Staging an item until it is published', () => {
test('a new item is held back from the storefront until published', async ({ page }) => {
const name = `Staged ${suffix()}`;
await createStagedItem(page, name);
// Not in the catalogue while pending.
await page.goto('/');
await expect(page.getByText(name)).toHaveCount(0);
// It is in the admin, marked as pending.
await page.goto('/admin');
const row = rowFor(page, name);
await expect(row).toBeVisible();
await expect(row.getByText('PENDING')).toBeVisible();
await row.getByRole('button', { name: 'Publish' }).click();
await expect(row.getByText('AVAILABLE')).toBeVisible();
// And now a customer can see it.
await page.goto('/');
await expect(page.getByText(name)).toBeVisible();
});
test('publishing can be undone while nobody is holding the item', async ({ page }) => {
const name = `Staged ${suffix()}`;
const id = await createStagedItem(page, name);
expect((await page.request.post(`/api/admin/items/${id}/mark-available`)).ok()).toBeTruthy();
await page.goto('/');
await expect(page.getByText(name)).toBeVisible();
await page.goto('/admin');
const row = rowFor(page, name);
await row.getByRole('button', { name: 'Unpublish' }).click();
await expect(row.getByText('PENDING')).toBeVisible();
await page.goto('/');
await expect(page.getByText(name)).toHaveCount(0);
});
// The two features together, which is the reason for wanting both: a staged
// item is previewed as it will look once live, because a customer never sees
// the pending state and "how will this look" is the question being asked.
test('a pending item previews as it will look once published', async ({ page }) => {
const name = `Staged ${suffix()}`;
await createStagedItem(page, name);
await page.goto('/admin');
await page.getByRole('button', { name }).click();
const drawer = page.getByRole('dialog', { name: `Preview: ${name}` });
await expect(drawer).toBeVisible();
await expect(drawer.getByText('$55.00')).toBeVisible();
// The live card's action, not a pending placeholder.
await expect(drawer.getByRole('button', { name: 'Add to Cart' })).toBeVisible();
});
});