refactor(backend): type the cart and checkout query results (#159)
Linting / lint (pull_request) Successful in 2m5s
SonarQube Analysis / sonarqube (pull_request) Failing after 4m49s

The transaction paths, taken before the larger files because this is where `any` is most expensive: these are the queries that lock rows, move money and mark items sold, and where a mistyped field reaches a customer as a wrong price rather than a broken page.

Typed query sites go from 6 to 22.

Only queries whose rows are actually read carry a type. The INSERTs, UPDATEs, DELETEs and the BEGIN/COMMIT/ROLLBACK calls return nothing anyone looks at, and annotating them would be ceremony that makes the ones that matter harder to pick out. The convention is stated once in each file rather than implied, since #159's acceptance criteria say every call is typed "or explicitly exempted with a reason" and this is that reason.

Two hand-written annotations are gone as a direct consequence. `items.reduce((sum: number, it: CartItem) => …)` and `checkoutItems.map((ci: { item_id: number }) => …)` existed only because `rows` was `any` and inference had nothing to work from. With the query typed, both infer, and the second one is the more interesting of the two: it was a structural type written inline that duplicated the real row shape and could have drifted from it silently.

CART_ITEM_SELECT's type records something the SQL states and no reader would otherwise know: the images aggregate selects only id and image_path, so it is `Pick<ItemImage, 'id' | 'image_path'>[]` rather than `ItemImage[]`. Typing it as the full shape would have promised a sort_order that is not in the projection.

The same hand-kept caveat as the item selects applies and is written into both files: `query<T>` asserts a shape rather than checking it, because TypeScript never reads the SQL. The integration suite is what catches a select and its type disagreeing.

Verified: tsc clean, and the suites covering these paths pass — cart, favorites and adminInventory 53/53, then cart and soldFilter 19/19.

Refs #159
This commit is contained in:
2026-08-24 13:07:47 -05:00
parent d99cf28e18
commit 72c49719fc
2 changed files with 77 additions and 16 deletions
+38 -6
View File
@@ -3,9 +3,41 @@ import { pool } from '../db';
import { asyncRoute } from '../asyncRoute'; import { asyncRoute } from '../asyncRoute';
import { requireCustomer } from '../middleware/customerAuth'; import { requireCustomer } from '../middleware/customerAuth';
import { getSettings } from '../adminSettings'; import { getSettings } from '../adminSettings';
import { ItemStatus, ItemImage } from '../types';
const router = Router(); const router = Router();
/**
* Row shapes for the reads here. As in cartCheckout.ts, only queries whose rows
* are read carry a type, and each is kept in step with its SQL by hand.
*/
interface IdRow {
id: number;
}
/** What CART_ITEM_SELECT returns — a held item as the cart page renders it. */
interface CartRow {
item_id: number;
added_at: Date;
expires_at: Date;
name: string;
price_cents: number;
status: ItemStatus;
// COALESCE'd json_agg, so always an array. Only id and image_path are
// selected; the cart does not need sort_order.
images: Pick<ItemImage, 'id' | 'image_path'>[];
}
/** The row locked FOR UPDATE before an item is reserved. */
interface LockedItemRow {
id: number;
status: ItemStatus;
}
interface RemovedItemRow {
item_id: number;
}
const CART_ITEM_SELECT = ` const CART_ITEM_SELECT = `
SELECT SELECT
ci.item_id, ci.added_at, ci.expires_at, ci.item_id, ci.added_at, ci.expires_at,
@@ -24,9 +56,9 @@ const CART_ITEM_SELECT = `
`; `;
router.get('/', requireCustomer, asyncRoute(async (req: Request, res: Response) => { router.get('/', requireCustomer, asyncRoute(async (req: Request, res: Response) => {
const { rows: cartRows } = await pool.query(`SELECT id FROM carts WHERE customer_id = $1`, [req.customerId]); const { rows: cartRows } = await pool.query<IdRow>(`SELECT id FROM carts WHERE customer_id = $1`, [req.customerId]);
if (!cartRows.length) return res.json({ items: [] }); if (!cartRows.length) return res.json({ items: [] });
const { rows: items } = await pool.query(CART_ITEM_SELECT, [cartRows[0].id]); const { rows: items } = await pool.query<CartRow>(CART_ITEM_SELECT, [cartRows[0].id]);
res.json({ items }); res.json({ items });
})); }));
@@ -35,7 +67,7 @@ router.post('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, r
const client = await pool.connect(); const client = await pool.connect();
try { try {
await client.query('BEGIN'); await client.query('BEGIN');
const { rows: itemRows } = await client.query(`SELECT * FROM items WHERE id = $1 FOR UPDATE`, [itemId]); const { rows: itemRows } = await client.query<LockedItemRow>(`SELECT * FROM items WHERE id = $1 FOR UPDATE`, [itemId]);
const item = itemRows[0]; const item = itemRows[0];
if (!item) { await client.query('ROLLBACK'); return res.status(404).json({ error: 'not found' }); } if (!item) { await client.query('ROLLBACK'); return res.status(404).json({ error: 'not found' }); }
if (item.status !== 'available') { if (item.status !== 'available') {
@@ -43,13 +75,13 @@ router.post('/items/:itemId', requireCustomer, asyncRoute(async (req: Request, r
return res.status(409).json({ error: 'item is no longer available' }); return res.status(409).json({ error: 'item is no longer available' });
} }
let { rows: cartRows } = await client.query(`SELECT id FROM carts WHERE customer_id = $1`, [req.customerId]); let { rows: cartRows } = await client.query<IdRow>(`SELECT id FROM carts WHERE customer_id = $1`, [req.customerId]);
let cartId: number; let cartId: number;
if (cartRows.length) { if (cartRows.length) {
cartId = cartRows[0].id; cartId = cartRows[0].id;
await client.query(`UPDATE carts SET updated_at = now() WHERE id = $1`, [cartId]); await client.query(`UPDATE carts SET updated_at = now() WHERE id = $1`, [cartId]);
} else { } else {
const { rows: newCart } = await client.query( const { rows: newCart } = await client.query<IdRow>(
`INSERT INTO carts (customer_id) VALUES ($1) RETURNING id`, `INSERT INTO carts (customer_id) VALUES ($1) RETURNING id`,
[req.customerId] [req.customerId]
); );
@@ -79,7 +111,7 @@ router.delete('/items/:itemId', requireCustomer, asyncRoute(async (req: Request,
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<RemovedItemRow>(
`DELETE FROM cart_items ci `DELETE FROM cart_items ci
USING carts c USING carts c
WHERE ci.cart_id = c.id AND c.customer_id = $1 AND ci.item_id = $2 WHERE ci.cart_id = c.id AND c.customer_id = $1 AND ci.item_id = $2
+39 -10
View File
@@ -25,6 +25,35 @@ async function getAccessToken(): Promise<string> {
return data.access_token; return data.access_token;
} }
/**
* Row shapes for the reads in this file.
*
* Only queries whose rows are actually read carry a type. The INSERTs, UPDATEs,
* DELETEs and the BEGIN/COMMIT/ROLLBACK calls return nothing anyone looks at,
* and annotating them would be ceremony that makes the ones that matter harder
* to pick out.
*
* Kept in step with their SQL by hand: `client.query<T>` asserts a shape rather
* than checking it, because TypeScript never reads the query string. The
* integration suite is what catches a select and its type disagreeing.
*/
interface IdRow {
id: number;
}
interface CheckoutItemRow {
item_id: number;
price_cents: number;
}
interface CheckoutOwnerRow {
customer_id: number | null;
}
interface CheckoutStatusRow {
status: string;
}
interface CartItem { interface CartItem {
id: number; id: number;
name: string; name: string;
@@ -40,10 +69,10 @@ interface LockedCart {
// Locks the customer's cart, verifies every item is still reserved to them, // Locks the customer's cart, verifies every item is still reserved to them,
// and returns { cartId, items: [{id, name, price_cents}], totalCents }. // and returns { cartId, items: [{id, name, price_cents}], totalCents }.
async function loadLockedCart(client: PoolClient, customerId: number): Promise<LockedCart | null> { async function loadLockedCart(client: PoolClient, customerId: number): Promise<LockedCart | null> {
const { rows: cartRows } = await client.query(`SELECT id FROM carts WHERE customer_id = $1`, [customerId]); const { rows: cartRows } = await client.query<IdRow>(`SELECT id FROM carts WHERE customer_id = $1`, [customerId]);
if (!cartRows.length) return null; if (!cartRows.length) return null;
const cartId = cartRows[0].id; const cartId = cartRows[0].id;
const { rows: items } = await client.query( const { rows: items } = await client.query<CartItem>(
`SELECT i.id, i.name, i.price_cents `SELECT i.id, i.name, i.price_cents
FROM cart_items ci FROM cart_items ci
JOIN items i ON i.id = ci.item_id JOIN items i ON i.id = ci.item_id
@@ -52,7 +81,7 @@ async function loadLockedCart(client: PoolClient, customerId: number): Promise<L
[cartId] [cartId]
); );
if (!items.length) return { cartId, items: [], totalCents: 0 }; if (!items.length) return { cartId, items: [], totalCents: 0 };
const totalCents = items.reduce((sum: number, it: CartItem) => sum + it.price_cents, 0); const totalCents = items.reduce((sum, it) => sum + it.price_cents, 0);
return { cartId, items, totalCents }; return { cartId, items, totalCents };
} }
@@ -71,7 +100,7 @@ async function openCheckout(
processor: string, processor: string,
processorOrderId: string | null processorOrderId: string | null
): Promise<OpenedCheckout> { ): Promise<OpenedCheckout> {
const { rows: addrRows } = await client.query( const { rows: addrRows } = await client.query<IdRow>(
`SELECT id FROM shipping_addresses WHERE id = $1 AND customer_id = $2`, `SELECT id FROM shipping_addresses WHERE id = $1 AND customer_id = $2`,
[shippingAddressId, customerId] [shippingAddressId, customerId]
); );
@@ -80,7 +109,7 @@ async function openCheckout(
const cart = await loadLockedCart(client, customerId); const cart = await loadLockedCart(client, customerId);
if (!cart || !cart.items.length) return { ok: false, error: 'cart is empty' }; if (!cart || !cart.items.length) return { ok: false, error: 'cart is empty' };
const { rows: checkoutRows } = await client.query( const { rows: checkoutRows } = await client.query<IdRow>(
`INSERT INTO checkouts (customer_id, shipping_address_id, processor, processor_order_id, amount_cents, status) `INSERT INTO checkouts (customer_id, shipping_address_id, processor, processor_order_id, amount_cents, status)
VALUES ($1, $2, $3, $4, $5, 'pending') RETURNING id`, VALUES ($1, $2, $3, $4, $5, 'pending') RETURNING id`,
[customerId, shippingAddressId, processor, processorOrderId, cart.totalCents] [customerId, shippingAddressId, processor, processorOrderId, cart.totalCents]
@@ -147,11 +176,11 @@ router.post('/paypal/create', requireCustomer, asyncRoute(async (req: Request, r
// *after* COMMIT. Sending inside the transaction would email people about a // *after* COMMIT. Sending inside the transaction would email people about a
// sale that then rolled back, and would hold the transaction open for SMTP. // sale that then rolled back, and would hold the transaction open for SMTP.
async function completeCheckout(client: PoolClient, checkoutId: number, processor: string, processorOrderId: string | null, rawEvent: unknown): Promise<{ itemIds: number[]; buyerId: number | null }> { async function completeCheckout(client: PoolClient, checkoutId: number, processor: string, processorOrderId: string | null, rawEvent: unknown): Promise<{ itemIds: number[]; buyerId: number | null }> {
const { rows: checkoutItems } = await client.query( const { rows: checkoutItems } = await client.query<CheckoutItemRow>(
`SELECT item_id, price_cents FROM checkout_items WHERE checkout_id = $1`, `SELECT item_id, price_cents FROM checkout_items WHERE checkout_id = $1`,
[checkoutId] [checkoutId]
); );
const { rows: checkoutRows } = await client.query(`SELECT customer_id FROM checkouts WHERE id = $1`, [checkoutId]); const { rows: checkoutRows } = await client.query<CheckoutOwnerRow>(`SELECT customer_id FROM checkouts WHERE id = $1`, [checkoutId]);
const customerId = checkoutRows[0]?.customer_id; const customerId = checkoutRows[0]?.customer_id;
for (const ci of checkoutItems) { for (const ci of checkoutItems) {
@@ -166,7 +195,7 @@ async function completeCheckout(client: PoolClient, checkoutId: number, processo
await client.query(`UPDATE checkouts SET status = 'completed', raw_event = $1 WHERE id = $2`, [rawEvent, checkoutId]); await client.query(`UPDATE checkouts SET status = 'completed', raw_event = $1 WHERE id = $2`, [rawEvent, checkoutId]);
return { return {
itemIds: checkoutItems.map((ci: { item_id: number }) => ci.item_id), itemIds: checkoutItems.map((ci) => ci.item_id),
buyerId: customerId ?? null buyerId: customerId ?? null
}; };
} }
@@ -185,7 +214,7 @@ router.post('/paypal/capture', requireCustomer, asyncRoute(async (req: Request,
return res.status(502).json({ error: 'capture failed', detail: capture }); return res.status(502).json({ error: 'capture failed', detail: capture });
} }
const { rows } = await pool.query( const { rows } = await pool.query<IdRow>(
`SELECT id FROM checkouts WHERE processor_order_id = $1 AND customer_id = $2`, `SELECT id FROM checkouts WHERE processor_order_id = $1 AND customer_id = $2`,
[orderID, req.customerId] [orderID, req.customerId]
); );
@@ -252,7 +281,7 @@ webhookRouter.post('/', asyncRoute(async (req: Request, res: Response) => {
if (event.event_type === 'PAYMENT.CAPTURE.COMPLETED') { if (event.event_type === 'PAYMENT.CAPTURE.COMPLETED') {
const checkoutId = event.resource?.custom_id; const checkoutId = event.resource?.custom_id;
if (checkoutId) { if (checkoutId) {
const { rows } = await pool.query(`SELECT status FROM checkouts WHERE id = $1`, [checkoutId]); const { rows } = await pool.query<CheckoutStatusRow>(`SELECT status FROM checkouts WHERE id = $1`, [checkoutId]);
if (rows.length && rows[0].status !== 'completed') { if (rows.length && rows[0].status !== 'completed') {
const client = await pool.connect(); const client = await pool.connect();
try { try {