diff --git a/actions/approvals.ts b/actions/approvals.ts index 348f9ef..1594745 100644 --- a/actions/approvals.ts +++ b/actions/approvals.ts @@ -5,7 +5,7 @@ import { db } from "@/lib/db"; import { revalidatePath } from "next/cache"; import { z } from "zod"; import { ApprovalStatus } from "@prisma/client"; -import { recalcShotStatus } from "@/lib/shot-status"; +import { recalcShotStatus, clearStaleShotApproval } from "@/lib/shot-status"; import { notifyApprovalChange } from "@/lib/notifications"; import { slackNotifyApproval } from "@/lib/slack"; import { versionLabel } from "@/lib/utils"; @@ -29,57 +29,62 @@ export async function submitApproval(data: z.infer) { throw new Error("Insufficient permissions to review versions"); } - // Create the approval record - const approval = await db.approval.create({ - data: { - versionId: parsed.versionId, - userId: session.user.id, - status: parsed.status, - notes: parsed.notes, - }, - }); + // The verdict, the version, the task and the derived shot status are one + // decision and are written as one: a failure partway through used to leave the + // three layers disagreeing, with no way to tell which one was right. + const { approval, version } = await db.$transaction(async (tx) => { + const approval = await tx.approval.create({ + data: { + versionId: parsed.versionId, + userId: session.user.id, + status: parsed.status, + notes: parsed.notes, + }, + }); - // Update version approval status - await db.version.update({ - where: { id: parsed.versionId }, - data: { approvalStatus: parsed.status }, - }); + await tx.version.update({ + where: { id: parsed.versionId }, + data: { approvalStatus: parsed.status }, + }); - // Load version + task + shot + project for downstream effects - const version = await db.version.findUnique({ - where: { id: parsed.versionId }, - include: { - task: { - include: { - shot: true, - project: true, + // Load version + task + shot + project for downstream effects + const version = await tx.version.findUnique({ + where: { id: parsed.versionId }, + include: { + task: { + include: { + shot: true, + project: true, + }, }, }, - }, + }); + + if (!version) throw new Error("Version not found"); + + // Update task status based on approval result + if (version.task && parsed.status !== "PENDING_REVIEW") { + if (parsed.status === "APPROVED") { + await tx.task.update({ + where: { id: version.task.id }, + data: { status: "DONE" }, + }); + } else if (parsed.status === "REJECTED" || parsed.status === "NEEDS_CHANGES") { + await tx.task.update({ + where: { id: version.task.id }, + data: { status: "CHANGES" }, + }); + } + + // Recalculate shot status from updated task states + if (version.task.shot) { + await recalcShotStatus(version.task.shot.id, tx); + } + } + + return { approval, version }; }); - if (!version) throw new Error("Version not found"); - - // Update task status based on approval result - if (version.task && parsed.status !== "PENDING_REVIEW") { - if (parsed.status === "APPROVED") { - await db.task.update({ - where: { id: version.task.id }, - data: { status: "DONE" }, - }); - } else if (parsed.status === "REJECTED" || parsed.status === "NEEDS_CHANGES") { - await db.task.update({ - where: { id: version.task.id }, - data: { status: "CHANGES" }, - }); - } - - // Recalculate shot status from updated task states - if (version.task.shot) { - await recalcShotStatus(version.task.shot.id).catch(() => {}); - } - } - // Notifications const reviewer = await db.user.findUnique({ where: { id: session.user.id } }); const reviewerName = reviewer?.name ?? "Reviewer"; @@ -166,34 +171,39 @@ export async function unapproveVersion(versionId: string) { throw new Error("Only approved versions can be unapproved"); } - // Create an audit record for the unapproval - await db.approval.create({ - data: { - versionId, - userId: session.user.id, - status: "PENDING_REVIEW", - notes: "Approval undone", - }, - }); - - // Reset version approval status - await db.version.update({ - where: { id: versionId }, - data: { approvalStatus: "PENDING_REVIEW" }, - }); - - // Reset task back to INTERNAL_REVIEW if it was moved to DONE - if (version.task && version.task.status === "DONE") { - await db.task.update({ - where: { id: version.task.id }, - data: { status: "INTERNAL_REVIEW" }, + // One decision, one write — same reasoning as submitApproval. + await db.$transaction(async (tx) => { + // Create an audit record for the unapproval + await tx.approval.create({ + data: { + versionId, + userId: session.user.id, + status: "PENDING_REVIEW", + notes: "Approval undone", + }, }); - } - // Recalculate shot status - if (version.task?.shot) { - await recalcShotStatus(version.task.shot.id).catch(() => {}); - } + // Reset version approval status + await tx.version.update({ + where: { id: versionId }, + data: { approvalStatus: "PENDING_REVIEW" }, + }); + + // Reset task back to INTERNAL_REVIEW if it was moved to DONE + if (version.task && version.task.status === "DONE") { + await tx.task.update({ + where: { id: version.task.id }, + data: { status: "INTERNAL_REVIEW" }, + }); + } + + // Undoing a version approval also invalidates any shot-level sign-off that + // rested on it, otherwise the shot keeps reading COMPLETE / READY_FOR_CLIENT. + if (version.task?.shot) { + await clearStaleShotApproval(tx, version.task.shot.id); + await recalcShotStatus(version.task.shot.id, tx); + } + }); revalidatePath(`/review/${versionId}`); if (version.task) { diff --git a/actions/shots.ts b/actions/shots.ts index 31d4884..e4fb331 100644 --- a/actions/shots.ts +++ b/actions/shots.ts @@ -5,7 +5,7 @@ import { db } from "@/lib/db"; import { revalidatePath } from "next/cache"; import { z } from "zod"; import { ShotStatus, ShotPriority, ShotApprovalStatus, Prisma } from "@prisma/client"; -import { recalcShotStatus } from "@/lib/shot-status"; +import { recalcShotStatus, markShotForRevisions } from "@/lib/shot-status"; const createShotSchema = z.object({ scene: z.string().min(1).max(50).regex(/^[A-Z0-9_]+$/i, "Alphanumeric and underscore only"), @@ -341,6 +341,15 @@ export async function importShotsFromCsv( return { success: true, created, errors }; } +/** + * Manual override of the shot's status. + * + * Deliberately the one place that writes Shot.status without deriving it — but + * note the override is transient: Shot.status is a derived field, so the next + * recalc (any task move, approval, upload or render) recomputes it from the + * tasks and approval flags and discards whatever was set here. Change the + * underlying task states if the new status needs to stick. + */ export async function updateShotStatus( shotId: string, status: ShotStatus @@ -737,13 +746,19 @@ export async function internallyApproveShot(shotId: string) { }); if (!shot) throw new Error("Shot not found"); - await db.shot.update({ - where: { id: shotId }, - data: { - shotApprovalStatus: "INTERNALLY_APPROVED", - sharedWithClient: false, - status: "READY_FOR_CLIENT", - }, + // Set the approval inputs and let the derivation pick the status. Writing + // READY_FOR_CLIENT directly skipped the rule that an outstanding CHANGES task + // outranks any approval, so a shot could be pushed past open rework and then + // snap back to REVISIONS on the next unrelated recalc. + await db.$transaction(async (tx) => { + await tx.shot.update({ + where: { id: shotId }, + data: { + shotApprovalStatus: "INTERNALLY_APPROVED", + sharedWithClient: false, + }, + }); + await recalcShotStatus(shotId, tx); }); revalidatePath(`/projects/${shot.projectId}`); @@ -786,7 +801,7 @@ export async function shareWithClient(shotId: string) { // Mark shot as shared await tx.shot.update({ where: { id: shotId }, - data: { sharedWithClient: true, status: "CLIENT_REVIEW" }, + data: { sharedWithClient: true }, }); // For each task, make the latest version client-visible @@ -806,6 +821,8 @@ export async function shareWithClient(shotId: string) { }); } } + + await recalcShotStatus(shotId, tx); }); revalidatePath(`/projects/${shot.projectId}`); @@ -838,7 +855,7 @@ export async function unshareFromClient(shotId: string) { await db.$transaction(async (tx) => { await tx.shot.update({ where: { id: shotId }, - data: { sharedWithClient: false, status: "READY_FOR_CLIENT" }, + data: { sharedWithClient: false }, }); // Hide all client-visible versions on this shot's tasks @@ -849,6 +866,8 @@ export async function unshareFromClient(shotId: string) { data: { isClientVisible: false }, }); } + + await recalcShotStatus(shotId, tx); }); revalidatePath(`/projects/${shot.projectId}`); @@ -877,9 +896,12 @@ export async function clientApproveShot(shotId: string) { if (!shot) throw new Error("Shot not found"); if (!shot.sharedWithClient) throw new Error("Shot is not shared with client"); - await db.shot.update({ - where: { id: shotId }, - data: { shotApprovalStatus: "CLIENT_APPROVED", status: "COMPLETE" }, + await db.$transaction(async (tx) => { + await tx.shot.update({ + where: { id: shotId }, + data: { shotApprovalStatus: "CLIENT_APPROVED" }, + }); + await recalcShotStatus(shotId, tx); }); revalidatePath(`/projects/${shot.projectId}`); @@ -909,14 +931,15 @@ export async function unapproveShot(shotId: string) { throw new Error("Shot is not client-approved"); } - const newStatus = shot.sharedWithClient ? "CLIENT_REVIEW" : "READY_FOR_CLIENT"; - - await db.shot.update({ - where: { id: shotId }, - data: { - shotApprovalStatus: "INTERNALLY_APPROVED", - status: newStatus, - }, + // The derivation already encodes "CLIENT_REVIEW if still shared, else + // READY_FOR_CLIENT" for an INTERNALLY_APPROVED shot, and additionally honours + // any open CHANGES task. + await db.$transaction(async (tx) => { + await tx.shot.update({ + where: { id: shotId }, + data: { shotApprovalStatus: "INTERNALLY_APPROVED" }, + }); + await recalcShotStatus(shotId, tx); }); revalidatePath(`/projects/${shot.projectId}`); @@ -1217,9 +1240,12 @@ export async function assignShotGroups( } /** - * Internal: handle client requesting changes on a shot. - * Resets shotApprovalStatus = PENDING, sharedWithClient = false. - * Recalculates shot status (will become REVISIONS if tasks set to CHANGES). + * Session-authenticated wrapper for requesting changes on a shot — a supervisor + * or producer relaying a client note from inside the app. + * + * The client portal cannot use this: it runs on a review token, not a session, + * so it calls markShotForRevisions directly from its route. Both go through that + * one helper so the two entry points cannot drift apart again. */ export async function clientRequestShotChanges(shotId: string, taskId?: string) { const session = await auth(); @@ -1234,28 +1260,7 @@ export async function clientRequestShotChanges(shotId: string, taskId?: string) }); if (!shot) throw new Error("Shot not found"); - await db.$transaction(async (tx) => { - // Reset shot approval - await tx.shot.update({ - where: { id: shotId }, - data: { shotApprovalStatus: "PENDING", sharedWithClient: false }, - }); - - // If a specific task is called out, mark it CHANGES; otherwise mark all non-DONE tasks - if (taskId) { - await tx.task.update({ - where: { id: taskId }, - data: { status: "CHANGES" }, - }); - } else { - await tx.task.updateMany({ - where: { shotId, status: { not: "DONE" } }, - data: { status: "CHANGES" }, - }); - } - - await recalcShotStatus(shotId, tx); - }); + await db.$transaction((tx) => markShotForRevisions(tx, shotId, taskId)); revalidatePath(`/projects/${shot.projectId}`); revalidatePath(`/projects/${shot.projectId}/shots/${shotId}`); diff --git a/actions/versions.ts b/actions/versions.ts index 7a06721..f223388 100644 --- a/actions/versions.ts +++ b/actions/versions.ts @@ -4,7 +4,7 @@ import { auth } from "@/auth"; import { db } from "@/lib/db"; import { revalidatePath } from "next/cache"; import { z } from "zod"; -import { recalcShotStatus } from "@/lib/shot-status"; +import { recalcShotStatus, clearStaleShotApproval } from "@/lib/shot-status"; import { notifyNewVersionUploaded } from "@/lib/notifications"; import { slackNotifyVersionUploaded } from "@/lib/slack"; @@ -50,9 +50,16 @@ export async function createVersion(data: z.infer) { data: { status: "INTERNAL_REVIEW" }, }); - // Recalculate shot status + // Recalculate shot status — clearing any stale sign-off first, so a new + // version does not land under a shot still reading COMPLETE. if (task.shot) { - await recalcShotStatus(task.shot.id).catch(() => {}); + const shotId = task.shot.id; + await db + .$transaction(async (tx) => { + await clearStaleShotApproval(tx, shotId); + await recalcShotStatus(shotId, tx); + }) + .catch(() => {}); } // Get the next version number for this task diff --git a/app/api/batch-upload/upload/route.ts b/app/api/batch-upload/upload/route.ts index ef3543d..6966874 100644 --- a/app/api/batch-upload/upload/route.ts +++ b/app/api/batch-upload/upload/route.ts @@ -2,7 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { auth } from "@/auth"; import { db } from "@/lib/db"; import { uploadToHetzner, deleteFromHetzner } from "@/lib/storage"; -import { recalcShotStatus } from "@/lib/shot-status"; +import { recalcShotStatus, clearStaleShotApproval } from "@/lib/shot-status"; export const maxDuration = 300; // 5 min — large .mov high-res files @@ -194,8 +194,14 @@ export async function POST( data: { status: "INTERNAL_REVIEW" }, }); - // Recalculate the parent shot's status - await recalcShotStatus(shotId).catch(() => {}); + // Recalculate the parent shot's status, clearing any stale sign-off first so + // the new version does not land under a shot still reading COMPLETE. + await db + .$transaction(async (tx) => { + await clearStaleShotApproval(tx, shotId); + await recalcShotStatus(shotId, tx); + }) + .catch(() => {}); return NextResponse.json({ success: true, diff --git a/app/api/client/[token]/approve/route.ts b/app/api/client/[token]/approve/route.ts index aecdab9..82deee6 100644 --- a/app/api/client/[token]/approve/route.ts +++ b/app/api/client/[token]/approve/route.ts @@ -1,7 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { db } from "@/lib/db"; import { ApprovalStatus } from "@prisma/client"; -import { recalcShotStatus } from "@/lib/shot-status"; +import { recalcShotStatus, markShotForRevisions } from "@/lib/shot-status"; import { validateReviewToken } from "@/lib/review-auth"; async function getOrCreateClientUser(email: string, label?: string | null) { @@ -48,9 +48,12 @@ export async function POST( } if (action === "APPROVE") { - await db.shot.update({ - where: { id: shotId }, - data: { shotApprovalStatus: "CLIENT_APPROVED", status: "COMPLETE" }, + await db.$transaction(async (tx) => { + await tx.shot.update({ + where: { id: shotId }, + data: { shotApprovalStatus: "CLIENT_APPROVED" }, + }); + await recalcShotStatus(shotId, tx); }); } else if (action === "NEEDS_CHANGES") { // Increment shotVersion: v001 → v002, v009 → v010, etc. @@ -59,17 +62,42 @@ export async function POST( const nextNum = verMatch ? parseInt(verMatch[1], 10) + 1 : 2; const nextVersion = "v" + String(nextNum).padStart(3, "0"); + const email = session.email ?? `client+${token.slice(0, 8)}@review.external`; + const user = await getOrCreateClientUser(email, session.label); + await db.$transaction(async (tx) => { await tx.shot.update({ where: { id: shotId }, - data: { shotApprovalStatus: "PENDING", sharedWithClient: false, shotVersion: nextVersion }, + data: { shotVersion: nextVersion }, }); - // Mark all non-DONE tasks as CHANGES - await tx.task.updateMany({ - where: { shotId, status: { not: "DONE" } }, - data: { status: "CHANGES" }, + + // Clears the approval, unshares, marks the tasks needing rework and + // recalculates — including the case where every task was already DONE, + // which used to match nothing and leave the request with no effect. + await markShotForRevisions(tx, shotId); + + // The shot-level verdict has to land on the versions too: the dashboard + // counters, the version badges and the client portal's own filters all + // read Version.approvalStatus, so a shot-level change request was + // invisible everywhere those look. + const shown = await tx.version.findMany({ + where: { task: { shotId }, isClientVisible: true, isLatest: true }, + select: { id: true }, }); - await recalcShotStatus(shotId, tx); + for (const v of shown) { + await tx.approval.create({ + data: { + versionId: v.id, + userId: user.id, + status: "NEEDS_CHANGES", + notes: notes ?? "Changes requested on the shot", + }, + }); + await tx.version.update({ + where: { id: v.id }, + data: { approvalStatus: "NEEDS_CHANGES" }, + }); + } }); } else { return NextResponse.json({ error: "Invalid action" }, { status: 400 }); @@ -126,9 +154,12 @@ export async function POST( select: { sharedWithClient: true, shotApprovalStatus: true }, }); if (shot?.sharedWithClient && shot.shotApprovalStatus !== "CLIENT_APPROVED") { + // status is left to the recalc below — writing COMPLETE here was + // overwritten by it anyway, and disagreed with it whenever another + // task on the shot was still on CHANGES. await db.shot.update({ where: { id: version.task.shot.id }, - data: { shotApprovalStatus: "CLIENT_APPROVED", status: "COMPLETE" }, + data: { shotApprovalStatus: "CLIENT_APPROVED" }, }); } } diff --git a/components/tasks/KanbanCard.tsx b/components/tasks/KanbanCard.tsx index bdeb5ce..53bf6ad 100644 --- a/components/tasks/KanbanCard.tsx +++ b/components/tasks/KanbanCard.tsx @@ -143,7 +143,8 @@ export function KanbanCard({ task, isDragOverlay = false }: KanbanCardProps) { {latestVersion.approvalStatus === "APPROVED" ? ( - ) : latestVersion.approvalStatus === "CHANGES_REQUESTED" ? ( + ) : latestVersion.approvalStatus === "NEEDS_CHANGES" || + latestVersion.approvalStatus === "REJECTED" ? ( ) : ( diff --git a/lib/render-pipeline/preview.test.ts b/lib/render-pipeline/preview.test.ts index f1ed659..4e95357 100644 --- a/lib/render-pipeline/preview.test.ts +++ b/lib/render-pipeline/preview.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { nextTaskTitle } from "./preview"; +import { nextTaskTitle, carryApproval } from "./preview"; describe("nextTaskTitle", () => { it("swaps an existing version suffix", () => { @@ -30,3 +30,28 @@ describe("nextTaskTitle", () => { expect(nextTaskTitle("SHOT_cmp_TT_V12", "v013")).toBe("SHOT_cmp_TT_v013"); }); }); + +describe("carryApproval", () => { + it("carries an approval forward, so a re-export does not reopen a signed-off shot", () => { + expect(carryApproval("APPROVED")).toBe("APPROVED"); + }); + + // The regression: a render queued to address a note used to arrive already + // wearing that note, so nothing registered that the work had been redone. + it("resets NEEDS_CHANGES — this render is the fix for it", () => { + expect(carryApproval("NEEDS_CHANGES")).toBe("PENDING_REVIEW"); + }); + + it("resets REJECTED for the same reason", () => { + expect(carryApproval("REJECTED")).toBe("PENDING_REVIEW"); + }); + + it("leaves an unreviewed version pending", () => { + expect(carryApproval("PENDING_REVIEW")).toBe("PENDING_REVIEW"); + }); + + it("defaults to pending when there is no previous version", () => { + expect(carryApproval(null)).toBe("PENDING_REVIEW"); + expect(carryApproval(undefined)).toBe("PENDING_REVIEW"); + }); +}); diff --git a/lib/render-pipeline/preview.ts b/lib/render-pipeline/preview.ts index d8a64ae..3ac9c37 100644 --- a/lib/render-pipeline/preview.ts +++ b/lib/render-pipeline/preview.ts @@ -1,10 +1,11 @@ -import { ExportStatus, Prisma } from "@prisma/client"; +import { ApprovalStatus, ExportStatus, Prisma } from "@prisma/client"; import { db } from "@/lib/db"; import { PipelineError } from "./errors"; import { applyTransition } from "./transitions"; import { getConfigNumber, getPreviewConfig } from "./config"; import { resolveProjectTemplates, autoShotThumbnailEnabled } from "./project-templates"; import { clientShotCode } from "@/lib/shot-code"; +import { recalcShotStatus, clearStaleShotApproval } from "@/lib/shot-status"; import { renderOutputBase } from "./exports"; /** Slate date format used by the panel's getDateString (YYYY/MM/DD). */ @@ -261,6 +262,22 @@ export async function createPreviewJob( }); } +/** + * What approval a pipeline-created Version inherits from the one it supersedes. + * + * Only APPROVED carries. A re-export re-renders EXRs for delivery from a shot + * that has usually already been signed off, and defaulting to PENDING_REVIEW + * would make an approved shot silently reappear as awaiting review. + * + * The rejecting verdicts are the opposite case: NEEDS_CHANGES and REJECTED + * describe the version someone asked to be fixed, and the render coming through + * here is the fix. Carrying them forward left every re-render wearing the note + * that prompted it, so nothing in the app registered that the work was redone. + */ +export function carryApproval(previous: ApprovalStatus | null | undefined): ApprovalStatus { + return previous === "APPROVED" ? "APPROVED" : "PENDING_REVIEW"; +} + /** * `UNG_102_019_010_cmp_TT_v004` + v005 → `UNG_102_019_010_cmp_TT_v005`. * Mirrors `nextExrOutputBase` (exports.ts) so the task title, the EXR base and @@ -330,6 +347,7 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi let versionId: string | null = null; let versionReplaced = false; + let versionCreated = false; let inheritedApproval: string | null = null; if (artifacts.previewMovKey) { const mediaFields = { @@ -362,12 +380,6 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi // Version numbering stays monotonic for the task (existing review flows // rely on the newest version having the highest number) while matching // the export version whenever it is ahead. - // A re-export does not change the picture — it re-renders EXRs for - // delivery from a shot that has usually already been approved. A fresh - // Version defaults to PENDING_REVIEW, which is what the dashboard, the - // shot page and the client portal all read, so an approved shot would - // silently reappear as awaiting review. The approval carries over from - // the version this one supersedes. const previousLatest = exportRow.taskId ? await tx.version.findFirst({ where: { taskId: exportRow.taskId }, @@ -375,6 +387,7 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi select: { approvalStatus: true, versionNumber: true }, }) : null; + const carriedApproval = carryApproval(previousLatest?.approvalStatus); const taskMax = exportRow.taskId ? await tx.version.aggregate({ @@ -395,13 +408,14 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi // client portal, so the pipeline stays out of it. What this version // is and where it came from lives on the Export and its timeline, // which is where someone would go looking anyway. - approvalStatus: previousLatest?.approvalStatus ?? "PENDING_REVIEW", + approvalStatus: carriedApproval, isLatest: true, isClientVisible: false, // never shared to the client portal (§10.0) }, }); versionId = version.id; - inheritedApproval = previousLatest?.approvalStatus ?? null; + versionCreated = true; + inheritedApproval = carriedApproval; } if (exportRow.taskId) { @@ -473,6 +487,40 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi } } + // A genuinely new version is new work awaiting review, so the task moves the + // same way a manual upload moves it (actions/versions.ts, batch-upload) — + // otherwise a task left on CHANGES by a review or a QC fail stays there + // through every re-render, and the shot stays on REVISIONS with no way out + // but a manual kanban drag. + // + // A *replaced* version is deliberately excluded: regeneratePreview + // re-delivers the same version after a slate fix, and that is not a new + // submission. Nor is an already-approved carry-over, which would drag a + // signed-off shot back into review. + let taskStatusMoved = false; + let approvalCleared = false; + if (versionCreated && exportRow.taskId && inheritedApproval !== "APPROVED") { + const task = await tx.task.findUnique({ + where: { id: exportRow.taskId }, + select: { status: true }, + }); + if (task && task.status !== "INTERNAL_REVIEW") { + await tx.task.update({ + where: { id: exportRow.taskId }, + data: { status: "INTERNAL_REVIEW" }, + }); + taskStatusMoved = true; + } + approvalCleared = await clearStaleShotApproval(tx, exportRow.shotId); + } + + // Shot.status is derived from task states (lib/shot-status.ts); nothing in + // the pipeline recalculated it, so shot boards went stale the moment a + // render changed a task. + if (taskStatusMoved || approvalCleared) { + await recalcShotStatus(exportRow.shotId, tx); + } + const updated = await applyTransition( tx, exportRow, @@ -488,6 +536,7 @@ export async function finalizePreview(jobId: string, machineId: string, body: Fi : ` — review version created (${inheritedApproval ?? "PENDING_REVIEW"})` : " — no preview media reported") + (taskRenamed ? ` — task renamed to ${taskTitle}` : "") + + (taskStatusMoved ? " — task moved to INTERNAL_REVIEW" : "") + thumbnailNote, }, { diff --git a/lib/render-pipeline/qc.ts b/lib/render-pipeline/qc.ts index d2cc348..5c4ab56 100644 --- a/lib/render-pipeline/qc.ts +++ b/lib/render-pipeline/qc.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { db } from "@/lib/db"; import { PipelineError } from "./errors"; import { applyTransition } from "./transitions"; +import { recalcShotStatus } from "@/lib/shot-status"; /** * QC workflow (RenderPipeline2 §10) — technical/final-delivery QC on shots the @@ -104,6 +105,9 @@ export async function submitQCReview(exportId: string, input: QCSubmitInput, act if (task && task.status !== "CHANGES") { await tx.task.update({ where: { id: task.id }, data: { status: "CHANGES" } }); taskUpdated = { id: task.id, status: "CHANGES" }; + // Shot.status is derived from its tasks and does not recompute on its + // own, so without this the shot never actually reached REVISIONS. + await recalcShotStatus(exportRow.shotId, tx); } } diff --git a/lib/shot-status.test.ts b/lib/shot-status.test.ts index 1cdfe14..7ecf255 100644 --- a/lib/shot-status.test.ts +++ b/lib/shot-status.test.ts @@ -3,8 +3,13 @@ import { describe, it, expect, vi } from "vitest"; // Mock Prisma db so the pure deriveShotStatus function can be tested in isolation vi.mock("@/lib/db", () => ({ db: {} })); -import { deriveShotStatus } from "./shot-status"; -import type { TaskStatus } from "@prisma/client"; +import { + deriveShotStatus, + clearStaleShotApproval, + markShotForRevisions, + recalcShotStatus, +} from "./shot-status"; +import type { TaskStatus, ShotApprovalStatus } from "@prisma/client"; // Helper to build task arrays quickly const tasks = (...statuses: TaskStatus[]) => statuses.map((status) => ({ status })); @@ -142,3 +147,186 @@ describe("deriveShotStatus", () => { }); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// Fake Prisma client — enough of the surface for the two write helpers. Each +// call mutates the in-memory shot so the assertions read final state, not spies. +// ───────────────────────────────────────────────────────────────────────────── + +type FakeTask = { id: string; status: TaskStatus }; +type FakeVersion = { id: string; taskId: string; isLatest: boolean; createdAt: Date }; + +// The helpers take a Prisma client; the fake implements just the calls they make. +type TxArg = Parameters[1]; +const asTx = (c: { shotState: unknown }): NonNullable => + c as unknown as NonNullable; + +function fakeClient(shot: { + id: string; + shotApprovalStatus: ShotApprovalStatus; + sharedWithClient: boolean; + status?: string; + tasks: FakeTask[]; + versions?: FakeVersion[]; +}) { + const versions = shot.versions ?? []; + return { + shot: { + findUnique: async () => shot, + update: async ({ data }: { data: Record }) => { + Object.assign(shot, data); + return shot; + }, + }, + task: { + findMany: async ({ where }: { where: { status?: { not: TaskStatus } } }) => + where.status?.not + ? shot.tasks.filter((t) => t.status !== where.status!.not) + : shot.tasks, + update: async ({ where, data }: { where: { id: string }; data: { status: TaskStatus } }) => { + const t = shot.tasks.find((x) => x.id === where.id); + if (t) t.status = data.status; + return t; + }, + updateMany: async ({ + where, + data, + }: { + where: { status?: { not: TaskStatus } }; + data: { status: TaskStatus }; + }) => { + for (const t of shot.tasks) { + if (!where.status?.not || t.status !== where.status.not) t.status = data.status; + } + return { count: shot.tasks.length }; + }, + }, + version: { + findFirst: async () => + [...versions].sort( + (a, b) => + Number(b.isLatest) - Number(a.isLatest) || + b.createdAt.getTime() - a.createdAt.getTime() + )[0] ?? null, + }, + shotState: shot, + }; +} + +describe("clearStaleShotApproval", () => { + it("clears an internal sign-off so new work is visible", async () => { + const c = fakeClient({ + id: "s1", + shotApprovalStatus: "INTERNALLY_APPROVED", + sharedWithClient: false, + tasks: [{ id: "t1", status: "INTERNAL_REVIEW" }], + }); + expect(await clearStaleShotApproval(asTx(c), "s1")).toBe(true); + expect(c.shotState.shotApprovalStatus).toBe("PENDING"); + }); + + it("clears a client sign-off, so an upload cannot hide under COMPLETE", async () => { + const c = fakeClient({ + id: "s1", + shotApprovalStatus: "CLIENT_APPROVED", + sharedWithClient: true, + tasks: [{ id: "t1", status: "INTERNAL_REVIEW" }], + }); + expect(await clearStaleShotApproval(asTx(c), "s1")).toBe(true); + expect(c.shotState.shotApprovalStatus).toBe("PENDING"); + }); + + it("is a no-op when there is no sign-off to clear", async () => { + const c = fakeClient({ + id: "s1", + shotApprovalStatus: "PENDING", + sharedWithClient: false, + tasks: [{ id: "t1", status: "IN_PROGRESS" }], + }); + expect(await clearStaleShotApproval(asTx(c), "s1")).toBe(false); + }); + + it("leaves sharedWithClient alone — revoking access is a separate act", async () => { + const c = fakeClient({ + id: "s1", + shotApprovalStatus: "CLIENT_APPROVED", + sharedWithClient: true, + tasks: [{ id: "t1", status: "DONE" }], + }); + await clearStaleShotApproval(asTx(c), "s1"); + expect(c.shotState.sharedWithClient).toBe(true); + }); +}); + +describe("markShotForRevisions", () => { + it("marks the open tasks and lands the shot on REVISIONS", async () => { + const c = fakeClient({ + id: "s1", + shotApprovalStatus: "CLIENT_APPROVED", + sharedWithClient: true, + tasks: [ + { id: "t1", status: "DONE" }, + { id: "t2", status: "IN_PROGRESS" }, + ], + }); + const changed = await markShotForRevisions(asTx(c), "s1"); + expect(changed).toEqual(["t2"]); + expect(c.shotState.tasks.find((t) => t.id === "t1")!.status).toBe("DONE"); + expect(c.shotState.tasks.find((t) => t.id === "t2")!.status).toBe("CHANGES"); + expect(c.shotState.shotApprovalStatus).toBe("PENDING"); + expect(c.shotState.sharedWithClient).toBe(false); + expect(c.shotState.status).toBe("REVISIONS"); + }); + + // The regression this helper exists for: a shot out for client review has all + // its tasks DONE, so the old "non-DONE" filter matched nothing and the change + // request silently derived to INTERNAL_REVIEW instead of REVISIONS. + it("reopens the task behind the latest version when every task is DONE", async () => { + const c = fakeClient({ + id: "s1", + shotApprovalStatus: "CLIENT_APPROVED", + sharedWithClient: true, + tasks: [ + { id: "t1", status: "DONE" }, + { id: "t2", status: "DONE" }, + ], + versions: [ + { id: "v1", taskId: "t1", isLatest: false, createdAt: new Date("2026-01-01") }, + { id: "v2", taskId: "t2", isLatest: true, createdAt: new Date("2026-02-01") }, + ], + }); + const changed = await markShotForRevisions(asTx(c), "s1"); + expect(changed).toEqual(["t2"]); + expect(c.shotState.tasks.find((t) => t.id === "t2")!.status).toBe("CHANGES"); + expect(c.shotState.status).toBe("REVISIONS"); + }); + + it("marks only the called-out task when one is named", async () => { + const c = fakeClient({ + id: "s1", + shotApprovalStatus: "INTERNALLY_APPROVED", + sharedWithClient: false, + tasks: [ + { id: "t1", status: "TODO" }, + { id: "t2", status: "DONE" }, + ], + }); + const changed = await markShotForRevisions(asTx(c), "s1", "t2"); + expect(changed).toEqual(["t2"]); + expect(c.shotState.tasks.find((t) => t.id === "t1")!.status).toBe("TODO"); + expect(c.shotState.tasks.find((t) => t.id === "t2")!.status).toBe("CHANGES"); + }); + + it("still clears the approval when a shot has no tasks to mark", async () => { + const c = fakeClient({ + id: "s1", + shotApprovalStatus: "CLIENT_APPROVED", + sharedWithClient: true, + tasks: [], + }); + const changed = await markShotForRevisions(asTx(c), "s1"); + expect(changed).toEqual([]); + expect(c.shotState.shotApprovalStatus).toBe("PENDING"); + expect(c.shotState.status).toBe("WAITING"); + }); +}); diff --git a/lib/shot-status.ts b/lib/shot-status.ts index f5305b0..14991df 100644 --- a/lib/shot-status.ts +++ b/lib/shot-status.ts @@ -77,3 +77,100 @@ export async function recalcShotStatus( data: { status: newStatus }, }); } + +/** + * New review material arrived on a shot, so any standing sign-off is stale. + * + * deriveShotStatus short-circuits on shotApprovalStatus: a CLIENT_APPROVED shot + * reads COMPLETE and an INTERNALLY_APPROVED one reads READY_FOR_CLIENT no matter + * what its tasks say. Without this, uploading a new version onto an approved + * shot moved the task to INTERNAL_REVIEW and changed nothing visible — the shot + * still read as signed off while carrying work nobody had looked at. + * + * sharedWithClient is left alone; clearing the approval is enough to pull the + * shot back to INTERNAL_REVIEW, and revoking client access is a separate, + * deliberate act (unshareFromClient). + * + * @returns true if a stale approval was actually cleared. + */ +export async function clearStaleShotApproval( + client: TxClient, + shotId: string +): Promise { + const shot = await client.shot.findUnique({ + where: { id: shotId }, + select: { shotApprovalStatus: true }, + }); + if (!shot || shot.shotApprovalStatus === "PENDING") return false; + + await client.shot.update({ + where: { id: shotId }, + data: { shotApprovalStatus: "PENDING" }, + }); + return true; +} + +/** + * Put a shot into revisions: clear its approval, unshare it, and mark the tasks + * that need rework so the derivation lands on REVISIONS. + * + * The task selection is the subtle part. Marking every non-DONE task as CHANGES + * matches nothing at all on the shot this is most often called for — one that is + * out for client review precisely *because* all of its tasks are finished. That + * left the shot deriving to INTERNAL_REVIEW and the change request with no + * visible effect anywhere. So when nothing was picked up, the task behind the + * shot's latest version is marked instead: that is the deliverable the reviewer + * was actually looking at. + * + * @param taskId Mark only this task (a reviewer calling out one specific task). + * @returns ids of the tasks moved to CHANGES. + */ +export async function markShotForRevisions( + client: TxClient, + shotId: string, + taskId?: string +): Promise { + await client.shot.update({ + where: { id: shotId }, + data: { shotApprovalStatus: "PENDING", sharedWithClient: false }, + }); + + let changed: string[]; + + if (taskId) { + await client.task.update({ where: { id: taskId }, data: { status: "CHANGES" } }); + changed = [taskId]; + } else { + const open = await client.task.findMany({ + where: { shotId, status: { not: "DONE" } }, + select: { id: true }, + }); + + if (open.length > 0) { + await client.task.updateMany({ + where: { shotId, status: { not: "DONE" } }, + data: { status: "CHANGES" }, + }); + changed = open.map((t) => t.id); + } else { + // Every task is DONE — reopen the one that produced what was reviewed. + const latest = await client.version.findFirst({ + where: { task: { shotId } }, + orderBy: [{ isLatest: "desc" }, { createdAt: "desc" }], + select: { taskId: true }, + }); + if (latest?.taskId) { + await client.task.update({ + where: { id: latest.taskId }, + data: { status: "CHANGES" }, + }); + changed = [latest.taskId]; + } else { + changed = []; + } + } + } + + await recalcShotStatus(shotId, client); + return changed; +}