diff --git a/packages/core/src/git/router-schemas.ts b/packages/core/src/git/router-schemas.ts index 7feadcce0..dc9e03780 100644 --- a/packages/core/src/git/router-schemas.ts +++ b/packages/core/src/git/router-schemas.ts @@ -352,10 +352,18 @@ export const getPrChangedFilesInput = z.object({ export const getPrChangedFilesOutput = z.array(changedFileSchema); // getPrDiffStatsBatch schemas +// +// Beyond the diff numbers, the batch also carries the PR's live status +// (`state`/`merged`/`draft`) so list cards can render it from the single +// batched GraphQL request instead of firing one REST call per visible PR. export const prDiffStatsSchema = z.object({ additions: z.number(), deletions: z.number(), changedFiles: z.number(), + /** Lowercased GitHub PR state. */ + state: z.enum(["open", "closed", "merged"]), + merged: z.boolean(), + draft: z.boolean(), }); export type PrDiffStats = z.infer; diff --git a/packages/ui/src/features/git-interaction/usePrActions.ts b/packages/ui/src/features/git-interaction/usePrActions.ts index e26a248bd..bcb9fbab6 100644 --- a/packages/ui/src/features/git-interaction/usePrActions.ts +++ b/packages/ui/src/features/git-interaction/usePrActions.ts @@ -20,6 +20,13 @@ export function usePrActions(prUrl: string | null) { trpc.git.getPrDetailsByUrl.queryKey({ prUrl: variables.prUrl }), getOptimisticPrState(variables.action), ); + // The inbox Pulls list reads PR status from the batched + // `getPrDiffStatsBatch` query (separate cache, 5-min staleTime), so + // patching the detail cache above isn't enough — invalidate the batch + // so the list badge reflects the new state on next view. + void queryClient.invalidateQueries( + trpc.git.getPrDiffStatsBatch.pathFilter(), + ); } else { toast.error("Failed to update PR", { description: data.message }); } diff --git a/packages/ui/src/features/inbox/components/PullRequestCard.tsx b/packages/ui/src/features/inbox/components/PullRequestCard.tsx index e0efc16b9..8a1c21cf0 100644 --- a/packages/ui/src/features/inbox/components/PullRequestCard.tsx +++ b/packages/ui/src/features/inbox/components/PullRequestCard.tsx @@ -14,6 +14,7 @@ import { InboxCardTitle } from "@posthog/ui/features/inbox/components/InboxCardT import { PrDiffStats } from "@posthog/ui/features/inbox/components/PrDiffStats"; import { PriorityMonogram } from "@posthog/ui/features/inbox/components/PriorityMonogram"; import { SuggestedReviewerAvatarStack } from "@posthog/ui/features/inbox/components/SuggestedReviewerAvatarStack"; +import { ReportImplementationPrLink } from "@posthog/ui/features/inbox/components/utils/ReportImplementationPrLink"; import { useInboxReportDetailPrefetch } from "@posthog/ui/features/inbox/hooks/useInboxReportDetailPrefetch"; import { useInboxReportArtefacts } from "@posthog/ui/features/inbox/hooks/useInboxReports"; import { Button as UiButton } from "@posthog/ui/primitives/Button"; @@ -121,10 +122,16 @@ export function PullRequestCard({ > {report.implementation_pr_url && ( - + + + + )} void; } -function parseGitHubPrReference(prUrl: string): { - reference: string; - prNumber: string; -} { - try { - const parsed = new URL(prUrl); - const match = parsed.pathname.match( - /^\/([^/]+)\/([^/]+)\/pull\/(\d+)(?:$|[/?#])/, - ); - if (match) { - return { - reference: `${match[1]}/${match[2]}#${match[3]}`, - prNumber: `#${match[3]}`, - }; - } - } catch { - // Fall through to regex fallback for non-URL-safe inputs - } - - const prMatch = prUrl.match(/\/pull\/(\d+)(?:$|[/?#])/); - const prNumber = prMatch ? `#${prMatch[1]}` : "PR"; - return { - reference: prUrl, - prNumber, - }; -} +/** The only states we render a badge for; anything else is treated as unknown. */ +const KNOWN_PR_STATES = new Set(["open", "closed", "merged"]); export function ReportImplementationPrLink({ prUrl, size = "sm", onLinkClick, }: ReportImplementationPrLinkProps) { - const { - meta: { state, merged, draft, isLoading }, - } = usePrDetails(prUrl); + // On list surfaces the surrounding `PrDiffStatsBatchContext` already carries + // this PR's status from the single batched request, so read it from there and + // skip the per-PR query. Fall back to `usePrDetails` only on the standalone + // detail view, where no batch provider is mounted. + const batchEntry = usePrDiffStatsFromBatch(prUrl); + const fallback = usePrDetails(batchEntry.hasBatch ? null : prUrl); + + const state = batchEntry.hasBatch + ? (batchEntry.stats?.state ?? null) + : fallback.meta.state; + const merged = batchEntry.hasBatch + ? (batchEntry.stats?.merged ?? false) + : fallback.meta.merged; + const draft = batchEntry.hasBatch + ? (batchEntry.stats?.draft ?? false) + : fallback.meta.draft; + const isLoading = batchEntry.hasBatch + ? batchEntry.isLoading && !batchEntry.stats + : fallback.meta.isLoading; + + // Only render for a canonical GitHub PR URL. This both keeps the badge in + // sync with the gated "Open in GitHub" action and avoids turning an arbitrary + // (possibly unsafe-scheme) string into a clickable external link. + const prRef = parsePrUrl(prUrl); + if (!prRef) return null; + + // `getPrDetailsByUrl` falls back to `{ state: "unknown" }` when the lookup + // fails (gh offline, private repo, unparseable URL), and a batch miss leaves + // state null. Once settled, render nothing for an unresolved state rather + // than a misleading green "open" badge. + if (!isLoading && (state === null || !KNOWN_PR_STATES.has(state))) { + return null; + } const isSm = size === "sm"; @@ -63,15 +71,18 @@ export function ReportImplementationPrLink({ ? "bg-gray-4 text-gray-11 hover:bg-gray-5" : "bg-green-4 text-green-11 hover:bg-green-5"; - const { reference: prReference, prNumber } = parseGitHubPrReference(prUrl); + const prReference = `${prRef.repoSlug}#${prRef.number}`; + const prNumber = `#${prRef.number}`; - const tooltip = merged - ? `Merged – ${prReference}` - : state === "closed" - ? `Closed – ${prReference}` - : draft - ? `Draft – ${prReference}` - : `Open – ${prReference}`; + const tooltip = isLoading + ? prReference + : merged + ? `Merged – ${prReference}` + : state === "closed" + ? `Closed – ${prReference}` + : draft + ? `Draft – ${prReference}` + : `Open – ${prReference}`; const iconSize = isSm ? 10 : 12; diff --git a/packages/workspace-server/src/services/git/schemas.ts b/packages/workspace-server/src/services/git/schemas.ts index 9b417616d..d39c56f43 100644 --- a/packages/workspace-server/src/services/git/schemas.ts +++ b/packages/workspace-server/src/services/git/schemas.ts @@ -308,10 +308,18 @@ export type PrDetailsByUrlOutput = z.infer; export const getPrChangedFilesInput = z.object({ prUrl: z.string() }); +// Also carries the PR's live status (`state`/`merged`/`draft`) so list cards +// can render it from the single batched GraphQL request instead of firing one +// REST call per visible PR. Mirrors `prDiffStatsSchema` in +// `@posthog/core/git/router-schemas`. export const prDiffStatsSchema = z.object({ additions: z.number(), deletions: z.number(), changedFiles: z.number(), + /** Lowercased GitHub PR state. */ + state: z.enum(["open", "closed", "merged"]), + merged: z.boolean(), + draft: z.boolean(), }); export type PrDiffStats = z.infer; diff --git a/packages/workspace-server/src/services/git/service.ts b/packages/workspace-server/src/services/git/service.ts index 11ba8e535..211839469 100644 --- a/packages/workspace-server/src/services/git/service.ts +++ b/packages/workspace-server/src/services/git/service.ts @@ -117,6 +117,24 @@ function toUnifiedDiffPatch( return `diff --git a/${oldPath} b/${filename}\n--- ${fromPath}\n+++ ${toPath}\n${rawPatch}`; } +/** + * Narrow GitHub GraphQL's `PullRequestState` (OPEN | CLOSED | MERGED) to the + * lowercased literal the batch schema expects. Anything unexpected falls back + * to "open" so one odd value can never fail validation for the whole batch. + */ +function normalizeGraphqlPrState( + graphqlState: string, +): "open" | "closed" | "merged" { + switch (graphqlState) { + case "MERGED": + return "merged"; + case "CLOSED": + return "closed"; + default: + return "open"; + } +} + export function mapPrState( state: string | null, merged: boolean, @@ -1054,7 +1072,7 @@ export class GitService extends TypedEventEmitter { ): Promise> { const aliasFragments = chunk .map(([, { parsed }], index) => { - return `pr${index}: repository(owner: "${escapeGraphqlString(parsed.owner)}", name: "${escapeGraphqlString(parsed.repo)}") { pullRequest(number: ${parsed.number}) { additions deletions changedFiles } }`; + return `pr${index}: repository(owner: "${escapeGraphqlString(parsed.owner)}", name: "${escapeGraphqlString(parsed.repo)}") { pullRequest(number: ${parsed.number}) { additions deletions changedFiles state isDraft } }`; }) .join("\n"); const query = `query InboxPrDiffStatsBatch {\n${aliasFragments}\n}`; @@ -1075,6 +1093,8 @@ export class GitService extends TypedEventEmitter { additions: number; deletions: number; changedFiles: number; + state: string; + isDraft: boolean; } | null; } | null >; @@ -1085,10 +1105,15 @@ export class GitService extends TypedEventEmitter { const [, { urls }] = chunk[i]; const node = parsed.data?.[`pr${i}`]?.pullRequest; if (!node) continue; + // GraphQL `PullRequestState` is OPEN | CLOSED | MERGED; normalise to the + // lowercase state + merged boolean shape the badge expects. const stats: PrDiffStats = { additions: node.additions, deletions: node.deletions, changedFiles: node.changedFiles, + state: normalizeGraphqlPrState(node.state), + merged: node.state === "MERGED", + draft: node.isDraft, }; for (const url of urls) { out[url] = stats;