diff --git a/frontend/__tests__/components/features/sidebar/sidebar.test.tsx b/frontend/__tests__/components/features/sidebar/sidebar.test.tsx index 1e80607301b0..9d96e7a49bdc 100644 --- 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 index bf695cbc6ba7..92a3359e83c9 100644 --- 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" > - + {t(I18nKey.ACCOUNT_SETTINGS$SETTINGS)} diff --git a/frontend/src/components/features/sidebar/sidebar.tsx b/frontend/src/components/features/sidebar/sidebar.tsx index f6a1728ce5ed..cee990f9e963 100644 --- 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() { )} {settingsIsError || - (showSettingsModal && ( + (showSettingsModal && settingsSuccessfulyFetched && ( 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 index fb162103eb5f..6e1363274944 100644 --- 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) => { + const handleSubmit = async (event: React.FormEvent) => { 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 ( - +
@@ -137,6 +137,7 @@ export function AccountSettingsForm({
diff --git a/frontend/src/components/shared/modals/settings/settings-form.tsx b/frontend/src/components/shared/modals/settings/settings-form.tsx index 1265d6035514..b60bba79d7f3 100644 --- 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 new file mode 100644 index 000000000000..4b85d2e27784 --- /dev/null +++ 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) => Promise; + settings: Settings | undefined; +} + +const SettingsContext = React.createContext( + 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) => { + const updatedSettings: Partial = { + ...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 {children}; +} + +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 deleted file mode 100644 index e4a6341e262e..000000000000 --- a/frontend/src/context/settings-up-to-date-context.tsx +++ /dev/null @@ -1,40 +0,0 @@ -import React from "react"; -import { settingsAreUpToDate } from "#/services/settings"; - -interface SettingsUpToDateContextType { - isUpToDate: boolean; - setIsUpToDate: (value: boolean) => void; -} - -const SettingsUpToDateContext = React.createContext< - SettingsUpToDateContextType | undefined ->(undefined); - -interface SettingsUpToDateProviderProps { - children: React.ReactNode; -} - -export function SettingsUpToDateProvider({ - children, -}: SettingsUpToDateProviderProps) { - const [isUpToDate, setIsUpToDate] = React.useState(settingsAreUpToDate()); - - const value = React.useMemo( - () => ({ isUpToDate, setIsUpToDate }), - [isUpToDate, setIsUpToDate], - ); - - return ( - {children} - ); -} - -export function useSettingsUpToDate() { - const context = React.useContext(SettingsUpToDateContext); - if (context === undefined) { - throw new Error( - "useSettingsUpToDate must be used within a SettingsUpToDateProvider", - ); - } - return context; -} diff --git a/frontend/src/entry.client.tsx b/frontend/src/entry.client.tsx index ba17326b4699..47f0090489cf 100644 --- 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(() => - - + + - - + + , diff --git a/frontend/src/hooks/mutation/use-save-settings.ts b/frontend/src/hooks/mutation/use-save-settings.ts index f9731e981d5b..ef334b0f1db8 100644 --- a/frontend/src/hooks/mutation/use-save-settings.ts +++ b/frontend/src/hooks/mutation/use-save-settings.ts @@ -1,12 +1,6 @@ import { useMutation, useQueryClient } from "@tanstack/react-query"; -import { - ApiSettings, - DEFAULT_SETTINGS, - LATEST_SETTINGS_VERSION, - Settings, -} from "#/services/settings"; +import { ApiSettings, DEFAULT_SETTINGS, Settings } from "#/services/settings"; import OpenHands from "#/api/open-hands"; -import { useSettingsUpToDate } from "#/context/settings-up-to-date-context"; const saveSettingsMutationFn = async (settings: Partial) => { const apiSettings: Partial = { @@ -24,19 +18,11 @@ const saveSettingsMutationFn = async (settings: Partial) => { export const useSaveSettings = () => { const queryClient = useQueryClient(); - const { isUpToDate, setIsUpToDate } = useSettingsUpToDate(); return useMutation({ mutationFn: saveSettingsMutationFn, onSuccess: async () => { await queryClient.invalidateQueries({ queryKey: ["settings"] }); - if (!isUpToDate) { - localStorage.setItem( - "SETTINGS_VERSION", - LATEST_SETTINGS_VERSION.toString(), - ); - setIsUpToDate(true); - } }, }); }; diff --git a/frontend/src/hooks/query/use-settings.ts b/frontend/src/hooks/query/use-settings.ts index a4d430ccf88d..3daa8c8780b7 100644 --- a/frontend/src/hooks/query/use-settings.ts +++ b/frontend/src/hooks/query/use-settings.ts @@ -39,7 +39,6 @@ export const useSettings = () => { const query = useQuery({ queryKey: ["settings"], queryFn: getSettingsQueryFn, - initialData: DEFAULT_SETTINGS, }); React.useEffect(() => { diff --git a/frontend/src/hooks/use-maybe-migrate-settings.ts b/frontend/src/hooks/use-maybe-migrate-settings.ts index 4f5bbd4712a0..a2da7ef47be5 100644 --- a/frontend/src/hooks/use-maybe-migrate-settings.ts +++ b/frontend/src/hooks/use-maybe-migrate-settings.ts @@ -1,7 +1,7 @@ // Sometimes we ship major changes, like a new default agent. import React from "react"; -import { useSettingsUpToDate } from "#/context/settings-up-to-date-context"; +import { useCurrentSettings } from "#/context/settings-context"; import { getCurrentSettingsVersion, DEFAULT_SETTINGS, @@ -12,7 +12,7 @@ import { useSaveSettings } from "./mutation/use-save-settings"; // In this case, we may want to override a previous choice made by the user. export const useMaybeMigrateSettings = () => { const { mutateAsync: saveSettings } = useSaveSettings(); - const { isUpToDate } = useSettingsUpToDate(); + const { isUpToDate } = useCurrentSettings(); const maybeMigrateSettings = async () => { const currentVersion = getCurrentSettingsVersion(); diff --git a/frontend/src/mocks/handlers.ts b/frontend/src/mocks/handlers.ts index f24c4a55ddae..9d1a42d4305d 100644 --- a/frontend/src/mocks/handlers.ts +++ b/frontend/src/mocks/handlers.ts @@ -6,7 +6,7 @@ import { } from "#/api/open-hands.types"; import { DEFAULT_SETTINGS } from "#/services/settings"; -const userPreferences = { +export const MOCK_USER_PREFERENCES = { settings: { llm_model: DEFAULT_SETTINGS.LLM_MODEL, llm_base_url: DEFAULT_SETTINGS.LLM_BASE_URL, @@ -169,14 +169,14 @@ export const handlers = [ return HttpResponse.json(config); }), http.get("/api/settings", async () => - HttpResponse.json(userPreferences.settings), + HttpResponse.json(MOCK_USER_PREFERENCES.settings), ), http.post("/api/settings", async ({ request }) => { const body = await request.json(); if (body) { - userPreferences.settings = { - ...userPreferences.settings, + MOCK_USER_PREFERENCES.settings = { + ...MOCK_USER_PREFERENCES.settings, // @ts-expect-error - We know this is a settings object ...body, }; diff --git a/frontend/src/routes/_oh.app/route.tsx b/frontend/src/routes/_oh.app/route.tsx index 182a7a98e57c..701d6b3edb1e 100644 --- a/frontend/src/routes/_oh.app/route.tsx +++ b/frontend/src/routes/_oh.app/route.tsx @@ -176,13 +176,15 @@ function AppContent() { - + {settings && ( + + )}
diff --git a/frontend/src/routes/_oh/route.tsx b/frontend/src/routes/_oh/route.tsx index 16492b533343..ca5868d15572 100644 --- a/frontend/src/routes/_oh/route.tsx +++ b/frontend/src/routes/_oh/route.tsx @@ -63,10 +63,10 @@ export default function MainApp() { }); React.useEffect(() => { - if (settings.LANGUAGE) { + if (settings?.LANGUAGE) { i18n.changeLanguage(settings.LANGUAGE); } - }, [settings.LANGUAGE]); + }, [settings?.LANGUAGE]); const isInWaitlist = !isFetchingAuth && !isAuthed && config.data?.APP_MODE === "saas"; diff --git a/frontend/test-utils.tsx b/frontend/test-utils.tsx index 8eeb5e453fb6..42bd9ec6e0d9 100644 --- a/frontend/test-utils.tsx +++ b/frontend/test-utils.tsx @@ -11,7 +11,7 @@ import { vi } from "vitest"; import { AppStore, RootState, rootReducer } from "./src/store"; import { AuthProvider } from "#/context/auth-context"; import { ConversationProvider } from "#/context/conversation-context"; -import { SettingsUpToDateProvider } from "#/context/settings-up-to-date-context"; +import { SettingsProvider } from "#/context/settings-context"; // Mock useParams before importing components vi.mock("react-router", async () => { @@ -67,19 +67,19 @@ export function renderWithProviders( return ( - - + + {children} - - + + );