diff --git a/docs/superpowers/specs/2026-07-09-po-pdf-token-vs-filename-design.md b/docs/superpowers/specs/2026-07-09-po-pdf-token-vs-filename-design.md new file mode 100644 index 0000000..b89250c --- /dev/null +++ b/docs/superpowers/specs/2026-07-09-po-pdf-token-vs-filename-design.md @@ -0,0 +1,176 @@ +# PO PDF: token vs filename split + GET through file proxy + +**Goal:** Fix the `Upstream PB file fetch failed: 404` / `File not found` errors when downloading PO PDFs from the order detail page, and stop the PDF download path from bypassing the file proxy. + +## Scope + +### In Scope + +- Add a `token` field to `PurchaseOrderDocumentEntry` so we store the PB-side file token alongside the human-readable filename. +- `filename` keeps being set on upload (audit value), `token` is the PB-stored token from the upload response. +- POST route writes `token` from PB's response, not from the original filename. +- GET route resolves the token by either reading it from `documents_history` directly, or by listing `purchase_orders.documents` once. Either way, GET hands the token to the file proxy instead of fetching PB directly. +- One-time migration: backfill `token` on existing history rows that don't have one yet, by reading `purchase_orders.documents` once and matching by index. +- Affected UI is unchanged — the version pills on the order detail page keep calling the same URL. + +### Out of Scope + +- Changing the `documents_history` JSON field type. +- Touching any other collection (quotes, sprint, etc.). +- Re-issuing v1 PDFs for old POs — existing rows are migrated in place. +- Removing the legacy `purchase_order_pdfs` collection or its proxy whitelist entry. + +## Current State + +The PO PDF route stores the human-readable filename in +`documents_history[].file` (e.g. `PO-CNL-...-v4.pdf`). PB stores the +file on disk under a slugged token, which is the value in +`purchase_orders.documents`. The GET handler builds +`pb.files.getURL({ collectionId: "purchase_orders", id: poId }, entry.file)` +and fetches PB directly, bypassing the file proxy. Result: 404 +because `entry.file` is not a token PB recognises. + +The PDF download buttons on `/orders/[id]/page.tsx` (lines 727 and 819) +hit this broken GET endpoint. + +## User Confirmed Rule + +- Option 1: keep `entry.file` as the human-readable name and add a + separate `entry.token` field. Cleanest schema; migration of existing + rows is one pass. + +## Data Model + +### Updated `PurchaseOrderDocumentEntry` + +```ts +interface PurchaseOrderDocumentEntry { + version: number; + file: string; // human-readable filename, audit value + token: string; // PB-stored file token, used to fetch bytes + generatedAt: string; + generatedBy: string; + reason: string; + status: "current" | "superseded"; +} +``` + +### Migration + +For each `purchase_orders` row with a non-empty `documents_history`: + +1. Read `documents_history`. For any entry missing `token`, look up + the corresponding token in `purchase_orders.documents` by index. + The two arrays are kept in sync by the POST handler (each regen + appends to both), so `documents[i]` corresponds to + `documents_history[i]`. +2. If `documents[i]` exists, set `entry.token = documents[i]`. +3. If `documents[i]` is missing (e.g. legacy single-file rows where + PB returns a string instead of an array), set + `entry.token = documents[0]` (best-effort fallback) and flag the + row for manual re-gen via a UI banner. + +Migration runs once via a script in `scripts/migrations/`. Idempotent: +re-running is a no-op once all rows have `token`. + +## Behavior + +### POST (regenerate / first generate) + +1. Upload the new PDF blob via FormData to `purchase_orders.documents`. +2. Read the response — PB returns the stored file token(s). The new + token is `response.documents[response.documents.length - 1]`. +3. Append `documents_history` with `file: , + token: `. +4. As today: write `total_amount`. + +### GET (download / view) + +1. Resolve the entry the same way as today (`?version=N` or "current" + fallback). +2. If `entry.token` is present, build the file proxy URL: + `/api/v1/files/purchase_orders//`. +3. If `entry.token` is missing (legacy row, migration skipped it), + build the same URL using `po.documents[0]` if available; else + return 404 with a clear error. +4. Stream from the proxy. The proxy already has `purchase_orders` in + its whitelist. + +The GET handler stops calling `pb.files.getURL` directly. The file +proxy becomes the single source of truth for "stream a PB file behind +admin auth." + +### Error UX + +- If a row has no `token` and no `documents[0]`, the version pill + shows a "PDF unavailable" tooltip. Same on download click — returns + 404 with `{ error: "PDF token missing for version N — please + regenerate" }`. + +## Testing + +### Unit / route tests + +`src/app/api/v1/purchase-orders/[id]/pdf/route.test.ts` (new): + +1. POST stores `token` from the PB response, not from the original + filename. +2. POST `documents_history[].token` matches `documents[]` index for + index. +3. GET routes through `/api/v1/files/purchase_orders//`, + not via direct PB fetch. +4. GET 404s with a clear message when the row has no token and no + `documents[0]`. +5. GET respects `?version=N` and `?download=1` (existing behavior, + regression test). + +### File proxy test + +`src/app/api/v1/files/[collection]/[recordId]/[filename]/route.test.ts` +— existing test should still pass; no changes needed. Add one +regression test that confirms `purchase_orders` is whitelisted. + +### UI regression + +`src/app/(app)/orders/[id]/page.test.tsx` (if exists) — existing +version-pill click handler should still call the same URL. No +changes expected; add an assertion that clicking a pill calls +`/api/v1/purchase-orders//pdf?version=N&download=1`. + +## Migration Script + +`scripts/migrations/2026-07-09-po-pdf-tokens.ts`: + +- For every row in `purchase_orders` where `documents_history` is + non-empty: + - For each entry lacking `token`, read `documents[i]` (or + `documents[0]` if `documents` is a string). + - Patch the row with the new `documents_history`. +- Log a summary: rows patched, rows with no token resolvable. +- Idempotent (re-runnable). + +Add the script to `package.json` as `npm run pb:migrate:po-pdf-tokens` +so it's discoverable alongside the other migrations. + +## Acceptance Criteria + +- Clicking any version pill on the order detail page downloads / + opens the PDF without a 404. +- `documents_history[].token` is populated for every entry written + after the migration. +- GET routes through the file proxy (one place to change admin-auth + behaviour). +- Migration is idempotent and logs a clear summary. +- All targeted tests pass. + +## Risk / Rollback + +- Medium risk: every operator-visible PO download depends on this + path. Mitigated by: + - Migration preserves the original `file` (filename) field, so + rollback just ignores `token` and the existing data is + untouched. + - GET falls back to `documents[0]` if `token` is missing, so a + migration that misses a row still renders the right bytes. +- Rollback: revert the route + migration script. No schema change to + PocketBase (the JSON field is flexible). \ No newline at end of file