refactor(backend): type the admin item queries, and fix the stale status union (#159)
admin.ts has no untyped reads left. Typed sites go from 43 to 49. New ItemRecord in itemSelect.ts for the bare `items` row that `RETURNING *` gives back. Deliberately not AdminItemRow: that describes a select which joins the category and adds images and tags as subqueries, so typing a RETURNING * as it would promise three fields the result does not contain. Three shapes for one table, because three different queries return three different things. The typing found a real defect on its first run, which is the case for doing this at all. `ItemStatus` in types.ts was `'available' | 'reserved' | 'sold'`. The database has four values and defaults to 'pending' — items have arrived pending since #90. itemFilters.ts declared its own copy that had all four and was correct. Two declarations of one union with nothing connecting them: one went stale and nothing said so. It was invisible while query rows were `any`. Typing them turned `if (status === 'pending')` in admin.ts into TS2367, "this comparison appears to be unintentional because the types 'ItemStatus' and '\"pending\"' have no overlap" — a compiler telling us the unpublish route's guard could never be true, against a type that was simply wrong. Confirmed against the database rather than by picking the more plausible of the two declarations: `SELECT DISTINCT status FROM items` returns pending, available, reserved and sold. Fixed by removing the duplication rather than by patching both copies. types.ts now holds the only declaration and itemFilters.ts imports it, re-exporting so its existing importers are unaffected. Patching both would have left the next drift free to happen the same way. Verified: tsc clean, unit 254/254, integration 238/238, and backend lint unchanged — the four warnings it reports are identical to those on main with these changes stashed, so none of them are new. Refs #159
This commit is contained in:
@@ -4,7 +4,8 @@ import { promises as fs } from 'fs';
|
||||
import { randomUUID } from 'crypto';
|
||||
import { PoolClient } from 'pg';
|
||||
import { pool } from '../db';
|
||||
import { ADMIN_ITEM_SELECT, AdminItemRow } from '../itemSelect';
|
||||
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';
|
||||
@@ -19,6 +20,16 @@ import { notifyFavoritersOfSale, notifyFavoritersOfRemoval, collectFavoriteRecip
|
||||
|
||||
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
|
||||
@@ -255,7 +266,7 @@ router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Respons
|
||||
const client = await pool.connect();
|
||||
try {
|
||||
await client.query('BEGIN');
|
||||
const { rows } = await client.query(
|
||||
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]
|
||||
);
|
||||
@@ -310,7 +321,7 @@ router.put('/items/:id', uploadImages, asyncRoute(async (req: Request, res: Resp
|
||||
await setItemTags(client, Number(req.params.id), await resolveTagIds(client, tagNames));
|
||||
}
|
||||
if (files.length) {
|
||||
const { rows: existing } = await client.query(
|
||||
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]
|
||||
);
|
||||
@@ -356,7 +367,7 @@ router.delete('/items/:id/images/:imageId', asyncRoute(async (req: Request, res:
|
||||
}));
|
||||
|
||||
router.post('/items/:id/mark-sold', asyncRoute(async (req: Request, res: Response) => {
|
||||
const { rows } = await pool.query(
|
||||
const { rows } = await pool.query<ItemRecord>(
|
||||
`UPDATE items SET status='sold', sold_at=now() WHERE id=$1 RETURNING *`,
|
||||
[req.params.id]
|
||||
);
|
||||
@@ -372,7 +383,7 @@ router.post('/items/:id/mark-sold', asyncRoute(async (req: Request, res: Respons
|
||||
// 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(`SELECT status FROM items WHERE id = $1`, [req.params.id]);
|
||||
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' });
|
||||
}
|
||||
@@ -393,7 +404,7 @@ router.post('/items/:id/unpublish', asyncRoute(async (req: Request, res: Respons
|
||||
return res.status(400).json({ error: 'a sold item cannot be unpublished' });
|
||||
}
|
||||
|
||||
const { rows: updated } = await pool.query(
|
||||
const { rows: updated } = await pool.query<ItemRecord>(
|
||||
`UPDATE items SET status='pending' WHERE id=$1 RETURNING *`,
|
||||
[req.params.id]
|
||||
);
|
||||
@@ -401,7 +412,7 @@ router.post('/items/:id/unpublish', asyncRoute(async (req: Request, res: Respons
|
||||
}));
|
||||
|
||||
router.post('/items/:id/mark-available', asyncRoute(async (req: Request, res: Response) => {
|
||||
const { rows } = await pool.query(
|
||||
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]
|
||||
|
||||
Reference in New Issue
Block a user