Files
redefined-designs/backend/src/itemFilters.ts
T
bermudalamb d43e2d5871
Linting / lint (pull_request) Successful in 2m11s
SonarQube Analysis / sonarqube (pull_request) Failing after 5m2s
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
2026-08-24 14:15:27 -05:00

293 lines
11 KiB
TypeScript

// Parsing and SQL construction for the storefront's category / tag / price
// filters. Kept apart from the route so the rules can be unit-tested without a
// database, and so items.ts stays a thin handler.
import { ItemStatus } from './types';
export type { ItemStatus };
export class FilterError extends Error {}
export interface ItemFilters {
categoryId: number | null;
tagIds: number[];
minPriceCents: number | null;
maxPriceCents: number | null;
// Several statuses rather than one, because the control this exists to serve
// is not a status filter. "Not sold" is available-or-reserved on the
// storefront and available-or-reserved-or-pending in the admin, so it cannot
// be expressed as equality against a single value. A single-status filter is
// still expressible: it arrives as a list of one, which is how the admin's
// old `?status=sold` keeps working unchanged.
//
// Null means the caller expressed no preference, which is distinct from
// asking for every status — the storefront turns the first into its default
// and the second into an explicit list.
status: ItemStatus[] | null;
// Storefront only: "just the items I have favorited". Which customer that
// means is not part of the parsed filter — it comes from the session at build
// time, so a query string can never name someone else's favorites.
favoritesOnly: boolean;
}
// Matched exactly, not case-insensitively: `items.status` only ever holds these
// lowercase values, so accepting 'Reserved' would quietly return nothing rather
// than reporting that the filter was wrong.
const ITEM_STATUSES: readonly string[] = ['pending', 'available', 'reserved', 'sold'];
// Storefront-invalid statuses. This parser is shared with the admin routes,
// where filtering by 'pending' is exactly the point, so the public routes have
// to refuse it themselves rather than the parser refusing it for everyone.
export const NON_PUBLIC_STATUSES: readonly ItemStatus[] = ['pending'];
// What the storefront lists when the caller expressed no preference. Named here
// rather than implied by the absence of a parameter, because the absence is now
// meaningful: before this change no status meant every status, and afterwards it
// means these two. Anything reading a shared link from before will get the new
// meaning, which is the accepted cost of the default changing.
export const STOREFRONT_DEFAULT_STATUSES: readonly ItemStatus[] = ['available', 'reserved'];
// What "All" can mean on the storefront, which is not all of them. Pending items
// are excluded from every public read unconditionally, so a filter labelled All
// must not promise the fourth — a label that delivers less than it says is the
// shape this codebase keeps designing against.
export const STOREFRONT_ALL_STATUSES: readonly ItemStatus[] = ['available', 'reserved', 'sold'];
export interface BuiltFilter {
clauses: string[];
params: unknown[];
}
// Deliberately excludes a leading sign and any decimal point: every filter
// value is a non-negative integer (an id, or a price in cents), so '-1' and
// '10.5' are caller mistakes worth surfacing rather than silently coercing.
const NON_NEGATIVE_INTEGER = /^\d+$/;
// Accepts both spellings because these URLs get hand-edited and shared, but
// nothing else: '?favorites=yes' is a mistake worth reporting rather than
// treating as either on or off.
const TRUE_VALUES: readonly string[] = ['1', 'true'];
const FALSE_VALUES: readonly string[] = ['0', 'false'];
function singleValue(value: unknown, name: string): string | null {
if (value === undefined || value === null) {
return null;
}
// Express parses `?category=1&category=2` into an array. Picking one silently
// would make a malformed link look like it worked, so refuse it instead.
if (Array.isArray(value)) {
throw new FilterError(`${name} may only be given once`);
}
if (typeof value !== 'string') {
throw new FilterError(`invalid ${name}`);
}
return value;
}
function parseNonNegativeInteger(raw: string, name: string): number {
if (!NON_NEGATIVE_INTEGER.test(raw)) {
throw new FilterError(`invalid ${name}`);
}
const parsed = Number(raw);
if (!Number.isSafeInteger(parsed)) {
throw new FilterError(`invalid ${name}`);
}
return parsed;
}
function parseId(raw: string, name: string): number {
const parsed = parseNonNegativeInteger(raw, name);
if (parsed < 1) {
throw new FilterError(`invalid ${name}`);
}
return parsed;
}
function parsePrice(value: unknown, name: string): number | null {
const raw = singleValue(value, name);
if (raw === null || raw === '') {
return null;
}
return parseNonNegativeInteger(raw, name);
}
function parseCategoryId(value: unknown): number | null {
const raw = singleValue(value, 'category');
if (raw === null || raw === '') {
return null;
}
return parseId(raw, 'category');
}
function parseTagIds(value: unknown): number[] {
const raw = singleValue(value, 'tags');
const tagIds: number[] = [];
if (!raw) {
return tagIds;
}
for (const part of raw.split(',')) {
const trimmed = part.trim();
if (trimmed === '') {
continue;
}
const id = parseId(trimmed, 'tags');
// Duplicates would inflate the required-match count in buildItemFilterSql
// and make the filter match nothing at all.
if (!tagIds.includes(id)) {
tagIds.push(id);
}
}
return tagIds;
}
// Comma-separated, matching how `tags` already works, so the two multi-value
// parameters in this parser read the same way in a URL.
//
// An unrecognised name is refused rather than dropped. Silently ignoring one
// would turn `?status=available,sold_out` into "available only" — narrower than
// what was asked for, and indistinguishable from a filter that worked.
function parseStatus(value: unknown): ItemStatus[] | null {
const raw = singleValue(value, 'status');
if (raw === null || raw === '') {
return null;
}
const statuses: ItemStatus[] = [];
for (const part of raw.split(',')) {
const trimmed = part.trim();
if (trimmed === '') {
continue;
}
if (!ITEM_STATUSES.includes(trimmed)) {
throw new FilterError('invalid status');
}
// Duplicates are harmless in `= ANY(...)`, but removing them keeps the
// parsed filter a faithful description of what was asked for.
if (!statuses.includes(trimmed as ItemStatus)) {
statuses.push(trimmed as ItemStatus);
}
}
// `?status=,,` asked for something and named nothing. Returning null would
// silently mean "no status filter", which on the storefront now means the
// default rather than everything — a different answer from the one requested.
if (statuses.length === 0) {
throw new FilterError('invalid status');
}
return statuses;
}
function parseFavoritesOnly(value: unknown): boolean {
const raw = singleValue(value, 'favorites');
if (raw === null || raw === '') {
return false;
}
if (TRUE_VALUES.includes(raw)) {
return true;
}
if (FALSE_VALUES.includes(raw)) {
return false;
}
throw new FilterError('invalid favorites');
}
// The per-field parsing lives in the helpers above; what stays here is the
// order they run in and the one rule that spans two fields. Order is
// deliberate and observable: a query wrong in two ways reports the first
// field, so moving these lines around changes which error a caller sees.
export function parseItemFilters(query: Record<string, unknown>): ItemFilters {
const categoryId = parseCategoryId(query.category);
const tagIds = parseTagIds(query.tags);
const minPriceCents = parsePrice(query.min_price, 'min_price');
const maxPriceCents = parsePrice(query.max_price, 'max_price');
if (minPriceCents !== null && maxPriceCents !== null && minPriceCents > maxPriceCents) {
throw new FilterError('min_price may not exceed max_price');
}
const status = parseStatus(query.status);
const favoritesOnly = parseFavoritesOnly(query.favorites);
return { categoryId, tagIds, minPriceCents, maxPriceCents, status, favoritesOnly };
}
// Returns WHERE fragments plus their parameters, with placeholders numbered
// from `startIndex` so the caller can splice these in after its own params.
//
// `favoritesCustomerId` is required rather than optional so a caller has to say
// whose favorites it means, even when it means nobody's. Both routes already
// reject a favorites filter they cannot satisfy, so reaching the throw below is
// a programming error — but it is here so that a future caller which forgets
// the guard fails loudly instead of quietly ignoring the filter and listing the
// whole catalogue.
export function buildItemFilterSql(
filters: ItemFilters,
startIndex: number,
favoritesCustomerId: number | null
): BuiltFilter {
const clauses: string[] = [];
const params: unknown[] = [];
let next = startIndex;
if (filters.categoryId !== null) {
params.push(filters.categoryId);
// Selecting a category means "and everything filed beneath it", so walk the
// tree down from the chosen node. A recursive CTE keeps the tree
// un-denormalized: reparenting stays a single UPDATE with no stored paths
// to rewrite.
clauses.push(`i.category_id IN (
WITH RECURSIVE subtree AS (
SELECT id FROM categories WHERE id = $${next}
UNION ALL
SELECT c.id FROM categories c JOIN subtree s ON c.parent_id = s.id
)
SELECT id FROM subtree
)`);
next++;
}
if (filters.tagIds.length) {
params.push(filters.tagIds, filters.tagIds.length);
// AND, not OR: the item must carry every selected tag. Matching with
// `tag_id = ANY(...)` alone would return items holding just one of them, so
// the count of matched rows has to equal the number requested.
clauses.push(
`(SELECT COUNT(*) FROM item_tags it
WHERE it.item_id = i.id AND it.tag_id = ANY($${next}::int[])) = $${next + 1}`
);
next += 2;
}
if (filters.minPriceCents !== null) {
params.push(filters.minPriceCents);
clauses.push(`i.price_cents >= $${next}`);
next++;
}
if (filters.maxPriceCents !== null) {
params.push(filters.maxPriceCents);
clauses.push(`i.price_cents <= $${next}`);
next++;
}
if (filters.status !== null) {
params.push(filters.status);
// ANY rather than equality, so one status and several use the same clause.
// The ::text[] cast is explicit because `status` is a text column and the
// driver would otherwise have to infer the array's element type.
clauses.push(`i.status = ANY($${next}::text[])`);
next++;
}
if (filters.favoritesOnly) {
if (favoritesCustomerId === null) {
throw new Error('favorites filter requires a customer id');
}
params.push(favoritesCustomerId);
// EXISTS rather than a join: an item is favorited by a customer at most
// once, but joining would still risk multiplying rows if that ever changed,
// and this reads as the membership test it is.
clauses.push(`EXISTS (SELECT 1 FROM favorites f WHERE f.item_id = i.id AND f.customer_id = $${next})`);
next++;
}
return { clauses, params };
}