Files
redefined-designs/backend/src/routes/admin.ts
T
bermudalamb 61c12fd438
Linting / lint (pull_request) Successful in 1m58s
SonarQube Analysis / sonarqube (pull_request) Failing after 22m40s
refactor: remove the duplicated blocks SonarQube found (#182)
Three of the four candidates were real. The fourth was my mistake in the issue.

**The category tree adapter**, duplicated verbatim between `CategoryTreeSelect.tsx` and `FilterDrawer.tsx`. This one was mine: #139 moved the storefront filter to a `TreeSelect` and copied the admin's adapter rather than sharing it, with a comment saying the shape "matches the admin's CategoryTreeSelect so the two stay comparable" — an argument for one implementation that instead produced two. It now lives in `filters.ts` beside `buildCategoryTree`, which was already shared for exactly the same reason: one meaning, one implementation.

`Categories.tsx` keeps its own. It builds a different shape for a real antd `Tree`, keyed rather than valued, with a title that is a React node carrying that screen's buttons. Genuinely different, and folding it in would mean a parameterised adapter that serves neither case clearly.

**The `item_images` insert loop**, written separately by create and update and differing only in where the id came from and where the sort order started. Both are parameters now, which also means the `/uploads/` prefix is written once — #103 made that the value `uploadUrl` joins an origin onto, so it is a contract rather than a string.

Extracting it turned up two things the inline versions hid. Create indexed `files[i]?.filename ?? ''`, so a missing element would have stored a path pointing at the uploads directory itself; iterating by entry removes the possibility rather than defending against it. And the helper's typed `itemId` surfaced that `req.params.id` is `string | undefined` under `noUncheckedIndexedAccess`, which the old inline `unknown[]` swallowed — now `Number()`, as the `setItemTags` call two lines above already did.

**The optional-field guards**, eight identical lines opening both routes. The distinction worth preserving 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. That is what makes it more than a null check and worth stating once.

**`TAG_COLORS` was not a duplication.** The issue listed four files on the strength of a grep that also matched `STATUS_TAG_COLORS` in `Admin.tsx` — a status-to-colour map for the inventory table, unrelated to the tag palette. What remains is one definition in `backend/src/utils.ts` and one mirror in `frontend/src/admin/Tags.tsx`, already carrying a comment pointing at the other, which is the same treatment `ALLOWED_IMAGE_TYPES` gets and is correct: there is no shared package, and creating one for a colour list would cost more than it saves.

Verified beyond the type checker, since three of these are pure moves that compile either way: 278 unit and 254 integration tests, and the end-to-end specs covering both consumers of the shared adapter — the storefront drawer and the admin item form's category picker, including inline category creation.

Closes #182
2026-08-25 13:08:58 -05:00

505 lines
20 KiB
TypeScript
Executable File

import { Router, Request, Response, NextFunction } from 'express';
import multer from 'multer';
import { promises as fs } from 'fs';
import { randomUUID } from 'crypto';
import { PoolClient } from 'pg';
import { pool, requireRow } from '../db';
import { ADMIN_ITEM_SELECT, AdminItemRow, ItemRecord } from '../itemSelect';
import { ItemStatus } from '../types';
import { asyncRoute } from '../asyncRoute';
import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters';
import { tagColorFor } from '../utils';
import {
ALLOWED_IMAGE_TYPES,
SIGNATURE_BYTES,
extensionFor,
isAllowedImageType,
signatureMatches
} from '../uploadTypes';
import { notifyFavoritersOfSale, notifyFavoritersOfRemoval, collectFavoriteRecipients } from '../favoriteAlerts';
const router = Router();
/** The next image slot, from a COALESCE'd MAX so it is never null. */
interface MaxSortRow {
max_sort: number;
}
/** Just the status column, read before deciding whether a transition is legal. */
interface ItemStatusRow {
status: ItemStatus;
}
const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads';
// Multer writes to disk with no size cap unless one is given, so a single
// request could fill the uploads volume. Bound every dimension of the
// multipart body: image count, bytes per image, and the small text fields
// (name/description/price) that accompany them.
const MAX_IMAGES_PER_REQUEST = 6;
// 8 MB, not 8 MiB — this is the ceiling S5693 treats as safe, and 8 * 1024 *
// 1024 sits just over it. Plenty for a product photo either way.
const MAX_IMAGE_BYTES = 8_000_000;
const MAX_TEXT_FIELDS = 8;
const MAX_TEXT_FIELD_BYTES = 64 * 1024;
// Refused before a byte is written. This catches the honest mistake — picking a
// PDF by accident — and nothing more, because file.mimetype is whatever the
// caller wrote in the multipart headers. The bytes are checked after the write;
// see verifyUploadedImages.
class UnsupportedImageTypeError extends Error {}
const storage = multer.diskStorage({
destination: UPLOADS_DIR,
// Stored names come from a CSPRNG rather than a timestamp plus Math.random,
// which is predictable enough that a caller could guess (or collide with)
// another upload's path.
//
// The extension comes from the validated content type rather than from
// path.extname(file.originalname), so the name on disk cannot disagree with
// what the file claims to be — a caller cannot get `.html` onto the uploads
// volume by naming their file that way.
filename: (_req, file, cb) => {
const ext = extensionFor(file.mimetype);
if (!ext) {
// Unreachable while fileFilter runs first, and here so that it stays
// unreachable rather than silently writing a file with no extension.
cb(new UnsupportedImageTypeError(`unsupported image type ${file.mimetype}`), '');
return;
}
cb(null, `${randomUUID()}${ext}`);
}
});
// Reviewed for #180. Bounding one request is only half the problem — see
// discardUnlessAccepted below for the other half, which is bounding what the
// volume accumulates across requests that were refused.
const upload = multer({
storage,
limits: {
fileSize: MAX_IMAGE_BYTES,
files: MAX_IMAGES_PER_REQUEST,
fields: MAX_TEXT_FIELDS,
fieldSize: MAX_TEXT_FIELD_BYTES
},
fileFilter: (_req, file, cb) => {
if (!isAllowedImageType(file.mimetype)) {
cb(new UnsupportedImageTypeError(
`${file.mimetype} is not an accepted image type — allowed: ${ALLOWED_IMAGE_TYPES.join(', ')}`
));
return;
}
cb(null, true);
}
});
// Reads only the leading bytes — enough to identify a format, not enough to
// care how large the file is. The handle is closed before anything is unlinked,
// because an open handle makes the unlink fail on Windows.
//
// Reviewed for #180. The path is not caller-controlled despite arriving from a
// request: multer composes it from `destination`, which is a server constant,
// and `filename`, which the storage above sets to `randomUUID()` plus an
// extension looked up from the validated content type. The caller's
// `originalname` is never consulted, so no part of the path traverses anywhere.
async function readHead(filePath: string): Promise<Buffer> {
const handle = await fs.open(filePath, 'r');
try {
const buffer = Buffer.alloc(SIGNATURE_BYTES);
const { bytesRead } = await handle.read(buffer, 0, SIGNATURE_BYTES, 0);
return buffer.subarray(0, bytesRead);
} finally {
await handle.close();
}
}
// Best effort: a file that cannot be removed should not turn a 400 into a 500,
// but it must not be left behind quietly either.
async function discardUploads(files: Express.Multer.File[]): Promise<void> {
await Promise.all(
files.map((file) =>
fs.unlink(file.path).catch((err: unknown) => {
console.error(`[upload] could not remove rejected file ${file.path}:`, err);
})
)
);
}
/**
* Removes a request's uploaded files unless the request actually succeeded.
*
* multer writes to disk before any route logic runs, and its own cleanup only
* covers errors it raised itself. Everything after that — a failed signature
* check, a malformed `category_id`, a database error, a dropped connection —
* previously left the bytes on the volume with nothing referencing them: no row
* to find them by, and no bound on how many could accumulate. Bounding the size
* of one upload does not help if every refused upload is kept forever (#180).
*
* Registered as soon as multer succeeds rather than at each `return`, so a
* route added later inherits it instead of having to remember it. That is the
* whole reason it is a hook and not a call: the failure it prevents is someone
* adding a fourth early return.
*
* `close` rather than `finish`, so an aborted connection is covered too, and
* `writableEnded` distinguishes a response that completed from one that never
* did — the latter is not a success however its status code reads.
*/
function discardUnlessAccepted(req: Request, res: Response): void {
res.on('close', () => {
if (res.writableEnded && res.statusCode < 400) return;
void discardUploads((req.files as Express.Multer.File[]) || []);
});
}
/**
* Confirms each stored file actually is what it was declared to be.
*
* This cannot happen in multer's fileFilter, which runs before the stream has
* been read — there are no bytes to look at yet. So the check runs after the
* write.
*
* Checking only: removing the files is discardUnlessAccepted's job, and doing
* it here as well would unlink twice and log an ENOENT for every refused
* upload. That also covers the case this function used to miss — `readHead`
* itself throwing, which returned no message and so cleaned up nothing.
*
* Returns the message to refuse with, or null when everything checks out.
*/
async function verifyUploadedImages(req: Request): Promise<string | null> {
const files = (req.files as Express.Multer.File[]) || [];
for (const file of files) {
const head = await readHead(file.path);
if (!signatureMatches(file.mimetype, head)) {
return `${file.originalname} does not contain ${file.mimetype} data`;
}
}
return null;
}
// No error-handling middleware is mounted on the app, so translate multer's
// limit errors here instead of letting them surface as a generic 500.
const uploadImages = (req: Request, res: Response, next: NextFunction) => {
upload.array('images', MAX_IMAGES_PER_REQUEST)(req, res, (err: unknown) => {
if (err instanceof UnsupportedImageTypeError) {
return res.status(400).json({ error: err.message });
}
if (err instanceof multer.MulterError) {
const status = err.code === 'LIMIT_FILE_SIZE' ? 413 : 400;
return res.status(status).json({ error: err.message });
}
if (err) {
return next(err);
}
// Every file is on disk by this point and multer will not clean up after
// itself again, so the bytes become this request's responsibility before
// anything else is allowed to fail.
discardUnlessAccepted(req, res);
verifyUploadedImages(req)
.then((problem) => {
if (problem) {
res.status(400).json({ error: problem });
return;
}
next();
})
.catch(next);
});
};
// The multipart body carries category_id and tags as text fields. An absent
// field means "leave as-is" on update, which is why these return undefined
// rather than null for a missing value.
function readCategoryId(value: unknown): number | null | undefined {
if (value === undefined) return undefined;
if (value === null || value === '') return null;
const parsed = Number(value);
if (!Number.isSafeInteger(parsed) || parsed < 1) return undefined;
return parsed;
}
function readTagNames(value: unknown): string[] | undefined {
if (value === undefined) return undefined;
if (typeof value !== 'string' || value.trim() === '') return [];
try {
const parsed = JSON.parse(value);
if (!Array.isArray(parsed)) return undefined;
return parsed.filter((name): name is string => typeof name === 'string');
} catch {
return undefined;
}
}
// Tags typed into the item form may not exist yet. Look each name up
// case-insensitively — matching the unique index — and create the missing ones
// inside the caller's transaction so a later failure rolls them back too.
async function resolveTagIds(client: PoolClient, names: string[]): Promise<number[]> {
const ids: number[] = [];
for (const raw of names) {
const name = raw.trim();
if (!name) continue;
const existing = await client.query(`SELECT id FROM tags WHERE lower(name) = lower($1)`, [name]);
if (existing.rows.length) {
if (!ids.includes(existing.rows[0].id)) ids.push(existing.rows[0].id);
continue;
}
const inserted = await client.query(
`INSERT INTO tags (name, color) VALUES ($1, $2) RETURNING id`,
[name, tagColorFor(name)]
);
ids.push(inserted.rows[0].id);
}
return ids;
}
// Tags are replaced wholesale rather than merged — the form submits the full
// set it wants, so removing a chip has to actually remove the row.
async function setItemTags(client: PoolClient, itemId: number, tagIds: number[]): Promise<void> {
await client.query(`DELETE FROM item_tags WHERE item_id = $1`, [itemId]);
for (const tagId of tagIds) {
await client.query(`INSERT INTO item_tags (item_id, tag_id) VALUES ($1, $2)`, [itemId, tagId]);
}
}
/**
* 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<void> {
// 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<string, unknown>): 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
// the Inventory tab surfaces Reserved.
let filters;
try {
filters = parseItemFilters(req.query as Record<string, unknown>);
} catch (err) {
if (err instanceof FilterError) {
return res.status(400).json({ error: err.message });
}
throw err;
}
// Favorites belong to a customer, and the admin inventory view is not
// browsing as one. Refused rather than ignored so the mistake is visible.
if (filters.favoritesOnly) {
return res.status(400).json({ error: 'favorites is not a valid inventory filter' });
}
const { clauses, params } = buildItemFilterSql(filters, 1, null);
const where = clauses.length ? `WHERE ${clauses.join(' AND ')}` : '';
const { rows } = await pool.query<AdminItemRow>(`${ADMIN_ITEM_SELECT} ${where} ORDER BY i.created_at DESC`, params);
res.json(rows);
}));
router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Response) => {
const { name, description, price } = req.body;
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();
try {
await client.query('BEGIN');
const { rows } = await client.query<ItemRecord>(
`INSERT INTO items (name, description, price_cents, category_id) VALUES ($1, $2, $3, $4) RETURNING *`,
[name, description, Math.round(parseFloat(price) * 100), categoryId ?? null]
);
const item = requireRow(rows, 'the item INSERT');
await insertItemImages(client, item.id, files, 0);
if (tagNames) {
await setItemTags(client, item.id, await resolveTagIds(client, tagNames));
}
await client.query('COMMIT');
const { rows: full } = await pool.query<AdminItemRow>(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [item.id]);
res.json(requireRow(full, 'the item just inserted'));
} catch (err) {
await client.query('ROLLBACK');
console.error(err);
res.status(500).json({ error: 'internal error' });
} finally {
client.release();
}
}));
router.put('/items/:id', uploadImages, asyncRoute(async (req: Request, res: Response) => {
const { name, description, price } = req.body;
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();
try {
await client.query('BEGIN');
await client.query(
`UPDATE items SET name=$1, description=$2, price_cents=$3 WHERE id=$4`,
[name, description, Math.round(parseFloat(price) * 100), req.params.id]
);
// Only touch the category when the field was actually submitted, so a
// caller that omits it doesn't silently uncategorize the item.
if (categoryId !== undefined) {
await client.query(`UPDATE items SET category_id=$1 WHERE id=$2`, [categoryId, req.params.id]);
}
if (tagNames) {
await setItemTags(client, Number(req.params.id), await resolveTagIds(client, tagNames));
}
if (files.length) {
const { rows: existing } = await client.query<MaxSortRow>(
`SELECT COALESCE(MAX(sort_order), -1) AS max_sort FROM item_images WHERE item_id = $1`,
[req.params.id]
);
// COALESCE'd MAX, so the aggregate always returns exactly one row.
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<AdminItemRow>(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [req.params.id]);
res.json(full[0]);
} catch (err) {
await client.query('ROLLBACK');
console.error(err);
res.status(500).json({ error: 'internal error' });
} finally {
client.release();
}
}));
router.delete('/items/:id', asyncRoute(async (req: Request, res: Response) => {
const itemId = Number(req.params.id);
// Collected before the delete: favorites cascade with the item, so after it
// is gone there is no record of who was watching. Restricted to unsold items
// because anyone watching a sold one has already been told it went.
const recipients = await collectFavoriteRecipients([itemId], null, true);
await pool.query(`DELETE FROM items WHERE id = $1`, [itemId]);
// Sent only once the delete has succeeded, so nobody hears about a withdrawal
// that did not happen.
await notifyFavoritersOfRemoval(recipients);
res.status(204).end();
}));
router.delete('/items/:id/images/:imageId', asyncRoute(async (req: Request, res: Response) => {
await pool.query(`DELETE FROM item_images WHERE id = $1 AND item_id = $2`, [req.params.imageId, req.params.id]);
res.status(204).end();
}));
router.post('/items/:id/mark-sold', asyncRoute(async (req: Request, res: Response) => {
const { rows } = await pool.query<ItemRecord>(
`UPDATE items SET status='sold', sold_at=now() WHERE id=$1 RETURNING *`,
[req.params.id]
);
// No buyer to exclude: an admin marking an item sold has no associated
// customer, so everyone watching it hears about it.
await notifyFavoritersOfSale([Number(req.params.id)], null);
res.json(rows[0]);
}));
// Publishing is the existing mark-available: it already sets status='available'
// and clears sold_at, reserved_until and paypal_order_id, all of which are
// no-ops on a pending item. A second endpoint running the same UPDATE would be
// duplication, so the admin UI labels that button "Publish" when the item is
// pending. This is the reverse, and it is not symmetrical — see the guard.
router.post('/items/:id/unpublish', asyncRoute(async (req: Request, res: Response) => {
const { rows } = await pool.query<ItemStatusRow>(`SELECT status FROM items WHERE id = $1`, [req.params.id]);
if (!rows.length) {
return res.status(404).json({ error: 'not found' });
}
const status = requireRow(rows, 'the item status lookup').status;
if (status === 'pending') {
return res.status(400).json({ error: 'this item is already pending' });
}
// Reserved and sold are not drafts. A reserved item is in someone's cart
// right now and hiding it would strand them mid-checkout; a sold item is a
// record of something that happened, and pulling it back would quietly
// rewrite that. Both are refused by name so the reason is on screen rather
// than left to be guessed from a generic error.
if (status === 'reserved') {
return res.status(400).json({ error: 'a customer is holding this item — it cannot be unpublished' });
}
if (status === 'sold') {
return res.status(400).json({ error: 'a sold item cannot be unpublished' });
}
const { rows: updated } = await pool.query<ItemRecord>(
`UPDATE items SET status='pending' WHERE id=$1 RETURNING *`,
[req.params.id]
);
res.json(updated[0]);
}));
router.post('/items/:id/mark-available', asyncRoute(async (req: Request, res: Response) => {
const { rows } = await pool.query<ItemRecord>(
`UPDATE items SET status='available', sold_at=NULL, reserved_until=NULL, paypal_order_id=NULL
WHERE id=$1 RETURNING *`,
[req.params.id]
);
res.json(rows[0]);
}));
export default router;