diff --git a/CHANGELOG.md b/CHANGELOG.md index 73eca1d..1c34c8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ pin and a line to read before you move. ## [Unreleased] +### Added + +- PR list icons reflect approvals, requested changes, closed and merged states using GitHub-style + colors. Detail metadata names current approving reviewers. + ## [1.0.1] — 2026-09-12 ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index 828e6e5..ddec007 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -565,3 +565,19 @@ what let a *cold* client — a fresh reload, a different device, a daemon restar calls too. `force` on `board.load` is still the Refresh button asking both layers to bypass themselves at once. Whatever should outlive an unmount and is not shaped like a cache belongs in one of the two settings stores instead — that is the only other thing here that survives one. + +## Review status in PR lists + +`shared/pr-review.ts` normalizes GitHub's `reviewDecision`, with `latestOpinionatedReviews` as the +fallback for repositories without required reviews. A truncated review page must not imply approval. +`server/board/item.ts` maps this information for every caller, including independent search views. +Projects fetch the same review fields in `server/projects/single.ts`; `client/lib/project-item-status.ts` +adapts their state to the shared PR icon renderer. Approval and requested changes keep the PR glyph, +with color and an accessible label carrying the review state. +Detail loading paginates current opinions to list all approving reviewers, ignoring dismissed, +comment-only, pending and superseded approvals. Cache namespaces change when review fields are added. + +`client/lib/pr-status.ts` owns terminal/draft/review precedence and accessible status labels. +`client/theme/pr-colors.ts` uses GitHub's light/dark state hues because Paseo has no merged/purple +semantic token and arbitrary accent colors must not look like merge status. This intentional color +exception serves the GitHub status convention; other UI continues to use the host theme. diff --git a/README.md b/README.md index a627008..b26c8fd 100644 --- a/README.md +++ b/README.md @@ -131,3 +131,14 @@ and the other plugins that lived beside it there are not carried here. Thanks to starting point and for the MIT licence that made it possible. Licensed under the MIT licence — see [LICENSE](LICENSE). + +## Pull request review status + +In the main board, saved views, and Projects, the leading PR icon is green for approval and orange for requested changes, +a red closed-PR icon for closed work, or a purple merge icon for merged work. Drafts remain neutral. +Merged and closed states take precedence over reviews. Status is also included in the row's +accessibility label; no extra review badges or text are added to the list. + +The detail metadata lists **Approved by USERNAME** for each current approving reviewer. +Dismissed or superseded approvals are excluded. GitHub's aggregate review decision takes precedence; +when a repository has no required reviews, the plugin falls back to current reviewer opinions. diff --git a/client/board/item-row-format.ts b/client/board/item-row-format.ts index fbaa594..eb717ce 100644 --- a/client/board/item-row-format.ts +++ b/client/board/item-row-format.ts @@ -1,3 +1,4 @@ +import { prStatusIcon, type PrStatusColors } from "../lib/pr-status"; import type { BoardItem, CheckSummary, ColumnId } from "../../shared/board"; import type { SortOrder } from "../lib/sort"; @@ -43,6 +44,7 @@ export function describeRow({ type, accentColor, mutedColor, + statusColors, closes, hasLabelMenu, isWeb, @@ -53,6 +55,7 @@ export function describeRow({ type: ColumnId; accentColor: string; mutedColor: string; + statusColors: PrStatusColors; /** The linked issues this row closes, already joined for the label. */ closes: string; hasLabelMenu: boolean; @@ -63,27 +66,30 @@ export function describeRow({ const stampLabel = stampMissing ? "updated" : order.rowLabel; const stampDate = stampMissing ? item.updatedAt : stamp; - const iconName = - type === "draft-prs" - ? "GitPullRequestDraft" - : type === "open-prs" - ? "GitPullRequest" - : type === "discussions" - ? "MessageSquare" - : "CircleDot"; - const iconColor = type === "draft-prs" || type === "discussions" ? mutedColor : accentColor; + const pr = + type === "draft-prs" || type === "open-prs" ? prStatusIcon(item, type, statusColors) : null; + const iconName = pr?.iconName ?? (type === "discussions" ? "MessageSquare" : "CircleDot"); + const iconColor = pr?.iconColor ?? (type === "discussions" ? mutedColor : accentColor); const byline = item.author !== null && item.author !== viewerLogin ? item.author : null; const openedBy = byline === null ? "" : `, opened by ${byline}`; const linkedTo = closes === "" ? "" : `, closes ${closes}`; const checksLabel = item.checks === null ? "" : `, checks ${checksSentence(item.checks)}`; - const accessibilityLabel = `${item.repository} #${item.number}: ${item.title}${openedBy}${linkedTo}${checksLabel}`; + const accessibilityLabel = `${item.repository} #${item.number}: ${item.title}${pr === null ? "" : `, ${pr.label}`}${openedBy}${linkedTo}${checksLabel}`; let accessibilityHint: string | undefined; if (hasLabelMenu) { accessibilityHint = isWeb ? "Right-click to edit labels." : "Press and hold to edit labels."; } - return { stampLabel, stampDate, iconName, iconColor, byline, accessibilityLabel, accessibilityHint }; + return { + stampLabel, + stampDate, + iconName, + iconColor, + byline, + accessibilityLabel, + accessibilityHint, + }; } diff --git a/client/board/item-row.tsx b/client/board/item-row.tsx index b365fa2..c2843d3 100644 --- a/client/board/item-row.tsx +++ b/client/board/item-row.tsx @@ -156,6 +156,7 @@ export const ItemRow = memo(function ItemRow({ type, accentColor, mutedColor, + statusColors: styles.prStatusColors, closes, hasLabelMenu: onLabels !== null, isWeb, diff --git a/client/detail/detail-body.tsx b/client/detail/detail-body.tsx index be6e228..ab14f11 100644 --- a/client/detail/detail-body.tsx +++ b/client/detail/detail-body.tsx @@ -49,6 +49,11 @@ export function DetailSummary({ {item.title} {meta} + {(details?.approvedBy ?? []).map((login) => ( + + Approved by {login} + + ))} {details?.branches ? ( {details.branches.head} → {details.branches.base} diff --git a/client/lib/pr-status.test.ts b/client/lib/pr-status.test.ts new file mode 100644 index 0000000..2e80708 --- /dev/null +++ b/client/lib/pr-status.test.ts @@ -0,0 +1,63 @@ +import { expect, it } from "vitest"; +import { prStatusIcon } from "./pr-status"; +import type { BoardItem } from "../../shared/board"; + +const item: BoardItem = { + id: "pr", + number: 1, + title: "Example", + url: "", + repository: "example/project", + owner: "example", + updatedAt: "", + createdAt: "", + lastCommitAt: null, + commentsCount: 0, + labels: [], + author: null, + detail: null, + relations: [], + linkedIssues: [], + checks: null, +}; +const colors = { + approved: "green", + changesRequested: "orange", + closed: "red", + merged: "purple", + neutral: "gray", +}; + +it("keeps PR icons with accessible review colors without adding list text", () => { + expect(prStatusIcon({ ...item, prReview: "approved" }, "open-prs", colors)).toEqual({ + iconName: "GitPullRequest", + iconColor: "green", + label: "Approved", + }); + expect(prStatusIcon({ ...item, prReview: "changes-requested" }, "open-prs", colors)).toEqual({ + iconName: "GitPullRequest", + iconColor: "orange", + label: "Changes requested", + }); + expect(prStatusIcon(item, "open-prs", colors)).toMatchObject({ + iconName: "GitPullRequest", + iconColor: "gray", + }); +}); + +it("gives terminal and draft state precedence over old reviews", () => { + expect( + prStatusIcon( + { ...item, prState: "MERGED", prReview: "changes-requested" }, + "draft-prs", + colors, + ), + ).toMatchObject({ iconName: "GitMerge", iconColor: "purple" }); + expect( + prStatusIcon({ ...item, prState: "CLOSED", prReview: "approved" }, "open-prs", colors), + ).toMatchObject({ iconName: "GitPullRequestClosed", iconColor: "red" }); + expect(prStatusIcon({ ...item, prReview: "approved" }, "draft-prs", colors)).toMatchObject({ + iconName: "GitPullRequestDraft", + iconColor: "gray", + }); +}); diff --git a/client/lib/pr-status.ts b/client/lib/pr-status.ts new file mode 100644 index 0000000..dc3ae55 --- /dev/null +++ b/client/lib/pr-status.ts @@ -0,0 +1,48 @@ +import type { BoardItem, ColumnId } from "../../shared/board"; + +export interface PrStatusColors { + approved: string; + changesRequested: string; + closed: string; + merged: string; + neutral: string; +} + +/** Terminal state and draft status take precedence over earlier review opinions. */ +export function prStatusIcon( + item: Pick, + type: ColumnId, + colors: PrStatusColors, +) { + if (item.prState === "MERGED") + return { iconName: "GitMerge", iconColor: colors.merged, label: "Merged" }; + if (item.prState === "CLOSED") + return { + iconName: "GitPullRequestClosed", + iconColor: colors.closed, + label: "Closed", + }; + if (type === "draft-prs") + return { + iconName: "GitPullRequestDraft", + iconColor: colors.neutral, + label: "Draft", + }; + if (item.prReview === "changes-requested") + return { + iconName: "GitPullRequest", + iconColor: colors.changesRequested, + label: "Changes requested", + }; + if (item.prReview === "approved") + return { + iconName: "GitPullRequest", + iconColor: colors.approved, + label: "Approved", + }; + return { + iconName: "GitPullRequest", + iconColor: colors.neutral, + label: item.prReview === "review-required" ? "Review required" : "Open", + }; +} diff --git a/client/lib/project-item-status.ts b/client/lib/project-item-status.ts new file mode 100644 index 0000000..bb0fd0e --- /dev/null +++ b/client/lib/project-item-status.ts @@ -0,0 +1,39 @@ +import type { PluginTheme } from "@getpaseo/plugin"; +import type { ProjectItem } from "../../shared/board"; +import { prStatusColors } from "../theme/pr-colors"; +import { prStatusIcon } from "./pr-status"; + +export function projectItemGlyph(item: ProjectItem, colors: PluginTheme["colors"]) { + if (item.kind === "draft") + return { + name: "FileText", + color: colors.foregroundMuted, + label: "Draft note", + }; + if (item.kind === "pull-request") { + const status = prStatusIcon( + { + prState: item.state === "merged" ? "MERGED" : item.state === "closed" ? "CLOSED" : "OPEN", + prReview: item.prReview, + }, + item.state === "draft" ? "draft-prs" : "open-prs", + prStatusColors(colors), + ); + return { + name: status.iconName, + color: status.iconColor, + label: status.label, + }; + } + if (item.state === "closed") + return { + name: "CircleSlash", + color: colors.statusDanger, + label: "Closed issue", + }; + return { + name: "CircleDot", + color: colors.statusSuccess, + label: "Open issue", + }; +} diff --git a/client/projects/project-board-view.tsx b/client/projects/project-board-view.tsx index a31a7d7..0409c0a 100644 --- a/client/projects/project-board-view.tsx +++ b/client/projects/project-board-view.tsx @@ -8,7 +8,7 @@ import type { ProjectItem } from "../../shared/board"; import { loadProject } from "../../shared/board"; import type { ProjectsStyles } from "./projects.styles"; -type ThemeColors = PluginSurfaceProps["theme"]["colors"]; +import { projectItemGlyph } from "../lib/project-item-status"; /** What `loadProject` answered with, plus the reference it was asked for. */ interface ProjectDetail { @@ -17,23 +17,6 @@ interface ProjectDetail { columns: readonly { name: string; items: readonly ProjectItem[] }[]; } -/** - * One item's leading glyph and colour, by what it is and where it stands. - * `merged` borrows the accent colour rather than inventing a purple the theme - * does not expose: the six status/accent tokens are what a plugin gets. - */ -function itemGlyph(item: ProjectItem, colors: ThemeColors): { name: string; color: string } { - if (item.kind === "draft") return { name: "FileText", color: colors.foregroundMuted }; - if (item.kind === "pull-request") { - if (item.state === "merged") return { name: "GitMerge", color: colors.accent }; - if (item.state === "closed") return { name: "GitPullRequestClosed", color: colors.statusDanger }; - if (item.state === "draft") return { name: "GitPullRequestDraft", color: colors.foregroundMuted }; - return { name: "GitPullRequest", color: colors.statusSuccess }; - } - if (item.state === "closed") return { name: "CircleSlash", color: colors.statusDanger }; - return { name: "CircleDot", color: colors.statusSuccess }; -} - /** * One project rendered as its own board: the columns GitHub's own project * view groups items into, folded down to the ones that hold something @@ -70,7 +53,12 @@ export function ProjectBoardView({ setDetailLoading(true); fetchProject({ owner, number, force: false }) .then((result) => { - if (live) setDetail({ title: result.title, url: result.url, columns: result.columns }); + if (live) + setDetail({ + title: result.title, + url: result.url, + columns: result.columns, + }); }) .catch((cause: unknown) => { if (live) setDetailError(cause instanceof Error ? cause.message : String(cause)); @@ -96,13 +84,16 @@ export function ProjectBoardView({ const renderProjectItem = useCallback( (item: ProjectItem, spaced: boolean) => { - const glyph = itemGlyph(item, theme.colors); + const glyph = projectItemGlyph(item, theme.colors); const rowStyle = spaced ? [styles.itemRow, styles.itemRowSpaced] : styles.itemRow; return ( value !== null && value !== undefined) + .join(", ")} disabled={item.url === null} onPress={item.url !== null ? () => onOpenUrl(item.url as string) : undefined} > @@ -153,7 +144,9 @@ export function ProjectBoardView({ {visibleColumns.map((column) => ( - {column.name === "" ? "No status" : column.name} + + {column.name === "" ? "No status" : column.name} + {column.items.map((item, index) => renderProjectItem(item, index > 0))} ))} diff --git a/client/theme/pr-colors.ts b/client/theme/pr-colors.ts new file mode 100644 index 0000000..bf50867 --- /dev/null +++ b/client/theme/pr-colors.ts @@ -0,0 +1,28 @@ +import type { PluginTheme } from "@getpaseo/plugin"; +import type { PrStatusColors } from "../lib/pr-status"; + +/** GitHub's state hues stay distinct even when a Paseo theme uses a purple accent. */ +export function prStatusColors(colors: PluginTheme["colors"]): PrStatusColors { + const hex = /^#([0-9a-f]{6})$/i.exec(colors.surface0)?.[1]; + const brightness = + hex === undefined + ? 255 + : 0.2126 * parseInt(hex.slice(0, 2), 16) + + 0.7152 * parseInt(hex.slice(2, 4), 16) + + 0.0722 * parseInt(hex.slice(4, 6), 16); + return brightness < 128 + ? { + approved: "#3fb950", + changesRequested: "#db6d28", + closed: "#f85149", + merged: "#a371f7", + neutral: colors.foregroundMuted, + } + : { + approved: "#1a7f37", + changesRequested: "#bc4c00", + closed: "#d1242f", + merged: "#8250df", + neutral: colors.foregroundMuted, + }; +} diff --git a/client/theme/use-styles.ts b/client/theme/use-styles.ts index d34494c..83e51ce 100644 --- a/client/theme/use-styles.ts +++ b/client/theme/use-styles.ts @@ -1,3 +1,4 @@ +import { prStatusColors } from "./pr-colors"; import { useMemo } from "react"; import type { PluginSurfaceProps } from "@getpaseo/plugin/client"; @@ -18,6 +19,7 @@ export function useStyles(props: PluginSurfaceProps) { return useMemo(() => { const tokens = computeThemeTokens(props); return { + prStatusColors: prStatusColors(props.theme.colors), ...buildBoardStyles(props, tokens), ...buildLaunchStyles(props, tokens), ...buildSettingsStyles(props, tokens), diff --git a/server/board/cache.ts b/server/board/cache.ts index 487a5fb..c535d7e 100644 --- a/server/board/cache.ts +++ b/server/board/cache.ts @@ -14,7 +14,7 @@ export interface CachedBoard { } /** Keyed by login, limit and the watched owners — see `loadBoardHandler`. */ -export const boardCache = new Cache("board"); +export const boardCache = new Cache("board-reviews-v1"); /** * Keeps the cached board honest. Without this a label edited now would be diff --git a/server/board/item.ts b/server/board/item.ts index 2b37b3a..db4ac24 100644 --- a/server/board/item.ts +++ b/server/board/item.ts @@ -1,3 +1,4 @@ +import { reviewStatus } from "../../shared/pr-review"; import type { BoardItem } from "../../shared/board"; import type { GhSearchNode } from "./types"; @@ -24,6 +25,8 @@ export function toItem( typeof node.repository?.nameWithOwner === "string" ? node.repository.nameWithOwner : ""; const ownerSeparator = repository.indexOf("/"); return { + ...(node.state === "OPEN" || node.state === "CLOSED" || node.state === "MERGED" ? { prState: node.state } : {}), + prReview: reviewStatus(node.reviewDecision, node.latestOpinionatedReviews), id: typeof node.id === "string" ? node.id : String(node.url), number: typeof node.number === "number" ? node.number : 0, title: typeof node.title === "string" ? node.title : "", diff --git a/server/board/pull-requests.ts b/server/board/pull-requests.ts index ed838e2..529cc6a 100644 --- a/server/board/pull-requests.ts +++ b/server/board/pull-requests.ts @@ -19,6 +19,12 @@ const PULL_REQUEST_SELECTION = `... on PullRequest { updatedAt createdAt isDraft + state + reviewDecision + latestOpinionatedReviews(first: 100) { + nodes { state } + pageInfo { hasNextPage endCursor } + } author { login } comments { totalCount } labels(first: 20) { nodes { name } } @@ -33,6 +39,9 @@ const PULL_REQUEST_SELECTION = `... on PullRequest { interface GhPullRequestNode extends GhSearchNode { isDraft?: unknown; + state?: unknown; + reviewDecision?: unknown; + latestOpinionatedReviews?: unknown; closingIssuesReferences?: { nodes?: unknown }; commits?: { nodes?: unknown }; } diff --git a/server/board/types.ts b/server/board/types.ts index 8123729..bbea8a4 100644 --- a/server/board/types.ts +++ b/server/board/types.ts @@ -3,6 +3,9 @@ import { RELATION_IDS } from "../../shared/board"; /** The fields every relation-bucket search selection asks for, regardless of item type. */ export interface GhSearchNode { + state?: unknown; + reviewDecision?: unknown; + latestOpinionatedReviews?: unknown; id?: unknown; number?: unknown; title?: unknown; diff --git a/server/items/details.test.ts b/server/items/details.test.ts new file mode 100644 index 0000000..2fff30e --- /dev/null +++ b/server/items/details.test.ts @@ -0,0 +1,33 @@ +import { expect, it, vi } from "vitest"; +import { fetchItemDetails } from "./details"; + +const response = (states: string[], more: boolean, offset = 0) => + JSON.stringify({ + data: { + node: { + state: "OPEN", + headRefName: "feature", + baseRefName: "main", + latestOpinionatedReviews: { + nodes: states.map((state, i) => ({ state, author: { login: `reviewer${i + offset}` } })), + pageInfo: { hasNextPage: more, endCursor: more ? "next" : null }, + }, + }, + }, + }); + +it("loads all pages of current reviewers and excludes dismissed approvals", async () => { + const execute = vi + .fn() + .mockResolvedValueOnce(response(["DISMISSED", "APPROVED"], true)) + .mockResolvedValueOnce(response(["APPROVED"], false, 2)); + const details = await fetchItemDetails("pr-id", execute); + expect(details.approvedBy).toEqual(["reviewer1", "reviewer2"]); + expect(execute.mock.calls[1]?.[0]).toContain("cursor=next"); +}); + +it("does not interpret issue or discussion metadata as PR reviews", async () => { + const execute = vi.fn().mockResolvedValue(JSON.stringify({ data: { node: { state: "OPEN" } } })); + expect((await fetchItemDetails("issue-id", execute)).review).toBeNull(); + expect(execute).toHaveBeenCalledTimes(1); +}); diff --git a/server/items/details.ts b/server/items/details.ts index 209c4d4..5d9f9ea 100644 --- a/server/items/details.ts +++ b/server/items/details.ts @@ -1,3 +1,4 @@ +import { approvalLogins, OpinionatedReviewsSchema } from "../../shared/pr-review"; import type { z } from "zod"; import type { ItemDetails, MergeMethod, ReviewState, loadItem } from "../../shared/board"; import { gh } from "../github/gh"; @@ -12,7 +13,7 @@ import { Cache } from "../cache/cache"; * the inline fragments decide which fields come back. A discussion carries no * assignees on GitHub, and only a pull request has branches. */ -const ITEM_QUERY = `query($id: ID!) { +const ITEM_QUERY = `query($id: ID!, $cursor: String) { node(id: $id) { ... on Issue { body state createdAt @@ -22,6 +23,10 @@ const ITEM_QUERY = `query($id: ID!) { body state isDraft createdAt baseRefName headRefName viewerDidAuthor mergeable viewerLatestReview { state } + latestOpinionatedReviews(first: 100, after: $cursor) { + nodes { state author { login } } + pageInfo { hasNextPage endCursor } + } assignees(first: 20) { nodes { login } } repository { viewerPermission @@ -35,6 +40,7 @@ const ITEM_QUERY = `query($id: ID!) { }`; interface GhItemNode { + latestOpinionatedReviews?: unknown; body?: unknown; state?: unknown; isDraft?: unknown; @@ -139,14 +145,48 @@ function toItemDetails(node: GhItemNode): ItemDetails { }; } -export async function fetchItemDetails(id: string): Promise { - const raw = await gh(["api", "graphql", "-f", `query=${ITEM_QUERY}`, "-f", `id=${id}`]); +export async function fetchItemDetails(id: string, execute = gh): Promise { + const raw = await execute(["api", "graphql", "-f", `query=${ITEM_QUERY}`, "-f", `id=${id}`]); const parsed: unknown = JSON.parse(raw); const node = (parsed as { data?: { node?: unknown } }).data?.node; if (typeof node !== "object" || node === null) { throw new Error("GitHub no longer has this item, or the account cannot see it."); } - return toItemDetails(node as GhItemNode); + const details = toItemDetails(node as GhItemNode); + if (details.branches === null) return details; + let page = OpinionatedReviewsSchema.parse(Reflect.get(node, "latestOpinionatedReviews")); + const opinions = [...page.nodes]; + const cursors = new Set(); + while (page.pageInfo.hasNextPage) { + const cursor = page.pageInfo.endCursor; + if (cursor === null || cursors.has(cursor)) + throw new Error("GitHub returned an invalid review cursor."); + cursors.add(cursor); + const next: unknown = JSON.parse( + await execute([ + "api", + "graphql", + "-f", + `query=${ITEM_QUERY}`, + "-f", + `id=${id}`, + "-f", + `cursor=${cursor}`, + ]), + ); + if (typeof next !== "object" || next === null) + throw new Error("Invalid GitHub review response."); + const data: unknown = Reflect.get(next, "data"); + const nextNode: unknown = + typeof data === "object" && data !== null ? Reflect.get(data, "node") : null; + page = OpinionatedReviewsSchema.parse( + typeof nextNode === "object" && nextNode !== null + ? Reflect.get(nextNode, "latestOpinionatedReviews") + : null, + ); + opinions.push(...page.nodes); + } + return { ...details, approvedBy: approvalLogins(opinions) }; } /** @@ -156,7 +196,7 @@ export async function fetchItemDetails(id: string): Promise { */ export const DETAILS_TTL_MS = 5 * 60_000; -export const detailsCache = new Cache("item-details"); +export const detailsCache = new Cache("item-details-reviews-v1"); export async function loadItemHandler({ id, diff --git a/server/projects/single.test.ts b/server/projects/single.test.ts new file mode 100644 index 0000000..589f925 --- /dev/null +++ b/server/projects/single.test.ts @@ -0,0 +1,41 @@ +import { expect, it } from "vitest"; +import { ProjectItemSchema } from "../../shared/board"; +import { toProjectItem } from "./single"; + +it("preserves project PR review status through the RPC schema", () => { + for (const [decision, expected] of [ + ["APPROVED", "approved"], + ["CHANGES_REQUESTED", "changes-requested"], + ["REVIEW_REQUIRED", "review-required"], + ]) { + const item = ProjectItemSchema.parse( + toProjectItem({ + __typename: "PullRequest", + state: "OPEN", + reviewDecision: decision, + }), + ); + expect(item.prReview).toBe(expected); + } +}); + +it("uses current opinions when projects have no required review decision", () => { + expect( + toProjectItem({ + __typename: "PullRequest", + state: "OPEN", + reviewDecision: null, + latestOpinionatedReviews: { + nodes: [{ state: "APPROVED" }], + pageInfo: { hasNextPage: false, endCursor: null }, + }, + })?.prReview, + ).toBe("approved"); + expect(toProjectItem({ __typename: "Issue", reviewDecision: "APPROVED" })?.prReview).toBeNull(); +}); + +it("shows a closed project PR as closed even when it was a draft", () => { + expect(toProjectItem({ __typename: "PullRequest", state: "CLOSED", isDraft: true })?.state).toBe( + "closed", + ); +}); diff --git a/server/projects/single.ts b/server/projects/single.ts index f464c11..6d7e055 100644 --- a/server/projects/single.ts +++ b/server/projects/single.ts @@ -1,3 +1,4 @@ +import { reviewStatus } from "../../shared/pr-review"; import type { z } from "zod"; import type { ProjectItem, loadProject } from "../../shared/board"; import { ghGraphqlRaw, nodesOf } from "../github/graphql"; @@ -17,6 +18,8 @@ interface GhProjectContentNode { url?: unknown; state?: unknown; isDraft?: unknown; + reviewDecision?: unknown; + latestOpinionatedReviews?: unknown; repository?: { nameWithOwner?: unknown }; author?: { login?: unknown }; labels?: { nodes?: unknown }; @@ -68,6 +71,11 @@ const PROJECT_QUERY = `fragment ProjectFields on ProjectV2 { url state isDraft + reviewDecision + latestOpinionatedReviews(first: 100) { + nodes { state } + pageInfo { hasNextPage endCursor } + } repository { nameWithOwner } author { login } labels(first: 20) { nodes { name } } @@ -107,14 +115,14 @@ function projectItemStateOf( node: GhProjectContentNode, ): ProjectItem["state"] { if (kind === "draft") return null; - if (kind === "pull-request" && node.isDraft === true) return "draft"; - if (node.state === "OPEN") return "open"; if (node.state === "MERGED") return "merged"; if (node.state === "CLOSED") return "closed"; + if (kind === "pull-request" && node.isDraft === true) return "draft"; + if (node.state === "OPEN") return "open"; return null; } -function toProjectItem(node: GhProjectContentNode): ProjectItem | null { +export function toProjectItem(node: GhProjectContentNode): ProjectItem | null { const kind: ProjectItem["kind"] | null = node.__typename === "Issue" ? "issue" @@ -136,6 +144,10 @@ function toProjectItem(node: GhProjectContentNode): ProjectItem | null { : null, number: kind !== "draft" && typeof node.number === "number" ? node.number : null, state: projectItemStateOf(kind, node), + prReview: + kind === "pull-request" + ? reviewStatus(node.reviewDecision, node.latestOpinionatedReviews) + : null, author: typeof node.author?.login === "string" ? node.author.login : null, labels: labelNodeNames(node.labels?.nodes), updatedAt: typeof node.updatedAt === "string" ? node.updatedAt : "", @@ -149,7 +161,9 @@ function toProjectItem(node: GhProjectContentNode): ProjectItem | null { * lack of one) each item actually carries; only the *order* falls back to * insertion order, with the no-status group always last regardless. */ -function groupProjectItems(project: GhProjectV2Node): Array<{ name: string; items: ProjectItem[] }> { +function groupProjectItems( + project: GhProjectV2Node, +): Array<{ name: string; items: ProjectItem[] }> { const order = statusOptionNamesOf(project.field); const groups = new Map(); for (const raw of nodesOf(project.items)) { @@ -158,7 +172,8 @@ function groupProjectItems(project: GhProjectV2Node): Array<{ name: string; item if (typeof row.content !== "object" || row.content === null) continue; const item = toProjectItem(row.content); if (item === null) continue; - const statusName = typeof row.fieldValueByName?.name === "string" ? row.fieldValueByName.name : ""; + const statusName = + typeof row.fieldValueByName?.name === "string" ? row.fieldValueByName.name : ""; const bucket = groups.get(statusName); if (bucket === undefined) groups.set(statusName, [item]); else bucket.push(item); @@ -182,7 +197,7 @@ const PROJECT_TTL_MS = 5 * 60_000; type LoadProjectResult = z.input; -const projectCache = new Cache("project"); +const projectCache = new Cache("project-reviews-v1"); export async function loadProjectHandler({ owner, @@ -207,10 +222,14 @@ export async function loadProjectHandler({ if (data === null) { if (needsProjectScope(errors)) throw new Error(PROJECT_SCOPE_MESSAGE); - throw new Error(errors.map((error) => error.message).join(" ") || "GitHub returned no data."); + throw new Error( + errors.map((error) => error.message).join(" ") || "GitHub returned no data.", + ); } - const asUser = data.asUser as { projectV2?: GhProjectV2Node | null } | null; + const asUser = data.asUser as { + projectV2?: GhProjectV2Node | null; + } | null; const asOrg = data.asOrg as { projectV2?: GhProjectV2Node | null } | null; const project = asUser?.projectV2 ?? asOrg?.projectV2; diff --git a/shared/board.ts b/shared/board.ts index 54a63e1..01b7e26 100644 --- a/shared/board.ts +++ b/shared/board.ts @@ -58,6 +58,8 @@ export const RelationSchema = z.enum(RELATION_IDS); export type Relation = z.output; export const BoardItemSchema = z.object({ + prState: z.enum(["OPEN", "CLOSED", "MERGED"]).optional(), + prReview: z.enum(["approved", "changes-requested", "review-required"]).nullable().optional(), id: z.string(), number: z.number().int(), title: z.string(), @@ -200,7 +202,10 @@ export type Board = z.output; */ const GITHUB_LOGIN_PATTERN = /^[a-zA-Z\d](?:[a-zA-Z\d]|-(?=[a-zA-Z\d]))*$/; -const GitHubLoginSchema = z.string().max(39).regex(GITHUB_LOGIN_PATTERN, "not a valid GitHub login"); +const GitHubLoginSchema = z + .string() + .max(39) + .regex(GITHUB_LOGIN_PATTERN, "not a valid GitHub login"); /** * Bounded the same way `limit` is bounded right beside it: each entry becomes @@ -292,6 +297,7 @@ export const ProjectItemSchema = z.object({ repository: z.string().nullable(), number: z.number().int().nullable(), state: z.enum(["open", "draft", "closed", "merged"]).nullable(), + prReview: z.enum(["approved", "changes-requested", "review-required"]).nullable().optional(), author: z.string().nullable(), labels: z.array(z.string()), updatedAt: z.string(), @@ -554,6 +560,8 @@ export type ReviewState = z.output; * moment it opens, and this round trip only adds what the search never fetched. */ export const ItemDetailsSchema = z.object({ + /** Current approvals, excluding dismissed and superseded opinions. */ + approvedBy: z.array(z.string()).optional(), /** * `open` for everything the board lists today; the rest cover an item that * changed on GitHub after the board was fetched, which the panel is the first @@ -700,7 +708,9 @@ export const takeLegacySettings = defineRpc({ z.object({ found: z.literal(true), hiddenRepositories: z.array(z.string()).nullable(), - prompts: PromptSettingsSchema.extend({ byType: PromptSetSchema.partial() }).nullable(), + prompts: PromptSettingsSchema.extend({ + byType: PromptSetSchema.partial(), + }).nullable(), detailWidthFraction: z.number().min(0).max(1).nullable(), }), ]), diff --git a/shared/pr-review.test.ts b/shared/pr-review.test.ts new file mode 100644 index 0000000..487c774 --- /dev/null +++ b/shared/pr-review.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import { approvalLogins, reviewStatus } from "./pr-review"; + +const page = (states: string[], more = false) => ({ + nodes: states.map((state) => ({ state })), + pageInfo: { hasNextPage: more, endCursor: null }, +}); + +describe("current PR review opinions", () => { + it("honors the aggregate decision even when some reviewers approved", () => { + expect(reviewStatus("REVIEW_REQUIRED", page(["APPROVED"]))).toBe("review-required"); + expect(reviewStatus("CHANGES_REQUESTED", page(["APPROVED"]))).toBe("changes-requested"); + expect(reviewStatus("APPROVED", null)).toBe("approved"); + }); + it("falls back to current opinions when review requirements are absent", () => { + expect(reviewStatus(null, page(["APPROVED"]))).toBe("approved"); + expect(reviewStatus(null, page(["APPROVED", "CHANGES_REQUESTED"]))).toBe("changes-requested"); + expect(reviewStatus(null, page(["DISMISSED", "COMMENTED", "PENDING"]))).toBeNull(); + expect(reviewStatus(null, page(["APPROVED"], true))).toBeNull(); + }); + it("attributes only current approvals, deduplicating and ignoring deleted accounts", () => { + expect( + approvalLogins([ + { state: "APPROVED", author: { login: "alice" } }, + { state: "APPROVED", author: { login: "alice" } }, + { state: "DISMISSED", author: { login: "bob" } }, + { state: "CHANGES_REQUESTED", author: { login: "carol" } }, + { state: "COMMENTED", author: { login: "dave" } }, + { state: "APPROVED", author: null }, + null, + ]), + ).toEqual(["alice"]); + }); +}); diff --git a/shared/pr-review.ts b/shared/pr-review.ts new file mode 100644 index 0000000..f245092 --- /dev/null +++ b/shared/pr-review.ts @@ -0,0 +1,38 @@ +import { z } from "zod"; + +export const OpinionatedReviewsSchema = z.object({ + nodes: z.array( + z + .object({ + state: z.string(), + author: z.object({ login: z.string() }).nullable().optional(), + }) + .nullable(), + ), + pageInfo: z.object({ hasNextPage: z.boolean(), endCursor: z.string().nullable() }), +}); + +export function reviewStatus(decision: unknown, reviews: unknown) { + if (decision === "APPROVED") return "approved"; + if (decision === "CHANGES_REQUESTED") return "changes-requested"; + if (decision === "REVIEW_REQUIRED") return "review-required"; + // Repositories without required reviews can have approvals but no reviewDecision. + const parsed = OpinionatedReviewsSchema.safeParse(reviews); + if (!parsed.success) return null; + if (parsed.data.nodes.some((review) => review?.state === "CHANGES_REQUESTED")) + return "changes-requested"; + if (parsed.data.pageInfo.hasNextPage) return null; + return parsed.data.nodes.some((review) => review?.state === "APPROVED") ? "approved" : null; +} + +export function approvalLogins( + reviews: z.output["nodes"], +): string[] { + return [ + ...new Set( + reviews.flatMap((review) => + review?.state === "APPROVED" && review.author != null ? [review.author.login] : [], + ), + ), + ].sort((a, b) => a.localeCompare(b)); +}