From 50598472ad34a177af5fed4b140a90b1eb0fda28 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Wed, 2 Sep 2026 22:03:26 +0200 Subject: [PATCH] fix(polls): enforce shared persistence invariants --- src/actions/polls.test.ts | 179 +++++++- src/actions/polls.ts | 410 ++++++++++++------ .../mutation-runtime-database.test.ts | 66 ++- .../services/mutation-runtime-database.ts | 28 +- src/lib/polls/poll-semantics.ts | 33 ++ src/lib/validators/poll.test.ts | 55 +++ src/lib/validators/poll.ts | 53 ++- 7 files changed, 655 insertions(+), 169 deletions(-) diff --git a/src/actions/polls.test.ts b/src/actions/polls.test.ts index 87adb568..4e25bdc4 100644 --- a/src/actions/polls.test.ts +++ b/src/actions/polls.test.ts @@ -6,6 +6,9 @@ const database = vi.hoisted(() => { let mutationSteps: string[] = []; let validationVoteQuestionIds: number[] = []; let insertedVoteQuestionIds: number[] = []; + let readScopes: Array<{ table: string; transaction: boolean }> = []; + let transactionActive = false; + let insertFailure: unknown; const nextSelected = () => selectedQueue.shift() ?? []; const conditionPart = (condition: unknown, key: "column" | "value") => { @@ -20,7 +23,10 @@ const database = vi.hoisted(() => { from: vi.fn((table: { tableName?: string }) => ({ where: vi.fn((condition: unknown) => { const tableName = table.tableName ?? "unknown"; - const run = async () => nextSelected(); + const run = async () => { + readScopes.push({ table: tableName, transaction: transactionActive }); + return nextSelected(); + }; return { limit: vi.fn(async () => { if (tableName === "WebsitePollVote") { @@ -64,6 +70,7 @@ const database = vi.hoisted(() => { const update = vi.fn(() => ({ set: updateSet })); const insert = vi.fn((table: { tableName?: string }) => ({ values: vi.fn(async (values: Record) => { + if (insertFailure !== undefined) throw insertFailure; mutationSteps.push(`insert:${table.tableName ?? "unknown"}`); if (table.tableName === "WebsitePollVote") { insertedVoteQuestionIds.push(Number(values.questionId)); @@ -76,12 +83,19 @@ const database = vi.hoisted(() => { mutationSteps.push(`delete:${table.tableName ?? "unknown"}`); }), })); - const tx = { select, insert, delete: deleteFrom }; + const tx = { select, update, insert, delete: deleteFrom }; const transaction = vi.fn( async ( run: (transaction: typeof tx) => Promise, _config?: unknown, - ) => run(tx), + ) => { + transactionActive = true; + try { + return await run(tx); + } finally { + transactionActive = false; + } + }, ); return { @@ -95,17 +109,25 @@ const database = vi.hoisted(() => { mutationSteps: () => [...mutationSteps], validationVoteQuestionIds: () => [...validationVoteQuestionIds], insertedVoteQuestionIds: () => [...insertedVoteQuestionIds], + readScopes: () => [...readScopes], + failInsertWith(error: unknown) { + insertFailure = error; + }, reset() { selectedQueue = []; mutationSteps = []; validationVoteQuestionIds = []; insertedVoteQuestionIds = []; + readScopes = []; + transactionActive = false; + insertFailure = undefined; }, }; }); vi.mock("drizzle-orm", () => ({ and: vi.fn((...clauses: unknown[]) => ({ operator: "and", clauses })), + count: vi.fn(() => "count"), eq: vi.fn((column: unknown, value: unknown) => ({ operator: "eq", column, @@ -182,6 +204,7 @@ vi.mock("@/lib/safe-action-shared", () => ({ })); import { + addPollQuestion, deletePoll, updatePoll, updatePollQuestion, @@ -217,6 +240,52 @@ describe("poll update merge validation", () => { expect(database.updateSet).not.toHaveBeenCalled(); }); + it("rejects legacy activation above the public 50-question batch limit", async () => { + database.queueSelected( + [ + { + id: 1, + title: "Schedule", + description: null, + status: "draft", + showResults: 1, + multipleChoice: 0, + startsAt: null, + endsAt: null, + }, + ], + [{ value: 51 }], + ); + + const result = await updatePoll({ id: 1, status: "active" }); + + expect(result).toEqual({ + ok: false, + error: "Poll cannot be activated with more than 50 questions", + }); + expect(database.updateSet).not.toHaveBeenCalled(); + expect(database.transaction).toHaveBeenCalledWith(expect.any(Function), { + isolationLevel: "read committed", + }); + }); + + it("rejects legacy question 51 while the parent poll is active", async () => { + database.queueSelected([{ id: 1, status: "active" }], [{ value: 50 }]); + + const result = await addPollQuestion({ + pollId: 1, + question: "Question 51", + type: "single", + options: "Yes\nNo", + }); + + expect(result).toEqual({ + ok: false, + error: "Poll question limit reached", + }); + expect(database.insertedVoteQuestionIds()).toEqual([]); + }); + it("rejects a partial options patch for a persisted text question without updating", async () => { database.queueSelected([ { @@ -233,6 +302,54 @@ describe("poll update merge validation", () => { expect(result).toEqual({ ok: false, error: "Invalid poll question" }); expect(database.updateSet).not.toHaveBeenCalled(); }); + + it("rejects semantic edits to a legacy question after votes exist", async () => { + database.queueSelected( + [ + { + id: 1, + pollId: 1, + question: "Why?", + type: "text", + sortOrder: 0, + options: "", + }, + ], + [{ id: 91 }], + ); + + const result = await updatePollQuestion({ id: 1, question: "Why now?" }); + + expect(result).toEqual({ + ok: false, + error: "A voted question can only be reordered", + }); + expect(database.updateSet).not.toHaveBeenCalled(); + }); + + it("allows a sort-order-only legacy edit after votes exist", async () => { + database.queueSelected( + [ + { + id: 1, + pollId: 1, + question: "Why?", + type: "text", + sortOrder: 0, + options: "", + }, + ], + [{ id: 91 }], + ); + + const result = await updatePollQuestion({ id: 1, sortOrder: 2 }); + + expect(result).toEqual({ ok: true, data: { id: 1 } }); + expect(database.updateSet).toHaveBeenCalledWith({ sortOrder: 2 }); + expect(database.mutationSteps()).toContain( + "lock:WebsitePollQuestion:eq:id:1:update", + ); + }); }); describe("poll transaction lock order", () => { @@ -261,7 +378,7 @@ describe("poll transaction lock order", () => { ]); }); - it("validates votes in request order but inserts a sorted copy", async () => { + it("locks and validates current vote state in one transaction before sorted inserts", async () => { database.queueSelected( [ { @@ -271,10 +388,9 @@ describe("poll transaction lock order", () => { endsAt: null, }, ], - [ - { id: 9, pollId: 7, type: "single", options: "A\nB" }, - { id: 4, pollId: 7, type: "single", options: "A\nB" }, - ], + [{ id: 4 }, { id: 9 }], + [{ id: 4, pollId: 7, type: "single", options: "A\nB" }], + [{ id: 9, pollId: 7, type: "single", options: "A\nB" }], [], [], ); @@ -292,5 +408,52 @@ describe("poll transaction lock order", () => { expect(database.validationVoteQuestionIds()).toEqual([9, 4]); expect(database.insertedVoteQuestionIds()).toEqual([4, 9]); expect(input.votes.map(({ questionId }) => questionId)).toEqual([9, 4]); + expect(database.readScopes().every((read) => read.transaction)).toBe(true); + expect(database.mutationSteps().slice(0, 4)).toEqual([ + "lock:WebsitePoll:eq:id:7:update", + "read:WebsitePollQuestion:eq:pollId:orderBy:id", + "lock:WebsitePollQuestion:eq:id:4:update", + "lock:WebsitePollQuestion:eq:id:9:update", + ]); + }); + + it("rechecks a poll closure atomically before inserting votes", async () => { + database.queueSelected([ + { id: 7, status: "closed", startsAt: null, endsAt: null }, + ]); + + const result = await voteOnPoll({ + pollId: 7, + votes: [{ questionId: 4, answer: "A" }], + }); + + expect(result).toEqual({ + ok: false, + error: "This poll is not open for voting", + }); + expect(database.insertedVoteQuestionIds()).toEqual([]); + expect(database.readScopes()).toEqual([ + { table: "WebsitePoll", transaction: true }, + ]); + }); + + it("maps a duplicate-key insert race to the existing duplicate response", async () => { + database.queueSelected( + [{ id: 7, status: "active", startsAt: null, endsAt: null }], + [{ id: 4 }], + [{ id: 4, pollId: 7, type: "single", options: "A\nB" }], + [], + ); + database.failInsertWith({ code: "ER_DUP_ENTRY" }); + + const result = await voteOnPoll({ + pollId: 7, + votes: [{ questionId: 4, answer: "A" }], + }); + + expect(result).toEqual({ + ok: false, + error: "You have already voted on this poll", + }); }); }); diff --git a/src/actions/polls.ts b/src/actions/polls.ts index e8bd227c..2731e900 100644 --- a/src/actions/polls.ts +++ b/src/actions/polls.ts @@ -1,6 +1,6 @@ "use server"; -import { and, eq, inArray } from "drizzle-orm"; +import { and, count, eq, inArray } from "drizzle-orm"; import { revalidatePath } from "next/cache"; import { z } from "zod"; import { @@ -11,12 +11,21 @@ import { } from "@/lib/db"; import { PERMS } from "@/lib/permissions"; import { + hasPollQuestionSemanticChange, + POLL_PUBLIC_QUESTION_LIMIT, type PollQuestionType, parsePollAnswerSelections, parsePollOptions, + pollQuestionLimitReached, + serializePollOptions, } from "@/lib/polls/poll-semantics"; import { adminAction, authAction } from "@/lib/safe-action"; -import { ActionError, actionError, actionOk } from "@/lib/safe-action-shared"; +import { + ActionError, + type ActionResult, + actionError, + actionOk, +} from "@/lib/safe-action-shared"; import { logAudit } from "@/lib/services/audit"; import { createPollSchema, @@ -56,28 +65,45 @@ export const updatePoll = adminAction( { permission: PERMS.POLLS_EDIT, schema: updatePollInput }, async (ctx) => { const { id, ...data } = ctx.data; - const [existing] = await db - .select({ - id: WebsitePoll.id, - title: WebsitePoll.title, - description: WebsitePoll.description, - status: WebsitePoll.status, - showResults: WebsitePoll.showResults, - multipleChoice: WebsitePoll.multipleChoice, - startsAt: WebsitePoll.startsAt, - endsAt: WebsitePoll.endsAt, - }) - .from(WebsitePoll) - .where(eq(WebsitePoll.id, id)) - .limit(1); - if (!existing) throw new ActionError("Poll not found"); - const merged = createPollSchema.safeParse({ ...existing, ...data }); - if (!merged.success) throw new ActionError("Invalid poll schedule"); - - await db - .update(WebsitePoll) - .set({ ...data, updatedAt: new Date() }) - .where(eq(WebsitePoll.id, id)); + const existing = await db.transaction( + async (tx) => { + const [poll] = await tx + .select({ + id: WebsitePoll.id, + title: WebsitePoll.title, + description: WebsitePoll.description, + status: WebsitePoll.status, + showResults: WebsitePoll.showResults, + multipleChoice: WebsitePoll.multipleChoice, + startsAt: WebsitePoll.startsAt, + endsAt: WebsitePoll.endsAt, + }) + .from(WebsitePoll) + .where(eq(WebsitePoll.id, id)) + .for("update"); + if (!poll) throw new ActionError("Poll not found"); + const merged = createPollSchema.safeParse({ ...poll, ...data }); + if (!merged.success) throw new ActionError("Invalid poll schedule"); + if (merged.data.status === "active") { + const [questionCount] = await tx + .select({ value: count() }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.pollId, id)) + .limit(1); + if (Number(questionCount?.value ?? 0) > POLL_PUBLIC_QUESTION_LIMIT) { + throw new ActionError( + "Poll cannot be activated with more than 50 questions", + ); + } + } + await tx + .update(WebsitePoll) + .set({ ...data, updatedAt: new Date() }) + .where(eq(WebsitePoll.id, id)); + return poll; + }, + { isolationLevel: "read committed" }, + ); logAudit({ userId: ctx.session.user.id, action: "poll_update", @@ -150,7 +176,34 @@ export const deletePoll = adminAction( export const addPollQuestion = adminAction( { permission: PERMS.POLLS_EDIT, schema: pollQuestionSchema }, async (ctx) => { - const [result] = await db.insert(WebsitePollQuestion).values(ctx.data); + const [result] = await db.transaction( + async (tx) => { + const [parent] = await tx + .select({ id: WebsitePoll.id, status: WebsitePoll.status }) + .from(WebsitePoll) + .where(eq(WebsitePoll.id, ctx.data.pollId)) + .for("update"); + if (!parent) throw new ActionError("Poll not found"); + const [questionCount] = await tx + .select({ value: count() }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.pollId, parent.id)) + .limit(1); + if ( + pollQuestionLimitReached( + parent.status, + Number(questionCount?.value ?? 0), + ) + ) { + throw new ActionError("Poll question limit reached"); + } + return tx.insert(WebsitePollQuestion).values({ + ...ctx.data, + options: serializePollOptions(ctx.data.type, ctx.data.options), + }); + }, + { isolationLevel: "read committed" }, + ); return actionOk({ id: Number(result.insertId) }); }, ); @@ -163,25 +216,59 @@ export const updatePollQuestion = adminAction( { permission: PERMS.POLLS_EDIT, schema: updateQuestionInput }, async (ctx) => { const { id, ...data } = ctx.data; - const [existing] = await db - .select({ - pollId: WebsitePollQuestion.pollId, - question: WebsitePollQuestion.question, - type: WebsitePollQuestion.type, - sortOrder: WebsitePollQuestion.sortOrder, - options: WebsitePollQuestion.options, - }) - .from(WebsitePollQuestion) - .where(eq(WebsitePollQuestion.id, id)) - .limit(1); - if (!existing) throw new ActionError("Poll question not found"); - const merged = pollQuestionSchema.safeParse({ ...existing, ...data }); - if (!merged.success) throw new ActionError("Invalid poll question"); - - await db - .update(WebsitePollQuestion) - .set(data) - .where(eq(WebsitePollQuestion.id, id)); + await db.transaction( + async (tx) => { + const [existing] = await tx + .select({ + id: WebsitePollQuestion.id, + pollId: WebsitePollQuestion.pollId, + question: WebsitePollQuestion.question, + type: WebsitePollQuestion.type, + sortOrder: WebsitePollQuestion.sortOrder, + options: WebsitePollQuestion.options, + }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.id, id)) + .for("update"); + if (!existing) throw new ActionError("Poll question not found"); + const merged = pollQuestionSchema.safeParse({ ...existing, ...data }); + if (!merged.success) throw new ActionError("Invalid poll question"); + const values = { + ...data, + ...(Object.hasOwn(data, "options") + ? { + options: serializePollOptions( + merged.data.type, + merged.data.options, + ), + } + : {}), + }; + if ( + hasPollQuestionSemanticChange( + { + ...existing, + type: existing.type as PollQuestionType, + }, + merged.data, + ) + ) { + const [vote] = await tx + .select({ id: WebsitePollVote.id }) + .from(WebsitePollVote) + .where(eq(WebsitePollVote.questionId, id)) + .limit(1); + if (vote) { + throw new ActionError("A voted question can only be reordered"); + } + } + await tx + .update(WebsitePollQuestion) + .set(values) + .where(eq(WebsitePollQuestion.id, id)); + }, + { isolationLevel: "read committed" }, + ); return actionOk({ id }); }, ); @@ -193,15 +280,39 @@ const deleteQuestionInput = z.object({ export const deletePollQuestion = adminAction( { permission: PERMS.POLLS_EDIT, schema: deleteQuestionInput }, async (ctx) => { - await db - .delete(WebsitePollQuestion) - .where(eq(WebsitePollQuestion.id, ctx.data.id)); + await db.transaction( + async (tx) => { + const [question] = await tx + .select({ id: WebsitePollQuestion.id }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.id, ctx.data.id)) + .for("update"); + if (!question) throw new ActionError("Poll question not found"); + await tx + .delete(WebsitePollVote) + .where(eq(WebsitePollVote.questionId, question.id)); + await tx + .delete(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.id, question.id)); + }, + { isolationLevel: "read committed" }, + ); return actionOk(); }, ); // ── Public site: vote ─────────────────────────────────────────────── +function isDuplicateKey(error: unknown): boolean { + if (!error || typeof error !== "object") return false; + const candidate = error as { code?: string | number; errno?: number }; + return ( + candidate.code === "P2002" || + candidate.code === "ER_DUP_ENTRY" || + candidate.errno === 1062 + ); +} + export const voteOnPoll = authAction( { schema: voteOnPollSchema, @@ -215,107 +326,134 @@ export const voteOnPoll = authAction( return actionError("Unauthorized"); } - const [poll] = await db - .select({ - id: WebsitePoll.id, - status: WebsitePoll.status, - startsAt: WebsitePoll.startsAt, - endsAt: WebsitePoll.endsAt, - }) - .from(WebsitePoll) - .where(eq(WebsitePoll.id, ctx.data.pollId)) - .limit(1); + let outcome: ActionResult<{ pollId: number }>; + try { + outcome = await db.transaction( + async (tx) => { + const [poll] = await tx + .select({ + id: WebsitePoll.id, + status: WebsitePoll.status, + startsAt: WebsitePoll.startsAt, + endsAt: WebsitePoll.endsAt, + }) + .from(WebsitePoll) + .where(eq(WebsitePoll.id, ctx.data.pollId)) + .for("update"); + if (!poll) return actionError("Poll not found"); + if (poll.status !== "active") { + return actionError("This poll is not open for voting"); + } + const now = Date.now(); + if (poll.startsAt && poll.startsAt.getTime() > now) { + return actionError("This poll has not started yet"); + } + if (poll.endsAt && poll.endsAt.getTime() < now) { + return actionError("This poll has ended"); + } - if (!poll) return actionError("Poll not found"); - if (poll.status !== "active") { - return actionError("This poll is not open for voting"); - } - const now = Date.now(); - if (poll.startsAt && poll.startsAt.getTime() > now) { - return actionError("This poll has not started yet"); - } - if (poll.endsAt && poll.endsAt.getTime() < now) { - return actionError("This poll has ended"); - } + const questionIds = await tx + .select({ id: WebsitePollQuestion.id }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.pollId, poll.id)) + .orderBy(WebsitePollQuestion.id); + const questions = []; + for (const questionId of questionIds) { + const [question] = await tx + .select({ + id: WebsitePollQuestion.id, + pollId: WebsitePollQuestion.pollId, + type: WebsitePollQuestion.type, + options: WebsitePollQuestion.options, + }) + .from(WebsitePollQuestion) + .where(eq(WebsitePollQuestion.id, questionId.id)) + .for("update"); + if (question) questions.push(question); + } - const questions = await db - .select({ - id: WebsitePollQuestion.id, - pollId: WebsitePollQuestion.pollId, - type: WebsitePollQuestion.type, - options: WebsitePollQuestion.options, - }) - .from(WebsitePollQuestion) - .where(eq(WebsitePollQuestion.pollId, poll.id)); + const questionById = new Map( + questions.map((question) => [question.id, question]), + ); + const seen = new Set(); + const normalizedAnswers = new Map(); - const questionById = new Map(questions.map((q) => [q.id, q])); - const seen = new Set(); + for (const vote of ctx.data.votes) { + if (seen.has(vote.questionId)) { + return actionError("Duplicate vote for the same question"); + } + seen.add(vote.questionId); - for (const vote of ctx.data.votes) { - if (seen.has(vote.questionId)) { - return actionError("Duplicate vote for the same question"); - } - seen.add(vote.questionId); + const question = questionById.get(vote.questionId); + if (!question || question.pollId !== poll.id) { + return actionError("Invalid question for this poll"); + } - const question = questionById.get(vote.questionId); - if (!question || question.pollId !== poll.id) { - return actionError("Invalid question for this poll"); - } + const answer = vote.answer.trim(); + if (!answer) return actionError("Answer is required"); + const options = parsePollOptions(question.options); + const selected = parsePollAnswerSelections( + question.type as PollQuestionType, + answer, + ); - const answer = vote.answer.trim(); - if (!answer) return actionError("Answer is required"); + if (question.type === "text") { + normalizedAnswers.set(vote.questionId, answer); + } else if (question.type === "multiple") { + if (selected.length === 0) { + return actionError("Select at least one option"); + } + if (selected.some((selection) => !options.includes(selection))) { + return actionError("Invalid option selected"); + } + normalizedAnswers.set(vote.questionId, selected.join("\n")); + } else { + const selectedOption = selected[0] ?? ""; + if (!options.includes(selectedOption)) { + return actionError("Invalid option selected"); + } + normalizedAnswers.set(vote.questionId, selectedOption); + } - const options = parsePollOptions(question.options); - const selected = parsePollAnswerSelections( - question.type as PollQuestionType, - answer, + const [existing] = await tx + .select({ id: WebsitePollVote.id }) + .from(WebsitePollVote) + .where( + and( + eq(WebsitePollVote.questionId, vote.questionId), + eq(WebsitePollVote.userId, userId), + ), + ) + .limit(1); + if (existing) { + return actionError("You have already voted on this poll"); + } + } + + const votesToInsert = [...ctx.data.votes].sort( + (left, right) => left.questionId - right.questionId, + ); + for (const vote of votesToInsert) { + await tx.insert(WebsitePollVote).values({ + questionId: vote.questionId, + userId, + answer: normalizedAnswers.get(vote.questionId) ?? "", + }); + } + return actionOk({ pollId: poll.id }); + }, + { isolationLevel: "read committed" }, ); - - if (question.type === "text") { - if (answer.length > 500) { - return actionError("Answer is too long"); - } - } else if (question.type === "multiple") { - if (selected.length === 0) { - return actionError("Select at least one option"); - } - if (selected.some((selection) => !options.includes(selection))) { - return actionError("Invalid option selected"); - } - } else if (!options.includes(selected[0] ?? "")) { - return actionError("Invalid option selected"); - } - - const [existing] = await db - .select({ id: WebsitePollVote.id }) - .from(WebsitePollVote) - .where( - and( - eq(WebsitePollVote.questionId, vote.questionId), - eq(WebsitePollVote.userId, userId), - ), - ) - .limit(1); - if (existing) { + } catch (error) { + if (isDuplicateKey(error)) { return actionError("You have already voted on this poll"); } + throw error; } - - const votesToInsert = [...ctx.data.votes].sort( - (left, right) => left.questionId - right.questionId, - ); - await db.transaction(async (tx) => { - for (const vote of votesToInsert) { - await tx.insert(WebsitePollVote).values({ - questionId: vote.questionId, - userId, - answer: vote.answer.trim(), - }); - } - }); + if (!outcome.ok) return outcome; revalidatePath("/polls"); - revalidatePath(`/polls/${poll.id}`); - return actionOk({ pollId: poll.id }); + revalidatePath(`/polls/${ctx.data.pollId}`); + return outcome; }, ); diff --git a/src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts b/src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts index 83b9ce8f..27f014b4 100644 --- a/src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts +++ b/src/features/housekeeping/domains/content/services/mutation-runtime-database.test.ts @@ -486,14 +486,17 @@ describe("Content database mutation runtime partial updates", () => { }); it("canonicalizes submitted question options and preserves omitted columns", async () => { - database.selected({ - id: 7, - pollId: 3, - question: "Favourite colour?", - type: "single", - sortOrder: 0, - options: "Red\nBlue", - }); + database.queueSelected( + { + id: 7, + pollId: 3, + question: "Favourite colour?", + type: "single", + sortOrder: 0, + options: "Red\nBlue", + }, + [], + ); await executeContentDatabaseMutation( "poll-question.change", @@ -508,6 +511,53 @@ describe("Content database mutation runtime partial updates", () => { ]); }); + it("rejects semantic question edits after votes exist", async () => { + database.queueSelected( + { + id: 7, + pollId: 3, + question: "Favourite colour?", + type: "single", + sortOrder: 0, + options: "Red\nBlue", + }, + { id: 91 }, + ); + + await expect( + executeContentDatabaseMutation( + "poll-question.change", + { action: "update", id: 7, options: "Green\nYellow" }, + context, + undefined, + ), + ).rejects.toMatchObject({ code: "CONFLICT" }); + expect(database.set).not.toHaveBeenCalled(); + }); + + it("allows sort-order-only question edits after votes exist", async () => { + database.queueSelected( + { + id: 7, + pollId: 3, + question: "Favourite colour?", + type: "single", + sortOrder: 0, + options: "Red\nBlue", + }, + { id: 91 }, + ); + + await executeContentDatabaseMutation( + "poll-question.change", + { action: "update", id: 7, sortOrder: 2 }, + context, + undefined, + ); + + expect(database.set).toHaveBeenCalledWith({ sortOrder: 2 }); + }); + it("blocks deletion of an event type referenced by an event", async () => { database.queueSelected({ id: 7, name: "Tournament" }, { id: 11 }); diff --git a/src/features/housekeeping/domains/content/services/mutation-runtime-database.ts b/src/features/housekeeping/domains/content/services/mutation-runtime-database.ts index 84bb5f47..f0d28b37 100644 --- a/src/features/housekeeping/domains/content/services/mutation-runtime-database.ts +++ b/src/features/housekeeping/domains/content/services/mutation-runtime-database.ts @@ -26,7 +26,13 @@ import { } from "@/lib/db"; import { slugify } from "@/lib/format"; import { canonicalize } from "@/lib/foundation/security"; -import { serializePollOptions } from "@/lib/polls/poll-semantics"; +import { + hasPollQuestionSemanticChange, + POLL_PUBLIC_QUESTION_LIMIT, + type PollQuestionType, + pollQuestionLimitReached, + serializePollOptions, +} from "@/lib/polls/poll-semantics"; import { logStaffActivity } from "@/lib/services/staff-activity"; import { createEventSchema, @@ -682,7 +688,7 @@ async function pollChange( if (!merged.success) throw validation(merged.error); if ( merged.data.status === "active" && - (await pollQuestionCount(connection, id)) > 50 + (await pollQuestionCount(connection, id)) > POLL_PUBLIC_QUESTION_LIMIT ) { throw conflict(); } @@ -713,10 +719,7 @@ async function pollQuestionChange( .for("update"); if (!parent) throw notFound(); const questionCount = await pollQuestionCount(connection, pollId); - if ( - questionCount >= 100 || - (parent.status === "active" && questionCount >= 50) - ) { + if (pollQuestionLimitReached(parent.status, questionCount)) { throw conflict(); } const parsed = pollQuestionSchema.safeParse(data); @@ -783,6 +786,19 @@ async function pollQuestionChange( merged.data.options, ); } + if ( + hasPollQuestionSemanticChange( + { ...existing, type: existing.type as PollQuestionType }, + merged.data, + ) + ) { + const [vote] = await connection + .select({ id: WebsitePollVote.id }) + .from(WebsitePollVote) + .where(eq(WebsitePollVote.questionId, id)) + .limit(1); + if (vote) throw conflict(); + } await connection .update(WebsitePollQuestion) .set(values) diff --git a/src/lib/polls/poll-semantics.ts b/src/lib/polls/poll-semantics.ts index fd01ce2a..368ef78d 100644 --- a/src/lib/polls/poll-semantics.ts +++ b/src/lib/polls/poll-semantics.ts @@ -1,5 +1,8 @@ export const POLL_QUESTION_TYPES = ["single", "multiple", "text"] as const; export type PollQuestionType = (typeof POLL_QUESTION_TYPES)[number]; +export const POLL_PUBLIC_QUESTION_LIMIT = 50; +export const POLL_DRAFT_QUESTION_LIMIT = 100; +export const POLL_ANSWER_MAX_LENGTH = 500; export function parsePollOptions(value: string): string[] { return value @@ -24,3 +27,33 @@ export function parsePollAnswerSelections( ? parsePollOptions(normalized) : [normalized.trim()].filter(Boolean); } + +export function pollQuestionLimitReached( + status: string, + currentCount: number, +): boolean { + const limit = + status === "active" + ? POLL_PUBLIC_QUESTION_LIMIT + : POLL_DRAFT_QUESTION_LIMIT; + return currentCount >= limit; +} + +interface PollQuestionSemantics { + readonly question: string; + readonly type: PollQuestionType; + readonly options: string; +} + +export function hasPollQuestionSemanticChange( + before: PollQuestionSemantics, + after: PollQuestionSemantics, +): boolean { + return ( + before.question.normalize("NFC").trim() !== + after.question.normalize("NFC").trim() || + before.type !== after.type || + serializePollOptions(before.type, before.options) !== + serializePollOptions(after.type, after.options) + ); +} diff --git a/src/lib/validators/poll.test.ts b/src/lib/validators/poll.test.ts index 94302020..5767c51b 100644 --- a/src/lib/validators/poll.test.ts +++ b/src/lib/validators/poll.test.ts @@ -112,6 +112,50 @@ describe("pollQuestionSchema", () => { }).success, ).toBe(false); }); + + it("keeps every single-choice option compatible with the public answer limit", () => { + const exactly500 = `A${"x".repeat(499)}`; + const tooLong = `B${"x".repeat(500)}`; + + expect( + pollQuestionSchema.safeParse({ + pollId: 1, + question: "Boundary?", + type: "single", + options: `${exactly500}\nShort`, + }).success, + ).toBe(true); + expect( + pollQuestionSchema.safeParse({ + pollId: 1, + question: "Boundary?", + type: "single", + options: `${tooLong}\nShort`, + }).success, + ).toBe(false); + }); + + it("bounds the possible serialized multiple-choice answer at 500 characters", () => { + const exactly500 = `${"A".repeat(249)}\n${"B".repeat(250)}`; + const tooLong = `${"A".repeat(250)}\n${"B".repeat(250)}`; + + expect( + pollQuestionSchema.safeParse({ + pollId: 1, + question: "Boundary?", + type: "multiple", + options: exactly500, + }).success, + ).toBe(true); + expect( + pollQuestionSchema.safeParse({ + pollId: 1, + question: "Boundary?", + type: "multiple", + options: tooLong, + }).success, + ).toBe(false); + }); }); describe("poll schedule validation", () => { it("requires the end time to be later than the start time", () => { @@ -145,6 +189,17 @@ describe("pollVoteSchema", () => { pollVoteSchema.safeParse({ questionId: 0, answer: "x" }).success, ).toBe(false); }); + + it("accepts 500 characters and rejects 501", () => { + expect( + pollVoteSchema.safeParse({ questionId: 5, answer: "A".repeat(500) }) + .success, + ).toBe(true); + expect( + pollVoteSchema.safeParse({ questionId: 5, answer: "A".repeat(501) }) + .success, + ).toBe(false); + }); }); describe("voteOnPollSchema", () => { diff --git a/src/lib/validators/poll.ts b/src/lib/validators/poll.ts index 4658f109..fca3756d 100644 --- a/src/lib/validators/poll.ts +++ b/src/lib/validators/poll.ts @@ -1,5 +1,6 @@ import { z } from "zod"; import { + POLL_ANSWER_MAX_LENGTH, POLL_QUESTION_TYPES, parsePollOptions, } from "@/lib/polls/poll-semantics"; @@ -7,9 +8,9 @@ import { const pollFields = z.object({ title: z.string().trim().min(1, "Title is required").max(255), description: z.string().max(2_000).nullable().optional(), - status: z.enum(["draft", "active", "closed"]).default("draft"), - showResults: z.coerce.number().int().min(0).max(1).default(1), - multipleChoice: z.coerce.number().int().min(0).max(1).default(0), + status: z.enum(["draft", "active", "closed"]), + showResults: z.coerce.number().int().min(0).max(1), + multipleChoice: z.coerce.number().int().min(0).max(1), startsAt: z.coerce.date().nullable().optional(), endsAt: z.coerce.date().nullable().optional(), }); @@ -27,7 +28,13 @@ function validateSchedule( } } -export const createPollSchema = pollFields.superRefine(validateSchedule); +export const createPollSchema = pollFields + .extend({ + status: pollFields.shape.status.default("draft"), + showResults: pollFields.shape.showResults.default(1), + multipleChoice: pollFields.shape.multipleChoice.default(0), + }) + .superRefine(validateSchedule); export const updatePollSchema = pollFields .partial() .superRefine(validateSchedule); @@ -35,9 +42,9 @@ export const updatePollSchema = pollFields const pollQuestionFields = z.object({ pollId: z.coerce.number().int().positive(), question: z.string().trim().min(1).max(500), - type: z.enum(POLL_QUESTION_TYPES).default("single"), - sortOrder: z.coerce.number().int().min(0).default(0), - options: z.string().max(20_000).default(""), + type: z.enum(POLL_QUESTION_TYPES), + sortOrder: z.coerce.number().int().min(0), + options: z.string().max(20_000), }); function validateQuestionOptions( @@ -70,18 +77,42 @@ function validateQuestionOptions( path: ["options"], message: "At most 100 options are allowed", }); + if ( + data.type !== "text" && + options.some((option) => option.length > POLL_ANSWER_MAX_LENGTH) + ) { + context.addIssue({ + code: "custom", + path: ["options"], + message: "Each option must fit the public answer limit", + }); + } + if ( + data.type === "multiple" && + options.join("\n").length > POLL_ANSWER_MAX_LENGTH + ) { + context.addIssue({ + code: "custom", + path: ["options"], + message: "All selected options must fit the public answer limit", + }); + } } -export const pollQuestionSchema = pollQuestionFields.superRefine( - validateQuestionOptions, -); +export const pollQuestionSchema = pollQuestionFields + .extend({ + type: pollQuestionFields.shape.type.default("single"), + sortOrder: pollQuestionFields.shape.sortOrder.default(0), + options: pollQuestionFields.shape.options.default(""), + }) + .superRefine(validateQuestionOptions); export const pollQuestionPatchSchema = pollQuestionFields .omit({ pollId: true }) .partial(); export const pollVoteSchema = z.object({ questionId: z.coerce.number().int().positive(), - answer: z.string().min(1).max(500), + answer: z.string().min(1).max(POLL_ANSWER_MAX_LENGTH), }); export const voteOnPollSchema = z.object({