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
29 changes: 29 additions & 0 deletions apps/server/src/pullRequest/PullRequestService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,35 @@ it.effect("lists GitHub Enterprise PRs for a stored unknown repository after hos
}),
);

it.effect("reports each host's actions with its summary, so rows can offer them unread", () =>
Effect.gen(function* () {
const service = yield* makeService({
projects: [
project({
id: "p1",
title: "web",
workspaceRoot: "/repo",
repository: "acme/web",
provider: SourceControlProviderKind.make("bitbucket"),
host: "bitbucket.org",
}),
],
providers: [
fakeProvider(SourceControlProviderKind.make("bitbucket"), {
capabilities: {
...fakeProvider(SourceControlProviderKind.make("bitbucket")).capabilities,
actions: ["merge", "close"],
},
}),
],
});

const result = yield* service.list({ state: "open" });

assert.deepStrictEqual(result.providers[0]?.actions, ["merge", "close"]);
}),
);

it.effect("refines unknown self-hosted GitLab projects before listing merge requests", () =>
Effect.gen(function* () {
let refinementCalls = 0;
Expand Down
24 changes: 14 additions & 10 deletions apps/server/src/pullRequest/PullRequestService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1252,20 +1252,24 @@ export const make = Effect.gen(function* () {
// One summary per host, which is what the viewer lookup already answers for: two GitHub
// hosts sign in separately, so collapsing them by kind would report one as the other.
const providers: ReadonlyArray<PullRequestProviderSummary> = [
...viewerResults.map((result) => ({
host: result.host,
kind: result.kind,
searchesOnHost:
projects.find((project) => project.host === result.host)?.api.capabilities.search ??
false,
projectCount: projectCounts.get(result.host) ?? 1,
configured: result.viewer !== null,
detail: result.error === null ? null : providerDetail(result.error),
})),
...viewerResults.map((result) => {
const capabilities = projects.find((project) => project.host === result.host)?.api
.capabilities;
return {
host: result.host,
kind: result.kind,
searchesOnHost: capabilities?.search ?? false,
actions: capabilities?.actions ?? [],
projectCount: projectCounts.get(result.host) ?? 1,
configured: result.viewer !== null,
detail: result.error === null ? null : providerDetail(result.error),
};
}),
...[...unimplemented].map(([host, { kind, projectCount }]) => ({
host,
kind,
searchesOnHost: false,
actions: [],
projectCount,
configured: false,
detail: "This host cannot be browsed here yet.",
Expand Down
7 changes: 6 additions & 1 deletion apps/web/src/components/pullRequest/PullRequestRow.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { PullRequestAction } from "@t3tools/contracts";
import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients";
import { SearchIcon } from "lucide-react";
import { PullRequestStackPopover } from "./PullRequestStackPopover";
Expand Down Expand Up @@ -91,6 +92,7 @@ function PullRequestRowImpl({
statsRef,
onSelect,
speedMode,
hostActions,
onActed,
closing = false,
sweeping = false,
Expand All @@ -113,6 +115,8 @@ function PullRequestRowImpl({
statsRef?: RefCallback<HTMLDivElement>;
onSelect: (entry: PullRequestRowTarget) => void;
speedMode: boolean;
/** What this row's host can do, which is what speed mode offers. */
hostActions: ReadonlySet<PullRequestAction>;
onActed: (result: PullRequestSpeedActionResult) => void;
closing?: boolean;
sweeping?: boolean;
Expand Down Expand Up @@ -256,9 +260,10 @@ function PullRequestRowImpl({
updatedAt={entry.updatedAt}
/>
</button>
{entry.state !== "merged" && entry.provider === "github" ? (
{hostActions.size > 0 ? (
<PullRequestSpeedActions
entry={entry}
hostActions={hostActions}
visible={speedMode}
onActed={onActed}
closing={closing}
Expand Down
13 changes: 6 additions & 7 deletions apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Spinner } from "../ui/spinner";
import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip";
import { resolvePullRequestMergeMethod } from "./pullRequestDetail.logic";
import { PullRequestGlyph } from "./pullRequestIcons";
import type { EnvironmentPullRequestEntry } from "./pullRequestList.logic";
import { pullRequestQuickActions, type EnvironmentPullRequestEntry } from "./pullRequestList.logic";
import {
usePullRequestActionRunner,
usePullRequestDefaultMergeMethodResolver,
Expand All @@ -26,13 +26,16 @@ export interface PullRequestSpeedActionResult<Entry = EnvironmentPullRequestEntr
/** No detail or stack reads until a merge is clicked, even on a long list. */
export function PullRequestSpeedActions<Entry extends PullRequestSpeedActionEntry>({
entry,
hostActions,
visible,
onActed,
closing = false,
sweeping = false,
onCloseSweepStart,
}: {
entry: Entry;
/** What the row's host can do; actions it cannot are not offered. */
hostActions: ReadonlySet<PullRequestAction>;
visible: boolean;
onActed?: (result: PullRequestSpeedActionResult<Entry>) => void;
closing?: boolean;
Expand Down Expand Up @@ -70,13 +73,9 @@ export function PullRequestSpeedActions<Entry extends PullRequestSpeedActionEntr
);
},
});
const actions =
entry.state === "closed"
? (["reopen"] as const)
: entry.isDraft
? (["close", "ready"] as const)
: (["close", "merge"] as const);
const actions = pullRequestQuickActions(entry, hostActions);
const busy = actionPending || closing || sweeping;
if (actions.length === 0) return null;
return (
<div
className="shrink-0 items-center gap-1 pr-3"
Expand Down
27 changes: 19 additions & 8 deletions apps/web/src/components/pullRequest/ThreadPullRequestsPanel.tsx
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type { ProjectId, ScopedThreadRef, ThreadPullRequestLink } from "@t3tools/contracts";
import { detectSourceControlProviderFromRemoteUrl } from "@t3tools/shared/sourceControl";
import { sourceControlClients } from "@t3tools/client-runtime/source-control-clients";
import {
sourceControlClients,
UNKNOWN_SOURCE_CONTROL_CLIENT,
} from "@t3tools/client-runtime/source-control-clients";
import {
resolveThreadPullRequestChains,
visibleThreadPullRequests,
Expand Down Expand Up @@ -45,6 +47,7 @@ import {
pullRequestChecksStatePresentation,
} from "./pullRequestPresentation";
import { PullRequestGlyph } from "./pullRequestIcons";
import { pullRequestQuickActions } from "./pullRequestList.logic";
import { PullRequestSpeedActions } from "./PullRequestSpeedActions";

const SOURCE_LABELS: Record<ThreadPullRequestLink["source"], string> = {
Expand Down Expand Up @@ -105,11 +108,12 @@ function LinkRow({
const snapshot = link.snapshot;
const open = snapshot === null || snapshot.state === "open";
const watching = link.watch !== undefined;
// A link carries no host summary, so the host's own definition says what it can do.
const linkHostActions = (
sourceControlClients.findByChangeRequestUrl(link.url) ?? UNKNOWN_SOURCE_CONTROL_CLIENT
).changeRequestActions;
const actionEntry =
projectId !== null &&
snapshot !== null &&
snapshot.state !== "merged" &&
detectSourceControlProviderFromRemoteUrl(link.url)?.kind === "github"
projectId !== null && snapshot !== null && snapshot.state !== "merged"
? {
environmentId: threadRef.environmentId,
projectId,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand All @@ -121,6 +125,9 @@ function LinkRow({
...(link.stack === null ? {} : { stack: link.stack }),
}
: null;
// Speed mode swaps the menu for quick actions only where a row has some to offer.
const hasQuickActions =
actionEntry !== null && pullRequestQuickActions(actionEntry, linkHostActions).length > 0;
return (
<div
className={cn(PULL_REQUEST_ROW_CLASS, "relative hover:bg-accent/60")}
Expand Down Expand Up @@ -246,7 +253,11 @@ function LinkRow({
/>
</a>
{actionEntry !== null ? (
<PullRequestSpeedActions entry={actionEntry} visible={speedMode} />
<PullRequestSpeedActions
entry={actionEntry}
hostActions={linkHostActions}
visible={speedMode}
/>
) : null}
{/* Out of the row's flow, so no row reserves a column for a button only the hovered one
shows. It sits over the right end of the second line on the row's own hover color,
Expand All @@ -261,7 +272,7 @@ function LinkRow({
"has-[[data-popup-open]]:pointer-events-auto has-[[data-popup-open]]:opacity-100",
"has-[:focus-visible]:pointer-events-auto has-[:focus-visible]:opacity-100",
"group-has-[[data-pull-request-action-pending=true]]/pr-row:hidden",
speedMode && actionEntry !== null && "hidden",
speedMode && hasQuickActions && "hidden",
)}
>
<span aria-hidden className="absolute inset-0 bg-accent/60" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ function row(overrides: Partial<EnvironmentPullRequestEntry>): ReactNode {
showProvider: false,
onSelect: () => {},
speedMode: false,
hostActions: new Set(),
onActed: () => {},
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { SourceControlProviderKind } from "@t3tools/contracts";
import type { EnvironmentId, ProjectId, PullRequestListEntry } from "@t3tools/contracts";
import type {
EnvironmentId,
ProjectId,
PullRequestAction,
PullRequestListEntry,
} from "@t3tools/contracts";
import { describe, expect, it } from "vite-plus/test";

import {
pullRequestHostActions,
pullRequestQuickActions,
filterPullRequestsByInvolvement,
findScopedProject,
mergePullRequestLists,
Expand Down Expand Up @@ -1720,3 +1727,35 @@ describe("pull request list override settlement", () => {
expect(swapped.map((row) => row.number)).toEqual([2, 1]);
});
});

describe("pull request quick actions", () => {
const summary = (host: string, kind: string, actions?: ReadonlyArray<PullRequestAction>) => ({
host,
kind: SourceControlProviderKind.make(kind),
searchesOnHost: true,
projectCount: 1,
configured: true,
detail: null,
...(actions === undefined ? {} : { actions }),
});

it("offers what each host reports, and GitHub's legacy set when a server reports none", () => {
const actionsOf = pullRequestHostActions([
summary("bitbucket.org", "bitbucket", ["merge", "close"]),
summary("github.com", "github"),
]);
const bitbucket = actionsOf({
host: "bitbucket.org",
provider: SourceControlProviderKind.make("bitbucket"),
});
expect(pullRequestQuickActions({ state: "closed", isDraft: false }, bitbucket)).toEqual([]);
const github = actionsOf({
host: "github.com",
provider: SourceControlProviderKind.make("github"),
});
expect(pullRequestQuickActions({ state: "open", isDraft: true }, github)).toEqual([
"close",
"ready",
]);
});
});
42 changes: 42 additions & 0 deletions apps/web/src/components/pullRequest/pullRequestList.logic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1262,3 +1262,45 @@ export function settlePullRequestOverrides<Entry extends PullRequestListEntry>(
}
return kept.size === overrides.size ? overrides : kept;
}

/** The row actions speed mode offers, in the order they are drawn. */
export type PullRequestQuickAction = "close" | "merge" | "ready" | "reopen";

/**
* Which actions each host can take on a listed row, before any row's detail is read. Hosts are
* matched by the host summary; a server that does not report actions per host only ever offered
* these on GitHub, so that is what it keeps getting.
*/
export function pullRequestHostActions(
providers: ReadonlyArray<PullRequestListResult["providers"][number]>,
): (
entry: Pick<EnvironmentPullRequestEntry, "host" | "provider">,
) => ReadonlySet<PullRequestAction> {
const byHost = new Map(
providers.flatMap((provider) =>
provider.actions === undefined
? []
: [[provider.host.toLowerCase(), new Set(provider.actions)] as const],
),
);
const legacy = new Set<PullRequestAction>(["close", "merge", "ready", "reopen"]);
const none = new Set<PullRequestAction>();
return (entry) =>
byHost.get(entry.host.toLowerCase()) ?? (entry.provider === "github" ? legacy : none);
}

/** The quick actions a row offers: what its state allows, narrowed to what its host can do. */
export function pullRequestQuickActions(
entry: Pick<EnvironmentPullRequestEntry, "state" | "isDraft">,
hostActions: ReadonlySet<PullRequestAction>,
): ReadonlyArray<PullRequestQuickAction> {
const forState: ReadonlyArray<PullRequestQuickAction> =
entry.state === "merged"
? []
: entry.state === "closed"
? ["reopen"]
: entry.isDraft
? ["close", "ready"]
: ["close", "merge"];
return forState.filter((action) => hostActions.has(action));
}
4 changes: 2 additions & 2 deletions apps/web/src/components/pullRequest/usePullRequestActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ export function usePullRequestCloseBatch(onClosed: (entry: EnvironmentPullReques
async (entries: readonly EnvironmentPullRequestEntry[]) => {
const batch = entries.filter((entry) => {
const key = pullRequestEntryKey(entry);
if (entry.state !== "open" || entry.provider !== "github" || pending.current.has(key))
return false;
// The sweep only gathers rows whose host can close them, so state is all that is left.
if (entry.state !== "open" || pending.current.has(key)) return false;
pending.current.add(key);
return true;
});
Expand Down
Loading
Loading