From 880c101a6797b122006bea50496d666f17c1ab83 Mon Sep 17 00:00:00 2001 From: badcuban <108198679+badcuban@users.noreply.github.com> Date: Sun, 6 Sep 2026 03:43:21 -0400 Subject: [PATCH 1/2] feat(web): pull request list splits what you can merge from what you cannot The open list filed every pull request the viewer wrote under Yours, whether it sat on their own repository or was a contribution to someone else's, and called everything else Others. The rows the viewer can actually land were mixed in with rows that are out of their hands. The list now reads the push access it already carries per row and groups the open tab as Needs you, Yours, Incoming (other people's work on repositories the viewer can merge), Contributions (the viewer's work on repositories they cannot), and Elsewhere (other people's work on repositories they only follow). The Involvement filter offers the same five, and a host that never says whether the viewer may push leaves a row under Yours or Incoming as before. --- .../pull-requests/PullRequestFilters.tsx | 8 +- .../PullRequestsView.browser.tsx | 9 +- .../pull-requests/pullRequests.logic.test.ts | 33 +++++-- .../pull-requests/pullRequests.logic.ts | 88 +++++++++++++------ docs/design/pull-requests.md | 15 ++-- 5 files changed, 107 insertions(+), 46 deletions(-) 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..41397306 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 From ebf2a772b32ad9fc4840beafa7ce4bd9adea0a52 Mon Sep 17 00:00:00 2001 From: badcuban <108198679+badcuban@users.noreply.github.com> Date: Sun, 6 Sep 2026 03:47:29 -0400 Subject: [PATCH 2/2] docs: indent the wrapped lines in the pull request group note --- docs/design/pull-requests.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/design/pull-requests.md b/docs/design/pull-requests.md index 41397306..ec2733c4 100644 --- a/docs/design/pull-requests.md +++ b/docs/design/pull-requests.md @@ -192,10 +192,10 @@ 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", "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. + "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