fix(app): require review notes to contain readable text

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();

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");
+  });
+});

app/src/lib/reviewValidation.ts

@@ -0,0 +1,31 @@
+export type ReviewDecisionInput = {
+  decision: string;
+  notes: string;
+  reasons: Array<string>;
+};
+
+// 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 ")}`;
+}