diff --git a/apps/web/src/components/pull-requests/PullRequestFilters.tsx b/apps/web/src/components/pull-requests/PullRequestFilters.tsx index 2ed8b879..2ebe4d26 100644 --- a/apps/web/src/components/pull-requests/PullRequestFilters.tsx +++ b/apps/web/src/components/pull-requests/PullRequestFilters.tsx @@ -29,6 +29,7 @@ import { PullRequestActorAvatar } from "./pullRequestPresentation"; import { hasPullRequestLabel, PULL_REQUEST_INVOLVEMENT_WORDS, + PULL_REQUEST_INVOLVEMENTS, PULL_REQUEST_SORT_LABELS, pullRequestAuthorFacets, pullRequestFilterChips, @@ -52,9 +53,10 @@ interface FilterOption { readonly label: string; } -const INVOLVEMENT_OPTIONS: readonly FilterOption[] = ( - ["all", "needs-you", "yours", "others"] as const -).map((value) => ({ value, label: PULL_REQUEST_INVOLVEMENT_WORDS[value] })); +const INVOLVEMENT_OPTIONS: readonly FilterOption[] = [ + "all" as const, + ...PULL_REQUEST_INVOLVEMENTS, +].map((value) => ({ value, label: PULL_REQUEST_INVOLVEMENT_WORDS[value] })); const DRAFT_OPTIONS: readonly FilterOption[] = [ { value: "any", label: "Any" }, diff --git a/apps/web/src/components/pull-requests/PullRequestsView.browser.tsx b/apps/web/src/components/pull-requests/PullRequestsView.browser.tsx index 3fee047c..6d174543 100644 --- a/apps/web/src/components/pull-requests/PullRequestsView.browser.tsx +++ b/apps/web/src/components/pull-requests/PullRequestsView.browser.tsx @@ -329,13 +329,20 @@ describe("PullRequestsView", () => { makeEntry({ number: 2, title: "Mine and quiet", viewerIsAuthor: true }), makeEntry({ number: 3, title: "Mine and approved", viewerIsAuthor: true }), makeEntry({ number: 4, title: "Someone else's work" }), + makeEntry({ + number: 5, + title: "Sent upstream", + viewerIsAuthor: true, + viewerCanWrite: false, + }), ], errors: [], }); await expect.element(page.getByText("Needs you · 1")).toBeVisible(); await expect.element(page.getByText("Yours · 2")).toBeVisible(); - await expect.element(page.getByText("Others · 1")).toBeVisible(); + await expect.element(page.getByText("Incoming · 1")).toBeVisible(); + await expect.element(page.getByText("Contributions · 1")).toBeVisible(); await rendered.cleanup(); }); diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.test.ts b/apps/web/src/components/pull-requests/pullRequests.logic.test.ts index cf214b9d..fe78720d 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.test.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.test.ts @@ -243,15 +243,15 @@ describe("groupPullRequests", () => { viewerIsAuthor: true, updatedAt: "2026-09-01T13:00:00.000Z", }); - const others = entry({ number: 4 }); + const incoming = entry({ number: 4 }); const groups = groupPullRequests({ - entries: [others, yoursOlder, needsYou, yoursNewer], + entries: [incoming, yoursOlder, needsYou, yoursNewer], viewer: "ada", state: "open", }); - expect(groups.map((group) => group.label)).toEqual(["Needs you", "Yours", "Others"]); + expect(groups.map((group) => group.label)).toEqual(["Needs you", "Yours", "Incoming"]); expect(groups[0]?.entries.map((row) => row.number)).toEqual([1]); expect(groups[1]?.entries.map((row) => row.number)).toEqual([3, 2]); expect(groups[2]?.entries.map((row) => row.number)).toEqual([4]); @@ -263,7 +263,7 @@ describe("groupPullRequests", () => { viewer: "ada", state: "open", }); - expect(groups.map((group) => group.label)).toEqual(["Others"]); + expect(groups.map((group) => group.label)).toEqual(["Incoming"]); }); it("falls back to one unlabelled list without a viewer or outside the open tab", () => { @@ -295,7 +295,7 @@ describe("groupPullRequests", () => { expect(groups[0]?.label).toBeNull(); }); - it("files work on a repository the viewer cannot push to under Yours", () => { + it("ranks what the viewer can land above what is out of their hands", () => { const upstream = entry({ number: 1, viewerIsAuthor: true, @@ -303,15 +303,26 @@ describe("groupPullRequests", () => { reviewDecision: "approved", }); const asked = entry({ number: 2, viewerCanWrite: false, viewerReviewRequested: true }); + const followed = entry({ number: 3, viewerCanWrite: false }); + const onMine = entry({ number: 4, viewerCanWrite: true }); + // A host that never said is taken as if the viewer may push. + const unsaid = entry({ number: 5, viewerIsAuthor: true }); - const groups = groupPullRequests({ entries: [upstream, asked], viewer: "ada", state: "open" }); + const groups = groupPullRequests({ + entries: [followed, upstream, onMine, asked, unsaid], + viewer: "ada", + state: "open", + }); expect(groups.map((group) => [group.label, group.entries.map((row) => row.number)])).toEqual([ ["Needs you", [2]], - ["Yours", [1]], + ["Yours", [5]], + ["Incoming", [4]], + ["Contributions", [1]], + ["Elsewhere", [3]], ]); // The sidebar count reads the same rows the page groups. - expect(countNeedsYou([upstream, asked])).toBe(1); + expect(countNeedsYou([upstream, asked, followed, onMine, unsaid])).toBe(1); }); it("counts only the rows that need the viewer", () => { @@ -649,6 +660,8 @@ describe("narrowPullRequests", () => { entry({ number: 1, viewerReviewRequested: true }), entry({ number: 2, viewerIsAuthor: true }), entry({ number: 3, projectId: OTHER_PROJECT_ID }), + entry({ number: 4, viewerIsAuthor: true, viewerCanWrite: false }), + entry({ number: 5, viewerCanWrite: false }), ]; const involved = (filters: Partial) => narrowPullRequests(grouped, { ...EMPTY_PULL_REQUEST_FILTERS, ...filters }).map( @@ -657,7 +670,9 @@ describe("narrowPullRequests", () => { expect(involved({ involvement: "needs-you" })).toEqual([1]); expect(involved({ involvement: "yours" })).toEqual([2]); - expect(involved({ involvement: "others" })).toEqual([3]); + expect(involved({ involvement: "incoming" })).toEqual([3]); + expect(involved({ involvement: "contributions" })).toEqual([4]); + expect(involved({ involvement: "elsewhere" })).toEqual([5]); expect(involved({ project: `${ENVIRONMENT_ID}:${OTHER_PROJECT_ID}` })).toEqual([3]); }); diff --git a/apps/web/src/components/pull-requests/pullRequests.logic.ts b/apps/web/src/components/pull-requests/pullRequests.logic.ts index d7ae7fa4..64a195cf 100644 --- a/apps/web/src/components/pull-requests/pullRequests.logic.ts +++ b/apps/web/src/components/pull-requests/pullRequests.logic.ts @@ -56,7 +56,34 @@ export type PullRequestNeedsYouReason = | "Checks failing" | "Approved"; -export type PullRequestGroupId = "needs-you" | "yours" | "others" | "all"; +export type PullRequestGroupId = PullRequestInvolvement | "all"; + +/** + * Where an open row stands with the viewer, from what they can act on down to + * what they can only watch. `yours` and `incoming` are on repositories the + * viewer can merge; `contributions` and `elsewhere` are the same two split on + * repositories they cannot. + */ +export type PullRequestInvolvement = + | "needs-you" + | "yours" + | "incoming" + | "contributions" + | "elsewhere"; + +export const PULL_REQUEST_INVOLVEMENTS: readonly PullRequestInvolvement[] = [ + "needs-you", + "yours", + "incoming", + "contributions", + "elsewhere", +]; + +function isPullRequestInvolvement(value: unknown): value is PullRequestInvolvement { + return ( + typeof value === "string" && (PULL_REQUEST_INVOLVEMENTS as readonly string[]).includes(value) + ); +} export interface PullRequestGroup { readonly id: PullRequestGroupId; @@ -83,8 +110,8 @@ export type PullRequestDraftFilter = "any" | "only" | "hide"; /** `none` is a row no reviewer has answered on yet, which the host omits. */ export type PullRequestReviewFilter = "any" | "none" | PullRequestReviewDecision; export type PullRequestChecksFilter = "any" | "passing" | "failing" | "running"; -/** The same three groups the open list heads, as a narrowing of its own. */ -export type PullRequestInvolvementFilter = "all" | "needs-you" | "yours" | "others"; +/** The same groups the open list heads, as a narrowing of its own. */ +export type PullRequestInvolvementFilter = "all" | PullRequestInvolvement; export type PullRequestSort = | "readiness" | "updated" @@ -160,9 +187,7 @@ export function parsePullRequestsSearch(search: Record): PullRe ...searchText(search["author"], "author"), ...searchText(search["labels"], "labels"), ...searchText(search["project"], "project"), - ...(involvement === "needs-you" || involvement === "yours" || involvement === "others" - ? { involvement } - : {}), + ...(isPullRequestInvolvement(involvement) ? { involvement } : {}), ...(draft === "only" || draft === "hide" ? { draft } : {}), ...(review === "approved" || review === "changes-requested" || @@ -736,21 +761,29 @@ export function countNeedsYou(entries: readonly PullRequestEntry[]): number { return count; } -/** Which of the open list's three groups a row belongs to. */ -export function pullRequestInvolvement( - entry: PullRequestEntry, -): Exclude { +/** + * Which of the open list's groups a row belongs to. A host that does not say + * whether the viewer may push is taken as if they may, so a row is never + * demoted on a silence: it stays under Yours or Incoming as it always did. + */ +export function pullRequestInvolvement(entry: PullRequestEntry): PullRequestInvolvement { if (resolveNeedsYouReason(entry) !== null) { return "needs-you"; } - return entry.viewerIsAuthor ? "yours" : "others"; + const canMerge = entry.viewerCanWrite !== false; + if (entry.viewerIsAuthor) { + return canMerge ? "yours" : "contributions"; + } + return canMerge ? "incoming" : "elsewhere"; } /** - * The open list answers "what needs me" first, then the user's own work, then - * everything else; a row belongs to exactly one group. Without a signed-in - * viewer none of that is knowable, so the list stays flat, as it does for the - * merged and closed tabs where the question does not apply. + * The open list answers "what needs me" first, then what the user can land + * (their own work, then other people's work on their repositories), then what + * is out of their hands (their contributions elsewhere, then everything they + * only follow); a row belongs to exactly one group. Without a signed-in viewer + * none of that is knowable, so the list stays flat, as it does for the merged + * and closed tabs where the question does not apply. */ export function groupPullRequests(input: { readonly entries: readonly PullRequestEntry[]; @@ -773,21 +806,18 @@ export function groupPullRequests(input: { return sorted.length === 0 ? [] : [{ id: "all", label: null, entries: sorted }]; } - const needsYou: PullRequestEntry[] = []; - const yours: PullRequestEntry[] = []; - const others: PullRequestEntry[] = []; - const byInvolvement = { "needs-you": needsYou, yours, others } as const; + const byInvolvement = new Map( + PULL_REQUEST_INVOLVEMENTS.map((involvement) => [involvement, []]), + ); for (const entry of sorted) { - byInvolvement[pullRequestInvolvement(entry)].push(entry); + byInvolvement.get(pullRequestInvolvement(entry))?.push(entry); } - return ( - [ - { id: "needs-you", label: "Needs you", entries: needsYou }, - { id: "yours", label: "Yours", entries: yours }, - { id: "others", label: "Others", entries: others }, - ] as const - ).filter((group) => group.entries.length > 0); + return PULL_REQUEST_INVOLVEMENTS.map((id) => ({ + id, + label: PULL_REQUEST_INVOLVEMENT_WORDS[id], + entries: byInvolvement.get(id) ?? [], + })).filter((group) => group.entries.length > 0); } /** @@ -977,7 +1007,9 @@ export const PULL_REQUEST_INVOLVEMENT_WORDS: Readonly< all: "All", "needs-you": "Needs you", yours: "Yours", - others: "Others", + incoming: "Incoming", + contributions: "Contributions", + elsewhere: "Elsewhere", }; /** diff --git a/docs/design/pull-requests.md b/docs/design/pull-requests.md index f33e2c66..ec2733c4 100644 --- a/docs/design/pull-requests.md +++ b/docs/design/pull-requests.md @@ -191,7 +191,11 @@ Header block: List: - Group header: same voice as the General chats page (`font-mono text-[10px] uppercase -tracking-wider text-muted-foreground/55`), text "Needs you · 3", "Yours · 5", "Others · 12". +tracking-wider text-muted-foreground/55`), text "Needs you · 3", "Yours · 5", "Incoming · 12", + "Contributions · 2", "Elsewhere · 1". Yours and Incoming are on repositories the viewer can merge + (the viewer's own work, then other people's); Contributions and Elsewhere are the same two on + repositories the viewer cannot. A host that does not say whether the viewer may push leaves the + row under Yours or Incoming. - Rows separated by `divide-y divide-border/50`. Each row is a `button` (`hover:bg-muted`, `rounded-md`, `py-2.5`, same as `ChatRow`) laid out as a grid: glyph column, content column. - Glyph: `GitPullRequestIcon` emerald for open (reuse the exact classes from @@ -261,7 +265,7 @@ Action `action:pull-requests`, title "Open pull requests", search terms - `pullRequests.logic.test.ts`: grouping and reasons (one row per group, first-match rule), thread linking (archived excluded, branch must match, project must match), query matching. - `PullRequestsView.browser.tsx`: with `__setEnvironmentApiOverrideForTests` stubbing - `pullRequests.list`: renders three groups with the right counts; the sign-in empty state when + `pullRequests.list`: renders the groups with the right counts; the sign-in empty state when every project is `unauthenticated`; clicking "Review in a thread" opens the dialog with the PR URL prefilled. Keep it to those three. - Regenerate `routeTree.gen.ts` the way the router plugin does (check `apps/web/vite.config.ts` @@ -1006,8 +1010,9 @@ provider like the GitHub ones, on decoders and argv. # Step 5: your pull requests anywhere The page lists the repositories in the workspace. Will also wants the pull requests he opened on -repositories that are not projects here, such as an upstream contribution. Those join the **Yours** -group with the repository named; everything else stays as it is. +repositories that are not projects here, such as an upstream contribution. Those show with the +repository named, under **Yours** where he can merge them and **Contributions** where he cannot; +everything else stays as it is. ## Contracts @@ -1113,7 +1118,7 @@ text-muted-foreground`, dot `size-2 rounded-full` coloured from the label's hex Largest/Smallest use `additions + deletions`. URL `sort` values: `readiness | updated | newest | oldest | largest | smallest` (today's `created` and `size` map to `newest` and `largest`). - Filters menu, one submenu per line with the current value right-aligned in muted text: - Involvement (All, Needs you, Yours, Others), separator, Author (searchable: an input at the top + Involvement (All, Needs you, Yours, Incoming, Contributions, Elsewhere), separator, Author (searchable: an input at the top of the submenu, then "Anyone" and the logins seen in the loaded rows of every state that has been read, avatar + login, selected one first, max ten shown), Labels (searchable checklist of the labels seen in loaded rows, with colour dots; "Any" clears), Draft (Any, Only drafts, No