docs(intake): refresh the extraction task for the changes #226 made (#222)

This plan was written on 2026-08-29, before #226 landed. Its Task 2 lists what to move out of routes/admin.ts into the shared image pipeline, and that list is now missing `stripUploadedImages` and the `reencodeInPlace` import it depends on, because neither existed when the list was written.

Executing it as written would have left the re-encode behind in admin.ts, and the public intake route added in Task 5 would then have had an upload path that skips EXIF stripping entirely. That is precisely what #226 exists to prevent — a stranger photographing an item at home publishing the coordinates it was taken at — and nothing in the suite would have failed to say so, because the intake tests are written against a route that does not exist yet.

The task now names the function, says why it matters, and adds a check with a definite answer: after the move, routes/admin.ts must no longer import imageProcessing. If it still does, something was left behind.

Ref #222, #226
This commit is contained in:
2026-08-31 15:42:25 -05:00
parent 7100352d98
commit 7d8ac15ef8
@@ -188,7 +188,7 @@ This is a pure refactor. No behaviour changes; the existing tests are the safety
**Interfaces:** **Interfaces:**
- Consumes: `uploadTypes.ts` (`ALLOWED_IMAGE_TYPES`, `SIGNATURE_BYTES`, `extensionFor`, `isAllowedImageType`, `signatureMatches`) - Consumes: `uploadTypes.ts` (`ALLOWED_IMAGE_TYPES`, `SIGNATURE_BYTES`, `extensionFor`, `isAllowedImageType`, `signatureMatches`)
- Produces: - Produces:
- `uploadImages: (req, res, next) => void` — multer middleware, field name `images` - `uploadImages: (req, res, next) => void` — multer middleware, field name `images`. Internally runs `verifyUploadedImages` then `stripUploadedImages`, so a caller mounting this gets validation *and* EXIF stripping without asking for either
- `verifyUploadedImages(req: Request): Promise<string | null>` — refusal message, or null - `verifyUploadedImages(req: Request): Promise<string | null>` — refusal message, or null
- `insertItemImages(client: PoolClient, itemId: number, files: Express.Multer.File[], firstSortOrder: number): Promise<void>` - `insertItemImages(client: PoolClient, itemId: number, files: Express.Multer.File[], firstSortOrder: number): Promise<void>`
- `MAX_IMAGES_PER_REQUEST: number`, `MAX_IMAGE_BYTES: number` - `MAX_IMAGES_PER_REQUEST: number`, `MAX_IMAGE_BYTES: number`
@@ -203,9 +203,13 @@ Expected: PASS. Record this — it is the comparison for Step 4.
- [ ] **Step 2: Move the code** - [ ] **Step 2: Move the code**
Create `backend/src/imageUpload.ts` and move into it, unchanged, from `routes/admin.ts`: `UnsupportedImageTypeError`, `UPLOADS_DIR`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`, `MAX_TEXT_FIELDS`, `MAX_TEXT_FIELD_BYTES`, `storage`, `upload`, `readHead`, `discardUploads`, `discardUnlessAccepted`, `verifyUploadedImages`, `uploadImages`, and `insertItemImages`. Create `backend/src/imageUpload.ts` and move into it, unchanged, from `routes/admin.ts`: `UnsupportedImageTypeError`, `UPLOADS_DIR`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`, `MAX_TEXT_FIELDS`, `MAX_TEXT_FIELD_BYTES`, `storage`, `upload`, `readHead`, `discardUploads`, `discardUnlessAccepted`, `verifyUploadedImages`, **`stripUploadedImages`**, `uploadImages`, and `insertItemImages`.
Keep every existing comment verbatim. They record why the code is shaped as it is (#95, #103, #180) and are the most valuable thing being moved. **`stripUploadedImages` and its `import { reencodeInPlace } from '../imageProcessing'` did not exist when this plan was written.** They arrived with #226 after it, and they are the step that removes EXIF — including the GPS coordinates a phone writes — from every accepted upload. Leaving them behind in `admin.ts` would give the public intake route in Task 5 an upload path that skips stripping entirely, which is the one thing #226 exists to prevent, and nothing would fail to say so.
The promise chain inside `uploadImages` runs `verifyUploadedImages` and then `stripUploadedImages`, and moves as a whole. Verify after moving that `routes/admin.ts` no longer imports `imageProcessing` — if it still does, something was left behind.
Keep every existing comment verbatim. They record why the code is shaped as it is (#95, #103, #180, #226) and are the most valuable thing being moved.
Export `uploadImages`, `verifyUploadedImages`, `insertItemImages`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`. Everything else stays module-private. Export `uploadImages`, `verifyUploadedImages`, `insertItemImages`, `MAX_IMAGES_PER_REQUEST`, `MAX_IMAGE_BYTES`. Everything else stays module-private.