diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index dbe87bf..dd3cfd8 100755 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -266,6 +266,60 @@ async function setItemTags(client: PoolClient, itemId: number, tagIds: number[]) } } +/** + * Records uploaded files as an item's images. + * + * Create and update wrote this loop separately, differing only in where the id + * came from and where the sort order started — zero for a new item, one past + * the current maximum for an existing one. Both are parameters now. + * + * It also means the `/uploads/` prefix is written once. That matters more than + * it looks: #103 made the stored value the path `uploadUrl` joins an origin + * onto, so it is a contract rather than a string, and two places to change it + * is one place to forget. + */ +async function insertItemImages( + client: PoolClient, + itemId: number, + files: Express.Multer.File[], + firstSortOrder: number +): Promise { + // Iterated by entry rather than by index, so there is no possibly-undefined + // element to guard — the create path used to fall back to an empty filename, + // which would have stored a path pointing at the uploads directory itself. + for (const [offset, file] of files.entries()) { + await client.query( + `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, + [itemId, `/uploads/${file.filename}`, firstSortOrder + offset] + ); + } +} + +/** + * The two optional fields the item form submits as multipart text. + * + * Both routes parsed them and refused them identically, eight lines each. The + * distinction being preserved is that `undefined` means "not submitted", which + * update reads as "leave as-is" — so an unparseable value has to be told apart + * from an absent one, which is what makes this more than a null check and worth + * having in one place. + */ +type ParsedItemFields = + | { ok: true; categoryId: number | null | undefined; tagNames: string[] | undefined } + | { ok: false; error: string }; + +function readOptionalItemFields(body: Record): ParsedItemFields { + const categoryId = readCategoryId(body.category_id); + if (categoryId === undefined && body.category_id !== undefined) { + return { ok: false, error: 'invalid category_id' }; + } + const tagNames = readTagNames(body.tags); + if (tagNames === undefined && body.tags !== undefined) { + return { ok: false, error: 'invalid tags' }; + } + return { ok: true, categoryId, tagNames }; +} + router.get('/items', asyncRoute(async (req: Request, res: Response) => { // Same parser and query builder as the storefront, so admin filtering cannot // drift from what customers see. The one addition is `status`, which is how @@ -295,14 +349,9 @@ router.get('/items', asyncRoute(async (req: Request, res: Response) => { router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Response) => { const { name, description, price } = req.body; - const categoryId = readCategoryId(req.body.category_id); - if (categoryId === undefined && req.body.category_id !== undefined) { - return res.status(400).json({ error: 'invalid category_id' }); - } - const tagNames = readTagNames(req.body.tags); - if (tagNames === undefined && req.body.tags !== undefined) { - return res.status(400).json({ error: 'invalid tags' }); - } + const parsed = readOptionalItemFields(req.body); + if (!parsed.ok) return res.status(400).json({ error: parsed.error }); + const { categoryId, tagNames } = parsed; const files = (req.files as Express.Multer.File[]) || []; const client = await pool.connect(); @@ -313,12 +362,7 @@ router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Respons [name, description, Math.round(parseFloat(price) * 100), categoryId ?? null] ); const item = requireRow(rows, 'the item INSERT'); - for (let i = 0; i < files.length; i++) { - await client.query( - `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, - [item.id, `/uploads/${files[i]?.filename ?? ''}`, i] - ); - } + await insertItemImages(client, item.id, files, 0); if (tagNames) { await setItemTags(client, item.id, await resolveTagIds(client, tagNames)); } @@ -337,14 +381,9 @@ router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Respons router.put('/items/:id', uploadImages, asyncRoute(async (req: Request, res: Response) => { const { name, description, price } = req.body; - const categoryId = readCategoryId(req.body.category_id); - if (categoryId === undefined && req.body.category_id !== undefined) { - return res.status(400).json({ error: 'invalid category_id' }); - } - const tagNames = readTagNames(req.body.tags); - if (tagNames === undefined && req.body.tags !== undefined) { - return res.status(400).json({ error: 'invalid tags' }); - } + const parsed = readOptionalItemFields(req.body); + if (!parsed.ok) return res.status(400).json({ error: parsed.error }); + const { categoryId, tagNames } = parsed; const files = (req.files as Express.Multer.File[]) || []; const client = await pool.connect(); @@ -368,13 +407,12 @@ router.put('/items/:id', uploadImages, asyncRoute(async (req: Request, res: Resp [req.params.id] ); // COALESCE'd MAX, so the aggregate always returns exactly one row. - let nextSort = requireRow(existing, 'the MAX(sort_order) aggregate').max_sort + 1; - for (const file of files) { - await client.query( - `INSERT INTO item_images (item_id, image_path, sort_order) VALUES ($1, $2, $3)`, - [req.params.id, `/uploads/${file.filename}`, nextSort++] - ); - } + const nextSort = requireRow(existing, 'the MAX(sort_order) aggregate').max_sort + 1; + // Number(), as the setItemTags call above already does: a matched route + // always has this param, but noUncheckedIndexedAccess cannot know that, + // and the helper's typed parameter surfaces what the old inline query's + // unknown[] hid. + await insertItemImages(client, Number(req.params.id), files, nextSort); } await client.query('COMMIT'); const { rows: full } = await pool.query(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [req.params.id]); diff --git a/frontend/src/admin/CategoryTreeSelect.tsx b/frontend/src/admin/CategoryTreeSelect.tsx index 0586fc0..a1289c1 100644 --- a/frontend/src/admin/CategoryTreeSelect.tsx +++ b/frontend/src/admin/CategoryTreeSelect.tsx @@ -5,21 +5,7 @@ import Button from 'antd/es/button'; import Divider from 'antd/es/divider'; import message from 'antd/es/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 - })); -} +import { buildCategoryTree, toCategoryTreeData } from '../filters'; type Props = Readonly<{ // Supplied by antd's Form.Item. `id` has to be forwarded or the field loses @@ -40,7 +26,7 @@ export default function CategoryTreeSelect({ value, onChange, id, categories, on const [newCategoryName, setNewCategoryName] = useState(''); const [creating, setCreating] = useState(false); - const treeData = useMemo(() => toTreeData(buildCategoryTree(categories)), [categories]); + const treeData = useMemo(() => toCategoryTreeData(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 diff --git a/frontend/src/components/FilterDrawer.tsx b/frontend/src/components/FilterDrawer.tsx index efb7b03..84a2f03 100644 --- a/frontend/src/components/FilterDrawer.tsx +++ b/frontend/src/components/FilterDrawer.tsx @@ -9,7 +9,7 @@ import Empty from 'antd/es/empty'; import Switch from 'antd/es/switch'; import Grid from 'antd/es/grid'; import type { Category, Tag as ItemTag } from '../api'; -import { ItemFilters, ItemStatus, STATUS_OPTIONS, buildCategoryTree, CategoryNode } from '../filters'; +import { ItemFilters, ItemStatus, STATUS_OPTIONS, buildCategoryTree, toCategoryTreeData } from '../filters'; // One drawer for the storefront and the admin, with the sections that differ // driven by props rather than by a second component that would drift (#169). @@ -37,23 +37,6 @@ type Props = Readonly<{ showStatus?: boolean; }>; -// `value` rather than `key`: this fed an antd `Tree`, which identifies nodes by -// key, and now feeds a `TreeSelect`, which selects and searches by value. The -// shape matches the admin's CategoryTreeSelect so the two stay comparable. -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 - })); -} - const sectionHeading: React.CSSProperties = { margin: '0 0 8px', fontSize: 12, @@ -146,7 +129,7 @@ export default function FilterDrawer({ // read off highlighting. The admin's CategoryTreeSelect is the same // control, so the two screens behave alike. ({ + value: node.id, + title: node.name, + children: node.children.length ? toCategoryTreeData(node.children) : undefined + })); +} + // "Furniture / Tables / Coffee Tables" — used on chips and in the admin form so // a leaf name like "Vintage" isn't ambiguous between branches. export function categoryPath(categories: Category[], id: number): string {