From 7390c458ed85fcb928038554f76c3e6b207b829b Mon Sep 17 00:00:00 2001 From: null Date: Mon, 6 Jul 2026 14:46:32 -0500 Subject: [PATCH] =?UTF-8?q?refactor(routes):=20categories=20=E2=86=92=20un?= =?UTF-8?q?iform=20Express-5=20throw=20pattern=20(Pillar=20B=20template)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Establishes the canonical route error pattern (see docs/CODE_STANDARDS.md): handlers `throw ValidationError/NotFoundError/ConflictError(...)` and let Express 5 forward to the single terminal handler — no per-handler try/catch swallows, no ad-hoc `res.status().json({error})`. Only the UNIQUE-constraint → 409 mapping keeps a tight try/catch (`asConflict`). - apiError factories NotFoundError/ConflictError now take an optional `field` (parity with ValidationError) so form-field info is preserved. - routes/categories.cts fully converted; identical status/messages, now uniform `{ error, message, code, field }` bodies. - Test harnesses (categoryGroups, categoryReorder) updated to simulate the terminal handler (thrown ApiError → formatted response). Verified: 236/236 server tests; e2e probe 17/17; live smoke confirms 400 VALIDATION_ERROR / 404 NOT_FOUND with correct field + status. Co-Authored-By: Claude Opus 4.8 --- routes/categories.cts | 173 +++++++++++++--------------------- tests/categoryGroups.test.js | 18 +++- tests/categoryReorder.test.js | 18 +++- utils/apiError.cts | 12 ++- 4 files changed, 104 insertions(+), 117 deletions(-) diff --git a/routes/categories.cts b/routes/categories.cts index 6f2ee29..b50fd21 100644 --- a/routes/categories.cts +++ b/routes/categories.cts @@ -1,20 +1,26 @@ // @ts-nocheck — route controller converted to .cts; handler req/res typed, full type-check deferred (thin glue over typed services). import type { Req, Res, Next } from '../types/http'; const express = require('express'); -const { standardizeError } = require('../middleware/errorFormatter.cts'); +const { ValidationError, NotFoundError, ConflictError } = require('../utils/apiError.cts'); const router = express.Router(); const { getDb, ensureUserDefaultCategories } = require('../db/database.cts'); const { accountingActiveSql } = require('../services/paymentAccountingService.cts'); +// Maps a SQLite UNIQUE-constraint violation to a friendly 409; re-throws anything +// else so the terminal handler returns a safe 500. +function asConflict(e, message, field) { + if (e.message?.includes('UNIQUE')) throw ConflictError(message, field); + throw e; +} + // GET /api/categories router.get('/', (req: Req, res: Res) => { - try { - const db = getDb(); - ensureUserDefaultCategories(req.user.id); + const db = getDb(); + ensureUserDefaultCategories(req.user.id); - const categories = db - .prepare( - ` + const categories = db + .prepare( + ` SELECT id, user_id, name, sort_order, spending_enabled, group_id, created_at, updated_at FROM categories WHERE user_id = ? @@ -23,10 +29,10 @@ router.get('/', (req: Req, res: Res) => { sort_order ASC, name COLLATE NOCASE ASC `, - ) - .all(req.user.id); + ) + .all(req.user.id); - const billsByCategory = db.prepare(` + const billsByCategory = db.prepare(` SELECT b.id, b.category_id, @@ -49,36 +55,32 @@ router.get('/', (req: Req, res: Res) => { ORDER BY b.active DESC, b.due_day ASC, b.name COLLATE NOCASE ASC `); - const shaped = categories.map((category) => { - const bills = billsByCategory.all(req.user.id, category.id).map((bill) => ({ - ...bill, - active: !!bill.active, - payment_count: Number(bill.payment_count || 0), - total_paid: Number(bill.total_paid || 0), - last_paid_date: bill.last_paid_date || null, - })); + const shaped = categories.map((category) => { + const bills = billsByCategory.all(req.user.id, category.id).map((bill) => ({ + ...bill, + active: !!bill.active, + payment_count: Number(bill.payment_count || 0), + total_paid: Number(bill.total_paid || 0), + last_paid_date: bill.last_paid_date || null, + })); - const activeBillCount = bills.filter((bill) => bill.active).length; - const inactiveBillCount = bills.length - activeBillCount; - const paymentCount = bills.reduce((sum, bill) => sum + bill.payment_count, 0); + const activeBillCount = bills.filter((bill) => bill.active).length; + const inactiveBillCount = bills.length - activeBillCount; + const paymentCount = bills.reduce((sum, bill) => sum + bill.payment_count, 0); - return { - ...category, - spending_enabled: !!category.spending_enabled, - bill_count: activeBillCount, - active_bill_count: activeBillCount, - inactive_bill_count: inactiveBillCount, - payment_count: paymentCount, - bill_names: bills.map((bill) => bill.name), - bills, - }; - }); + return { + ...category, + spending_enabled: !!category.spending_enabled, + bill_count: activeBillCount, + active_bill_count: activeBillCount, + inactive_bill_count: inactiveBillCount, + payment_count: paymentCount, + bill_names: bills.map((bill) => bill.name), + bills, + }; + }); - res.json(shaped); - } catch (err) { - console.error('[categories GET]', err.message); - res.status(500).json({ error: 'Failed to load categories' }); - } + res.json(shaped); }); // PUT /api/categories/reorder @@ -90,11 +92,7 @@ router.put('/reorder', (req: Req, res: Res) => { })); if (entries.length === 0) { - return res - .status(400) - .json( - standardizeError('At least one category order is required', 'VALIDATION_ERROR', 'reorder'), - ); + throw ValidationError('At least one category order is required', 'reorder'); } const invalid = entries.find( @@ -105,15 +103,10 @@ router.put('/reorder', (req: Req, res: Res) => { sortOrder < 0, ); if (invalid) { - return res - .status(400) - .json( - standardizeError( - 'Reorder payload must map category ids to non-negative integer positions', - 'VALIDATION_ERROR', - 'reorder', - ), - ); + throw ValidationError( + 'Reorder payload must map category ids to non-negative integer positions', + 'reorder', + ); } const ids = entries.map((item) => item.categoryId); @@ -128,9 +121,7 @@ router.put('/reorder', (req: Req, res: Res) => { ) .all(req.user.id, ...ids); if (owned.length !== ids.length) { - return res - .status(404) - .json(standardizeError('One or more categories were not found', 'NOT_FOUND', 'category_id')); + throw NotFoundError('One or more categories were not found', 'category_id'); } const update = db.prepare( @@ -147,8 +138,7 @@ router.put('/reorder', (req: Req, res: Res) => { router.post('/', (req: Req, res: Res) => { const db = getDb(); const { name } = req.body; - if (!name) - return res.status(400).json(standardizeError('name is required', 'VALIDATION_ERROR', 'name')); + if (!name) throw ValidationError('name is required', 'name'); try { const result = db @@ -157,10 +147,7 @@ router.post('/', (req: Req, res: Res) => { const created = db.prepare('SELECT * FROM categories WHERE id = ?').get(result.lastInsertRowid); res.status(201).json(created); } catch (e) { - if (e.message.includes('UNIQUE')) { - return res.status(409).json(standardizeError('Category already exists', 'CONFLICT', 'name')); - } - throw e; + asConflict(e, 'Category already exists', 'name'); } }); @@ -168,22 +155,18 @@ router.post('/', (req: Req, res: Res) => { router.put('/:id', (req: Req, res: Res) => { const db = getDb(); const { name, spending_enabled, group_id } = req.body; - if (!name) - return res.status(400).json(standardizeError('name is required', 'VALIDATION_ERROR', 'name')); + if (!name) throw ValidationError('name is required', 'name'); const cat = db .prepare('SELECT id FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL') .get(req.params.id, req.user.id); - if (!cat) return res.status(404).json(standardizeError('Category not found', 'NOT_FOUND', 'id')); + if (!cat) throw NotFoundError('Category not found', 'id'); if (group_id !== undefined && group_id !== null) { const group = db .prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?') .get(group_id, req.user.id); - if (!group) - return res - .status(404) - .json(standardizeError('Category group not found', 'NOT_FOUND', 'group_id')); + if (!group) throw NotFoundError('Category group not found', 'group_id'); } try { @@ -209,11 +192,7 @@ router.put('/:id', (req: Req, res: Res) => { .get(req.params.id, req.user.id); res.json({ ...updated, spending_enabled: !!updated.spending_enabled }); } catch (e) { - if (e.message?.includes('UNIQUE')) { - return res.status(409).json(standardizeError('Category already exists', 'CONFLICT', 'name')); - } - console.error('[categories PUT]', e.message); - res.status(500).json({ error: 'Failed to update category' }); + asConflict(e, 'Category already exists', 'name'); } }); @@ -239,8 +218,7 @@ router.get('/groups', (req: Req, res: Res) => { router.post('/groups', (req: Req, res: Res) => { const db = getDb(); const { name } = req.body; - if (!name?.trim()) - return res.status(400).json(standardizeError('name is required', 'VALIDATION_ERROR', 'name')); + if (!name?.trim()) throw ValidationError('name is required', 'name'); try { const maxOrder = db @@ -256,13 +234,7 @@ router.post('/groups', (req: Req, res: Res) => { .get(result.lastInsertRowid); res.status(201).json(created); } catch (e) { - if (e.message?.includes('UNIQUE')) { - return res - .status(409) - .json(standardizeError('Category group already exists', 'CONFLICT', 'name')); - } - console.error('[category-groups POST]', e.message); - res.status(500).json({ error: 'Failed to create category group' }); + asConflict(e, 'Category group already exists', 'name'); } }); @@ -274,17 +246,13 @@ router.put('/groups/:id', (req: Req, res: Res) => { const group = db .prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?') .get(req.params.id, req.user.id); - if (!group) - return res.status(404).json(standardizeError('Category group not found', 'NOT_FOUND', 'id')); + if (!group) throw NotFoundError('Category group not found', 'id'); try { const fields = ["updated_at = datetime('now')"]; const values = []; if (name !== undefined) { - if (!name.trim()) - return res - .status(400) - .json(standardizeError('name is required', 'VALIDATION_ERROR', 'name')); + if (!name.trim()) throw ValidationError('name is required', 'name'); fields.push('name = ?'); values.push(name.trim()); } @@ -304,13 +272,7 @@ router.put('/groups/:id', (req: Req, res: Res) => { .get(req.params.id, req.user.id); res.json(updated); } catch (e) { - if (e.message?.includes('UNIQUE')) { - return res - .status(409) - .json(standardizeError('Category group already exists', 'CONFLICT', 'name')); - } - console.error('[category-groups PUT]', e.message); - res.status(500).json({ error: 'Failed to update category group' }); + asConflict(e, 'Category group already exists', 'name'); } }); @@ -320,8 +282,7 @@ router.delete('/groups/:id', (req: Req, res: Res) => { const group = db .prepare('SELECT id FROM category_groups WHERE id = ? AND user_id = ?') .get(req.params.id, req.user.id); - if (!group) - return res.status(404).json(standardizeError('Category group not found', 'NOT_FOUND', 'id')); + if (!group) throw NotFoundError('Category group not found', 'id'); db.prepare('DELETE FROM category_groups WHERE id = ? AND user_id = ?').run( req.params.id, @@ -338,19 +299,14 @@ router.patch('/:id/spending', (req: Req, res: Res) => { 'SELECT id, spending_enabled FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL', ) .get(req.params.id, req.user.id); - if (!cat) return res.status(404).json(standardizeError('Category not found', 'NOT_FOUND', 'id')); + if (!cat) throw NotFoundError('Category not found', 'id'); const enabled = req.body?.spending_enabled !== undefined ? !!req.body.spending_enabled : !cat.spending_enabled; - try { - db.prepare( - "UPDATE categories SET spending_enabled=?, updated_at=datetime('now') WHERE id=? AND user_id=?", - ).run(enabled ? 1 : 0, req.params.id, req.user.id); - res.json({ id: cat.id, spending_enabled: enabled }); - } catch (err) { - console.error('[categories PATCH spending]', err.message); - res.status(500).json({ error: 'Failed to update category' }); - } + db.prepare( + "UPDATE categories SET spending_enabled=?, updated_at=datetime('now') WHERE id=? AND user_id=?", + ).run(enabled ? 1 : 0, req.params.id, req.user.id); + res.json({ id: cat.id, spending_enabled: enabled }); }); // DELETE /api/categories/:id @@ -359,7 +315,7 @@ router.delete('/:id', (req: Req, res: Res) => { const cat = db .prepare('SELECT id, name FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NULL') .get(req.params.id, req.user.id); - if (!cat) return res.status(404).json(standardizeError('Category not found', 'NOT_FOUND', 'id')); + if (!cat) throw NotFoundError('Category not found', 'id'); const deleted = db .prepare( @@ -384,8 +340,7 @@ router.post('/:id/restore', (req: Req, res: Res) => { 'SELECT id, name FROM categories WHERE id = ? AND user_id = ? AND deleted_at IS NOT NULL', ) .get(req.params.id, req.user.id); - if (!cat) - return res.status(404).json(standardizeError('Deleted category not found', 'NOT_FOUND', 'id')); + if (!cat) throw NotFoundError('Deleted category not found', 'id'); db.prepare( "UPDATE categories SET deleted_at = NULL, updated_at = datetime('now') WHERE id = ? AND user_id = ?", diff --git a/tests/categoryGroups.test.js b/tests/categoryGroups.test.js index bf05e70..0b27b9e 100644 --- a/tests/categoryGroups.test.js +++ b/tests/categoryGroups.test.js @@ -54,10 +54,24 @@ function callCategoriesRoute(routePath, method, { userId, params = {}, body = {} resolve({ status: this.statusCode, data }); }, }; + // Simulate the terminal error handler: a thrown ApiError becomes a formatted + // response (mirrors utils/apiError formatError + server.cts). + const onError = (err) => { + resolve({ + status: err.status || 500, + data: { + error: err.code || 'INTERNAL_ERROR', + message: err.message, + code: err.code || 'INTERNAL_ERROR', + field: err.field ?? null, + }, + }); + }; try { - handler(req, res); + const maybe = handler(req, res); + if (maybe && typeof maybe.then === 'function') maybe.catch(onError); } catch (err) { - reject(err); + onError(err); } }); } diff --git a/tests/categoryReorder.test.js b/tests/categoryReorder.test.js index b8a9bc1..89b1217 100644 --- a/tests/categoryReorder.test.js +++ b/tests/categoryReorder.test.js @@ -55,10 +55,24 @@ function callCategoriesRoute(routePath, method, { userId, params = {}, body = {} resolve({ status: this.statusCode, data }); }, }; + // Simulate the terminal error handler: a thrown ApiError becomes a formatted + // response (mirrors utils/apiError formatError + server.cts). + const onError = (err) => { + resolve({ + status: err.status || 500, + data: { + error: err.code || 'INTERNAL_ERROR', + message: err.message, + code: err.code || 'INTERNAL_ERROR', + field: err.field ?? null, + }, + }); + }; try { - handler(req, res); + const maybe = handler(req, res); + if (maybe && typeof maybe.then === 'function') maybe.catch(onError); } catch (err) { - reject(err); + onError(err); } }); } diff --git a/utils/apiError.cts b/utils/apiError.cts index 7c7e8f3..4f0141d 100644 --- a/utils/apiError.cts +++ b/utils/apiError.cts @@ -56,15 +56,19 @@ function ForbiddenError(message = 'Access denied', code = 'FORBIDDEN'): ApiError /** * Create a standardized not found error */ -function NotFoundError(message = 'Resource not found', code = 'NOT_FOUND'): ApiError { - return new ApiError(code, message, 404); +function NotFoundError( + message = 'Resource not found', + field?: string, + code = 'NOT_FOUND', +): ApiError { + return new ApiError(code, message, 404, { field }); } /** * Create a standardized conflict error */ -function ConflictError(message = 'Resource conflict', code = 'CONFLICT'): ApiError { - return new ApiError(code, message, 409); +function ConflictError(message = 'Resource conflict', field?: string, code = 'CONFLICT'): ApiError { + return new ApiError(code, message, 409, { field }); } /**