Skip to content

Commit 2b4cf36

Browse files
authored
feat(inbox): show live PR status on pull request list cards (#2695)
1 parent 07c52f9 commit 2b4cf36

6 files changed

Lines changed: 108 additions & 42 deletions

File tree

packages/core/src/git/router-schemas.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,18 @@ export const getPrChangedFilesInput = z.object({
352352
export const getPrChangedFilesOutput = z.array(changedFileSchema);
353353

354354
// getPrDiffStatsBatch schemas
355+
//
356+
// Beyond the diff numbers, the batch also carries the PR's live status
357+
// (`state`/`merged`/`draft`) so list cards can render it from the single
358+
// batched GraphQL request instead of firing one REST call per visible PR.
355359
export const prDiffStatsSchema = z.object({
356360
additions: z.number(),
357361
deletions: z.number(),
358362
changedFiles: z.number(),
363+
/** Lowercased GitHub PR state. */
364+
state: z.enum(["open", "closed", "merged"]),
365+
merged: z.boolean(),
366+
draft: z.boolean(),
359367
});
360368
export type PrDiffStats = z.infer<typeof prDiffStatsSchema>;
361369

packages/ui/src/features/git-interaction/usePrActions.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,13 @@ export function usePrActions(prUrl: string | null) {
2020
trpc.git.getPrDetailsByUrl.queryKey({ prUrl: variables.prUrl }),
2121
getOptimisticPrState(variables.action),
2222
);
23+
// The inbox Pulls list reads PR status from the batched
24+
// `getPrDiffStatsBatch` query (separate cache, 5-min staleTime), so
25+
// patching the detail cache above isn't enough — invalidate the batch
26+
// so the list badge reflects the new state on next view.
27+
void queryClient.invalidateQueries(
28+
trpc.git.getPrDiffStatsBatch.pathFilter(),
29+
);
2330
} else {
2431
toast.error("Failed to update PR", { description: data.message });
2532
}

packages/ui/src/features/inbox/components/PullRequestCard.tsx

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { InboxCardTitle } from "@posthog/ui/features/inbox/components/InboxCardT
1414
import { PrDiffStats } from "@posthog/ui/features/inbox/components/PrDiffStats";
1515
import { PriorityMonogram } from "@posthog/ui/features/inbox/components/PriorityMonogram";
1616
import { SuggestedReviewerAvatarStack } from "@posthog/ui/features/inbox/components/SuggestedReviewerAvatarStack";
17+
import { ReportImplementationPrLink } from "@posthog/ui/features/inbox/components/utils/ReportImplementationPrLink";
1718
import { useInboxReportDetailPrefetch } from "@posthog/ui/features/inbox/hooks/useInboxReportDetailPrefetch";
1819
import { useInboxReportArtefacts } from "@posthog/ui/features/inbox/hooks/useInboxReports";
1920
import { Button as UiButton } from "@posthog/ui/primitives/Button";
@@ -121,10 +122,16 @@ export function PullRequestCard({
121122
>
122123
<Flex align="center" gap="2" className="shrink-0">
123124
{report.implementation_pr_url && (
124-
<PrDiffStats
125-
prUrl={report.implementation_pr_url}
126-
hideWhileLoading
127-
/>
125+
<Flex direction="column" align="end" gap="1" className="shrink-0">
126+
<ReportImplementationPrLink
127+
prUrl={report.implementation_pr_url}
128+
size="sm"
129+
/>
130+
<PrDiffStats
131+
prUrl={report.implementation_pr_url}
132+
hideWhileLoading
133+
/>
134+
</Flex>
128135
)}
129136
<SuggestedReviewerAvatarStack
130137
reportId={report.id}

packages/ui/src/features/inbox/components/utils/ReportImplementationPrLink.tsx

Lines changed: 48 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { GitMergeIcon, GitPullRequestIcon } from "@phosphor-icons/react";
2+
import { parsePrUrl } from "@posthog/core/inbox/reportPresentation";
23
import { cn } from "@posthog/quill";
34
import { usePrDetails } from "@posthog/ui/features/git-interaction/usePrDetails";
5+
import { usePrDiffStatsFromBatch } from "@posthog/ui/features/inbox/context/PrDiffStatsBatchContext";
46
import { Tooltip } from "@radix-ui/themes";
57

68
export type ImplementationPrLinkSize = "sm" | "md";
@@ -13,41 +15,47 @@ interface ReportImplementationPrLinkProps {
1315
onLinkClick?: () => void;
1416
}
1517

16-
function parseGitHubPrReference(prUrl: string): {
17-
reference: string;
18-
prNumber: string;
19-
} {
20-
try {
21-
const parsed = new URL(prUrl);
22-
const match = parsed.pathname.match(
23-
/^\/([^/]+)\/([^/]+)\/pull\/(\d+)(?:$|[/?#])/,
24-
);
25-
if (match) {
26-
return {
27-
reference: `${match[1]}/${match[2]}#${match[3]}`,
28-
prNumber: `#${match[3]}`,
29-
};
30-
}
31-
} catch {
32-
// Fall through to regex fallback for non-URL-safe inputs
33-
}
34-
35-
const prMatch = prUrl.match(/\/pull\/(\d+)(?:$|[/?#])/);
36-
const prNumber = prMatch ? `#${prMatch[1]}` : "PR";
37-
return {
38-
reference: prUrl,
39-
prNumber,
40-
};
41-
}
18+
/** The only states we render a badge for; anything else is treated as unknown. */
19+
const KNOWN_PR_STATES = new Set(["open", "closed", "merged"]);
4220

4321
export function ReportImplementationPrLink({
4422
prUrl,
4523
size = "sm",
4624
onLinkClick,
4725
}: ReportImplementationPrLinkProps) {
48-
const {
49-
meta: { state, merged, draft, isLoading },
50-
} = usePrDetails(prUrl);
26+
// On list surfaces the surrounding `PrDiffStatsBatchContext` already carries
27+
// this PR's status from the single batched request, so read it from there and
28+
// skip the per-PR query. Fall back to `usePrDetails` only on the standalone
29+
// detail view, where no batch provider is mounted.
30+
const batchEntry = usePrDiffStatsFromBatch(prUrl);
31+
const fallback = usePrDetails(batchEntry.hasBatch ? null : prUrl);
32+
33+
const state = batchEntry.hasBatch
34+
? (batchEntry.stats?.state ?? null)
35+
: fallback.meta.state;
36+
const merged = batchEntry.hasBatch
37+
? (batchEntry.stats?.merged ?? false)
38+
: fallback.meta.merged;
39+
const draft = batchEntry.hasBatch
40+
? (batchEntry.stats?.draft ?? false)
41+
: fallback.meta.draft;
42+
const isLoading = batchEntry.hasBatch
43+
? batchEntry.isLoading && !batchEntry.stats
44+
: fallback.meta.isLoading;
45+
46+
// Only render for a canonical GitHub PR URL. This both keeps the badge in
47+
// sync with the gated "Open in GitHub" action and avoids turning an arbitrary
48+
// (possibly unsafe-scheme) string into a clickable external link.
49+
const prRef = parsePrUrl(prUrl);
50+
if (!prRef) return null;
51+
52+
// `getPrDetailsByUrl` falls back to `{ state: "unknown" }` when the lookup
53+
// fails (gh offline, private repo, unparseable URL), and a batch miss leaves
54+
// state null. Once settled, render nothing for an unresolved state rather
55+
// than a misleading green "open" badge.
56+
if (!isLoading && (state === null || !KNOWN_PR_STATES.has(state))) {
57+
return null;
58+
}
5159

5260
const isSm = size === "sm";
5361

@@ -63,15 +71,18 @@ export function ReportImplementationPrLink({
6371
? "bg-gray-4 text-gray-11 hover:bg-gray-5"
6472
: "bg-green-4 text-green-11 hover:bg-green-5";
6573

66-
const { reference: prReference, prNumber } = parseGitHubPrReference(prUrl);
74+
const prReference = `${prRef.repoSlug}#${prRef.number}`;
75+
const prNumber = `#${prRef.number}`;
6776

68-
const tooltip = merged
69-
? `Merged – ${prReference}`
70-
: state === "closed"
71-
? `Closed – ${prReference}`
72-
: draft
73-
? `Draft – ${prReference}`
74-
: `Open – ${prReference}`;
77+
const tooltip = isLoading
78+
? prReference
79+
: merged
80+
? `Merged – ${prReference}`
81+
: state === "closed"
82+
? `Closed – ${prReference}`
83+
: draft
84+
? `Draft – ${prReference}`
85+
: `Open – ${prReference}`;
7586

7687
const iconSize = isSm ? 10 : 12;
7788

packages/workspace-server/src/services/git/schemas.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,18 @@ export type PrDetailsByUrlOutput = z.infer<typeof getPrDetailsByUrlOutput>;
308308

309309
export const getPrChangedFilesInput = z.object({ prUrl: z.string() });
310310

311+
// Also carries the PR's live status (`state`/`merged`/`draft`) so list cards
312+
// can render it from the single batched GraphQL request instead of firing one
313+
// REST call per visible PR. Mirrors `prDiffStatsSchema` in
314+
// `@posthog/core/git/router-schemas`.
311315
export const prDiffStatsSchema = z.object({
312316
additions: z.number(),
313317
deletions: z.number(),
314318
changedFiles: z.number(),
319+
/** Lowercased GitHub PR state. */
320+
state: z.enum(["open", "closed", "merged"]),
321+
merged: z.boolean(),
322+
draft: z.boolean(),
315323
});
316324
export type PrDiffStats = z.infer<typeof prDiffStatsSchema>;
317325

packages/workspace-server/src/services/git/service.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,24 @@ function toUnifiedDiffPatch(
117117
return `diff --git a/${oldPath} b/${filename}\n--- ${fromPath}\n+++ ${toPath}\n${rawPatch}`;
118118
}
119119

120+
/**
121+
* Narrow GitHub GraphQL's `PullRequestState` (OPEN | CLOSED | MERGED) to the
122+
* lowercased literal the batch schema expects. Anything unexpected falls back
123+
* to "open" so one odd value can never fail validation for the whole batch.
124+
*/
125+
function normalizeGraphqlPrState(
126+
graphqlState: string,
127+
): "open" | "closed" | "merged" {
128+
switch (graphqlState) {
129+
case "MERGED":
130+
return "merged";
131+
case "CLOSED":
132+
return "closed";
133+
default:
134+
return "open";
135+
}
136+
}
137+
120138
export function mapPrState(
121139
state: string | null,
122140
merged: boolean,
@@ -1054,7 +1072,7 @@ export class GitService extends TypedEventEmitter<GitCloneEvents> {
10541072
): Promise<Record<string, PrDiffStats>> {
10551073
const aliasFragments = chunk
10561074
.map(([, { parsed }], index) => {
1057-
return `pr${index}: repository(owner: "${escapeGraphqlString(parsed.owner)}", name: "${escapeGraphqlString(parsed.repo)}") { pullRequest(number: ${parsed.number}) { additions deletions changedFiles } }`;
1075+
return `pr${index}: repository(owner: "${escapeGraphqlString(parsed.owner)}", name: "${escapeGraphqlString(parsed.repo)}") { pullRequest(number: ${parsed.number}) { additions deletions changedFiles state isDraft } }`;
10581076
})
10591077
.join("\n");
10601078
const query = `query InboxPrDiffStatsBatch {\n${aliasFragments}\n}`;
@@ -1075,6 +1093,8 @@ export class GitService extends TypedEventEmitter<GitCloneEvents> {
10751093
additions: number;
10761094
deletions: number;
10771095
changedFiles: number;
1096+
state: string;
1097+
isDraft: boolean;
10781098
} | null;
10791099
} | null
10801100
>;
@@ -1085,10 +1105,15 @@ export class GitService extends TypedEventEmitter<GitCloneEvents> {
10851105
const [, { urls }] = chunk[i];
10861106
const node = parsed.data?.[`pr${i}`]?.pullRequest;
10871107
if (!node) continue;
1108+
// GraphQL `PullRequestState` is OPEN | CLOSED | MERGED; normalise to the
1109+
// lowercase state + merged boolean shape the badge expects.
10881110
const stats: PrDiffStats = {
10891111
additions: node.additions,
10901112
deletions: node.deletions,
10911113
changedFiles: node.changedFiles,
1114+
state: normalizeGraphqlPrState(node.state),
1115+
merged: node.state === "MERGED",
1116+
draft: node.isDraft,
10921117
};
10931118
for (const url of urls) {
10941119
out[url] = stats;

0 commit comments

Comments
 (0)