From 881729b49c268fdec1b6d79eedc5d4d5e0807115 Mon Sep 17 00:00:00 2001 From: chuckbutkus Date: Wed, 6 Aug 2025 23:57:52 -0400 Subject: [PATCH] Fix user info api calls (#10137) Co-authored-by: openhands --- .../components/user-actions.test.tsx | 49 +++++++++++++++++-- .../features/sidebar/user-actions.tsx | 10 ++-- frontend/src/hooks/query/use-git-user.ts | 8 +-- .../hooks/use-should-show-user-features.ts | 28 +++++++++++ 4 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 frontend/src/hooks/use-should-show-user-features.ts diff --git a/frontend/__tests__/components/user-actions.test.tsx b/frontend/__tests__/components/user-actions.test.tsx index 9e9f2bdc4e..64631738ba 100644 --- a/frontend/__tests__/components/user-actions.test.tsx +++ b/frontend/__tests__/components/user-actions.test.tsx @@ -5,16 +5,32 @@ import { UserActions } from "#/components/features/sidebar/user-actions"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { ReactElement } from "react"; -// Create a mock for useIsAuthed that we can control per test +// Create mocks for all the hooks we need const useIsAuthedMock = vi .fn() .mockReturnValue({ data: true, isLoading: false }); -// Mock the useIsAuthed hook +const useConfigMock = vi + .fn() + .mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + +const useUserProvidersMock = vi + .fn() + .mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); + +// Mock the hooks vi.mock("#/hooks/query/use-is-authed", () => ({ useIsAuthed: () => useIsAuthedMock(), })); +vi.mock("#/hooks/query/use-config", () => ({ + useConfig: () => useConfigMock(), +})); + +vi.mock("#/hooks/use-user-providers", () => ({ + useUserProviders: () => useUserProvidersMock(), +})); + describe("UserActions", () => { const user = userEvent.setup(); const onClickAccountSettingsMock = vi.fn(); @@ -40,8 +56,10 @@ describe("UserActions", () => { }; beforeEach(() => { - // Reset the mock to default value before each test + // Reset all mocks to default values before each test useIsAuthedMock.mockReturnValue({ data: true, isLoading: false }); + useConfigMock.mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + useUserProvidersMock.mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); }); afterEach(() => { @@ -102,6 +120,9 @@ describe("UserActions", () => { it("should NOT show context menu when user is not authenticated and avatar is clicked", async () => { // Set isAuthed to false for this test useIsAuthedMock.mockReturnValue({ data: false, isLoading: false }); + // Keep other mocks with default values + useConfigMock.mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + useUserProvidersMock.mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); renderWithQueryClient(); @@ -131,6 +152,9 @@ describe("UserActions", () => { it("should NOT be able to access logout when user is not authenticated", async () => { // Set isAuthed to false for this test useIsAuthedMock.mockReturnValue({ data: false, isLoading: false }); + // Keep other mocks with default values + useConfigMock.mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + useUserProvidersMock.mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); renderWithQueryClient(); @@ -149,6 +173,9 @@ describe("UserActions", () => { it("should handle user prop changing from undefined to defined", async () => { // Start with no authentication useIsAuthedMock.mockReturnValue({ data: false, isLoading: false }); + // Keep other mocks with default values + useConfigMock.mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + useUserProvidersMock.mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); const { rerender } = renderWithQueryClient( , @@ -163,6 +190,9 @@ describe("UserActions", () => { // Set authentication to true for the rerender useIsAuthedMock.mockReturnValue({ data: true, isLoading: false }); + // Ensure config and providers are set correctly + useConfigMock.mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + useUserProvidersMock.mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); // Add user prop and create a new QueryClient to ensure fresh state const queryClient = new QueryClient({ @@ -195,6 +225,11 @@ describe("UserActions", () => { }); it("should handle user prop changing from defined to undefined", async () => { + // Start with authentication and providers + useIsAuthedMock.mockReturnValue({ data: true, isLoading: false }); + useConfigMock.mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + useUserProvidersMock.mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); + const { rerender } = renderWithQueryClient( { // Set authentication to false for the rerender useIsAuthedMock.mockReturnValue({ data: false, isLoading: false }); + // Keep other mocks with default values + useConfigMock.mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + useUserProvidersMock.mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); // Remove user prop - menu should disappear because user is no longer authenticated rerender( @@ -229,6 +267,11 @@ describe("UserActions", () => { }); it("should work with loading state and user provided", async () => { + // Ensure authentication and providers are set correctly + useIsAuthedMock.mockReturnValue({ data: true, isLoading: false }); + useConfigMock.mockReturnValue({ data: { APP_MODE: "saas" }, isLoading: false }); + useUserProvidersMock.mockReturnValue({ providers: [{ id: "github", name: "GitHub" }] }); + renderWithQueryClient( void; @@ -10,10 +10,12 @@ interface UserActionsProps { } export function UserActions({ onLogout, user, isLoading }: UserActionsProps) { - const { data: isAuthed } = useIsAuthed(); const [accountContextMenuIsVisible, setAccountContextMenuIsVisible] = React.useState(false); + // Use the shared hook to determine if user actions should be shown + const shouldShowUserActions = useShouldShowUserFeatures(); + const toggleAccountMenu = () => { // Always toggle the menu, even if user is undefined setAccountContextMenuIsVisible((prev) => !prev); @@ -28,8 +30,8 @@ export function UserActions({ onLogout, user, isLoading }: UserActionsProps) { closeAccountMenu(); }; - // Always show the menu for authenticated users, even without user data - const showMenu = accountContextMenuIsVisible && isAuthed; + // Show the menu based on the new logic + const showMenu = accountContextMenuIsVisible && shouldShowUserActions; return (
diff --git a/frontend/src/hooks/query/use-git-user.ts b/frontend/src/hooks/query/use-git-user.ts index dcb5750a6b..b4de5b4bf0 100644 --- a/frontend/src/hooks/query/use-git-user.ts +++ b/frontend/src/hooks/query/use-git-user.ts @@ -3,16 +3,18 @@ import React from "react"; import posthog from "posthog-js"; import { useConfig } from "./use-config"; import OpenHands from "#/api/open-hands"; -import { useIsAuthed } from "#/hooks/query/use-is-authed"; +import { useShouldShowUserFeatures } from "#/hooks/use-should-show-user-features"; export const useGitUser = () => { const { data: config } = useConfig(); - const { data: isAuthed } = useIsAuthed(); + + // Use the shared hook to determine if we should fetch user data + const shouldFetchUser = useShouldShowUserFeatures(); const user = useQuery({ queryKey: ["user"], queryFn: OpenHands.getGitUser, - enabled: !!config?.APP_MODE && isAuthed, // Enable regardless of providers + enabled: shouldFetchUser, retry: false, staleTime: 1000 * 60 * 5, // 5 minutes gcTime: 1000 * 60 * 15, // 15 minutes diff --git a/frontend/src/hooks/use-should-show-user-features.ts b/frontend/src/hooks/use-should-show-user-features.ts new file mode 100644 index 0000000000..3f6af98da4 --- /dev/null +++ b/frontend/src/hooks/use-should-show-user-features.ts @@ -0,0 +1,28 @@ +import React from "react"; +import { useConfig } from "./query/use-config"; +import { useIsAuthed } from "./query/use-is-authed"; +import { useUserProviders } from "./use-user-providers"; + +/** + * Hook to determine if user-related features should be shown or enabled + * based on authentication status and provider configuration. + * + * @returns boolean indicating if user features should be shown + */ +export const useShouldShowUserFeatures = (): boolean => { + const { data: config } = useConfig(); + const { data: isAuthed } = useIsAuthed(); + const { providers } = useUserProviders(); + + return React.useMemo(() => { + if (!config?.APP_MODE || !isAuthed) return false; + + // In OSS mode, only show user features if Git providers are configured + if (config.APP_MODE === "oss") { + return providers.length > 0; + } + + // In non-OSS modes (saas), always show user features when authenticated + return true; + }, [config?.APP_MODE, isAuthed, providers.length]); +};