From 668cab60223e2c9b2de5ac5dc6289c925f25fdb0 Mon Sep 17 00:00:00 2001 From: Jiayuan Zhang Date: Sat, 16 May 2026 21:26:30 +0200 Subject: [PATCH] feat(github): mirror PR CI checks and merge conflict status (MUL-2228) (#2632) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(github): mirror PR CI checks and merge conflict status (MUL-2228) Surface "checks passed/failed" and "conflicts/no conflicts" badges under each linked PR on the issue page so users can judge readiness without flipping over to GitHub. CI state is fed by check_suite webhooks (GitHub Actions + apps using the Checks API; legacy status events are out of scope for MVP); conflicts are read from pull_request.mergeable_state. Data model: * github_pull_request: add head_sha + mergeable_state * github_pull_request_check_suite: per-suite rows keyed by (pr_id, suite_id) * Aggregation done at query time, filtering by current head_sha so late-arriving suites for a stale head can't contaminate the new head's pending view; per-app latest suite chosen first so a single app firing multiple suites isn't counted N times. Webhook hardening: * synchronize/opened/reopened/edited(base) explicitly clear mergeable_state * single-row ordering protection on the check_suite upsert prevents a late-delivered older event from overwriting a newer one * check_suite.pull_requests is iterated; unknown PRs are logged and dropped UI: * PR row shows Checks + Conflicts badges; opaque mergeable values (blocked/behind/unstable/...) render as no badge, not as conflicts. * Terminal PR states (merged/closed) suppress the status row entirely. Tests: * Pure unit coverage for derivePRMergeableState + aggregateChecksConclusion * Webhook integration tests: multi-app aggregation, old-head ignore, late-older-event ignore, synchronize clears mergeable_state * Vitest coverage for pull-request-list badge rendering across CI/conflict combinations and the legacy (null) fallback. Co-authored-by: multica-agent * fix(github): scope check_suite PR lookup; preserve mergeable on metadata Addresses code review on PR #2632. 1. check_suite handler now resolves the PR through the workspace-scoped GetGitHubPullRequest query instead of GetGitHubPullRequestByRepoNumber. The (workspace_id, repo_owner, repo_name, pr_number) tuple is the real uniqueness key, so a bare (owner, repo, number) lookup could return a stale row from another workspace and either land the suite on the wrong PR or skip the right one when the installation ids drifted. The old unscoped query is removed. 2. derivePRMergeableState now returns (value, clear) and the upsert SQL distinguishes three cases: state-changing actions clear the column to NULL, non-empty payloads write the value, and metadata events with an empty payload preserve the existing column. Previously every empty payload became NULL, so a labeled/assigned event silently wiped a known clean/dirty verdict in violation of the RFC's "metadata empty payload preserves" rule. 3. ListPullRequestsByIssue narrows to the issue's PR ids before running the per-app check_suite aggregation, avoiding a full-table scan over github_pull_request_check_suite when only a handful of rows belong to the requested issue. New helper test covers labeled+empty preserves; new integration test verifies a metadata event after a known mergeable_state keeps the value. Co-authored-by: multica-agent * feat(github): PR card layout v3 increment — stats + segmented progress bar Replaces the row + badge layout under "Pull requests" on the issue detail sidebar with a card that mirrors the GitHub PR summary look: title, author/avatar, +N −M · K files diff stats, segmented progress bar (failed → pending → passed, failure leftmost), and a one-line status caption following an explicit priority pass-through. Backend - Migration 092: github_pull_request adds additions / deletions / changed_files (INT NOT NULL DEFAULT 0). Zero defaults are what the new frontend treats as "legacy backend — hide the stats row" so old PR rows that pre-date this migration don't render "+0 −0 · 0 files". - pull_request webhook handler reads stats off the top-level payload. - ListPullRequestsByIssue now surfaces per-suite counts (checks_passed / failed / pending) alongside the existing aggregate conclusion, so the segmented bar reuses the already-computed counts with no new aggregation. Frontend (packages) - core/github/pull-request-status.{ts,test.ts}: pure-function module for the status-kind priority table and the segment derivation; 15 cases covered, includes the "all-zero → hide stats" guard. - views/issues/components/pull-request-list.tsx: PullRequestCard plus a compact-row fallback used when count > 4 (first 3 as cards, the remainder collapsed behind a Show more toggle). - i18n: new `pull_request_card_*` keys in en + zh-Hans. Tests - 12 component tests covering each rule of the priority table, the legacy-zero stats fallback, and the collapse threshold. - Reuse of the v3 webhook handler tests confirmed. Verification - pnpm typecheck + pnpm test green (60 test files, 536 tests). - go build ./... + go vet ./... clean. - 6 demo issues (DEV-2..DEV-7) screenshotted via Playwright; see the PR comments for the visual check matrix. Co-authored-by: multica-agent * fix(views): collapse PR cards at N>=4, not N>4 The card-vs-collapse threshold used `>` so 4 PRs slipped past it and all rendered as full cards, contrary to RFC v3 (N >= 4 collapses to 3 cards + compact tail). Switch to `>=` and update the threshold- boundary test to expect "Show 1 more". Co-authored-by: multica-agent * fix(views): align PR sidebar rows with existing list style Co-authored-by: multica-agent * fix(views): hide terminal PR status badges Co-authored-by: multica-agent --------- Co-authored-by: multica-agent --- packages/core/github/index.ts | 1 + .../core/github/pull-request-status.test.ts | 146 +++++++ packages/core/github/pull-request-status.ts | 101 +++++ packages/core/types/github.ts | 25 ++ packages/core/types/index.ts | 2 + .../components/pull-request-list.test.tsx | 211 ++++++++++ .../issues/components/pull-request-list.tsx | 292 +++++++++++++- packages/views/locales/en/issues.json | 18 + packages/views/locales/zh-Hans/issues.json | 18 + scripts/screenshot-pr-cards.mjs | 91 +++++ server/internal/handler/github.go | 330 ++++++++++++++-- server/internal/handler/github_test.go | 368 ++++++++++++++++++ server/migrations/091_pr_ci_conflict.down.sql | 6 + server/migrations/091_pr_ci_conflict.up.sql | 22 ++ server/migrations/092_pr_stats.down.sql | 4 + server/migrations/092_pr_stats.up.sql | 12 + server/pkg/db/generated/github.sql.go | 229 +++++++++-- server/pkg/db/generated/models.go | 15 + server/pkg/db/queries/github.sql | 103 ++++- 19 files changed, 1919 insertions(+), 75 deletions(-) create mode 100644 packages/core/github/pull-request-status.test.ts create mode 100644 packages/core/github/pull-request-status.ts create mode 100644 packages/views/issues/components/pull-request-list.test.tsx create mode 100644 scripts/screenshot-pr-cards.mjs create mode 100644 server/migrations/091_pr_ci_conflict.down.sql create mode 100644 server/migrations/091_pr_ci_conflict.up.sql create mode 100644 server/migrations/092_pr_stats.down.sql create mode 100644 server/migrations/092_pr_stats.up.sql diff --git a/packages/core/github/index.ts b/packages/core/github/index.ts index b69c25120..3dd4aaff0 100644 --- a/packages/core/github/index.ts +++ b/packages/core/github/index.ts @@ -1 +1,2 @@ export * from "./queries"; +export * from "./pull-request-status"; diff --git a/packages/core/github/pull-request-status.test.ts b/packages/core/github/pull-request-status.test.ts new file mode 100644 index 000000000..b557fd43f --- /dev/null +++ b/packages/core/github/pull-request-status.test.ts @@ -0,0 +1,146 @@ +import { describe, expect, it } from "vitest"; +import { + derivePullRequestStatusKind, + derivePullRequestProgressSegments, + shouldShowPullRequestStats, + type PullRequestStatusInput, +} from "./pull-request-status"; + +const base: PullRequestStatusInput = { state: "open" }; + +describe("derivePullRequestStatusKind", () => { + it("closed beats every other signal", () => { + expect( + derivePullRequestStatusKind({ + state: "closed", + mergeable_state: "dirty", + checks_failed: 99, + checks_pending: 99, + checks_passed: 99, + }), + ).toBe("closed"); + }); + + it("merged beats every other signal except closed", () => { + expect( + derivePullRequestStatusKind({ + state: "merged", + mergeable_state: "dirty", + checks_failed: 5, + }), + ).toBe("merged"); + }); + + it("dirty conflicts wins over check signals", () => { + expect( + derivePullRequestStatusKind({ + ...base, + mergeable_state: "dirty", + checks_passed: 3, + }), + ).toBe("conflicts"); + }); + + it("any failed check beats pending and passed", () => { + expect( + derivePullRequestStatusKind({ + ...base, + checks_failed: 1, + checks_pending: 3, + checks_passed: 5, + }), + ).toBe("checks_failed"); + }); + + it("pending beats passed when no failure", () => { + expect( + derivePullRequestStatusKind({ + ...base, + checks_pending: 1, + checks_passed: 5, + }), + ).toBe("checks_pending"); + }); + + it("all-passed is checks_passed regardless of mergeable=clean", () => { + expect( + derivePullRequestStatusKind({ + ...base, + mergeable_state: "clean", + checks_passed: 5, + }), + ).toBe("checks_passed"); + }); + + it("clean + no suites is ready-to-merge", () => { + expect( + derivePullRequestStatusKind({ ...base, mergeable_state: "clean" }), + ).toBe("ready"); + }); + + it("opaque mergeable values render as unknown", () => { + for (const m of ["blocked", "behind", "unstable", "has_hooks", "unknown", null, undefined]) { + expect(derivePullRequestStatusKind({ ...base, mergeable_state: m })).toBe("unknown"); + } + }); +}); + +describe("derivePullRequestProgressSegments", () => { + it("returns null for terminal PRs (merged / closed)", () => { + expect(derivePullRequestProgressSegments({ state: "merged", checks_passed: 5 })).toBeNull(); + expect(derivePullRequestProgressSegments({ state: "closed", checks_failed: 3 })).toBeNull(); + }); + + it("returns null when no suite has been observed", () => { + expect(derivePullRequestProgressSegments({ ...base })).toBeNull(); + expect( + derivePullRequestProgressSegments({ ...base, checks_failed: 0, checks_pending: 0, checks_passed: 0 }), + ).toBeNull(); + }); + + it("orders segments failed → pending → passed (failure leftmost)", () => { + const segs = derivePullRequestProgressSegments({ + ...base, + checks_failed: 1, + checks_pending: 2, + checks_passed: 3, + }); + expect(segs).not.toBeNull(); + expect(segs!.map((s) => s.kind)).toEqual(["failed", "pending", "passed"]); + }); + + it("emits a zero-width segment-free output (no entry with ratio 0)", () => { + const segs = derivePullRequestProgressSegments({ + ...base, + checks_failed: 0, + checks_pending: 0, + checks_passed: 4, + }); + expect(segs).toEqual([{ kind: "passed", ratio: 1 }]); + }); + + it("ratios sum to ~1 across segments", () => { + const segs = derivePullRequestProgressSegments({ + ...base, + checks_failed: 1, + checks_pending: 1, + checks_passed: 2, + })!; + const total = segs.reduce((acc, s) => acc + s.ratio, 0); + expect(total).toBeCloseTo(1, 6); + }); +}); + +describe("shouldShowPullRequestStats", () => { + it("hides when every field is 0 or missing (legacy backend)", () => { + expect(shouldShowPullRequestStats({})).toBe(false); + expect(shouldShowPullRequestStats({ additions: 0, deletions: 0, changed_files: 0 })).toBe(false); + }); + + it("shows when at least one number is non-zero", () => { + expect(shouldShowPullRequestStats({ additions: 1 })).toBe(true); + expect(shouldShowPullRequestStats({ deletions: 1 })).toBe(true); + expect(shouldShowPullRequestStats({ changed_files: 1 })).toBe(true); + expect(shouldShowPullRequestStats({ additions: 437, deletions: 6, changed_files: 6 })).toBe(true); + }); +}); diff --git a/packages/core/github/pull-request-status.ts b/packages/core/github/pull-request-status.ts new file mode 100644 index 000000000..2b80b436c --- /dev/null +++ b/packages/core/github/pull-request-status.ts @@ -0,0 +1,101 @@ +import type { GitHubPullRequest } from "../types"; + +// Status kinds rendered in the PR sidebar row's detail line. Order in the +// pass-through table matters — the first matching rule wins. The order is +// chosen so terminal PR states (closed / merged) short-circuit before any +// transient CI/conflict signal, since those signals are no longer actionable +// on a terminal PR. +// +// Priority (high → low): +// 1. closed (not merged) → status_closed +// 2. merged → status_merged +// 3. mergeable_state = "dirty" → status_conflicts +// 4. any failed suite → status_checks_failed +// 5. any pending suite → status_checks_pending +// 6. any passed suite → status_checks_passed +// 7. no suite + mergeable=clean → status_ready +// 8. otherwise → status_unknown +// +// Note: this table is the single source of truth for the sidebar PR row. The +// older row-with-badges implementation used a separate "hide status row for +// terminal PRs" branch — the current row renders +// with status_closed / status_merged text, never falling through to a +// conflicts / checks line on a terminal PR. Keep this priority order in sync +// with the i18n keys `pull_request_card_status_*` and with the progress-strip +// derivation in `derivePullRequestProgressSegments` (terminal kinds get a +// solid bar; the rest map onto the per-suite counts). +export type PullRequestStatusKind = + | "closed" + | "merged" + | "conflicts" + | "checks_failed" + | "checks_pending" + | "checks_passed" + | "ready" + | "unknown"; + +export interface PullRequestStatusInput { + state: GitHubPullRequest["state"]; + mergeable_state?: string | null; + checks_failed?: number; + checks_pending?: number; + checks_passed?: number; +} + +export function derivePullRequestStatusKind(input: PullRequestStatusInput): PullRequestStatusKind { + if (input.state === "closed") return "closed"; + if (input.state === "merged") return "merged"; + if (input.mergeable_state === "dirty") return "conflicts"; + if ((input.checks_failed ?? 0) > 0) return "checks_failed"; + if ((input.checks_pending ?? 0) > 0) return "checks_pending"; + if ((input.checks_passed ?? 0) > 0) return "checks_passed"; + if (input.mergeable_state === "clean") return "ready"; + return "unknown"; +} + +export interface PullRequestProgressSegment { + kind: "failed" | "pending" | "passed"; + ratio: number; +} + +// Segmented progress bar input. Returns null when: +// - the PR is terminal (closed/merged) — the card paints a solid bar +// in a state-specific color, no segmentation needed; +// - no check_suite has been observed (total === 0) — the card hides +// the bar entirely. +// Otherwise emits the segments left-to-right: failed → pending → passed. +// "Failure first" is intentional: problems should be visible before signal +// that everything is fine. +export function derivePullRequestProgressSegments( + input: PullRequestStatusInput, +): PullRequestProgressSegment[] | null { + if (input.state === "closed" || input.state === "merged") return null; + const failed = input.checks_failed ?? 0; + const pending = input.checks_pending ?? 0; + const passed = input.checks_passed ?? 0; + const total = failed + pending + passed; + if (total === 0) return null; + const segments: PullRequestProgressSegment[] = []; + if (failed > 0) segments.push({ kind: "failed", ratio: failed / total }); + if (pending > 0) segments.push({ kind: "pending", ratio: pending / total }); + if (passed > 0) segments.push({ kind: "passed", ratio: passed / total }); + return segments; +} + +export interface PullRequestStatsInput { + additions?: number; + deletions?: number; + changed_files?: number; +} + +// shouldShowPullRequestStats encodes the "old backend → new frontend" guard: +// when the backend that served this PR row doesn't know about the stats +// columns yet, every numeric field defaults to 0. Rendering "+0 −0 · 0 files" +// in that case would be a lie (the PR almost certainly has real changes), +// so we hide the entire stats row until at least one signal is non-zero. +export function shouldShowPullRequestStats(input: PullRequestStatsInput): boolean { + const a = input.additions ?? 0; + const d = input.deletions ?? 0; + const f = input.changed_files ?? 0; + return a + d + f > 0; +} diff --git a/packages/core/types/github.ts b/packages/core/types/github.ts index 0cf2ebdec..dafbca2fe 100644 --- a/packages/core/types/github.ts +++ b/packages/core/types/github.ts @@ -1,5 +1,16 @@ export type GitHubPullRequestState = "open" | "closed" | "merged" | "draft"; +/** Aggregated CI status for a PR's current head SHA, computed server-side from + * the latest check_suite per app. `null` when no completed suite has been seen + * yet (e.g. PR just opened, or repository has no CI configured). */ +export type GitHubPullRequestChecksConclusion = "passed" | "failed" | "pending"; + +/** Raw mirror of GitHub's `mergeable_state`. The UI only surfaces `clean` and + * `dirty`; the other values (`blocked`, `behind`, `unstable`, `unknown`, + * `has_hooks`, `draft`) round-trip but render as unknown to avoid asserting + * "conflicts" for blocking reasons that aren't actual conflicts. */ +export type GitHubMergeableState = string; + export interface GitHubInstallation { id: string; workspace_id: string; @@ -26,6 +37,20 @@ export interface GitHubPullRequest { closed_at: string | null; pr_created_at: string; pr_updated_at: string; + /** Optional; older backends omit this field. */ + mergeable_state?: GitHubMergeableState | null; + /** Optional; older backends omit this field. */ + checks_conclusion?: GitHubPullRequestChecksConclusion | null; + /** Per-suite counts that feed the segmented progress bar. Older backends + * omit these; treat absence as 0 (the card renders only when sum > 0). */ + checks_passed?: number; + checks_failed?: number; + checks_pending?: number; + /** Diff stats from GitHub's `pull_request` payload. Older backends omit + * these fields; we treat 0/0/0 as "unknown" and hide the stats row. */ + additions?: number; + deletions?: number; + changed_files?: number; } export interface ListGitHubInstallationsResponse { diff --git a/packages/core/types/index.ts b/packages/core/types/index.ts index 5b4c47e10..75e997402 100644 --- a/packages/core/types/index.ts +++ b/packages/core/types/index.ts @@ -79,7 +79,9 @@ export type { export type { PinnedItem, PinnedItemType, CreatePinRequest, ReorderPinsRequest } from "./pin"; export type { GitHubInstallation, + GitHubMergeableState, GitHubPullRequest, + GitHubPullRequestChecksConclusion, GitHubPullRequestState, ListGitHubInstallationsResponse, GitHubConnectResponse, diff --git a/packages/views/issues/components/pull-request-list.test.tsx b/packages/views/issues/components/pull-request-list.test.tsx new file mode 100644 index 000000000..f1719d3fe --- /dev/null +++ b/packages/views/issues/components/pull-request-list.test.tsx @@ -0,0 +1,211 @@ +import { describe, expect, it, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { I18nProvider } from "@multica/core/i18n/react"; +import type { GitHubPullRequest } from "@multica/core/types"; +import enCommon from "../../locales/en/common.json"; +import enIssues from "../../locales/en/issues.json"; + +const TEST_RESOURCES = { en: { common: enCommon, issues: enIssues } }; + +vi.mock("@multica/core/github/queries", async () => { + const actual = await vi.importActual( + "@multica/core/github/queries", + ); + return { + ...actual, + issuePullRequestsOptions: (issueId: string) => ({ + queryKey: ["github", "pull-requests", issueId], + queryFn: async () => ({ pull_requests: mockPRs }), + enabled: !!issueId, + }), + }; +}); + +import { PullRequestList } from "./pull-request-list"; + +let mockPRs: GitHubPullRequest[] = []; + +function makePR(overrides: Partial = {}): GitHubPullRequest { + return { + id: "pr-1", + workspace_id: "ws-1", + repo_owner: "acme", + repo_name: "widget", + number: 1, + title: "Test PR", + state: "open", + html_url: "https://example.test/pr/1", + branch: "feat/x", + author_login: "octocat", + author_avatar_url: null, + merged_at: null, + closed_at: null, + pr_created_at: "2026-01-01T00:00:00Z", + pr_updated_at: "2026-01-01T00:00:00Z", + mergeable_state: null, + checks_conclusion: null, + checks_passed: 0, + checks_failed: 0, + checks_pending: 0, + additions: 0, + deletions: 0, + changed_files: 0, + ...overrides, + }; +} + +function renderList() { + const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + + + , + ); +} + +async function waitForRender() { + return screen.findAllByRole("link"); +} + +describe("PullRequestList sidebar rows", () => { + it("uses the sidebar list-row surface instead of a card surface", async () => { + mockPRs = [makePR({ title: "Visual row" })]; + renderList(); + await waitForRender(); + const row = screen.getByTestId("pull-request-row"); + expect(row).toHaveClass("rounded-md", "-mx-2", "hover:bg-accent/50"); + expect(row).not.toHaveClass("rounded-lg", "border", "bg-card"); + }); + + it("renders All-checks-passed status when only passed counts are non-zero", async () => { + mockPRs = [makePR({ checks_passed: 3 })]; + renderList(); + await waitForRender(); + expect(screen.getByText("All checks passed")).toBeInTheDocument(); + }); + + it("renders Some-checks-failed when any failed count is non-zero", async () => { + mockPRs = [makePR({ checks_failed: 1, checks_passed: 5 })]; + renderList(); + await waitForRender(); + expect(screen.getByText("Some checks failed")).toBeInTheDocument(); + }); + + it("renders pending status when only pending suites remain", async () => { + mockPRs = [makePR({ checks_pending: 2, checks_passed: 1 })]; + renderList(); + await waitForRender(); + expect(screen.getByText("Some checks haven't completed yet")).toBeInTheDocument(); + }); + + it("renders conflicts status when mergeable_state=dirty", async () => { + mockPRs = [makePR({ mergeable_state: "dirty" })]; + renderList(); + await waitForRender(); + expect(screen.getByText("Has merge conflicts")).toBeInTheDocument(); + }); + + it("renders Ready-to-merge when mergeable=clean and no suites observed", async () => { + mockPRs = [makePR({ mergeable_state: "clean" })]; + renderList(); + await waitForRender(); + expect(screen.getByText("Ready to merge")).toBeInTheDocument(); + }); + + it("renders Merged status for merged PRs, suppressing conflict/check text", async () => { + mockPRs = [ + makePR({ + state: "merged", + mergeable_state: "dirty", + checks_conclusion: "failed", + checks_failed: 5, + }), + ]; + renderList(); + await waitForRender(); + expect(screen.getByText("Merged")).toBeInTheDocument(); + expect(screen.queryByText("Has merge conflicts")).not.toBeInTheDocument(); + expect(screen.queryByText("Some checks failed")).not.toBeInTheDocument(); + expect(screen.queryByText("Conflicts")).not.toBeInTheDocument(); + expect(screen.queryByText("Checks failed")).not.toBeInTheDocument(); + }); + + it("renders Closed-without-merging status for closed PRs, suppressing conflict/check badges", async () => { + mockPRs = [ + makePR({ + state: "closed", + mergeable_state: "clean", + checks_conclusion: "passed", + checks_passed: 3, + }), + ]; + renderList(); + await waitForRender(); + expect(screen.getByText("Closed without merging")).toBeInTheDocument(); + expect(screen.queryByText("Ready to merge")).not.toBeInTheDocument(); + expect(screen.queryByText("All checks passed")).not.toBeInTheDocument(); + expect(screen.queryByText("No conflicts")).not.toBeInTheDocument(); + expect(screen.queryByText("Checks passed")).not.toBeInTheDocument(); + }); + + it("hides stats row when all stats are 0 (legacy backend)", async () => { + mockPRs = [makePR()]; + renderList(); + await waitForRender(); + expect(screen.queryByText(/files?$/)).not.toBeInTheDocument(); + expect(screen.queryByText(/^\+0/)).not.toBeInTheDocument(); + }); + + it("shows stats row with additions / deletions / file count when present", async () => { + mockPRs = [makePR({ additions: 437, deletions: 6, changed_files: 6 })]; + renderList(); + await waitForRender(); + expect(screen.getByText("+437")).toBeInTheDocument(); + expect(screen.getByText("−6")).toBeInTheDocument(); + expect(screen.getByText("6 files")).toBeInTheDocument(); + }); + + it("uses singular file copy when changed_files=1", async () => { + mockPRs = [makePR({ additions: 1, changed_files: 1 })]; + renderList(); + await waitForRender(); + expect(screen.getByText("1 file")).toBeInTheDocument(); + }); + + it("collapses extra PR rows past the visible limit behind Show more toggle", async () => { + mockPRs = [ + makePR({ id: "a", number: 1, title: "PR-A" }), + makePR({ id: "b", number: 2, title: "PR-B" }), + makePR({ id: "c", number: 3, title: "PR-C" }), + makePR({ id: "d", number: 4, title: "PR-D" }), + makePR({ id: "e", number: 5, title: "PR-E" }), + ]; + renderList(); + await waitForRender(); + expect(screen.getByText("PR-A")).toBeInTheDocument(); + expect(screen.getByText("PR-B")).toBeInTheDocument(); + expect(screen.getByText("PR-C")).toBeInTheDocument(); + expect(screen.queryByText("PR-D")).not.toBeInTheDocument(); + expect(screen.queryByText("PR-E")).not.toBeInTheDocument(); + expect(screen.getByText("Show 2 more")).toBeInTheDocument(); + }); + + it("collapses to 3 rows + hidden tail when count == threshold", async () => { + mockPRs = [ + makePR({ id: "a", number: 1, title: "PR-A" }), + makePR({ id: "b", number: 2, title: "PR-B" }), + makePR({ id: "c", number: 3, title: "PR-C" }), + makePR({ id: "d", number: 4, title: "PR-D" }), + ]; + renderList(); + await waitForRender(); + expect(screen.getByText("PR-A")).toBeInTheDocument(); + expect(screen.getByText("PR-B")).toBeInTheDocument(); + expect(screen.getByText("PR-C")).toBeInTheDocument(); + expect(screen.queryByText("PR-D")).not.toBeInTheDocument(); + expect(screen.getByText("Show 1 more")).toBeInTheDocument(); + }); +}); diff --git a/packages/views/issues/components/pull-request-list.tsx b/packages/views/issues/components/pull-request-list.tsx index c9e444acb..e54be4fae 100644 --- a/packages/views/issues/components/pull-request-list.tsx +++ b/packages/views/issues/components/pull-request-list.tsx @@ -1,18 +1,40 @@ "use client"; +import { useState } from "react"; import { useQuery } from "@tanstack/react-query"; import { + CheckCircle2, + CircleDashed, + GitMerge, GitPullRequest, GitPullRequestArrow, GitPullRequestClosed, - GitMerge, GitPullRequestDraft, + TriangleAlert, + XCircle, } from "lucide-react"; -import { issuePullRequestsOptions } from "@multica/core/github/queries"; -import type { GitHubPullRequest, GitHubPullRequestState } from "@multica/core/types"; +import { + issuePullRequestsOptions, + derivePullRequestStatusKind, + derivePullRequestProgressSegments, + shouldShowPullRequestStats, + type PullRequestStatusKind, + type PullRequestProgressSegment, +} from "@multica/core/github"; +import type { + GitHubPullRequest, + GitHubPullRequestChecksConclusion, + GitHubPullRequestState, +} from "@multica/core/types"; import { cn } from "@multica/ui/lib/utils"; import { useT } from "../../i18n"; +type IssuesT = ReturnType>["t"]; + +// Keep the existing sidebar density: show the first 3 PR rows inline, then +// collapse the rest once the section reaches 4 rows. +const PR_LIMIT_BEFORE_COLLAPSE = 4; + const STATE_ICON: Record< GitHubPullRequestState, { icon: React.ComponentType<{ className?: string }>; className: string } @@ -23,8 +45,18 @@ const STATE_ICON: Record< closed: { icon: GitPullRequestClosed, className: "text-rose-600 dark:text-rose-400" }, }; +const CHECKS_ICON: Record< + GitHubPullRequestChecksConclusion, + { icon: React.ComponentType<{ className?: string }>; className: string } +> = { + passed: { icon: CheckCircle2, className: "text-emerald-600 dark:text-emerald-400" }, + failed: { icon: XCircle, className: "text-rose-600 dark:text-rose-400" }, + pending: { icon: CircleDashed, className: "text-amber-600 dark:text-amber-400" }, +}; + export function PullRequestList({ issueId }: { issueId: string }) { const { t } = useT("issues"); + const [expanded, setExpanded] = useState(false); const { data, isLoading } = useQuery(issuePullRequestsOptions(issueId)); const prs = data?.pull_requests ?? []; @@ -39,11 +71,35 @@ export function PullRequestList({ issueId }: { issueId: string }) { ); } + // Render rule: + // - < PR_LIMIT_BEFORE_COLLAPSE: every PR row is visible. + // - >= PR_LIMIT_BEFORE_COLLAPSE: first (LIMIT - 1) rows are visible and + // the remainder sits behind a toggle. + const useCollapse = prs.length >= PR_LIMIT_BEFORE_COLLAPSE; + const expandedHead = useCollapse ? prs.slice(0, PR_LIMIT_BEFORE_COLLAPSE - 1) : prs; + const collapsedTail = useCollapse ? prs.slice(PR_LIMIT_BEFORE_COLLAPSE - 1) : []; + return (
- {prs.map((pr) => ( + {expandedHead.map((pr) => ( ))} + {useCollapse ? ( +
+ {expanded + ? collapsedTail.map((pr) => ) + : null} + +
+ ) : null}
); } @@ -51,32 +107,230 @@ export function PullRequestList({ issueId }: { issueId: string }) { function PullRequestRow({ pr }: { pr: GitHubPullRequest }) { const { t } = useT("issues"); const cfg = STATE_ICON[pr.state] ?? { icon: GitPullRequest, className: "" }; - const Icon = cfg.icon; - const label = - pr.state === "open" - ? t(($) => $.detail.pull_request_state_open) - : pr.state === "draft" - ? t(($) => $.detail.pull_request_state_draft) - : pr.state === "merged" - ? t(($) => $.detail.pull_request_state_merged) - : pr.state === "closed" - ? t(($) => $.detail.pull_request_state_closed) - : pr.state; + const StateIcon = cfg.icon; + const kind = derivePullRequestStatusKind({ + state: pr.state, + mergeable_state: pr.mergeable_state, + checks_failed: pr.checks_failed, + checks_pending: pr.checks_pending, + checks_passed: pr.checks_passed, + }); + const segments = derivePullRequestProgressSegments({ + state: pr.state, + checks_failed: pr.checks_failed, + checks_pending: pr.checks_pending, + checks_passed: pr.checks_passed, + }); + const showStats = shouldShowPullRequestStats({ + additions: pr.additions, + deletions: pr.deletions, + changed_files: pr.changed_files, + }); + const statusText = useStatusText(kind); + const draftPrefix = pr.state === "draft"; + const stateLabel = getStateLabel(pr.state, t); + return ( - +
-

{pr.title}

+

+ {pr.title} +

- {pr.repo_owner}/{pr.repo_name}#{pr.number} · {label} + {pr.repo_owner}/{pr.repo_name}#{pr.number} · {stateLabel} {pr.author_login ? ` · @${pr.author_login}` : null}

+ $.detail.pull_request_card_draft_prefix, { status: statusText }) + : statusText + } + statusKind={kind} + />
); } + +function PullRequestRowDetails({ + pr, + segments, + showStats, + statusText, + statusKind, +}: { + pr: GitHubPullRequest; + segments: PullRequestProgressSegment[] | null; + showStats: boolean; + statusText: string; + statusKind: PullRequestStatusKind; +}) { + const { t } = useT("issues"); + const checksBadge = getChecksBadge(pr, t); + const conflictsBadge = getConflictsBadge(pr, t); + const isTerminal = statusKind === "closed" || statusKind === "merged"; + const showChecksBadge = + !isTerminal && + !!checksBadge && + statusKind !== "checks_failed" && + statusKind !== "checks_pending" && + statusKind !== "checks_passed"; + const showConflictsBadge = + !isTerminal && !!conflictsBadge && statusKind !== "conflicts" && statusKind !== "ready"; + + return ( +
+ {showStats ? : null} + + {statusText} + {showChecksBadge ? : null} + {showConflictsBadge ? : null} +
+ ); +} + +function PullRequestStats({ pr }: { pr: GitHubPullRequest }) { + const { t } = useT("issues"); + return ( + + +{pr.additions ?? 0} + −{pr.deletions ?? 0} + + + {t(($) => $.detail.pull_request_card_files_count, { + count: pr.changed_files ?? 0, + })} + + + ); +} + +function PullRequestProgressStrip({ + segments, +}: { + segments: PullRequestProgressSegment[] | null; +}) { + if (!segments) return null; + return ( +