Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

feat: group review files by type #3526

Closed
MattPua wants to merge 1 commit into from
Closed

feat: group review files by type #3526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
114 changes: 100 additions & 14 deletions packages/ui/src/features/task-detail/components/ChangesPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
FilePlus,
MinusIcon,
PlusIcon,
StackIcon,
TreeStructure,
} from "@phosphor-icons/react";
import { getFileExtension } from "@posthog/shared";
import {
Expand All @@ -19,6 +21,7 @@ import {
DropdownMenu,
Flex,
IconButton,
SegmentedControl,
Spinner,
Text,
} from "@radix-ui/themes";
Expand Down Expand Up @@ -47,12 +50,17 @@ import { useCloudChangedFiles } from "../hooks/useCloudChangedFiles";
import { useDiscardFile } from "../hooks/useDiscardFile";
import { useStageToggle } from "../hooks/useStageToggle";
import { ChangesTreeView } from "./ChangesTreeView";
import type { ChangesGrouping } from "./changesTree";

interface ChangesPanelProps {
taskId: string;
task: Task;
}

interface GroupedChangesPanelProps extends ChangesPanelProps {
grouping: ChangesGrouping;
}

interface ChangedFileItemProps {
file: ChangedFile;
taskId: string;
Expand All @@ -63,6 +71,7 @@ interface ChangedFileItemProps {
onStageToggle?: (file: ChangedFile) => void;
onDiscard?: (file: ChangedFile, fileName: string) => void;
depth?: number;
showFullPath?: boolean;
}

function CompactIconButton({
Expand Down Expand Up @@ -99,6 +108,7 @@ function ChangedFileItem({
onStageToggle,
onDiscard,
depth = 0,
showFullPath = false,
}: ChangedFileItemProps) {
const requestScrollToFile = useReviewNavigationStore(
(state) => state.requestScrollToFile,
Expand Down Expand Up @@ -283,6 +293,7 @@ function ChangedFileItem({
<Tooltip content={tooltipContent} side="top" delayDuration={500}>
<TreeFileRow
fileName={fileName}
displayName={showFullPath ? file.path : fileName}
depth={depth}
isActive={isActive}
onClick={handleClick}
Expand All @@ -296,7 +307,11 @@ function ChangedFileItem({
);
}

function CloudChangesPanel({ taskId, task }: ChangesPanelProps) {
function CloudChangesPanel({
taskId,
task,
grouping,
}: GroupedChangesPanelProps) {
const {
prUrl,
effectiveBranch,
Expand All @@ -313,14 +328,15 @@ function CloudChangesPanel({ taskId, task }: ChangesPanelProps) {
const effectiveFiles = changedFiles;

const renderFile = useCallback(
(file: ChangedFile, depth: number) => (
(file: ChangedFile, depth: number, showFullPath: boolean) => (
<ChangedFileItem
key={file.path}
file={file}
taskId={taskId}
fileKey={file.path}
isActive={activeFilePath === file.path}
depth={depth}
showFullPath={showFullPath}
/>
),
[taskId, activeFilePath],
Expand Down Expand Up @@ -379,7 +395,11 @@ function CloudChangesPanel({ taskId, task }: ChangesPanelProps) {
return (
<Box height="100%" overflowY="auto" py="2" id="changes-panel-cloud">
<Flex direction="column">
<ChangesTreeView files={effectiveFiles} renderFile={renderFile} />
<ChangesTreeView
files={effectiveFiles}
grouping={grouping}
renderFile={renderFile}
/>
{isRunActive && (
<Flex align="center" gap="2" px="3" py="2">
<Spinner size="1" />
Expand All @@ -395,15 +415,55 @@ function CloudChangesPanel({ taskId, task }: ChangesPanelProps) {

export function ChangesPanel({ taskId, task }: ChangesPanelProps) {
const isCloud = useIsCloudTask(taskId);
const isExpanded = useReviewNavigationStore(
(state) => state.reviewModes[taskId] === "expanded",
);
const [grouping, setGrouping] = useState<ChangesGrouping>("directory");

if (isCloud) {
return <CloudChangesPanel taskId={taskId} task={task} />;
const content = isCloud ? (
<CloudChangesPanel taskId={taskId} task={task} grouping={grouping} />
) : (
<LocalChangesPanel taskId={taskId} task={task} grouping={grouping} />
);

if (!isExpanded) {
return content;
}

return <LocalChangesPanel taskId={taskId} task={task} />;
return (
<Flex direction="column" className="h-full min-h-0">
<Flex className="shrink-0 border-(--gray-5) border-b px-2 py-1.5">
<SegmentedControl.Root
value={grouping}
size="1"
onValueChange={(value) => setGrouping(value as ChangesGrouping)}
aria-label="Changed files grouping"
className="w-full"
>
<SegmentedControl.Item value="directory">
<span className="inline-flex items-center gap-1.5">
<TreeStructure size={12} />
Folders
</span>
</SegmentedControl.Item>
<SegmentedControl.Item value="file-type">
<span className="inline-flex items-center gap-1.5">
<StackIcon size={12} />
File type
</span>
</SegmentedControl.Item>
</SegmentedControl.Root>
</Flex>
<Box className="min-h-0 flex-1">{content}</Box>
</Flex>
);
}

function LocalChangesPanel({ taskId, task }: ChangesPanelProps) {
function LocalChangesPanel({
taskId,
task,
grouping,
}: GroupedChangesPanelProps) {
const { effectiveSource, prUrl, linkedBranch } =
useEffectiveDiffSource(taskId);
const repoPath = useCwd(taskId);
Expand All @@ -414,21 +474,29 @@ function LocalChangesPanel({ taskId, task }: ChangesPanelProps) {
taskId={taskId}
repoPath={repoPath}
branch={linkedBranch}
grouping={grouping}
/>
);
}

if (effectiveSource === "pr") {
return <PrChangesPanel taskId={taskId} prUrl={prUrl} />;
return <PrChangesPanel taskId={taskId} prUrl={prUrl} grouping={grouping} />;
}

return <LocalWorkingTreeChangesPanel taskId={taskId} task={task} />;
return (
<LocalWorkingTreeChangesPanel
taskId={taskId}
task={task}
grouping={grouping}
/>
);
}

function LocalWorkingTreeChangesPanel({
taskId,
task: _task,
}: ChangesPanelProps) {
grouping,
}: GroupedChangesPanelProps) {
const workspace = useWorkspace(taskId);
const repoPath = useCwd(taskId);
const activeFilePath = useReviewNavigationStore(
Expand All @@ -446,7 +514,7 @@ function LocalWorkingTreeChangesPanel({
const hasStagedFiles = stagedFiles.length > 0;

const renderLocalFile = useCallback(
(file: ChangedFile, depth: number) => {
(file: ChangedFile, depth: number, showFullPath: boolean) => {
const key = makeFileKey(file.staged, file.path);
return (
<ChangedFileItem
Expand All @@ -460,6 +528,7 @@ function LocalWorkingTreeChangesPanel({
onStageToggle={handleStageToggle}
onDiscard={handleDiscard}
depth={depth}
showFullPath={showFullPath}
/>
);
},
Expand Down Expand Up @@ -512,7 +581,11 @@ function LocalWorkingTreeChangesPanel({
</Text>
</Flex>
)}
<ChangesTreeView files={files} renderFile={renderLocalFile} />
<ChangesTreeView
files={files}
grouping={grouping}
renderFile={renderLocalFile}
/>
</Fragment>
))}
</Flex>
Expand All @@ -526,6 +599,7 @@ interface RemoteChangesListProps {
isLoading: boolean;
emptyMessage: string;
panelId: string;
grouping: ChangesGrouping;
}

function RemoteChangesList({
Expand All @@ -534,20 +608,22 @@ function RemoteChangesList({
isLoading,
emptyMessage,
panelId,
grouping,
}: RemoteChangesListProps) {
const activeFilePath = useReviewNavigationStore(
(s) => s.activeFilePaths[taskId] ?? null,
);

const renderFile = useCallback(
(file: ChangedFile, depth: number) => (
(file: ChangedFile, depth: number, showFullPath: boolean) => (
<ChangedFileItem
key={file.path}
file={file}
taskId={taskId}
fileKey={file.path}
isActive={activeFilePath === file.path}
depth={depth}
showFullPath={showFullPath}
/>
),
[taskId, activeFilePath],
Expand All @@ -564,7 +640,11 @@ function RemoteChangesList({
return (
<Box height="100%" overflowY="auto" py="2" id={panelId}>
<Flex direction="column">
<ChangesTreeView files={files} renderFile={renderFile} />
<ChangesTreeView
files={files}
grouping={grouping}
renderFile={renderFile}
/>
</Flex>
</Box>
);
Expand All @@ -574,10 +654,12 @@ function BranchChangesPanel({
taskId,
repoPath,
branch,
grouping,
}: {
taskId: string;
repoPath: string | undefined;
branch: string | null;
grouping: ChangesGrouping;
}) {
const { data: files = [], isLoading } = useLocalBranchChangedFiles(
repoPath ?? null,
Expand All @@ -595,16 +677,19 @@ function BranchChangesPanel({
isLoading={isLoading}
emptyMessage="No file changes in branch"
panelId="changes-panel-branch"
grouping={grouping}
/>
);
}

function PrChangesPanel({
taskId,
prUrl,
grouping,
}: {
taskId: string;
prUrl: string | null;
grouping: ChangesGrouping;
}) {
const { data: files = [], isLoading } = usePrChangedFiles(prUrl);

Expand All @@ -619,6 +704,7 @@ function PrChangesPanel({
isLoading={isLoading}
emptyMessage="No file changes in pull request"
panelId="changes-panel-pr"
grouping={grouping}
/>
);
}
60 changes: 60 additions & 0 deletions packages/ui/src/features/task-detail/components/ChangesTreeView.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import type { ChangedFile } from "@posthog/shared/domain-types";
import { describe, expect, it } from "vitest";
import {
classifyChangedFile,
type FileTypeCategory,
groupChangesByFileType,
} from "./changesTree";

const changedFile = (path: string): ChangedFile => ({
path,
status: "modified",
});

describe("classifyChangedFile", () => {
it.each<[string, FileTypeCategory]>([
["packages/core/src/service.ts", "Implementation"],
["packages/core/src/service.test.ts", "Tests"],
["tests/e2e/review.spec.ts", "Tests"],
["packages/core/service_test.go", "Tests"],
["packages/core/test_service.py", "Tests"],
["packages/api/src/__generated__/schema.ts", "Generated"],
["packages/api/src/schema.generated.ts", "Generated"],
["packages/api/src/messages.pb.go", "Generated"],
["pnpm-lock.yaml", "Generated"],
["docs/code-review.md", "Documentation"],
["README.md", "Documentation"],
[".github/workflows/ci.yml", "Configuration"],
["package.json", "Configuration"],
["requirements.txt", "Configuration"],
["packages/ui/src/assets/review.png", "Assets"],
["Makefile", "Other"],
])("classifies %s as %s", (path, expected) => {
expect(classifyChangedFile(path)).toBe(expected);
});
});

describe("groupChangesByFileType", () => {
it("orders categories and files consistently", () => {
const groups = groupChangesByFileType([
changedFile("docs/z-last.md"),
changedFile("src/z-last.ts"),
changedFile("src/a-first.ts"),
changedFile("src/service.test.ts"),
changedFile("src/schema.generated.ts"),
changedFile("package.json"),
]);

expect(groups.map((group) => group.category)).toEqual([
"Implementation",
"Tests",
"Generated",
"Documentation",
"Configuration",
]);
expect(groups[0]?.files.map((file) => file.path)).toEqual([
"src/a-first.ts",
"src/z-last.ts",
]);
});
});
Loading
Loading