diff --git a/src/components/DraftReviewBar.svelte b/src/components/DraftReviewBar.svelte index edac7af1c..6dd31e7d9 100644 --- a/src/components/DraftReviewBar.svelte +++ b/src/components/DraftReviewBar.svelte @@ -9,6 +9,7 @@ import type { DraftReview } from "@app/lib/draftReviewStorage"; import { draftReviewStorage } from "@app/lib/draftReviewStorage"; import { formatAnchor } from "@app/lib/pierreComments"; + import { isPublishableReview, verdictBadge } from "@app/lib/utils"; import Button from "@app/components/Button.svelte"; import Icon from "@app/components/Icon.svelte"; @@ -182,20 +183,9 @@ } const verdictLabel = $derived( - verdict === "accept" - ? "Accept revision" - : verdict === "reject" - ? "Reject revision" - : "Comment", - ); - - const verdictColorClass = $derived( - verdict === "accept" - ? "accept" - : verdict === "reject" - ? "reject" - : "comment", + verdictOptions.find(option => option.value === verdict)?.label, ); + const verdictColorClass = $derived(verdictBadge(verdict).variant); async function persistSummary() { draftReviewStorage.update(draftReview.id, { @@ -233,12 +223,8 @@ } } - // The protocol rejects a verdict-less review with no summary (EmptyReview), - // so a comment-only review requires summary text; accept/reject don't. const publishDisabled = $derived( - publishing || - alreadyReviewed || - (verdict === undefined && summary.trim() === ""), + publishing || alreadyReviewed || !isPublishableReview(verdict, summary), ); const summaryRequired = $derived(verdict === undefined); diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 791e3eff4..fc01caebb 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -326,6 +326,15 @@ export function verdictAction(verdict: Review["verdict"]): string { } } +// The protocol rejects a verdict-less review with no summary (EmptyReview), +// so a comment-only review requires summary text; accept/reject don't. +export function isPublishableReview( + verdict: Review["verdict"], + summary: string, +): boolean { + return verdict !== undefined || summary.trim() !== ""; +} + export function verdictBadge(verdict: Review["verdict"]): { label: string; variant: "accept" | "reject" | "comment"; diff --git a/tests/unit/utils.test.ts b/tests/unit/utils.test.ts index b4f52cfea..017bb595f 100644 --- a/tests/unit/utils.test.ts +++ b/tests/unit/utils.test.ts @@ -10,6 +10,7 @@ import { formatTimestamp, identityKey, isCommit, + isPublishableReview, parseNodeId, parseRepositoryId, pluralize, @@ -17,6 +18,9 @@ import { safeHttpUrl, truncateDid, unqualifyBranch, + verdictAction, + verdictBadge, + verdictIcon, } from "@app/lib/utils"; const rid = "rad:z3fpY7nttPPa6MBnAv2DccHzQJnqe"; @@ -273,3 +277,27 @@ describe("explorer links", () => { ); }); }); + +describe("verdicts", () => { + test.each([ + ["accept", "Accept", "Accepted", "accept", "thumbs-up"], + ["reject", "Reject", "Rejected", "reject", "stop"], + [undefined, "Comment", "Reviewed", "comment", "comment"], + ] as const)("%j", (verdict, action, label, variant, icon) => { + expect(verdictAction(verdict)).toBe(action); + expect(verdictBadge(verdict)).toEqual({ label, variant }); + expect(verdictIcon(verdict)).toBe(icon); + }); + + test.each([ + ["accept", "", true], + ["reject", " ", true], + [undefined, "Looks fine", true], + [undefined, " \n", false], + ] as const)( + "a %j review with summary %j can be published: %j", + (verdict, summary, expected) => { + expect(isPublishableReview(verdict, summary)).toBe(expected); + }, + ); +});