From d796e81efc3abb97fa81a86c8d6dddccfe593239 Mon Sep 17 00:00:00 2001
From: Hardik <shardik99@gmail.com>
Date: Mon, 22 Jun 2026 23:32:30 +0530
Subject: [PATCH] fix(crewing): harden onboardCandidate (D1 guard, D3 metadata,
 atomic contract)

- D1: require a Manager-approved SalaryStructure before onboarding; a SELECTED
  application with none is now blocked instead of silently binding zero salary
  rows.
- D3 AC2: the CREW_ONBOARDED CrewAction records the created IDs
  (assignmentId, employeeId, salaryStructureId) in metadata.
- Atomicity: the contract letter is uploaded before the transaction and its row
  is created INSIDE it, so onboarding is one atomic write (no half-onboarded
  crew member without a contract on failure).

onboarding.test.ts asserts the metadata and the new D1 block (no assignment, the
candidate stays a CANDIDATE).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../(portal)/crewing/applications/actions.ts  | 51 +++++++++++++++----
 App/tests/integration/onboarding.test.ts      | 23 +++++++++
 2 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/App/app/(portal)/crewing/applications/actions.ts b/App/app/(portal)/crewing/applications/actions.ts
index 4ea7f32..8abf91c 100644
--- a/App/app/(portal)/crewing/applications/actions.ts
+++ b/App/app/(portal)/crewing/applications/actions.ts
@@ -562,8 +562,33 @@ export async function onboardCandidate(formData: FormData): Promise<ActionResult
   });
   if (!app) return { error: "Application not found" };
   if (app.stage !== "SELECTED") return { error: `Only a SELECTED candidate can be onboarded (currently ${app.stage})` };
+
+  // D1 (spec §8.5): onboarding is blocked until the salary structure is
+  // Manager-approved. Without this guard a SELECTED application that somehow has
+  // no approved structure would still "succeed" but bind zero salary rows
+  // (the updateMany below would match nothing) — a silent payroll gap.
+  const approvedSalary = await db.salaryStructure.findFirst({
+    where: { applicationId: id, approvedById: { not: null }, assignmentId: null },
+    select: { id: true },
+    orderBy: { createdAt: "desc" },
+  });
+  if (!approvedSalary) return { error: "Salary structure must be Manager-approved before onboarding" };
+
   const joiningDate = new Date(joiningStr);
 
+  // Upload the optional contract letter BEFORE the transaction (storage I/O),
+  // then persist its row INSIDE the tx so onboarding is one atomic side-effecting
+  // event (spec §11). The blob key is keyed on the crew member (stable before the
+  // assignment exists); if the tx fails we leave only a harmless orphan blob,
+  // never a fully-onboarded crew member with no contract row.
+  const file = formData.get("contract");
+  let contract: { fileKey: string; salaryRestricted: boolean } | null = null;
+  if (file instanceof File && file.size > 0) {
+    const key = buildStorageKey("contract", app.crewMember.id, file.name);
+    await uploadBuffer(key, Buffer.from(await file.arrayBuffer()), file.type || "application/octet-stream");
+    contract = { fileKey: key, salaryRestricted: formData.get("salaryRestricted") !== "false" };
+  }
+
   const result = await db.$transaction(async (tx) => {
     const employeeId = await generateEmployeeId(tx);
     const assignment = await tx.crewAssignment.create({
@@ -582,9 +607,23 @@ export async function onboardCandidate(formData: FormData): Promise<ActionResult
       where: { applicationId: id, approvedById: { not: null }, assignmentId: null },
       data: { assignmentId: assignment.id, effectiveFrom: joiningDate },
     });
+    if (contract) {
+      await tx.contractLetter.create({ data: { assignmentId: assignment.id, fileKey: contract.fileKey, salaryRestricted: contract.salaryRestricted } });
+    }
+    // D3 AC2 (spec §11): the single CREW_ONBOARDED audit row records the created IDs.
     await tx.application.update({
       where: { id },
-      data: { stage: "ONBOARDED", actions: { create: { actionType: "CREW_ONBOARDED", actorId: g.userId, crewMemberId: app.crewMember.id } } },
+      data: {
+        stage: "ONBOARDED",
+        actions: {
+          create: {
+            actionType: "CREW_ONBOARDED",
+            actorId: g.userId,
+            crewMemberId: app.crewMember.id,
+            metadata: { assignmentId: assignment.id, employeeId, salaryStructureId: approvedSalary.id },
+          },
+        },
+      },
     });
     await tx.requisition.update({
       where: { id: app.requisition.id },
@@ -599,16 +638,6 @@ export async function onboardCandidate(formData: FormData): Promise<ActionResult
     return { assignmentId: assignment.id, employeeId };
   });
 
-  // Contract letter (optional) — stored after the core transaction.
-  const file = formData.get("contract");
-  if (file instanceof File && file.size > 0) {
-    const key = buildStorageKey("contract", result.assignmentId, file.name);
-    await uploadBuffer(key, Buffer.from(await file.arrayBuffer()), file.type || "application/octet-stream");
-    await db.contractLetter.create({
-      data: { assignmentId: result.assignmentId, fileKey: key, salaryRestricted: formData.get("salaryRestricted") !== "false" },
-    });
-  }
-
   revalidateApp(id, app.requisition.id);
   return { ok: true, id: result.employeeId };
 }
diff --git a/App/tests/integration/onboarding.test.ts b/App/tests/integration/onboarding.test.ts
index 4e833ff..6e20488 100644
--- a/App/tests/integration/onboarding.test.ts
+++ b/App/tests/integration/onboarding.test.ts
@@ -88,6 +88,29 @@ describe("onboardCandidate", () => {
 
     const action = await db.crewAction.findFirstOrThrow({ where: { actionType: "CREW_ONBOARDED" } });
     expect(action.actorId).toBe(managerId);
+    // D3 AC2: the audit row records the created IDs in metadata.
+    const meta = action.metadata as { assignmentId?: string; employeeId?: string; salaryStructureId?: string } | null;
+    expect(meta?.assignmentId).toBe(assignment.id);
+    expect(meta?.employeeId).toBe(cm.employeeId);
+    expect(meta?.salaryStructureId).toBe(sal.id);
+  });
+
+  it("blocks onboarding when no salary structure is Manager-approved (D1)", async () => {
+    seq += 1;
+    const req = await db.requisition.create({ data: { code: `REQ-O${seq}`, rankId, vesselId, reason: "NEW_VACANCY", status: "SELECTED" } });
+    const cand = await db.crewMember.create({ data: { name: "Unapproved Sal", type: "NEW", status: "CANDIDATE", source: "CAREERS", appliedRankId: rankId } });
+    const appRow = await db.application.create({ data: { requisitionId: req.id, crewMemberId: cand.id, stage: "SELECTED", type: "NEW" } });
+    // Salary agreed but NOT Manager-approved (approvedById null).
+    await db.salaryStructure.create({ data: { applicationId: appRow.id, rateBasis: "MONTHLY", basic: 40000 } });
+
+    as(managerId, "MANAGER");
+    const res = await onboardCandidate(fd({ applicationId: appRow.id, joiningDate: "2026-07-01" }));
+    expect("error" in res).toBe(true);
+    expect(await db.crewAssignment.count()).toBe(0);
+    // The candidate is untouched — still a CANDIDATE, no employee number.
+    const after = await db.crewMember.findUniqueOrThrow({ where: { id: cand.id } });
+    expect(after.status).toBe("CANDIDATE");
+    expect(after.employeeId).toBeNull();
   });
 
   it("requires a joining date", async () => {