Merge commit '3876a00aa574d12075807268c0fd1427831b475a' into add-loca…

…l-runtime
All-Hands-AI · Jan 10, 2025 · bef37fd · bef37fd
2 parents d3370e6 + 3876a00
commit bef37fd
Show file tree

Hide file tree

Showing 16 changed files with 230 additions and 103 deletions.
diff --git a/frontend/__tests__/components/features/sidebar/sidebar.test.tsx b/frontend/__tests__/components/features/sidebar/sidebar.test.tsx
@@ -1,10 +1,12 @@
-import { screen } from "@testing-library/react";
+import { screen, within } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
-import { describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 import { renderWithProviders } from "test-utils";
 import { createRoutesStub } from "react-router";
 import { Sidebar } from "#/components/features/sidebar/sidebar";
 import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags";
+import OpenHands from "#/api/open-hands";
+import { MOCK_USER_PREFERENCES } from "#/mocks/handlers";
 
 const renderSidebar = () => {
   const RouterStub = createRoutesStub([
@@ -43,4 +45,101 @@ describe("Sidebar", () => {
       ).not.toBeInTheDocument();
     },
   );
+
+  describe("Settings", () => {
+    const getSettingsSpy = vi.spyOn(OpenHands, "getSettings");
+    const saveSettingsSpy = vi.spyOn(OpenHands, "saveSettings");
+
+    afterEach(() => {
+      vi.clearAllMocks();
+    });
+
+    it("should fetch settings data on mount", () => {
+      renderSidebar();
+      expect(getSettingsSpy).toHaveBeenCalledOnce();
+    });
+
+    it("should send all settings data when saving AI configuration", async () => {
+      const user = userEvent.setup();
+      renderSidebar();
+
+      const settingsButton = screen.getByTestId("settings-button");
+      await user.click(settingsButton);
+
+      const settingsModal = screen.getByTestId("ai-config-modal");
+      const saveButton = within(settingsModal).getByTestId(
+        "save-settings-button",
+      );
+      await user.click(saveButton);
+
+      expect(saveSettingsSpy).toHaveBeenCalledWith({
+        ...MOCK_USER_PREFERENCES.settings,
+        // the actual values are falsey (null or "") but we're checking for undefined
+        llm_api_key: undefined,
+        llm_base_url: undefined,
+        security_analyzer: undefined,
+      });
+    });
+
+    it("should send all settings data when saving account settings", async () => {
+      const user = userEvent.setup();
+      renderSidebar();
+
+      const userAvatar = screen.getByTestId("user-avatar");
+      await user.click(userAvatar);
+
+      const menu = screen.getByTestId("account-settings-context-menu");
+      const accountSettingsButton = within(menu).getByTestId(
+        "account-settings-button",
+      );
+      await user.click(accountSettingsButton);
+
+      const accountSettingsModal = screen.getByTestId("account-settings-form");
+      const saveButton =
+        within(accountSettingsModal).getByTestId("save-settings");
+      await user.click(saveButton);
+
+      expect(saveSettingsSpy).toHaveBeenCalledWith({
+        ...MOCK_USER_PREFERENCES.settings,
+        llm_api_key: undefined, // null or undefined
+      });
+    });
+
+    it("should not reset AI configuration when saving account settings", async () => {
+      const user = userEvent.setup();
+      renderSidebar();
+
+      const userAvatar = screen.getByTestId("user-avatar");
+      await user.click(userAvatar);
+
+      const menu = screen.getByTestId("account-settings-context-menu");
+      const accountSettingsButton = within(menu).getByTestId(
+        "account-settings-button",
+      );
+      await user.click(accountSettingsButton);
+
+      const accountSettingsModal = screen.getByTestId("account-settings-form");
+
+      const languageInput =
+        within(accountSettingsModal).getByLabelText(/language/i);
+      await user.click(languageInput);
+
+      const norskOption = screen.getByText(/norsk/i);
+      await user.click(norskOption);
+
+      const tokenInput =
+        within(accountSettingsModal).getByLabelText(/github token/i);
+      await user.type(tokenInput, "new-token");
+
+      const saveButton =
+        within(accountSettingsModal).getByTestId("save-settings");
+      await user.click(saveButton);
+
+      expect(saveSettingsSpy).toHaveBeenCalledWith({
+        ...MOCK_USER_PREFERENCES.settings,
+        language: "no",
+        llm_api_key: undefined, // null or undefined
+      });
+    });
+  });
 });
diff --git a/frontend/src/components/features/context-menu/account-settings-context-menu.tsx b/frontend/src/components/features/context-menu/account-settings-context-menu.tsx
@@ -27,7 +27,10 @@ export function AccountSettingsContextMenu({
       ref={ref}
       className="absolute left-full -top-1 z-10"
     >
-      <ContextMenuListItem onClick={onClickAccountSettings}>
+      <ContextMenuListItem
+        testId="account-settings-button"
+        onClick={onClickAccountSettings}
+      >
         {t(I18nKey.ACCOUNT_SETTINGS$SETTINGS)}
       </ContextMenuListItem>
       <ContextMenuSeparator />

diff --git a/frontend/src/components/features/sidebar/sidebar.tsx b/frontend/src/components/features/sidebar/sidebar.tsx
@@ -12,7 +12,7 @@ import { SettingsButton } from "#/components/shared/buttons/settings-button";
 import { LoadingSpinner } from "#/components/shared/loading-spinner";
 import { AccountSettingsModal } from "#/components/shared/modals/account-settings/account-settings-modal";
 import { SettingsModal } from "#/components/shared/modals/settings/settings-modal";
-import { useSettingsUpToDate } from "#/context/settings-up-to-date-context";
+import { useCurrentSettings } from "#/context/settings-context";
 import { useSettings } from "#/hooks/query/use-settings";
 import { ConversationPanel } from "../conversation-panel/conversation-panel";
 import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags";
@@ -28,8 +28,13 @@ export function Sidebar() {
   const user = useGitHubUser();
   const { data: isAuthed } = useIsAuthed();
   const { logout } = useAuth();
-  const { data: settings, isError: settingsIsError } = useSettings();
-  const { isUpToDate: settingsAreUpToDate } = useSettingsUpToDate();
+  const {
+    data: settings,
+    isError: settingsIsError,
+    isSuccess: settingsSuccessfulyFetched,
+  } = useSettings();
+
+  const { isUpToDate: settingsAreUpToDate } = useCurrentSettings();
 
   const [accountSettingsModalOpen, setAccountSettingsModalOpen] =
     React.useState(false);
@@ -106,7 +111,7 @@ export function Sidebar() {
         <AccountSettingsModal onClose={handleAccountSettingsModalClose} />
       )}
       {settingsIsError ||
-        (showSettingsModal && (
+        (showSettingsModal && settingsSuccessfulyFetched && (
           <SettingsModal
             settings={settings}
             onClose={() => setSettingsModalIsOpen(false)}

diff --git a/frontend/src/components/shared/modals/account-settings/account-settings-form.tsx b/frontend/src/components/shared/modals/account-settings/account-settings-form.tsx
@@ -13,7 +13,7 @@ import { ModalButton } from "../../buttons/modal-button";
 import { CustomInput } from "../../custom-input";
 import { FormFieldset } from "../../form-fieldset";
 import { useConfig } from "#/hooks/query/use-config";
-import { useSaveSettings } from "#/hooks/mutation/use-save-settings";
+import { useCurrentSettings } from "#/context/settings-context";
 
 interface AccountSettingsFormProps {
   onClose: () => void;
@@ -30,10 +30,10 @@ export function AccountSettingsForm({
 }: AccountSettingsFormProps) {
   const { gitHubToken, setGitHubToken, logout } = useAuth();
   const { data: config } = useConfig();
-  const { mutate: saveSettings } = useSaveSettings();
+  const { saveUserSettings } = useCurrentSettings();
   const { t } = useTranslation();
 
-  const handleSubmit = (event: React.FormEvent<HTMLFormElement>) => {
+  const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
     event.preventDefault();
     const formData = new FormData(event.currentTarget);
 
@@ -50,7 +50,7 @@ export function AccountSettingsForm({
         ({ label }) => label === language,
       )?.value;
 
-      if (languageKey) saveSettings({ LANGUAGE: languageKey });
+      if (languageKey) await saveUserSettings({ LANGUAGE: languageKey });
     }
 
     handleCaptureConsent(analytics);
@@ -61,7 +61,7 @@ export function AccountSettingsForm({
   };
 
   return (
-    <ModalBody>
+    <ModalBody testID="account-settings-form">
       <form className="flex flex-col w-full gap-6" onSubmit={handleSubmit}>
         <div className="w-full flex flex-col gap-2">
           <BaseModalTitle title="Account Settings" />
@@ -137,6 +137,7 @@ export function AccountSettingsForm({
 
         <div className="flex flex-col gap-2 w-full">
           <ModalButton
+            testId="save-settings"
             type="submit"
             intent="account"
             text={t(I18nKey.ACCOUNT_SETTINGS_MODAL$SAVE)}

diff --git a/frontend/src/components/shared/modals/account-settings/account-settings-modal.tsx b/frontend/src/components/shared/modals/account-settings/account-settings-modal.tsx
@@ -18,7 +18,7 @@ export function AccountSettingsModal({ onClose }: AccountSettingsModalProps) {
     <ModalBackdrop onClose={onClose}>
       <AccountSettingsForm
         onClose={onClose}
-        selectedLanguage={settings.LANGUAGE}
+        selectedLanguage={settings?.LANGUAGE || "en"}
         gitHubError={user.isError}
         analyticsConsent={analyticsConsent}
       />

diff --git a/frontend/src/components/shared/modals/settings/settings-form.tsx b/frontend/src/components/shared/modals/settings/settings-form.tsx
@@ -19,10 +19,10 @@ import { CustomModelInput } from "../../inputs/custom-model-input";
 import { SecurityAnalyzerInput } from "../../inputs/security-analyzers-input";
 import { ModalBackdrop } from "../modal-backdrop";
 import { ModelSelector } from "./model-selector";
-import { useSaveSettings } from "#/hooks/mutation/use-save-settings";
 
 import { RuntimeSizeSelector } from "./runtime-size-selector";
 import { useConfig } from "#/hooks/query/use-config";
+import { useCurrentSettings } from "#/context/settings-context";
 
 interface SettingsFormProps {
   disabled?: boolean;
@@ -41,7 +41,7 @@ export function SettingsForm({
   securityAnalyzers,
   onClose,
 }: SettingsFormProps) {
-  const { mutateAsync: saveSettings } = useSaveSettings();
+  const { saveUserSettings } = useCurrentSettings();
   const endSession = useEndSession();
   const { data: config } = useConfig();
 
@@ -95,7 +95,8 @@ export function SettingsForm({
     const newSettings = extractSettings(formData);
 
     saveSettingsView(isUsingAdvancedOptions ? "advanced" : "basic");
-    await saveSettings(newSettings, { onSuccess: onClose });
+    await saveUserSettings(newSettings);
+    onClose();
     resetOngoingSession();
 
     posthog.capture("settings_saved", {
@@ -107,7 +108,8 @@ export function SettingsForm({
   };
 
   const handleConfirmResetSettings = async () => {
-    await saveSettings(getDefaultSettings(), { onSuccess: onClose });
+    await saveUserSettings(getDefaultSettings());
+    onClose();
     resetOngoingSession();
     posthog.capture("settings_reset");
   };

diff --git a/frontend/src/context/settings-context.tsx b/frontend/src/context/settings-context.tsx
@@ -0,0 +1,70 @@
+import React from "react";
+import {
+  LATEST_SETTINGS_VERSION,
+  Settings,
+  settingsAreUpToDate,
+} from "#/services/settings";
+import { useSettings } from "#/hooks/query/use-settings";
+import { useSaveSettings } from "#/hooks/mutation/use-save-settings";
+
+interface SettingsContextType {
+  isUpToDate: boolean;
+  setIsUpToDate: (value: boolean) => void;
+  saveUserSettings: (newSettings: Partial<Settings>) => Promise<void>;
+  settings: Settings | undefined;
+}
+
+const SettingsContext = React.createContext<SettingsContextType | undefined>(
+  undefined,
+);
+
+interface SettingsProviderProps {
+  children: React.ReactNode;
+}
+
+export function SettingsProvider({ children }: SettingsProviderProps) {
+  const { data: userSettings } = useSettings();
+  const { mutateAsync: saveSettings } = useSaveSettings();
+
+  const [isUpToDate, setIsUpToDate] = React.useState(settingsAreUpToDate());
+
+  const saveUserSettings = async (newSettings: Partial<Settings>) => {
+    const updatedSettings: Partial<Settings> = {
+      ...userSettings,
+      ...newSettings,
+    };
+    await saveSettings(updatedSettings, {
+      onSuccess: () => {
+        if (!isUpToDate) {
+          localStorage.setItem(
+            "SETTINGS_VERSION",
+            LATEST_SETTINGS_VERSION.toString(),
+          );
+          setIsUpToDate(true);
+        }
+      },
+    });
+  };
+
+  const value = React.useMemo(
+    () => ({
+      isUpToDate,
+      setIsUpToDate,
+      saveUserSettings,
+      settings: userSettings,
+    }),
+    [isUpToDate, setIsUpToDate, saveUserSettings, userSettings],
+  );
+
+  return <SettingsContext value={value}>{children}</SettingsContext>;
+}
+
+export function useCurrentSettings() {
+  const context = React.useContext(SettingsContext);
+  if (context === undefined) {
+    throw new Error(
+      "useCurrentSettings must be used within a SettingsProvider",
+    );
+  }
+  return context;
+}
diff --git a/frontend/src/context/settings-up-to-date-context.tsx b/frontend/src/context/settings-up-to-date-context.tsx
diff --git a/frontend/src/entry.client.tsx b/frontend/src/entry.client.tsx
@@ -20,7 +20,7 @@ import toast from "react-hot-toast";
 import store from "./store";
 import { useConfig } from "./hooks/query/use-config";
 import { AuthProvider } from "./context/auth-context";
-import { SettingsUpToDateProvider } from "./context/settings-up-to-date-context";
+import { SettingsProvider } from "./context/settings-context";
 
 function PosthogInit() {
   const { data: config } = useConfig();
@@ -79,12 +79,12 @@ prepareApp().then(() =>
       <StrictMode>
         <Provider store={store}>
           <AuthProvider>
-            <SettingsUpToDateProvider>
-              <QueryClientProvider client={queryClient}>
+            <QueryClientProvider client={queryClient}>
+              <SettingsProvider>
                 <HydratedRouter />
                 <PosthogInit />
-              </QueryClientProvider>
-            </SettingsUpToDateProvider>
+              </SettingsProvider>
+            </QueryClientProvider>
           </AuthProvider>
         </Provider>
       </StrictMode>,