refactor(backend): type the admin item queries, and fix the stale status union (#159) #164
@@ -2,6 +2,9 @@
|
|||||||
// filters. Kept apart from the route so the rules can be unit-tested without a
|
// filters. Kept apart from the route so the rules can be unit-tested without a
|
||||||
// database, and so items.ts stays a thin handler.
|
// database, and so items.ts stays a thin handler.
|
||||||
|
|
||||||
|
import { ItemStatus } from './types';
|
||||||
|
export type { ItemStatus };
|
||||||
|
|
||||||
export class FilterError extends Error {}
|
export class FilterError extends Error {}
|
||||||
|
|
||||||
export interface ItemFilters {
|
export interface ItemFilters {
|
||||||
@@ -26,8 +29,6 @@ export interface ItemFilters {
|
|||||||
favoritesOnly: boolean;
|
favoritesOnly: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export type ItemStatus = 'pending' | 'available' | 'reserved' | 'sold';
|
|
||||||
|
|
||||||
// Matched exactly, not case-insensitively: `items.status` only ever holds these
|
// Matched exactly, not case-insensitively: `items.status` only ever holds these
|
||||||
// lowercase values, so accepting 'Reserved' would quietly return nothing rather
|
// lowercase values, so accepting 'Reserved' would quietly return nothing rather
|
||||||
// than reporting that the filter was wrong.
|
// than reporting that the filter was wrong.
|
||||||
|
|||||||
@@ -87,3 +87,24 @@ export interface AdminItemRow extends ItemRowBase {
|
|||||||
sold_at: Date | null;
|
sold_at: Date | null;
|
||||||
paypal_order_id: string | null;
|
paypal_order_id: string | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A bare `items` row, as `RETURNING *` gives it back.
|
||||||
|
*
|
||||||
|
* Distinct from the two select rows above and not interchangeable with them:
|
||||||
|
* this is the table, so it has no category_name, no images and no tags. Those
|
||||||
|
* come from the joins and subqueries the selects add, and typing a RETURNING *
|
||||||
|
* as AdminItemRow would promise three fields that are not in the result.
|
||||||
|
*/
|
||||||
|
export interface ItemRecord {
|
||||||
|
id: number;
|
||||||
|
name: string;
|
||||||
|
description: string | null;
|
||||||
|
price_cents: number;
|
||||||
|
status: ItemStatus;
|
||||||
|
reserved_until: Date | null;
|
||||||
|
sold_at: Date | null;
|
||||||
|
paypal_order_id: string | null;
|
||||||
|
created_at: Date;
|
||||||
|
category_id: number | null;
|
||||||
|
}
|
||||||
|
|||||||
@@ -4,7 +4,8 @@ import { promises as fs } from 'fs';
|
|||||||
import { randomUUID } from 'crypto';
|
import { randomUUID } from 'crypto';
|
||||||
import { PoolClient } from 'pg';
|
import { PoolClient } from 'pg';
|
||||||
import { pool } from '../db';
|
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 { asyncRoute } from '../asyncRoute';
|
||||||
import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters';
|
import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters';
|
||||||
import { tagColorFor } from '../utils';
|
import { tagColorFor } from '../utils';
|
||||||
@@ -19,6 +20,16 @@ import { notifyFavoritersOfSale, notifyFavoritersOfRemoval, collectFavoriteRecip
|
|||||||
|
|
||||||
const router = Router();
|
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';
|
const UPLOADS_DIR = process.env.UPLOADS_DIR || '/app/uploads';
|
||||||
|
|
||||||
// Multer writes to disk with no size cap unless one is given, so a single
|
// 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();
|
const client = await pool.connect();
|
||||||
try {
|
try {
|
||||||
await client.query('BEGIN');
|
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 *`,
|
`INSERT INTO items (name, description, price_cents, category_id) VALUES ($1, $2, $3, $4) RETURNING *`,
|
||||||
[name, description, Math.round(parseFloat(price) * 100), categoryId ?? null]
|
[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));
|
await setItemTags(client, Number(req.params.id), await resolveTagIds(client, tagNames));
|
||||||
}
|
}
|
||||||
if (files.length) {
|
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`,
|
`SELECT COALESCE(MAX(sort_order), -1) AS max_sort FROM item_images WHERE item_id = $1`,
|
||||||
[req.params.id]
|
[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) => {
|
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 *`,
|
`UPDATE items SET status='sold', sold_at=now() WHERE id=$1 RETURNING *`,
|
||||||
[req.params.id]
|
[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
|
// 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.
|
// pending. This is the reverse, and it is not symmetrical — see the guard.
|
||||||
router.post('/items/:id/unpublish', asyncRoute(async (req: Request, res: Response) => {
|
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) {
|
if (!rows.length) {
|
||||||
return res.status(404).json({ error: 'not found' });
|
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' });
|
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 *`,
|
`UPDATE items SET status='pending' WHERE id=$1 RETURNING *`,
|
||||||
[req.params.id]
|
[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) => {
|
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
|
`UPDATE items SET status='available', sold_at=NULL, reserved_until=NULL, paypal_order_id=NULL
|
||||||
WHERE id=$1 RETURNING *`,
|
WHERE id=$1 RETURNING *`,
|
||||||
[req.params.id]
|
[req.params.id]
|
||||||
|
|||||||
+13
-1
@@ -1,4 +1,16 @@
|
|||||||
export type ItemStatus = 'available' | 'reserved' | 'sold';
|
/**
|
||||||
|
* Every value `items.status` can hold.
|
||||||
|
*
|
||||||
|
* 'pending' was missing here from the moment items started arriving pending,
|
||||||
|
* while itemFilters.ts declared its own copy that had it. Two declarations of
|
||||||
|
* one union is how that happens: nothing connects them, so one goes stale and
|
||||||
|
* nothing says so. The stale one was harmless only because query rows were
|
||||||
|
* `any` — typing them turned `status === 'pending'` in admin.ts into a compile
|
||||||
|
* error about a comparison with no overlap, which is how it was found.
|
||||||
|
*
|
||||||
|
* This is now the single declaration. itemFilters.ts imports it.
|
||||||
|
*/
|
||||||
|
export type ItemStatus = 'pending' | 'available' | 'reserved' | 'sold';
|
||||||
|
|
||||||
export interface ItemImage {
|
export interface ItemImage {
|
||||||
id: number;
|
id: number;
|
||||||
|
|||||||
Reference in New Issue
Block a user