mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-26 05:48:36 +08:00
Fix user info api calls (#10137)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
parent
42ed36e5cc
commit
881729b49c
@ -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(<UserActions onLogout={onLogoutMock} />);
|
||||
|
||||
@ -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(<UserActions onLogout={onLogoutMock} />);
|
||||
|
||||
@ -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(
|
||||
<UserActions onLogout={onLogoutMock} />,
|
||||
@ -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(
|
||||
<UserActions
|
||||
onLogout={onLogoutMock}
|
||||
@ -211,6 +246,9 @@ describe("UserActions", () => {
|
||||
|
||||
// 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(
|
||||
<UserActions
|
||||
onLogout={onLogoutMock}
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
import React from "react";
|
||||
import { UserAvatar } from "./user-avatar";
|
||||
import { AccountSettingsContextMenu } from "../context-menu/account-settings-context-menu";
|
||||
import { useIsAuthed } from "#/hooks/query/use-is-authed";
|
||||
import { useShouldShowUserFeatures } from "#/hooks/use-should-show-user-features";
|
||||
|
||||
interface UserActionsProps {
|
||||
onLogout: () => 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 (
|
||||
<div data-testid="user-actions" className="w-8 h-8 relative cursor-pointer">
|
||||
|
||||
@ -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
|
||||
|
||||
28
frontend/src/hooks/use-should-show-user-features.ts
Normal file
28
frontend/src/hooks/use-should-show-user-features.ts
Normal file
@ -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]);
|
||||
};
|
||||
Loading…
x
Reference in New Issue
Block a user