feat(db): convert routes/adminCategories.ts to Drizzle (#218)

All nine sites, chosen because the file is awkward rather than easy: a recursive CTE consumed two ways, a correlated subquery, an array match, and two error paths keyed on a Postgres SQLSTATE. A file of plain CRUD would have produced a flattering number that does not generalise.

The hand-declared row interfaces are gone. CATEGORY_COLUMNS is written once and the row type is inferred from it, which closes the drift itemSelect.ts documents as "KEPT IN STEP BY HAND". That mapping has to be explicit rather than selecting the table: the mirror names columns in camelCase and this API answers in snake_case, so selecting the table directly would have silently changed the JSON contract the admin frontend reads, and no test asserting status codes would have caught it.

strict and noUncheckedIndexedAccess hold with no non-null assertions added. requireRow covers the RETURNING rows and the existing lookup destructures and branches, exactly as before.

Two bugs were introduced and caught by the integration suite, and both are worth recording because neither produced a type error.

Drizzle renders a column reference inside a `sql` template UNQUALIFIED. `${items.categoryId} = ${categories.id}` became `WHERE "category_id" = "id"`, which Postgres resolved against items on both sides — so the item count came back plausible and wrong rather than failing. That is worse than the documented array trap, which at least produces invalid SQL. The fragment is now literal text, which is honest since it binds no values.

And the driver's error code moved. Drizzle wraps errors, so the SQLSTATE that sat on err.code now sits on err.cause.code; the old check compiled, never matched, and turned two 409s into 500s. isUniqueViolation accepts both shapes.

inArray replaced the ANY(...::int[]) match and sidesteps the sql.param trap entirely — there is no template to forget it in, and the builder emits the placeholder list correctly by construction.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-02 09:03:11 -05:00
co-authored by Claude Opus 5
parent 667aeb4155
commit 3476afcd70
+125 -69
View File
@@ -1,35 +1,38 @@
import { Router, Request, Response } from 'express'; import { Router, Request, Response } from 'express';
import { pool, requireRow } from '../db'; import { eq, inArray, sql } from 'drizzle-orm';
import { db, requireRow } from '../db';
import { categories, items } from '../db-drizzle/schema';
import { asyncRoute } from '../asyncRoute'; import { asyncRoute } from '../asyncRoute';
interface CategoryRow { /**
id: number; * The first file converted to Drizzle (#218), chosen because it is awkward
name: string; * rather than because it is easy — nine sites including a recursive CTE and an
parent_id: number | null; * array match. See src/db-drizzle/CONVENTIONS.md.
sort_order: number; *
} * The pool is still available and most of the application still uses it. This
* is one file moving, not a cutover.
/** The tree adds a usage count, cast to int so it arrives as a number. */ */
interface CategoryListRow extends CategoryRow {
item_count: number;
}
interface IdRow {
id: number;
}
interface CountRow {
n: number;
}
/** /**
* `SELECT 1 ...`, used only for `.length`. The column has no name of its own — * The response shape, written once.
* Postgres calls it `?column?` — so the shape is an index signature rather than *
* a field, and nothing reads a value out of it. * The generated mirror names columns in camelCase — `parentId`, `sortOrder` —
* and this API answers in snake_case, which the admin frontend reads. So the
* mapping is explicit here rather than implicit anywhere: selecting the table
* directly would silently change the JSON contract, and no test that checks
* status codes would catch it.
*
* It also answers the question #218 asked. The row type is inferred from this
* object rather than hand-declared beside the query, so the interfaces that used
* to sit at the top of this file are gone and cannot drift from what is
* selected.
*/ */
interface ExistsProbe { const CATEGORY_COLUMNS = {
[column: string]: number; id: categories.id,
} name: categories.name,
parent_id: categories.parentId,
sort_order: categories.sortOrder
};
const router = Router(); const router = Router();
@@ -37,11 +40,35 @@ const router = Router();
// stop siblings sharing a name. // stop siblings sharing a name.
const UNIQUE_VIOLATION = '23505'; const UNIQUE_VIOLATION = '23505';
// Walks down from a node, collecting it and every descendant. Used both for /**
// cycle detection on reparent and for reporting the blast radius of a delete. * Whether a thrown error is that unique violation.
const SUBTREE_CTE = ` *
* Drizzle wraps driver errors, so the SQLSTATE that used to sit on `err.code`
* now sits on `err.cause.code`. The old check still compiled and simply never
* matched, turning two 409s into 500s — a conversion hazard with no type error
* and no failing build behind it, only two integration tests. Both shapes are
* accepted so this keeps working either side of a conversion. See #218.
*/
function isUniqueViolation(err: unknown): boolean {
const direct = (err as { code?: string }).code;
const wrapped = (err as { cause?: { code?: string } }).cause?.code;
return direct === UNIQUE_VIOLATION || wrapped === UNIQUE_VIOLATION;
}
/**
* Walks down from a node, collecting it and every descendant. Used both for
* cycle detection on reparent and for reporting the blast radius of a delete.
*
* Still a `sql` template. Drizzle has `$with()` for CTEs, but this one is
* recursive and is consumed in two different shapes, and expressing it through
* the builder bought nothing over the SQL that is already correct and reviewed.
* The important part is that `${id}` here is a bind parameter, not text — there
* is no way to spell string interpolation in this template by accident, which is
* the property the whole adoption is for.
*/
const subtreeOf = (id: number) => sql`
WITH RECURSIVE subtree AS ( WITH RECURSIVE subtree AS (
SELECT id FROM categories WHERE id = $1 SELECT id FROM categories WHERE id = ${id}
UNION ALL UNION ALL
SELECT c.id FROM categories c JOIN subtree s ON c.parent_id = s.id SELECT c.id FROM categories c JOIN subtree s ON c.parent_id = s.id
)`; )`;
@@ -62,17 +89,29 @@ function readParentId(value: unknown): number | null | undefined {
} }
async function parentExists(id: number): Promise<boolean> { async function parentExists(id: number): Promise<boolean> {
const { rows } = await pool.query<ExistsProbe>(`SELECT 1 FROM categories WHERE id = $1`, [id]); const rows = await db
.select({ id: categories.id })
.from(categories)
.where(eq(categories.id, id))
.limit(1);
return rows.length > 0; return rows.length > 0;
} }
router.get('/', asyncRoute(async (_req: Request, res: Response) => { router.get('/', asyncRoute(async (_req: Request, res: Response) => {
const { rows } = await pool.query<CategoryListRow>( const rows = await db
`SELECT c.id, c.name, c.parent_id, c.sort_order, .select({
(SELECT COUNT(*)::int FROM items i WHERE i.category_id = c.id) AS item_count ...CATEGORY_COLUMNS,
FROM categories c // Written as literal SQL, NOT with ${items.categoryId} and
ORDER BY c.sort_order, lower(c.name)` // ${categories.id}. Drizzle renders a column reference inside a sql
); // template UNQUALIFIED — those two produced `WHERE "category_id" = "id"`,
// which Postgres resolved against items for both sides and answered with
// a plausible wrong number rather than an error. There are no values to
// bind in this fragment, so literal text is the honest form. See #218.
item_count: sql<number>`(SELECT COUNT(*)::int FROM items WHERE items.category_id = categories.id)`
})
.from(categories)
.orderBy(categories.sortOrder, sql`lower(categories.name)`);
res.json(rows); res.json(rows);
})); }));
@@ -94,14 +133,14 @@ router.post('/', asyncRoute(async (req: Request, res: Response) => {
const sortOrder = Number.isSafeInteger(req.body.sort_order) ? req.body.sort_order : 0; const sortOrder = Number.isSafeInteger(req.body.sort_order) ? req.body.sort_order : 0;
try { try {
const { rows } = await pool.query<CategoryRow>( const rows = await db
`INSERT INTO categories (name, parent_id, sort_order) VALUES ($1, $2, $3) .insert(categories)
RETURNING id, name, parent_id, sort_order`, .values({ name, parentId: parent, sortOrder })
[name, parent, sortOrder] .returning(CATEGORY_COLUMNS);
);
res.status(201).json({ ...rows[0], item_count: 0 }); res.status(201).json({ ...requireRow(rows, 'the category INSERT'), item_count: 0 });
} catch (err) { } catch (err) {
if ((err as { code?: string }).code === UNIQUE_VIOLATION) { if (isUniqueViolation(err)) {
return res.status(409).json({ error: 'a category with that name already exists here' }); return res.status(409).json({ error: 'a category with that name already exists here' });
} }
throw err; throw err;
@@ -136,11 +175,10 @@ async function resolveParentId(
// Moving a node beneath itself or one of its own descendants would detach // Moving a node beneath itself or one of its own descendants would detach
// that whole branch from the tree into an unreachable cycle. // that whole branch from the tree into an unreachable cycle.
const { rows: cycle } = await pool.query<ExistsProbe>( const cycle = await db.execute(
`${SUBTREE_CTE} SELECT 1 FROM subtree WHERE id = $2`, sql`${subtreeOf(id)} SELECT 1 FROM subtree WHERE id = ${parsed}`
[id, parsed]
); );
if (cycle.length) { if (cycle.rows.length) {
return { error: 'a category cannot be moved beneath itself' }; return { error: 'a category cannot be moved beneath itself' };
} }
@@ -149,12 +187,18 @@ async function resolveParentId(
router.put('/:id', asyncRoute(async (req: Request, res: Response) => { router.put('/:id', asyncRoute(async (req: Request, res: Response) => {
const id = Number(req.params.id); const id = Number(req.params.id);
const existing = await pool.query(`SELECT id, name, parent_id, sort_order FROM categories WHERE id = $1`, [id]); const existing = await db
if (!existing.rows.length) { .select(CATEGORY_COLUMNS)
.from(categories)
.where(eq(categories.id, id))
.limit(1);
const current = existing[0];
if (!current) {
return res.status(404).json({ error: 'not found' }); return res.status(404).json({ error: 'not found' });
} }
let name = existing.rows[0].name; let name = current.name;
if (req.body.name !== undefined) { if (req.body.name !== undefined) {
const parsed = readName(req.body.name); const parsed = readName(req.body.name);
if (!parsed) { if (!parsed) {
@@ -163,7 +207,7 @@ router.put('/:id', asyncRoute(async (req: Request, res: Response) => {
name = parsed; name = parsed;
} }
const resolved = await resolveParentId(req.body.parent_id, id, existing.rows[0].parent_id); const resolved = await resolveParentId(req.body.parent_id, id, current.parent_id);
if ('error' in resolved) { if ('error' in resolved) {
return res.status(400).json({ error: resolved.error }); return res.status(400).json({ error: resolved.error });
} }
@@ -171,17 +215,18 @@ router.put('/:id', asyncRoute(async (req: Request, res: Response) => {
const sortOrder = Number.isSafeInteger(req.body.sort_order) const sortOrder = Number.isSafeInteger(req.body.sort_order)
? req.body.sort_order ? req.body.sort_order
: existing.rows[0].sort_order; : current.sort_order;
try { try {
const { rows } = await pool.query<CategoryRow>( const rows = await db
`UPDATE categories SET name = $1, parent_id = $2, sort_order = $3 WHERE id = $4 .update(categories)
RETURNING id, name, parent_id, sort_order`, .set({ name, parentId: parent, sortOrder })
[name, parent, sortOrder, id] .where(eq(categories.id, id))
); .returning(CATEGORY_COLUMNS);
res.json(rows[0]);
res.json(requireRow(rows, 'the category UPDATE'));
} catch (err) { } catch (err) {
if ((err as { code?: string }).code === UNIQUE_VIOLATION) { if (isUniqueViolation(err)) {
return res.status(409).json({ error: 'a category with that name already exists here' }); return res.status(409).json({ error: 'a category with that name already exists here' });
} }
throw err; throw err;
@@ -190,22 +235,33 @@ router.put('/:id', asyncRoute(async (req: Request, res: Response) => {
router.delete('/:id', asyncRoute(async (req: Request, res: Response) => { router.delete('/:id', asyncRoute(async (req: Request, res: Response) => {
const id = Number(req.params.id); const id = Number(req.params.id);
const { rows: subtree } = await pool.query<IdRow>(`${SUBTREE_CTE} SELECT id FROM subtree`, [id]);
if (!subtree.length) { const subtree = await db.execute<{ id: number }>(
sql`${subtreeOf(id)} SELECT id FROM subtree`
);
if (!subtree.rows.length) {
return res.status(404).json({ error: 'not found' }); return res.status(404).json({ error: 'not found' });
} }
const ids = subtree.map((row: { id: number }) => row.id); const ids = subtree.rows.map((row) => row.id);
const { rows: affected } = await pool.query<CountRow>(
`SELECT COUNT(*)::int AS n FROM items WHERE category_id = ANY($1::int[])`, // inArray rather than the ANY(...::int[]) this replaced, which sidesteps the
[ids] // array trap in CONVENTIONS.md entirely: there is no template to forget
); // sql.param() in. The builder emits the placeholder list itself and it is
// correct by construction.
const affected = await db
.select({ n: sql<number>`COUNT(*)::int` })
.from(items)
.where(inArray(items.categoryId, ids));
// The FK cascade takes the descendants; items fall back to NULL rather than // The FK cascade takes the descendants; items fall back to NULL rather than
// being deleted along with their category. // being deleted along with their category.
await pool.query(`DELETE FROM categories WHERE id = $1`, [id]); await db.delete(categories).where(eq(categories.id, id));
res.json({ deleted_categories: ids.length, uncategorized_items: requireRow(affected, 'the affected-items COUNT').n }); res.json({
deleted_categories: ids.length,
uncategorized_items: requireRow(affected, 'the affected-items COUNT').n
});
})); }));
export default router; export default router;