diff --git a/App/.env.example b/App/.env.example index e4a3b60..a92b171 100644 --- a/App/.env.example +++ b/App/.env.example @@ -78,6 +78,11 @@ FORGEJO_TOKEN= # Let submitters (TECHNICAL/MANNING) read & export every PO and open the History # page (read-only). Opt-in — on only when exactly "true". # NEXT_PUBLIC_SUBMITTER_VIEW_ALL_ENABLED=true +# Let a PO's own submitter (plus Accounts/Manager/SuperUser) add attachments to it +# in any state except rejected/cancelled — remediation for POs whose uploads were +# lost to the document-upload bug, and the general "attach after the fact" affordance. +# Opt-in — on only when exactly "true". +# NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED=true # ── Non-production banner ───────────────────────────────────── # When set, a fixed "internal dev / staging" banner is shown (EnvBanner). diff --git a/App/CLAUDE.md b/App/CLAUDE.md index 24aecdf..26ad355 100644 --- a/App/CLAUDE.md +++ b/App/CLAUDE.md @@ -295,6 +295,7 @@ APP_INTERNAL_URL # Base URL PdfService reaches the app at (falls back to NEXT_PUBLIC_INVENTORY_ENABLED # Inventory feature flag NEXT_PUBLIC_SUBMITTER_VIEW_ALL_ENABLED # Opt-in ("true"): submitters (TECHNICAL/MANNING) read & export every PO + History (read-only) NEXT_PUBLIC_CREWING_ENABLED # Crewing module feature flag (opt-in "true"; off by default) +NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED # Opt-in ("true"): a PO's own submitter + Accounts/Manager/SuperUser may add attachments in any state except rejected/cancelled (upload-bug remediation + general "attach after the fact"). Off by default. NEXT_PUBLIC_ENV_LABEL # When set, shows a non-prod banner (EnvBanner). Leave unset in prod. ``` 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 073180c..64f3da4 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 }; @@ -94,7 +94,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..3e1f8f5 --- /dev/null +++ b/App/app/actions/upload-po-documents.ts @@ -0,0 +1,86 @@ +"use server"; + +import { auth } from "@/auth"; +import { db } from "@/lib/db"; +import { buildStorageKey, uploadBuffer } from "@/lib/storage"; +import { canAddPoAttachment } from "@/lib/permissions"; +import { CLOSED_PO_ATTACHMENTS_ENABLED } from "@/lib/feature-flags"; +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, status: true, submitterId: true }, + }); + if (!po) return { error: "PO not found" }; + + // A voided PO never accepts attachments, regardless of the flag. + if (po.status === "REJECTED" || po.status === "CANCELLED") { + return { error: "Attachments can't be added to a rejected or cancelled purchase order." }; + } + + if (CLOSED_PO_ATTACHMENTS_ENABLED) { + // Feature on: only the PO's submitter + Accounts / Manager / SuperUser may + // attach, in any non-voided state. The normal create / receipt flows are run + // by exactly those actors, so they keep working. + const allowed = canAddPoAttachment(session.user.role, po.status, { + isSubmitter: po.submitterId === session.user.id, + }); + if (!allowed) { + return { error: "Adding attachments to this purchase order isn't allowed." }; + } + } else if (po.status === "CLOSED") { + // Feature off: a closed PO stays immutable (legacy behaviour). The create / + // receipt flows upload while the PO is pre-CLOSED, so they're unaffected. + return { error: "Adding attachments to a closed purchase order isn't allowed." }; + } + + 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/components/po/po-attachment-uploader.tsx b/App/components/po/po-attachment-uploader.tsx new file mode 100644 index 0000000..da57000 --- /dev/null +++ b/App/components/po/po-attachment-uploader.tsx @@ -0,0 +1,55 @@ +"use client"; + +import { useState } from "react"; +import { useRouter } from "next/navigation"; +import { FileUploader } from "@/components/po/file-uploader"; +import { uploadPoDocuments } from "@/app/actions/upload-po-documents"; + +/** + * Feature-flagged uploader shown on a PO's detail page so its submitter (or + * Accounts / Manager / SuperUser) can add documents after the fact — in any state + * except rejected/cancelled. Gating is decided server-side in po-detail.tsx; the + * server action re-checks the permission, so this component is only the UI. + */ +export function PoAttachmentUploader({ poId }: { poId: string }) { + const router = useRouter(); + const [files, setFiles] = useState([]); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(""); + + async function handleUpload() { + if (files.length === 0) return; + setBusy(true); + setError(""); + const err = await uploadPoDocuments(poId, files); + if (err) { + setError(err.error); + setBusy(false); + return; + } + setFiles([]); + setBusy(false); + router.refresh(); + } + + return ( +
+

