refactor(backend): type the item query results (#159)
First stage of typing the query results, and the one that sets the pattern. `pg` types `rows` as `any[]`, so every row this application reads entered a strict codebase as `any` — 1 of roughly 184 query sites carried a type before this. The row types live in itemSelect.ts, beside the selects that produce them, rather than in types.ts. They describe a projection rather than a table, and the two projections differ on purpose: ADMIN_ITEM_SELECT takes `i.*` while PUBLIC_ITEM_SELECT names its columns so the storefront never sees paypal_order_id or reserved_until. Typing both as "an items row" would quietly re-admit exactly the columns that select was written to exclude, so PublicItemRow and AdminItemRow share a base and the admin one adds the three fields it is allowed. types.ts gains ItemTag, which the tags subquery has always built and nothing had named. What this buys, demonstrated rather than claimed: introducing `rows[0].price_cent` at a read site now fails the build with "Property 'price_cent' does not exist on type 'ItemRowBase'. Did you mean 'price_cents'?". Before this it compiled, returned undefined, and reached the customer as an empty price. What it does not buy is written into itemSelect.ts rather than left for the next reader to assume. `pool.query<T>` asserts a shape; it does not check the SQL, which TypeScript never reads. Dropping a column from a select without dropping it from its type compiles cleanly and every read goes on type-checking while being undefined at runtime. The selects and their types are kept in step by hand, and the integration suite is the only thing that catches them disagreeing, because it runs the real queries against a real schema. The acceptance criteria on #159 originally claimed the compiler would catch that; it will not, and the issue has been corrected. Verified: tsc clean, and the four integration suites that exercise these selects pass 87/87. Refs #159
This commit is contained in:
@@ -1,4 +1,18 @@
|
||||
// Shared item SELECT shapes for the public and admin routes.
|
||||
// Shared item SELECT shapes for the public and admin routes, and the row types
|
||||
// they return.
|
||||
//
|
||||
// The types live here rather than in types.ts because they describe a
|
||||
// projection, not a table. ADMIN_ITEM_SELECT takes `i.*` and PUBLIC_ITEM_SELECT
|
||||
// names its columns so the storefront never sees paypal_order_id or
|
||||
// reserved_until — typing both as "an items row" would quietly re-admit exactly
|
||||
// the columns that select was written to exclude.
|
||||
//
|
||||
// KEPT IN STEP BY HAND. `pool.query<T>` asserts a shape; it does not check the
|
||||
// SQL, which TypeScript never reads. Dropping a column from a select below
|
||||
// without dropping it from its type compiles cleanly and every read of it goes
|
||||
// on type-checking while being undefined at runtime. The integration suite is
|
||||
// the only thing that catches that, because it runs these queries against a
|
||||
// real schema. Change a select and its type together.
|
||||
//
|
||||
// Images and tags are pulled as scalar subqueries rather than LEFT JOIN +
|
||||
// GROUP BY. Joining two one-to-many relations in the same query multiplies
|
||||
@@ -6,6 +20,8 @@
|
||||
// rows, silently repeating every image three times. Subqueries keep each
|
||||
// aggregate independent and drop the GROUP BY entirely.
|
||||
|
||||
import { ItemStatus, ItemImage, ItemTag } from './types';
|
||||
|
||||
const IMAGES_SUBQUERY = `
|
||||
COALESCE((
|
||||
SELECT json_agg(json_build_object('id', img.id, 'image_path', img.image_path, 'sort_order', img.sort_order)
|
||||
@@ -41,3 +57,33 @@ export const ADMIN_ITEM_SELECT = `
|
||||
${IMAGES_SUBQUERY},
|
||||
${TAGS_SUBQUERY}
|
||||
${FROM_CLAUSE}`;
|
||||
|
||||
/** The columns every item select returns, whichever of the two it is. */
|
||||
interface ItemRowBase {
|
||||
id: number;
|
||||
name: string;
|
||||
description: string | null;
|
||||
price_cents: number;
|
||||
status: ItemStatus;
|
||||
created_at: Date;
|
||||
category_id: number | null;
|
||||
category_name: string | null;
|
||||
// json_agg with a COALESCE fallback, so these are always arrays and never null.
|
||||
images: ItemImage[];
|
||||
tags: ItemTag[];
|
||||
}
|
||||
|
||||
/** What PUBLIC_ITEM_SELECT returns. Deliberately no payment or reservation columns. */
|
||||
export type PublicItemRow = ItemRowBase;
|
||||
|
||||
/**
|
||||
* What ADMIN_ITEM_SELECT returns: `i.*`, so every column on the table.
|
||||
*
|
||||
* The extra fields are the ones the storefront is not allowed to see, which is
|
||||
* the whole reason the two selects differ.
|
||||
*/
|
||||
export interface AdminItemRow extends ItemRowBase {
|
||||
reserved_until: Date | null;
|
||||
sold_at: Date | null;
|
||||
paypal_order_id: string | null;
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@ import { promises as fs } from 'fs';
|
||||
import { randomUUID } from 'crypto';
|
||||
import { PoolClient } from 'pg';
|
||||
import { pool } from '../db';
|
||||
import { ADMIN_ITEM_SELECT } from '../itemSelect';
|
||||
import { ADMIN_ITEM_SELECT, AdminItemRow } from '../itemSelect';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters';
|
||||
import { tagColorFor } from '../utils';
|
||||
@@ -235,7 +235,7 @@ router.get('/items', asyncRoute(async (req: Request, res: Response) => {
|
||||
|
||||
const { clauses, params } = buildItemFilterSql(filters, 1, null);
|
||||
const where = clauses.length ? `WHERE ${clauses.join(' AND ')}` : '';
|
||||
const { rows } = await pool.query(`${ADMIN_ITEM_SELECT} ${where} ORDER BY i.created_at DESC`, params);
|
||||
const { rows } = await pool.query<AdminItemRow>(`${ADMIN_ITEM_SELECT} ${where} ORDER BY i.created_at DESC`, params);
|
||||
res.json(rows);
|
||||
}));
|
||||
|
||||
@@ -270,7 +270,7 @@ router.post('/items', uploadImages, asyncRoute(async (req: Request, res: Respons
|
||||
await setItemTags(client, item.id, await resolveTagIds(client, tagNames));
|
||||
}
|
||||
await client.query('COMMIT');
|
||||
const { rows: full } = await pool.query(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [item.id]);
|
||||
const { rows: full } = await pool.query<AdminItemRow>(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [item.id]);
|
||||
res.json(full[0]);
|
||||
} catch (err) {
|
||||
await client.query('ROLLBACK');
|
||||
@@ -323,7 +323,7 @@ router.put('/items/:id', uploadImages, asyncRoute(async (req: Request, res: Resp
|
||||
}
|
||||
}
|
||||
await client.query('COMMIT');
|
||||
const { rows: full } = await pool.query(`${ADMIN_ITEM_SELECT} WHERE i.id = $1`, [req.params.id]);
|
||||
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');
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { Router, Request, Response } from 'express';
|
||||
import { pool } from '../db';
|
||||
import { asyncRoute } from '../asyncRoute';
|
||||
import { PUBLIC_ITEM_SELECT } from '../itemSelect';
|
||||
import { PUBLIC_ITEM_SELECT, PublicItemRow } from '../itemSelect';
|
||||
import {
|
||||
parseItemFilters,
|
||||
buildItemFilterSql,
|
||||
@@ -74,7 +74,7 @@ router.get('/', asyncRoute(async (req: Request, res: Response) => {
|
||||
|
||||
const { clauses, params } = buildItemFilterSql(effectiveFilters, 1, req.customerId ?? null);
|
||||
const where = [EXCLUDE_PENDING, ...clauses].join(' AND ');
|
||||
const { rows } = await pool.query(
|
||||
const { rows } = await pool.query<PublicItemRow>(
|
||||
`${PUBLIC_ITEM_SELECT} WHERE ${where} ORDER BY i.created_at DESC`,
|
||||
params
|
||||
);
|
||||
@@ -85,7 +85,7 @@ router.get('/:id', asyncRoute(async (req: Request, res: Response) => {
|
||||
// Excluded here too, not only from the list. A pending item that stayed
|
||||
// fetchable by id would be hidden from the catalogue and still reachable by
|
||||
// anyone who guessed or kept a link.
|
||||
const { rows } = await pool.query(
|
||||
const { rows } = await pool.query<PublicItemRow>(
|
||||
`${PUBLIC_ITEM_SELECT} WHERE i.id = $1 AND ${EXCLUDE_PENDING}`,
|
||||
[req.params.id]
|
||||
);
|
||||
|
||||
@@ -6,6 +6,12 @@ export interface ItemImage {
|
||||
sort_order: number;
|
||||
}
|
||||
|
||||
export interface ItemTag {
|
||||
id: number;
|
||||
name: string;
|
||||
color: string;
|
||||
}
|
||||
|
||||
export interface Item {
|
||||
id: number;
|
||||
name: string;
|
||||
|
||||
Reference in New Issue
Block a user