diff --git a/backend/src/itemFilters.ts b/backend/src/itemFilters.ts index 25ae2e7..0965610 100644 --- a/backend/src/itemFilters.ts +++ b/backend/src/itemFilters.ts @@ -81,26 +81,67 @@ function parsePrice(value: unknown, name: string): number | null { return parseNonNegativeInteger(raw, name); } -export function parseItemFilters(query: Record): ItemFilters { - const categoryRaw = singleValue(query.category, 'category'); - const categoryId = categoryRaw === null || categoryRaw === '' ? null : parseId(categoryRaw, 'category'); +function parseCategoryId(value: unknown): number | null { + const raw = singleValue(value, 'category'); + if (raw === null || raw === '') { + return null; + } + return parseId(raw, 'category'); +} - const tagsRaw = singleValue(query.tags, 'tags'); +function parseTagIds(value: unknown): number[] { + const raw = singleValue(value, 'tags'); const tagIds: number[] = []; - if (tagsRaw) { - for (const part of tagsRaw.split(',')) { - const trimmed = part.trim(); - if (trimmed === '') { - continue; - } - const id = parseId(trimmed, 'tags'); - // Duplicates would inflate the required-match count below and make the - // filter match nothing at all. - if (!tagIds.includes(id)) { - tagIds.push(id); - } + if (!raw) { + return tagIds; + } + for (const part of raw.split(',')) { + const trimmed = part.trim(); + if (trimmed === '') { + continue; + } + const id = parseId(trimmed, 'tags'); + // Duplicates would inflate the required-match count in buildItemFilterSql + // and make the filter match nothing at all. + if (!tagIds.includes(id)) { + tagIds.push(id); } } + return tagIds; +} + +function parseStatus(value: unknown): ItemStatus | null { + const raw = singleValue(value, 'status'); + if (raw === null || raw === '') { + return null; + } + if (!ITEM_STATUSES.includes(raw)) { + throw new FilterError('invalid status'); + } + return raw as ItemStatus; +} + +function parseFavoritesOnly(value: unknown): boolean { + const raw = singleValue(value, 'favorites'); + if (raw === null || raw === '') { + return false; + } + if (TRUE_VALUES.includes(raw)) { + return true; + } + if (FALSE_VALUES.includes(raw)) { + return false; + } + throw new FilterError('invalid favorites'); +} + +// The per-field parsing lives in the helpers above; what stays here is the +// order they run in and the one rule that spans two fields. Order is +// deliberate and observable: a query wrong in two ways reports the first +// field, so moving these lines around changes which error a caller sees. +export function parseItemFilters(query: Record): ItemFilters { + const categoryId = parseCategoryId(query.category); + const tagIds = parseTagIds(query.tags); const minPriceCents = parsePrice(query.min_price, 'min_price'); const maxPriceCents = parsePrice(query.max_price, 'max_price'); @@ -108,24 +149,8 @@ export function parseItemFilters(query: Record): ItemFilters { throw new FilterError('min_price may not exceed max_price'); } - const statusRaw = singleValue(query.status, 'status'); - let status: ItemStatus | null = null; - if (statusRaw !== null && statusRaw !== '') { - if (!ITEM_STATUSES.includes(statusRaw)) { - throw new FilterError('invalid status'); - } - status = statusRaw as ItemStatus; - } - - const favoritesRaw = singleValue(query.favorites, 'favorites'); - let favoritesOnly = false; - if (favoritesRaw !== null && favoritesRaw !== '') { - if (TRUE_VALUES.includes(favoritesRaw)) { - favoritesOnly = true; - } else if (!FALSE_VALUES.includes(favoritesRaw)) { - throw new FilterError('invalid favorites'); - } - } + const status = parseStatus(query.status); + const favoritesOnly = parseFavoritesOnly(query.favorites); return { categoryId, tagIds, minPriceCents, maxPriceCents, status, favoritesOnly }; } diff --git a/backend/src/routes/adminCategories.ts b/backend/src/routes/adminCategories.ts index 3852c9e..8b1e597 100644 --- a/backend/src/routes/adminCategories.ts +++ b/backend/src/routes/adminCategories.ts @@ -79,6 +79,45 @@ router.post('/', asyncRoute(async (req: Request, res: Response) => { } })); +// Works out what parent_id an update should land on. Absent means "leave it +// alone", so the current value is echoed back rather than treated as a clear. +// Returns the refusal instead of sending it, keeping the response the +// handler's business and the two ways a parent can be invalid out of its body. +type ParentResolution = { error: string } | { parent: number | null }; + +async function resolveParentId( + submitted: unknown, + id: number, + current: number | null +): Promise { + if (submitted === undefined) { + return { parent: current }; + } + + const parsed = readParentId(submitted); + if (parsed === undefined) { + return { error: 'invalid parent_id' }; + } + if (parsed === null) { + return { parent: null }; + } + if (!(await parentExists(parsed))) { + return { error: 'parent category does not exist' }; + } + + // Moving a node beneath itself or one of its own descendants would detach + // that whole branch from the tree into an unreachable cycle. + const { rows: cycle } = await pool.query( + `${SUBTREE_CTE} SELECT 1 FROM subtree WHERE id = $2`, + [id, parsed] + ); + if (cycle.length) { + return { error: 'a category cannot be moved beneath itself' }; + } + + return { parent: parsed }; +} + router.put('/:id', asyncRoute(async (req: Request, res: Response) => { const id = Number(req.params.id); const existing = await pool.query(`SELECT id, name, parent_id, sort_order FROM categories WHERE id = $1`, [id]); @@ -95,28 +134,11 @@ router.put('/:id', asyncRoute(async (req: Request, res: Response) => { name = parsed; } - let parent = existing.rows[0].parent_id; - if (req.body.parent_id !== undefined) { - const parsed = readParentId(req.body.parent_id); - if (parsed === undefined) { - return res.status(400).json({ error: 'invalid parent_id' }); - } - if (parsed !== null) { - if (!(await parentExists(parsed))) { - return res.status(400).json({ error: 'parent category does not exist' }); - } - // Moving a node beneath itself or one of its own descendants would - // detach that whole branch from the tree into an unreachable cycle. - const { rows: cycle } = await pool.query( - `${SUBTREE_CTE} SELECT 1 FROM subtree WHERE id = $2`, - [id, parsed] - ); - if (cycle.length) { - return res.status(400).json({ error: 'a category cannot be moved beneath itself' }); - } - } - parent = parsed; + const resolved = await resolveParentId(req.body.parent_id, id, existing.rows[0].parent_id); + if ('error' in resolved) { + return res.status(400).json({ error: resolved.error }); } + const parent = resolved.parent; const sortOrder = Number.isSafeInteger(req.body.sort_order) ? req.body.sort_order diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 55282f6..8d210d4 100755 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -26,6 +26,78 @@ const { Title } = Typography; // would become its own request. const FILTER_DEBOUNCE_MS = 250; +interface CatalogueProps { + failed: boolean; + loading: boolean; + items: Item[]; + filters: ItemFilters; + needsFavoritesAuth: boolean; + onRetry: () => void; + onSignIn: () => void; + onClearFilters: () => void; + onChanged: () => void; +} + +// The body of the catalogue: an outage, a sign-in prompt, an empty state, or +// the grid. Extracted from App so the four cases read as early returns rather +// than a ternary chain, and because a function's cognitive complexity counts +// everything nested inside it — leaving this inline is what put App over the +// limit. +function Catalogue({ + failed, + loading, + items, + filters, + needsFavoritesAuth, + onRetry, + onSignIn, + onClearFilters, + onChanged +}: CatalogueProps) { + if (failed) { + return ( + Retry} + /> + ); + } + + // Prompted rather than requested: see the effect in App that sets this. + if (needsFavoritesAuth) { + return ( + + + + + ); + } + + if (!loading && !items.length) { + // Distinguished so "no items match these filters" never reads as an empty + // shop, and so the way out is offered only when there is one. + const filtered = hasActiveFilters(filters); + return ( + + {filtered ? : null} + + ); + } + + return ( + + {items.map(item => ( + + + + ))} + + ); +} + export default function App() { const [items, setItems] = useState([]); const [loading, setLoading] = useState(true); @@ -108,6 +180,13 @@ export default function App() { fetchFilterOptions().then(setOptions).catch(() => undefined); }, [load]); + const handleRetry = useCallback(() => { + setLoading(true); + void load(); + }, [load]); + + const openAuthModal = useCallback(() => setAuthModalOpen(true), []); + const activeCount = activeFilterCount(filters); return ( @@ -172,38 +251,17 @@ export default function App() { {loading && !items.length && !failed ? : null} - {failed ? ( - { setLoading(true); void load(); }}>Retry} - /> - ) : needsFavoritesAuth ? ( - - - - - ) : !loading && !items.length ? ( - - {hasActiveFilters(filters) ? : null} - - ) : ( - - {items.map(item => ( - - - - ))} - - )} +