Fix/197 restrict scan publishing #267
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: bermudalamb/redefined-designs#267
Reference in New Issue
Block a user
Three routes in admin.ts answered a miss with a success. PUT /items/:id ran an UPDATE that matched nothing, committed happily, selected nothing back and replied 200 with an empty body — a success the admin client could do nothing with, and no record anywhere that the item was not found. mark-sold and mark-available did the same. The create route beside them has always used requireRow for exactly this, which is why this reads as an oversight rather than a decision. A garbage id was worse in a different direction. Number('abc') is NaN, the driver sends it to Postgres as the text "NaN", Postgres raises 22P02 for an integer column, and the catch turned that into a 500 — so a caller asking for an item that cannot exist was told the server broke. Both now answer 404, because from the caller's side "/items/abc" identifies no item in exactly the way "/items/999999" does. readId is shared rather than repeated, and rejects zero, negatives and fractions as well as text: every id in this schema is a positive serial, so anything else identifies nothing. mark-sold now notifies favouriters only after the row is known to exist, so nobody is told about a sale that did not happen. The issue asked for the same shape to be checked across the other admin routes. It was: unpublish already looks the item up and 404s, and the tags and categories PUT routes both do an existence check before their UPDATE, so their rows[0] is guaranteed. items.ts already guards the public read. These three were the only ones lying about a miss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>All nine sites, chosen because the file is awkward rather than easy: a recursive CTE consumed two ways, a correlated subquery, an array match, and two error paths keyed on a Postgres SQLSTATE. A file of plain CRUD would have produced a flattering number that does not generalise. The hand-declared row interfaces are gone. CATEGORY_COLUMNS is written once and the row type is inferred from it, which closes the drift itemSelect.ts documents as "KEPT IN STEP BY HAND". That mapping has to be explicit rather than selecting the table: the mirror names columns in camelCase and this API answers in snake_case, so selecting the table directly would have silently changed the JSON contract the admin frontend reads, and no test asserting status codes would have caught it. strict and noUncheckedIndexedAccess hold with no non-null assertions added. requireRow covers the RETURNING rows and the existing lookup destructures and branches, exactly as before. Two bugs were introduced and caught by the integration suite, and both are worth recording because neither produced a type error. Drizzle renders a column reference inside a `sql` template UNQUALIFIED. `${items.categoryId} = ${categories.id}` became `WHERE "category_id" = "id"`, which Postgres resolved against items on both sides — so the item count came back plausible and wrong rather than failing. That is worse than the documented array trap, which at least produces invalid SQL. The fragment is now literal text, which is honest since it binds no values. And the driver's error code moved. Drizzle wraps errors, so the SQLSTATE that sat on err.code now sits on err.cause.code; the old check compiled, never matched, and turned two 409s into 500s. isUniqueViolation accepts both shapes. inArray replaced the ANY(...::int[]) match and sidesteps the sql.param trap entirely — there is no template to forget it in, and the builder emits the placeholder list correctly by construction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>