From 2afdeec6ad1eeb3372aed1fa3057b8dab98be1b4 Mon Sep 17 00:00:00 2001 From: Hardik Date: Sun, 28 Jun 2026 00:28:16 +0530 Subject: [PATCH] fix(po): upload attachments server-side so they persist & show 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 --- .../(portal)/po/[id]/receipt/receipt-form.tsx | 4 +- App/app/(portal)/po/new/new-po-form.tsx | 4 +- App/app/actions/link-document.ts | 32 ---- App/app/actions/upload-po-documents.ts | 60 ++++++++ App/app/api/files/sign/route.ts | 34 ----- App/lib/upload-files.ts | 34 ----- .../integration/po-document-upload.test.ts | 140 ++++++++++++++++++ 7 files changed, 204 insertions(+), 104 deletions(-) delete mode 100644 App/app/actions/link-document.ts create mode 100644 App/app/actions/upload-po-documents.ts delete mode 100644 App/app/api/files/sign/route.ts delete mode 100644 App/lib/upload-files.ts create mode 100644 App/tests/integration/po-document-upload.test.ts diff --git a/App/app/(portal)/po/[id]/receipt/receipt-form.tsx b/App/app/(portal)/po/[id]/receipt/receipt-form.tsx index c9927bc..9682e5b 100644 --- a/App/app/(portal)/po/[id]/receipt/receipt-form.tsx +++ b/App/app/(portal)/po/[id]/receipt/receipt-form.tsx @@ -4,7 +4,7 @@ import { useState } from "react"; import { useRouter } from "next/navigation"; import { confirmReceipt } from "./actions"; import { FileUploader } from "@/components/po/file-uploader"; -import { uploadAndLinkFiles } from "@/lib/upload-files"; +import { uploadPoDocuments } from "@/app/actions/upload-po-documents"; interface LineItem { id: string; @@ -66,7 +66,7 @@ export function ReceiptForm({ poId, lineItems, isPartiallyReceived, isPartiallyP return; } if (files.length > 0) { - const uploadErr = await uploadAndLinkFiles(poId, files, "receipt"); + const uploadErr = await uploadPoDocuments(poId, files, "receipt"); if (uploadErr) { setError(uploadErr.error); setSubmitting(false); diff --git a/App/app/(portal)/po/new/new-po-form.tsx b/App/app/(portal)/po/new/new-po-form.tsx index 2896ec9..4fe0b8e 100644 --- a/App/app/(portal)/po/new/new-po-form.tsx +++ b/App/app/(portal)/po/new/new-po-form.tsx @@ -13,7 +13,7 @@ import { ProjectCodeField } from "@/components/po/project-code-field"; import { PoTermsEditor } from "@/components/po/po-terms-editor"; import { UnsavedChangesGuard } from "@/components/po/unsaved-changes-guard"; import type { CatalogueCategory, PoTerm } from "@/lib/terms"; -import { uploadAndLinkFiles } from "@/lib/upload-files"; +import { uploadPoDocuments } from "@/app/actions/upload-po-documents"; import type { LineItemInput } from "@/lib/validations/po"; export type VesselOption = { id: string; code: string; name: string }; @@ -80,7 +80,7 @@ export function NewPoForm({ vessels, accounts, vendors, companies, deliveryOptio return; } if (files.length > 0) { - const uploadErr = await uploadAndLinkFiles(result.id, files); + const uploadErr = await uploadPoDocuments(result.id, files); if (uploadErr) { setError(uploadErr.error); setSubmitting(null); diff --git a/App/app/actions/link-document.ts b/App/app/actions/link-document.ts deleted file mode 100644 index 2e355fd..0000000 --- a/App/app/actions/link-document.ts +++ /dev/null @@ -1,32 +0,0 @@ -"use server"; - -import { auth } from "@/auth"; -import { db } from "@/lib/db"; -import { revalidatePath } from "next/cache"; - -export async function linkDocument({ - poId, - storageKey, - fileName, - fileSize, - mimeType, -}: { - poId: string; - storageKey: string; - fileName: string; - fileSize: number; - mimeType: string; -}): Promise<{ ok: true } | { error: string }> { - const session = await auth(); - if (!session?.user) return { error: "Unauthorized" }; - - const po = await db.purchaseOrder.findUnique({ where: { id: poId }, select: { id: true } }); - if (!po) return { error: "PO not found" }; - - await db.pODocument.create({ - data: { poId, storageKey, fileName, fileSize, mimeType }, - }); - - revalidatePath(`/po/${poId}`); - return { ok: true }; -} diff --git a/App/app/actions/upload-po-documents.ts b/App/app/actions/upload-po-documents.ts new file mode 100644 index 0000000..713ea0a --- /dev/null +++ b/App/app/actions/upload-po-documents.ts @@ -0,0 +1,60 @@ +"use server"; + +import { auth } from "@/auth"; +import { db } from "@/lib/db"; +import { buildStorageKey, uploadBuffer } from "@/lib/storage"; +import { revalidatePath } from "next/cache"; + +// Matches the FileUploader hint ("up to 10 MB each") and +// next.config.ts → experimental.serverActions.bodySizeLimit. +const MAX_BYTES = 10 * 1024 * 1024; + +/** + * Persist PO attachments **server-side**: write each file to storage with + * `uploadBuffer` and create its `PODocument` row in the same step. + * + * Replaces the earlier browser-presigned-PUT flow (`POST /api/files/sign` → the + * browser `PUT`s the file straight to R2 → `linkDocument` creates the row). That + * direct browser→R2 `PUT` only works if the R2 bucket carries a CORS policy + * allowing `PUT` from the portal's origin. In production that policy was missing, + * so the browser silently blocked the upload, `linkDocument` was never reached, + * and **no `PODocument` row was created** — the "documents uploaded but not + * visible anywhere" report (0 PODocument rows in prod/staging). + * + * Uploading through the server — the same pattern the crewing module already uses + * for CVs / crew documents (`uploadBuffer`) — removes the CORS dependency and + * makes the store-and-link atomic, so a created row always has its file. + * + * Returns `{ error }` on the first failure, or `null` on success (the contract + * the PO and receipt forms already expect). + */ +export async function uploadPoDocuments( + poId: string, + files: File[], + type: "po-document" | "receipt" = "po-document" +): Promise<{ error: string } | null> { + const session = await auth(); + if (!session?.user) return { error: "Unauthorized" }; + + const po = await db.purchaseOrder.findUnique({ where: { id: poId }, select: { id: true } }); + if (!po) return { error: "PO not found" }; + + for (const file of files) { + if (!(file instanceof File) || file.size === 0) continue; + if (file.size > MAX_BYTES) { + return { error: `${file.name} is larger than the 10 MB limit.` }; + } + + const key = buildStorageKey(type, poId, file.name); + const mimeType = file.type || "application/octet-stream"; + const buffer = Buffer.from(await file.arrayBuffer()); + + await uploadBuffer(key, buffer, mimeType); + await db.pODocument.create({ + data: { poId, storageKey: key, fileName: file.name, fileSize: file.size, mimeType }, + }); + } + + revalidatePath(`/po/${poId}`); + return null; +} diff --git a/App/app/api/files/sign/route.ts b/App/app/api/files/sign/route.ts deleted file mode 100644 index b67dddd..0000000 --- a/App/app/api/files/sign/route.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { auth } from "@/auth"; -import { generateUploadUrl, buildStorageKey } from "@/lib/storage"; -import { NextRequest, NextResponse } from "next/server"; -import { z } from "zod"; - -const signSchema = z.object({ - fileName: z.string().min(1), - mimeType: z.string().min(1), - poId: z.string().min(1), - type: z.enum(["po-document", "receipt"]), -}); - -export async function POST(request: NextRequest) { - const session = await auth(); - if (!session?.user) { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); - } - - const body = await request.json(); - const parsed = signSchema.safeParse(body); - if (!parsed.success) { - return NextResponse.json({ error: "Invalid request" }, { status: 400 }); - } - - const { fileName, mimeType, poId, type } = parsed.data; - const key = buildStorageKey(type, poId, fileName); - - try { - const uploadUrl = await generateUploadUrl(key, mimeType); - return NextResponse.json({ uploadUrl, key }); - } catch { - return NextResponse.json({ error: "Failed to generate upload URL" }, { status: 500 }); - } -} diff --git a/App/lib/upload-files.ts b/App/lib/upload-files.ts deleted file mode 100644 index 2c64ddf..0000000 --- a/App/lib/upload-files.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { linkDocument } from "@/app/actions/link-document"; - -export async function uploadAndLinkFiles( - poId: string, - files: File[], - type: "po-document" | "receipt" = "po-document" -): Promise<{ error: string } | null> { - for (const file of files) { - const signRes = await fetch("/api/files/sign", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ fileName: file.name, mimeType: file.type || "application/octet-stream", poId, type }), - }); - if (!signRes.ok) return { error: `Failed to get upload URL for ${file.name}` }; - const { uploadUrl, key } = await signRes.json(); - - const putRes = await fetch(uploadUrl, { - method: "PUT", - headers: { "Content-Type": file.type || "application/octet-stream" }, - body: file, - }); - if (!putRes.ok) return { error: `Failed to upload ${file.name}` }; - - const result = await linkDocument({ - poId, - storageKey: key, - fileName: file.name, - fileSize: file.size, - mimeType: file.type || "application/octet-stream", - }); - if ("error" in result) return { error: result.error }; - } - return null; -} diff --git a/App/tests/integration/po-document-upload.test.ts b/App/tests/integration/po-document-upload.test.ts new file mode 100644 index 0000000..ac6ef3d --- /dev/null +++ b/App/tests/integration/po-document-upload.test.ts @@ -0,0 +1,140 @@ +/** + * Integration test for PO document upload + visibility (regression for the + * "documents uploaded but not visible anywhere" report). + * + * Drives the real `uploadPoDocuments` server action against a real DB and asserts + * that a `PODocument` row is created AND surfaced by the exact include the PO + * detail page uses — i.e. the attachment is actually visible. Storage I/O + * (`uploadBuffer`) is stubbed so the test doesn't depend on R2 / the filesystem; + * the bug was the missing DB row, which is asserted here for real. + */ +import { vi, describe, it, expect, beforeAll, afterEach } from "vitest"; + +vi.mock("@/auth", () => ({ auth: vi.fn() })); +vi.mock("next/cache", () => ({ revalidatePath: vi.fn() })); +// Keep buildStorageKey real (it shapes the storage key the UI groups on); stub +// only the actual storage write so the test is hermetic. +vi.mock("@/lib/storage", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, uploadBuffer: vi.fn().mockResolvedValue(undefined) }; +}); + +import { auth } from "@/auth"; +import { db } from "@/lib/db"; +import { uploadBuffer } from "@/lib/storage"; +import { createPo } from "@/app/(portal)/po/new/actions"; +import { uploadPoDocuments } from "@/app/actions/upload-po-documents"; +import { makeSession, getSeedUser, getSeedVessel, getSeedAccount, makePoForm, deletePosByTitle } from "./helpers"; + +const PREFIX = "INTTEST_PODOC_"; +let submitterId: string; +let vesselId: string; +let accountId: string; + +beforeAll(async () => { + const [user, vessel, account] = await Promise.all([ + getSeedUser("manager@pelagia.local"), + getSeedVessel("MV Pelagia Star"), + getSeedAccount("700201"), + ]); + submitterId = user.id; + vesselId = vessel.id; + accountId = account.id; +}); + +afterEach(async () => { + await deletePosByTitle(PREFIX); + vi.clearAllMocks(); +}); + +async function makePo(title: string): Promise { + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(makeSession(submitterId, "MANAGER")); + const result = await createPo(makePoForm({ title, vesselId, accountId, intent: "draft" })); + expect(result).not.toHaveProperty("error"); + return (result as { id: string }).id; +} + +function pdf(name: string, contents = "%PDF-1.4 hello"): File { + return new File([contents], name, { type: "application/pdf" }); +} + +describe("uploadPoDocuments", () => { + it("creates a PODocument row and stores the file, so it is visible on the PO", async () => { + const poId = await makePo(`${PREFIX}Visible`); + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(makeSession(submitterId, "MANAGER")); + + const file = pdf("invoice.pdf"); + const err = await uploadPoDocuments(poId, [file]); + expect(err).toBeNull(); + + // The file was actually handed to storage with its bytes + mime type. + expect(uploadBuffer).toHaveBeenCalledTimes(1); + const [key, buffer, mime] = vi.mocked(uploadBuffer).mock.calls[0]; + expect(key).toMatch(/^po-document\//); + expect(mime).toBe("application/pdf"); + expect(Buffer.isBuffer(buffer)).toBe(true); + + // The row exists — this is what was missing in the broken flow. + const docs = await db.pODocument.findMany({ where: { poId } }); + expect(docs).toHaveLength(1); + expect(docs[0]).toMatchObject({ + fileName: "invoice.pdf", + mimeType: "application/pdf", + storageKey: key, + }); + expect(docs[0].fileSize).toBeGreaterThan(0); + + // And it is surfaced by the exact include the PO detail page renders from. + const po = await db.purchaseOrder.findUnique({ + where: { id: poId }, + include: { documents: { orderBy: { uploadedAt: "desc" } } }, + }); + expect(po?.documents.map((d) => d.fileName)).toEqual(["invoice.pdf"]); + }); + + it("tags receipt uploads with the receipt prefix (delivery group)", async () => { + const poId = await makePo(`${PREFIX}Receipt`); + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(makeSession(submitterId, "MANAGER")); + + const err = await uploadPoDocuments(poId, [pdf("delivery-note.pdf")], "receipt"); + expect(err).toBeNull(); + + const doc = await db.pODocument.findFirstOrThrow({ where: { poId } }); + expect(doc.storageKey).toMatch(/^receipt\//); + }); + + it("stores every file when several are uploaded at once", async () => { + const poId = await makePo(`${PREFIX}Multi`); + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(makeSession(submitterId, "MANAGER")); + + const err = await uploadPoDocuments(poId, [pdf("a.pdf"), pdf("b.pdf"), pdf("c.pdf")]); + expect(err).toBeNull(); + expect(await db.pODocument.count({ where: { poId } })).toBe(3); + }); + + it("skips empty files without creating rows", async () => { + const poId = await makePo(`${PREFIX}Empty`); + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(makeSession(submitterId, "MANAGER")); + + const err = await uploadPoDocuments(poId, [new File([], "blank.pdf", { type: "application/pdf" })]); + expect(err).toBeNull(); + expect(uploadBuffer).not.toHaveBeenCalled(); + expect(await db.pODocument.count({ where: { poId } })).toBe(0); + }); + + it("rejects an unauthenticated caller and writes nothing", async () => { + const poId = await makePo(`${PREFIX}NoAuth`); + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(null); + + const err = await uploadPoDocuments(poId, [pdf("x.pdf")]); + expect(err).toEqual({ error: "Unauthorized" }); + expect(await db.pODocument.count({ where: { poId } })).toBe(0); + }); + + it("errors when the PO does not exist", async () => { + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(makeSession(submitterId, "MANAGER")); + const err = await uploadPoDocuments("nonexistent-po-id", [pdf("x.pdf")]); + expect(err).toEqual({ error: "PO not found" }); + expect(uploadBuffer).not.toHaveBeenCalled(); + }); +});