mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-26 05:48:36 +08:00
fix(frontend): llm settings view resets to basic after saving (#12097)
This commit is contained in:
parent
5553d3ca2e
commit
10edb28729
@ -910,6 +910,162 @@ describe("Form submission", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("View persistence after saving advanced settings", () => {
|
||||||
|
it("should remain on Advanced view after saving when memory condenser is disabled", async () => {
|
||||||
|
// Arrange: Start with default settings (basic view)
|
||||||
|
const getSettingsSpy = vi.spyOn(SettingsService, "getSettings");
|
||||||
|
getSettingsSpy.mockResolvedValue({
|
||||||
|
...MOCK_DEFAULT_USER_SETTINGS,
|
||||||
|
});
|
||||||
|
const saveSettingsSpy = vi.spyOn(SettingsService, "saveSettings");
|
||||||
|
saveSettingsSpy.mockResolvedValue(true);
|
||||||
|
|
||||||
|
renderLlmSettingsScreen();
|
||||||
|
await screen.findByTestId("llm-settings-screen");
|
||||||
|
|
||||||
|
// Verify we start in basic view
|
||||||
|
expect(screen.getByTestId("llm-settings-form-basic")).toBeInTheDocument();
|
||||||
|
|
||||||
|
// Act: User manually switches to Advanced view
|
||||||
|
const advancedSwitch = screen.getByTestId("advanced-settings-switch");
|
||||||
|
await userEvent.click(advancedSwitch);
|
||||||
|
await screen.findByTestId("llm-settings-form-advanced");
|
||||||
|
|
||||||
|
// User disables memory condenser (advanced-only setting)
|
||||||
|
const condenserSwitch = screen.getByTestId(
|
||||||
|
"enable-memory-condenser-switch",
|
||||||
|
);
|
||||||
|
expect(condenserSwitch).toBeChecked();
|
||||||
|
await userEvent.click(condenserSwitch);
|
||||||
|
expect(condenserSwitch).not.toBeChecked();
|
||||||
|
|
||||||
|
// Mock the updated settings that will be returned after save
|
||||||
|
getSettingsSpy.mockResolvedValue({
|
||||||
|
...MOCK_DEFAULT_USER_SETTINGS,
|
||||||
|
enable_default_condenser: false, // Now disabled
|
||||||
|
});
|
||||||
|
|
||||||
|
// User saves settings
|
||||||
|
const submitButton = screen.getByTestId("submit-button");
|
||||||
|
await userEvent.click(submitButton);
|
||||||
|
|
||||||
|
// Assert: View should remain on Advanced after save
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(
|
||||||
|
screen.getByTestId("llm-settings-form-advanced"),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
expect(
|
||||||
|
screen.queryByTestId("llm-settings-form-basic"),
|
||||||
|
).not.toBeInTheDocument();
|
||||||
|
expect(advancedSwitch).toBeChecked();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should remain on Advanced view after saving when condenser max size is customized", async () => {
|
||||||
|
// Arrange: Start with default settings
|
||||||
|
const getSettingsSpy = vi.spyOn(SettingsService, "getSettings");
|
||||||
|
getSettingsSpy.mockResolvedValue({
|
||||||
|
...MOCK_DEFAULT_USER_SETTINGS,
|
||||||
|
});
|
||||||
|
const saveSettingsSpy = vi.spyOn(SettingsService, "saveSettings");
|
||||||
|
saveSettingsSpy.mockResolvedValue(true);
|
||||||
|
|
||||||
|
renderLlmSettingsScreen();
|
||||||
|
await screen.findByTestId("llm-settings-screen");
|
||||||
|
|
||||||
|
// Act: User manually switches to Advanced view
|
||||||
|
const advancedSwitch = screen.getByTestId("advanced-settings-switch");
|
||||||
|
await userEvent.click(advancedSwitch);
|
||||||
|
await screen.findByTestId("llm-settings-form-advanced");
|
||||||
|
|
||||||
|
// User sets custom condenser max size (advanced-only setting)
|
||||||
|
const condenserMaxSizeInput = screen.getByTestId(
|
||||||
|
"condenser-max-size-input",
|
||||||
|
);
|
||||||
|
await userEvent.clear(condenserMaxSizeInput);
|
||||||
|
await userEvent.type(condenserMaxSizeInput, "200");
|
||||||
|
|
||||||
|
// Mock the updated settings that will be returned after save
|
||||||
|
getSettingsSpy.mockResolvedValue({
|
||||||
|
...MOCK_DEFAULT_USER_SETTINGS,
|
||||||
|
condenser_max_size: 200, // Custom value
|
||||||
|
});
|
||||||
|
|
||||||
|
// User saves settings
|
||||||
|
const submitButton = screen.getByTestId("submit-button");
|
||||||
|
await userEvent.click(submitButton);
|
||||||
|
|
||||||
|
// Assert: View should remain on Advanced after save
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(
|
||||||
|
screen.getByTestId("llm-settings-form-advanced"),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
expect(
|
||||||
|
screen.queryByTestId("llm-settings-form-basic"),
|
||||||
|
).not.toBeInTheDocument();
|
||||||
|
expect(advancedSwitch).toBeChecked();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should remain on Advanced view after saving when search API key is set", async () => {
|
||||||
|
// Arrange: Start with default settings (non-SaaS mode to show search API key field)
|
||||||
|
const getConfigSpy = vi.spyOn(OptionService, "getConfig");
|
||||||
|
getConfigSpy.mockResolvedValue({
|
||||||
|
APP_MODE: "oss",
|
||||||
|
GITHUB_CLIENT_ID: "fake-github-client-id",
|
||||||
|
POSTHOG_CLIENT_KEY: "fake-posthog-client-key",
|
||||||
|
FEATURE_FLAGS: {
|
||||||
|
ENABLE_BILLING: false,
|
||||||
|
HIDE_LLM_SETTINGS: false,
|
||||||
|
ENABLE_JIRA: false,
|
||||||
|
ENABLE_JIRA_DC: false,
|
||||||
|
ENABLE_LINEAR: false,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const getSettingsSpy = vi.spyOn(SettingsService, "getSettings");
|
||||||
|
getSettingsSpy.mockResolvedValue({
|
||||||
|
...MOCK_DEFAULT_USER_SETTINGS,
|
||||||
|
search_api_key: "", // Default empty value
|
||||||
|
});
|
||||||
|
const saveSettingsSpy = vi.spyOn(SettingsService, "saveSettings");
|
||||||
|
saveSettingsSpy.mockResolvedValue(true);
|
||||||
|
|
||||||
|
renderLlmSettingsScreen();
|
||||||
|
await screen.findByTestId("llm-settings-screen");
|
||||||
|
|
||||||
|
// Act: User manually switches to Advanced view
|
||||||
|
const advancedSwitch = screen.getByTestId("advanced-settings-switch");
|
||||||
|
await userEvent.click(advancedSwitch);
|
||||||
|
await screen.findByTestId("llm-settings-form-advanced");
|
||||||
|
|
||||||
|
// User sets search API key (advanced-only setting)
|
||||||
|
const searchApiKeyInput = screen.getByTestId("search-api-key-input");
|
||||||
|
await userEvent.type(searchApiKeyInput, "test-search-api-key");
|
||||||
|
|
||||||
|
// Mock the updated settings that will be returned after save
|
||||||
|
getSettingsSpy.mockResolvedValue({
|
||||||
|
...MOCK_DEFAULT_USER_SETTINGS,
|
||||||
|
search_api_key: "test-search-api-key", // Now set
|
||||||
|
});
|
||||||
|
|
||||||
|
// User saves settings
|
||||||
|
const submitButton = screen.getByTestId("submit-button");
|
||||||
|
await userEvent.click(submitButton);
|
||||||
|
|
||||||
|
// Assert: View should remain on Advanced after save
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(
|
||||||
|
screen.getByTestId("llm-settings-form-advanced"),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
expect(
|
||||||
|
screen.queryByTestId("llm-settings-form-basic"),
|
||||||
|
).not.toBeInTheDocument();
|
||||||
|
expect(advancedSwitch).toBeChecked();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("Status toasts", () => {
|
describe("Status toasts", () => {
|
||||||
describe("Basic form", () => {
|
describe("Basic form", () => {
|
||||||
it("should call displaySuccessToast when the settings are saved", async () => {
|
it("should call displaySuccessToast when the settings are saved", async () => {
|
||||||
|
|||||||
@ -29,5 +29,75 @@ describe("hasAdvancedSettingsSet", () => {
|
|||||||
}),
|
}),
|
||||||
).toBe(true);
|
).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("enable_default_condenser is disabled", () => {
|
||||||
|
// Arrange
|
||||||
|
const settings = {
|
||||||
|
...DEFAULT_SETTINGS,
|
||||||
|
enable_default_condenser: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
// Act
|
||||||
|
const result = hasAdvancedSettingsSet(settings);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("condenser_max_size is customized above default", () => {
|
||||||
|
// Arrange
|
||||||
|
const settings = {
|
||||||
|
...DEFAULT_SETTINGS,
|
||||||
|
condenser_max_size: 200,
|
||||||
|
};
|
||||||
|
|
||||||
|
// Act
|
||||||
|
const result = hasAdvancedSettingsSet(settings);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("condenser_max_size is customized below default", () => {
|
||||||
|
// Arrange
|
||||||
|
const settings = {
|
||||||
|
...DEFAULT_SETTINGS,
|
||||||
|
condenser_max_size: 50,
|
||||||
|
};
|
||||||
|
|
||||||
|
// Act
|
||||||
|
const result = hasAdvancedSettingsSet(settings);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("search_api_key is set to non-empty value", () => {
|
||||||
|
// Arrange
|
||||||
|
const settings = {
|
||||||
|
...DEFAULT_SETTINGS,
|
||||||
|
search_api_key: "test-api-key-123",
|
||||||
|
};
|
||||||
|
|
||||||
|
// Act
|
||||||
|
const result = hasAdvancedSettingsSet(settings);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("search_api_key with whitespace is treated as set", () => {
|
||||||
|
// Arrange
|
||||||
|
const settings = {
|
||||||
|
...DEFAULT_SETTINGS,
|
||||||
|
search_api_key: " test-key ",
|
||||||
|
};
|
||||||
|
|
||||||
|
// Act
|
||||||
|
const result = hasAdvancedSettingsSet(settings);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
expect(result).toBe(true);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@ -1,6 +1,48 @@
|
|||||||
import { DEFAULT_SETTINGS } from "#/services/settings";
|
import { DEFAULT_SETTINGS } from "#/services/settings";
|
||||||
import { Settings } from "#/types/settings";
|
import { Settings } from "#/types/settings";
|
||||||
|
|
||||||
export const hasAdvancedSettingsSet = (settings: Partial<Settings>): boolean =>
|
/**
|
||||||
Object.keys(settings).length > 0 &&
|
* Determines if any advanced-only settings are configured.
|
||||||
(!!settings.llm_base_url || settings.agent !== DEFAULT_SETTINGS.agent);
|
* Advanced-only settings are those that appear only in the Advanced Settings view
|
||||||
|
* and not in the Basic Settings view.
|
||||||
|
*
|
||||||
|
* Advanced-only fields:
|
||||||
|
* - llm_base_url: Custom base URL for LLM API
|
||||||
|
* - agent: Custom agent selection (when not using default)
|
||||||
|
* - enable_default_condenser: Memory condenser toggle (when disabled, as default is enabled)
|
||||||
|
* - condenser_max_size: Custom condenser size (when different from default)
|
||||||
|
* - search_api_key: Search API key (when set)
|
||||||
|
*/
|
||||||
|
export const hasAdvancedSettingsSet = (
|
||||||
|
settings: Partial<Settings>,
|
||||||
|
): boolean => {
|
||||||
|
if (Object.keys(settings).length === 0) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check for advanced-only settings that differ from defaults
|
||||||
|
const hasBaseUrl =
|
||||||
|
!!settings.llm_base_url && settings.llm_base_url.trim() !== "";
|
||||||
|
const hasCustomAgent =
|
||||||
|
settings.agent !== undefined && settings.agent !== DEFAULT_SETTINGS.agent;
|
||||||
|
// Default is true, so only check if explicitly disabled
|
||||||
|
const hasDisabledCondenser = settings.enable_default_condenser === false;
|
||||||
|
// Check if condenser size differs from default (default is 120)
|
||||||
|
const hasCustomCondenserSize =
|
||||||
|
settings.condenser_max_size !== undefined &&
|
||||||
|
settings.condenser_max_size !== null &&
|
||||||
|
settings.condenser_max_size !== DEFAULT_SETTINGS.condenser_max_size;
|
||||||
|
// Check if search API key is set (non-empty string)
|
||||||
|
const hasSearchApiKey =
|
||||||
|
settings.search_api_key !== undefined &&
|
||||||
|
settings.search_api_key !== null &&
|
||||||
|
settings.search_api_key.trim() !== "";
|
||||||
|
|
||||||
|
return (
|
||||||
|
hasBaseUrl ||
|
||||||
|
hasCustomAgent ||
|
||||||
|
hasDisabledCondenser ||
|
||||||
|
hasCustomCondenserSize ||
|
||||||
|
hasSearchApiKey
|
||||||
|
);
|
||||||
|
};
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user