diff --git a/backend/src/db-kysely/CONVENTIONS.md b/backend/src/db-kysely/CONVENTIONS.md index 891ae45..ae4fc60 100644 --- a/backend/src/db-kysely/CONVENTIONS.md +++ b/backend/src/db-kysely/CONVENTIONS.md @@ -37,7 +37,7 @@ Kysely uses the `pg` driver directly, so a Postgres SQLSTATE stays on `err.code` ## The worked example -`buildItemFilterSql` is the hardest query in the codebase — six optional clauses composed at run time, a recursive CTE for the category subtree, an `ANY(...::int[])` tag match with a count equality, and array parameters. It was the #216 spike's target and #297's, and it is still raw `pg` in `src/itemFilters.ts`, used by both the storefront and the admin listing. The conversion below is an example rather than a source file: committing it as one would put a second copy of a live function in `src/` that nothing calls, which is precisely what the #216 spike became. +`buildItemFilterSql` was the hardest query in the codebase — six optional clauses composed at run time, a recursive CTE for the category subtree, an `ANY(...::int[])` tag match with a count equality, and array parameters. It was the #216 spike's target and #297's, and #308 converted it: it is now `itemFilterExpressions` in `src/itemFilters.ts`, returning Kysely expressions that the storefront and admin listings compose with `eb.and`. What follows is the shape it took, kept here because it is the worked reference for converting anything else of that difficulty. ```ts if (filters.categoryIds.length) { diff --git a/backend/src/itemSelect.ts b/backend/src/itemSelect.ts index ab5d487..ab24510 100644 --- a/backend/src/itemSelect.ts +++ b/backend/src/itemSelect.ts @@ -21,12 +21,27 @@ // aggregate independent and drop the GROUP BY entirely. `jsonArrayFrom` emits // `coalesce(json_agg(agg), '[]')`, which is what these hand-wrote before. -import { ExpressionBuilder } from 'kysely'; +import { ExpressionBuilder, Generated, Kysely } from 'kysely'; import { jsonArrayFrom } from 'kysely/helpers/postgres'; import { db } from './db'; import { DB } from './db-kysely/schema'; import { ItemStatus, ItemImage, ItemTag } from './types'; +/** + * The mirror types `items.status` as `Generated` because Postgres holds + * it as CHECK-constrained text rather than a native enum, so `kysely-codegen` + * has nothing narrower to emit. `types.ts` already states the real domain. + * + * Narrowed here, once, rather than asserted at each call site. `$castTo` at the + * call site would have replaced the entire row type with an assertion — which + * would silently accept a projection that had lost a column, and losing the + * compile error on exactly that is what this change exists to prevent. + */ +type ItemsWithStatus = Omit & { status: Generated }; +type ItemDB = Omit & { items: ItemsWithStatus }; + +const itemDb = db as unknown as Kysely; + /** * The aliases every item query and every filter clause is written against. * @@ -36,7 +51,7 @@ import { ItemStatus, ItemImage, ItemTag } from './types'; * the builder would have made the diff unreadable against the SQL it replaces. */ export type ItemContext = ExpressionBuilder< - DB & { i: DB['items']; c: DB['categories'] }, + ItemDB & { i: ItemDB['items']; c: ItemDB['categories'] }, 'i' | 'c' >; @@ -92,7 +107,7 @@ function tagsFor(eb: ItemContext) { * obvious that adding a `where` does not affect anyone else. */ export function publicItemQuery() { - return db + return itemDb .selectFrom('items as i') .leftJoin('categories as c', 'c.id', 'i.category_id') .select([ @@ -111,7 +126,7 @@ export function publicItemQuery() { /** The admin projection — every item column, plus the admin image fields. */ export function adminItemQuery() { - return db + return itemDb .selectFrom('items as i') .leftJoin('categories as c', 'c.id', 'i.category_id') .selectAll('i') @@ -135,7 +150,7 @@ interface ItemRowBase { tags: ItemTag[]; } -/** What PUBLIC_ITEM_SELECT returns. Deliberately no payment or reservation columns. */ +/** What publicItemQuery returns. Deliberately no payment or reservation columns. */ export type PublicItemRow = ItemRowBase; /** @@ -148,7 +163,7 @@ export interface AdminItemImage extends ItemImage { } /** - * What ADMIN_ITEM_SELECT returns: `i.*`, so every column on the table. + * What adminItemQuery 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. `images` is narrowed rather than diff --git a/backend/src/routes/admin.ts b/backend/src/routes/admin.ts index 7f416c0..d29b20e 100755 --- a/backend/src/routes/admin.ts +++ b/backend/src/routes/admin.ts @@ -137,15 +137,9 @@ router.get('/items', asyncRoute(async (req: Request, res: Response) => { // sixteen lines in itemFilters.ts explained why that was safe. The clauses // are Kysely expressions now: a value cannot reach the SQL text, because the // types do not let it. - // `.$castTo` narrows `status` from the schema mirror's generic `string` (the - // column is a CHECK-constrained text column, not a native Postgres enum, so - // kysely-codegen has no literal union to give it) to the app-level - // `ItemStatus` the CHECK constraint actually enforces. Every other field on - // AdminItemRow already matches the projection without a cast. const rows: AdminItemRow[] = await adminItemQuery() .where((eb) => eb.and(itemFilterExpressions(eb, filters, null))) .orderBy('i.created_at', 'desc') - .$castTo() .execute(); res.json(rows); diff --git a/backend/src/routes/items.ts b/backend/src/routes/items.ts index 5e1e8c4..c94064b 100755 --- a/backend/src/routes/items.ts +++ b/backend/src/routes/items.ts @@ -79,11 +79,6 @@ router.get('/', asyncRoute(async (req: Request, res: Response) => { status: filters.status ?? [...defaultStatuses] }; - // `.$castTo` narrows `status` from the schema mirror's generic `string` (the - // column is a CHECK-constrained text column, not a native Postgres enum, so - // kysely-codegen has no literal union to give it) to the app-level - // `ItemStatus` the CHECK constraint actually enforces. Every other field on - // PublicItemRow already matches the projection without a cast. const rows: PublicItemRow[] = await publicItemQuery() .where((eb) => eb.and([ @@ -92,7 +87,6 @@ router.get('/', asyncRoute(async (req: Request, res: Response) => { ]) ) .orderBy('i.created_at', 'desc') - .$castTo() .execute(); res.json(rows);