fix(po): upload attachments server-side so they persist & show #144
10 changed files with 272 additions and 3 deletions
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
```
|
||||
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
55
App/components/po/closed-po-attachment-uploader.tsx
Normal file
55
App/components/po/closed-po-attachment-uploader.tsx
Normal file
|
|
@ -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<File[]>([]);
|
||||
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 (
|
||||
<div className="mt-5 border-t border-neutral-100 pt-4">
|
||||
<p className="text-xs font-semibold uppercase tracking-wide text-neutral-500">Add attachments</p>
|
||||
<p className="mt-0.5 text-xs text-neutral-400">
|
||||
This purchase order is closed. Attach any documents that are missing.
|
||||
</p>
|
||||
<div className="mt-3">
|
||||
<FileUploader files={files} onChange={setFiles} disabled={busy} />
|
||||
</div>
|
||||
{error && <p className="mt-2 text-sm text-danger-700">{error}</p>}
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleUpload}
|
||||
disabled={busy || files.length === 0}
|
||||
className="mt-3 rounded-lg bg-primary-600 px-4 py-2 text-sm font-semibold text-white hover:opacity-90 disabled:opacity-50"
|
||||
>
|
||||
{busy ? "Uploading…" : `Upload${files.length > 0 ? ` ${files.length} file${files.length > 1 ? "s" : ""}` : ""}`}
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
@ -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) && (
|
||||
<div className="rounded-lg border border-neutral-200 bg-white p-6">
|
||||
<h3 className="text-sm font-semibold text-neutral-900 mb-4">Attachments</h3>
|
||||
{attachmentGroups.length === 0 && (
|
||||
<p className="text-sm text-neutral-400">No attachments yet.</p>
|
||||
)}
|
||||
<div className="space-y-5">
|
||||
{attachmentGroups.map((group) => (
|
||||
<div key={group.meta.key}>
|
||||
|
|
@ -531,6 +543,7 @@ export async function PoDetail({ po, currentUserId, currentRole, readOnly = fals
|
|||
</div>
|
||||
))}
|
||||
</div>
|
||||
{canAddClosedAttachment && <ClosedPoAttachmentUploader poId={po.id} />}
|
||||
</div>
|
||||
)}
|
||||
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
131
App/tests/integration/closed-po-attachments.test.ts
Normal file
131
App/tests/integration/closed-po-attachments.test.ts
Normal file
|
|
@ -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<typeof import("@/lib/storage")>();
|
||||
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<typeof import("@/lib/feature-flags")>();
|
||||
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<string, string> = {};
|
||||
|
||||
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<unknown>).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<string> {
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
|
@ -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<unknown>).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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue