fix(admin): confirm writes succeeded and allow inline category creation (#23)

Reported from testing: the item form said "Item added" for a save that
never happened.

saveItem and the other admin calls returned res.json() without checking
res.ok, so a 4xx/5xx resolved normally and every caller reported success
for a write the server had rejected. That is worse than failing outright,
because nothing prompts the user to look for the missing row. All admin
calls now throw on a non-OK response, and the handlers report the error,
keep the form open so entered values survive, and only claim success once
the server has accepted the write. Mark sold/available previously did
nothing visible on failure at all.

Categories can now be created from the item form, as tags already could.
Previously a category that did not exist yet meant abandoning a
half-filled form for the Categories tab. New categories are created at
the top level; nesting stays in the Categories tab.

The control lives in its own component: inline, every keystroke
re-rendered the whole Inventory component and rebuilt the category tree,
which visibly jittered the open popup. It sits above the tree rather than
below it, where a long list both hid it and made its position depend on
the list's measured height. The tree no longer expands everything on
open, which does not scale past a screenful; it has search instead.

The app now honours prefers-reduced-motion by disabling antd transitions,
and the e2e suite runs with that preference set.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-17 11:07:44 -05:00
co-authored by Claude Opus 5
parent c77fdad2b9
commit 77e58c0b92
7 changed files with 361 additions and 38 deletions
+6 -1
View File
@@ -7,7 +7,12 @@ export default defineConfig({
reporter: [['list']], reporter: [['list']],
use: { use: {
baseURL: 'http://localhost:5173', baseURL: 'http://localhost:5173',
trace: 'on-first-retry' trace: 'on-first-retry',
// The app turns off antd's transitions under this preference. Animated
// popups never settle long enough for Playwright's stability check when
// the machine is loaded, which showed up as clicks timing out on a button
// that was plainly visible and enabled.
reducedMotion: 'reduce'
}, },
webServer: { webServer: {
command: 'npm run dev', command: 'npm run dev',
+41 -26
View File
@@ -2,7 +2,7 @@ import { useEffect, useState } from 'react';
import { import {
Layout, Table, Button, Form, Input, InputNumber, Upload, Modal, Layout, Table, Button, Form, Input, InputNumber, Upload, Modal,
Space, Tag, Typography, Switch, message, Image as AntImage, theme, Tabs, Space, Tag, Typography, Switch, message, Image as AntImage, theme, Tabs,
TreeSelect, Select Select
} from 'antd'; } from 'antd';
import { UploadOutlined, DeleteOutlined } from '@ant-design/icons'; import { UploadOutlined, DeleteOutlined } from '@ant-design/icons';
import type { UploadFile } from 'antd/es/upload/interface'; import type { UploadFile } from 'antd/es/upload/interface';
@@ -14,26 +14,12 @@ import {
fetchAdminItems, saveItem, deleteItem, deleteItemImage, markSold, markAvailable, fetchAdminItems, saveItem, deleteItem, deleteItemImage, markSold, markAvailable,
fetchAdminCategories, fetchAdminTags fetchAdminCategories, fetchAdminTags
} from '../api'; } from '../api';
import { buildCategoryTree, CategoryNode } from '../filters';
import { useThemeMode } from '../theme/ThemeContext'; import { useThemeMode } from '../theme/ThemeContext';
import Customers from './Customers'; import Customers from './Customers';
import Settings from './Settings'; import Settings from './Settings';
import Categories from './Categories'; import Categories from './Categories';
import Tags from './Tags'; import Tags from './Tags';
import CategoryTreeSelect from './CategoryTreeSelect';
interface CategoryTreeOption {
value: number;
title: string;
children?: CategoryTreeOption[];
}
function toCategoryTreeData(nodes: CategoryNode[]): CategoryTreeOption[] {
return nodes.map(node => ({
value: node.id,
title: node.name,
children: node.children.length ? toCategoryTreeData(node.children) : undefined
}));
}
const { Header, Content } = Layout; const { Header, Content } = Layout;
const { Title } = Typography; const { Title } = Typography;
@@ -47,6 +33,7 @@ function Inventory() {
const [description, setDescription] = useState<string>(''); const [description, setDescription] = useState<string>('');
const [categories, setCategories] = useState<Category[]>([]); const [categories, setCategories] = useState<Category[]>([]);
const [tags, setTags] = useState<TagRecord[]>([]); const [tags, setTags] = useState<TagRecord[]>([]);
const [saving, setSaving] = useState(false);
const { mode } = useThemeMode(); const { mode } = useThemeMode();
const load = () => fetchAdminItems().then(setItems); const load = () => fetchAdminItems().then(setItems);
@@ -94,21 +81,55 @@ function Inventory() {
fd.append('category_id', values.category_id == null ? '' : String(values.category_id)); fd.append('category_id', values.category_id == null ? '' : String(values.category_id));
fd.append('tags', JSON.stringify(values.tags ?? [])); fd.append('tags', JSON.stringify(values.tags ?? []));
fileList.forEach(f => { if (f.originFileObj) fd.append('images', f.originFileObj as File); }); fileList.forEach(f => { if (f.originFileObj) fd.append('images', f.originFileObj as File); });
// Only report success, close the form, and discard the entered values once
// the server has actually accepted the write.
setSaving(true);
try {
await saveItem(editingItem?.id ?? null, fd); await saveItem(editingItem?.id ?? null, fd);
} catch (err) {
message.error(`Couldn't save item — ${(err as Error).message}`);
return;
} finally {
setSaving(false);
}
message.success(editingItem ? 'Item updated' : 'Item added'); message.success(editingItem ? 'Item updated' : 'Item added');
setModalOpen(false); setModalOpen(false);
load(); load();
loadOptions(); loadOptions();
} }
// Status changes silently did nothing on failure — the row simply stayed put
// with no indication why.
async function handleStatusChange(action: (id: number) => Promise<Item>, id: number, label: string) {
try {
await action(id);
} catch (err) {
message.error(`Couldn't ${label}${(err as Error).message}`);
return;
}
load();
}
async function handleDelete(id: number) { async function handleDelete(id: number) {
try {
await deleteItem(id); await deleteItem(id);
} catch (err) {
message.error(`Couldn't delete item — ${(err as Error).message}`);
return;
}
message.success('Item deleted'); message.success('Item deleted');
load(); load();
} }
async function handleDeleteImage(itemId: number, imageId: number) { async function handleDeleteImage(itemId: number, imageId: number) {
try {
await deleteItemImage(itemId, imageId); await deleteItemImage(itemId, imageId);
} catch (err) {
message.error(`Couldn't remove image — ${(err as Error).message}`);
return;
}
message.success('Image removed'); message.success('Image removed');
load(); load();
setEditingItem(prev => prev && prev.id === itemId setEditingItem(prev => prev && prev.id === itemId
@@ -159,8 +180,8 @@ function Inventory() {
<Button size="small" onClick={() => openEdit(item)}>Edit</Button> <Button size="small" onClick={() => openEdit(item)}>Edit</Button>
<Button size="small" danger onClick={() => handleDelete(item.id)}>Delete</Button> <Button size="small" danger onClick={() => handleDelete(item.id)}>Delete</Button>
{item.status !== 'sold' {item.status !== 'sold'
? <Button size="small" onClick={() => markSold(item.id).then(load)}>Mark Sold</Button> ? <Button size="small" onClick={() => handleStatusChange(markSold, item.id, 'mark sold')}>Mark Sold</Button>
: <Button size="small" onClick={() => markAvailable(item.id).then(load)}>Mark Available</Button>} : <Button size="small" onClick={() => handleStatusChange(markAvailable, item.id, 'mark available')}>Mark Available</Button>}
</Space> </Space>
) )
} }
@@ -174,7 +195,7 @@ function Inventory() {
</div> </div>
<Table rowKey="id" dataSource={items} columns={columns} scroll={{ x: true }} /> <Table rowKey="id" dataSource={items} columns={columns} scroll={{ x: true }} />
<Modal title={editingItem ? 'Edit Item' : 'Add Item'} open={modalOpen} onOk={handleOk} onCancel={() => setModalOpen(false)} destroyOnClose width={720}> <Modal title={editingItem ? 'Edit Item' : 'Add Item'} open={modalOpen} onOk={handleOk} confirmLoading={saving} onCancel={() => setModalOpen(false)} destroyOnHidden width={720}>
<div data-color-mode={mode}> <div data-color-mode={mode}>
<Form form={form} layout="vertical"> <Form form={form} layout="vertical">
<Form.Item name="name" label="Name" rules={[{ required: true }]}> <Form.Item name="name" label="Name" rules={[{ required: true }]}>
@@ -187,13 +208,7 @@ function Inventory() {
<InputNumber min={0} step={0.01} style={{ width: '100%' }} /> <InputNumber min={0} step={0.01} style={{ width: '100%' }} />
</Form.Item> </Form.Item>
<Form.Item name="category_id" label="Category"> <Form.Item name="category_id" label="Category">
<TreeSelect <CategoryTreeSelect categories={categories} onCategoriesChanged={setCategories} />
allowClear
placeholder="Uncategorized"
treeDefaultExpandAll
treeData={toCategoryTreeData(buildCategoryTree(categories))}
style={{ width: '100%' }}
/>
</Form.Item> </Form.Item>
<Form.Item <Form.Item
name="tags" name="tags"
+107
View File
@@ -0,0 +1,107 @@
import { useMemo, useState } from 'react';
import TreeSelect from 'antd/lib/tree-select';
import Input from 'antd/lib/input';
import Button from 'antd/lib/button';
import Divider from 'antd/lib/divider';
import message from 'antd/lib/message';
import { Category, createCategory, fetchAdminCategories } from '../api';
import { buildCategoryTree, CategoryNode } from '../filters';
interface CategoryTreeOption {
value: number;
title: string;
children?: CategoryTreeOption[];
}
function toTreeData(nodes: CategoryNode[]): CategoryTreeOption[] {
return nodes.map((node) => ({
value: node.id,
title: node.name,
children: node.children.length ? toTreeData(node.children) : undefined
}));
}
interface Props {
// Supplied by antd's Form.Item. `id` has to be forwarded or the field loses
// its association with the rendered <label>, which breaks both screen readers
// and any lookup by label.
value?: number;
onChange?: (value: number | undefined) => void;
id?: string;
categories: Category[];
onCategoriesChanged: (categories: Category[]) => void;
}
// Pulled out of the item form so that typing a new category name re-renders
// only this control. Left inline, every keystroke re-rendered the whole
// Inventory component and rebuilt the tree data, which visibly jittered the
// open popup and moved the buttons under the pointer.
export default function CategoryTreeSelect({ value, onChange, id, categories, onCategoriesChanged }: Props) {
const [newCategoryName, setNewCategoryName] = useState('');
const [creating, setCreating] = useState(false);
const treeData = useMemo(() => toTreeData(buildCategoryTree(categories)), [categories]);
// Tags can be invented from the item form, so categories should be too —
// otherwise adding an item in a new category means abandoning a half-filled
// form. New categories land at the top level; nesting is done in the
// Categories tab, keeping this control to a single decision.
async function handleCreate() {
const name = newCategoryName.trim();
if (!name) return;
setCreating(true);
try {
const created = await createCategory(name, null);
onCategoriesChanged(await fetchAdminCategories());
onChange?.(created.id);
setNewCategoryName('');
message.success(`Category "${created.name}" added`);
} catch (err) {
message.error(`Couldn't create category — ${(err as Error).message}`);
} finally {
setCreating(false);
}
}
return (
<TreeSelect
allowClear
showSearch
placeholder="Uncategorized"
// Deliberately not treeDefaultExpandAll: expanding a large tree on open
// makes the popup reflow while it measures, and buries the create field
// below every row. Search is the better affordance past a screenful.
treeNodeFilterProp="title"
listHeight={256}
treeData={treeData}
id={id}
value={value}
onChange={onChange}
style={{ width: '100%' }}
popupRender={(menu) => (
<>
{/* Above the tree, not below it. Under a long list the control is
both invisible without scrolling and positioned by the list's
measured height, so it shifts as the virtualized rows settle. */}
<div style={{ display: 'flex', gap: 8, padding: '8px 8px 0' }}>
<Input
placeholder="New category name"
value={newCategoryName}
onChange={(event) => setNewCategoryName(event.target.value)}
// Without this the tree steals the keystrokes for its own
// type-ahead and arrow-key navigation.
onKeyDown={(event) => event.stopPropagation()}
onPressEnter={handleCreate}
/>
<Button type="primary" loading={creating} onClick={handleCreate}>
Create category
</Button>
</div>
<Divider style={{ margin: '8px 0' }} />
{menu}
</>
)}
/>
);
}
+28 -6
View File
@@ -67,32 +67,54 @@ export async function fetchFilterOptions(): Promise<FilterOptions> {
return res.json(); return res.json();
} }
// Every admin call goes through this. Without the res.ok check a 4xx/5xx still
// resolves — the caller then reports success for a write that never happened,
// which is worse than failing outright because nothing prompts the user to look
// for the missing row.
async function expectOk(res: Response, action: string): Promise<Response> {
if (res.ok) return res;
const detail = await res.json().catch(() => null);
throw new Error(detail?.error ? `${action}: ${detail.error}` : action);
}
export async function fetchAdminItems(): Promise<Item[]> { export async function fetchAdminItems(): Promise<Item[]> {
const res = await fetch('/api/admin/items'); const res = await expectOk(await fetch('/api/admin/items'), 'failed to load items');
return res.json(); return res.json();
} }
export async function saveItem(id: number | null, formData: FormData): Promise<Item> { export async function saveItem(id: number | null, formData: FormData): Promise<Item> {
const url = id ? `/api/admin/items/${id}` : '/api/admin/items'; const url = id ? `/api/admin/items/${id}` : '/api/admin/items';
const res = await fetch(url, { method: id ? 'PUT' : 'POST', body: formData }); const res = await expectOk(
await fetch(url, { method: id ? 'PUT' : 'POST', body: formData }),
'failed to save item'
);
return res.json(); return res.json();
} }
export async function deleteItem(id: number): Promise<void> { export async function deleteItem(id: number): Promise<void> {
await fetch(`/api/admin/items/${id}`, { method: 'DELETE' }); await expectOk(await fetch(`/api/admin/items/${id}`, { method: 'DELETE' }), 'failed to delete item');
} }
export async function deleteItemImage(itemId: number, imageId: number): Promise<void> { export async function deleteItemImage(itemId: number, imageId: number): Promise<void> {
await fetch(`/api/admin/items/${itemId}/images/${imageId}`, { method: 'DELETE' }); await expectOk(
await fetch(`/api/admin/items/${itemId}/images/${imageId}`, { method: 'DELETE' }),
'failed to remove image'
);
} }
export async function markSold(id: number): Promise<Item> { export async function markSold(id: number): Promise<Item> {
const res = await fetch(`/api/admin/items/${id}/mark-sold`, { method: 'POST' }); const res = await expectOk(
await fetch(`/api/admin/items/${id}/mark-sold`, { method: 'POST' }),
'failed to mark sold'
);
return res.json(); return res.json();
} }
export async function markAvailable(id: number): Promise<Item> { export async function markAvailable(id: number): Promise<Item> {
const res = await fetch(`/api/admin/items/${id}/mark-available`, { method: 'POST' }); const res = await expectOk(
await fetch(`/api/admin/items/${id}/mark-available`, { method: 'POST' }),
'failed to mark available'
);
return res.json(); return res.json();
} }
+24 -2
View File
@@ -1,4 +1,4 @@
import React from 'react'; import React, { useEffect, useState } from 'react';
import ReactDOM from 'react-dom/client'; import ReactDOM from 'react-dom/client';
import { BrowserRouter, Routes, Route } from 'react-router-dom'; import { BrowserRouter, Routes, Route } from 'react-router-dom';
import { ConfigProvider, theme as antdTheme } from 'antd'; import { ConfigProvider, theme as antdTheme } from 'antd';
@@ -16,13 +16,35 @@ import { CartProvider } from './cart/CartContext';
import { ThemeModeProvider, useThemeMode } from './theme/ThemeContext'; import { ThemeModeProvider, useThemeMode } from './theme/ThemeContext';
import './styles.css'; import './styles.css';
const REDUCED_MOTION_QUERY = '(prefers-reduced-motion: reduce)';
// Respects the OS-level "reduce motion" accessibility setting by turning off
// antd's transitions. Beyond the accessibility win, animated popups are a
// standing source of flake in end-to-end tests, which drive the app with this
// preference enabled.
function usePrefersReducedMotion(): boolean {
const [prefers, setPrefers] = useState(
() => typeof window !== 'undefined' && window.matchMedia(REDUCED_MOTION_QUERY).matches
);
useEffect(() => {
const query = window.matchMedia(REDUCED_MOTION_QUERY);
const update = () => setPrefers(query.matches);
query.addEventListener('change', update);
return () => query.removeEventListener('change', update);
}, []);
return prefers;
}
function Root() { function Root() {
const { mode } = useThemeMode(); const { mode } = useThemeMode();
const prefersReducedMotion = usePrefersReducedMotion();
return ( return (
<ConfigProvider <ConfigProvider
theme={{ theme={{
algorithm: mode === 'dark' ? antdTheme.darkAlgorithm : antdTheme.defaultAlgorithm, algorithm: mode === 'dark' ? antdTheme.darkAlgorithm : antdTheme.defaultAlgorithm,
token: { colorPrimary: '#1a1a1a' } token: { colorPrimary: '#1a1a1a', motion: !prefersReducedMotion }
}} }}
> >
<BrowserRouter> <BrowserRouter>
@@ -0,0 +1,77 @@
import { test, expect } from '@playwright/test';
const suffix = () => `i${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
// Opening the form kicks off a categories fetch. Interacting with the category
// field before it lands means the tree re-renders under the cursor, so wait for
// the data rather than racing it.
async function openItemForm(page: import('@playwright/test').Page) {
// Opening the form fetches both categories and tags, and each one re-renders
// the modal as it lands. Waiting for only one still leaves the second to
// reflow the popup mid-interaction.
const loaded = Promise.all([
page.waitForResponse((res) => res.url().includes('/api/admin/categories') && res.request().method() === 'GET'),
page.waitForResponse((res) => res.url().includes('/api/admin/tags') && res.request().method() === 'GET')
]);
await page.getByRole('button', { name: 'Add Item' }).click();
await loaded;
}
test.describe('Inline category creation from the item form', () => {
test('creates a category without leaving the item form and assigns it', async ({ page }) => {
const RUN = suffix();
const categoryName = `Inline ${RUN}`;
const itemName = `Item ${RUN}`;
await page.goto('/admin');
await openItemForm(page);
await page.getByLabel('Name').fill(itemName);
await page.getByLabel('Price (USD)').fill('99');
await page.getByRole('dialog').getByLabel('Category', { exact: true }).click();
const nameInput = page.getByPlaceholder('New category name');
await expect(nameInput).toBeVisible();
await expect(page.getByRole('button', { name: 'Create category' })).toBeVisible();
await nameInput.fill(categoryName);
// Submitted with Enter rather than a click: the popup sits over a
// virtualized tree that keeps re-measuring, so a click target inside it is
// never geometrically stable. Enter runs the same handler as the button.
await nameInput.press('Enter');
// The new category should be selected straight away — having to hunt for it
// in the tree afterwards defeats the point of creating it inline.
await expect(page.getByRole('dialog').getByText(categoryName)).toBeVisible();
await page.getByRole('button', { name: 'OK' }).click();
await expect(page.getByText('Item added')).toBeVisible();
const items = await (await page.request.get('/api/admin/items')).json();
const saved = items.find((item: { name: string }) => item.name === itemName);
expect(saved).toBeTruthy();
expect(saved.category_name).toBe(categoryName);
});
test('reports a duplicate category name instead of silently doing nothing', async ({ page }) => {
const RUN = suffix();
const categoryName = `Dupe ${RUN}`;
const created = await page.request.post('/api/admin/categories', {
data: { name: categoryName, parent_id: null }
});
expect(created.status()).toBe(201);
await page.goto('/admin');
await openItemForm(page);
await page.getByRole('dialog').getByLabel('Category', { exact: true }).click();
const nameInput = page.getByPlaceholder('New category name');
await expect(nameInput).toBeVisible();
await expect(page.getByRole('button', { name: 'Create category' })).toBeVisible();
await nameInput.fill(categoryName);
// Submitted with Enter rather than a click: the popup sits over a
// virtualized tree that keeps re-measuring, so a click target inside it is
// never geometrically stable. Enter runs the same handler as the button.
await nameInput.press('Enter');
await expect(page.getByText(/already exists/i)).toBeVisible();
});
});
@@ -0,0 +1,75 @@
import { test, expect } from '@playwright/test';
const suffix = () => `s${Date.now().toString(36)}${Math.random().toString(36).slice(2, 7)}`;
test.describe('Admin save failures', () => {
test('does not claim an item was saved when the request failed', async ({ page }) => {
await page.route('**/api/admin/items', (route) => {
if (route.request().method() === 'POST') {
return route.fulfill({
status: 500,
contentType: 'application/json',
body: '{"error":"internal error"}'
});
}
return route.continue();
});
await page.goto('/admin');
await page.getByRole('button', { name: 'Add Item' }).click();
await page.getByLabel('Name').fill(`Broken ${suffix()}`);
await page.getByLabel('Price (USD)').fill('12');
await page.getByRole('button', { name: 'OK' }).click();
// Reporting success for a failed save is worse than failing loudly: the
// item is silently absent and the user has no reason to look for it.
await expect(page.getByText('Item added')).toBeHidden();
await expect(page.getByText("Couldn't save item")).toBeVisible();
// The form must stay open so the entered values aren't lost.
await expect(page.getByRole('dialog')).toBeVisible();
});
test('reports a failed delete rather than claiming success', async ({ page }) => {
// Seeded through the API so the test owns a known row rather than clicking
// whichever Delete button happens to be first in a paginated table.
const name = `Doomed ${suffix()}`;
const created = await page.request.post('/api/admin/items', {
multipart: { name, description: '', price: '10', category_id: '', tags: '[]' }
});
expect(created.ok()).toBeTruthy();
await page.route('**/api/admin/items/*', (route) => {
if (route.request().method() === 'DELETE') {
return route.fulfill({ status: 500, contentType: 'application/json', body: '{"error":"nope"}' });
}
return route.continue();
});
await page.goto('/admin');
// Items list newest-first, so the seeded row is on the first page.
const row = page.getByRole('row').filter({ hasText: name });
await expect(row).toBeVisible();
await row.getByRole('button', { name: 'Delete' }).click();
await expect(page.getByText('Item deleted')).toBeHidden();
await expect(page.getByText("Couldn't delete item")).toBeVisible();
// The row must survive a failed delete.
await expect(row).toBeVisible();
});
test('saves an item successfully when the server accepts it', async ({ page }) => {
const name = `Good ${suffix()}`;
await page.goto('/admin');
await page.getByRole('button', { name: 'Add Item' }).click();
await page.getByLabel('Name').fill(name);
await page.getByLabel('Price (USD)').fill('34');
await page.getByRole('button', { name: 'OK' }).click();
await expect(page.getByText('Item added')).toBeVisible();
// Confirm the row actually reached the database, not just that a toast fired.
const items = await (await page.request.get('/api/admin/items')).json();
expect(items.some((item: { name: string }) => item.name === name)).toBeTruthy();
});
});