diff --git a/actions/comments.ts b/actions/comments.ts index ceee6ac..c0eeb31 100644 --- a/actions/comments.ts +++ b/actions/comments.ts @@ -4,7 +4,7 @@ import { auth } from "@/auth"; import { db } from "@/lib/db"; import { revalidatePath } from "next/cache"; import { z } from "zod"; -import { notifyFeedbackAdded, notifyCommentReply } from "@/lib/notifications"; +import { notifyCommentsAdded, notifyCommentReply } from "@/lib/notifications"; import { slackNotifyNewFeedback } from "@/lib/slack"; import { createDisplayEvent } from "@/lib/display-events"; @@ -36,7 +36,6 @@ export async function addComment(data: z.infer) { }, }); - // Notify the shot artist if commenter is not the artist const version = await db.version.findUnique({ where: { id: parsed.versionId }, include: { @@ -56,16 +55,19 @@ export async function addComment(data: z.infer) { version?.task?.title ?? "Task"; - if (version?.shot?.artistId && version.shot.artistId !== session.user.id) { - const user = await db.user.findUnique({ where: { id: session.user.id } }); - await notifyFeedbackAdded({ - artistId: version.shot.artistId, - shotCode: version.shot.shotCode, - frameNumber: parsed.frameNumber, + // Staff and the shot's artist — never the commenter. A failed notification + // must not fail a comment that is already saved. + const user = await db.user.findUnique({ where: { id: session.user.id }, select: { name: true } }); + await notifyCommentsAdded({ + notes: [{ versionId: parsed.versionId, - authorName: user?.name ?? session.user.email ?? "Someone", - }); - } + shotCode, + frameNumber: parsed.frameNumber, + artistIds: [version?.shot?.artistId, version?.task?.assignedArtistId], + }], + authorId: session.user.id, + authorName: user?.name ?? session.user.email ?? "Someone", + }).catch((err) => console.error("[comments] notify failed", err)); // Slack notification if (slackWebhook) { diff --git a/actions/feedback.ts b/actions/feedback.ts index 24ee4f5..7bcb0d5 100644 --- a/actions/feedback.ts +++ b/actions/feedback.ts @@ -7,6 +7,7 @@ import { z } from "zod"; import { findShot, suggestShot, type BoardShot } from "@/lib/board-parse"; import { parseFeedback, slateFrames, toPlayerFrame } from "@/lib/feedback-parse"; import { updateTaskStatus } from "@/actions/tasks"; +import { notifyCommentsAdded, type NewCommentNote } from "@/lib/notifications"; /** Comment.source for notes pasted from a client message */ const SOURCE = "WHATSAPP"; @@ -176,7 +177,13 @@ export async function commitFeedback(data: z.infer) { const versionIds = [...new Set(notes.map((n) => n.versionId).filter(Boolean) as string[])]; const versions = await db.version.findMany({ where: { id: { in: versionIds } }, - select: { id: true, fps: true, shotId: true, task: { select: { id: true, shotId: true, status: true, projectId: true } }, shot: { select: { projectId: true } } }, + select: { + id: true, + fps: true, + shotId: true, + task: { select: { id: true, shotId: true, status: true, projectId: true, assignedArtistId: true, shot: { select: { shotCode: true } } } }, + shot: { select: { projectId: true, shotCode: true, artistId: true } }, + }, }); const vById = new Map(versions.map((v) => [v.id, v])); for (const n of notes) { @@ -195,6 +202,7 @@ export async function commitFeedback(data: z.infer) { let sort = (last?.sortOrder ?? -1) + 1; let comments = 0; + const saved: NewCommentNote[] = []; await db.$transaction(async (tx) => { for (const r of parsed.rounds) { const round = await tx.feedbackRound.create({ @@ -221,7 +229,15 @@ export async function commitFeedback(data: z.infer) { select: { id: true }, }) : null; - if (comment) comments++; + if (comment && v) { + comments++; + saved.push({ + versionId: v.id, + shotCode: v.shot?.shotCode ?? v.task?.shot?.shotCode ?? n.code.toUpperCase(), + frameNumber: frame, + artistIds: [v.shot?.artistId, v.task?.assignedArtistId], + }); + } await tx.boardItem.create({ data: { projectId: parsed.projectId, @@ -247,6 +263,14 @@ export async function commitFeedback(data: z.infer) { } for (const taskId of tasks) await updateTaskStatus(taskId, "CHANGES"); + // Only after the transaction commits — a rolled-back paste notifies no one. + await notifyCommentsAdded({ + notes: saved, + authorId: user.id, + authorName: user.name ?? user.email ?? "Someone", + origin: "whatsapp", + }).catch((err) => console.error("[feedback] notify failed", err)); + for (const id of versionIds) revalidatePath(`/review/${id}`); revalidatePath("/board"); return { success: true, notes: notes.length, comments, tasksChanged: tasks.size }; diff --git a/app/api/client/[token]/comment/route.ts b/app/api/client/[token]/comment/route.ts index b007d9b..c9de757 100644 --- a/app/api/client/[token]/comment/route.ts +++ b/app/api/client/[token]/comment/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { db } from "@/lib/db"; import { slackNotifyNewFeedback } from "@/lib/slack"; +import { notifyCommentsAdded } from "@/lib/notifications"; import { validateReviewToken, getOrCreateReviewUser } from "@/lib/review-auth"; export async function POST( @@ -28,8 +29,8 @@ export async function POST( const version = await db.version.findUnique({ where: { id: versionId }, include: { - shot: { select: { projectId: true, shotCode: true, project: { select: { slackWebhook: true } } } }, - task: { select: { projectId: true, title: true, project: { select: { slackWebhook: true } }, shot: { select: { shotCode: true } } } }, + shot: { select: { projectId: true, shotCode: true, artistId: true, project: { select: { slackWebhook: true } } } }, + task: { select: { projectId: true, title: true, assignedArtistId: true, project: { select: { slackWebhook: true } }, shot: { select: { shotCode: true } } } }, }, }); const projectId = version?.shot?.projectId ?? version?.task?.projectId; @@ -70,6 +71,18 @@ export async function POST( }); } + await notifyCommentsAdded({ + notes: [{ + versionId, + shotCode, + frameNumber, + artistIds: [version.shot?.artistId, version.task?.assignedArtistId], + }], + authorId: user.id, + authorName: user.name ?? user.email, + origin: "client", + }).catch((err) => console.error("[client comment] notify failed", err)); + // canEdit mirrors the GET shape — a note you just wrote is always your own. return NextResponse.json({ comment: { ...comment, canEdit: true } }, { status: 201 }); } diff --git a/lib/notifications.test.ts b/lib/notifications.test.ts new file mode 100644 index 0000000..0b66f05 --- /dev/null +++ b/lib/notifications.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ db: {} })); + +import { buildCommentNotifications } from "./notifications"; + +const note = (versionId: string, shotCode: string, frameNumber: number, artistIds: (string | null)[] = []) => ({ + versionId, + shotCode, + frameNumber, + artistIds, +}); + +describe("buildCommentNotifications", () => { + it("tells every staff member and the artist, never the author", () => { + const out = buildCommentNotifications({ + notes: [note("v1", "DH2_010", 1012, ["artist"])], + authorId: "sup", + authorName: "Sam", + staffIds: ["sup", "prod", "admin"], + }); + expect(out.map((p) => p.userId).sort()).toEqual(["admin", "artist", "prod"]); + expect(out[0]).toMatchObject({ + type: "FEEDBACK_ADDED", + title: "New feedback", + message: "Sam commented on DH2_010 at frame 1012", + data: { versionId: "v1", shotCode: "DH2_010", frameNumber: 1012 }, + }); + }); + + it("does not notify an artist twice when they are also staff or on both shot and task", () => { + const out = buildCommentNotifications({ + notes: [note("v1", "A", 1, ["prod", "prod", null])], + authorId: "x", + authorName: "X", + staffIds: ["prod"], + }); + expect(out.map((p) => p.userId)).toEqual(["prod"]); + }); + + it("labels client comments", () => { + const [p] = buildCommentNotifications({ + notes: [note("v1", "A", 5)], + authorId: "client", + authorName: "Netflix", + origin: "client", + staffIds: ["prod"], + }); + expect(p.title).toBe("New client feedback"); + expect(p.message).toBe("Client Netflix commented on A at frame 5"); + }); + + it("groups a pasted round into one notification per version", () => { + const out = buildCommentNotifications({ + notes: [note("v1", "A", 1, ["art1"]), note("v1", "A", 9), note("v2", "B", 3, ["art2"])], + authorId: "prod", + authorName: "Pat", + origin: "whatsapp", + staffIds: ["prod", "sup"], + }); + const msgs = out.map((p) => `${p.userId}: ${p.message}`).sort(); + expect(msgs).toEqual([ + "art1: Client feedback (logged by Pat) left 2 notes on A", + "art2: Client feedback (logged by Pat) commented on B at frame 3", + "sup: Client feedback (logged by Pat) commented on B at frame 3", + "sup: Client feedback (logged by Pat) left 2 notes on A", + ]); + }); + + it("sends nothing for no notes", () => { + expect( + buildCommentNotifications({ notes: [], authorId: "a", authorName: "A", staffIds: ["b"] }) + ).toEqual([]); + }); +}); diff --git a/lib/notifications.ts b/lib/notifications.ts index dc58504..0eadbec 100644 --- a/lib/notifications.ts +++ b/lib/notifications.ts @@ -78,29 +78,6 @@ export async function notifyNewVersionUploaded(params: { ); } -/** - * Notify the shot's artist about new feedback. - */ -export async function notifyFeedbackAdded(params: { - artistId: string; - shotCode: string; - frameNumber: number; - versionId: string; - authorName: string; -}) { - await createNotification({ - userId: params.artistId, - type: NotificationType.FEEDBACK_ADDED, - title: "New feedback on your shot", - message: `${params.authorName} commented on ${params.shotCode} at frame ${params.frameNumber}`, - data: { - versionId: params.versionId, - shotCode: params.shotCode, - frameNumber: params.frameNumber, - }, - }); -} - /** * Notify relevant users of an approval status change. */ @@ -250,3 +227,97 @@ export async function notifyCommentReply(params: { }, }); } + +/** One new comment, as the recipients of its notification need it. */ +export interface NewCommentNote { + versionId: string; + shotCode: string; + frameNumber: number; + /** The shot's artist and, for a task version, the task's artist. */ + artistIds: (string | null | undefined)[]; +} + +/** + * Who hears about new comments, and what they read. Pure so it can be tested: + * every active staff member plus each note's artist, never the author. + * Notes are grouped per version so a pasted round of client feedback is one + * notification per shot rather than one per line. + */ +export function buildCommentNotifications(params: { + notes: NewCommentNote[]; + authorId: string; + authorName: string; + /** "client" = typed on a client review link; "whatsapp" = pasted by staff. */ + origin?: "client" | "whatsapp"; + staffIds: string[]; +}): CreateNotificationPayload[] { + const byVersion = new Map(); + for (const n of params.notes) { + byVersion.set(n.versionId, [...(byVersion.get(n.versionId) ?? []), n]); + } + + const who = + params.origin === "client" + ? `Client ${params.authorName}` + : params.origin === "whatsapp" + ? `Client feedback (logged by ${params.authorName})` + : params.authorName; + const title = params.origin ? "New client feedback" : "New feedback"; + + const payloads: CreateNotificationPayload[] = []; + for (const [versionId, notes] of byVersion) { + const { shotCode, frameNumber } = notes[0]; + const message = + notes.length === 1 + ? `${who} commented on ${shotCode} at frame ${frameNumber}` + : `${who} left ${notes.length} notes on ${shotCode}`; + const recipients = new Set([ + ...params.staffIds, + ...notes.flatMap((n) => n.artistIds).filter((id): id is string => !!id), + ]); + recipients.delete(params.authorId); + for (const userId of recipients) { + payloads.push({ + userId, + type: NotificationType.FEEDBACK_ADDED, + title, + message, + data: { versionId, shotCode, frameNumber }, + }); + } + } + return payloads; +} + +/** + * Notify staff and the shot's artist about new comments. Artists who are not + * staff are only told when they are still active. + */ +export async function notifyCommentsAdded(params: { + notes: NewCommentNote[]; + authorId: string; + authorName: string; + origin?: "client" | "whatsapp"; +}) { + if (params.notes.length === 0) return; + const artistIds = [ + ...new Set(params.notes.flatMap((n) => n.artistIds).filter((id): id is string => !!id)), + ]; + const [staff, activeArtists] = await Promise.all([ + db.user.findMany({ + where: { role: { in: ["ADMIN", "PRODUCER", "SUPERVISOR"] }, isActive: true }, + select: { id: true }, + }), + db.user.findMany({ + where: { id: { in: artistIds }, isActive: true, role: { not: "CLIENT" } }, + select: { id: true }, + }), + ]); + const active = new Set(activeArtists.map((u) => u.id)); + const payloads = buildCommentNotifications({ + ...params, + notes: params.notes.map((n) => ({ ...n, artistIds: n.artistIds.filter((id) => id && active.has(id)) })), + staffIds: staff.map((u) => u.id), + }); + if (payloads.length) await createNotifications(payloads); +}