fix(po): upload attachments server-side so they persist & show #144

Merged

shad0w merged 2 commits from claude/flamboyant-gagarin-370922 into master

2026-06-27 19:47:27 +00:00

shad0w commented

2026-06-27 18:59:04 +00:00

Owner

Two parts

1. Fix: PO attachments never persisted (root cause)

PO/receipt attachments were uploaded via a browser presigned PUT straight to R2 (POST /api/files/sign -> browser PUT -> linkDocument). That direct browser->R2 PUT needs an R2 CORS policy allowing PUT from the portal origin; in production it was missing, so the browser silently blocked the PUT, linkDocument never ran, and no PODocument row was created (0 rows in prod/staging).

Now uploads go through a server action (uploadPoDocuments) that writes the file with uploadBuffer and creates the PODocument row atomically -- the same server-side pattern crewing already uses, within the 10mb serverActions limit. Removes the CORS dependency. Files still land in R2 (server-to-server). Removes the dead presigned path.

2. Feature: add attachments to CLOSED POs (remediation)

New opt-in flag NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED. When on, a CLOSED PO's own submitter, plus Accounts / Manager / SuperUser, can attach documents to it -- so POs whose uploads were lost to the bug can be fixed without reopening them. Off by default (production unchanged); enabled on staging. Enforced both in the UI (PO detail Attachments card) and server-side in uploadPoDocuments.

Note: POs created during the broken window have no rows to recover -- affected users re-upload via this path.

Tests

po-document-upload.test.ts -- upload creates a PODocument row, file bytes reach storage, doc is surfaced by the exact PO-detail include; plus the flag-off case (closed PO immutable).
closed-po-attachments.test.ts -- flag-on role matrix (submitter / Accounts / Manager / SuperUser allowed; other submitter-role + auditor refused; non-closed PO unaffected).

Full unit (338) + integration (293) suites green; tsc --noEmit clean.

Note on the earlier CI failure

The first run failed at the Checkout step -- the pms1 runner couldn't reach code.forgejo.org to fetch actions/checkout@v4 (TCP timeout), unrelated to the diff. Re-triggered by this push.

Generated with Claude Code.

## Two parts ### 1. Fix: PO attachments never persisted (root cause) PO/receipt attachments were uploaded via a **browser presigned PUT straight to R2** (`POST /api/files/sign` -> browser PUT -> `linkDocument`). That direct browser->R2 PUT needs an R2 CORS policy allowing PUT from the portal origin; in production it was missing, so the browser silently blocked the PUT, `linkDocument` never ran, and **no PODocument row was created** (0 rows in prod/staging). Now uploads go through a server action (`uploadPoDocuments`) that writes the file with `uploadBuffer` and creates the PODocument row **atomically** -- the same server-side pattern crewing already uses, within the 10mb serverActions limit. Removes the CORS dependency. Files still land in R2 (server-to-server). Removes the dead presigned path. ### 2. Feature: add attachments to CLOSED POs (remediation) New opt-in flag `NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED`. When on, a CLOSED PO's **own submitter**, plus **Accounts / Manager / SuperUser**, can attach documents to it -- so POs whose uploads were lost to the bug can be fixed without reopening them. Off by default (production unchanged); enabled on staging. Enforced both in the UI (PO detail Attachments card) and server-side in `uploadPoDocuments`. > Note: POs created during the broken window have no rows to recover -- affected users re-upload via this path. ## Tests - `po-document-upload.test.ts` -- upload creates a PODocument row, file bytes reach storage, doc is surfaced by the exact PO-detail include; plus the flag-off case (closed PO immutable). - `closed-po-attachments.test.ts` -- flag-on role matrix (submitter / Accounts / Manager / SuperUser allowed; other submitter-role + auditor refused; non-closed PO unaffected). Full unit (338) + integration (293) suites green; `tsc --noEmit` clean. ## Note on the earlier CI failure The first run failed at the **Checkout** step -- the pms1 runner couldn't reach `code.forgejo.org` to fetch `actions/checkout@v4` (TCP timeout), unrelated to the diff. Re-triggered by this push. Generated with Claude Code.

shad0w added 1 commit 2026-06-27 18:59:05 +00:00

fix(po): upload attachments server-side so they persist & show

PR checks / checks (pull_request) Failing after 2m14s

Details

PR checks / integration (pull_request) Failing after 2m13s

Details

2afdeec6ad

PO/receipt attachments were uploaded via a browser presigned PUT straight
to R2 (POST /api/files/sign -> PUT uploadUrl -> linkDocument). That direct
browser->R2 PUT only succeeds when the bucket carries a CORS policy
allowing PUT from the portal origin; in production that policy was missing,
so the browser silently blocked the PUT, linkDocument never ran, and no
PODocument row was created -- "documents uploaded but not visible anywhere"
(0 PODocument rows in prod/staging).

Route uploads through a server action (uploadPoDocuments) that writes the
file with uploadBuffer and creates the PODocument row atomically -- the
same pattern the crewing module already uses for CV/crew-document uploads,
and within the 10mb serverActions.bodySizeLimit. This removes the R2-CORS
dependency and guarantees a created row always has its stored file.

Removes the now-dead presigned path (lib/upload-files.ts,
app/actions/link-document.ts, api/files/sign/route.ts).

Adds an integration test that drives the action against a real DB and
asserts the PODocument row is created and surfaced by the exact include
the PO detail page renders from (i.e. the attachment is visible).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

shad0w added 1 commit 2026-06-27 19:41:36 +00:00

feat(po): feature-flagged attachments on closed POs (bug remediation)

PR checks / checks (pull_request) Successful in 51s

Details

PR checks / integration (pull_request) Successful in 31s

Details

158b446117

Adds NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED. When on, a CLOSED PO's own
submitter -- plus Accounts / Manager / SuperUser -- can attach documents to
it, so POs whose uploads were lost to the document-upload bug can be fixed
without reopening them. Off by default, so production stays unchanged until
enabled.

- lib/permissions.ts: canAddClosedPoAttachment(role, { isSubmitter }) gated
  by the flag; allowed roles are ACCOUNTS/MANAGER/SUPERUSER (plus the PO's
  own submitter regardless of role).
- uploadPoDocuments: a CLOSED PO is otherwise immutable, so it now enforces
  the permission server-side; the normal create/receipt flows upload while
  the PO is pre-CLOSED and are unaffected.
- po-detail.tsx: when allowed, the Attachments card renders an uploader
  (ClosedPoAttachmentUploader) and shows even when the PO has no docs yet.
- Enabled on staging (staging-up.sh) so the remediation can be exercised;
  documented in .env.example and CLAUDE.md.

Tests: closed-po-attachments.test.ts covers the flag-on role matrix (own
submitter / Accounts / Manager / SuperUser allowed; other submitter-role and
auditor refused; non-closed PO unaffected); po-document-upload.test.ts adds
the flag-off case (closed PO stays immutable). Full unit + integration suites
green; tsc clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>