diff --git a/frontend/src/components/modals/settings/SettingsForm.test.tsx b/frontend/src/components/modals/settings/SettingsForm.test.tsx index 35c0a4c011..c9b8414e59 100644 --- a/frontend/src/components/modals/settings/SettingsForm.test.tsx +++ b/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", () => { diff --git a/frontend/src/components/modals/settings/SettingsModal.test.tsx b/frontend/src/components/modals/settings/SettingsModal.test.tsx index 8530f185f0..ba540b2a20 100644 --- a/frontend/src/components/modals/settings/SettingsModal.test.tsx +++ b/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 }); }); diff --git a/frontend/src/components/modals/settings/SettingsModal.tsx b/frontend/src/components/modals/settings/SettingsModal.tsx index 58e1660a28..d3e636c142 100644 --- a/frontend/src/components/modals/settings/SettingsModal.tsx +++ b/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 ( !value) || - JSON.stringify(settings) === JSON.stringify(currentSettings), + isDisabled, closeAfterAction: true, className: "bg-primary rounded-lg", }, diff --git a/frontend/src/services/settings.test.ts b/frontend/src/services/settings.test.ts index caef2486c2..6dc2d0597b 100644 --- a/frontend/src/services/settings.test.ts +++ b/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", () => { diff --git a/frontend/src/services/settings.ts b/frontend/src/services/settings.ts index 29129b13f3..e5997c6f4d 100644 --- a/frontend/src/services/settings.ts +++ b/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. diff --git a/package-lock.json b/package-lock.json deleted file mode 100644 index d57e233291..0000000000 --- a/package-lock.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "name": "OpenDevin", - "lockfileVersion": 3, - "requires": true, - "packages": {} -}