update tests after setting modals upgrade (#1467)

frontend/src/components/modals/settings/SettingsForm.test.tsx

@@ -1,9 +1,9 @@
-import { Settings } from "#/services/settings";
-import AgentTaskState from "#/types/AgentTaskState";
 import { act, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import React from "react";
 import { renderWithProviders } from "test-utils";
+import AgentTaskState from "#/types/AgentTaskState";
+import { Settings } from "#/services/settings";
 import SettingsForm from "./SettingsForm";
 
 const onModelChangeMock = vi.fn();
@@ -106,7 +106,6 @@ describe("SettingsForm", () => {
       });
 
       expect(onModelChangeMock).toHaveBeenCalledWith("model3");
-      expect(onAPIKeyChangeMock).toHaveBeenCalledWith("");
     });
 
     it("should call the onAgentChange handler when the agent changes", () => {

frontend/src/components/modals/settings/SettingsModal.test.tsx

@@ -1,13 +1,13 @@
-import { fetchAgents, fetchModels } from "#/api";
-import { initializeAgent } from "#/services/agent";
-import { Settings, getSettings, saveSettings } from "#/services/settings";
-import toast from "#/utils/toast";
 import { act, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import i18next from "i18next";
 import React from "react";
 import { renderWithProviders } from "test-utils";
 import { Mock } from "vitest";
+import toast from "#/utils/toast";
+import { Settings, getSettings, saveSettings } from "#/services/settings";
+import { initializeAgent } from "#/services/agent";
+import { fetchAgents, fetchModels } from "#/api";
 import SettingsModal from "./SettingsModal";
 
 const toastSpy = vi.spyOn(toast, "settingsChanged");
@@ -129,6 +129,7 @@ describe("SettingsModal", () => {
       expect(saveSettings).toHaveBeenCalledWith({
         ...initialSettings,
         LLM_MODEL: "model3",
+        LLM_API_KEY: "", // reset after model change
       });
     });
 
@@ -160,6 +161,7 @@ describe("SettingsModal", () => {
       expect(initializeAgent).toHaveBeenCalledWith({
         ...initialSettings,
         LLM_MODEL: "model3",
+        LLM_API_KEY: "", // reset after model change
       });
     });
 

frontend/src/components/modals/settings/SettingsModal.tsx

@@ -1,3 +1,7 @@
+import { Spinner } from "@nextui-org/react";
+import i18next from "i18next";
+import React from "react";
+import { useTranslation } from "react-i18next";
 import { fetchAgents, fetchModels } from "#/api";
 import { AvailableLanguages } from "#/i18n";
 import { I18nKey } from "#/i18n/declaration";
@@ -9,10 +13,6 @@ import {
   saveSettings,
 } from "#/services/settings";
 import toast from "#/utils/toast";
-import { Spinner } from "@nextui-org/react";
-import i18next from "i18next";
-import React from "react";
-import { useTranslation } from "react-i18next";
 import BaseModal from "../base-modal/BaseModal";
 import SettingsForm from "./SettingsForm";
 
@@ -86,6 +86,13 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
     );
   };
 
+  const isDisabled =
+    Object.entries(settings)
+      // filter api key
+      .filter(([key]) => key !== "LLM_API_KEY")
+      .some(([, value]) => !value) ||
+    JSON.stringify(settings) === JSON.stringify(currentSettings);
+
   return (
     <BaseModal
       isOpen={isOpen}
@@ -96,9 +103,7 @@ function SettingsModal({ isOpen, onOpenChange }: SettingsProps) {
         {
           label: t(I18nKey.CONFIGURATION$MODAL_SAVE_BUTTON_LABEL),
           action: handleSaveSettings,
-          isDisabled:
-            Object.values(settings).some((value) => !value) ||
-            JSON.stringify(settings) === JSON.stringify(currentSettings),
+          isDisabled,
           closeAfterAction: true,
           className: "bg-primary rounded-lg",
         },

frontend/src/services/settings.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, it, vi, Mock } from "vitest";
 import {
   DEFAULT_SETTINGS,
+  Settings,
   getSettings,
   getSettingsDifference,
   saveSettings,
@@ -18,7 +19,8 @@ describe("getSettings", () => {
     (localStorage.getItem as Mock)
       .mockReturnValueOnce("llm_value")
       .mockReturnValueOnce("agent_value")
-      .mockReturnValueOnce("language_value");
+      .mockReturnValueOnce("language_value")
+      .mockReturnValueOnce("api_key");
 
     const settings = getSettings();
 
@@ -26,11 +28,13 @@ describe("getSettings", () => {
       LLM_MODEL: "llm_value",
       AGENT: "agent_value",
       LANGUAGE: "language_value",
+      LLM_API_KEY: "api_key",
     });
   });
 
   it("should handle return defaults if localStorage key does not exist", () => {
     (localStorage.getItem as Mock)
+      .mockReturnValueOnce(null)
       .mockReturnValueOnce(null)
       .mockReturnValueOnce(null)
       .mockReturnValueOnce(null);
@@ -41,16 +45,18 @@ describe("getSettings", () => {
       LLM_MODEL: DEFAULT_SETTINGS.LLM_MODEL,
       AGENT: DEFAULT_SETTINGS.AGENT,
       LANGUAGE: DEFAULT_SETTINGS.LANGUAGE,
+      LLM_API_KEY: "",
     });
   });
 });
 
 describe("saveSettings", () => {
   it("should save the settings", () => {
-    const settings = {
+    const settings: Settings = {
       LLM_MODEL: "llm_value",
       AGENT: "agent_value",
       LANGUAGE: "language_value",
+      LLM_API_KEY: "some_key",
     };
 
     saveSettings(settings);
@@ -61,6 +67,10 @@ describe("saveSettings", () => {
       "LANGUAGE",
       "language_value",
     );
+    expect(localStorage.setItem).toHaveBeenCalledWith(
+      "LLM_API_KEY",
+      "some_key",
+    );
   });
 
   it("should save partial settings", () => {

frontend/src/services/settings.ts

@@ -17,15 +17,19 @@ const validKeys = Object.keys(DEFAULT_SETTINGS) as (keyof Settings)[];
 /**
  * Get the settings from local storage or use the default settings if not found
  */
-export const getSettings = (): Settings => ({
-  LLM_MODEL: localStorage.getItem("LLM_MODEL") || DEFAULT_SETTINGS.LLM_MODEL,
-  AGENT: localStorage.getItem("AGENT") || DEFAULT_SETTINGS.AGENT,
-  LANGUAGE: localStorage.getItem("LANGUAGE") || DEFAULT_SETTINGS.LANGUAGE,
-  LLM_API_KEY:
-    localStorage.getItem(
-      `API_KEY_${localStorage.getItem("LLM_MODEL") || DEFAULT_SETTINGS.LLM_MODEL}`,
-    ) || DEFAULT_SETTINGS.LLM_API_KEY,
-});
+export const getSettings = (): Settings => {
+  const model = localStorage.getItem("LLM_MODEL");
+  const agent = localStorage.getItem("AGENT");
+  const language = localStorage.getItem("LANGUAGE");
+  const apiKey = localStorage.getItem(`API_KEY_${model}`);
+
+  return {
+    LLM_MODEL: model || DEFAULT_SETTINGS.LLM_MODEL,
+    AGENT: agent || DEFAULT_SETTINGS.AGENT,
+    LANGUAGE: language || DEFAULT_SETTINGS.LANGUAGE,
+    LLM_API_KEY: apiKey || DEFAULT_SETTINGS.LLM_API_KEY,
+  };
+};
 
 /**
  * Save the settings to local storage. Only valid settings are saved.

package-lock.json

@@ -1,6 +0,0 @@
-{
-  "name": "OpenDevin",
-  "lockfileVersion": 3,
-  "requires": true,
-  "packages": {}
-}