diff --git a/packages/ui/src/features/code-review/components/PatchedFileDiff.test.tsx b/packages/ui/src/features/code-review/components/PatchedFileDiff.test.tsx new file mode 100644 index 0000000000..bc2fac4499 --- /dev/null +++ b/packages/ui/src/features/code-review/components/PatchedFileDiff.test.tsx @@ -0,0 +1,82 @@ +import type { FileDiffMetadata } from "@pierre/diffs"; +import type { PrCommentThread } from "@posthog/core/code-review/types"; +import type { ChangedFile } from "@posthog/shared/domain-types"; +import { render, screen } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { describe, expect, it, vi } from "vitest"; + +vi.mock("../../../primitives/FileIcon", () => ({ + FileIcon: () => , +})); + +vi.mock("./InteractiveFileDiff", () => ({ + InteractiveFileDiff: ({ + fileDiff, + renderCustomHeader, + }: { + fileDiff: FileDiffMetadata; + renderCustomHeader: (fileDiff: FileDiffMetadata) => ReactNode; + }) => renderCustomHeader(fileDiff), +})); + +import { PatchedFileDiff } from "./PatchedFileDiff"; + +const patch = `diff --git a/src/reviewed.ts b/src/reviewed.ts +index 1111111..2222222 100644 +--- a/src/reviewed.ts ++++ b/src/reviewed.ts +@@ -1 +1 @@ +-before ++after`; + +describe.each([ + [ + "regular", + { + path: "src/reviewed.ts", + originalPath: "src/original.ts", + patch, + }, + ], + ["binary", { path: "assets/reviewed.png", patch: null }], + ["unavailable", { path: "src/unavailable.ts", patch: null }], +] as const)("PatchedFileDiff %s header", (_kind, fileInput) => { + it("renders metadata before line change stats", () => { + const file = { + ...fileInput, + linesAdded: 2, + linesRemoved: 1, + } as ChangedFile; + const threadPath = file.originalPath ?? file.path; + const commentThreads = new Map([ + [ + 1, + { + rootId: 1, + nodeId: "thread-1", + isResolved: false, + filePath: threadPath, + comments: [{ id: 1 }, { id: 2 }] as PrCommentThread["comments"], + }, + ], + ]); + + render( + {}} + commentThreads={commentThreads} + />, + ); + + const header = screen.getByRole("button"); + const text = header.textContent ?? ""; + const additions = _kind === "regular" ? "+1" : "+2"; + + expect(screen.getByTitle("2 comments")).toBeInTheDocument(); + expect(text.indexOf("2 comments")).toBeLessThan(text.indexOf(additions)); + }); +}); diff --git a/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx b/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx index e648537e1d..93b987636a 100644 --- a/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx +++ b/packages/ui/src/features/code-review/components/PatchedFileDiff.tsx @@ -49,6 +49,7 @@ export function PatchedFileDiff({ } return null; }, [fileDiff, fallback, file.path]); + const commentCount = countPrCommentsForFile(commentThreads, file); // Branch/PR diffs have no reliable local working-tree file to preview (the // checkout may be on a different ref, and GitHub omits binary patches), so @@ -63,6 +64,7 @@ export function PatchedFileDiff({ collapsed={collapsed} onToggle={onToggle} externalUrl={externalUrl} + commentCount={commentCount} headerTrailing={headerTrailing} /> ); @@ -78,6 +80,7 @@ export function PatchedFileDiff({ collapsed={collapsed} onToggle={onToggle} externalUrl={externalUrl} + commentCount={commentCount} headerTrailing={headerTrailing} /> ); @@ -95,9 +98,26 @@ export function PatchedFileDiff({ fileDiff={fd} collapsed={collapsed} onToggle={onToggle} + commentCount={commentCount} trailing={headerTrailing} /> )} /> ); } + +function countPrCommentsForFile( + threads: Map | undefined, + file: Pick, +): number { + let count = 0; + for (const thread of threads?.values() ?? []) { + if ( + thread.filePath === file.path || + (file.originalPath != null && thread.filePath === file.originalPath) + ) { + count += thread.comments.length; + } + } + return count; +} diff --git a/packages/ui/src/features/code-review/reviewShellParts.test.tsx b/packages/ui/src/features/code-review/reviewShellParts.test.tsx index 2361dae47a..a07d37b55b 100644 --- a/packages/ui/src/features/code-review/reviewShellParts.test.tsx +++ b/packages/ui/src/features/code-review/reviewShellParts.test.tsx @@ -36,12 +36,13 @@ function findSpan( return found; } -function renderHeader(path: string) { +function renderHeader(path: string, commentCount?: number) { const diff = render( {}} + commentCount={commentCount} />, ); const deferred = render( @@ -52,6 +53,7 @@ function renderHeader(path: string) { reason="line-limit" collapsed={false} onToggle={() => {}} + commentCount={commentCount} />, ); return { diff, deferred }; @@ -96,4 +98,12 @@ describe.each([ expect(dirSpan.parentElement).toBe(fileSpan.parentElement); expect(dirSpan.parentElement?.classList.contains("flex")).toBe(true); }); + + it("renders metadata before line changes", () => { + const rendered = renderHeader("src/ReviewShell.tsx", 2)[which]; + const text = rendered.container.querySelector("button")?.textContent ?? ""; + const additions = which === "diff" ? "+3" : "+10"; + + expect(text.indexOf("2 comments")).toBeLessThan(text.indexOf(additions)); + }); }); diff --git a/packages/ui/src/features/code-review/reviewShellParts.tsx b/packages/ui/src/features/code-review/reviewShellParts.tsx index cafe7f1ceb..ae283f7e92 100644 --- a/packages/ui/src/features/code-review/reviewShellParts.tsx +++ b/packages/ui/src/features/code-review/reviewShellParts.tsx @@ -2,6 +2,7 @@ import { ArrowCounterClockwise, ArrowSquareOut, CaretDown, + ChatCircle, Minus, Plus, } from "@phosphor-icons/react"; @@ -13,6 +14,7 @@ import { splitFilePath, sumHunkStats, } from "@posthog/core/code-review/reviewShellGeometry"; +import { Badge } from "@posthog/quill"; import type { ChangedFile, Task } from "@posthog/shared/domain-types"; import { type ReactNode, useCallback, useMemo, useState } from "react"; import { FileIcon } from "../../primitives/FileIcon"; @@ -139,6 +141,7 @@ export function FileHeaderRow({ deletions, collapsed, onToggle, + commentCount, trailing, }: { dirPath: string; @@ -147,6 +150,7 @@ export function FileHeaderRow({ deletions: number; collapsed: boolean; onToggle: () => void; + commentCount?: number; trailing?: ReactNode; }) { return ( @@ -176,6 +180,9 @@ export function FileHeaderRow({ {dirPath} + {commentCount != null && commentCount > 0 && ( + + )} {additions > 0 && ( +{additions} @@ -210,6 +217,7 @@ export function DiffFileHeader({ onDiscard, onStage, staged, + commentCount, trailing, }: { fileDiff: FileDiffMetadata; @@ -219,6 +227,7 @@ export function DiffFileHeader({ onDiscard?: () => void; onStage?: () => void; staged?: boolean; + commentCount?: number; /** Extra controls rendered after the action buttons (e.g. a "Viewed" toggle). */ trailing?: ReactNode; }) { @@ -237,6 +246,7 @@ export function DiffFileHeader({ deletions={deletions} collapsed={collapsed} onToggle={onToggle} + commentCount={commentCount} trailing={ (onStage || onDiscard || onOpenFile || trailing) && ( @@ -299,6 +309,7 @@ export function DeferredDiffPlaceholder({ onToggle, onShow, externalUrl, + commentCount, headerTrailing, }: { filePath: string; @@ -309,6 +320,7 @@ export function DeferredDiffPlaceholder({ onToggle: () => void; onShow?: () => void; externalUrl?: string; + commentCount?: number; /** Extra controls in the header row (e.g. a "Viewed" toggle). */ headerTrailing?: ReactNode; }) { @@ -323,6 +335,7 @@ export function DeferredDiffPlaceholder({ deletions={linesRemoved} collapsed={collapsed} onToggle={onToggle} + commentCount={commentCount} trailing={ headerTrailing && ( @@ -369,3 +382,18 @@ export function DeferredDiffPlaceholder({ ); } + +function PrCommentCountBadge({ count }: { count: number }) { + const label = `${count} comment${count === 1 ? "" : "s"}`; + return ( + + + {count} + comment{count === 1 ? "" : "s"} + + ); +} diff --git a/packages/ui/src/features/git-interaction/usePrDetails.ts b/packages/ui/src/features/git-interaction/usePrDetails.ts index 657b027290..270309103b 100644 --- a/packages/ui/src/features/git-interaction/usePrDetails.ts +++ b/packages/ui/src/features/git-interaction/usePrDetails.ts @@ -8,18 +8,10 @@ interface UsePrDetailsOptions { includeComments?: boolean; } -function threadsToMap(threads: PrReviewThread[]): Map { - const map = new Map(); - for (const thread of threads) { - map.set(thread.rootId, { - rootId: thread.rootId, - nodeId: thread.nodeId, - isResolved: thread.isResolved, - comments: thread.comments, - filePath: thread.filePath, - }); - } - return map; +function mapPrCommentThreads( + threads: PrReviewThread[], +): Map { + return new Map(threads.map((thread) => [thread.rootId, thread])); } export interface PrStateDetails { @@ -80,7 +72,7 @@ export function usePrDetails( }); const commentThreads = useMemo( - () => threadsToMap(commentsQuery.data ?? []), + () => mapPrCommentThreads(commentsQuery.data ?? []), [commentsQuery.data], );