From 11557d629a10a65d145b25320c1e8d76887923de Mon Sep 17 00:00:00 2001 From: Elliot Date: Sat, 1 Aug 2026 08:41:09 -0500 Subject: [PATCH] fix(history): add per-entry remove UI to QueryHistory (closes #327) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S-scope fix. `removeEntry` already exists in historyStore (covered by store tests); QueryHistory never wired it. Adds hover `×` button per entry, calling removeEntry(id). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/components/history/QueryHistory.tsx | 24 ++++++-- .../history/__tests__/QueryHistory.test.tsx | 56 ++++++++++++++++++- 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/src/components/history/QueryHistory.tsx b/src/components/history/QueryHistory.tsx index 28ca250..59bf4d4 100644 --- a/src/components/history/QueryHistory.tsx +++ b/src/components/history/QueryHistory.tsx @@ -1,4 +1,4 @@ -import { CheckCircle, Clock, Search, Trash2, XCircle } from "lucide-react"; +import { CheckCircle, Clock, Search, Trash2, X, XCircle } from "lucide-react"; import { useMemo, useState } from "react"; import { useEditorStore } from "../../stores/editorStore"; import { type HistoryEntry, useHistoryStore } from "../../stores/historyStore"; @@ -48,6 +48,11 @@ export function QueryHistory() { } }; + const handleRemove = (e: React.MouseEvent, entryId: string) => { + e.stopPropagation(); + useHistoryStore.getState().removeEntry(entryId); + }; + return (
@@ -85,9 +90,20 @@ export function QueryHistory() { onClick={() => handleClick(entry)} className="group flex w-full flex-col gap-0.5 border-b border-[var(--color-border)] px-2.5 py-2 text-left hover:bg-[var(--color-bg-tertiary)]" > -
-                {entry.sql}
-                
+
+
+                    {entry.sql}
+                  
+ handleRemove(e, entry.id)} + className="inline-flex shrink-0 cursor-pointer rounded p-0.5 text-[var(--color-text-muted)] opacity-0 hover:bg-[var(--color-bg-secondary)] hover:text-red-400 group-hover:opacity-100" + > + + +
{entry.status === "success" ? diff --git a/src/components/history/__tests__/QueryHistory.test.tsx b/src/components/history/__tests__/QueryHistory.test.tsx index 4339603..d1c0f63 100644 --- a/src/components/history/__tests__/QueryHistory.test.tsx +++ b/src/components/history/__tests__/QueryHistory.test.tsx @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { QueryHistory } from "../QueryHistory"; vi.mock("../../../stores/historyStore", () => ({ - useHistoryStore: vi.fn(), + useHistoryStore: Object.assign(vi.fn(), { getState: vi.fn() }), })); vi.mock("../../../stores/editorStore", () => ({ @@ -15,6 +15,8 @@ vi.mock("../../../stores/editorStore", () => ({ import { useEditorStore } from "../../../stores/editorStore"; import { useHistoryStore } from "../../../stores/historyStore"; +const mockRemoveEntry = vi.fn(); + const mockEntries = [ { id: "entry-1", @@ -62,11 +64,19 @@ describe("QueryHistory", () => { entries: mockEntries, clearHistory: mockClearHistory, addEntry: vi.fn(), - removeEntry: vi.fn(), + removeEntry: mockRemoveEntry, }); } return mockEntries; }); + ( + useHistoryStore as unknown as { getState: ReturnType } + ).getState.mockImplementation(() => ({ + entries: mockEntries, + clearHistory: mockClearHistory, + addEntry: vi.fn(), + removeEntry: mockRemoveEntry, + })); }); it("renders history entries", () => { @@ -86,7 +96,12 @@ describe("QueryHistory", () => { it("shows empty state when no entries", () => { vi.mocked(useHistoryStore).mockImplementation((selector) => { if (typeof selector === "function") { - return selector({ entries: [], clearHistory: mockClearHistory, addEntry: vi.fn(), removeEntry: vi.fn() }); + return selector({ + entries: [], + clearHistory: mockClearHistory, + addEntry: vi.fn(), + removeEntry: mockRemoveEntry, + }); } return []; }); @@ -169,4 +184,39 @@ describe("QueryHistory", () => { const successEntries = screen.getAllByText(/users|INSERT INTO logs/); expect(successEntries.length).toBeGreaterThanOrEqual(2); }); + + it("renders a delete button for each entry", () => { + render(); + const deleteButtons = screen.getAllByLabelText("Delete entry"); + expect(deleteButtons).toHaveLength(mockEntries.length); + }); + + it("calls removeEntry with the entry id when × button is clicked", () => { + render(); + const deleteButtons = screen.getAllByLabelText("Delete entry"); + + fireEvent.click(deleteButtons[0]); + + expect(mockRemoveEntry).toHaveBeenCalledTimes(1); + expect(mockRemoveEntry).toHaveBeenCalledWith("entry-1"); + }); + + it("does not trigger entry load when × button is clicked", () => { + const mockTabs = [{ id: "tab-1", title: "Query", content: "" }]; + vi.mocked(useEditorStore.getState).mockReturnValue({ + tabs: mockTabs, + activeTabId: "tab-1", + updateTabContent: mockUpdateTabContent, + addTab: mockAddTab, + }); + + render(); + const deleteButtons = screen.getAllByLabelText("Delete entry"); + + fireEvent.click(deleteButtons[2]); + + expect(mockUpdateTabContent).not.toHaveBeenCalled(); + expect(mockAddTab).not.toHaveBeenCalled(); + expect(mockRemoveEntry).toHaveBeenCalledWith("entry-3"); + }); });