mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-04-29 03:00:45 -04:00
feat(frontend): Remove onboarding feature flags (#13947)
Co-authored-by: Hiep Le <69354317+hieptl@users.noreply.github.com> Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: hieptl <hieptl.developer@gmail.com>
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 (
|
||||
<div className="min-h-screen flex items-center justify-center">
|
||||
<div className="min-h-screen flex items-center justify-center bg-base">
|
||||
<div
|
||||
data-testid="onboarding-form"
|
||||
className="w-[500px] max-w-[calc(100vw-2rem)] mx-auto p-4 sm:p-6 flex flex-col justify-center overflow-hidden"
|
||||
|
||||
@@ -17,7 +17,6 @@ export const HIDE_LLM_SETTINGS = () => 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");
|
||||
|
||||
Reference in New Issue
Block a user