Просмотр исходного кода

refactor(frontend): load previous session modal (#1284)

* create new modal for loading previous session

* style and replace modal

* retire old components and group modals into folder

* Utilise i18n for text content and add en translations

* prevent modal from being dismissed via the backdrop

* reference issue that its fixing

* fix incorrect role in tests

---------

Co-authored-by: Jim Su <jimsu@protonmail.com>
sp.wack 1 год назад
Родитель
Сommit
0669b27522

+ 22 - 10
frontend/src/App.tsx

@@ -5,7 +5,6 @@ import { useDisclosure } from "@nextui-org/react";
 import CogTooth from "./assets/cog-tooth";
 import ChatInterface from "./components/ChatInterface";
 import Errors from "./components/Errors";
-import LoadMessageModal from "./components/LoadMessageModal";
 import { Container, Orientation } from "./components/Resizable";
 import Terminal from "./components/Terminal";
 import Workspace from "./components/Workspace";
@@ -13,7 +12,8 @@ import { fetchMsgTotal } from "./services/session";
 import { initializeAgent } from "./services/settingsService";
 import Socket from "./services/socket";
 import { ResFetchMsgTotal } from "./types/ResponseType";
-import SettingsModal from "./components/settings/SettingsModal";
+import SettingsModal from "./components/modals/settings/SettingsModal";
+import LoadPreviousSessionModal from "./components/modals/load-previous-session/LoadPreviousSessionModal";
 
 interface Props {
   setSettingOpen: (isOpen: boolean) => void;
@@ -37,16 +37,25 @@ let initOnce = false;
 
 function App(): JSX.Element {
   const [isWarned, setIsWarned] = useState(false);
-  const [loadMsgWarning, setLoadMsgWarning] = useState(false);
 
-  const { isOpen, onOpen, onOpenChange } = useDisclosure();
+  const {
+    isOpen: settingsModalIsOpen,
+    onOpen: onSettingsModalOpen,
+    onOpenChange: onSettingsModalOpenChange,
+  } = useDisclosure();
+
+  const {
+    isOpen: loadPreviousSessionModalIsOpen,
+    onOpen: onLoadPreviousSessionModalOpen,
+    onOpenChange: onLoadPreviousSessionModalOpenChange,
+  } = useDisclosure();
 
   const getMsgTotal = () => {
     if (isWarned) return;
     fetchMsgTotal()
       .then((data: ResFetchMsgTotal) => {
         if (data.msg_total > 0) {
-          setLoadMsgWarning(true);
+          onLoadPreviousSessionModalOpen();
           setIsWarned(true);
         }
       })
@@ -67,7 +76,7 @@ function App(): JSX.Element {
   return (
     <div className="h-screen w-screen flex flex-col">
       <div className="flex grow bg-neutral-900 text-white min-h-0">
-        <LeftNav setSettingOpen={onOpen} />
+        <LeftNav setSettingOpen={onSettingsModalOpen} />
         <Container
           orientation={Orientation.VERTICAL}
           className="grow p-3 py-3 pr-3 min-w-0"
@@ -91,10 +100,13 @@ function App(): JSX.Element {
       {/* This div is for the footer that will be added later
       <div className="h-8 w-full border-t border-border px-2" />
       */}
-      <SettingsModal isOpen={isOpen} onOpenChange={onOpenChange} />
-      <LoadMessageModal
-        isOpen={loadMsgWarning}
-        onClose={() => setLoadMsgWarning(false)}
+      <SettingsModal
+        isOpen={settingsModalIsOpen}
+        onOpenChange={onSettingsModalOpenChange}
+      />
+      <LoadPreviousSessionModal
+        isOpen={loadPreviousSessionModalIsOpen}
+        onOpenChange={onLoadPreviousSessionModalOpenChange}
       />
       <Errors />
       <Toaster />

+ 0 - 83
frontend/src/components/LoadMessageModal.tsx

@@ -1,83 +0,0 @@
-import React from "react";
-import { Button } from "@nextui-org/react";
-import { fetchMsgs, clearMsgs } from "../services/session";
-import { sendChatMessageFromEvent } from "../services/chatService";
-import { handleAssistantMessage } from "../services/actions";
-import { ResFetchMsg } from "../types/ResponseType";
-import ODModal from "./ODModal";
-import toast from "../utils/toast";
-
-interface LoadMessageModalProps {
-  isOpen: boolean;
-  onClose: () => void;
-}
-
-function LoadMessageModal({
-  isOpen,
-  onClose,
-}: LoadMessageModalProps): JSX.Element {
-  const handleStartNewSession = () => {
-    clearMsgs().then().catch();
-    onClose();
-  };
-
-  const handleResumeSession = async () => {
-    try {
-      const data = await fetchMsgs();
-      if (!data || !data.messages || data.messages.length === 0) {
-        return;
-      }
-
-      data.messages.forEach((msg: ResFetchMsg) => {
-        switch (msg.role) {
-          case "user":
-            sendChatMessageFromEvent(msg.payload);
-            break;
-          case "assistant":
-            handleAssistantMessage(msg.payload);
-            break;
-          default:
-            break;
-        }
-      });
-
-      onClose();
-    } catch (error) {
-      toast.stickyError("ws", "Error fetching the session");
-    }
-  };
-
-  return (
-    <ODModal
-      size="md"
-      isOpen={isOpen}
-      onClose={onClose}
-      hideCloseButton
-      backdrop="blur"
-      title="Unfinished Session Detected"
-      primaryAction={
-        <Button
-          className="bg-primary rounded-small"
-          onPress={handleResumeSession}
-        >
-          Resume Session
-        </Button>
-      }
-      secondaryAction={
-        <Button
-          className="bg-neutral-500 rounded-small"
-          onPress={handleStartNewSession}
-        >
-          Start New Session
-        </Button>
-      }
-    >
-      <p>
-        You seem to have an unfinished task. Would you like to pick up where you
-        left off or start fresh?
-      </p>
-    </ODModal>
-  );
-}
-
-export default LoadMessageModal;

+ 0 - 71
frontend/src/components/ODModal.tsx

@@ -1,71 +0,0 @@
-import React from "react";
-import {
-  ModalProps,
-  Modal,
-  ModalBody,
-  ModalContent,
-  ModalFooter,
-  ModalHeader,
-} from "@nextui-org/react";
-
-interface ODModalProps extends Omit<ModalProps, "children"> {
-  title?: string;
-  subtitle?: string;
-  primaryAction?: React.ReactNode;
-  secondaryAction?: React.ReactNode;
-  children: React.ReactNode;
-  isOpen: boolean;
-  onClose: () => void;
-  size: "sm" | "md";
-}
-
-function ODModal(props: ODModalProps): React.ReactElement {
-  const {
-    children,
-    title,
-    subtitle,
-    primaryAction,
-    secondaryAction,
-    size,
-    ...modalProps
-  } = props;
-
-  return (
-    <Modal
-      className="bg-neutral-900 rounded-large"
-      // eslint-disable-next-line react/jsx-props-no-spreading
-      {...modalProps}
-    >
-      <ModalContent
-        className={`${size === "sm" ? "max-w-[24rem]" : "max-w-[52rem]"} p-[40px]`}
-      >
-        <ModalHeader className="flex flex-col p-0">
-          {title && <h3>{title}</h3>}
-          {subtitle && (
-            <span className="text-neutral-400 text-sm font-light">
-              {subtitle}
-            </span>
-          )}
-        </ModalHeader>
-        <ModalBody className="px-0 py-[20px]">{children}</ModalBody>
-        {(primaryAction || secondaryAction) && (
-          <ModalFooter
-            className={`${size === "sm" ? "flex-col" : "flex-row"} flex justify-start p-0`}
-          >
-            {primaryAction}
-            {secondaryAction}
-          </ModalFooter>
-        )}
-      </ModalContent>
-    </Modal>
-  );
-}
-
-ODModal.defaultProps = {
-  title: "",
-  subtitle: "",
-  primaryAction: null,
-  secondaryAction: null,
-};
-
-export default ODModal;

+ 18 - 0
frontend/src/components/base-modal/BaseModal.test.tsx → frontend/src/components/modals/base-modal/BaseModal.test.tsx

@@ -95,4 +95,22 @@ describe("BaseModal", () => {
     );
     expect(screen.getByText("Children")).toBeInTheDocument();
   });
+
+  it.skip("should not close if the backdrop or escape key is pressed", () => {
+    const onOpenChangeMock = vi.fn();
+    render(
+      <BaseModal
+        isOpen
+        onOpenChange={onOpenChangeMock}
+        title="Settings"
+        isDismissable={false}
+      />,
+    );
+
+    act(() => {
+      userEvent.keyboard("{esc}");
+    });
+    // fails because the nextui component wraps the modal content in an aria-hidden div
+    expect(screen.getByRole("dialog")).toBeVisible();
+  });
 });

+ 4 - 0
frontend/src/components/base-modal/BaseModal.tsx → frontend/src/components/modals/base-modal/BaseModal.tsx

@@ -13,6 +13,7 @@ interface BaseModalProps {
   isOpen: boolean;
   onOpenChange: (isOpen: boolean) => void;
   title: string;
+  isDismissable?: boolean;
   subtitle?: string;
   actions?: Action[];
   children?: React.ReactNode;
@@ -22,6 +23,7 @@ function BaseModal({
   isOpen,
   onOpenChange,
   title,
+  isDismissable = true,
   subtitle,
   actions,
   children,
@@ -31,6 +33,7 @@ function BaseModal({
       isOpen={isOpen}
       onOpenChange={onOpenChange}
       title={title}
+      isDismissable={isDismissable}
       backdrop="blur"
       hideCloseButton
       size="sm"
@@ -58,6 +61,7 @@ function BaseModal({
 }
 
 BaseModal.defaultProps = {
+  isDismissable: true,
   subtitle: undefined,
   actions: [],
   children: null,

+ 0 - 0
frontend/src/components/base-modal/FooterContent.tsx → frontend/src/components/modals/base-modal/FooterContent.tsx


+ 0 - 0
frontend/src/components/base-modal/HeaderContent.tsx → frontend/src/components/modals/base-modal/HeaderContent.tsx


+ 128 - 0
frontend/src/components/modals/load-previous-session/LoadPreviousSessionModal.test.tsx

@@ -0,0 +1,128 @@
+import React from "react";
+import { act, render, screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import LoadPreviousSessionModal from "./LoadPreviousSessionModal";
+import { clearMsgs, fetchMsgs } from "../../../services/session";
+import { sendChatMessageFromEvent } from "../../../services/chatService";
+import { handleAssistantMessage } from "../../../services/actions";
+import toast from "../../../utils/toast";
+
+const RESUME_SESSION_BUTTON_LABEL_KEY =
+  "LOAD_SESSION$RESUME_SESSION_MODAL_ACTION_LABEL";
+const START_NEW_SESSION_BUTTON_LABEL_KEY =
+  "LOAD_SESSION$START_NEW_SESSION_MODAL_ACTION_LABEL";
+
+const mocks = vi.hoisted(() => ({
+  fetchMsgsMock: vi.fn(),
+}));
+
+vi.mock("../../../services/session", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../../../services/session")>()),
+  clearMsgs: vi.fn(),
+  fetchMsgs: mocks.fetchMsgsMock.mockResolvedValue({
+    messages: [
+      {
+        id: "1",
+        role: "user",
+        payload: { type: "action" },
+      },
+      {
+        id: "2",
+        role: "assistant",
+        payload: { type: "observation" },
+      },
+    ],
+  }),
+}));
+
+vi.mock("../../../services/chatService", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../../../services/chatService")>()),
+  sendChatMessageFromEvent: vi.fn(),
+}));
+
+vi.mock("../../../services/actions", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("../../../services/actions")>()),
+  handleAssistantMessage: vi.fn(),
+}));
+
+vi.mock("../../../utils/toast", () => ({
+  default: {
+    stickyError: vi.fn(),
+  },
+}));
+
+describe("LoadPreviousSession", () => {
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("should render two buttons", () => {
+    render(<LoadPreviousSessionModal isOpen onOpenChange={vi.fn} />);
+
+    screen.getByRole("button", { name: START_NEW_SESSION_BUTTON_LABEL_KEY });
+    screen.getByRole("button", { name: RESUME_SESSION_BUTTON_LABEL_KEY });
+  });
+
+  it("should clear messages if user chooses to start a new session", () => {
+    const onOpenChangeMock = vi.fn();
+    render(<LoadPreviousSessionModal isOpen onOpenChange={onOpenChangeMock} />);
+
+    const startNewSessionButton = screen.getByRole("button", {
+      name: START_NEW_SESSION_BUTTON_LABEL_KEY,
+    });
+
+    act(() => {
+      userEvent.click(startNewSessionButton);
+    });
+
+    expect(clearMsgs).toHaveBeenCalledTimes(1);
+    // modal should close right after clearing messages
+    expect(onOpenChangeMock).toHaveBeenCalledWith(false);
+  });
+
+  it("should load previous messages if user chooses to resume session", async () => {
+    const onOpenChangeMock = vi.fn();
+    render(<LoadPreviousSessionModal isOpen onOpenChange={onOpenChangeMock} />);
+
+    const resumeSessionButton = screen.getByRole("button", {
+      name: RESUME_SESSION_BUTTON_LABEL_KEY,
+    });
+
+    act(() => {
+      userEvent.click(resumeSessionButton);
+    });
+
+    await waitFor(() => {
+      expect(fetchMsgs).toHaveBeenCalledTimes(1);
+      expect(sendChatMessageFromEvent).toHaveBeenCalledTimes(1);
+      expect(handleAssistantMessage).toHaveBeenCalledTimes(1);
+    });
+    // modal should close right after fetching messages
+    expect(onOpenChangeMock).toHaveBeenCalledWith(false);
+  });
+
+  it("should show an error toast if there is an error fetching the session", async () => {
+    mocks.fetchMsgsMock.mockRejectedValue(new Error("Get messages failed."));
+
+    render(<LoadPreviousSessionModal isOpen onOpenChange={vi.fn} />);
+
+    const resumeSessionButton = screen.getByRole("button", {
+      name: RESUME_SESSION_BUTTON_LABEL_KEY,
+    });
+
+    act(() => {
+      userEvent.click(resumeSessionButton);
+    });
+
+    await waitFor(async () => {
+      await expect(() => fetchMsgs()).rejects.toThrow();
+      expect(handleAssistantMessage).not.toHaveBeenCalled();
+      expect(sendChatMessageFromEvent).not.toHaveBeenCalled();
+      // error toast should be shown
+      expect(toast.stickyError).toHaveBeenCalledWith(
+        "ws",
+        "Error fetching the session",
+      );
+    });
+  });
+});

+ 69 - 0
frontend/src/components/modals/load-previous-session/LoadPreviousSessionModal.tsx

@@ -0,0 +1,69 @@
+import React from "react";
+import { useTranslation } from "react-i18next";
+import BaseModal from "../base-modal/BaseModal";
+import { clearMsgs, fetchMsgs } from "../../../services/session";
+import { sendChatMessageFromEvent } from "../../../services/chatService";
+import { handleAssistantMessage } from "../../../services/actions";
+import toast from "../../../utils/toast";
+import { I18nKey } from "../../../i18n/declaration";
+
+interface LoadPreviousSessionModalProps {
+  isOpen: boolean;
+  onOpenChange: (isOpen: boolean) => void;
+}
+
+function LoadPreviousSessionModal({
+  isOpen,
+  onOpenChange,
+}: LoadPreviousSessionModalProps) {
+  const { t } = useTranslation();
+
+  const onStartNewSession = async () => {
+    await clearMsgs();
+  };
+
+  const onResumeSession = async () => {
+    try {
+      const { messages } = await fetchMsgs();
+
+      messages.forEach((message) => {
+        if (message.role === "user") {
+          sendChatMessageFromEvent(message.payload);
+        }
+
+        if (message.role === "assistant") {
+          handleAssistantMessage(message.payload);
+        }
+      });
+    } catch (error) {
+      toast.stickyError("ws", "Error fetching the session");
+    }
+  };
+
+  return (
+    <BaseModal
+      isOpen={isOpen}
+      title={t(I18nKey.LOAD_SESSION$MODAL_TITLE)}
+      onOpenChange={onOpenChange}
+      isDismissable={false} // prevent unnecessary messages from being stored (issue #1285)
+      actions={[
+        {
+          label: t(I18nKey.LOAD_SESSION$RESUME_SESSION_MODAL_ACTION_LABEL),
+          className: "bg-primary rounded-small",
+          action: onResumeSession,
+          closeAfterAction: true,
+        },
+        {
+          label: t(I18nKey.LOAD_SESSION$START_NEW_SESSION_MODAL_ACTION_LABEL),
+          className: "bg-neutral-500 rounded-small",
+          action: onStartNewSession,
+          closeAfterAction: true,
+        },
+      ]}
+    >
+      <p>{t(I18nKey.LOAD_SESSION$MODAL_CONTENT)}</p>
+    </BaseModal>
+  );
+}
+
+export default LoadPreviousSessionModal;

+ 0 - 0
frontend/src/components/settings/AutocompleteCombobox.test.tsx → frontend/src/components/modals/settings/AutocompleteCombobox.test.tsx


+ 1 - 1
frontend/src/components/settings/AutocompleteCombobox.tsx → frontend/src/components/modals/settings/AutocompleteCombobox.tsx

@@ -1,7 +1,7 @@
 import { Autocomplete, AutocompleteItem } from "@nextui-org/react";
 import React from "react";
 import { useTranslation } from "react-i18next";
-import { I18nKey } from "../../i18n/declaration";
+import { I18nKey } from "../../../i18n/declaration";
 
 type Label = "model" | "agent" | "language";
 

+ 0 - 0
frontend/src/components/settings/SettingsForm.test.tsx → frontend/src/components/modals/settings/SettingsForm.test.tsx


+ 1 - 1
frontend/src/components/settings/SettingsForm.tsx → frontend/src/components/modals/settings/SettingsForm.tsx

@@ -1,5 +1,5 @@
 import React from "react";
-import { AvailableLanguages } from "../../i18n";
+import { AvailableLanguages } from "../../../i18n";
 import { AutocompleteCombobox } from "./AutocompleteCombobox";
 
 interface SettingsFormProps {

+ 5 - 3
frontend/src/components/settings/SettingsModal.test.tsx → frontend/src/components/modals/settings/SettingsModal.test.tsx

@@ -7,11 +7,13 @@ import {
   fetchAgents,
   saveSettings,
   getCurrentSettings,
-} from "../../services/settingsService";
+} from "../../../services/settingsService";
 import SettingsModal from "./SettingsModal";
 
-vi.mock("../../services/settingsService", async (importOriginal) => ({
-  ...(await importOriginal<typeof import("../../services/settingsService")>()),
+vi.mock("../../../services/settingsService", async (importOriginal) => ({
+  ...(await importOriginal<
+    typeof import("../../../services/settingsService")
+  >()),
   getCurrentSettings: vi.fn().mockReturnValue({}),
   saveSettings: vi.fn(),
   fetchModels: vi

+ 3 - 3
frontend/src/components/settings/SettingsModal.tsx → frontend/src/components/modals/settings/SettingsModal.tsx

@@ -8,9 +8,9 @@ import {
   fetchModels,
   getCurrentSettings,
   saveSettings,
-} from "../../services/settingsService";
-import { I18nKey } from "../../i18n/declaration";
-import { AvailableLanguages } from "../../i18n";
+} from "../../../services/settingsService";
+import { I18nKey } from "../../../i18n/declaration";
+import { AvailableLanguages } from "../../../i18n";
 
 interface SettingsProps {
   isOpen: boolean;

+ 12 - 0
frontend/src/i18n/translation.json

@@ -229,6 +229,18 @@
     "pt": "Salvar",
     "es": "Guardar"
   },
+  "LOAD_SESSION$MODAL_TITLE": {
+    "en": "Unfinished Session Detected"
+  },
+  "LOAD_SESSION$MODAL_CONTENT": {
+    "en": "You seem to have an unfinished task. Would you like to pick up where you left off or start fresh?"
+  },
+  "LOAD_SESSION$RESUME_SESSION_MODAL_ACTION_LABEL": {
+    "en": "Resume Session"
+  },
+  "LOAD_SESSION$START_NEW_SESSION_MODAL_ACTION_LABEL": {
+    "en": "Start New Session"
+  },
   "CHAT_INTERFACE$INITIALZING_AGENT_LOADING_MESSAGE": {
     "en": "Initializing agent (may take up to 10 seconds)...",
     "zh-CN": "初始化智能体(可能需要 10 秒以上时间)",

+ 1 - 1
frontend/src/services/session.test.ts

@@ -62,7 +62,7 @@ describe("Session Service", () => {
         messages: [
           {
             id: "1",
-            role: "admin",
+            role: "user",
             payload: {} as ResFetchMsg["payload"],
           },
         ],

+ 3 - 1
frontend/src/types/ResponseType.tsx

@@ -1,5 +1,7 @@
 import { ActionMessage, ObservationMessage } from "./Message";
 
+type Role = "user" | "assistant";
+
 interface ResConfigurations {
   [key: string]: string | boolean | number;
 }
@@ -14,7 +16,7 @@ interface ResFetchMsgTotal {
 
 interface ResFetchMsg {
   id: string;
-  role: string;
+  role: Role;
   payload: SocketMessage;
 }