Command Menu improvements (#857)

* Add dialogs for reset and clear actions + clear stores * Catch failures * Improve tests, don't reset store on failure * Remove unnecessary i18n string --------- Co-authored-by: philon- <philon-@users.noreply.github.com>
2025-09-18 08:13:19 -04:00
parent 1bdd923365
commit 8cc451546d
14 changed files with 567 additions and 18 deletions
@@ -25,7 +25,6 @@ import {
  EraserIcon,
  FactoryIcon,
  HardDriveUpload,
-  LayersIcon,
  LinkIcon,
  type LucideIcon,
  MapIcon,
@@ -71,7 +70,7 @@ export const CommandPalette = () => {
  } = useAppStore();
  const { getDevices } = useDeviceStore();
  const { setDialogOpen, connection } = useDevice();
-  const { getNode, removeAllNodeErrors, removeAllNodes } = useNodeDB();
+  const { getNode } = useNodeDB();
  const { pinnedItems, togglePinnedItem } = usePinnedItems({
    storageName: "pinnedCommandMenuGroups",
  });
@@ -106,13 +105,6 @@ export const CommandPalette = () => {
          },
          tags: ["settings"],
        },
-        {
-          label: t("goto.command.channels"),
-          icon: LayersIcon,
-          action() {
-            navigate({ to: "/channels" });
-          },
-        },
        {
          label: t("goto.command.nodes"),
          icon: UsersIcon,
@@ -206,9 +198,7 @@ export const CommandPalette = () => {
          label: t("contextual.command.resetNodeDb"),
          icon: TrashIcon,
          action() {
-            connection?.resetNodes();
-            removeAllNodeErrors();
-            removeAllNodes(true);
+            setDialogOpen("resetNodeDb", true);
          },
        },
        {
@@ -224,16 +214,14 @@ export const CommandPalette = () => {
          label: t("contextual.command.factoryResetDevice"),
          icon: FactoryIcon,
          action() {
-            connection?.factoryResetDevice();
-            removeAllNodeErrors();
-            removeAllNodes();
+            setDialogOpen("factoryResetDevice", true);
          },
        },
        {
          label: t("contextual.command.factoryResetConfig"),
          icon: FactoryIcon,
          action() {
-            connection?.factoryResetConfig();
+            setDialogOpen("factoryResetConfig", true);
          },
        },
      ],
@@ -257,6 +245,13 @@ export const CommandPalette = () => {
            setDialogOpen("deleteMessages", true);
          },
        },
+        {
+          label: t("debug.command.clearAllStores"),
+          icon: EraserIcon,
+          action() {
+            setDialogOpen("clearAllStores", true);
+          },
+        },
      ],
    },
  ];