Add attachments

+

+ Attach any documents that are missing from this purchase order. +

+
+ +
+ {error &&

{error}

} + +
+ ); +} diff --git a/App/components/po/po-detail.tsx b/App/components/po/po-detail.tsx index 36cebc2..afab8f2 100644 --- a/App/components/po/po-detail.tsx +++ b/App/components/po/po-detail.tsx @@ -5,13 +5,14 @@ import { DiscardDraftButton } from "@/components/po/discard-draft-button"; import { SubmitDraftButton } from "@/components/po/submit-draft-button"; import { CancelPoButton, SupersedeForm } from "@/components/po/cancel-po-controls"; import { EmailVendorButton } from "@/components/po/email-vendor-button"; +import { PoAttachmentUploader } from "@/components/po/po-attachment-uploader"; import { formatCurrency, formatDate, formatDateTime } from "@/lib/utils"; import { generateDownloadUrl } from "@/lib/storage"; import { groupAttachments } from "@/lib/attachments"; +import { canAddPoAttachment, hasPermission } from "@/lib/permissions"; import { TC_FIXED_LINE } from "@/lib/validations/po"; import { parsePoTerms } from "@/lib/terms"; import { actionLabel } from "@/lib/po-activity"; -import { hasPermission } from "@/lib/permissions"; import type { LineItemInput } from "@/lib/validations/po"; import type { Role } from "@prisma/client"; @@ -153,6 +154,13 @@ export async function PoDetail({ po, currentUserId, currentRole, readOnly = fals ); const attachmentGroups = groupAttachments(docsWithUrls); + // Feature-flagged: the PO's submitter (or Accounts / Manager / SuperUser) may add + // attachments after the fact, in any state except rejected/cancelled. Never in + // readOnly. The server action re-checks this permission. + const canAddAttachment = + !readOnly && + canAddPoAttachment(currentRole, po.status, { isSubmitter: po.submitter.id === currentUserId }); + const canConfirmReceipt = (po.status === "PAID_DELIVERED" || po.status === "PARTIALLY_CLOSED" || po.status === "PARTIALLY_PAID") && (po.submitter.id === currentUserId || currentRole === "SUPERUSER") && @@ -490,9 +498,12 @@ export async function PoDetail({ po, currentUserId, currentRole, readOnly = fals })()} {/* Documents — grouped by lifecycle stage (submission / payment / delivery) */} - {attachmentGroups.length > 0 && ( + {(attachmentGroups.length > 0 || canAddAttachment) && (

Attachments

+ {attachmentGroups.length === 0 && ( +

No attachments yet.

+ )}
{attachmentGroups.map((group) => (
@@ -523,6 +534,7 @@ export async function PoDetail({ po, currentUserId, currentRole, readOnly = fals
))}
+ {canAddAttachment && }
)} diff --git a/App/lib/feature-flags.ts b/App/lib/feature-flags.ts index be88910..b875545 100644 --- a/App/lib/feature-flags.ts +++ b/App/lib/feature-flags.ts @@ -15,6 +15,13 @@ * etc.). Opt-in (off unless explicitly "true") because the feature is built incrementally; * keeping it dark by default leaves production unchanged. See lib/permissions.ts (§6 matrix) * and wiki Crewing-Implementation-Spec. + * + * NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED=true → lets a PO's own submitter, plus + * Accounts / Manager / SuperUser, add attachments to it in any state EXCEPT + * rejected/cancelled. Remediation path for the upload bug where documents never persisted + * (no PODocument row), and the general "attach a document after the fact" affordance. + * Opt-in (off unless "true") so production is unchanged until enabled. + * See lib/permissions.ts (canAddPoAttachment). */ export const INVENTORY_ENABLED = @@ -25,3 +32,6 @@ export const SUBMITTER_VIEW_ALL_ENABLED = export const CREWING_ENABLED = process.env.NEXT_PUBLIC_CREWING_ENABLED === "true"; + +export const CLOSED_PO_ATTACHMENTS_ENABLED = + process.env.NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED === "true"; diff --git a/App/lib/permissions.ts b/App/lib/permissions.ts index bba6b6b..b094d1e 100644 --- a/App/lib/permissions.ts +++ b/App/lib/permissions.ts @@ -1,5 +1,5 @@ -import type { Role } from "@prisma/client"; -import { SUBMITTER_VIEW_ALL_ENABLED } from "./feature-flags"; +import type { Role, POStatus } from "@prisma/client"; +import { SUBMITTER_VIEW_ALL_ENABLED, CLOSED_PO_ATTACHMENTS_ENABLED } from "./feature-flags"; export type Permission = | "create_po" @@ -278,3 +278,31 @@ export function submitterCanViewAll(role: Role): boolean { export function canViewAllPos(role: Role): boolean { return hasPermission(role, "view_all_pos") || submitterCanViewAll(role); } + +// ── PO attachments (feature-flagged) ────────────────────────────────────────── +// Roles that may attach to a PO (besides the PO's own submitter, who is always +// allowed) when NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED=true. +const PO_ATTACHMENT_ROLES: Role[] = ["ACCOUNTS", "MANAGER", "SUPERUSER"]; + +// A PO in one of these terminal/voided states never accepts new attachments. +const NO_ATTACHMENT_STATUSES: POStatus[] = ["REJECTED", "CANCELLED"]; + +/** + * Feature-flagged: whether the current user may add attachments to a PO. + * + * When `NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED` is on, the PO's own submitter + * — plus Accounts / Manager / SuperUser — may attach documents to a PO in **any + * state except `REJECTED` / `CANCELLED`** (a voided PO is never editable). This + * is the remediation path for the upload bug where documents never persisted, and + * the general "add a document after the fact" affordance. Off by default ⇒ no + * post-hoc attachment UI and closed POs stay immutable. + */ +export function canAddPoAttachment( + role: Role, + status: POStatus, + opts: { isSubmitter: boolean } +): boolean { + if (!CLOSED_PO_ATTACHMENTS_ENABLED) return false; + if (NO_ATTACHMENT_STATUSES.includes(status)) return false; + return opts.isSubmitter || PO_ATTACHMENT_ROLES.includes(role); +} 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-attachment-permissions.test.ts b/App/tests/integration/po-attachment-permissions.test.ts new file mode 100644 index 0000000..0a541c8 --- /dev/null +++ b/App/tests/integration/po-attachment-permissions.test.ts @@ -0,0 +1,170 @@ +/** + * Integration test for the feature-flagged PO-attachment permission + * (NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED). With the flag ON, a PO's own + * submitter — plus Accounts / Manager / SuperUser — may attach documents to a PO + * in **any state except REJECTED / CANCELLED**; everyone else, and any voided PO, + * is refused. (The flag-OFF behaviour lives in po-document-upload.test.ts.) + */ +import { vi, describe, it, expect, beforeAll, afterEach } from "vitest"; + +vi.mock("@/auth", () => ({ auth: vi.fn() })); +vi.mock("next/cache", () => ({ revalidatePath: vi.fn() })); +vi.mock("@/lib/storage", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, uploadBuffer: vi.fn().mockResolvedValue(undefined) }; +}); +// Flip ONLY the attachment flag on; everything else stays real. +vi.mock("@/lib/feature-flags", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, CLOSED_PO_ATTACHMENTS_ENABLED: true }; +}); + +import { auth } from "@/auth"; +import { db } from "@/lib/db"; +import { uploadBuffer } from "@/lib/storage"; +import type { Role, POStatus } from "@prisma/client"; +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_POATTACH_"; +const VOID_ERROR = "Attachments can't be added to a rejected or cancelled purchase order."; +const DENY_ERROR = "Adding attachments to this purchase order isn't allowed."; + +let techId: string; // the PO's submitter +let vesselId: string; +let accountId: string; +const userIds: Record = {}; + +beforeAll(async () => { + const [tech, accounts, manager, superuser, manning, auditor, vessel, account] = await Promise.all([ + getSeedUser("tech@pelagia.local"), + getSeedUser("accounts@pelagia.local"), + getSeedUser("manager@pelagia.local"), + getSeedUser("superuser@pelagia.local"), + getSeedUser("manning@pelagia.local"), + getSeedUser("auditor@pelagia.local"), + getSeedVessel("MV Pelagia Star"), + getSeedAccount("700201"), + ]); + techId = tech.id; + vesselId = vessel.id; + accountId = account.id; + userIds.ACCOUNTS = accounts.id; + userIds.MANAGER = manager.id; + userIds.SUPERUSER = superuser.id; + userIds.MANNING = manning.id; + userIds.AUDITOR = auditor.id; +}); + +afterEach(async () => { + await deletePosByTitle(PREFIX); + vi.clearAllMocks(); +}); + +function as(userId: string, role: Role) { + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(makeSession(userId, role)); +} + +function pdf(name: string): File { + return new File(["%PDF-1.4 hello"], name, { type: "application/pdf" }); +} + +// A PO submitted by the TECHNICAL user, forced into `status`. +async function makePo(title: string, status: POStatus): Promise { + as(techId, "TECHNICAL"); + const result = await createPo(makePoForm({ title, vesselId, accountId, intent: "draft" })); + expect(result).not.toHaveProperty("error"); + const poId = (result as { id: string }).id; + if (status !== "DRAFT") { + await db.purchaseOrder.update({ where: { id: poId }, data: { status } }); + } + return poId; +} + +describe("PO attachment permissions (flag on)", () => { + it("lets the PO's own submitter attach to their PO", async () => { + const poId = await makePo(`${PREFIX}Submitter`, "CLOSED"); + as(techId, "TECHNICAL"); + + const err = await uploadPoDocuments(poId, [pdf("missing-invoice.pdf")]); + expect(err).toBeNull(); + expect(await db.pODocument.count({ where: { poId } })).toBe(1); + }); + + it.each<[string, Role]>([ + ["ACCOUNTS", "ACCOUNTS"], + ["MANAGER", "MANAGER"], + ["SUPERUSER", "SUPERUSER"], + ])("lets %s attach to a closed PO they did not submit", async (key, role) => { + const poId = await makePo(`${PREFIX}${key}`, "CLOSED"); + as(userIds[key], role); + + const err = await uploadPoDocuments(poId, [pdf("doc.pdf")]); + expect(err).toBeNull(); + expect(await db.pODocument.count({ where: { poId } })).toBe(1); + }); + + // The headline of this change: not just CLOSED — any live state. + it.each(["MGR_REVIEW", "MGR_APPROVED", "SENT_FOR_PAYMENT", "PAID_DELIVERED", "EDITS_REQUESTED"])( + "lets Manager attach to a PO in %s", + async (status) => { + const poId = await makePo(`${PREFIX}${status}`, status); + as(userIds.MANAGER, "MANAGER"); + + const err = await uploadPoDocuments(poId, [pdf("doc.pdf")]); + expect(err).toBeNull(); + expect(await db.pODocument.count({ where: { poId } })).toBe(1); + } + ); + + it.each(["REJECTED", "CANCELLED"])( + "refuses attachments to a %s PO, even for Manager", + async (status) => { + const poId = await makePo(`${PREFIX}${status}`, status); + as(userIds.MANAGER, "MANAGER"); + + const err = await uploadPoDocuments(poId, [pdf("doc.pdf")]); + expect(err).toEqual({ error: VOID_ERROR }); + expect(uploadBuffer).not.toHaveBeenCalled(); + expect(await db.pODocument.count({ where: { poId } })).toBe(0); + } + ); + + it("refuses a voided PO even for its own submitter", async () => { + const poId = await makePo(`${PREFIX}SubmitterRejected`, "REJECTED"); + as(techId, "TECHNICAL"); + + const err = await uploadPoDocuments(poId, [pdf("doc.pdf")]); + expect(err).toEqual({ error: VOID_ERROR }); + expect(await db.pODocument.count({ where: { poId } })).toBe(0); + }); + + it("refuses a submitter-role user who is not this PO's submitter", async () => { + const poId = await makePo(`${PREFIX}OtherSubmitter`, "MGR_APPROVED"); + as(userIds.MANNING, "MANNING"); // a submitter role, but not the PO's submitter + + const err = await uploadPoDocuments(poId, [pdf("doc.pdf")]); + expect(err).toEqual({ error: DENY_ERROR }); + expect(uploadBuffer).not.toHaveBeenCalled(); + expect(await db.pODocument.count({ where: { poId } })).toBe(0); + }); + + it("refuses a role outside the allow-list (auditor)", async () => { + const poId = await makePo(`${PREFIX}Auditor`, "CLOSED"); + as(userIds.AUDITOR, "AUDITOR"); + + const err = await uploadPoDocuments(poId, [pdf("doc.pdf")]); + expect(err).toEqual({ error: DENY_ERROR }); + expect(await db.pODocument.count({ where: { poId } })).toBe(0); + }); + + it("still allows the normal create flow (DRAFT submitter)", async () => { + const poId = await makePo(`${PREFIX}Draft`, "DRAFT"); + as(techId, "TECHNICAL"); + + const err = await uploadPoDocuments(poId, [pdf("draft-doc.pdf")]); + expect(err).toBeNull(); + expect(await db.pODocument.count({ where: { poId } })).toBe(1); + }); +}); 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..db97290 --- /dev/null +++ b/App/tests/integration/po-document-upload.test.ts @@ -0,0 +1,154 @@ +/** + * 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(); + }); + + // The closed-PO remediation flag is OFF here (no mock; env unset), so a CLOSED PO + // stays immutable even for its own submitter. The flag-ON matrix lives in + // closed-po-attachments.test.ts. + it("refuses to attach to a CLOSED PO while the flag is off", async () => { + const poId = await makePo(`${PREFIX}ClosedOff`); + await db.purchaseOrder.update({ where: { id: poId }, data: { status: "CLOSED" } }); + vi.mocked(auth as unknown as () => Promise).mockResolvedValue(makeSession(submitterId, "MANAGER")); + + const err = await uploadPoDocuments(poId, [pdf("late.pdf")]); + expect(err).toEqual({ error: "Adding attachments to a closed purchase order isn't allowed." }); + expect(uploadBuffer).not.toHaveBeenCalled(); + expect(await db.pODocument.count({ where: { poId } })).toBe(0); + }); +}); diff --git a/automation/staging-up.sh b/automation/staging-up.sh index b80f9fb..8475890 100644 --- a/automation/staging-up.sh +++ b/automation/staging-up.sh @@ -43,6 +43,7 @@ AZURE_AD_TENANT_ID="dev-placeholder" DATABASE_URL="$TEST_URL" GST_SERVICE_URL="http://localhost:3003" NEXT_PUBLIC_SUBMITTER_VIEW_ALL_ENABLED=true +NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED=true NEXT_PUBLIC_ENV_LABEL="INTERNAL DEV / STAGING - NOT PRODUCTION" PORT=$PORT EOF @@ -55,6 +56,10 @@ fi if ! grep -qE '^NEXT_PUBLIC_SUBMITTER_VIEW_ALL_ENABLED=' "$DIR/App/.env"; then printf 'NEXT_PUBLIC_SUBMITTER_VIEW_ALL_ENABLED=true\n' >> "$DIR/App/.env" fi +# Closed-PO attachment remediation — let staging exercise fixing bug-affected POs. +if ! grep -qE '^NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED=' "$DIR/App/.env"; then + printf 'NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED=true\n' >> "$DIR/App/.env" +fi # pm2-run wrapper so the dev server always gets nvm on PATH and the right port. # Bind to 127.0.0.1 only -- staging is reachable solely via SSH tunnel