diff --git a/packages/core/src/inbox/reportActions.test.ts b/packages/core/src/inbox/reportActions.test.ts index 03790e687d..9b97012997 100644 --- a/packages/core/src/inbox/reportActions.test.ts +++ b/packages/core/src/inbox/reportActions.test.ts @@ -2,8 +2,51 @@ import { describe, expect, it } from "vitest"; import { buildCreatePrReportPrompt, buildDiscussReportPrompt, + isImplementationPrClosedOrMerged, } from "./reportActions"; +describe("isImplementationPrClosedOrMerged", () => { + it.each([ + { label: "missing state", pr: null, expected: false }, + { label: "undefined state", pr: undefined, expected: false }, + { label: "unresolved (no fields)", pr: {}, expected: false }, + { + label: "open PR (GraphQL batch)", + pr: { state: "open", merged: false }, + expected: false, + }, + { + label: "draft PR", + pr: { state: "open", merged: false }, + expected: false, + }, + { + label: "merged PR (GraphQL batch)", + pr: { state: "merged", merged: true }, + expected: true, + }, + // GitHub's REST endpoint reports merged PRs as closed + merged:true. + { + label: "merged PR (REST)", + pr: { state: "closed", merged: true }, + expected: true, + }, + { + label: "closed-unmerged PR", + pr: { state: "closed", merged: false }, + expected: true, + }, + { label: "merged flag only", pr: { merged: true }, expected: true }, + { + label: "uppercase CLOSED", + pr: { state: "CLOSED", merged: false }, + expected: true, + }, + ])("is $expected for $label", ({ pr, expected }) => { + expect(isImplementationPrClosedOrMerged(pr)).toBe(expected); + }); +}); + describe("buildCreatePrReportPrompt", () => { it("embeds the report web URL as a clickable backlink when provided", () => { const prompt = buildCreatePrReportPrompt({ diff --git a/packages/core/src/inbox/reportActions.ts b/packages/core/src/inbox/reportActions.ts index 6907ab9d12..58831fab20 100644 --- a/packages/core/src/inbox/reportActions.ts +++ b/packages/core/src/inbox/reportActions.ts @@ -22,6 +22,38 @@ export function canCreateImplementationPr(report: SignalReport): boolean { return false; } +/** + * Live GitHub lifecycle of an implementation PR, as returned by + * `getPrDetailsByUrl` (REST: `state` is `open`/`closed`, and merged PRs report + * `state: "closed"` with `merged: true`) or the inbox diff-stats batch (GraphQL: + * `state` is `open`/`closed`/`merged`). + */ +export interface ImplementationPrLiveState { + /** Lowercased PR state; `null`/`undefined` while unresolved. */ + state?: string | null; + merged?: boolean; +} + +/** + * Is the report's implementation PR no longer open on GitHub — merged or closed? + * + * The cloud report `status` lags GitHub: it stays `ready` until the backend + * reconciles a merge/close, so a merged PR can leave its report stuck in the + * Pull requests tab looking like active review work. The live PR state is the + * source of truth here, used to de-emphasise already-shipped PRs and to + * suppress the misleading "continue the open PR" path (which otherwise claims an + * open PR exists for one that has already merged). + * + * Merged PRs surface as `merged: true` in both data sources; closed-unmerged + * PRs as `state === "closed"`. + */ +export function isImplementationPrClosedOrMerged( + pr: ImplementationPrLiveState | null | undefined, +): boolean { + if (!pr) return false; + return pr.merged === true || pr.state?.toLowerCase() === "closed"; +} + interface BuildCreatePrReportPromptOptions { reportId: string; /** diff --git a/packages/ui/src/features/inbox/components/PullRequestCard.tsx b/packages/ui/src/features/inbox/components/PullRequestCard.tsx index b20765161c..97f8f58344 100644 --- a/packages/ui/src/features/inbox/components/PullRequestCard.tsx +++ b/packages/ui/src/features/inbox/components/PullRequestCard.tsx @@ -1,5 +1,6 @@ import { ArchiveIcon } from "@phosphor-icons/react"; import { extractRepoSelectionRepository } from "@posthog/core/inbox/artefacts"; +import { isImplementationPrClosedOrMerged } from "@posthog/core/inbox/reportActions"; import { deriveHeadline, displayConventionalCommitTitle, @@ -15,6 +16,7 @@ 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 { usePrDiffStatsFromBatch } from "@posthog/ui/features/inbox/context/PrDiffStatsBatchContext"; 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"; @@ -51,6 +53,13 @@ export function PullRequestCard({ const prRef = report.implementation_pr_url ? parsePrUrl(report.implementation_pr_url) : null; + // Live GitHub state from the surrounding diff-stats batch. The cloud report + // `status` lags a merge/close (it stays `ready` until the backend catches + // up), so a merged PR would otherwise keep its actionable primary "Review" + // button and read as active work. Once shipped, de-emphasise it to a neutral + // "View" — the violet merged badge on the PR link makes the state explicit. + const prBatch = usePrDiffStatsFromBatch(report.implementation_pr_url); + const prClosedOrMerged = isImplementationPrClosedOrMerged(prBatch.stats); const { data: artefactsResp } = useInboxReportArtefacts(report.id, { staleTime: 5 * 60 * 1000, refetchOnWindowFocus: false, @@ -158,7 +167,7 @@ export function PullRequestCard({ diff --git a/packages/ui/src/features/inbox/components/ReportDetailActions.tsx b/packages/ui/src/features/inbox/components/ReportDetailActions.tsx index 5582652bbe..63743bc277 100644 --- a/packages/ui/src/features/inbox/components/ReportDetailActions.tsx +++ b/packages/ui/src/features/inbox/components/ReportDetailActions.tsx @@ -5,9 +5,13 @@ import { GitPullRequestIcon, } from "@phosphor-icons/react"; import { extractRepoSelectionRepository } from "@posthog/core/inbox/artefacts"; -import { canCreateImplementationPr } from "@posthog/core/inbox/reportActions"; +import { + canCreateImplementationPr, + isImplementationPrClosedOrMerged, +} from "@posthog/core/inbox/reportActions"; import { Button } from "@posthog/quill"; import type { SignalReport } from "@posthog/shared/types"; +import { usePrDetails } from "@posthog/ui/features/git-interaction/usePrDetails"; import { useCreatePrReport } from "@posthog/ui/features/inbox/hooks/useCreatePrReport"; import { useDiscussReport } from "@posthog/ui/features/inbox/hooks/useDiscussReport"; import { useInboxReportArtefacts } from "@posthog/ui/features/inbox/hooks/useInboxReports"; @@ -50,7 +54,20 @@ export function ReportDetailActions({ report }: ReportDetailActionsProps) { const existingPrUrl = report.implementation_pr_url ?? (continuableTask ? getTaskPrUrl(continuableTask) : null); - const hasExistingPr = !!existingPrUrl || !!continuableTask; + + // Reconcile against live GitHub state: the cloud report `status` lags a + // merge/close (it stays `ready` until the backend catches up), so a PR that + // has already merged would otherwise still offer "Continue PR" and claim an + // open PR exists. Once the PR is merged/closed there is nothing open to + // continue, so drop the continuation path. `usePrDetails` reuses the same + // `getPrDetailsByUrl` query the header's PR badge already primed on this page. + const { meta: implementationPrMeta } = usePrDetails(existingPrUrl ?? null); + const prClosedOrMerged = + !implementationPrMeta.isLoading && + isImplementationPrClosedOrMerged(implementationPrMeta); + + const hasExistingPr = + (!!existingPrUrl || !!continuableTask) && !prClosedOrMerged; const fireAction = useReportActionTracker(report); const openTask = useOpenTask();