From 3c6edfe14bfeb589a0c484f68e6f86016c7e3e95 Mon Sep 17 00:00:00 2001 From: Pegasus <42954461+leonace924@users.noreply.github.com> Date: Wed, 14 Jan 2026 18:14:32 -0500 Subject: [PATCH] fix(frontend): Respect HIDE_LLM_SETTINGS flag in settings modal (#12400) --- .../features/sidebar/sidebar.test.tsx | 151 +++++++++++++++++- .../components/features/sidebar/sidebar.tsx | 20 ++- frontend/tests/avatar-menu.spec.ts | 58 +++++-- 3 files changed, 201 insertions(+), 28 deletions(-) diff --git a/frontend/__tests__/components/features/sidebar/sidebar.test.tsx b/frontend/__tests__/components/features/sidebar/sidebar.test.tsx index dc5be687f5..3314f325e5 100644 --- a/frontend/__tests__/components/features/sidebar/sidebar.test.tsx +++ b/frontend/__tests__/components/features/sidebar/sidebar.test.tsx @@ -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, "FEATURE_FLAGS"> & { + FEATURE_FLAGS?: Partial; + } = {}, +): 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: () => , }, ]); -const renderSidebar = () => - renderWithProviders(); +const SettingsRouterStub = createRoutesStub([ + { + path: "/settings", + Component: () => , + }, +]); + +const renderSidebar = (path: "conversation" | "settings" = "conversation") => { + if (path === "settings") { + return renderWithProviders( + , + ); + } + return renderWithProviders( + , + ); +}; 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(); + }); + }); + }); }); diff --git a/frontend/src/components/features/sidebar/sidebar.tsx b/frontend/src/components/features/sidebar/sidebar.tsx index ea352d046b..b5b2e23aab 100644 --- a/frontend/src/components/features/sidebar/sidebar.tsx +++ b/frontend/src/components/features/sidebar/sidebar.tsx @@ -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 ( diff --git a/frontend/tests/avatar-menu.spec.ts b/frontend/tests/avatar-menu.spec.ts index fbc4bd5782..c7d49ac302 100644 --- a/frontend/tests/avatar-menu.spec.ts +++ b/frontend/tests/avatar-menu.spec.ts @@ -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"); });