Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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: () => <span data-testid="file-icon" />,
}));

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<number, PrCommentThread>([
[
1,
{
rootId: 1,
nodeId: "thread-1",
isResolved: false,
filePath: threadPath,
comments: [{ id: 1 }, { id: 2 }] as PrCommentThread["comments"],
},
],
]);

render(
<PatchedFileDiff
file={file}
taskId="task"
options={{}}
collapsed
onToggle={() => {}}
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));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -63,6 +64,7 @@ export function PatchedFileDiff({
collapsed={collapsed}
onToggle={onToggle}
externalUrl={externalUrl}
commentCount={commentCount}
headerTrailing={headerTrailing}
/>
);
Expand All @@ -78,6 +80,7 @@ export function PatchedFileDiff({
collapsed={collapsed}
onToggle={onToggle}
externalUrl={externalUrl}
commentCount={commentCount}
headerTrailing={headerTrailing}
/>
);
Expand All @@ -95,9 +98,26 @@ export function PatchedFileDiff({
fileDiff={fd}
collapsed={collapsed}
onToggle={onToggle}
commentCount={commentCount}
trailing={headerTrailing}
/>
)}
/>
);
}

function countPrCommentsForFile(
threads: Map<number, PrCommentThread> | undefined,
file: Pick<ChangedFile, "path" | "originalPath">,
): 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;
}
12 changes: 11 additions & 1 deletion packages/ui/src/features/code-review/reviewShellParts.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ function findSpan(
return found;
}

function renderHeader(path: string) {
function renderHeader(path: string, commentCount?: number) {
const diff = render(
<DiffFileHeader
fileDiff={makeFileDiff(path)}
collapsed={false}
onToggle={() => {}}
commentCount={commentCount}
/>,
);
const deferred = render(
Expand All @@ -52,6 +53,7 @@ function renderHeader(path: string) {
reason="line-limit"
collapsed={false}
onToggle={() => {}}
commentCount={commentCount}
/>,
);
return { diff, deferred };
Expand Down Expand Up @@ -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));
});
});
28 changes: 28 additions & 0 deletions packages/ui/src/features/code-review/reviewShellParts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
ArrowCounterClockwise,
ArrowSquareOut,
CaretDown,
ChatCircle,
Minus,
Plus,
} from "@phosphor-icons/react";
Expand All @@ -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";
Expand Down Expand Up @@ -139,6 +141,7 @@ export function FileHeaderRow({
deletions,
collapsed,
onToggle,
commentCount,
trailing,
}: {
dirPath: string;
Expand All @@ -147,6 +150,7 @@ export function FileHeaderRow({
deletions: number;
collapsed: boolean;
onToggle: () => void;
commentCount?: number;
trailing?: ReactNode;
}) {
return (
Expand Down Expand Up @@ -176,6 +180,9 @@ export function FileHeaderRow({
{dirPath}
</span>
</span>
{commentCount != null && commentCount > 0 && (
<PrCommentCountBadge count={commentCount} />
)}
<span className="font-mono text-[10px]">
{additions > 0 && (
<span className="mr-[2px] text-(--green-9)">+{additions}</span>
Expand Down Expand Up @@ -210,6 +217,7 @@ export function DiffFileHeader({
onDiscard,
onStage,
staged,
commentCount,
trailing,
}: {
fileDiff: FileDiffMetadata;
Expand All @@ -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;
}) {
Expand All @@ -237,6 +246,7 @@ export function DiffFileHeader({
deletions={deletions}
collapsed={collapsed}
onToggle={onToggle}
commentCount={commentCount}
trailing={
(onStage || onDiscard || onOpenFile || trailing) && (
<span className="ml-auto inline-flex items-center gap-[2px]">
Expand Down Expand Up @@ -299,6 +309,7 @@ export function DeferredDiffPlaceholder({
onToggle,
onShow,
externalUrl,
commentCount,
headerTrailing,
}: {
filePath: string;
Expand All @@ -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;
}) {
Expand All @@ -323,6 +335,7 @@ export function DeferredDiffPlaceholder({
deletions={linesRemoved}
collapsed={collapsed}
onToggle={onToggle}
commentCount={commentCount}
trailing={
headerTrailing && (
<span className="ml-auto inline-flex items-center">
Expand Down Expand Up @@ -369,3 +382,18 @@ export function DeferredDiffPlaceholder({
</div>
);
}

function PrCommentCountBadge({ count }: { count: number }) {
const label = `${count} comment${count === 1 ? "" : "s"}`;
return (
<Badge
variant="default"
title={label}
className="shrink-0 gap-[3px] border-(--gray-7) bg-(--gray-3) text-[11px] text-gray-12 tabular-nums"
>
<ChatCircle size={12} weight="fill" />
{count}
<span className="sr-only"> comment{count === 1 ? "" : "s"}</span>
</Badge>
);
}
18 changes: 5 additions & 13 deletions packages/ui/src/features/git-interaction/usePrDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,10 @@ interface UsePrDetailsOptions {
includeComments?: boolean;
}

function threadsToMap(threads: PrReviewThread[]): Map<number, PrCommentThread> {
const map = new Map<number, PrCommentThread>();
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<number, PrCommentThread> {
return new Map(threads.map((thread) => [thread.rootId, thread]));
}

export interface PrStateDetails {
Expand Down Expand Up @@ -80,7 +72,7 @@ export function usePrDetails(
});

const commentThreads = useMemo(
() => threadsToMap(commentsQuery.data ?? []),
() => mapPrCommentThreads(commentsQuery.data ?? []),
[commentsQuery.data],
);

Expand Down
Loading