@@ -0,0 +1,69 @@
+import { fireEvent, render, screen } from "@testing-library/react";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { ClearAllStoresDialog } from "./ClearAllStoresDialog.tsx";
+
+const mockClearAllStores = vi.fn();
+
+vi.mock("@core/stores", () => ({
+  CurrentDeviceContext: {
+    _currentValue: { deviceId: 1234 },
+  },
+  clearAllStores: () => mockClearAllStores(),
+}));
+
+describe("ClearAllStoresDialog", () => {
+  const mockOnOpenChange = vi.fn();
+
+  // Capture window.location.href assignment without triggering real navigation
+  const originalLocation = window.location;
+  let assignedHref: string | undefined;
+
+  beforeEach(() => {
+    mockOnOpenChange.mockClear();
+    mockClearAllStores.mockClear();
+    assignedHref = undefined;
+
+    Object.defineProperty(window, "location", {
+      configurable: true,
+      value: {
+        ...originalLocation,
+        get href() {
+          return originalLocation.href;
+        },
+        set href(val: string) {
+          assignedHref = val;
+        },
+      },
+    });
+  });
+
+  // restore the real location object after each test
+  afterEach(() => {
+    Object.defineProperty(window, "location", {
+      configurable: true,
+      value: originalLocation,
+    });
+  });
+
+  it("calls clearAllStores and navigates to '/' when confirm is clicked", () => {
+    render(<ClearAllStoresDialog open onOpenChange={mockOnOpenChange} />);
+    fireEvent.click(
+      screen.getByRole("button", { name: "Clear all local storage" }),
+    );
+
+    expect(mockClearAllStores).toHaveBeenCalledTimes(1);
+    expect(assignedHref).toBe("/"); // forced reload target
+    // We reload instead of toggling the dialog, so ensure we didn't call onOpenChange
+    expect(mockOnOpenChange).not.toHaveBeenCalled();
+  });
+
+  it("calls onOpenChange with false when cancel is clicked", () => {
+    render(<ClearAllStoresDialog open onOpenChange={mockOnOpenChange} />);
+    fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
+
+    expect(mockClearAllStores).not.toHaveBeenCalled();
+    expect(assignedHref).toBeUndefined(); // no navigation
+    expect(mockOnOpenChange).toHaveBeenCalledTimes(1);
+    expect(mockOnOpenChange).toHaveBeenCalledWith(false);
+  });
+});
@@ -0,0 +1,35 @@
+import { clearAllStores } from "@core/stores";
+import { useTranslation } from "react-i18next";
+import { DialogWrapper } from "../DialogWrapper.tsx";
+
+export interface ClearAllStoresDialogProps {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+}
+
+export const ClearAllStoresDialog = ({
+  open,
+  onOpenChange,
+}: ClearAllStoresDialogProps) => {
+  const { t } = useTranslation("dialog");
+
+  const handleClearAllStores = () => {
+    clearAllStores();
+
+    // Reload the app to ensure all state is cleared
+    window.location.href = "/";
+  };
+
+  return (
+    <DialogWrapper
+      open={open}
+      onOpenChange={onOpenChange}
+      type="confirm"
+      variant="destructive"
+      title={t("clearAllStores.title")}
+      description={t("clearAllStores.description")}
+      confirmText={t("clearAllStores.confirm")}
+      onConfirm={handleClearAllStores}
+    />
+  );
+};
@@ -1,3 +1,6 @@
+import { FactoryResetConfigDialog } from "@app/components/Dialog/FactoryResetConfigDialog/FactoryResetConfigDialog";
+import { FactoryResetDeviceDialog } from "@app/components/Dialog/FactoryResetDeviceDialog/FactoryResetDeviceDialog";
+import { ClearAllStoresDialog } from "@components/Dialog/ClearAllStoresDialog/ClearAllStoresDialog.tsx";
 import { ClientNotificationDialog } from "@components/Dialog/ClientNotificationDialog/ClientNotificationDialog.tsx";
 import { DeleteMessagesDialog } from "@components/Dialog/DeleteMessagesDialog/DeleteMessagesDialog.tsx";
 import { DeviceNameDialog } from "@components/Dialog/DeviceNameDialog.tsx";
@@ -8,6 +11,7 @@ import { QRDialog } from "@components/Dialog/QRDialog.tsx";
 import { RebootDialog } from "@components/Dialog/RebootDialog.tsx";
 import { RefreshKeysDialog } from "@components/Dialog/RefreshKeysDialog/RefreshKeysDialog.tsx";
 import { RemoveNodeDialog } from "@components/Dialog/RemoveNodeDialog.tsx";
+import { ResetNodeDbDialog } from "@components/Dialog/ResetNodeDbDialog/ResetNodeDbDialog.tsx";
 import { ShutdownDialog } from "@components/Dialog/ShutdownDialog.tsx";
 import { UnsafeRolesDialog } from "@components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.tsx";
 import { useDevice } from "@core/stores";
@@ -91,6 +95,30 @@ export const DialogManager = () => {
          setDialogOpen("clientNotification", open);
        }}
      />
+      <ResetNodeDbDialog
+        open={dialog.resetNodeDb}
+        onOpenChange={(open) => {
+          setDialogOpen("resetNodeDb", open);
+        }}
+      />
+      <ClearAllStoresDialog
+        open={dialog.clearAllStores}
+        onOpenChange={(open) => {
+          setDialogOpen("clearAllStores", open);
+        }}
+      />
+      <FactoryResetDeviceDialog
+        open={dialog.factoryResetDevice}
+        onOpenChange={(open) => {
+          setDialogOpen("factoryResetDevice", open);
+        }}
+      />
+      <FactoryResetConfigDialog
+        open={dialog.factoryResetConfig}
+        onOpenChange={(open) => {
+          setDialogOpen("factoryResetConfig", open);
+        }}
+      />
    </>
  );
 };
