feat(KB-216): implement auto-load behavior for GitHub import modal

- Add auto-load behavior to GitHubImportModal component
- Modal now automatically fetches and displays importable repositories on open
- Update tests to cover auto-load state transitions and error handling
- Refactor test setup for improved maintainability
This commit is contained in:
gsxdsm
2026-03-30 17:58:29 -07:00
parent 859f2c53a7
commit 75189f51bc
2 changed files with 118 additions and 91 deletions

View File

@@ -25,6 +25,9 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks }: GitHubIm
const [loadingRemotes, setLoadingRemotes] = useState(false);
const [selectedRemoteName, setSelectedRemoteName] = useState<string>("");
const mountedRef = useRef(false);
// Track which owner/repo we've already auto-loaded to prevent duplicate loads
const autoLoadedRef = useRef<{ owner: string; repo: string; labels: string } | null>(null);
// Build set of already imported URLs from existing tasks
const importedUrls = new Set<string>();
@@ -48,6 +51,7 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks }: GitHubIm
setRemotes([]);
setLoadingRemotes(true);
setSelectedRemoteName("");
autoLoadedRef.current = null;
mountedRef.current = true;
@@ -100,16 +104,7 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks }: GitHubIm
}
}, [remotes]);
// Handle escape key
useEffect(() => {
if (!isOpen) return;
const handleKey = (e: KeyboardEvent) => {
if (e.key === "Escape") onClose();
};
document.addEventListener("keydown", handleKey);
return () => document.removeEventListener("keydown", handleKey);
}, [isOpen, onClose]);
// Handle load issues - defined BEFORE the auto-load useEffect
const handleLoad = useCallback(async () => {
if (!owner.trim() || !repo.trim()) {
setError("Repository must be selected");
@@ -138,6 +133,37 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks }: GitHubIm
}
}, [owner, repo, labels]);
// Auto-load issues when owner and repo are set and valid
useEffect(() => {
if (!isOpen) return;
if (!owner.trim() || !repo.trim()) return;
if (loading || importing) return;
// Check if we've already auto-loaded for this exact combination
const currentKey = { owner: owner.trim(), repo: repo.trim(), labels: labels.trim() };
if (
autoLoadedRef.current?.owner === currentKey.owner &&
autoLoadedRef.current?.repo === currentKey.repo &&
autoLoadedRef.current?.labels === currentKey.labels
) {
return;
}
// Mark as auto-loaded and trigger the load
autoLoadedRef.current = currentKey;
handleLoad();
}, [owner, repo, labels, isOpen, loading, importing, handleLoad]);
// Handle escape key
useEffect(() => {
if (!isOpen) return;
const handleKey = (e: KeyboardEvent) => {
if (e.key === "Escape") onClose();
};
document.addEventListener("keydown", handleKey);
return () => document.removeEventListener("keydown", handleKey);
}, [isOpen, onClose]);
const handleImport = useCallback(async () => {
if (selectedIssueNumber === null) return;
@@ -294,7 +320,7 @@ export function GitHubImportModal({ isOpen, onClose, onImport, tasks }: GitHubIm
onClick={handleLoad}
disabled={loading || importing || !owner.trim() || !repo.trim()}
>
{loading ? <Loader2 size={14} className="spin" /> : "Load"}
{loading ? <Loader2 size={14} className="spin" /> : "Refresh"}
</button>
<small>Load issues from the selected repository without changing any board data.</small>
</div>

View File

@@ -47,6 +47,8 @@ describe("GitHubImportModal", () => {
vi.mocked(fetchGitRemotes).mockReset();
vi.mocked(apiFetchGitHubIssues).mockReset();
vi.mocked(apiImportGitHubIssue).mockReset();
// Set default mock for apiFetchGitHubIssues to return empty array (prevents undefined issues state)
vi.mocked(apiFetchGitHubIssues).mockResolvedValue([]);
onClose.mockReset();
onImport.mockReset();
});
@@ -99,14 +101,17 @@ describe("GitHubImportModal", () => {
});
});
it("disables Load button when no remotes available", async () => {
it("disables Refresh button when no remotes available", async () => {
vi.mocked(fetchGitRemotes).mockResolvedValueOnce([]);
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
const loadButton = screen.getByRole("button", { name: /Load/i }) as HTMLButtonElement;
expect(loadButton.disabled).toBe(true);
expect(screen.getByText(/No GitHub remotes detected/)).toBeTruthy();
});
// Use id to find the button since text changes during loading
const refreshButton = screen.getByRole("button", { name: /Load issues/i }) as HTMLButtonElement;
expect(refreshButton.disabled).toBe(true);
});
});
@@ -133,31 +138,38 @@ describe("GitHubImportModal", () => {
});
});
it("enables Load button when remote is auto-selected", async () => {
vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote);
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
const loadButton = screen.getByRole("button", { name: /Load/i }) as HTMLButtonElement;
expect(loadButton.disabled).toBe(false);
});
});
it("calls apiFetchGitHubIssues when Load is clicked", async () => {
it("enables Refresh button when remote is auto-selected", async () => {
vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote);
// Mock empty response so loading finishes quickly
vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce([]);
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
// Wait for auto-load to complete (issues appear or empty state shows)
await waitFor(() => {
expect(screen.getByTestId("github-import-single-remote")).toBeTruthy();
// Either no issues found message or the results section updates
const resultsSection = screen.queryByText(/No open issues found/) || screen.queryByTestId("github-import-results-idle");
expect(resultsSection).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
// Use id to find the button since text changes during loading
const refreshButton = screen.getByRole("button", { name: /Load issues/i }) as HTMLButtonElement;
expect(refreshButton.disabled).toBe(false);
});
it("auto-loads issues when single remote is detected", async () => {
vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote);
vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce([
{ number: 1, title: "Auto-loaded Issue", body: "Body", html_url: "https://github.com/dustinbyrne/kb/issues/1", labels: [] },
]);
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined);
});
expect(screen.getByText("Auto-loaded Issue")).toBeTruthy();
});
});
@@ -184,18 +196,25 @@ describe("GitHubImportModal", () => {
});
});
it("disables Load button when no remote is selected", async () => {
it("disables Refresh button when no remote is selected", async () => {
vi.mocked(fetchGitRemotes).mockResolvedValueOnce(multipleRemotes);
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
const loadButton = screen.getByRole("button", { name: /Load/i }) as HTMLButtonElement;
expect(loadButton.disabled).toBe(true);
expect(screen.getByRole("combobox")).toBeTruthy();
});
// Use id to find the button since text changes during loading
const refreshButton = screen.getByRole("button", { name: /Load issues/i }) as HTMLButtonElement;
expect(refreshButton.disabled).toBe(true);
});
it("enables Load button after selecting a remote", async () => {
it("enables Refresh button and auto-loads after selecting a remote", async () => {
vi.mocked(fetchGitRemotes).mockResolvedValueOnce(multipleRemotes);
vi.mocked(apiFetchGitHubIssues).mockResolvedValueOnce([
{ number: 1, title: "Auto-loaded from origin", body: "", html_url: "https://github.com/dustinbyrne/kb/issues/1", labels: [] },
]);
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
@@ -204,11 +223,17 @@ describe("GitHubImportModal", () => {
fireEvent.change(screen.getByRole("combobox"), { target: { value: "origin" } });
const loadButton = screen.getByRole("button", { name: /Load/i }) as HTMLButtonElement;
expect(loadButton.disabled).toBe(false);
await waitFor(() => {
expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined);
expect(screen.getByText("Auto-loaded from origin")).toBeTruthy();
});
// After loading completes, button should be enabled
const refreshButton = screen.getByRole("button", { name: /Load issues/i }) as HTMLButtonElement;
expect(refreshButton.disabled).toBe(false);
});
it("switches owner/repo when changing remote selection", async () => {
it("switches owner/repo and auto-loads when changing remote selection", async () => {
vi.mocked(fetchGitRemotes).mockResolvedValueOnce(multipleRemotes);
vi.mocked(apiFetchGitHubIssues)
.mockResolvedValueOnce([{ number: 1, title: "Issue from origin", body: "", html_url: "https://github.com/dustinbyrne/kb/issues/1", labels: [] }])
@@ -222,7 +247,6 @@ describe("GitHubImportModal", () => {
const select = screen.getByRole("combobox");
fireEvent.change(select, { target: { value: "origin" } });
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined);
@@ -230,7 +254,6 @@ describe("GitHubImportModal", () => {
});
fireEvent.change(select, { target: { value: "upstream" } });
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(apiFetchGitHubIssues).toHaveBeenLastCalledWith("upstream", "kb", 30, undefined);
@@ -240,7 +263,7 @@ describe("GitHubImportModal", () => {
});
describe("issue loading and import", () => {
it("displays fetched issues after loading", async () => {
it("displays auto-loaded issues for single remote", async () => {
const issues = [
{ number: 1, title: "First Issue", body: "Body 1", html_url: "https://github.com/owner/repo/issues/1", labels: [] },
{ number: 2, title: "Second Issue", body: "Body 2", html_url: "https://github.com/owner/repo/issues/2", labels: [] },
@@ -250,12 +273,6 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-single-remote")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(screen.getByText("First Issue")).toBeTruthy();
expect(screen.getByText("Second Issue")).toBeTruthy();
@@ -271,12 +288,7 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-preview-empty")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
// Issues auto-load, so we wait for them to appear first
await waitFor(() => {
expect(screen.getByText("First Issue")).toBeTruthy();
});
@@ -298,12 +310,6 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-single-remote")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(screen.getByText("First Issue")).toBeTruthy();
});
@@ -321,12 +327,6 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-single-remote")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(screen.getByText("First Issue")).toBeTruthy();
});
@@ -354,12 +354,6 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[existingTask]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-single-remote")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(screen.getByText("Imported")).toBeTruthy();
});
@@ -378,12 +372,6 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[existingTask]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-single-remote")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
const radio = screen.getByRole("radio", { name: /Select issue #1/i }) as HTMLInputElement;
expect(radio.disabled).toBe(true);
@@ -396,12 +384,6 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-results-idle")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(screen.getByText("No open issues found")).toBeTruthy();
expect(screen.getByText(/Try a different label filter/)).toBeTruthy();
@@ -414,12 +396,6 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-single-remote")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(screen.getByText("Could not load issues")).toBeTruthy();
expect(screen.getByText("Repository not found")).toBeTruthy();
@@ -435,17 +411,42 @@ describe("GitHubImportModal", () => {
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
await waitFor(() => {
expect(screen.getByTestId("github-import-single-remote")).toBeTruthy();
});
fireEvent.click(screen.getByRole("button", { name: /Load/i }));
await waitFor(() => {
expect(screen.getByText("bug")).toBeTruthy();
expect(screen.getByText("urgent")).toBeTruthy();
});
});
it("re-fetches issues when Refresh is clicked with different labels", async () => {
// Set up mocks - first for auto-load, second for manual refresh
vi.mocked(apiFetchGitHubIssues)
.mockResolvedValueOnce([{ number: 1, title: "Issue without labels", body: "", html_url: "https://github.com/dustinbyrne/kb/issues/1", labels: [] }])
.mockResolvedValueOnce([{ number: 2, title: "Bug issue", body: "", html_url: "https://github.com/dustinbyrne/kb/issues/2", labels: [{ name: "bug" }] }]);
vi.mocked(fetchGitRemotes).mockResolvedValueOnce(singleRemote);
render(<GitHubImportModal isOpen={true} onClose={onClose} onImport={onImport} tasks={[]} />);
// Wait for initial auto-load without labels
await waitFor(() => {
expect(apiFetchGitHubIssues).toHaveBeenCalledWith("dustinbyrne", "kb", 30, undefined);
expect(screen.getByText("Issue without labels")).toBeTruthy();
});
// Enter label filter
const labelsInput = screen.getByLabelText(/Labels/);
fireEvent.change(labelsInput, { target: { value: "bug" } });
// Find and click the Refresh button (use aria-label since text changes)
const refreshButton = screen.getByRole("button", { name: /Load issues/i });
fireEvent.click(refreshButton);
// Verify re-fetch with labels
await waitFor(() => {
expect(apiFetchGitHubIssues).toHaveBeenLastCalledWith("dustinbyrne", "kb", 30, ["bug"]);
expect(screen.getByText("Bug issue")).toBeTruthy();
});
});
});
describe("modal actions", () => {