root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

FN-5639: fix PR panel merge button disabled state

Browse Source 
Fusion-Task-Id: FN-5639

Fusion-Task-Lineage: 12a37f9b-895e-4116-b8e1-33315785a42a
This commit is contained in:
gsxdsm
2026-05-28 21:32:54 -07:00
parent cec191eac3
commit f5cf974504
3 changed files with 143 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
packages/dashboard/app/components/PrPanel.tsx
View File

@@ -1,4 +1,4 @@
import { useCallback, useEffect, useMemo, useState } from "react";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
import { GitPullRequest, ExternalLink, RefreshCw, Plus, MessageSquare, CircleDot, XCircle, GitMerge, ChevronDown, ChevronUp } from "lucide-react";
import { getErrorMessage, type DirectMergeCommitStrategy, type StructuredGhError } from "@fusion/core";
import { fetchPrReviews, mergePr, reclaimPrConflict, refreshPrStatus, setAutoMergeOnGreen, unlinkPr, type PrCheckStatus, type PrInfo, type PrRefreshResponse, type PrReviewsResponse } from "../api";
@@ -95,6 +95,7 @@ function PrCard({
const [mergeStrategy, setMergeStrategy] = useState<"merge" | "squash" | "rebase">(
directMergeCommitStrategy === "always-rebase" ? "rebase" : directMergeCommitStrategy === "always-squash" ? "squash" : "squash",
);
const autoRefreshedPrNumberRef = useRef<number | null>(null);
useEffect(() => {
void fetchPrReviews(taskId, projectId, prInfo.number)
@@ -102,7 +103,7 @@ function PrCard({
.catch(() => setReviewsState(null));
}, [taskId, projectId, prInfo.number]);
const handleRefresh = useCallback(async () => {
const doRefresh = useCallback(async () => {
setIsRefreshing(true);
setLastGhError(null);
try {
@@ -114,16 +115,32 @@ function PrCard({
if (targetPr) onPrUpdated(targetPr);
const latestReviews = await fetchPrReviews(taskId, projectId, prInfo.number);
setReviewsState(latestReviews);
addToast("PR status refreshed", "success");
return true;
} catch (err) {
const details = (err as { details?: { githubError?: StructuredGhError } })?.details?.githubError;
const structured = details ? { ...details, operation: "refresh" as const } : { code: "unknown" as const, message: getErrorMessage(err) || "Failed to refresh PR", retryable: true, action: { kind: "retry" as const }, operation: "refresh" as const };
setLastGhError(structured);
addToast(structured.message || "Failed to refresh PR", "error");
return false;
} finally {
setIsRefreshing(false);
}
}, [taskId, projectId, prInfo.number, onPrUpdated, onPrsRefreshed, addToast]);
}, [addToast, onPrUpdated, onPrsRefreshed, prInfo.number, projectId, taskId]);
const handleRefresh = useCallback(async () => {
const refreshed = await doRefresh();
if (refreshed) {
addToast("PR status refreshed", "success");
}
}, [addToast, doRefresh]);
useEffect(() => {
const isDraft = (prInfo.draft ?? prInfo.isDraft) === true;
if (prInfo.status !== "open" || isDraft) return;
if (autoRefreshedPrNumberRef.current === prInfo.number) return;
autoRefreshedPrNumberRef.current = prInfo.number;
void doRefresh();
}, [doRefresh, prInfo.draft, prInfo.isDraft, prInfo.number, prInfo.status]);
const handleMerge = useCallback(async () => {
setIsMerging(true);
@@ -189,9 +206,10 @@ function PrCard({
initialRollup: checkSummary,
initialLastCheckedAt: prInfo.lastCheckedAt,
});
const mergeReady = (refreshState?.mergeReady ?? false) && prInfo.status === "open";
const isOpenNonDraft = prInfo.status === "open" && (prInfo.draft ?? prInfo.isDraft) !== true;
const mergeReady = isOpenNonDraft && (refreshState?.mergeReady === true || prInfo.mergeable === "clean");
const blockingReasonsTitle = (refreshState?.blockingReasons ?? []).join("; ");
const showMergeControls = prInfo.status === "open" && (prInfo.draft ?? prInfo.isDraft) !== true;
const showMergeControls = isOpenNonDraft;
const hasConflictBlockingReason = blockingReasons.some((reason) => reason.toLowerCase().includes("conflict"));
const showConflictHint = prInfo.mergeable === "conflicting" || hasConflictBlockingReason;
const conflictDiagnostics = refreshState?.conflictDiagnostics ?? prInfo.conflictDiagnostics;

96
packages/dashboard/app/components/__tests__/PrPanel.merge.test.tsx
View File

@@ -1,5 +1,5 @@
import { describe, expect, it, vi } from "vitest";
import { fireEvent, render, screen } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
import { PrPanel } from "../PrPanel";
import { mergePr, refreshPrStatus } from "../../api";
@@ -12,6 +12,45 @@ vi.mock("../../api", () => ({
}));
describe("PrPanel merge controls", () => {
beforeEach(() => {
vi.clearAllMocks();
});
it("auto-refreshes status once on mount for open non-draft PRs", async () => {
(refreshPrStatus as ReturnType<typeof vi.fn>).mockResolvedValue({
prInfo: { url: "https://github.com/o/r/pull/1", number: 1, status: "open", title: "t", headBranch: "h", baseBranch: "main", commentCount: 0 },
checks: [],
reviewDecision: null,
blockingReasons: [],
mergeReady: false,
});
render(<PrPanel taskId="FN-1" prAuthAvailable onPrUpdated={() => {}} addToast={() => {}} prInfo={{ url: "https://github.com/o/r/pull/1", number: 1, status: "open", title: "t", headBranch: "h", baseBranch: "main", commentCount: 0 }} />);
await waitFor(() => {
expect(refreshPrStatus).toHaveBeenCalledTimes(1);
});
});
it.each([
{ status: "closed" as const, draft: false },
{ status: "merged" as const, draft: false },
{ status: "open" as const, draft: true },
])("does not auto-refresh for non-eligible PRs %#", async (state) => {
(refreshPrStatus as ReturnType<typeof vi.fn>).mockResolvedValue({
prInfo: { url: "https://github.com/o/r/pull/1", number: 1, status: state.status, title: "t", headBranch: "h", baseBranch: "main", commentCount: 0, draft: state.draft },
checks: [],
reviewDecision: null,
blockingReasons: [],
mergeReady: false,
});
render(<PrPanel taskId="FN-1" prAuthAvailable onPrUpdated={() => {}} addToast={() => {}} prInfo={{ url: "https://github.com/o/r/pull/1", number: 1, title: "t", headBranch: "h", baseBranch: "main", commentCount: 0, ...state }} />);
await waitFor(() => {
expect(refreshPrStatus).not.toHaveBeenCalled();
});
});
it.each([
[{ status: "open", draft: false }, true],
[{ status: "open", draft: true }, false],
@@ -37,7 +76,7 @@ describe("PrPanel merge controls", () => {
mergeReady: true,
all: [
{ prInfo: { url: "https://github.com/o/r/pull/1", number: 1, status: "open", title: "A", headBranch: "h1", baseBranch: "main", commentCount: 0 }, checks: [], reviewDecision: null, blockingReasons: [], mergeReady: false },
{ prInfo: { url: "https://github.com/o/r/pull/2", number: 2, status: "open", title: "B", headBranch: "h2", baseBranch: "main", commentCount: 0 }, checks: [], reviewDecision: null, blockingReasons: [], mergeReady: true },
{ prInfo: { url: "https://github.com/o/r/pull/2", number: 2, status: "open", title: "B", headBranch: "h2", baseBranch: "main", commentCount: 0, mergeable: "clean" }, checks: [], reviewDecision: null, blockingReasons: [], mergeReady: true },
],
primary: { prInfo: { url: "https://github.com/o/r/pull/2", number: 2, status: "open", title: "B", headBranch: "h2", baseBranch: "main", commentCount: 0 }, checks: [], reviewDecision: null, blockingReasons: [], mergeReady: true },
});
@@ -50,17 +89,64 @@ describe("PrPanel merge controls", () => {
addToast={() => {}}
prInfos={[
{ url: "https://github.com/o/r/pull/1", number: 1, status: "open", title: "A", headBranch: "h1", baseBranch: "main", commentCount: 0 },
{ url: "https://github.com/o/r/pull/2", number: 2, status: "open", title: "B", headBranch: "h2", baseBranch: "main", commentCount: 0 },
{ url: "https://github.com/o/r/pull/2", number: 2, status: "open", title: "B", headBranch: "h2", baseBranch: "main", commentCount: 0, mergeable: "clean" },
]}
/>,
);
fireEvent.click(screen.getAllByTitle("Refresh PR status")[1]!);
await screen.findAllByRole("button", { name: "Merge pull request" });
fireEvent.click(screen.getAllByRole("button", { name: "Merge pull request" })[1]!);
expect(mergePr).toHaveBeenCalledWith("FN-1", "squash", undefined, 2);
});
it("enables merge on first render when mergeable is clean", () => {
render(
<PrPanel
taskId="FN-1"
prAuthAvailable
onPrUpdated={() => {}}
addToast={() => {}}
prInfo={{ url: "https://github.com/o/r/pull/1", number: 1, status: "open", title: "t", headBranch: "h", baseBranch: "main", commentCount: 0, mergeable: "clean" }}
/>,
);
const mergeButton = screen.getByRole("button", { name: "Merge pull request" });
expect(mergeButton).toBeEnabled();
fireEvent.click(mergeButton);
expect(mergePr).toHaveBeenCalledWith("FN-1", "squash", undefined, 1);
});
it.each([
["conflicting", "conflicting" as const],
["unknown", "unknown" as const],
])("keeps merge disabled when mergeable is %s", (label, mergeable) => {
render(
<PrPanel
taskId="FN-1"
prAuthAvailable
onPrUpdated={() => {}}
addToast={() => {}}
prInfo={{ url: "https://github.com/o/r/pull/1", number: 1, status: "open", title: "t", headBranch: "h", baseBranch: "main", commentCount: 0, mergeable }}
/>,
);
expect(screen.getByRole("button", { name: "Merge pull request" })).toBeDisabled();
});
it("keeps merge disabled when mergeable is undefined", () => {
render(
<PrPanel
taskId="FN-1"
prAuthAvailable
onPrUpdated={() => {}}
addToast={() => {}}
prInfo={{ url: "https://github.com/o/r/pull/1", number: 1, status: "open", title: "t", headBranch: "h", baseBranch: "main", commentCount: 0 }}
/>,
);
expect(screen.getByRole("button", { name: "Merge pull request" })).toBeDisabled();
});
it("shows error block and retry", () => {
render(<PrPanel taskId="FN-1" prAuthAvailable onPrUpdated={() => {}} addToast={() => {}} prInfo={{ url: "https://github.com/o/r/pull/1", number: 1, status: "open", title: "t", headBranch: "h", baseBranch: "main", commentCount: 0, lastMergeError: "boom" }} />);
expect(screen.getByText("boom")).toBeInTheDocument();

29
packages/dashboard/app/components/__tests__/PrPanel.test.tsx
View File

@@ -135,6 +135,9 @@ describe("PrPanel", () => {
render(<PrPanel taskId="FN-001" projectId="project-1" prInfo={mockPrInfo} prAuthAvailable={true} onPrUpdated={mockOnPrUpdated} addToast={mockAddToast} />);
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
await waitFor(() => {
@@ -157,6 +160,9 @@ describe("PrPanel", () => {
});
render(<PrPanel taskId="FN-001" prInfo={mockPrInfo} prAuthAvailable={true} onPrUpdated={mockOnPrUpdated} addToast={mockAddToast} />);
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
expect(await screen.findByText("1 passing, 1 failing, 1 pending")).toBeInTheDocument();
@@ -175,6 +181,9 @@ describe("PrPanel", () => {
});
render(<PrPanel taskId="FN-001" prInfo={mockPrInfo} prAuthAvailable={true} onPrUpdated={mockOnPrUpdated} addToast={mockAddToast} />);
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
expect(await screen.findByText(/No checks reported yet/i)).toBeInTheDocument();
@@ -195,6 +204,9 @@ describe("PrPanel", () => {
});
render(<PrPanel taskId="FN-001" prInfo={{ ...mockPrInfo, status, draft: status === "draft" }} prAuthAvailable={true} onPrUpdated={mockOnPrUpdated} addToast={mockAddToast} />);
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
await screen.findByText(/View on GitHub/i);
@@ -224,12 +236,21 @@ describe("PrPanel", () => {
render(<PrPanel taskId="FN-001" prInfo={mockPrInfo} prAuthAvailable={true} onPrUpdated={mockOnPrUpdated} addToast={mockAddToast} />);
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
expect(await screen.findByText("CHANGES_REQUESTED")).toBeInTheDocument();
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
expect(await screen.findByText("APPROVED")).toBeInTheDocument();
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
expect(await screen.findByText("No reviews yet")).toBeInTheDocument();
});
@@ -424,6 +445,9 @@ describe("PrPanel", () => {
/>,
);
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
expect(await screen.findByRole("button", { name: /Retry conflict reclaim/i })).toBeInTheDocument();
});
@@ -443,7 +467,10 @@ describe("PrPanel", () => {
}))
.mockResolvedValueOnce({ prInfo: mockPrInfo, checks: [], reviewDecision: null, blockingReasons: [] });
render(<PrPanel taskId="FN-001" prInfo={mockPrInfo} prAuthAvailable={true} onPrUpdated={mockOnPrUpdated} addToast={mockAddToast} />);
render(<PrPanel taskId="FN-001" prInfo={{ ...mockPrInfo, draft: true }} prAuthAvailable={true} onPrUpdated={mockOnPrUpdated} addToast={mockAddToast} />);
await waitFor(() => {
expect(screen.getByTitle("Refresh PR status")).toBeEnabled();
});
fireEvent.click(screen.getByTitle("Refresh PR status"));
expect((await screen.findAllByText(/gh auth login/i)).length).toBeGreaterThan(0);