@@ -0,0 +1,63 @@
+import { fireEvent, render, screen, waitFor } from "@testing-library/react";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { FactoryResetConfigDialog } from "./FactoryResetConfigDialog.tsx";
+
+const mockFactoryReset = vi.fn();
+const mockToast = vi.fn();
+
+vi.mock("@core/stores", () => ({
+  CurrentDeviceContext: {
+    _currentValue: { deviceId: 1234 },
+  },
+  useDevice: () => ({
+    connection: {
+      factoryResetConfig: mockFactoryReset,
+    },
+  }),
+}));
+
+vi.mock("@core/hooks/useToast.ts", () => ({
+  toast: (...args: unknown[]) => mockToast(...args),
+}));
+
+describe("FactoryResetConfigDialog", () => {
+  const mockOnOpenChange = vi.fn();
+
+  beforeEach(() => {
+    mockOnOpenChange.mockClear();
+    mockFactoryReset.mockReset();
+    mockToast.mockClear();
+    mockFactoryReset.mockResolvedValue(undefined);
+  });
+
+  it("calls factoryResetConfig and then closes the dialog on confirm", async () => {
+    render(<FactoryResetConfigDialog open onOpenChange={mockOnOpenChange} />);
+
+    fireEvent.click(
+      screen.getByRole("button", { name: "Factory Reset Config" }),
+    );
+
+    expect(mockFactoryReset).toHaveBeenCalledTimes(1);
+
+    await waitFor(() => {
+      expect(mockOnOpenChange).toHaveBeenCalledTimes(1);
+      expect(mockOnOpenChange).toHaveBeenCalledWith(false);
+    });
+
+    // success path: no toast
+    expect(mockToast).not.toHaveBeenCalled();
+  });
+
+  it("calls onOpenChange(false) and does not call factoryResetConfig when cancel is clicked", async () => {
+    render(<FactoryResetConfigDialog open onOpenChange={mockOnOpenChange} />);
+
+    fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
+
+    await waitFor(() => {
+      expect(mockOnOpenChange).toHaveBeenCalledTimes(1);
+      expect(mockOnOpenChange).toHaveBeenCalledWith(false);
+    });
+
+    expect(mockFactoryReset).not.toHaveBeenCalled();
+  });
+});
@@ -0,0 +1,39 @@
+import { toast } from "@core/hooks/useToast.ts";
+import { useDevice } from "@core/stores";
+import { useTranslation } from "react-i18next";
+import { DialogWrapper } from "../DialogWrapper.tsx";
+
+export interface FactoryResetConfigDialogProps {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+}
+
+export const FactoryResetConfigDialog = ({
+  open,
+  onOpenChange,
+}: FactoryResetConfigDialogProps) => {
+  const { t } = useTranslation("dialog");
+  const { connection } = useDevice();
+
+  const handleFactoryResetConfig = () => {
+    connection?.factoryResetConfig().catch((error) => {
+      toast({
+        title: t("factoryResetConfig.failedTitle"),
+      });
+      console.error("Failed to factory reset config:", error);
+    });
+  };
+
+  return (
+    <DialogWrapper
+      open={open}
+      onOpenChange={onOpenChange}
+      type="confirm"
+      variant="destructive"
+      title={t("factoryResetConfig.title")}
+      description={t("factoryResetConfig.description")}
+      confirmText={t("factoryResetConfig.confirm")}
+      onConfirm={handleFactoryResetConfig}
+    />
+  );
+};
@@ -0,0 +1,94 @@
+// FactoryResetDeviceDialog.test.tsx
+import { fireEvent, render, screen, waitFor } from "@testing-library/react";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { FactoryResetDeviceDialog } from "./FactoryResetDeviceDialog.tsx";
+
+const mockFactoryResetDevice = vi.fn();
+const mockDeleteAllMessages = vi.fn();
+const mockRemoveAllNodeErrors = vi.fn();
+const mockRemoveAllNodes = vi.fn();
+
+vi.mock("@core/stores", () => ({
+  CurrentDeviceContext: {
+    _currentValue: { deviceId: 1234 },
+  },
+  useDevice: () => ({
+    connection: {
+      factoryResetDevice: mockFactoryResetDevice,
+    },
+  }),
+  useMessages: () => ({
+    deleteAllMessages: mockDeleteAllMessages,
+  }),
+  useNodeDB: () => ({
+    removeAllNodeErrors: mockRemoveAllNodeErrors,
+    removeAllNodes: mockRemoveAllNodes,
+  }),
+}));
+
+describe("FactoryResetDeviceDialog", () => {
+  const mockOnOpenChange = vi.fn();
+
+  beforeEach(() => {
+    mockOnOpenChange.mockClear();
+    mockFactoryResetDevice.mockClear();
+    mockDeleteAllMessages.mockClear();
+    mockRemoveAllNodeErrors.mockClear();
+    mockRemoveAllNodes.mockClear();
+  });
+
+  it("calls factoryResetDevice, closes dialog, and after reset resolves clears messages and node DB", async () => {
+    // Control the promise returned by factoryResetDevice
+    let resolveReset: (() => void) | undefined;
+    mockFactoryResetDevice.mockImplementation(
+      () =>
+        new Promise<void>((resolve) => {
+          resolveReset = resolve;
+        }),
+    );
+
+    render(<FactoryResetDeviceDialog open onOpenChange={mockOnOpenChange} />);
+    fireEvent.click(
+      screen.getByRole("button", { name: "Factory Reset Device" }),
+    );
+
+    // Called immediately
+    expect(mockFactoryResetDevice).toHaveBeenCalledTimes(1);
+
+    // DialogWrapper awaits onConfirm (which returns undefined), so close happens on next microtask
+    await waitFor(() => {
+      expect(mockOnOpenChange).toHaveBeenCalledTimes(1);
+      expect(mockOnOpenChange).toHaveBeenCalledWith(false);
+    });
+
+    // Nothing else should have happened yet (the promise hasn't resolved)
+    expect(mockDeleteAllMessages).not.toHaveBeenCalled();
+    expect(mockRemoveAllNodeErrors).not.toHaveBeenCalled();
+    expect(mockRemoveAllNodes).not.toHaveBeenCalled();
+
+    // Resolve the reset
+    resolveReset?.();
+
+    // Now the .then() chain should fire
+    await waitFor(() => {
+      expect(mockDeleteAllMessages).toHaveBeenCalledTimes(1);
+      expect(mockRemoveAllNodeErrors).toHaveBeenCalledTimes(1);
+      expect(mockRemoveAllNodes).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  it("calls onOpenChange(false) and does not call factoryResetDevice when cancel is clicked", async () => {
+    render(<FactoryResetDeviceDialog open onOpenChange={mockOnOpenChange} />);
+    fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
+
+    await waitFor(() => {
+      expect(mockOnOpenChange).toHaveBeenCalledTimes(1);
+      expect(mockOnOpenChange).toHaveBeenCalledWith(false);
+    });
+
+    expect(mockFactoryResetDevice).not.toHaveBeenCalled();
+    expect(mockDeleteAllMessages).not.toHaveBeenCalled();
+    expect(mockRemoveAllNodeErrors).not.toHaveBeenCalled();
+    expect(mockRemoveAllNodes).not.toHaveBeenCalled();
+  });
+});
@@ -0,0 +1,48 @@
+import { toast } from "@core/hooks/useToast.ts";
+import { useDevice, useMessages, useNodeDB } from "@core/stores";
+import { useTranslation } from "react-i18next";
+import { DialogWrapper } from "../DialogWrapper.tsx";
+
+export interface FactoryResetDeviceDialogProps {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+}
+
+export const FactoryResetDeviceDialog = ({
+  open,
+  onOpenChange,
+}: FactoryResetDeviceDialogProps) => {
+  const { t } = useTranslation("dialog");
+  const { connection } = useDevice();
+  const { removeAllNodeErrors, removeAllNodes } = useNodeDB();
+  const { deleteAllMessages } = useMessages();
+
+  const handleFactoryResetDevice = () => {
+    connection
+      ?.factoryResetDevice()
+      .then(() => {
+        deleteAllMessages();
+        removeAllNodeErrors();
+        removeAllNodes();
+      })
+      .catch((error) => {
+        toast({
+          title: t("factoryResetDevice.failedTitle"),
+        });
+        console.error("Failed to factory reset device:", error);
+      });
+  };
+
+  return (
+    <DialogWrapper
+      open={open}
+      onOpenChange={onOpenChange}
+      type="confirm"
+      variant="destructive"
+      title={t("factoryResetDevice.title")}
+      description={t("factoryResetDevice.description")}
+      confirmText={t("factoryResetDevice.confirm")}
+      onConfirm={handleFactoryResetDevice}
+    />
+  );
+};
@@ -0,0 +1,95 @@
+// ResetNodeDbDialog.test.tsx
+import { fireEvent, render, screen, waitFor } from "@testing-library/react";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { ResetNodeDbDialog } from "./ResetNodeDbDialog.tsx";
+
+const mockResetNodes = vi.fn();
+const mockDeleteAllMessages = vi.fn();
+const mockRemoveAllNodeErrors = vi.fn();
+const mockRemoveAllNodes = vi.fn();
+
+vi.mock("@core/stores", () => ({
+  CurrentDeviceContext: {
+    _currentValue: { deviceId: 1234 },
+  },
+  useDevice: () => ({
+    connection: {
+      resetNodes: mockResetNodes,
+    },
+  }),
+  useMessages: () => ({
+    deleteAllMessages: mockDeleteAllMessages,
+  }),
+  useNodeDB: () => ({
+    removeAllNodeErrors: mockRemoveAllNodeErrors,
+    removeAllNodes: mockRemoveAllNodes,
+  }),
+}));
+
+describe("ResetNodeDbDialog", () => {
+  const mockOnOpenChange = vi.fn();
+
+  beforeEach(() => {
+    mockOnOpenChange.mockClear();
+    mockResetNodes.mockClear();
+    mockDeleteAllMessages.mockClear();
+    mockRemoveAllNodeErrors.mockClear();
+    mockRemoveAllNodes.mockClear();
+  });
+
+  it("calls resetNodes, closes dialog, and after resolve clears messages and node DB (with true flag)", async () => {
+    // Control the promise returned by resetNodes
+    let resolveReset: (() => void) | undefined;
+    mockResetNodes.mockImplementation(
+      () =>
+        new Promise<void>((resolve) => {
+          resolveReset = resolve;
+        }),
+    );
+
+    render(<ResetNodeDbDialog open onOpenChange={mockOnOpenChange} />);
+    fireEvent.click(
+      screen.getByRole("button", { name: "Reset Node Database" }),
+    );
+
+    // Called immediately
+    expect(mockResetNodes).toHaveBeenCalledTimes(1);
+
+    // DialogWrapper awaits onConfirm (which returns undefined), so close happens on next microtask
+    await waitFor(() => {
+      expect(mockOnOpenChange).toHaveBeenCalledTimes(1);
+      expect(mockOnOpenChange).toHaveBeenCalledWith(false);
+    });
+
+    // Nothing else should have happened yet (the promise hasn't resolved)
+    expect(mockDeleteAllMessages).not.toHaveBeenCalled();
+    expect(mockRemoveAllNodeErrors).not.toHaveBeenCalled();
+    expect(mockRemoveAllNodes).not.toHaveBeenCalled();
+
+    // Resolve the reset
+    resolveReset?.();
+
+    // Now the .then() chain should fire
+    await waitFor(() => {
+      expect(mockDeleteAllMessages).toHaveBeenCalledTimes(1);
+      expect(mockRemoveAllNodeErrors).toHaveBeenCalledTimes(1);
+      expect(mockRemoveAllNodes).toHaveBeenCalledTimes(1);
+      expect(mockRemoveAllNodes).toHaveBeenCalledWith(true);
+    });
+  });
+
+  it("calls onOpenChange(false) and does not call resetNodes when cancel is clicked", async () => {
+    render(<ResetNodeDbDialog open onOpenChange={mockOnOpenChange} />);
+    fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
+
+    await waitFor(() => {
+      expect(mockOnOpenChange).toHaveBeenCalledTimes(1);
+      expect(mockOnOpenChange).toHaveBeenCalledWith(false);
+    });
+
+    expect(mockResetNodes).not.toHaveBeenCalled();
+    expect(mockDeleteAllMessages).not.toHaveBeenCalled();
+    expect(mockRemoveAllNodeErrors).not.toHaveBeenCalled();
+    expect(mockRemoveAllNodes).not.toHaveBeenCalled();
+  });
+});
@@ -0,0 +1,48 @@
+import { toast } from "@core/hooks/useToast.ts";
+import { useDevice, useMessages, useNodeDB } from "@core/stores";
+import { useTranslation } from "react-i18next";
+import { DialogWrapper } from "../DialogWrapper.tsx";
+
+export interface ResetNodeDbDialogProps {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+}
+
+export const ResetNodeDbDialog = ({
+  open,
+  onOpenChange,
+}: ResetNodeDbDialogProps) => {
+  const { t } = useTranslation("dialog");
+  const { connection } = useDevice();
+  const { removeAllNodeErrors, removeAllNodes } = useNodeDB();
+  const { deleteAllMessages } = useMessages();
+
+  const handleResetNodeDb = () => {
+    connection
+      ?.resetNodes()
+      .then(() => {
+        deleteAllMessages();
+        removeAllNodeErrors();
+        removeAllNodes(true);
+      })
+      .catch((error) => {
+        toast({
+          title: t("resetNodeDb.failedTitle"),
+        });
+        console.error("Failed to reset Node DB:", error);
+      });
+  };
+
+  return (
+    <DialogWrapper
+      open={open}
+      onOpenChange={onOpenChange}
+      type="confirm"
+      variant="destructive"
+      title={t("resetNodeDb.title")}
+      description={t("resetNodeDb.description")}
+      confirmText={t("resetNodeDb.confirm")}
+      onConfirm={handleResetNodeDb}
+    />
+  );
+};