diff --git a/apps/desktop/src/renderer/lib/persisted-keys/persisted-key-registry.test-data.ts b/apps/desktop/src/renderer/lib/persisted-keys/persisted-key-registry.test-data.ts index d2a1ea65381..c3e3cecc9ea 100644 --- a/apps/desktop/src/renderer/lib/persisted-keys/persisted-key-registry.test-data.ts +++ b/apps/desktop/src/renderer/lib/persisted-keys/persisted-key-registry.test-data.ts @@ -133,6 +133,10 @@ export const PERSISTED_KEY_REGISTRY: ReadonlyArray< "src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.ts", ["pull-requests-filter-state"], ], + [ + "src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsSplitViewStore/pullRequestsSplitViewStore.ts", + ["pull-requests-split-view-state"], + ], [ "src/renderer/components/PostHogUserIdentifier/PostHogUserIdentifier.tsx", ["active_organization_id"], diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsx index 45e7c1c9384..bf3f6651a62 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsx @@ -142,6 +142,7 @@ export function DashboardSidebarHeader({ authorFilter: lastPullRequestsAuthorFilter, reviewFilter: lastPullRequestsReviewFilter, includeClosed: lastPullRequestsIncludeClosed, + mergedOnly: lastPullRequestsMergedOnly, } = usePullRequestsFilterStore(); const handleWorkspacesClick = () => { @@ -185,6 +186,7 @@ export function DashboardSidebarHeader({ authorFilter: lastPullRequestsAuthorFilter, reviewFilter: lastPullRequestsReviewFilter, includeClosed: lastPullRequestsIncludeClosed, + mergedOnly: lastPullRequestsMergedOnly, }), }); }); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/ProjectFilter/ProjectFilter.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/ProjectFilter/ProjectFilter.tsx index ee9a2139f5b..9a4fb1476d0 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/ProjectFilter/ProjectFilter.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/ProjectFilter/ProjectFilter.tsx @@ -8,6 +8,7 @@ import { CommandList, } from "@superset/ui/command"; import { Popover, PopoverContent, PopoverTrigger } from "@superset/ui/popover"; +import { cn } from "@superset/ui/utils"; import { useMemo, useState } from "react"; import { HiCheck, @@ -22,9 +23,17 @@ import { ProjectThumbnail } from "renderer/routes/_authenticated/components/Proj interface ProjectFilterProps { value: string[]; onChange: (value: string[]) => void; + /** Shows the label text regardless of container width — the default + * `@4xl:inline` gating assumes a wide inline toolbar row, which doesn't + * apply inside a narrow popover. */ + alwaysShowLabel?: boolean; } -export function ProjectFilter({ value, onChange }: ProjectFilterProps) { +export function ProjectFilter({ + value, + onChange, + alwaysShowLabel, +}: ProjectFilterProps) { const [open, setOpen] = useState(false); const [search, setSearch] = useState(""); @@ -97,7 +106,12 @@ export function ProjectFilter({ value, onChange }: ProjectFilterProps) { ) : ( )} - + {label} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx index ae10028e1d4..b95ea4aab77 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx @@ -1,3 +1,4 @@ +import { cn } from "@superset/ui/utils"; import { useMatchRoute, useParams } from "@tanstack/react-router"; import { HiOutlineWifi } from "react-icons/hi2"; import { ZoomStable } from "renderer/components/ZoomStable"; @@ -34,6 +35,8 @@ export function TopBar() { const isV2CloudEnabled = useIsV2CloudEnabled(); const isSidebarOpen = useWorkspaceSidebarStore((s) => s.isOpen); const isSidebarCollapsed = useWorkspaceSidebarStore((s) => s.isCollapsed()); + const isPullRequestsRoute = + matchRoute({ to: "/pull-requests", fuzzy: true }) !== false; // Default to Mac layout while loading to avoid overlap with traffic lights const isMac = platform === undefined || platform === "darwin"; // In v2 the expanded sidebar lives outside the TopBar column, so the TopBar @@ -55,7 +58,12 @@ export function TopBar() { // spacer + title filler), never on this container: `no-drag` carve-outs // under a `drag` ancestor are lost inside zoomed/masked/scrollable // wrappers, which makes the whole bar swallow clicks. - className="gap-2 h-12 w-full flex items-center justify-between bg-muted/45 relative dark:bg-muted/35" + className={cn( + "gap-2 h-12 w-full flex items-center justify-between relative dark:bg-muted/35", + isPullRequestsRoute && isSidebarCollapsed + ? "bg-sidebar" + : "bg-muted/45", + )} style={barStyle} >
diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/WorkItemsSearch/WorkItemsSearch.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/WorkItemsSearch/WorkItemsSearch.tsx index df53f55254e..f4078b4e676 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/WorkItemsSearch/WorkItemsSearch.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/WorkItemsSearch/WorkItemsSearch.tsx @@ -1,4 +1,5 @@ import { Input } from "@superset/ui/input"; +import { cn } from "@superset/ui/utils"; import { useRef } from "react"; import { HiOutlineMagnifyingGlass } from "react-icons/hi2"; import { useHotkey } from "renderer/hotkeys"; @@ -8,6 +9,7 @@ interface WorkItemsSearchProps { onChange: (value: string) => void; placeholder: string; label: string; + className?: string; } export function WorkItemsSearch({ @@ -15,6 +17,7 @@ export function WorkItemsSearch({ onChange, placeholder, label, + className, }: WorkItemsSearchProps) { const inputRef = useRef(null); @@ -45,7 +48,10 @@ export function WorkItemsSearch({ inputRef.current?.blur(); } }} - className="h-8 border-0 bg-muted/50 pl-9 pr-3 text-sm focus-visible:ring-1" + className={cn( + "h-8 border-0 bg-muted/50 pl-9 pr-3 text-sm focus-visible:ring-1", + className, + )} />
); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/$prNumber/page.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/$prNumber/page.tsx index 69d9183cf76..306f4a353fe 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/$prNumber/page.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/$prNumber/page.tsx @@ -1,19 +1,46 @@ +import { + AlertDialog, + AlertDialogAction, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + EnterEnabledAlertDialogContent, +} from "@superset/ui/alert-dialog"; +import { Button } from "@superset/ui/button"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuSeparator, + DropdownMenuTrigger, +} from "@superset/ui/dropdown-menu"; import { ScrollArea } from "@superset/ui/scroll-area"; -import { useQuery } from "@tanstack/react-query"; -import { createFileRoute, useNavigate } from "@tanstack/react-router"; -import { useMemo } from "react"; +import { toast } from "@superset/ui/sonner"; +import { cn } from "@superset/ui/utils"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { createFileRoute } from "@tanstack/react-router"; +import { useState } from "react"; +import { + LuExternalLink, + LuGitPullRequestClosed, + LuPlus, + LuRotateCcw, +} from "react-icons/lu"; +import { VscChevronDown, VscGitMerge } from "react-icons/vsc"; import { MarkdownRenderer } from "renderer/components/MarkdownRenderer"; import { useHostUrl } from "renderer/hooks/host-service/useHostTargetUrl"; import { getHostServiceClientByUrl } from "renderer/lib/host-service-client"; -import { resolveProjectFilterParams } from "renderer/routes/_authenticated/_dashboard/components/ProjectFilter/project-filter-utils"; -import { WorkItemDetailHeader } from "renderer/routes/_authenticated/_dashboard/components/WorkItemDetailHeader"; import { WorkItemDetailState } from "renderer/routes/_authenticated/_dashboard/components/WorkItemDetailState"; import { useProjectHost } from "renderer/routes/_authenticated/_dashboard/hooks/useProjectHost"; import { PullRequestChecksSection } from "renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestChecksSection"; +import { PullRequestListToggle } from "renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestListToggle"; import { parsePositiveIntegerParam } from "renderer/routes/_authenticated/_dashboard/utils/parsePositiveIntegerParam"; import { normalizePRState, PRIcon, + type PRState, } from "renderer/screens/main/components/PRIcon"; import { type LinkedPR, @@ -21,9 +48,6 @@ import { } from "renderer/stores/new-workspace-draft"; import { useOpenNewWorkspaceModal } from "renderer/stores/new-workspace-modal"; import { Route as PullRequestsLayoutRoute } from "../layout"; -import { pullRequestsSearchFromFilters } from "../stores/pullRequestsFilterStore"; -import { normalizeAuthorFilter } from "../utils/normalizeAuthorFilter"; -import { normalizePullRequestReviewFilter } from "../utils/pullRequestReviewFilter"; export const Route = createFileRoute( "/_authenticated/_dashboard/pull-requests/$prNumber/", @@ -31,11 +55,31 @@ export const Route = createFileRoute( component: PullRequestDetailPage, }); +type MergeMethod = "merge" | "squash" | "rebase"; +const MERGE_METHOD_LABELS: Record = { + squash: "Squash and merge", + merge: "Create merge commit", + rebase: "Rebase and merge", +}; + +type PendingAction = { kind: "close" } | { kind: "merge"; method: MergeMethod }; + +// Mirrors PRStatusGroup's state-tinted badge language, so a PR reads the +// same way here as it does in the v2 workspace sidebar. +const STATE_BADGE_STYLES: Record = { + open: "border-emerald-500/30 bg-emerald-500/10 text-emerald-600 dark:text-emerald-400", + merged: + "border-violet-500/30 bg-violet-500/10 text-violet-600 dark:text-violet-400", + closed: "border-rose-500/30 bg-rose-500/10 text-rose-600 dark:text-rose-400", + draft: "border-border bg-muted/40 text-muted-foreground", + queued: + "border-amber-500/30 bg-amber-500/10 text-amber-600 dark:text-amber-400", +}; + function PullRequestDetailPage() { const { prNumber: prNumberRaw } = Route.useParams(); const prNumber = parsePositiveIntegerParam(prNumberRaw); const search = PullRequestsLayoutRoute.useSearch(); - const navigate = useNavigate(); const projectId = search.project ?? null; const { hostId, @@ -49,25 +93,9 @@ function PullRequestDetailPage() { ); const resetDraft = useNewWorkspaceDraftStore((state) => state.resetDraft); const openModal = useOpenNewWorkspaceModal(); - - // `project` identifies this PR's repo, not the list filter: falling back - // to it would rewrite an "all repositories" view to a single repo on back. - const backSearch = useMemo( - () => - pullRequestsSearchFromFilters({ - search: search.search ?? "", - projectFilters: resolveProjectFilterParams(search.projects, null, []), - authorFilter: normalizeAuthorFilter(search.author), - reviewFilter: normalizePullRequestReviewFilter(search.review), - includeClosed: search.state === "all", - }), - [ - search.author, - search.projects, - search.review, - search.search, - search.state, - ], + const queryClient = useQueryClient(); + const [pendingAction, setPendingAction] = useState( + null, ); const { data, isLoading, error, refetch } = useQuery({ @@ -85,10 +113,68 @@ function PullRequestDetailPage() { gcTime: 10 * 60_000, }); - const handleBack = () => { - navigate({ to: "/pull-requests", search: backSearch }); + const invalidatePullRequestQueries = () => { + void queryClient.invalidateQueries({ + queryKey: ["pull-request-detail", projectId, hostUrl, prNumber], + }); + void queryClient.invalidateQueries({ queryKey: ["pullRequests"] }); + }; + + const setPullRequestState = useMutation({ + mutationFn: async (nextState: "open" | "closed") => { + if (!hostUrl || !projectId || prNumber === null) { + throw new Error("This project isn't linked to a GitHub repository."); + } + const client = getHostServiceClientByUrl(hostUrl); + return client.pullRequests.setState.mutate({ + projectId, + prNumber, + state: nextState, + }); + }, + onSuccess: invalidatePullRequestQueries, + onError: (mutationError) => { + toast.error("Couldn't update pull request", { + description: mutationError.message, + }); + }, + }); + + const mergePullRequest = useMutation({ + mutationFn: async (mergeMethod: MergeMethod) => { + if (!hostUrl || !projectId || prNumber === null) { + throw new Error("This project isn't linked to a GitHub repository."); + } + const client = getHostServiceClientByUrl(hostUrl); + return client.pullRequests.mergePR.mutate({ + projectId, + prNumber, + mergeMethod, + }); + }, + onSuccess: invalidatePullRequestQueries, + onError: (mutationError) => { + toast.error("Couldn't merge pull request", { + description: mutationError.message, + }); + }, + }); + + const isActionPending = + setPullRequestState.isPending || mergePullRequest.isPending; + + const handleConfirmAction = () => { + if (!pendingAction) return; + if (pendingAction.kind === "close") { + setPullRequestState.mutate("closed"); + } else { + mergePullRequest.mutate(pendingAction.method); + } + setPendingAction(null); }; + const handleReopen = () => setPullRequestState.mutate("open"); + const handleAddToWorkspace = () => { if (!projectId || !hostId || !data) return; const linkedPR: LinkedPR = { @@ -107,16 +193,127 @@ function PullRequestDetailPage() { const state = data ? normalizePRState(data.state, data.isDraft) : defaultState; + const canMerge = data?.state === "open" && !data.isDraft; + // The list pane is always visible in the split view (or reachable via the + // list-collapse toggle in the shared layout), so there's no "back" + // affordance here — just the PR identity and its actions. + const itemNumber = data?.number ?? prNumber; const header = ( - } - backLabel="Back to pull requests" - externalLabel="Open pull request in GitHub" - url={data?.url ?? null} - onBack={handleBack} - onAddToWorkspace={data ? handleAddToWorkspace : null} - /> +
+ + + + {itemNumber === null ? "#—" : `#${itemNumber}`} + + {data && ( + + {data.isDraft ? "Draft" : data.state} + + )} +
+
+ {data?.url && ( + + )} + {data && data.state !== "merged" && ( + + + + + + {canMerge && ( + <> + + Merge + + {(["squash", "merge", "rebase"] as const).map((method) => ( + + setPendingAction({ kind: "merge", method }) + } + > + + {MERGE_METHOD_LABELS[method]} + + ))} + + + )} + {data.state === "open" && ( + setPendingAction({ kind: "close" })} + > + + Close pull request + + )} + {data.state === "closed" && ( + + + Reopen pull request + + )} + + + )} + {data && ( + + )} +
+
); if (prNumber === null) { @@ -201,8 +398,55 @@ function PullRequestDetailPage() { return (
{header} + { + if (!open) setPendingAction(null); + }} + > + + + + {pendingAction?.kind === "close" + ? `Close #${data.number}?` + : `Merge #${data.number}?`} + + + {pendingAction?.kind === "close" + ? `"${data.title}" will be marked closed on GitHub. You can reopen it from here at any time.` + : `"${data.title}" will be merged into ${data.baseBranch}${ + pendingAction?.kind === "merge" + ? ` via ${MERGE_METHOD_LABELS[pendingAction.method].toLowerCase()}` + : "" + }. This can't be undone from here.`} + + + + + + {pendingAction?.kind === "close" + ? "Close pull request" + : "Merge pull request"} + + + + -
+
diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestDetailToggle/PullRequestDetailToggle.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestDetailToggle/PullRequestDetailToggle.tsx new file mode 100644 index 00000000000..23c76157ffc --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestDetailToggle/PullRequestDetailToggle.tsx @@ -0,0 +1,54 @@ +import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip"; +import { + LuPanelRight, + LuPanelRightClose, + LuPanelRightOpen, +} from "react-icons/lu"; +import { usePullRequestsSplitViewStore } from "../../stores/pullRequestsSplitViewStore"; + +/** + * Reclaims the detail pane's width for the list — the mirror image of + * `PullRequestListToggle`. Rendered from the list's own top bar so it stays + * reachable even when the detail pane is fully hidden. + */ +export function PullRequestDetailToggle() { + const isDetailCollapsed = usePullRequestsSplitViewStore( + (s) => s.isDetailCollapsed, + ); + const toggleDetailCollapsed = usePullRequestsSplitViewStore( + (s) => s.toggleDetailCollapsed, + ); + + return ( + + + + + + {isDetailCollapsed + ? "Show pull request preview" + : "Hide pull request preview"} + + + ); +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestDetailToggle/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestDetailToggle/index.ts new file mode 100644 index 00000000000..3094b0a5e80 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestDetailToggle/index.ts @@ -0,0 +1 @@ +export { PullRequestDetailToggle } from "./PullRequestDetailToggle"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestListToggle/PullRequestListToggle.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestListToggle/PullRequestListToggle.tsx new file mode 100644 index 00000000000..1ae625eb6b5 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestListToggle/PullRequestListToggle.tsx @@ -0,0 +1,49 @@ +import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip"; +import { LuPanelLeft, LuPanelLeftClose, LuPanelLeftOpen } from "react-icons/lu"; +import { usePullRequestsSplitViewStore } from "../../stores/pullRequestsSplitViewStore"; + +/** + * Reclaims the list pane's width for the detail pane. The empty index state + * has no header of its own, so `layout.tsx` renders this above its ``; the PR detail page has a header row and renders this inline instead, + * so it stays reachable from every child route without stacking two bars. + */ +export function PullRequestListToggle() { + const isListCollapsed = usePullRequestsSplitViewStore( + (s) => s.isListCollapsed, + ); + const toggleListCollapsed = usePullRequestsSplitViewStore( + (s) => s.toggleListCollapsed, + ); + + return ( + + + + + + {isListCollapsed ? "Show pull request list" : "Hide pull request list"} + + + ); +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestListToggle/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestListToggle/index.ts new file mode 100644 index 00000000000..94843c34044 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestListToggle/index.ts @@ -0,0 +1 @@ +export { PullRequestListToggle } from "./PullRequestListToggle"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/PullRequestsView.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/PullRequestsView.tsx index 226f59f13a8..351fd45c624 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/PullRequestsView.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/PullRequestsView.tsx @@ -19,7 +19,13 @@ interface PullRequestsViewProps { initialProjects?: string[]; initialAuthor?: string; initialReview?: string; - initialState?: "open" | "all"; + initialState?: "open" | "all" | "merged"; + /** The PR currently open in the detail pane, if any — filter/search + * changes navigate back to it instead of collapsing the detail pane. */ + selectedPrNumber?: number | null; + /** The open PR's own project id — distinct from the list's `projects` + * filter, and must survive filter-driven re-navigations. */ + selectedPrProjectId?: string | null; } export function PullRequestsView({ @@ -28,6 +34,8 @@ export function PullRequestsView({ initialAuthor, initialReview, initialState, + selectedPrNumber = null, + selectedPrProjectId = null, }: PullRequestsViewProps) { const navigate = useNavigate(); const { @@ -36,11 +44,13 @@ export function PullRequestsView({ authorFilter: storedAuthorFilter, reviewFilter: storedReviewFilter, includeClosed: storedIncludeClosed, + mergedOnly: storedMergedOnly, setSearch: storeSetSearch, setProjectFilters: storeSetProjectFilters, setAuthorFilter: storeSetAuthorFilter, setReviewFilter: storeSetReviewFilter, setIncludeClosed: storeSetIncludeClosed, + setMergedOnly: storeSetMergedOnly, } = usePullRequestsFilterStore(); const [searchQuery, setSearchQuery] = useState(initialSearch ?? storedSearch); const projectFilters = initialProjects ?? storedProjectFilters; @@ -53,7 +63,26 @@ export function PullRequestsView({ ? storedReviewFilter : normalizePullRequestReviewFilter(initialReview); const includeClosed = - initialState === undefined ? storedIncludeClosed : initialState === "all"; + initialState === undefined + ? storedIncludeClosed + : initialState === "all" || initialState === "merged"; + const mergedOnly = + initialState === undefined ? storedMergedOnly : initialState === "merged"; + // Filter/search changes must not collapse an open detail pane. + const navigateTo = useCallback( + (search: Record) => + selectedPrNumber != null + ? navigate({ + to: "/pull-requests/$prNumber", + params: { prNumber: String(selectedPrNumber) }, + search: selectedPrProjectId + ? { ...search, project: selectedPrProjectId } + : search, + replace: true, + }) + : navigate({ to: "/pull-requests", search, replace: true }), + [navigate, selectedPrNumber, selectedPrProjectId], + ); const { isReady: areProjectsReady, projects: hostProjects, @@ -78,6 +107,7 @@ export function PullRequestsView({ author?: string | null; review?: PullRequestReviewFilter | null; includeClosed?: boolean; + mergedOnly?: boolean; }) => pullRequestsSearchFromFilters({ search: overrides.search ?? searchQuery, @@ -90,18 +120,20 @@ export function PullRequestsView({ reviewFilter: overrides.review !== undefined ? overrides.review : reviewFilter, includeClosed: overrides.includeClosed ?? includeClosed, + mergedOnly: overrides.mergedOnly ?? mergedOnly, }), - [authorFilter, includeClosed, projectFilters, reviewFilter, searchQuery], + [ + authorFilter, + includeClosed, + mergedOnly, + projectFilters, + reviewFilter, + searchQuery, + ], ); const navigateSearch = useCallback( - (query: string) => { - navigate({ - to: "/pull-requests", - search: buildSearch({ search: query }), - replace: true, - }); - }, - [buildSearch, navigate], + (query: string) => navigateTo(buildSearch({ search: query })), + [buildSearch, navigateTo], ); const { cancelPendingSearchNavigation, @@ -124,6 +156,10 @@ export function PullRequestsView({ storeSetIncludeClosed(includeClosed); }, [includeClosed, storeSetIncludeClosed]); + useEffect(() => { + storeSetMergedOnly(mergedOnly); + }, [mergedOnly, storeSetMergedOnly]); + const projects = useMemo( () => hostProjects.map((project) => ({ @@ -132,6 +168,18 @@ export function PullRequestsView({ })), [hostProjects], ); + const repoSlugByProjectId = useMemo( + () => + new Map( + hostProjects.map((project) => [ + project.projectKey, + project.repoOwner && project.repoName + ? `${project.repoOwner}/${project.repoName}` + : project.name, + ]), + ), + [hostProjects], + ); useEffect(() => { if (!areProjectsReady) return; @@ -141,16 +189,12 @@ export function PullRequestsView({ ); if (availableFilters.length === projectFilters.length) return; cancelPendingSearchNavigation(); - navigate({ - to: "/pull-requests", - search: buildSearch({ projects: availableFilters }), - replace: true, - }); + navigateTo(buildSearch({ projects: availableFilters })); }, [ areProjectsReady, buildSearch, cancelPendingSearchNavigation, - navigate, + navigateTo, projectFilters, projects, ]); @@ -167,31 +211,28 @@ export function PullRequestsView({ const handleProjectFiltersChange = (projects: string[]) => { cancelPendingSearchNavigation(); storeSetProjectFilters(projects); - navigate({ - to: "/pull-requests", - search: buildSearch({ projects }), - replace: true, - }); + navigateTo(buildSearch({ projects })); }; - const handleIncludeClosedChange = (nextIncludeClosed: boolean) => { + /** Drives the All / Open / Merged segmented control as one control. */ + const handleStateFilterChange = (next: "open" | "all" | "merged") => { cancelPendingSearchNavigation(); + const nextIncludeClosed = next !== "open"; + const nextMergedOnly = next === "merged"; storeSetIncludeClosed(nextIncludeClosed); - navigate({ - to: "/pull-requests", - search: buildSearch({ includeClosed: nextIncludeClosed }), - replace: true, - }); + storeSetMergedOnly(nextMergedOnly); + navigateTo( + buildSearch({ + includeClosed: nextIncludeClosed, + mergedOnly: nextMergedOnly, + }), + ); }; const handleAuthorFilterChange = (nextAuthor: string | null) => { cancelPendingSearchNavigation(); storeSetAuthorFilter(nextAuthor); - navigate({ - to: "/pull-requests", - search: buildSearch({ author: nextAuthor }), - replace: true, - }); + navigateTo(buildSearch({ author: nextAuthor })); }; const handleReviewFilterChange = ( @@ -199,13 +240,15 @@ export function PullRequestsView({ ) => { cancelPendingSearchNavigation(); storeSetReviewFilter(nextReview); - navigate({ - to: "/pull-requests", - search: buildSearch({ review: nextReview }), - replace: true, - }); + navigateTo(buildSearch({ review: nextReview })); }; + const stateFilter: "open" | "all" | "merged" = mergedOnly + ? "merged" + : includeClosed + ? "all" + : "open"; + return (
diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestRow/PullRequestRow.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestRow/PullRequestRow.tsx new file mode 100644 index 00000000000..2c1b89599ff --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestRow/PullRequestRow.tsx @@ -0,0 +1,117 @@ +import { Avatar, AvatarFallback, AvatarImage } from "@superset/ui/avatar"; +import { cn } from "@superset/ui/utils"; +import { formatRelativeTime } from "renderer/lib/formatRelativeTime"; +import { PullRequestChecksSummary } from "renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestChecksSummary"; +import type { PullRequestCheck } from "renderer/routes/_authenticated/_dashboard/pull-requests/components/pull-request-checks"; +import { + normalizePRState, + PRIcon, +} from "renderer/screens/main/components/PRIcon"; + +export interface PullRequestRowData { + projectId: string; + prNumber: number; + title: string; + state: "open" | "closed" | "merged"; + isDraft: boolean; + authorLogin: string | null; + updatedAt: string | null; + additions: number | null; + deletions: number | null; + headRefName: string | null; + checks: PullRequestCheck[]; +} + +interface PullRequestRowProps { + pr: PullRequestRowData; + repoSlug: string | undefined; + isSelected: boolean; + onClick: () => void; +} + +export function PullRequestRow({ + pr, + repoSlug, + isSelected, + onClick, +}: PullRequestRowProps) { + const state = normalizePRState(pr.state, pr.isDraft); + const hasDiffStat = pr.additions != null || pr.deletions != null; + const parsedUpdatedAt = pr.updatedAt ? new Date(pr.updatedAt).getTime() : NaN; + const updatedAtMs = Number.isNaN(parsedUpdatedAt) ? null : parsedUpdatedAt; + + return ( + // biome-ignore lint/a11y/useSemanticElements: row is a composite list item, not a native control +
{ + if (e.target !== e.currentTarget) return; + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + onClick(); + } + }} + role="button" + tabIndex={0} + aria-current={isSelected ? "true" : undefined} + > + +
+ + {pr.title} + +
+ {pr.authorLogin && ( +
+ + + + {pr.authorLogin.slice(0, 1).toUpperCase()} + + +
+ )} + {repoSlug && ( + + {repoSlug} + + )} + + #{pr.prNumber} + + {pr.headRefName && ( + <> + · + + {pr.headRefName} + + + )} +
+
+
+ {updatedAtMs != null && {formatRelativeTime(updatedAtMs)}} +
+ + {hasDiffStat && ( + + + +{pr.additions ?? 0} + + + -{pr.deletions ?? 0} + + + )} +
+
+
+ ); +} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestRow/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestRow/index.ts new file mode 100644 index 00000000000..ac69325c9d8 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestRow/index.ts @@ -0,0 +1 @@ +export { PullRequestRow, type PullRequestRowData } from "./PullRequestRow"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsContent/PullRequestsContent.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsContent/PullRequestsContent.tsx index 00eb77835e6..ae211131a13 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsContent/PullRequestsContent.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsContent/PullRequestsContent.tsx @@ -1,25 +1,16 @@ import { Button } from "@superset/ui/button"; import { useNavigate } from "@tanstack/react-router"; import { GoGitPullRequest } from "react-icons/go"; -import { HiOutlineArrowTopRightOnSquare } from "react-icons/hi2"; -import { LuMinus, LuPlus, LuRefreshCw } from "react-icons/lu"; +import { LuRefreshCw } from "react-icons/lu"; import { useDebouncedValue } from "renderer/hooks/useDebouncedValue"; import { getHostServiceClientByUrl } from "renderer/lib/host-service-client"; import { LoadMoreSentinel } from "renderer/routes/_authenticated/_dashboard/components/LoadMoreSentinel"; import { serializeProjectFilters } from "renderer/routes/_authenticated/_dashboard/components/ProjectFilter/project-filter-utils"; import type { ProjectQueryTarget } from "renderer/routes/_authenticated/_dashboard/hooks/useProjectQueryTargets"; import { useWorkItemsList } from "renderer/routes/_authenticated/_dashboard/hooks/useWorkItemsList"; +import { usePullRequestsSplitViewStore } from "renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsSplitViewStore"; import type { PullRequestReviewFilter } from "renderer/routes/_authenticated/_dashboard/pull-requests/utils/pullRequestReviewFilter"; -import { - normalizePRState, - PRIcon, -} from "renderer/screens/main/components/PRIcon"; -import { - type LinkedPR, - useNewWorkspaceDraftStore, -} from "renderer/stores/new-workspace-draft"; -import { useOpenNewWorkspaceModal } from "renderer/stores/new-workspace-modal"; -import { PullRequestChecksSummary } from "../../../PullRequestChecksSummary"; +import { PullRequestRow } from "../PullRequestRow"; interface PullRequestsContentProps { projectFilters: string[]; @@ -30,7 +21,10 @@ interface PullRequestsContentProps { authorFilter: string | null; reviewFilter: PullRequestReviewFilter | null; includeClosed: boolean; - onCollapse?: () => void; + mergedOnly: boolean; + selectedPrNumber: number | null; + selectedPrProjectId: string | null; + repoSlugByProjectId: Map; } const PAGE_SIZE = 30; @@ -44,14 +38,14 @@ export function PullRequestsContent({ authorFilter, reviewFilter, includeClosed, - onCollapse, + mergedOnly, + selectedPrNumber, + selectedPrProjectId, + repoSlugByProjectId, }: PullRequestsContentProps) { const debouncedQuery = useDebouncedValue(searchQuery, 300); const navigate = useNavigate(); - const updateDraft = useNewWorkspaceDraftStore((s) => s.updateDraft); - const selectProject = useNewWorkspaceDraftStore((s) => s.selectProject); - const resetDraft = useNewWorkspaceDraftStore((s) => s.resetDraft); - const openModal = useOpenNewWorkspaceModal(); + const expandDetail = usePullRequestsSplitViewStore((s) => s.expandDetail); const { rows: pullRequests, @@ -66,7 +60,7 @@ export function PullRequestsContent({ sentinelRef, } = useWorkItemsList({ projectTargets, - resetKey: `${debouncedQuery.trim()}\0${authorFilter ?? ""}\0${reviewFilter ?? ""}\0${includeClosed}`, + resetKey: `${debouncedQuery.trim()}\0${authorFilter ?? ""}\0${reviewFilter ?? ""}\0${includeClosed}\0${mergedOnly}`, getQueryOptions: ({ target, page }) => ({ queryKey: [ "pullRequests", @@ -77,6 +71,7 @@ export function PullRequestsContent({ authorFilter, reviewFilter, includeClosed, + mergedOnly, page, ], queryFn: async () => { @@ -91,6 +86,7 @@ export function PullRequestsContent({ review: reviewFilter ?? undefined, limit: PAGE_SIZE, includeClosed, + mergedOnly, page, }); // The router types come from this build, the rows come from @@ -115,24 +111,8 @@ export function PullRequestsContent({ `${pullRequest.projectId}:${pullRequest.prNumber}`, }); - const handleAddToWorkspace = (pr: (typeof pullRequests)[number]) => { - const linkedPR: LinkedPR = { - prNumber: pr.prNumber, - title: pr.title, - url: pr.url, - state: normalizePRState(pr.state, pr.isDraft), - }; - resetDraft(); - selectProject(pr.projectId); - updateDraft({ hostId: pr.hostId, linkedPR }); - openModal(pr.projectId); - }; - - const handleOpenUrl = (url: string) => { - window.open(url, "_blank", "noopener,noreferrer"); - }; - const handleOpenPreview = (pr: (typeof pullRequests)[number]) => { + expandDetail(); navigate({ to: "/pull-requests/$prNumber", params: { prNumber: String(pr.prNumber) }, @@ -142,7 +122,7 @@ export function PullRequestsContent({ projects: serializeProjectFilters(projectFilters), author: authorFilter ?? undefined, review: reviewFilter ?? undefined, - state: includeClosed ? "all" : undefined, + state: mergedOnly ? "merged" : includeClosed ? "all" : undefined, }, }); }; @@ -178,6 +158,12 @@ export function PullRequestsContent({ } const isInitialLoad = isFetching && pullRequests.length === 0; + // Without a project to disambiguate, only the first row sharing this PR + // number gets marked selected — distinct repos can reuse the same number. + const firstMatchingPrIndex = + selectedPrProjectId == null + ? pullRequests.findIndex((pr) => pr.prNumber === selectedPrNumber) + : -1; const countLabel = isInitialLoad ? "Loading…" : totalCount === 0 @@ -212,17 +198,6 @@ export function PullRequestsContent({ } /> - {onCollapse && ( - - )}
@@ -245,15 +220,17 @@ export function PullRequestsContent({ ) : totalCount === 0 && !isFetching ? (
- {includeClosed - ? "No pull requests found." - : "No open pull requests."} + {mergedOnly + ? "No merged pull requests." + : includeClosed + ? "No pull requests found." + : "No open pull requests."}
) : ( -
+
{error instanceof Error && ( -
+
Some repositories could not be loaded: {error.message} @@ -262,74 +239,22 @@ export function PullRequestsContent({
)} - {pullRequests.map((pr) => { - const state = normalizePRState(pr.state, pr.isDraft); + {pullRequests.map((pr, index) => { const rowKey = `${pr.projectId}:${pr.prNumber}`; + const isSelected = + selectedPrNumber === pr.prNumber && + (selectedPrProjectId != null + ? selectedPrProjectId === pr.projectId + : firstMatchingPrIndex === index); + const repoSlug = repoSlugByProjectId.get(pr.projectId); return ( - // biome-ignore lint/a11y/useSemanticElements: row contains nested action buttons, so the outer element is a div with role/tabIndex -
handleOpenPreview(pr)} - onKeyDown={(e) => { - if (e.target !== e.currentTarget) return; - if (e.key === "Enter" || e.key === " ") { - e.preventDefault(); - handleOpenPreview(pr); - } - }} - role="button" - tabIndex={0} - > - - {projectTargets.length > 1 && ( - - {pr.projectName} - - )} - - #{pr.prNumber} - - - {pr.title} - - - {pr.authorLogin && ( - - {pr.authorLogin} - - )} -
- - -
-
+ /> ); })} void; projectFilters: string[]; onProjectFiltersChange: (projectIds: string[]) => void; + projectTargets: ProjectQueryTarget[]; authorFilter: string | null; onAuthorFilterChange: (author: string | null) => void; reviewFilter: PullRequestReviewFilter | null; onReviewFilterChange: (review: PullRequestReviewFilter | null) => void; - includeClosed: boolean; - onIncludeClosedChange: (includeClosed: boolean) => void; + stateFilter: PullRequestsStateFilter; + onStateFilterChange: (state: PullRequestsStateFilter) => void; } +const STATE_TABS: ReadonlyArray<{ + value: PullRequestsStateFilter; + label: string; +}> = [ + { value: "all", label: "All" }, + { value: "open", label: "Open" }, + { value: "merged", label: "Merged" }, +]; + export function PullRequestsTopBar({ searchQuery, onSearchChange, projectFilters, onProjectFiltersChange, + projectTargets, authorFilter, onAuthorFilterChange, reviewFilter, onReviewFilterChange, - includeClosed, - onIncludeClosedChange, + stateFilter, + onStateFilterChange, }: PullRequestsTopBarProps) { + const activeFilterCount = [ + projectFilters.length > 0, + !!authorFilter, + !!reviewFilter, + ].filter(Boolean).length; + return (
-
-
-
- Repository - -
-
-
- Author - -
-
-
- Reviews - -
-
- -
+
+ {STATE_TABS.map((tab) => ( + // biome-ignore lint/a11y/useSemanticElements: styled as a pill button, not a native radio input + + ))} {/* Window-drag leaf standing in for the hidden TopBar. */}
-
+
+ +
+
+
+
+ + + + + +
+ Repository + +
+
+ Author + +
+
+ Reviews + +
+
+
); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsTopBar/components/AuthorFilter/AuthorFilter.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsTopBar/components/AuthorFilter/AuthorFilter.tsx index bcd0c8ce61f..5ed3a752394 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsTopBar/components/AuthorFilter/AuthorFilter.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsTopBar/components/AuthorFilter/AuthorFilter.tsx @@ -1,44 +1,84 @@ +import { Avatar, AvatarFallback, AvatarImage } from "@superset/ui/avatar"; import { Button } from "@superset/ui/button"; -import { Input } from "@superset/ui/input"; +import { + Command, + CommandEmpty, + CommandGroup, + CommandInput, + CommandItem, + CommandList, +} from "@superset/ui/command"; import { Popover, PopoverContent, PopoverTrigger } from "@superset/ui/popover"; -import { type FormEvent, useId, useState } from "react"; -import { HiChevronDown, HiOutlineUserCircle } from "react-icons/hi2"; +import { useQuery } from "@tanstack/react-query"; +import { useMemo, useState } from "react"; +import { HiCheck, HiChevronDown, HiOutlineUserCircle } from "react-icons/hi2"; +import { getHostServiceClientByUrl } from "renderer/lib/host-service-client"; +import type { ProjectQueryTarget } from "renderer/routes/_authenticated/_dashboard/hooks/useProjectQueryTargets"; import { normalizeAuthorFilter } from "renderer/routes/_authenticated/_dashboard/pull-requests/utils/normalizeAuthorFilter"; interface AuthorFilterProps { value: string | null; onChange: (value: string | null) => void; + /** Scoped to a single repo, this drives a live contributor list instead + * of a bare text box — "all repositories" or multiple selected repos + * have no single contributor set to show. */ + projectTargets: ProjectQueryTarget[]; } -export function AuthorFilter({ value, onChange }: AuthorFilterProps) { +export function AuthorFilter({ + value, + onChange, + projectTargets, +}: AuthorFilterProps) { const [open, setOpen] = useState(false); - const [draft, setDraft] = useState(value ?? ""); - const [error, setError] = useState(null); - const inputId = useId(); - const errorId = useId(); + const [search, setSearch] = useState(""); const label = value ? `@${value}` : "All authors"; + const singleTarget = + projectTargets.length === 1 ? projectTargets[0] : undefined; + + const { + data: contributors, + isLoading, + error, + } = useQuery({ + queryKey: [ + "pullRequests", + "repoContributors", + singleTarget?.projectId, + singleTarget?.hostUrl, + ], + queryFn: async () => { + if (!singleTarget?.hostUrl) return []; + const client = getHostServiceClientByUrl(singleTarget.hostUrl); + return client.workspaceCreation.getRepoContributors.query({ + projectId: singleTarget.projectId, + }); + }, + enabled: !!singleTarget?.hostUrl, + staleTime: 5 * 60_000, + gcTime: 10 * 60_000, + }); + + const filtered = useMemo(() => { + const q = search.trim().replace(/^@/, "").toLowerCase(); + const list = contributors ?? []; + if (!q) return list; + return list.filter((c) => c.login.toLowerCase().includes(q)); + }, [contributors, search]); + + const normalizedSearch = normalizeAuthorFilter(search)?.toLowerCase() ?? null; + const showCustomOption = + !!normalizedSearch && + !filtered.some((c) => c.login.toLowerCase() === normalizedSearch); + const handleOpenChange = (nextOpen: boolean) => { setOpen(nextOpen); - setDraft(value ?? ""); - setError(null); + if (!nextOpen) setSearch(""); }; - const handleSubmit = (event: FormEvent) => { - event.preventDefault(); - const normalized = normalizeAuthorFilter(draft); - if (draft.trim() && !normalized) { - setError("Enter a valid GitHub username."); - return; - } - onChange(normalized); - setOpen(false); - }; - - const handleClear = () => { - onChange(null); - setDraft(""); - setError(null); + const handleSelect = (login: string | null) => { + onChange(login); setOpen(false); }; @@ -53,52 +93,83 @@ export function AuthorFilter({ value, onChange }: AuthorFilterProps) { className="h-8 max-w-44 gap-1.5 px-2 text-muted-foreground hover:text-foreground" > - {label} + {label} - -
-
- - { - setDraft(event.target.value); - setError(null); - }} - /> - {error && ( -

- {error} -

+ + + + + {singleTarget && isLoading && !contributors && ( +
+ Loading contributors… +
+ )} + {(!search || filtered.length > 0 || showCustomOption) && ( + + {!search && ( + handleSelect(null)}> + + All authors + {!value && ( + + )} + + )} + {filtered.map((contributor) => ( + handleSelect(contributor.login)} + > + + + + {contributor.login.slice(0, 1).toUpperCase()} + + + + {contributor.login} + + {value === contributor.login && ( + + )} + + ))} + {showCustomOption && normalizedSearch && ( + handleSelect(normalizedSearch)}> + + + Filter by @{normalizedSearch} + + + )} + + )} + {singleTarget && !isLoading && error && ( +
+ Couldn't load contributors — type a username instead. +
)} -
-
- - -
-
+ {!isLoading && + !error && + filtered.length === 0 && + !showCustomOption && + (!singleTarget || + (contributors && contributors.length === 0)) && ( + + {search ? "No authors found." : "No contributors found."} + + )} + +
); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsTopBar/components/ReviewFilter/ReviewFilter.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsTopBar/components/ReviewFilter/ReviewFilter.tsx index f2c114a4840..63aebb1233e 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsTopBar/components/ReviewFilter/ReviewFilter.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/components/PullRequestsView/components/PullRequestsTopBar/components/ReviewFilter/ReviewFilter.tsx @@ -37,7 +37,7 @@ export function ReviewFilter({ value, onChange }: ReviewFilterProps) { className="h-8 max-w-52 gap-1.5 px-2 text-muted-foreground hover:text-foreground" > - {label} + {label} diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/layout.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/layout.tsx index 7c5cb29c679..188aad95493 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/layout.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/layout.tsx @@ -1,4 +1,18 @@ -import { createFileRoute, Outlet } from "@tanstack/react-router"; +import { cn } from "@superset/ui/utils"; +import { createFileRoute, Outlet, useParams } from "@tanstack/react-router"; +import { useMemo } from "react"; +import { resolveProjectFilterParams } from "renderer/routes/_authenticated/_dashboard/components/ProjectFilter/project-filter-utils"; +import { parsePositiveIntegerParam } from "renderer/routes/_authenticated/_dashboard/utils/parsePositiveIntegerParam"; +import { ResizablePanel } from "renderer/screens/main/components/ResizablePanel"; +import { useWorkspaceSidebarStore } from "renderer/stores/workspace-sidebar-state"; +import { PullRequestListToggle } from "./components/PullRequestListToggle"; +import { PullRequestsView } from "./components/PullRequestsView"; +import { + DEFAULT_PULL_REQUESTS_LIST_WIDTH, + MAX_PULL_REQUESTS_LIST_WIDTH, + MIN_PULL_REQUESTS_LIST_WIDTH, + usePullRequestsSplitViewStore, +} from "./stores/pullRequestsSplitViewStore"; export type PullRequestsSearch = { search?: string; @@ -6,7 +20,7 @@ export type PullRequestsSearch = { projects?: string; author?: string; review?: string; - state?: "open" | "all"; + state?: "open" | "all" | "merged"; }; export const Route = createFileRoute( @@ -19,12 +33,104 @@ export const Route = createFileRoute( projects: typeof search.projects === "string" ? search.projects : undefined, author: typeof search.author === "string" ? search.author : undefined, review: typeof search.review === "string" ? search.review : undefined, - state: ["open", "all"].includes(search.state as string) + state: ["open", "all", "merged"].includes(search.state as string) ? (search.state as PullRequestsSearch["state"]) : undefined, }), }); +/** + * Split view, per Figma (SuperReviewSplit): the list stays mounted in a + * resizable left pane while the child route (index = empty state, + * $prNumber = detail) renders in the flexible right pane via ``. + * Selecting a different PR updates the right pane only — the list never + * unmounts, so scroll position and in-flight pagination survive. Either + * pane can be collapsed to give the other the full width; collapsing one + * always reveals the other, since hiding both would leave nothing on screen. + */ function PullRequestsLayout() { - return ; + const { search, project, projects, author, review, state } = + Route.useSearch(); + const params = useParams({ strict: false }) as { prNumber?: string }; + const selectedPrNumber = params.prNumber + ? parsePositiveIntegerParam(params.prNumber) + : null; + const isListCollapsed = usePullRequestsSplitViewStore( + (s) => s.isListCollapsed, + ); + const isDetailCollapsed = usePullRequestsSplitViewStore( + (s) => s.isDetailCollapsed, + ); + const listWidth = usePullRequestsSplitViewStore((s) => s.width); + const setListWidth = usePullRequestsSplitViewStore((s) => s.setWidth); + const isResizingList = usePullRequestsSplitViewStore((s) => s.isResizing); + const setIsResizingList = usePullRequestsSplitViewStore( + (s) => s.setIsResizing, + ); + const isAppSidebarCollapsed = useWorkspaceSidebarStore((s) => + s.isCollapsed(), + ); + // Stable identity: effects downstream key off this array. + const initialProjects = useMemo( + () => resolveProjectFilterParams(projects, project, undefined), + [projects, project], + ); + + const listContent = ( + + ); + + return ( +
+ {!isListCollapsed && ( + + setListWidth(DEFAULT_PULL_REQUESTS_LIST_WIDTH) + } + className={cn( + "flex min-h-0 flex-col bg-background", + isAppSidebarCollapsed && "rounded-tl-[8px]", + )} + > + {listContent} + + )} + {!isDetailCollapsed && ( +
+ {params.prNumber === undefined && ( +
+ +
+ )} + +
+ )} +
+ ); } diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/page.tsx b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/page.tsx index 02f1ba972af..6a99672e4d2 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/page.tsx +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/page.tsx @@ -1,31 +1,21 @@ import { createFileRoute } from "@tanstack/react-router"; -import { useMemo } from "react"; -import { resolveProjectFilterParams } from "renderer/routes/_authenticated/_dashboard/components/ProjectFilter/project-filter-utils"; -import { PullRequestsView } from "./components/PullRequestsView"; -import { Route as PullRequestsLayoutRoute } from "./layout"; +import { GoGitPullRequest } from "react-icons/go"; export const Route = createFileRoute( "/_authenticated/_dashboard/pull-requests/", )({ - component: PullRequestsPage, + component: PullRequestsIndexPage, }); -function PullRequestsPage() { - const { search, project, projects, author, review, state } = - PullRequestsLayoutRoute.useSearch(); - // Stable identity: effects downstream key off this array. - const initialProjects = useMemo( - () => resolveProjectFilterParams(projects, project, undefined), - [projects, project], - ); - +function PullRequestsIndexPage() { return ( - +
+
+ + + Select a pull request to preview it here. + +
+
); } diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.test.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.test.ts index 2b1caabbd32..248b023e420 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.test.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.test.ts @@ -33,6 +33,7 @@ describe("pullRequestsSearchFromFilters", () => { authorFilter: null, reviewFilter: null, includeClosed: false, + mergedOnly: false, }), ).toEqual({}); }); @@ -45,6 +46,7 @@ describe("pullRequestsSearchFromFilters", () => { authorFilter: "octocat", reviewFilter: "changes-requested", includeClosed: true, + mergedOnly: false, }), ).toEqual({ search: "remote host", @@ -54,6 +56,19 @@ describe("pullRequestsSearchFromFilters", () => { state: "all", }); }); + + test("mergedOnly wins over includeClosed in the serialized state", () => { + expect( + pullRequestsSearchFromFilters({ + search: "", + projectFilters: [], + authorFilter: null, + reviewFilter: null, + includeClosed: true, + mergedOnly: true, + }), + ).toEqual({ state: "merged" }); + }); }); describe("migratePullRequestsFilterState", () => { @@ -78,18 +93,21 @@ describe("migratePullRequestsFilterState", () => { authorFilter: "octocat author:someone-else", reviewFilter: "review:approved", includeClosed: "true", + mergedOnly: "true", }), ).toMatchObject({ projectFilters: ["project-1"], authorFilter: null, reviewFilter: null, includeClosed: false, + mergedOnly: false, }); expect(migratePullRequestsFilterState(null)).toMatchObject({ projectFilters: [], authorFilter: null, reviewFilter: null, includeClosed: false, + mergedOnly: false, }); }); }); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.ts index 76ff26d4e1c..707e9c4adac 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsFilterStore/pullRequestsFilterStore.ts @@ -17,16 +17,24 @@ interface PullRequestsFilterState { authorFilter: string | null; reviewFilter: PullRequestReviewFilter | null; includeClosed: boolean; + /** Narrows further to merged-only — independent of includeClosed, which + * the "is:merged" qualifier takes precedence over on the backend. */ + mergedOnly: boolean; setSearch: (search: string) => void; setProjectFilters: (projectFilters: string[]) => void; setAuthorFilter: (authorFilter: string | null) => void; setReviewFilter: (reviewFilter: PullRequestReviewFilter | null) => void; setIncludeClosed: (includeClosed: boolean) => void; + setMergedOnly: (mergedOnly: boolean) => void; } type PersistedPullRequestsFilterState = Pick< PullRequestsFilterState, - "projectFilters" | "authorFilter" | "reviewFilter" | "includeClosed" + | "projectFilters" + | "authorFilter" + | "reviewFilter" + | "includeClosed" + | "mergedOnly" >; export function migratePullRequestsFilterState( @@ -45,6 +53,7 @@ export function migratePullRequestsFilterState( authorFilter: normalizeAuthorFilter(state.authorFilter), reviewFilter: normalizePullRequestReviewFilter(state.reviewFilter), includeClosed: state.includeClosed === true, + mergedOnly: state.mergedOnly === true, }; } @@ -56,11 +65,10 @@ export const usePullRequestsFilterStore = create()( authorFilter: null, reviewFilter: null, includeClosed: false, + mergedOnly: false, setSearch: (search) => set({ search }), // Bail on equal content: views sync filters back through an effect, // so an always-fresh array here becomes an infinite update loop. - // Bail on equal content: views sync filters back through an effect, - // so an always-fresh array here becomes an infinite update loop. setProjectFilters: (projectFilters) => set((state) => { const next = normalizeProjectFilters(projectFilters); @@ -75,16 +83,18 @@ export const usePullRequestsFilterStore = create()( reviewFilter: normalizePullRequestReviewFilter(reviewFilter), }), setIncludeClosed: (includeClosed) => set({ includeClosed }), + setMergedOnly: (mergedOnly) => set({ mergedOnly }), }), { name: "pull-requests-filter-state", - version: 4, + version: 7, migrate: migratePullRequestsFilterState, partialize: (state) => ({ projectFilters: state.projectFilters, authorFilter: state.authorFilter, reviewFilter: state.reviewFilter, includeClosed: state.includeClosed, + mergedOnly: state.mergedOnly, }), }, ), @@ -96,6 +106,7 @@ interface PullRequestsFilters { authorFilter: string | null; reviewFilter: PullRequestReviewFilter | null; includeClosed: boolean; + mergedOnly: boolean; } export function pullRequestsSearchFromFilters( @@ -107,6 +118,7 @@ export function pullRequestsSearchFromFilters( if (projects) search.projects = projects; if (filters.authorFilter) search.author = filters.authorFilter; if (filters.reviewFilter) search.review = filters.reviewFilter; - if (filters.includeClosed) search.state = "all"; + if (filters.mergedOnly) search.state = "merged"; + else if (filters.includeClosed) search.state = "all"; return search; } diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsSplitViewStore/index.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsSplitViewStore/index.ts new file mode 100644 index 00000000000..91db3be0ee4 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsSplitViewStore/index.ts @@ -0,0 +1,6 @@ +export { + DEFAULT_PULL_REQUESTS_LIST_WIDTH, + MAX_PULL_REQUESTS_LIST_WIDTH, + MIN_PULL_REQUESTS_LIST_WIDTH, + usePullRequestsSplitViewStore, +} from "./pullRequestsSplitViewStore"; diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsSplitViewStore/pullRequestsSplitViewStore.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsSplitViewStore/pullRequestsSplitViewStore.ts new file mode 100644 index 00000000000..13a2686a558 --- /dev/null +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/pull-requests/stores/pullRequestsSplitViewStore/pullRequestsSplitViewStore.ts @@ -0,0 +1,75 @@ +import { create } from "zustand"; +import { persist } from "zustand/middleware"; + +export const DEFAULT_PULL_REQUESTS_LIST_WIDTH = 420; +export const MIN_PULL_REQUESTS_LIST_WIDTH = 300; +export const MAX_PULL_REQUESTS_LIST_WIDTH = 720; + +interface PullRequestsSplitViewState { + /** Hides the list pane — the detail pane takes the full width. */ + isListCollapsed: boolean; + /** Hides the detail pane — the list pane takes the full width. Mutually + * exclusive with isListCollapsed: hiding one pane always reveals the + * other, since hiding both would leave nothing on screen. */ + isDetailCollapsed: boolean; + width: number; + isResizing: boolean; + toggleListCollapsed: () => void; + toggleDetailCollapsed: () => void; + /** Explicitly expands the detail pane, e.g. when selecting a pull + * request — unlike toggleDetailCollapsed, this never re-collapses it. */ + expandDetail: () => void; + setWidth: (width: number) => void; + setIsResizing: (isResizing: boolean) => void; +} + +export const usePullRequestsSplitViewStore = + create()( + persist( + (set) => ({ + isListCollapsed: false, + isDetailCollapsed: false, + width: DEFAULT_PULL_REQUESTS_LIST_WIDTH, + isResizing: false, + toggleListCollapsed: () => + set((state) => { + const isListCollapsed = !state.isListCollapsed; + return { + isListCollapsed, + isDetailCollapsed: isListCollapsed + ? false + : state.isDetailCollapsed, + }; + }), + toggleDetailCollapsed: () => + set((state) => { + const isDetailCollapsed = !state.isDetailCollapsed; + return { + isDetailCollapsed, + isListCollapsed: isDetailCollapsed + ? false + : state.isListCollapsed, + }; + }), + expandDetail: () => set({ isDetailCollapsed: false }), + setWidth: (width) => + set({ + width: Math.max( + MIN_PULL_REQUESTS_LIST_WIDTH, + Math.min(MAX_PULL_REQUESTS_LIST_WIDTH, width), + ), + }), + setIsResizing: (isResizing) => set({ isResizing }), + }), + { + name: "pull-requests-split-view-state", + version: 2, + partialize: (state) => ({ + isListCollapsed: state.isListCollapsed, + isDetailCollapsed: state.isDetailCollapsed, + width: state.width, + // isResizing intentionally excluded - ephemeral UI state + }), + }, + ), + ); diff --git a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.test.ts b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.test.ts index 1b4fe1901cd..ca50f05558f 100644 --- a/apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.test.ts +++ b/apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/TasksView.test.ts @@ -80,12 +80,12 @@ describe("Tasks and pull requests navigation", () => { }); test("gives pull requests an independent route and view", () => { - const pageSource = readComponent("../../../pull-requests/page.tsx"); + const layoutSource = readComponent("../../../pull-requests/layout.tsx"); const viewSource = readComponent( "../../../pull-requests/components/PullRequestsView/PullRequestsView.tsx", ); - expect(pageSource).toContain("; + return ; } if (state === "closed") { - return ; + return ( + + ); } if (state === "queued") { - return ; + return ( + + ); } // open or draft - return ; + return ( + + ); } diff --git a/apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx b/apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx index eb5764a5353..63dd31bce27 100644 --- a/apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx +++ b/apps/desktop/src/renderer/screens/main/components/ResizablePanel/ResizablePanel.tsx @@ -29,6 +29,14 @@ interface ResizablePanelProps { clampWidth?: boolean; /** Callback when the resize handle is double-clicked */ onDoubleClickHandle?: () => void; + /** + * Renders as a flexible, full-width panel with no resize handle instead + * of a fixed-width one — for a sibling pane that can be collapsed, so + * this panel keeps the same element type (and its children stay mounted) + * across both states rather than swapping between this and a plain div. + * @default false + */ + disabled?: boolean; } export function ResizablePanel({ @@ -43,6 +51,7 @@ export function ResizablePanel({ className, clampWidth = true, onDoubleClickHandle, + disabled = false, }: ResizablePanelProps) { const startXRef = useRef(0); const startWidthRef = useRef(0); @@ -124,33 +133,36 @@ export function ResizablePanel({ return (
{children} - {/* biome-ignore lint/a11y/useSemanticElements:
is not appropriate for interactive resize handles */} -
+ {!disabled && ( + // biome-ignore lint/a11y/useSemanticElements:
is not appropriate for interactive resize handles +
+ )}
); } diff --git a/apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx b/apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx index ae0fb79cc01..a779ae75b36 100644 --- a/apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx +++ b/apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx @@ -58,6 +58,7 @@ export function WorkspaceSidebarHeader({ authorFilter: lastPullRequestsAuthorFilter, reviewFilter: lastPullRequestsReviewFilter, includeClosed: lastPullRequestsIncludeClosed, + mergedOnly: lastPullRequestsMergedOnly, } = usePullRequestsFilterStore(); const handleTasksClick = () => { @@ -87,6 +88,7 @@ export function WorkspaceSidebarHeader({ authorFilter: lastPullRequestsAuthorFilter, reviewFilter: lastPullRequestsReviewFilter, includeClosed: lastPullRequestsIncludeClosed, + mergedOnly: lastPullRequestsMergedOnly, }), }); }); diff --git a/packages/host-service/src/trpc/router/github/github.ts b/packages/host-service/src/trpc/router/github/github.ts index dc44654e098..687221909b6 100644 --- a/packages/host-service/src/trpc/router/github/github.ts +++ b/packages/host-service/src/trpc/router/github/github.ts @@ -462,7 +462,7 @@ interface PullRequestDetailQuery { * non-500 code (500s page Sentry) and GitHub's own wording, which is the only * text that says which of them happened. */ -function mergeRejectionError(error: unknown): TRPCError { +export function mergeRejectionError(error: unknown): TRPCError { const status = typeof error === "object" && error !== null && "status" in error ? Number((error as { status: unknown }).status) diff --git a/packages/host-service/src/trpc/router/pull-requests/procedures/merge.ts b/packages/host-service/src/trpc/router/pull-requests/procedures/merge.ts new file mode 100644 index 00000000000..4120cd8b22e --- /dev/null +++ b/packages/host-service/src/trpc/router/pull-requests/procedures/merge.ts @@ -0,0 +1,33 @@ +import { z } from "zod"; +import { protectedProcedure } from "../../../index"; +import { mergeRejectionError } from "../../github/github"; +import { resolveGithubRepo } from "../../workspace-creation/shared/project-helpers"; + +const mergeInputSchema = z.object({ + projectId: z.string(), + prNumber: z.number().int().positive(), + mergeMethod: z.enum(["merge", "squash", "rebase"]).default("merge"), +}); + +/** + * Project-scoped merge: resolves the repo live via resolveGithubRepo, same + * as setState, instead of trusting a project's cached repoOwner/repoName — + * those go stale if the remote is renamed or re-pointed after setup. + */ +export const mergePR = protectedProcedure + .input(mergeInputSchema) + .mutation(async ({ ctx, input }) => { + const repo = await resolveGithubRepo(ctx, input.projectId); + const octokit = await ctx.github(); + try { + const { data } = await octokit.pulls.merge({ + owner: repo.owner, + repo: repo.name, + pull_number: input.prNumber, + merge_method: input.mergeMethod, + }); + return data; + } catch (error) { + throw mergeRejectionError(error); + } + }); diff --git a/packages/host-service/src/trpc/router/pull-requests/procedures/set-state.ts b/packages/host-service/src/trpc/router/pull-requests/procedures/set-state.ts new file mode 100644 index 00000000000..324b375161d --- /dev/null +++ b/packages/host-service/src/trpc/router/pull-requests/procedures/set-state.ts @@ -0,0 +1,35 @@ +import { TRPCError } from "@trpc/server"; +import { z } from "zod"; +import { protectedProcedure } from "../../../index"; +import { resolveGithubRepo } from "../../workspace-creation/shared/project-helpers"; +import { execGh } from "../../workspace-creation/utils/exec-gh"; + +const setStateInputSchema = z.object({ + projectId: z.string(), + prNumber: z.number().int().positive(), + // Only open/closed — GitHub has no CLI verb to un-merge a PR, so a + // merged state isn't reachable through this mutation. + state: z.enum(["open", "closed"]), +}); + +export const setState = protectedProcedure + .input(setStateInputSchema) + .mutation(async ({ ctx, input }) => { + const repo = await resolveGithubRepo(ctx, input.projectId); + try { + await execGh([ + "pr", + input.state === "closed" ? "close" : "reopen", + String(input.prNumber), + "--repo", + `${repo.owner}/${repo.name}`, + ]); + return { ok: true }; + } catch (err) { + const verb = input.state === "closed" ? "close" : "reopen"; + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `Failed to ${verb} PR #${input.prNumber}: ${err instanceof Error ? err.message : String(err)}`, + }); + } + }); diff --git a/packages/host-service/src/trpc/router/pull-requests/pull-requests.ts b/packages/host-service/src/trpc/router/pull-requests/pull-requests.ts index ba9883b3e6d..611dd1f323c 100644 --- a/packages/host-service/src/trpc/router/pull-requests/pull-requests.ts +++ b/packages/host-service/src/trpc/router/pull-requests/pull-requests.ts @@ -1,6 +1,8 @@ import { z } from "zod"; import { protectedProcedure, router } from "../../index"; import { getContent } from "./procedures/get-content"; +import { mergePR } from "./procedures/merge"; +import { setState } from "./procedures/set-state"; export const pullRequestsRouter = router({ getByWorkspaces: protectedProcedure @@ -39,4 +41,6 @@ export const pullRequestsRouter = router({ return { ok: true }; }), getContent, + setState, + mergePR, }); diff --git a/packages/host-service/src/trpc/router/workspace-creation/procedures/get-repo-contributors.ts b/packages/host-service/src/trpc/router/workspace-creation/procedures/get-repo-contributors.ts new file mode 100644 index 00000000000..85b85217b0a --- /dev/null +++ b/packages/host-service/src/trpc/router/workspace-creation/procedures/get-repo-contributors.ts @@ -0,0 +1,99 @@ +import { TRPCError } from "@trpc/server"; +import { z } from "zod"; +import { protectedProcedure } from "../../../index"; +import { resolveGithubRepo } from "../shared/project-helpers"; +import { execGh } from "../utils/exec-gh"; + +const getRepoContributorsInputSchema = z.object({ + projectId: z.string(), +}); + +const ghContributorSchema = z.object({ + login: z.string(), + type: z.string().optional(), +}); + +export interface RepoContributor { + login: string; +} + +// The Author filter only needs a quick-pick list, not a complete roster — +// GitHub's contributors endpoint is sorted by commit count descending, so +// one page covers everyone anyone would actually look for. +const CONTRIBUTORS_PAGE_SIZE = 100; + +// Browsing the filter popover re-opens this list repeatedly; the +// contributor set for a repo changes rarely, so cache it like +// getContent caches PR bodies. +const REPO_CONTRIBUTORS_CACHE_TTL_MS = 5 * 60_000; +const repoContributorsCache = new Map< + string, + { promise: Promise; fetchedAt: number } +>(); + +export const getRepoContributors = protectedProcedure + .input(getRepoContributorsInputSchema) + .query(async ({ ctx, input }): Promise => { + const repo = await resolveGithubRepo(ctx, input.projectId); + const cacheKey = `${repo.owner.toLowerCase()}/${repo.name.toLowerCase()}`; + const cached = repoContributorsCache.get(cacheKey); + if ( + cached && + Date.now() - cached.fetchedAt < REPO_CONTRIBUTORS_CACHE_TTL_MS + ) { + return cached.promise; + } + + const fetchedAt = Date.now(); + const promise = (async (): Promise => { + // gh-first uses the user's local `gh auth login`; falls back to + // Octokit when gh is missing, unauthed, or errors. + try { + const raw = await execGh([ + "api", + `repos/${repo.owner}/${repo.name}/contributors`, + "-X", + "GET", + "-f", + `per_page=${CONTRIBUTORS_PAGE_SIZE}`, + ]); + // A repo with zero commits answers 204 No Content; execGh turns + // an empty body into {}, not [], since it can't tell the two apart. + const contributors = Array.isArray(raw) + ? z.array(ghContributorSchema).parse(raw) + : []; + return contributors + .filter((c) => c.type !== "Bot") + .map((c) => ({ login: c.login })); + } catch (ghErr) { + console.warn( + "[workspaceCreation.getRepoContributors] gh path failed; falling back to Octokit", + ghErr, + ); + } + + try { + const octokit = await ctx.github(); + const { data } = await octokit.repos.listContributors({ + owner: repo.owner, + repo: repo.name, + per_page: CONTRIBUTORS_PAGE_SIZE, + }); + return data + .filter((c) => c.type !== "Bot" && c.login) + .map((c) => ({ login: c.login as string })); + } catch (octokitErr) { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `Failed to fetch contributors for ${repo.owner}/${repo.name}: ${octokitErr instanceof Error ? octokitErr.message : String(octokitErr)}`, + }); + } + })(); + promise.catch(() => { + if (repoContributorsCache.get(cacheKey)?.promise === promise) { + repoContributorsCache.delete(cacheKey); + } + }); + repoContributorsCache.set(cacheKey, { promise, fetchedAt }); + return promise; + }); diff --git a/packages/host-service/src/trpc/router/workspace-creation/procedures/index.ts b/packages/host-service/src/trpc/router/workspace-creation/procedures/index.ts index b43d90230e4..f87e47cd231 100644 --- a/packages/host-service/src/trpc/router/workspace-creation/procedures/index.ts +++ b/packages/host-service/src/trpc/router/workspace-creation/procedures/index.ts @@ -1,4 +1,5 @@ export { adopt } from "./adopt"; +export { getRepoContributors } from "./get-repo-contributors"; export { listProjectWorktrees } from "./list-project-worktrees"; export { searchBranches } from "./search-branches"; export { searchGitHubIssues } from "./search-github-issues"; diff --git a/packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts b/packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts index 8d8e7394816..28c7765b46b 100644 --- a/packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts +++ b/packages/host-service/src/trpc/router/workspace-creation/procedures/search-pull-requests.ts @@ -45,6 +45,11 @@ interface PullRequestResult { updatedAt: string | null; checks: PullRequestCheck[]; checksStatus: ChecksStatus; + /** null until enriched — search-listed rows start unknown, direct + * lookups and the checks-enrichment pass fill these in. */ + additions: number | null; + deletions: number | null; + headRefName: string | null; } export interface PullRequestsPage { @@ -100,10 +105,44 @@ const REVIEW_DECISION_BY_FILTER: Record< // GitHub caps Search API results at 1000; paging past that returns 422. const GITHUB_SEARCH_RESULT_LIMIT = 1_000; -const searchPullRequestsInputSchema = githubSearchInputSchema.extend({ - author: githubAuthorSchema.optional(), - review: pullRequestReviewFilterSchema.optional(), -}); +// No combined "needs-review OR reviewed" value: GitHub's search API rejects +// qualifier-level OR (422 either with or without parens), so a "Reviewing" +// grouping has to run as two separate queries, not one. +const viewerRelationshipSchema = z.enum([ + "needs-review", + "reviewed", + "authored", +]); +type ViewerRelationship = z.infer; + +// Bypasses the free-typed `author`/`review` filters (and their username +// validation, which would mangle the literal "@me" token) for the grouped +// "my work" list sections — each maps to one fixed, unambiguous qualifier. +const VIEWER_RELATIONSHIP_QUALIFIERS: Record = { + "needs-review": "user-review-requested:@me", + reviewed: "reviewed-by:@me -user-review-requested:@me", + authored: "author:@me", +}; + +const searchPullRequestsInputSchema = githubSearchInputSchema + .extend({ + author: githubAuthorSchema.optional(), + review: pullRequestReviewFilterSchema.optional(), + // mergedOnly always wins over includeClosed — see the qualifiers + // construction below — so only pass this when includeClosed is true + // (or omitted); passing mergedOnly with includeClosed: false is + // pointless but not actively wrong. + mergedOnly: z.boolean().optional(), + // Mutually exclusive with author/review — see VIEWER_RELATIONSHIP_QUALIFIERS. + viewerRelationship: viewerRelationshipSchema.optional(), + }) + .refine( + (input) => !input.viewerRelationship || (!input.author && !input.review), + { + message: "viewerRelationship cannot be combined with author or review", + path: ["viewerRelationship"], + }, + ); function emptyPullRequestsPage(page: number): PullRequestsPage { return { @@ -172,10 +211,13 @@ const ghPrViewSchema = z.object({ reviewDecision: z.string().nullable().optional(), reviewRequests: z.array(ghReviewRequestSchema).nullable().optional(), latestReviews: z.array(ghLatestReviewSchema).nullable().optional(), + additions: z.number().optional(), + deletions: z.number().optional(), + headRefName: z.string().optional(), }); const PR_VIEW_FIELDS = - "number,title,url,state,isDraft,author,mergedAt,updatedAt,statusCheckRollup,reviewDecision,reviewRequests,latestReviews"; + "number,title,url,state,isDraft,author,mergedAt,updatedAt,statusCheckRollup,reviewDecision,reviewRequests,latestReviews,additions,deletions,headRefName"; interface GhDirectLookupReview { decision: string | null; @@ -217,6 +259,9 @@ async function ghDirectLookup( updatedAt: pr.updatedAt ?? null, checks, checksStatus, + additions: pr.additions ?? null, + deletions: pr.deletions ?? null, + headRefName: pr.headRefName ?? null, }, review: { decision: pr.reviewDecision || null, @@ -288,8 +333,44 @@ async function ghDirectLookupMatchesReviewFilter( } /** - * Direct-lookup a PR in one repo and apply the author/review filters. - * Returns null when the PR exists but doesn't match the filters. + * Mirrors VIEWER_RELATIONSHIP_QUALIFIERS for a single direct-looked-up PR: + * schema validation guarantees this never runs alongside author/review, so + * it doesn't need to compose with ghDirectLookupMatchesReviewFilter's result. + */ +async function ghDirectLookupMatchesViewerRelationship( + execGh: ExecGh, + repo: ResolvedGithubRepo, + pullRequest: PullRequestResult, + review: GhDirectLookupReview, + relationship: ViewerRelationship, +): Promise { + if (relationship === "authored") { + const viewerRaw = await execGh(["api", "user"], { + cwd: repo.repoPath ?? undefined, + }); + const viewerLogin = ghViewerSchema.parse(viewerRaw).login.toLowerCase(); + return pullRequest.authorLogin?.toLowerCase() === viewerLogin; + } + const viewerRequested = await ghDirectLookupMatchesReviewFilter( + execGh, + repo, + review, + "review-requested", + ); + if (relationship === "needs-review") return viewerRequested; + const reviewedByViewer = await ghDirectLookupMatchesReviewFilter( + execGh, + repo, + review, + "reviewed-by-me", + ); + return reviewedByViewer && !viewerRequested; +} + +/** + * Direct-lookup a PR in one repo and apply the active filters. Returns null + * when the PR exists but doesn't match — same contract as the search path, + * so a bare "#N" lookup can't leak a PR into the wrong tab/grouping. */ async function ghDirectLookupRow( execGh: ExecGh, @@ -297,12 +378,15 @@ async function ghDirectLookupRow( prNumber: number, author: string | undefined, reviewFilter: PullRequestReviewFilter | undefined, + mergedOnly: boolean | undefined, + viewerRelationship: ViewerRelationship | undefined, ): Promise { const { pullRequest, review } = await ghDirectLookup( execGh, target, prNumber, ); + if (mergedOnly && pullRequest.state !== "merged") return null; if (!matchesAuthor(pullRequest.authorLogin, author)) return null; if ( reviewFilter && @@ -315,6 +399,18 @@ async function ghDirectLookupRow( ) { return null; } + if ( + viewerRelationship && + !(await ghDirectLookupMatchesViewerRelationship( + execGh, + target.repo, + pullRequest, + review, + viewerRelationship, + )) + ) { + return null; + } return pullRequest; } @@ -400,9 +496,38 @@ async function octokitDirectLookupMatchesReviewFilter( return false; } +/** + * Octokit twin of {@link ghDirectLookupMatchesViewerRelationship}. + */ +async function octokitDirectLookupMatchesViewerRelationship( + octokit: Octokit, + repo: ResolvedGithubRepo, + pr: OctokitPullRequest, + relationship: ViewerRelationship, +): Promise { + if (relationship === "authored") { + const { data: viewer } = await octokit.users.getAuthenticated(); + return (pr.user?.login ?? "").toLowerCase() === viewer.login.toLowerCase(); + } + const viewerRequested = await octokitDirectLookupMatchesReviewFilter( + octokit, + repo, + pr, + "review-requested", + ); + if (relationship === "needs-review") return viewerRequested; + const reviewedByViewer = await octokitDirectLookupMatchesReviewFilter( + octokit, + repo, + pr, + "reviewed-by-me", + ); + return reviewedByViewer && !viewerRequested; +} + /** * Octokit twin of {@link ghDirectLookupRow}: direct-lookup one repo, apply - * author/review filters, enrich checks best-effort. Null = filtered out or + * the active filters, enrich checks best-effort. Null = filtered out or * PR not found/inaccessible. */ async function octokitDirectLookupRow( @@ -411,6 +536,8 @@ async function octokitDirectLookupRow( prNumber: number, author: string | undefined, reviewFilter: PullRequestReviewFilter | undefined, + mergedOnly: boolean | undefined, + viewerRelationship: ViewerRelationship | undefined, ): Promise { const { repo } = target; const response = await octokit.pulls @@ -427,6 +554,8 @@ async function octokitDirectLookupRow( }); if (!response) return null; const pr = response.data; + const state = normalizePullRequestState(pr.state, pr.merged_at); + if (mergedOnly && state !== "merged") return null; if (!matchesAuthor(pr.user?.login ?? null, author)) return null; if ( reviewFilter && @@ -439,7 +568,17 @@ async function octokitDirectLookupRow( ) { return null; } - const state = normalizePullRequestState(pr.state, pr.merged_at); + if ( + viewerRelationship && + !(await octokitDirectLookupMatchesViewerRelationship( + octokit, + repo, + pr, + viewerRelationship, + )) + ) { + return null; + } let checks: PullRequestCheck[] = []; let checksStatus: ChecksStatus = "none"; try { @@ -466,6 +605,9 @@ async function octokitDirectLookupRow( updatedAt: pr.updated_at ?? null, checks, checksStatus, + additions: pr.additions ?? null, + deletions: pr.deletions ?? null, + headRefName: pr.head?.ref ?? null, }; } @@ -540,6 +682,9 @@ async function ghApiSearchPullRequests( updatedAt: item.updated_at ?? null, checks: [], checksStatus: "none", + additions: null, + deletions: null, + headRefName: null, }, ]; }); @@ -586,6 +731,9 @@ async function octokitSearchPullRequests( updatedAt: item.updated_at ?? null, checks: [], checksStatus: "none", + additions: null, + deletions: null, + headRefName: null, }, ]; }); @@ -601,6 +749,9 @@ const checksGraphqlDataSchema = z.object({ z .object({ number: z.number(), + additions: z.number().optional(), + deletions: z.number().optional(), + headRefName: z.string().optional(), statusCheckRollup: z .object({ contexts: z.object({ @@ -624,16 +775,28 @@ type RunChecksGraphqlQuery = ( variables: Record, ) => Promise; +type PullRequestGraphqlDetails = Pick< + PullRequestResult, + "additions" | "deletions" | "headRefName" +>; + async function getPullRequestChecksViaGraphql( runQuery: RunChecksGraphqlQuery, repo: { owner: string; name: string }, pullRequestNumbers: number[], -): Promise>> { +): Promise< + Map< + number, + Pick & + PullRequestGraphqlDetails + > +> { if (pullRequestNumbers.length === 0) return new Map(); const contextsByPullRequest = new Map< number, z.infer[] >(); + const detailsByPullRequest = new Map(); let cursors = new Map( pullRequestNumbers.map((number) => [number, null]), ); @@ -648,6 +811,9 @@ async function getPullRequestChecksViaGraphql( .map( ([number, cursor]) => `pr${number}:pullRequest(number:${number}) { number + additions + deletions + headRefName statusCheckRollup { contexts(first: 100${cursor ? `, after: $cursor${number}` : ""}) { pageInfo { hasNextPage endCursor } @@ -702,6 +868,11 @@ async function getPullRequestChecksViaGraphql( ) ?? []; const existing = contextsByPullRequest.get(pullRequest.number) ?? []; contextsByPullRequest.set(pullRequest.number, [...existing, ...contexts]); + detailsByPullRequest.set(pullRequest.number, { + additions: pullRequest.additions ?? null, + deletions: pullRequest.deletions ?? null, + headRefName: pullRequest.headRefName ?? null, + }); const pageInfo = pullRequest.statusCheckRollup?.contexts.pageInfo; if (pageInfo?.hasNextPage) { if (!pageInfo.endCursor) { @@ -724,7 +895,12 @@ async function getPullRequestChecksViaGraphql( pullRequestNumbers.map((pullRequestNumber) => { const contexts = contextsByPullRequest.get(pullRequestNumber) ?? []; const { checks, checksStatus } = normalizePullRequestChecks(contexts); - return [pullRequestNumber, { checks, checksStatus }] as const; + const details = detailsByPullRequest.get(pullRequestNumber) ?? { + additions: null, + deletions: null, + headRefName: null, + }; + return [pullRequestNumber, { checks, checksStatus, ...details }] as const; }), ); } @@ -733,7 +909,7 @@ async function ghGetPullRequestChecks( execGh: ExecGh, repo: ResolvedGithubRepo, pullRequestNumbers: number[], -): Promise>> { +): Promise> { return getPullRequestChecksViaGraphql( (query, variables) => execGh( @@ -754,7 +930,10 @@ async function ghGetPullRequestChecks( ); } -type PullRequestChecksInfo = Pick; +type PullRequestChecksInfo = Pick< + PullRequestResult, + "checks" | "checksStatus" | "additions" | "deletions" | "headRefName" +>; /** * Enrich a merged page with checks: one GraphQL batch per repo that has @@ -840,12 +1019,15 @@ export const searchPullRequests = protectedProcedure normalizedTargets[0]?.normalized.query ?? "", input.author ? `author:${input.author}` : "", input.review ? REVIEW_QUERY_BY_FILTER[input.review] : "", + input.viewerRelationship + ? VIEWER_RELATIONSHIP_QUALIFIERS[input.viewerRelationship] + : "", ] .filter(Boolean) .join(" "); const qualifiers = [ "is:pr", - input.includeClosed ? "" : "is:open", + input.mergedOnly ? "is:merged" : input.includeClosed ? "" : "is:open", effectiveQuery, ] .filter(Boolean) @@ -863,6 +1045,8 @@ export const searchPullRequests = protectedProcedure lookupNumber, input.author, input.review, + input.mergedOnly, + input.viewerRelationship, ); if (!pullRequest) return emptyPullRequestsPage(page); return { @@ -882,6 +1066,8 @@ export const searchPullRequests = protectedProcedure lookupNumber, input.author, input.review, + input.mergedOnly, + input.viewerRelationship, ), ), ); @@ -954,6 +1140,8 @@ export const searchPullRequests = protectedProcedure lookupNumber, input.author, input.review, + input.mergedOnly, + input.viewerRelationship, ); if (!pullRequest) return emptyPullRequestsPage(page); return { @@ -971,6 +1159,8 @@ export const searchPullRequests = protectedProcedure lookupNumber, input.author, input.review, + input.mergedOnly, + input.viewerRelationship, ), ), ); diff --git a/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts b/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts index d8ef18398d1..756df0980b3 100644 --- a/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts +++ b/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts @@ -1,6 +1,7 @@ import { router } from "../../index"; import { adopt, + getRepoContributors, listProjectWorktrees, searchBranches, searchGitHubIssues, @@ -11,6 +12,7 @@ import { export const workspaceCreationRouter = router({ searchBranches, adopt, + getRepoContributors, listProjectWorktrees, searchGitHubIssues, searchPullRequests,