From 158b4461177db04c4a696580e453b6ece3629cf9 Mon Sep 17 00:00:00 2001 From: Hardik Date: Sun, 28 Jun 2026 01:11:29 +0530 Subject: [PATCH] feat(po): feature-flagged attachments on closed POs (bug remediation) 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 --- App/.env.example | 4 + App/CLAUDE.md | 1 + App/app/actions/upload-po-documents.ts | 18 ++- .../po/closed-po-attachment-uploader.tsx | 55 ++++++++ App/components/po/po-detail.tsx | 15 +- App/lib/feature-flags.ts | 9 ++ App/lib/permissions.ts | 23 ++- .../integration/closed-po-attachments.test.ts | 131 ++++++++++++++++++ .../integration/po-document-upload.test.ts | 14 ++ automation/staging-up.sh | 5 + 10 files changed, 272 insertions(+), 3 deletions(-) create mode 100644 App/components/po/closed-po-attachment-uploader.tsx create mode 100644 App/tests/integration/closed-po-attachments.test.ts diff --git a/App/.env.example b/App/.env.example index e4a3b60..3b7c413 100644 --- a/App/.env.example +++ b/App/.env.example @@ -78,6 +78,10 @@ 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 CLOSED PO's own submitter (plus Accounts/Manager/SuperUser) add attachments +# to it — remediation for POs whose uploads were lost to the document-upload bug. +# 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..4d02d19 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 CLOSED PO's own submitter + Accounts/Manager/SuperUser may add attachments (remediation for the upload bug). Off by default. NEXT_PUBLIC_ENV_LABEL # When set, shows a non-prod banner (EnvBanner). Leave unset in prod. ``` diff --git a/App/app/actions/upload-po-documents.ts b/App/app/actions/upload-po-documents.ts index 713ea0a..1dbabbd 100644 --- a/App/app/actions/upload-po-documents.ts +++ b/App/app/actions/upload-po-documents.ts @@ -3,6 +3,7 @@ import { auth } from "@/auth"; import { db } from "@/lib/db"; import { buildStorageKey, uploadBuffer } from "@/lib/storage"; +import { canAddClosedPoAttachment } from "@/lib/permissions"; import { revalidatePath } from "next/cache"; // Matches the FileUploader hint ("up to 10 MB each") and @@ -36,9 +37,24 @@ export async function uploadPoDocuments( const session = await auth(); if (!session?.user) return { error: "Unauthorized" }; - const po = await db.purchaseOrder.findUnique({ where: { id: poId }, select: { id: true } }); + const po = await db.purchaseOrder.findUnique({ + where: { id: poId }, + select: { id: true, status: true, submitterId: true }, + }); if (!po) return { error: "PO not found" }; + // A CLOSED PO is otherwise immutable; attaching to one is only allowed via the + // feature-flagged remediation path (canAddClosedPoAttachment). The normal create + // and receipt flows upload while the PO is pre-CLOSED, so they're unaffected. + if (po.status === "CLOSED") { + const allowed = canAddClosedPoAttachment(session.user.role, { + isSubmitter: po.submitterId === session.user.id, + }); + if (!allowed) { + 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) { diff --git a/App/components/po/closed-po-attachment-uploader.tsx b/App/components/po/closed-po-attachment-uploader.tsx new file mode 100644 index 0000000..c0eff65 --- /dev/null +++ b/App/components/po/closed-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 CLOSED PO's detail page so its submitter (or + * Accounts / Manager / SuperUser) can attach documents that were lost to the upload + * bug. 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 ClosedPoAttachmentUploader({ 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

+

+ This purchase order is closed. Attach any documents that are missing. +

