diff --git a/app/src/components/sidebar/ReviewSidebar.tsx b/app/src/components/sidebar/ReviewSidebar.tsx index 3ca442ea..7552ec80 100644 --- a/app/src/components/sidebar/ReviewSidebar.tsx +++ b/app/src/components/sidebar/ReviewSidebar.tsx @@ -14,6 +14,7 @@ import { Combobox } from "@/components/ui/combobox"; import { cn } from "@/lib/utils"; import { deadlineTickMs, formatTimeRemaining } from "@/lib/reviewDeadline"; +import { validateReviewDecision } from "@/lib/reviewValidation"; import { castDecision } from "@/lib/api/reviews"; import { getRevision, reviseRevision } from "@/lib/api/guideRevisions"; import { GuidelinesModal } from "@/components/modals/GuidelinesModal"; @@ -102,19 +103,7 @@ export const ReviewSidebar = ({ revisionData.case.status !== "approved" && revisionData.case.status !== "rejected"; - const validateReview = () => { - if (review.decision === "") - return "Choose approve or reject before submitting"; - if (review.decision === "approve") return ""; - - const missing = []; - if (review.reasons.length === 0) missing.push("at least one reason"); - if (review.notes.length === 0) missing.push("a note"); - - return missing.length === 0 - ? "" - : `Rejections require ${missing.join(" and ")}`; - }; + const validateReview = () => validateReviewDecision(review); const submitDecision = async () => { abortControllerRef.current?.abort(); diff --git a/app/src/lib/__tests__/reviewValidation.test.ts b/app/src/lib/__tests__/reviewValidation.test.ts new file mode 100644 index 00000000..20df5bf7 --- /dev/null +++ b/app/src/lib/__tests__/reviewValidation.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from "vitest"; + +import { + hasMeaningfulText, + validateReviewDecision, +} from "@/lib/reviewValidation"; + +const rejection = (notes: string) => ({ + decision: "reject", + notes, + reasons: ["inaccurate"], +}); + +describe("hasMeaningfulText", () => { + it("accepts letters and numbers in any script", () => { + expect(hasMeaningfulText("Needs a worked example.")).toBe(true); + expect(hasMeaningfulText("需要一个例子")).toBe(true); + expect(hasMeaningfulText("يحتاج إلى مثال")).toBe(true); + expect(hasMeaningfulText("Нужен пример")).toBe(true); + expect(hasMeaningfulText("42")).toBe(true); + }); + + it("rejects input with no letters or numbers", () => { + expect(hasMeaningfulText("")).toBe(false); + expect(hasMeaningfulText(" ")).toBe(false); + expect(hasMeaningfulText("\t\n")).toBe(false); + expect(hasMeaningfulText("...")).toBe(false); + expect(hasMeaningfulText("-")).toBe(false); + }); +}); + +describe("validateReviewDecision", () => { + it("asks for a decision before anything else", () => { + expect( + validateReviewDecision({ decision: "", notes: "", reasons: [] }) + ).toBe("Choose approve or reject before submitting"); + }); + + it("lets an approval through without notes or reasons", () => { + expect( + validateReviewDecision({ decision: "approve", notes: "", reasons: [] }) + ).toBe(""); + }); + + it("lets a complete rejection through", () => { + expect(validateReviewDecision(rejection("Missing prerequisites."))).toBe( + "" + ); + }); + + it("rejects a note with no readable text", () => { + expect(validateReviewDecision(rejection(" "))).toBe( + "Rejections require a note" + ); + expect(validateReviewDecision(rejection("..."))).toBe( + "Rejections require a note" + ); + }); + + it("still requires at least one reason", () => { + expect( + validateReviewDecision({ + decision: "reject", + notes: "Needs work.", + reasons: [], + }) + ).toBe("Rejections require at least one reason"); + }); + + it("reports both problems together", () => { + expect( + validateReviewDecision({ decision: "reject", notes: " ", reasons: [] }) + ).toBe("Rejections require at least one reason and a note"); + }); +}); diff --git a/app/src/lib/reviewValidation.ts b/app/src/lib/reviewValidation.ts new file mode 100644 index 00000000..80a4275b --- /dev/null +++ b/app/src/lib/reviewValidation.ts @@ -0,0 +1,31 @@ +export type ReviewDecisionInput = { + decision: string; + notes: string; + reasons: Array; +}; + +// Any letter or number in any script counts, so notes are not limited to Latin +// characters. Whitespace and bare punctuation are not a note. +const MEANINGFUL_TEXT = /[\p{L}\p{N}]/u; + +export function hasMeaningfulText(value: string): boolean { + return MEANINGFUL_TEXT.test(value); +} + +/** + * Describes what is stopping a decision from being submitted, or returns an + * empty string when it is ready to go. + */ +export function validateReviewDecision(review: ReviewDecisionInput): string { + if (review.decision === "") + return "Choose approve or reject before submitting"; + if (review.decision === "approve") return ""; + + const missing = []; + if (review.reasons.length === 0) missing.push("at least one reason"); + if (!hasMeaningfulText(review.notes)) missing.push("a note"); + + return missing.length === 0 + ? "" + : `Rejections require ${missing.join(" and ")}`; +}