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 <noreply@anthropic.com>
This commit is contained in:
parent
bf7ea1a9e6
commit
2afdeec6ad
7 changed files with 204 additions and 104 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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 };
|
||||
}
|
||||
60
App/app/actions/upload-po-documents.ts
Normal file
60
App/app/actions/upload-po-documents.ts
Normal file
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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 });
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
140
App/tests/integration/po-document-upload.test.ts
Normal file
140
App/tests/integration/po-document-upload.test.ts
Normal file
|
|
@ -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<typeof import("@/lib/storage")>();
|
||||
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<string> {
|
||||
vi.mocked(auth as unknown as () => Promise<unknown>).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<unknown>).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<unknown>).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<unknown>).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<unknown>).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<unknown>).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<unknown>).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();
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue