Files
vfxreview/lib/shot-status.test.ts
T
twotalesanimation 94792279c5 fix(status): reset review state on a new version, and derive shot status everywhere
Requesting changes on a shot and then re-rendering it through the render queue
left every layer of the app still showing the old verdict. Three independent
causes, all on the pipeline path.

finalizePreview inherited the superseded version's approvalStatus
unconditionally. The carry-over exists so a re-export of a signed-off shot does
not reappear as awaiting review, but it applied equally to NEEDS_CHANGES and
REJECTED, so the render queued to address a note arrived already wearing that
note and nothing registered that the work had been redone. carryApproval now
carries only APPROVED.

The pipeline never moved the task. One left on CHANGES by a review or a QC fail
stayed there through every re-render, with no way out but a manual kanban drag.
A genuinely new version now moves it to INTERNAL_REVIEW, as actions/versions.ts
and batch-upload already did. A replaced version is excluded: regeneratePreview
re-delivers the same version after a slate fix and is not a new submission.

The pipeline never called recalcShotStatus at all, so shot boards went stale the
moment a render changed a task. finalizePreview and the QC fail path both
recalculate now; the latter already documented the REVISIONS derivation it was
relying on and never triggering.

Shot.status is derived from tasks and approval flags, but six call sites wrote
it directly with the status they expected: internallyApproveShot,
shareWithClient, unshareFromClient, clientApproveShot, unapproveShot and the
client approve route. Rule 1 of the derivation is that any CHANGES task outranks
an approval, so a shot could be pushed past open rework and then snap back to
REVISIONS on the next unrelated recalc, which is close to unreportable as a bug.
They now set the approval inputs and let recalcShotStatus derive.
unshareFromClient additionally hardcoded READY_FOR_CLIENT even for a
client-approved shot. updateShotStatus keeps its direct write as the deliberate
manual override, now documented as transient for the same reason.

markShotForRevisions replaces the duplicated change-request logic. Both copies
marked all non-DONE tasks CHANGES, which matches nothing on the shot this is
most often called for: one out for client review precisely because all its tasks
are finished. The request then derived to INTERNAL_REVIEW and had no visible
effect anywhere. When nothing is picked up it now reopens the task behind the
shot's latest version, which is the deliverable the reviewer was looking at.

A client's shot-level change request also marks the versions and writes Approval
rows now. The dashboard counters, the version badges and the portal's own
filters all read Version.approvalStatus, so a shot-level verdict was invisible
everywhere those look.

clearStaleShotApproval covers the reverse case: uploading a new version onto an
approved shot moved the task to INTERNAL_REVIEW and changed nothing visible,
because the derivation short-circuits on shotApprovalStatus. The shot kept
reading COMPLETE while carrying work nobody had looked at. This is the one
user-visible behaviour change here, so watch the boards on a live show: an
approved shot now drops back to INTERNAL_REVIEW on a new upload. sharedWithClient
is deliberately left alone, since revoking client access stays a separate act.

submitApproval and unapproveVersion are transactional now. The verdict, the
version, the task and the derived shot status are one decision, and a failure
partway through left the three layers disagreeing with no way to tell which one
was right.

KanbanCard compared approvalStatus against "CHANGES_REQUESTED", which is not a
member of the enum, so the changes-requested icon never rendered at all.

Thirteen tests cover carryApproval, markShotForRevisions including the all-DONE
regression, and clearStaleShotApproval. What is not covered is the pipeline
transaction itself, which needs a database: queue a render against a shot in
REVISIONS and confirm the task and shot both move before trusting this on a
show.

Version.reviewStatus is still written nowhere while being published through
ext/shots/lookup and EXT_API_REFERENCE.md as a permanent "PENDING". Left alone
deliberately: removing a field from that contract could break the connector.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-31 18:17:04 +02:00

333 lines
14 KiB
TypeScript

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,
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 }));
describe("deriveShotStatus", () => {
// ── WAITING ───────────────────────────────────────────────────────────────
it("returns WAITING when there are no tasks", () => {
expect(deriveShotStatus([], "PENDING", false)).toBe("WAITING");
});
it("returns WAITING when no tasks and internally approved (edge case: no tasks)", () => {
// With INTERNALLY_APPROVED + no tasks the approval fields take precedence
expect(deriveShotStatus([], "INTERNALLY_APPROVED", false)).toBe("READY_FOR_CLIENT");
});
// ── IN_PROGRESS ───────────────────────────────────────────────────────────
it("returns IN_PROGRESS when a task is TODO", () => {
expect(deriveShotStatus(tasks("TODO"), "PENDING", false)).toBe("IN_PROGRESS");
});
it("returns IN_PROGRESS when a task is IN_PROGRESS", () => {
expect(deriveShotStatus(tasks("IN_PROGRESS"), "PENDING", false)).toBe("IN_PROGRESS");
});
it("returns IN_PROGRESS when tasks are mixed TODO and DONE", () => {
expect(deriveShotStatus(tasks("TODO", "DONE"), "PENDING", false)).toBe("IN_PROGRESS");
});
it("returns IN_PROGRESS for INTERNAL_REVIEW task status (task in internal review)", () => {
// Task is in INTERNAL_REVIEW — not TODO/IN_PROGRESS/CHANGES → falls through to INTERNAL_REVIEW shot status
expect(deriveShotStatus(tasks("INTERNAL_REVIEW"), "PENDING", false)).toBe("INTERNAL_REVIEW");
});
// ── REVISIONS (highest priority) ─────────────────────────────────────────
it("returns REVISIONS when any task is CHANGES", () => {
expect(deriveShotStatus(tasks("CHANGES"), "PENDING", false)).toBe("REVISIONS");
});
it("returns REVISIONS even when some tasks are DONE", () => {
expect(deriveShotStatus(tasks("DONE", "CHANGES"), "PENDING", false)).toBe("REVISIONS");
});
it("returns REVISIONS even when shot is INTERNALLY_APPROVED", () => {
// If somehow a task is in CHANGES after approval, REVISIONS wins
expect(deriveShotStatus(tasks("CHANGES"), "INTERNALLY_APPROVED", false)).toBe("REVISIONS");
});
it("returns REVISIONS even when shot is CLIENT_APPROVED", () => {
expect(deriveShotStatus(tasks("CHANGES"), "CLIENT_APPROVED", false)).toBe("REVISIONS");
});
// ── INTERNAL_REVIEW ───────────────────────────────────────────────────────
it("returns INTERNAL_REVIEW when all tasks are DONE and approval is PENDING", () => {
expect(deriveShotStatus(tasks("DONE", "DONE"), "PENDING", false)).toBe("INTERNAL_REVIEW");
});
it("returns INTERNAL_REVIEW when tasks are INTERNAL_REVIEW/CLIENT_REVIEW and approval is PENDING", () => {
expect(deriveShotStatus(tasks("INTERNAL_REVIEW", "CLIENT_REVIEW"), "PENDING", false)).toBe("INTERNAL_REVIEW");
});
it("returns INTERNAL_REVIEW when all tasks are DONE, regardless of sharedWithClient", () => {
// sharedWithClient has no effect when approval is still PENDING
expect(deriveShotStatus(tasks("DONE"), "PENDING", true)).toBe("INTERNAL_REVIEW");
});
// ── READY_FOR_CLIENT ──────────────────────────────────────────────────────
it("returns READY_FOR_CLIENT when internally approved and not shared", () => {
expect(deriveShotStatus(tasks("DONE"), "INTERNALLY_APPROVED", false)).toBe("READY_FOR_CLIENT");
});
it("returns READY_FOR_CLIENT when internally approved and not shared (no tasks)", () => {
expect(deriveShotStatus([], "INTERNALLY_APPROVED", false)).toBe("READY_FOR_CLIENT");
});
// ── CLIENT_REVIEW ─────────────────────────────────────────────────────────
it("returns CLIENT_REVIEW when internally approved and shared", () => {
expect(deriveShotStatus(tasks("DONE"), "INTERNALLY_APPROVED", true)).toBe("CLIENT_REVIEW");
});
it("returns CLIENT_REVIEW when internally approved, shared, and tasks still in progress", () => {
// Approval field takes precedence over task statuses (except CHANGES)
expect(deriveShotStatus(tasks("TODO"), "INTERNALLY_APPROVED", true)).toBe("CLIENT_REVIEW");
});
// ── COMPLETE ──────────────────────────────────────────────────────────────
it("returns COMPLETE when client approved", () => {
expect(deriveShotStatus(tasks("DONE"), "CLIENT_APPROVED", true)).toBe("COMPLETE");
});
it("returns COMPLETE when client approved even if not shared anymore", () => {
expect(deriveShotStatus(tasks("DONE"), "CLIENT_APPROVED", false)).toBe("COMPLETE");
});
// ── State transition scenarios ────────────────────────────────────────────
describe("full workflow transitions", () => {
it("Artist work → tasks TODO → IN_PROGRESS", () => {
expect(deriveShotStatus(tasks("TODO"), "PENDING", false)).toBe("IN_PROGRESS");
});
it("Tasks in internal review → INTERNAL_REVIEW shot status", () => {
expect(deriveShotStatus(tasks("INTERNAL_REVIEW"), "PENDING", false)).toBe("INTERNAL_REVIEW");
});
it("All tasks done → INTERNAL_REVIEW (awaiting supervisor)", () => {
expect(deriveShotStatus(tasks("DONE", "DONE", "DONE"), "PENDING", false)).toBe("INTERNAL_REVIEW");
});
it("Supervisor approves → READY_FOR_CLIENT", () => {
expect(deriveShotStatus(tasks("DONE"), "INTERNALLY_APPROVED", false)).toBe("READY_FOR_CLIENT");
});
it("Producer shares → CLIENT_REVIEW", () => {
expect(deriveShotStatus(tasks("DONE"), "INTERNALLY_APPROVED", true)).toBe("CLIENT_REVIEW");
});
it("Producer unshares → READY_FOR_CLIENT", () => {
expect(deriveShotStatus(tasks("DONE"), "INTERNALLY_APPROVED", false)).toBe("READY_FOR_CLIENT");
});
it("Client approves → COMPLETE", () => {
expect(deriveShotStatus(tasks("DONE"), "CLIENT_APPROVED", false)).toBe("COMPLETE");
});
it("Client requests changes → approval reset to PENDING + task CHANGES → REVISIONS", () => {
// When changes are requested: shotApprovalStatus → PENDING, sharedWithClient → false, task → CHANGES
expect(deriveShotStatus(tasks("CHANGES"), "PENDING", false)).toBe("REVISIONS");
});
});
});
// ─────────────────────────────────────────────────────────────────────────────
// 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<typeof recalcShotStatus>[1];
const asTx = (c: { shotState: unknown }): NonNullable<TxArg> =>
c as unknown as NonNullable<TxArg>;
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<string, unknown> }) => {
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");
});
});