+
+ +
+ {error &&

{error}

} + +
+ ); +} diff --git a/App/components/po/po-detail.tsx b/App/components/po/po-detail.tsx index 0982e34..590c9da 100644 --- a/App/components/po/po-detail.tsx +++ b/App/components/po/po-detail.tsx @@ -5,9 +5,11 @@ 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 { ClosedPoAttachmentUploader } from "@/components/po/closed-po-attachment-uploader"; import { formatCurrency, formatDate, formatDateTime } from "@/lib/utils"; import { generateDownloadUrl } from "@/lib/storage"; import { groupAttachments } from "@/lib/attachments"; +import { canAddClosedPoAttachment } from "@/lib/permissions"; import { TC_FIXED_LINE } from "@/lib/validations/po"; import { parsePoTerms } from "@/lib/terms"; import type { LineItemInput } from "@/lib/validations/po"; @@ -171,6 +173,13 @@ export async function PoDetail({ po, currentUserId, currentRole, readOnly = fals ); const attachmentGroups = groupAttachments(docsWithUrls); + // Feature-flagged remediation: a closed PO's submitter (or Accounts / Manager / + // SuperUser) may attach documents that the upload bug dropped. Never in readOnly. + const canAddClosedAttachment = + !readOnly && + po.status === "CLOSED" && + canAddClosedPoAttachment(currentRole, { 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") && @@ -498,9 +507,12 @@ export async function PoDetail({ po, currentUserId, currentRole, readOnly = fals })()} {/* Documents — grouped by lifecycle stage (submission / payment / delivery) */} - {attachmentGroups.length > 0 && ( + {(attachmentGroups.length > 0 || canAddClosedAttachment) && (

Attachments

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

No attachments yet.

+ )}
{attachmentGroups.map((group) => (
@@ -531,6 +543,7 @@ export async function PoDetail({ po, currentUserId, currentRole, readOnly = fals
))}
+ {canAddClosedAttachment && }
)} diff --git a/App/lib/feature-flags.ts b/App/lib/feature-flags.ts index be88910..c1fe01a 100644 --- a/App/lib/feature-flags.ts +++ b/App/lib/feature-flags.ts @@ -15,6 +15,12 @@ * 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 CLOSED PO's own submitter, plus + * Accounts / Manager / SuperUser, add attachments to it. Remediation path for the upload + * bug where documents never persisted (no PODocument row): closed POs whose files were lost + * can be fixed without reopening them. Opt-in (off unless "true") so production is unchanged + * until enabled. See lib/permissions.ts (canAddClosedPoAttachment). */ export const INVENTORY_ENABLED = @@ -25,3 +31,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..95be0ca 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 { SUBMITTER_VIEW_ALL_ENABLED, CLOSED_PO_ATTACHMENTS_ENABLED } from "./feature-flags"; export type Permission = | "create_po" @@ -278,3 +278,24 @@ export function submitterCanViewAll(role: Role): boolean { export function canViewAllPos(role: Role): boolean { return hasPermission(role, "view_all_pos") || submitterCanViewAll(role); } + +// ── Closed-PO attachments (feature-flagged remediation) ─────────────────────── +// Roles that may attach to ANY closed PO (the PO's own submitter is allowed too, +// regardless of role) when NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED=true. +const CLOSED_PO_ATTACHMENT_ROLES: Role[] = ["ACCOUNTS", "MANAGER", "SUPERUSER"]; + +/** + * Feature-flagged: whether the current user may add attachments to a CLOSED PO. + * This is the remediation path for the upload bug where documents never persisted + * — closed POs that lost their files can be re-attached without reopening them. + * + * Allowed (only when the flag is on) for the PO's own submitter, or for + * Accounts / Manager / SuperUser. Off by default ⇒ closed POs stay immutable. + */ +export function canAddClosedPoAttachment( + role: Role, + opts: { isSubmitter: boolean } +): boolean { + if (!CLOSED_PO_ATTACHMENTS_ENABLED) return false; + return opts.isSubmitter || CLOSED_PO_ATTACHMENT_ROLES.includes(role); +} diff --git a/App/tests/integration/closed-po-attachments.test.ts b/App/tests/integration/closed-po-attachments.test.ts new file mode 100644 index 0000000..017f5dc --- /dev/null +++ b/App/tests/integration/closed-po-attachments.test.ts @@ -0,0 +1,131 @@ +/** + * Integration test for the feature-flagged closed-PO attachment remediation + * (NEXT_PUBLIC_CLOSED_PO_ATTACHMENTS_ENABLED). With the flag ON, a CLOSED PO's own + * submitter — plus Accounts / Manager / SuperUser — may attach documents; everyone + * else is refused. (The flag-OFF case 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 remediation 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 } 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_CLOSEDPO_"; +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 CLOSED PO submitted by the TECHNICAL user. +async function makeClosedPo(title: string): 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; + await db.purchaseOrder.update({ where: { id: poId }, data: { status: "CLOSED" } }); + return poId; +} + +describe("closed-PO attachments (flag on)", () => { + it("lets the PO's own submitter attach to their closed PO", async () => { + const poId = await makeClosedPo(`${PREFIX}Submitter`); + 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 makeClosedPo(`${PREFIX}${key}`); + as(userIds[key], role); + + const err = await uploadPoDocuments(poId, [pdf("doc.pdf")]); + expect(err).toBeNull(); + expect(await db.pODocument.count({ where: { poId } })).toBe(1); + }); + + it("refuses a submitter-role user who is not this PO's submitter", async () => { + const poId = await makeClosedPo(`${PREFIX}OtherSubmitter`); + 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: "Adding attachments to a closed purchase order isn't allowed." }); + 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 makeClosedPo(`${PREFIX}Auditor`); + as(userIds.AUDITOR, "AUDITOR"); + + const err = await uploadPoDocuments(poId, [pdf("doc.pdf")]); + expect(err).toEqual({ error: "Adding attachments to a closed purchase order isn't allowed." }); + expect(await db.pODocument.count({ where: { poId } })).toBe(0); + }); + + it("still allows uploads to a non-closed PO (normal flow unaffected)", async () => { + as(techId, "TECHNICAL"); + const result = await createPo(makePoForm({ title: `${PREFIX}Draft`, vesselId, accountId, intent: "draft" })); + const poId = (result as { id: string }).id; // stays 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 index ac6ef3d..db97290 100644 --- a/App/tests/integration/po-document-upload.test.ts +++ b/App/tests/integration/po-document-upload.test.ts @@ -137,4 +137,18 @@ describe("uploadPoDocuments", () => { 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