Skip to content

Commit

Permalink
chore: Remove settings local storage logic (#6504)
Browse files Browse the repository at this point in the history
  • Loading branch information
amanape authored Jan 29, 2025
1 parent eb760f3 commit b987f33
Show file tree
Hide file tree
Showing 20 changed files with 56 additions and 266 deletions.
21 changes: 0 additions & 21 deletions frontend/__tests__/components/features/sidebar/sidebar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,27 +135,6 @@ describe("Sidebar", () => {
});

describe("Settings Modal", () => {
it("should open the settings modal if the settings version is out of date", async () => {
const user = userEvent.setup();
localStorage.clear();

const { rerender } = renderSidebar();

const settingsModal = await screen.findByTestId("ai-config-modal");
expect(settingsModal).toBeInTheDocument();

const saveSettingsButton = await within(settingsModal).findByTestId(
"save-settings-button",
);
await user.click(saveSettingsButton);

expect(screen.queryByTestId("ai-config-modal")).not.toBeInTheDocument();

rerender(<RouterStub />);

expect(screen.queryByTestId("ai-config-modal")).not.toBeInTheDocument();
});

it("should open the settings modal if the user clicks the settings button", async () => {
const user = userEvent.setup();
renderSidebar();
Expand Down
28 changes: 0 additions & 28 deletions frontend/__tests__/utils/storage.test.ts

This file was deleted.

2 changes: 1 addition & 1 deletion frontend/src/api/open-hands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
GetTrajectoryResponse,
} from "./open-hands.types";
import { openHands } from "./open-hands-axios";
import { ApiSettings } from "#/services/settings";
import { ApiSettings } from "#/types/settings";

class OpenHands {
/**
Expand Down
8 changes: 2 additions & 6 deletions frontend/src/components/features/sidebar/sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ export function Sidebar() {
const { data: config } = useConfig();
const { data: settings, isError: settingsError } = useSettings();
const { mutateAsync: logout } = useLogout();

const { saveUserSettings, isUpToDate: settingsAreUpToDate } =
useCurrentSettings();
const { saveUserSettings } = useCurrentSettings();

const [accountSettingsModalOpen, setAccountSettingsModalOpen] =
React.useState(false);
Expand Down Expand Up @@ -63,8 +61,6 @@ export function Sidebar() {
posthog.reset();
};

const showSettingsModal = !settingsAreUpToDate || settingsModalIsOpen;

return (
<>
<aside className="h-[40px] md:h-auto px-1 flex flex-row md:flex-col gap-1">
Expand Down Expand Up @@ -109,7 +105,7 @@ export function Sidebar() {
{accountSettingsModalOpen && (
<AccountSettingsModal onClose={handleAccountSettingsModalClose} />
)}
{(settingsError || showSettingsModal) && (
{(settingsError || settingsModalIsOpen) && (
<SettingsModal
settings={settings}
onClose={() => setSettingsModalIsOpen(false)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import { ModalButton } from "../../buttons/modal-button";
import { FormFieldset } from "../../form-fieldset";
import { useConfig } from "#/hooks/query/use-config";
import { useCurrentSettings } from "#/context/settings-context";
import { PostSettings } from "#/services/settings";
import { GitHubTokenInput } from "./github-token-input";
import { PostSettings } from "#/types/settings";

interface AccountSettingsFormProps {
onClose: () => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import React from "react";
import posthog from "posthog-js";
import { I18nKey } from "#/i18n/declaration";
import { organizeModelsAndProviders } from "#/utils/organize-models-and-providers";
import { getDefaultSettings, Settings } from "#/services/settings";
import { getDefaultSettings } from "#/services/settings";
import { extractModelAndProvider } from "#/utils/extract-model-and-provider";
import { DangerModal } from "../confirmation-modals/danger-modal";
import { extractSettings, saveSettingsView } from "#/utils/settings-utils";
import { extractSettings } from "#/utils/settings-utils";
import { useEndSession } from "#/hooks/use-end-session";
import { ModalButton } from "../../buttons/modal-button";
import { AdvancedOptionSwitch } from "../../inputs/advanced-option-switch";
Expand All @@ -24,6 +24,7 @@ import { RuntimeSizeSelector } from "./runtime-size-selector";
import { useConfig } from "#/hooks/query/use-config";
import { useCurrentSettings } from "#/context/settings-context";
import { MEMORY_CONDENSER } from "#/utils/feature-flags";
import { Settings } from "#/types/settings";

interface SettingsFormProps {
disabled?: boolean;
Expand Down Expand Up @@ -93,14 +94,11 @@ export function SettingsForm({
};

const handleFormSubmission = async (formData: FormData) => {
const keys = Array.from(formData.keys());
const isUsingAdvancedOptions = keys.includes("use-advanced-options");
const newSettings = extractSettings(formData);

// Inject the condenser config from the current feature flag value
newSettings.ENABLE_DEFAULT_CONDENSER = MEMORY_CONDENSER;

saveSettingsView(isUsingAdvancedOptions ? "advanced" : "basic");
await saveUserSettings(newSettings);
onClose();
resetOngoingSession();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { useTranslation } from "react-i18next";
import { useAIConfigOptions } from "#/hooks/query/use-ai-config-options";
import { Settings } from "#/services/settings";
import { I18nKey } from "#/i18n/declaration";
import { LoadingSpinner } from "../../loading-spinner";
import { ModalBackdrop } from "../modal-backdrop";
import { SettingsForm } from "./settings-form";
import { Settings } from "#/types/settings";

interface SettingsModalProps {
settings: Settings;
Expand Down
27 changes: 3 additions & 24 deletions frontend/src/context/settings-context.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
import React from "react";
import {
LATEST_SETTINGS_VERSION,
PostSettings,
Settings,
settingsAreUpToDate,
} from "#/services/settings";
import { useSettings } from "#/hooks/query/use-settings";
import { useSaveSettings } from "#/hooks/mutation/use-save-settings";
import { PostSettings, Settings } from "#/types/settings";

interface SettingsContextType {
isUpToDate: boolean;
setIsUpToDate: (value: boolean) => void;
saveUserSettings: (newSettings: Partial<PostSettings>) => Promise<void>;
settings: Settings | undefined;
}
Expand All @@ -27,8 +20,6 @@ export function SettingsProvider({ children }: SettingsProviderProps) {
const { data: userSettings } = useSettings();
const { mutateAsync: saveSettings } = useSaveSettings();

const [isUpToDate, setIsUpToDate] = React.useState(settingsAreUpToDate());

const saveUserSettings = async (newSettings: Partial<PostSettings>) => {
const updatedSettings: Partial<PostSettings> = {
...userSettings,
Expand All @@ -39,27 +30,15 @@ export function SettingsProvider({ children }: SettingsProviderProps) {
delete updatedSettings.LLM_API_KEY;
}

await saveSettings(updatedSettings, {
onSuccess: () => {
if (!isUpToDate) {
localStorage.setItem(
"SETTINGS_VERSION",
LATEST_SETTINGS_VERSION.toString(),
);
setIsUpToDate(true);
}
},
});
await saveSettings(updatedSettings);
};

const value = React.useMemo(
() => ({
isUpToDate,
setIsUpToDate,
saveUserSettings,
settings: userSettings,
}),
[isUpToDate, setIsUpToDate, saveUserSettings, userSettings],
[saveUserSettings, userSettings],
);

return <SettingsContext value={value}>{children}</SettingsContext>;
Expand Down
7 changes: 2 additions & 5 deletions frontend/src/hooks/mutation/use-save-settings.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { useMutation, useQueryClient } from "@tanstack/react-query";
import {
DEFAULT_SETTINGS,
PostApiSettings,
PostSettings,
} from "#/services/settings";
import { DEFAULT_SETTINGS } from "#/services/settings";
import OpenHands from "#/api/open-hands";
import { PostSettings, PostApiSettings } from "#/types/settings";

const saveSettingsMutationFn = async (settings: Partial<PostSettings>) => {
const apiSettings: Partial<PostApiSettings> = {
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/hooks/query/use-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useQuery } from "@tanstack/react-query";
import React from "react";
import posthog from "posthog-js";
import { AxiosError } from "axios";
import { DEFAULT_SETTINGS, getLocalStorageSettings } from "#/services/settings";
import { DEFAULT_SETTINGS } from "#/services/settings";
import OpenHands from "#/api/open-hands";
import { useAuth } from "#/context/auth-context";

Expand All @@ -26,7 +26,7 @@ const getSettingsQueryFn = async () => {
};
}

return getLocalStorageSettings();
return DEFAULT_SETTINGS;
} catch (error) {
if (error instanceof AxiosError) {
if (error.response?.status === 404) {
Expand Down
52 changes: 0 additions & 52 deletions frontend/src/hooks/use-maybe-migrate-settings.ts

This file was deleted.

7 changes: 2 additions & 5 deletions frontend/src/mocks/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@ import {
Conversation,
ResultSet,
} from "#/api/open-hands.types";
import {
ApiSettings,
DEFAULT_SETTINGS,
PostApiSettings,
} from "#/services/settings";
import { DEFAULT_SETTINGS } from "#/services/settings";
import { ApiSettings, PostApiSettings } from "#/types/settings";

export const MOCK_DEFAULT_USER_SETTINGS: ApiSettings | PostApiSettings = {
llm_model: DEFAULT_SETTINGS.LLM_MODEL,
Expand Down
3 changes: 0 additions & 3 deletions frontend/src/routes/_oh/route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { Sidebar } from "#/components/features/sidebar/sidebar";
import { WaitlistModal } from "#/components/features/waitlist/waitlist-modal";
import { AnalyticsConsentFormModal } from "#/components/features/analytics/analytics-consent-form-modal";
import { useSettings } from "#/hooks/query/use-settings";
import { useMaybeMigrateSettings } from "#/hooks/use-maybe-migrate-settings";
import { useAuth } from "#/context/auth-context";

export function ErrorBoundary() {
Expand Down Expand Up @@ -44,8 +43,6 @@ export function ErrorBoundary() {
}

export default function MainApp() {
useMaybeMigrateSettings();

const { githubTokenIsSet } = useAuth();
const { data: settings } = useSettings();

Expand Down
Loading

0 comments on commit b987f33

Please sign in to comment.