feat(FN-751): add Commits tab with diff viewer to TaskDetailModal
- Create CommitDiffTab component with commit list and inline diff display - Integrate Commits tab into TaskDetailModal with conditional visibility - Add comprehensive tests for CommitDiffTab rendering, selection, and diff display - Add TaskDetailModal tests for tab visibility based on task PR info - Remove stale changeset and unused store test code
This commit is contained in:
258
packages/dashboard/app/components/CommitDiffTab.tsx
Normal file
258
packages/dashboard/app/components/CommitDiffTab.tsx
Normal file
@@ -0,0 +1,258 @@
|
||||
import { useState, useEffect, useCallback } from "react";
|
||||
import { FileCode, ChevronDown, ChevronRight, AlertCircle, GitCommit } from "lucide-react";
|
||||
import type { MergeDetails } from "@fusion/core";
|
||||
import { fetchCommitDiff } from "../api";
|
||||
import { highlightDiff } from "../utils/highlightDiff";
|
||||
|
||||
interface CommitDiffTabProps {
|
||||
commitSha: string;
|
||||
mergeDetails?: MergeDetails;
|
||||
}
|
||||
|
||||
interface ParsedFile {
|
||||
path: string;
|
||||
status: "added" | "modified" | "deleted" | "unknown";
|
||||
additions: number;
|
||||
deletions: number;
|
||||
patch: string;
|
||||
}
|
||||
|
||||
function getStatusColor(status: ParsedFile["status"]): string {
|
||||
switch (status) {
|
||||
case "added":
|
||||
return "#3fb950";
|
||||
case "deleted":
|
||||
return "#f85149";
|
||||
case "modified":
|
||||
return "#58a6ff";
|
||||
default:
|
||||
return "#8b949e";
|
||||
}
|
||||
}
|
||||
|
||||
function getStatusLabel(status: ParsedFile["status"]): string {
|
||||
switch (status) {
|
||||
case "added":
|
||||
return "A";
|
||||
case "deleted":
|
||||
return "D";
|
||||
case "modified":
|
||||
return "M";
|
||||
default:
|
||||
return "?";
|
||||
}
|
||||
}
|
||||
|
||||
function parsePatch(rawPatch: string): ParsedFile[] {
|
||||
const files: ParsedFile[] = [];
|
||||
// Split on diff boundaries, keeping the delimiter
|
||||
const parts = rawPatch.split(/(?=^diff --git )/m);
|
||||
|
||||
for (const part of parts) {
|
||||
const trimmed = part.trim();
|
||||
if (!trimmed.startsWith("diff --git ")) continue;
|
||||
|
||||
// Extract file path from "diff --git a/path b/path"
|
||||
const headerMatch = trimmed.match(/^diff --git a\/(.+?) b\/(.+)/m);
|
||||
const path = headerMatch ? headerMatch[2] : "unknown";
|
||||
|
||||
// Determine status
|
||||
let status: ParsedFile["status"] = "modified";
|
||||
if (trimmed.includes("new file mode")) status = "added";
|
||||
else if (trimmed.includes("deleted file mode")) status = "deleted";
|
||||
|
||||
// Count additions and deletions from diff lines
|
||||
let additions = 0;
|
||||
let deletions = 0;
|
||||
const lines = trimmed.split("\n");
|
||||
for (const line of lines) {
|
||||
if (line.startsWith("+") && !line.startsWith("+++")) additions++;
|
||||
else if (line.startsWith("-") && !line.startsWith("---")) deletions++;
|
||||
}
|
||||
|
||||
files.push({ path, status, additions, deletions, patch: trimmed });
|
||||
}
|
||||
|
||||
return files;
|
||||
}
|
||||
|
||||
/**
|
||||
* CommitDiffTab displays the file-by-file diff for a merge commit.
|
||||
*
|
||||
* It fetches the diff using the commit SHA from `mergeDetails` and renders
|
||||
* an expandable file list with syntax-highlighted diff output, similar to
|
||||
* the in-progress `TaskChangesTab` but sourced from git history.
|
||||
*/
|
||||
export function CommitDiffTab({ commitSha, mergeDetails }: CommitDiffTabProps) {
|
||||
const [files, setFiles] = useState<ParsedFile[]>([]);
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [expandedFiles, setExpandedFiles] = useState<Set<string>>(new Set());
|
||||
|
||||
const loadDiff = useCallback(async () => {
|
||||
if (!commitSha) {
|
||||
setLoading(false);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
const data = await fetchCommitDiff(commitSha);
|
||||
const parsed = parsePatch(data.patch || "");
|
||||
setFiles(parsed);
|
||||
// Auto-expand first file
|
||||
if (parsed.length > 0) {
|
||||
setExpandedFiles(new Set([parsed[0].path]));
|
||||
}
|
||||
} catch (err: any) {
|
||||
setError(err.message || "Failed to load commit diff");
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
}, [commitSha]);
|
||||
|
||||
useEffect(() => {
|
||||
loadDiff();
|
||||
}, [loadDiff]);
|
||||
|
||||
const toggleFile = (path: string) => {
|
||||
setExpandedFiles((prev) => {
|
||||
const next = new Set(prev);
|
||||
if (next.has(path)) {
|
||||
next.delete(path);
|
||||
} else {
|
||||
next.add(path);
|
||||
}
|
||||
return next;
|
||||
});
|
||||
};
|
||||
|
||||
if (!commitSha) {
|
||||
return (
|
||||
<div className="detail-section">
|
||||
<div className="task-changes-state task-changes-state--empty">
|
||||
<GitCommit size={24} />
|
||||
<p>No commit SHA available.</p>
|
||||
<span className="task-changes-state-hint">
|
||||
Commit diff is only available for tasks that were merged.
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
if (loading) {
|
||||
return (
|
||||
<div className="detail-section">
|
||||
<div className="task-changes-state task-changes-state--loading">
|
||||
<div className="loading-spinner" />
|
||||
<span>Loading commit diff...</span>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
if (error) {
|
||||
return (
|
||||
<div className="detail-section">
|
||||
<div className="task-changes-state task-changes-state--error">
|
||||
<AlertCircle size={16} />
|
||||
<span>Error loading commit diff: {error}</span>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
if (files.length === 0) {
|
||||
return (
|
||||
<div className="detail-section">
|
||||
<div className="task-changes-state task-changes-state--empty">
|
||||
<FileCode size={24} />
|
||||
<p>No files changed in this commit.</p>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
const totalAdditions = mergeDetails?.insertions ?? files.reduce((sum, f) => sum + f.additions, 0);
|
||||
const totalDeletions = mergeDetails?.deletions ?? files.reduce((sum, f) => sum + f.deletions, 0);
|
||||
const totalFiles = mergeDetails?.filesChanged ?? files.length;
|
||||
|
||||
return (
|
||||
<div className="detail-section task-changes-tab">
|
||||
{/* Commit metadata */}
|
||||
{mergeDetails && (
|
||||
<div className="commit-diff-meta">
|
||||
<div className="commit-diff-sha">
|
||||
<GitCommit size={14} />
|
||||
<code>{commitSha.slice(0, 7)}</code>
|
||||
</div>
|
||||
{mergeDetails.mergeCommitMessage && (
|
||||
<div className="commit-diff-message">{mergeDetails.mergeCommitMessage}</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
||||
<div className="changes-header">
|
||||
<h4>
|
||||
<FileCode size={16} />
|
||||
Files Changed ({totalFiles})
|
||||
<span className="changes-stat-summary">
|
||||
<span className="diff-add">+{totalAdditions}</span>{" "}
|
||||
<span className="diff-del">-{totalDeletions}</span>
|
||||
</span>
|
||||
</h4>
|
||||
<button className="btn btn-sm" onClick={loadDiff} disabled={loading}>
|
||||
Refresh
|
||||
</button>
|
||||
</div>
|
||||
|
||||
<div className="changes-file-list">
|
||||
{files.map((file) => {
|
||||
const isExpanded = expandedFiles.has(file.path);
|
||||
|
||||
return (
|
||||
<div
|
||||
key={file.path}
|
||||
className={`changes-file-item ${isExpanded ? "expanded" : ""}`}
|
||||
>
|
||||
<button
|
||||
className="changes-file-header"
|
||||
onClick={() => toggleFile(file.path)}
|
||||
>
|
||||
<span className="changes-file-toggle">
|
||||
{isExpanded ? <ChevronDown size={14} /> : <ChevronRight size={14} />}
|
||||
</span>
|
||||
<span
|
||||
className="changes-file-status"
|
||||
style={{ color: getStatusColor(file.status) }}
|
||||
title={file.status}
|
||||
>
|
||||
{getStatusLabel(file.status)}
|
||||
</span>
|
||||
<span className="changes-file-path" title={file.path}>
|
||||
{file.path}
|
||||
</span>
|
||||
<span
|
||||
className="changes-file-stat"
|
||||
title={`+${file.additions} -${file.deletions}`}
|
||||
>
|
||||
+{file.additions} -{file.deletions}
|
||||
</span>
|
||||
</button>
|
||||
|
||||
{isExpanded && file.patch && (
|
||||
<div className="changes-file-content">
|
||||
<pre className="changes-diff-patch">
|
||||
<code>{highlightDiff(file.patch)}</code>
|
||||
</pre>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -13,6 +13,7 @@ import { PrSection } from "./PrSection";
|
||||
import { TaskComments } from "./TaskComments";
|
||||
import { MergeDetails } from "./MergeDetails";
|
||||
import { TaskChangesTab } from "./TaskChangesTab";
|
||||
import { CommitDiffTab } from "./CommitDiffTab";
|
||||
import { TaskForm, type PendingImage } from "./TaskForm";
|
||||
|
||||
interface ModelSelection {
|
||||
@@ -108,7 +109,7 @@ export function TaskDetailModal({
|
||||
addToast,
|
||||
githubTokenConfigured,
|
||||
}: TaskDetailModalProps) {
|
||||
const [activeTab, setActiveTab] = useState<"definition" | "activity" | "agent-log" | "changes" | "comments" | "model">("definition");
|
||||
const [activeTab, setActiveTab] = useState<"definition" | "activity" | "agent-log" | "changes" | "commits" | "comments" | "model">("definition");
|
||||
const [attachments, setAttachments] = useState<TaskAttachment[]>(task.attachments || []);
|
||||
const [uploading, setUploading] = useState(false);
|
||||
const [dependencies, setDependencies] = useState<string[]>(task.dependencies || []);
|
||||
@@ -716,6 +717,14 @@ export function TaskDetailModal({
|
||||
Changes
|
||||
</button>
|
||||
)}
|
||||
{task.column === "done" && task.mergeDetails?.commitSha && (
|
||||
<button
|
||||
className={`detail-tab${activeTab === "commits" ? " detail-tab-active" : ""}`}
|
||||
onClick={() => setActiveTab("commits")}
|
||||
>
|
||||
Commits
|
||||
</button>
|
||||
)}
|
||||
<button
|
||||
className={`detail-tab${activeTab === "comments" ? " detail-tab-active" : ""}`}
|
||||
onClick={() => setActiveTab("comments")}
|
||||
@@ -744,6 +753,8 @@ export function TaskDetailModal({
|
||||
</div>
|
||||
) : activeTab === "changes" ? (
|
||||
<TaskChangesTab taskId={task.id} worktree={task.worktree} projectId={projectId} />
|
||||
) : activeTab === "commits" ? (
|
||||
<CommitDiffTab commitSha={task.mergeDetails?.commitSha ?? ""} mergeDetails={task.mergeDetails} />
|
||||
) : activeTab === "comments" ? (
|
||||
<TaskComments task={task} addToast={addToast} projectId={projectId} />
|
||||
) : activeTab === "activity" ? (
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
|
||||
import { CommitDiffTab } from "../CommitDiffTab";
|
||||
import type { MergeDetails } from "@fusion/core";
|
||||
|
||||
const mockFetchCommitDiff = vi.fn();
|
||||
|
||||
vi.mock("../../api", () => ({
|
||||
fetchCommitDiff: (...args: any[]) => mockFetchCommitDiff(...args),
|
||||
}));
|
||||
|
||||
vi.mock("lucide-react", () => ({
|
||||
FileCode: () => null,
|
||||
ChevronDown: ({ size }: any) => <span data-testid="chevron-down" />,
|
||||
ChevronRight: ({ size }: any) => <span data-testid="chevron-right" />,
|
||||
AlertCircle: () => null,
|
||||
GitCommit: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("../../utils/highlightDiff", () => ({
|
||||
highlightDiff: (diff: string) => diff,
|
||||
}));
|
||||
|
||||
const SAMPLE_PATCH = `diff --git a/src/app.ts b/src/app.ts
|
||||
index abc1234..def5678 100644
|
||||
--- a/src/app.ts
|
||||
+++ b/src/app.ts
|
||||
@@ -1,3 +1,4 @@
|
||||
import express from "express";
|
||||
+import cors from "cors";
|
||||
const app = express();
|
||||
app.listen(3000);
|
||||
diff --git a/src/new-file.ts b/src/new-file.ts
|
||||
new file mode 100644
|
||||
index 0000000..abc1234
|
||||
--- /dev/null
|
||||
+++ b/src/new-file.ts
|
||||
@@ -0,0 +1,2 @@
|
||||
+export function hello() {}
|
||||
+export function world() {}
|
||||
diff --git a/src/removed.ts b/src/removed.ts
|
||||
deleted file mode 100644
|
||||
index abc1234..0000000
|
||||
--- a/src/removed.ts
|
||||
+++ /dev/null
|
||||
@@ -1 +0,0 @@
|
||||
-export const old = true;`;
|
||||
|
||||
const MERGE_DETAILS: MergeDetails = {
|
||||
commitSha: "abc1234567890def",
|
||||
filesChanged: 3,
|
||||
insertions: 3,
|
||||
deletions: 1,
|
||||
mergeCommitMessage: "Merge branch 'fusion/fn-001' into main",
|
||||
mergedAt: "2026-01-01T00:00:00Z",
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
mockFetchCommitDiff.mockReset();
|
||||
});
|
||||
|
||||
describe("CommitDiffTab", () => {
|
||||
it("renders loading state initially", () => {
|
||||
mockFetchCommitDiff.mockReturnValue(new Promise(() => {})); // never resolves
|
||||
render(<CommitDiffTab commitSha="abc123" />);
|
||||
expect(screen.getByText("Loading commit diff...")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("renders error state when fetch fails", async () => {
|
||||
mockFetchCommitDiff.mockRejectedValue(new Error("Network error"));
|
||||
render(<CommitDiffTab commitSha="abc123" />);
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText(/Error loading commit diff: Network error/)).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("renders empty state when no commit SHA", () => {
|
||||
render(<CommitDiffTab commitSha="" />);
|
||||
expect(screen.getByText("No commit SHA available.")).toBeTruthy();
|
||||
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("renders file list with correct file paths and statuses after successful fetch", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
render(<CommitDiffTab commitSha="abc123" mergeDetails={MERGE_DETAILS} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("src/app.ts")).toBeTruthy();
|
||||
});
|
||||
|
||||
expect(screen.getByText("src/new-file.ts")).toBeTruthy();
|
||||
expect(screen.getByText("src/removed.ts")).toBeTruthy();
|
||||
|
||||
// Check status badges - M for modified, A for added, D for deleted
|
||||
const statusElements = screen.getAllByTitle(/added|modified|deleted/);
|
||||
expect(statusElements).toHaveLength(3);
|
||||
expect(statusElements.find((el) => el.textContent === "M" && el.title === "modified")).toBeTruthy();
|
||||
expect(statusElements.find((el) => el.textContent === "A" && el.title === "added")).toBeTruthy();
|
||||
expect(statusElements.find((el) => el.textContent === "D" && el.title === "deleted")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("toggling file expansion shows/hides diff content", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
const { container } = render(<CommitDiffTab commitSha="abc123" />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("src/app.ts")).toBeTruthy();
|
||||
});
|
||||
|
||||
// First file should be auto-expanded
|
||||
let diffBlocks = container.querySelectorAll(".changes-file-content");
|
||||
expect(diffBlocks.length).toBe(1);
|
||||
|
||||
// Click on first file to collapse it
|
||||
fireEvent.click(screen.getByText("src/app.ts"));
|
||||
diffBlocks = container.querySelectorAll(".changes-file-content");
|
||||
expect(diffBlocks.length).toBe(0);
|
||||
|
||||
// Click on second file to expand it
|
||||
fireEvent.click(screen.getByText("src/new-file.ts"));
|
||||
diffBlocks = container.querySelectorAll(".changes-file-content");
|
||||
expect(diffBlocks.length).toBe(1);
|
||||
});
|
||||
|
||||
it("displays merge commit metadata (SHA and message)", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
render(<CommitDiffTab commitSha="abc1234567890def" mergeDetails={MERGE_DETAILS} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("abc1234")).toBeTruthy(); // short SHA
|
||||
});
|
||||
expect(screen.getByText("Merge branch 'fusion/fn-001' into main")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("renders empty file list when patch has no diffs", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: "" });
|
||||
render(<CommitDiffTab commitSha="abc123" />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("No files changed in this commit.")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("uses mergeDetails stats for summary when available", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
render(<CommitDiffTab commitSha="abc123" mergeDetails={MERGE_DETAILS} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("Files Changed (3)")).toBeTruthy();
|
||||
});
|
||||
expect(screen.getByText("+3")).toBeTruthy();
|
||||
expect(screen.getByText("-1")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
describe("CommitDiffTab in TaskDetailModal", () => {
|
||||
// These tests verify tab visibility logic via the CommitDiffTab integration
|
||||
// Testing against the TaskDetailModal directly would require extensive mocking
|
||||
// Instead we verify the component renders correctly for valid/invalid props
|
||||
|
||||
it("does not fetch when commitSha is empty (non-done task scenario)", () => {
|
||||
render(<CommitDiffTab commitSha="" />);
|
||||
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
|
||||
expect(screen.getByText("No commit SHA available.")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
@@ -2861,4 +2861,81 @@ describe("TaskDetailModal", () => {
|
||||
expect(descTextarea.value).toBe("My Description");
|
||||
});
|
||||
});
|
||||
|
||||
describe("Commits tab visibility", () => {
|
||||
it("shows Commits tab for done tasks with mergeDetails.commitSha", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({
|
||||
column: "done",
|
||||
mergeDetails: { commitSha: "abc1234567890", filesChanged: 3, insertions: 10, deletions: 2 },
|
||||
})}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByText("Commits")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("does NOT show Commits tab for non-done tasks", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({
|
||||
column: "in-progress",
|
||||
mergeDetails: { commitSha: "abc1234567890" },
|
||||
})}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByText("Commits")).toBeNull();
|
||||
});
|
||||
|
||||
it("does NOT show Commits tab for done tasks without commitSha", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({
|
||||
column: "done",
|
||||
mergeDetails: { filesChanged: 3 },
|
||||
})}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByText("Commits")).toBeNull();
|
||||
});
|
||||
|
||||
it("does NOT show Commits tab for done tasks without mergeDetails", () => {
|
||||
render(
|
||||
<TaskDetailModal
|
||||
task={makeTask({
|
||||
column: "done",
|
||||
})}
|
||||
onClose={noop}
|
||||
onMoveTask={noopMove}
|
||||
onDeleteTask={noopDelete}
|
||||
onMergeTask={noopMerge}
|
||||
onOpenDetail={noopOpenDetail}
|
||||
addToast={noop}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByText("Commits")).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user