From c78b92346830e1c83d090de4042f2d85b996bf02 Mon Sep 17 00:00:00 2001 From: HeyItsChloe <54480367+HeyItsChloe@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:28:27 -0700 Subject: [PATCH] feat(frontend): Remove onboarding feature flags (#13947) Co-authored-by: Hiep Le <69354317+hieptl@users.noreply.github.com> Co-authored-by: openhands Co-authored-by: hieptl --- enterprise/server/routes/auth.py | 7 +- .../onboarding/onboarding-form.test.tsx | 125 +++++++++++++++++- frontend/src/routes/onboarding-form.tsx | 5 +- frontend/src/utils/feature-flags.ts | 1 - 4 files changed, 127 insertions(+), 11 deletions(-) diff --git a/enterprise/server/routes/auth.py b/enterprise/server/routes/auth.py index 6f3438a4b6..52aff28693 100644 --- a/enterprise/server/routes/auth.py +++ b/enterprise/server/routes/auth.py @@ -581,12 +581,7 @@ async def authenticate(request: Request): async def _should_redirect_to_onboarding(user_id: str, user: User) -> bool: """Check if user should be redirected to onboarding after TOS acceptance. - - Backend always redirects applicable users to /onboarding. The frontend - checks the ENABLE_ONBOARDING feature flag (localStorage) and redirects - to / if the flag is disabled. This avoids needing helm chart changes. - - + Backend always redirects applicable users to /onboarding. Returns True if: - User has onboarding_completed explicitly set to False (new users) - Either: diff --git a/frontend/__tests__/components/features/onboarding/onboarding-form.test.tsx b/frontend/__tests__/components/features/onboarding/onboarding-form.test.tsx index 8f99b384cf..e024bcef84 100644 --- a/frontend/__tests__/components/features/onboarding/onboarding-form.test.tsx +++ b/frontend/__tests__/components/features/onboarding/onboarding-form.test.tsx @@ -5,7 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { I18nextProvider } from "react-i18next"; import i18n from "i18next"; -import OnboardingForm from "#/routes/onboarding-form"; +import OnboardingForm, { clientLoader } from "#/routes/onboarding-form"; const mockMutate = vi.fn(); const mockNavigate = vi.fn(); @@ -39,6 +39,23 @@ vi.mock("#/hooks/use-tracking", () => ({ }), })); +// Mocks for clientLoader tests +const mockQueryClientGetData = vi.fn(); +const mockQueryClientSetData = vi.fn(); +vi.mock("#/query-client-config", () => ({ + queryClient: { + getQueryData: (...args: unknown[]) => mockQueryClientGetData(...args), + setQueryData: (...args: unknown[]) => mockQueryClientSetData(...args), + }, +})); + +const mockGetConfig = vi.fn(); +vi.mock("#/api/option-service/option-service.api", () => ({ + default: { + getConfig: () => mockGetConfig(), + }, +})); + const renderOnboardingForm = async () => { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, @@ -537,3 +554,109 @@ describe("OnboardingForm - Self-Hosted Mode", () => { expect(mockMutate).toHaveBeenCalledTimes(1); }); }); + +describe("onboarding-form clientLoader", () => { + beforeEach(() => { + mockQueryClientGetData.mockReset(); + mockQueryClientSetData.mockReset(); + mockGetConfig.mockReset(); + }); + + describe("redirect behavior", () => { + it("should redirect to / when app_mode is oss", async () => { + const ossConfig = { + app_mode: "oss", + feature_flags: { deployment_mode: undefined }, + }; + mockQueryClientGetData.mockReturnValue(ossConfig); + + const result = await clientLoader(); + + expect(result).toBeDefined(); + expect((result as Response).status).toBe(302); + expect((result as Response).headers.get("Location")).toBe("/"); + }); + + it("should redirect to / when app_mode is undefined", async () => { + const undefinedConfig = { + app_mode: undefined, + feature_flags: { deployment_mode: "cloud" }, + }; + mockQueryClientGetData.mockReturnValue(undefinedConfig); + + const result = await clientLoader(); + + expect(result).toBeDefined(); + expect((result as Response).status).toBe(302); + expect((result as Response).headers.get("Location")).toBe("/"); + }); + + it("should redirect to / when config is null", async () => { + mockQueryClientGetData.mockReturnValue(null); + mockGetConfig.mockResolvedValue(null); + + const result = await clientLoader(); + + expect(result).toBeDefined(); + expect((result as Response).status).toBe(302); + expect((result as Response).headers.get("Location")).toBe("/"); + }); + + it("should allow access and return config when app_mode is saas with cloud deployment", async () => { + const saasCloudConfig = { + app_mode: "saas", + feature_flags: { deployment_mode: "cloud" }, + }; + mockQueryClientGetData.mockReturnValue(saasCloudConfig); + + const result = await clientLoader(); + + expect(result).toEqual({ config: saasCloudConfig }); + }); + + it("should allow access and return config when app_mode is saas with self_hosted deployment", async () => { + const saasSelfHostedConfig = { + app_mode: "saas", + feature_flags: { deployment_mode: "self_hosted" }, + }; + mockQueryClientGetData.mockReturnValue(saasSelfHostedConfig); + + const result = await clientLoader(); + + expect(result).toEqual({ config: saasSelfHostedConfig }); + }); + }); + + describe("config fetching", () => { + it("should use cached config from queryClient when available", async () => { + const cachedConfig = { + app_mode: "saas", + feature_flags: { deployment_mode: "cloud" }, + }; + mockQueryClientGetData.mockReturnValue(cachedConfig); + + await clientLoader(); + + expect(mockQueryClientGetData).toHaveBeenCalledWith(["web-client-config"]); + expect(mockGetConfig).not.toHaveBeenCalled(); + }); + + it("should fetch config from OptionService when not cached", async () => { + const fetchedConfig = { + app_mode: "saas", + feature_flags: { deployment_mode: "cloud" }, + }; + mockQueryClientGetData.mockReturnValue(null); + mockGetConfig.mockResolvedValue(fetchedConfig); + + const result = await clientLoader(); + + expect(mockGetConfig).toHaveBeenCalled(); + expect(mockQueryClientSetData).toHaveBeenCalledWith( + ["web-client-config"], + fetchedConfig, + ); + expect(result).toEqual({ config: fetchedConfig }); + }); + }); +}); diff --git a/frontend/src/routes/onboarding-form.tsx b/frontend/src/routes/onboarding-form.tsx index c64961f1e5..9799253056 100644 --- a/frontend/src/routes/onboarding-form.tsx +++ b/frontend/src/routes/onboarding-form.tsx @@ -8,7 +8,6 @@ import { I18nKey } from "#/i18n/declaration"; import OpenHandsLogoWhite from "#/assets/branding/openhands-logo-white.svg?react"; import { useSubmitOnboarding } from "#/hooks/mutation/use-submit-onboarding"; import { useTracking } from "#/hooks/use-tracking"; -import { ENABLE_ONBOARDING } from "#/utils/feature-flags"; import { cn } from "#/utils/utils"; import { useMe } from "#/hooks/query/use-me"; import { @@ -32,7 +31,7 @@ export const clientLoader = async () => { // Only allow access to onboarding for SaaS mode (cloud or self-hosted) // OSS users should never reach /onboarding - if (config?.app_mode !== "saas" || !ENABLE_ONBOARDING()) { + if (config?.app_mode !== "saas") { return redirect("/"); } @@ -209,7 +208,7 @@ function OnboardingForm() { const translatedInputFields = getTranslatedInputFields(currentStep, t); return ( -
+
loadFeatureFlag("HIDE_LLM_SETTINGS"); export const VSCODE_IN_NEW_TAB = () => loadFeatureFlag("VSCODE_IN_NEW_TAB"); export const ENABLE_TRAJECTORY_REPLAY = () => loadFeatureFlag("TRAJECTORY_REPLAY"); -export const ENABLE_ONBOARDING = () => loadFeatureFlag("ENABLE_ONBOARDING"); export const ENABLE_SANDBOX_GROUPING = () => loadFeatureFlag("SANDBOX_GROUPING"); export const ENABLE_AUTOMATIONS = () => loadFeatureFlag("AUTOMATIONS");