Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions apps/web/src/components/pull-requests/PullRequestFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { PullRequestActorAvatar } from "./pullRequestPresentation";
import {
hasPullRequestLabel,
PULL_REQUEST_INVOLVEMENT_WORDS,
PULL_REQUEST_INVOLVEMENTS,
PULL_REQUEST_SORT_LABELS,
pullRequestAuthorFacets,
pullRequestFilterChips,
Expand All @@ -52,9 +53,10 @@ interface FilterOption<Value extends string> {
readonly label: string;
}

const INVOLVEMENT_OPTIONS: readonly FilterOption<PullRequestInvolvementFilter>[] = (
["all", "needs-you", "yours", "others"] as const
).map((value) => ({ value, label: PULL_REQUEST_INVOLVEMENT_WORDS[value] }));
const INVOLVEMENT_OPTIONS: readonly FilterOption<PullRequestInvolvementFilter>[] = [
"all" as const,
...PULL_REQUEST_INVOLVEMENTS,
].map((value) => ({ value, label: PULL_REQUEST_INVOLVEMENT_WORDS[value] }));

const DRAFT_OPTIONS: readonly FilterOption<PullRequestDraftFilter>[] = [
{ value: "any", label: "Any" },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
33 changes: 24 additions & 9 deletions apps/web/src/components/pull-requests/pullRequests.logic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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", () => {
Expand Down Expand Up @@ -295,23 +295,34 @@ 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,
viewerCanWrite: false,
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", () => {
Expand Down Expand Up @@ -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<PullRequestFilters>) =>
narrowPullRequests(grouped, { ...EMPTY_PULL_REQUEST_FILTERS, ...filters }).map(
Expand All @@ -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]);
});

Expand Down
88 changes: 60 additions & 28 deletions apps/web/src/components/pull-requests/pullRequests.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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"
Expand Down Expand Up @@ -160,9 +187,7 @@ export function parsePullRequestsSearch(search: Record<string, unknown>): 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" ||
Expand Down Expand Up @@ -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<PullRequestInvolvementFilter, "all"> {
/**
* 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[];
Expand All @@ -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<PullRequestInvolvement, PullRequestEntry[]>(
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);
}

/**
Expand Down Expand Up @@ -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",
};

/**
Expand Down
15 changes: 10 additions & 5 deletions docs/design/pull-requests.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
Loading