mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
fix(frontend): Respect HIDE_LLM_SETTINGS flag in settings modal (#12400)
This commit is contained in:
@@ -1,25 +1,70 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { renderWithProviders } from "test-utils";
|
||||
import {
|
||||
renderWithProviders,
|
||||
createAxiosNotFoundErrorObject,
|
||||
} from "test-utils";
|
||||
import { createRoutesStub } from "react-router";
|
||||
import { waitFor } from "@testing-library/react";
|
||||
import { screen, waitFor } from "@testing-library/react";
|
||||
import { Sidebar } from "#/components/features/sidebar/sidebar";
|
||||
import SettingsService from "#/api/settings-service/settings-service.api";
|
||||
import OptionService from "#/api/option-service/option-service.api";
|
||||
import { MOCK_DEFAULT_USER_SETTINGS } from "#/mocks/handlers";
|
||||
import { GetConfigResponse } from "#/api/option-service/option.types";
|
||||
|
||||
// Helper to create mock config with sensible defaults
|
||||
const createMockConfig = (
|
||||
overrides: Omit<Partial<GetConfigResponse>, "FEATURE_FLAGS"> & {
|
||||
FEATURE_FLAGS?: Partial<GetConfigResponse["FEATURE_FLAGS"]>;
|
||||
} = {},
|
||||
): GetConfigResponse => {
|
||||
const { FEATURE_FLAGS: featureFlagOverrides, ...restOverrides } = overrides;
|
||||
return {
|
||||
APP_MODE: "oss",
|
||||
GITHUB_CLIENT_ID: "test-client-id",
|
||||
POSTHOG_CLIENT_KEY: "test-posthog-key",
|
||||
FEATURE_FLAGS: {
|
||||
ENABLE_BILLING: false,
|
||||
HIDE_LLM_SETTINGS: false,
|
||||
ENABLE_JIRA: false,
|
||||
ENABLE_JIRA_DC: false,
|
||||
ENABLE_LINEAR: false,
|
||||
...featureFlagOverrides,
|
||||
},
|
||||
...restOverrides,
|
||||
};
|
||||
};
|
||||
|
||||
// These tests will now fail because the conversation panel is rendered through a portal
|
||||
// and technically not a child of the Sidebar component.
|
||||
|
||||
const RouterStub = createRoutesStub([
|
||||
const ConversationRouterStub = createRoutesStub([
|
||||
{
|
||||
path: "/conversation/:conversationId",
|
||||
Component: () => <Sidebar />,
|
||||
},
|
||||
]);
|
||||
|
||||
const renderSidebar = () =>
|
||||
renderWithProviders(<RouterStub initialEntries={["/conversation/123"]} />);
|
||||
const SettingsRouterStub = createRoutesStub([
|
||||
{
|
||||
path: "/settings",
|
||||
Component: () => <Sidebar />,
|
||||
},
|
||||
]);
|
||||
|
||||
const renderSidebar = (path: "conversation" | "settings" = "conversation") => {
|
||||
if (path === "settings") {
|
||||
return renderWithProviders(
|
||||
<SettingsRouterStub initialEntries={["/settings"]} />,
|
||||
);
|
||||
}
|
||||
return renderWithProviders(
|
||||
<ConversationRouterStub initialEntries={["/conversation/123"]} />,
|
||||
);
|
||||
};
|
||||
|
||||
describe("Sidebar", () => {
|
||||
const getSettingsSpy = vi.spyOn(SettingsService, "getSettings");
|
||||
const getConfigSpy = vi.spyOn(OptionService, "getConfig");
|
||||
|
||||
afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -29,4 +74,100 @@ describe("Sidebar", () => {
|
||||
renderSidebar();
|
||||
await waitFor(() => expect(getSettingsSpy).toHaveBeenCalled());
|
||||
});
|
||||
|
||||
describe("Settings modal auto-open behavior", () => {
|
||||
it("should NOT open settings modal when HIDE_LLM_SETTINGS is true even with 404 error", async () => {
|
||||
getConfigSpy.mockResolvedValue(
|
||||
createMockConfig({ FEATURE_FLAGS: { HIDE_LLM_SETTINGS: true } }),
|
||||
);
|
||||
getSettingsSpy.mockRejectedValue(createAxiosNotFoundErrorObject());
|
||||
|
||||
renderSidebar();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getConfigSpy).toHaveBeenCalled();
|
||||
expect(getSettingsSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Settings modal should NOT appear when HIDE_LLM_SETTINGS is true
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("ai-config-modal")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it("should open settings modal when HIDE_LLM_SETTINGS is false and 404 error in OSS mode", async () => {
|
||||
getConfigSpy.mockResolvedValue(
|
||||
createMockConfig({ FEATURE_FLAGS: { HIDE_LLM_SETTINGS: false } }),
|
||||
);
|
||||
getSettingsSpy.mockRejectedValue(createAxiosNotFoundErrorObject());
|
||||
|
||||
renderSidebar();
|
||||
|
||||
// Settings modal should appear when HIDE_LLM_SETTINGS is false
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("ai-config-modal")).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it("should NOT open settings modal in SaaS mode even with 404 error", async () => {
|
||||
getConfigSpy.mockResolvedValue(
|
||||
createMockConfig({
|
||||
APP_MODE: "saas",
|
||||
FEATURE_FLAGS: { HIDE_LLM_SETTINGS: false },
|
||||
}),
|
||||
);
|
||||
getSettingsSpy.mockRejectedValue(createAxiosNotFoundErrorObject());
|
||||
|
||||
renderSidebar();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getConfigSpy).toHaveBeenCalled();
|
||||
expect(getSettingsSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Settings modal should NOT appear in SaaS mode (only opens in OSS mode)
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("ai-config-modal")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it("should NOT open settings modal when settings exist (no 404 error)", async () => {
|
||||
getConfigSpy.mockResolvedValue(
|
||||
createMockConfig({ FEATURE_FLAGS: { HIDE_LLM_SETTINGS: false } }),
|
||||
);
|
||||
getSettingsSpy.mockResolvedValue(MOCK_DEFAULT_USER_SETTINGS);
|
||||
|
||||
renderSidebar();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getConfigSpy).toHaveBeenCalled();
|
||||
expect(getSettingsSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Settings modal should NOT appear when settings exist
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("ai-config-modal")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it("should NOT open settings modal when on /settings path", async () => {
|
||||
getConfigSpy.mockResolvedValue(
|
||||
createMockConfig({ FEATURE_FLAGS: { HIDE_LLM_SETTINGS: false } }),
|
||||
);
|
||||
getSettingsSpy.mockRejectedValue(createAxiosNotFoundErrorObject());
|
||||
|
||||
renderSidebar("settings");
|
||||
|
||||
await waitFor(() => {
|
||||
expect(getConfigSpy).toHaveBeenCalled();
|
||||
expect(getSettingsSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Settings modal should NOT appear when on /settings path
|
||||
// (prevents modal from showing when user is already viewing settings)
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByTestId("ai-config-modal")).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,7 +16,7 @@ import { MicroagentManagementButton } from "#/components/shared/buttons/microage
|
||||
import { cn } from "#/utils/utils";
|
||||
|
||||
export function Sidebar() {
|
||||
const location = useLocation();
|
||||
const { pathname } = useLocation();
|
||||
const user = useGitUser();
|
||||
const { data: config } = useConfig();
|
||||
const {
|
||||
@@ -32,10 +32,8 @@ export function Sidebar() {
|
||||
const [conversationPanelIsOpen, setConversationPanelIsOpen] =
|
||||
React.useState(false);
|
||||
|
||||
const { pathname } = useLocation();
|
||||
|
||||
React.useEffect(() => {
|
||||
if (location.pathname === "/settings") {
|
||||
if (pathname === "/settings") {
|
||||
setSettingsModalIsOpen(false);
|
||||
} else if (
|
||||
!isFetchingSettings &&
|
||||
@@ -47,14 +45,20 @@ export function Sidebar() {
|
||||
displayErrorToast(
|
||||
"Something went wrong while fetching settings. Please reload the page.",
|
||||
);
|
||||
} else if (config?.APP_MODE === "oss" && settingsError?.status === 404) {
|
||||
} else if (
|
||||
config?.APP_MODE === "oss" &&
|
||||
settingsError?.status === 404 &&
|
||||
!config?.FEATURE_FLAGS?.HIDE_LLM_SETTINGS
|
||||
) {
|
||||
setSettingsModalIsOpen(true);
|
||||
}
|
||||
}, [
|
||||
settingsError?.status,
|
||||
settingsError,
|
||||
pathname,
|
||||
isFetchingSettings,
|
||||
location.pathname,
|
||||
settingsIsError,
|
||||
settingsError,
|
||||
config?.APP_MODE,
|
||||
config?.FEATURE_FLAGS?.HIDE_LLM_SETTINGS,
|
||||
]);
|
||||
|
||||
return (
|
||||
|
||||
@@ -5,39 +5,67 @@ import test, { expect } from "@playwright/test";
|
||||
*
|
||||
* This test verifies that the user can move their cursor diagonally from the
|
||||
* avatar to the context menu without the menu closing unexpectedly.
|
||||
*
|
||||
* The component supports both CSS hover and click-to-toggle for the menu.
|
||||
* We use click-to-toggle which is more reliable in automated tests than
|
||||
* CSS hover simulation.
|
||||
*/
|
||||
test("avatar context menu stays open when moving cursor diagonally to menu", async ({
|
||||
page,
|
||||
browserName,
|
||||
}) => {
|
||||
// WebKit: Playwright hover/mouse simulation is flaky for CSS hover-only menus.
|
||||
test.skip(browserName === "webkit", "Playwright hover simulation unreliable");
|
||||
|
||||
await page.goto("/");
|
||||
|
||||
// Wait for the page to be fully loaded and check for AI config modal
|
||||
// The modal may appear for new users in OSS mode without settings
|
||||
const aiConfigModal = page.getByTestId("ai-config-modal");
|
||||
if (await aiConfigModal.isVisible().catch(() => false)) {
|
||||
// In OSS mock mode, missing settings can open the AI-config modal; its backdrop
|
||||
// intercepts pointer events and prevents hover interactions.
|
||||
|
||||
// Give the modal a chance to appear (it may load asynchronously)
|
||||
try {
|
||||
await aiConfigModal.waitFor({ state: "visible", timeout: 3000 });
|
||||
// Modal appeared - dismiss it by clicking save
|
||||
await page.getByTestId("save-settings-button").click();
|
||||
await expect(aiConfigModal).toBeHidden();
|
||||
} catch {
|
||||
// Modal didn't appear within timeout - that's fine, continue with test
|
||||
}
|
||||
|
||||
const userAvatar = page.getByTestId("user-avatar");
|
||||
await expect(userAvatar).toBeVisible();
|
||||
|
||||
const avatarBox = await userAvatar.boundingBox();
|
||||
if (!avatarBox) {
|
||||
throw new Error("Could not get bounding box for avatar");
|
||||
}
|
||||
|
||||
const avatarCenterX = avatarBox.x + avatarBox.width / 2;
|
||||
const avatarCenterY = avatarBox.y + avatarBox.height / 2;
|
||||
await page.mouse.move(avatarCenterX, avatarCenterY);
|
||||
// Use force:true to bypass the hover bridge pseudo-element that can
|
||||
// intercept clicks when the mouse triggers group-hover state
|
||||
await userAvatar.click({ force: true });
|
||||
|
||||
const contextMenu = page.getByTestId("account-settings-context-menu");
|
||||
await expect(contextMenu).toBeVisible();
|
||||
|
||||
const menuWrapper = contextMenu.locator("..");
|
||||
await expect(menuWrapper).toHaveCSS("opacity", "1");
|
||||
|
||||
// Now test diagonal mouse movement - move from avatar to menu
|
||||
// The menu should stay open due to the hover bridge in the component
|
||||
const avatarBox = await userAvatar.boundingBox();
|
||||
const menuBox = await contextMenu.boundingBox();
|
||||
|
||||
if (!avatarBox || !menuBox) {
|
||||
throw new Error("Could not get bounding boxes");
|
||||
}
|
||||
|
||||
// Move diagonally from avatar center toward the menu
|
||||
const startX = avatarBox.x + avatarBox.width / 2;
|
||||
const startY = avatarBox.y + avatarBox.height / 2;
|
||||
const endX = menuBox.x + menuBox.width / 2;
|
||||
const endY = menuBox.y + menuBox.height / 2;
|
||||
|
||||
// Simulate diagonal movement with multiple steps
|
||||
const steps = 5;
|
||||
for (let i = 0; i <= steps; i++) {
|
||||
const x = startX + ((endX - startX) * i) / steps;
|
||||
const y = startY + ((endY - startY) * i) / steps;
|
||||
await page.mouse.move(x, y);
|
||||
}
|
||||
|
||||
// Menu should still be visible after diagonal movement
|
||||
await expect(contextMenu).toBeVisible();
|
||||
await expect(menuWrapper).toHaveCSS("opacity", "1");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user