From 5c46d74b80a912120cd0d269e64843868881be56 Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 1 Aug 2026 08:43:48 -0500 Subject: [PATCH] fix(favorites): confirm before deleting category (closes #337) S-scope fix. Wraps the right-click 'Delete Category' in a ConfirmDialog showing 'Delete category Reports? N favorites will be moved to Uncategorized.' The store action itself was correct; missing confirmation upstream destroyed organizational work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/components/favorites/QueryFavorites.tsx | 26 ++- .../__tests__/QueryFavorites.test.tsx | 213 ++++++++++++++---- 2 files changed, 200 insertions(+), 39 deletions(-) diff --git a/src/components/favorites/QueryFavorites.tsx b/src/components/favorites/QueryFavorites.tsx index eb10e9f..7e5233e 100644 --- a/src/components/favorites/QueryFavorites.tsx +++ b/src/components/favorites/QueryFavorites.tsx @@ -14,6 +14,7 @@ import { useMemo, useState } from "react"; import { useContextMenu } from "../../hooks/useContextMenu"; import { useEditorStore } from "../../stores/editorStore"; import { type Favorite, useFavoritesStore } from "../../stores/favoritesStore"; +import { ConfirmDialog } from "../common/ConfirmDialog"; export function QueryFavorites() { const favorites = useFavoritesStore((s) => s.favorites); @@ -35,6 +36,9 @@ export function QueryFavorites() { const [editDescValue, setEditDescValue] = useState(""); const [showNewCategory, setShowNewCategory] = useState(false); const [newCategoryName, setNewCategoryName] = useState(""); + const [pendingDelete, setPendingDelete] = useState<{ cat: string; count: number } | null>( + null, + ); const { contextMenu, showContextMenu } = useContextMenu(); @@ -212,7 +216,10 @@ export function QueryFavorites() { label: "Delete Category", icon: , danger: true, - onClick: () => deleteCategory(cat), + onClick: () => { + const count = favorites.filter((f) => f.category === cat).length; + setPendingDelete({ cat, count }); + }, }, ]); } @@ -365,6 +372,23 @@ export function QueryFavorites() { )} {contextMenu} + { + if (pendingDelete) { + deleteCategory(pendingDelete.cat); + } + setPendingDelete(null); + }} + onCancel={() => setPendingDelete(null)} + /> ); } diff --git a/src/components/favorites/__tests__/QueryFavorites.test.tsx b/src/components/favorites/__tests__/QueryFavorites.test.tsx index f5e0256..d6c79af 100644 --- a/src/components/favorites/__tests__/QueryFavorites.test.tsx +++ b/src/components/favorites/__tests__/QueryFavorites.test.tsx @@ -1,16 +1,16 @@ import { fireEvent, render, screen } from "@testing-library/react"; -import { beforeAll, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { QueryFavorites } from "../QueryFavorites"; const { useFavoritesStoreFn } = vi.hoisted(() => { return { useFavoritesStoreFn: vi.fn() }; }); -vi.mock("../../stores/favoritesStore", () => ({ +vi.mock("../../../stores/favoritesStore", () => ({ useFavoritesStore: useFavoritesStoreFn, })); -vi.mock("../../stores/editorStore", () => ({ +vi.mock("../../../stores/editorStore", () => ({ useEditorStore: { getState: vi.fn(() => ({ tabs: [{ id: "tab1", type: "query", content: "" }], @@ -21,43 +21,96 @@ vi.mock("../../stores/editorStore", () => ({ }, })); -vi.mock("../../hooks/useContextMenu", () => ({ - useContextMenu: vi.fn(() => ({ contextMenu: null, showContextMenu: vi.fn() })), -})); +// Context-menu mock that renders the items as buttons so tests can click +// them. Uses real React state so updates trigger re-renders of the host. +vi.mock("../../../hooks/useContextMenu", async () => { + const { useState, useCallback } = await import("react"); + return { + useContextMenu: () => { + const [items, setItems] = useState< + { label: string; onClick: () => void; danger?: boolean; separator?: boolean }[] + >([]); + const showContextMenu = useCallback( + (_e: unknown, newItems: typeof items) => { + setItems(newItems); + }, + [], + ); + const hideContextMenu = useCallback(() => setItems([]), []); + const contextMenu = items.length > 0 + ? ( +
+ {items.map((item, i) => + item.separator + ?
+ : ( + + ) + )} +
+ ) + : null; + return { contextMenu, showContextMenu, hideContextMenu }; + }, + }; +}); + +const baselineFavorites = [ + { + id: "fav1", + name: "Get Active Users", + sql: "SELECT * FROM users WHERE active = 1", + category: "Uncategorized", + description: "Returns all active users", + createdAt: "2025-01-01", + updatedAt: "2025-01-01", + }, + { + id: "fav2", + name: "Order Summary", + sql: "SELECT COUNT(*) FROM orders", + category: "Reports", + description: "Daily order count", + connectionName: "Prod DB", + createdAt: "2025-01-02", + updatedAt: "2025-01-02", + }, +]; + +const storeState = { + favorites: baselineFavorites, + categories: ["Uncategorized", "Reports"], + deleteFavorite: vi.fn(), + renameFavorite: vi.fn(), + moveToCategory: vi.fn(), + updateFavorite: vi.fn(), + addCategory: vi.fn(), + deleteCategory: vi.fn(), +}; beforeAll(() => { - useFavoritesStoreFn.mockImplementation((s: (v: any) => unknown) => - s({ - favorites: [ - { - id: "fav1", - name: "Get Active Users", - sql: "SELECT * FROM users WHERE active = 1", - category: "Uncategorized", - description: "Returns all active users", - createdAt: "2025-01-01", - updatedAt: "2025-01-01", - }, - { - id: "fav2", - name: "Order Summary", - sql: "SELECT COUNT(*) FROM orders", - category: "Reports", - description: "Daily order count", - connectionName: "Prod DB", - createdAt: "2025-01-02", - updatedAt: "2025-01-02", - }, - ], - categories: ["Uncategorized", "Reports"], - deleteFavorite: vi.fn(), - renameFavorite: vi.fn(), - moveToCategory: vi.fn(), - updateFavorite: vi.fn(), - addCategory: vi.fn(), - deleteCategory: vi.fn(), - }) - ); + useFavoritesStoreFn.mockImplementation((s: (v: typeof storeState) => unknown) => s(storeState)); +}); + +beforeEach(() => { + storeState.deleteFavorite.mockClear(); + storeState.renameFavorite.mockClear(); + storeState.moveToCategory.mockClear(); + storeState.updateFavorite.mockClear(); + storeState.addCategory.mockClear(); + storeState.deleteCategory.mockClear(); +}); + +afterEach(() => { + storeState.favorites = baselineFavorites; + storeState.categories = ["Uncategorized", "Reports"]; }); describe("QueryFavorites", () => { @@ -76,3 +129,87 @@ describe("QueryFavorites", () => { expect(container.querySelector(".flex.h-full.flex-col")).toBeInTheDocument(); }); }); + +// ───────────────────────────────────────────────────────────────────────── +// Issue #337: confirm before deleting a category that holds favorites. +// ───────────────────────────────────────────────────────────────────────── +describe("QueryFavorites — delete category confirmation (#337)", () => { + function makeReportsFavorites(count: number) { + return Array.from({ length: count }, (_, i) => ({ + id: `rep-${i}`, + name: `Report ${i}`, + sql: `SELECT ${i} FROM reports`, + category: "Reports", + description: undefined as string | undefined, + connectionName: undefined as string | undefined, + createdAt: "2025-01-01", + updatedAt: "2025-01-01", + })); + } + + it("shows a ConfirmDialog with category name and favorite count when Delete Category is clicked", () => { + storeState.favorites = makeReportsFavorites(50); + storeState.categories = ["Uncategorized", "Reports"]; + + render(); + + const reportsHeader = screen.getByText("Reports").closest("button"); + expect(reportsHeader).not.toBeNull(); + fireEvent.contextMenu(reportsHeader!); + + const deleteBtn = screen.getByTestId("ctx-item-Delete Category"); + expect(deleteBtn).toBeInTheDocument(); + fireEvent.click(deleteBtn); + + // ConfirmDialog renders the category name, plural favorite count, and target category. + expect(screen.getByText("Delete category \"Reports\"?")).toBeInTheDocument(); + expect(screen.getByText("50 favorites will be moved to Uncategorized.")).toBeInTheDocument(); + + // Store action has NOT fired yet — user has not confirmed. + expect(storeState.deleteCategory).not.toHaveBeenCalled(); + }); + + it("calls deleteCategory when the user confirms the dialog", () => { + storeState.favorites = makeReportsFavorites(50); + storeState.categories = ["Uncategorized", "Reports"]; + + render(); + fireEvent.contextMenu(screen.getByText("Reports").closest("button")!); + fireEvent.click(screen.getByTestId("ctx-item-Delete Category")); + + // "Delete" is the confirmLabel set by QueryFavorites for danger flows. + fireEvent.click(screen.getByRole("button", { name: /^delete$/i })); + + expect(storeState.deleteCategory).toHaveBeenCalledTimes(1); + expect(storeState.deleteCategory).toHaveBeenCalledWith("Reports"); + + expect(screen.queryByText("Delete category \"Reports\"?")).not.toBeInTheDocument(); + }); + + it("does NOT call deleteCategory when the user cancels", () => { + storeState.favorites = makeReportsFavorites(50); + storeState.categories = ["Uncategorized", "Reports"]; + + render(); + fireEvent.contextMenu(screen.getByText("Reports").closest("button")!); + fireEvent.click(screen.getByTestId("ctx-item-Delete Category")); + + fireEvent.click(screen.getByRole("button", { name: /^cancel$/i })); + + expect(storeState.deleteCategory).not.toHaveBeenCalled(); + + expect(screen.queryByText("Delete category \"Reports\"?")).not.toBeInTheDocument(); + expect(screen.getByText("Reports")).toBeInTheDocument(); + }); + + it("uses the singular 'favorite' when the category has exactly one entry", () => { + storeState.favorites = makeReportsFavorites(1); + storeState.categories = ["Uncategorized", "Reports"]; + + render(); + fireEvent.contextMenu(screen.getByText("Reports").closest("button")!); + fireEvent.click(screen.getByTestId("ctx-item-Delete Category")); + + expect(screen.getByText("1 favorite will be moved to Uncategorized.")).toBeInTheDocument(); + }); +});