Add PO PDF token-vs-filename spec
This commit is contained in:
SJCoetz
parent
2bee7aa4be
commit
538986d7db
1 changed files with 176 additions and 0 deletions
|
|
@ -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: <original-filename>,
|
||||
token: <actual-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/<poId>/<entry.token>`.
|
||||
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/<poId>/<token>`,
|
||||
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/<id>/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).
|
||||
Loading…
Reference in a new issue