From 669d341d5112aab1ecf954e1132e380fc919dbe0 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Thu, 16 Jul 2026 09:40:43 +0800 Subject: [PATCH] fix(namespace): preserve review menu visibility Signed-off-by: dongmucat <1127093059@qq.com> --- web/src/pages/dashboard/reviews.test.ts | 11 ++++- web/src/pages/dashboard/reviews.tsx | 2 +- web/src/shared/components/user-menu.test.tsx | 41 +++++++++++++++++-- web/src/shared/components/user-menu.tsx | 7 +++- .../hooks/use-namespace-queries.test.ts | 31 +++++++++++++- web/src/shared/hooks/use-namespace-queries.ts | 3 +- 6 files changed, 86 insertions(+), 9 deletions(-) diff --git a/web/src/pages/dashboard/reviews.test.ts b/web/src/pages/dashboard/reviews.test.ts index 186c15eb..36e81f53 100644 --- a/web/src/pages/dashboard/reviews.test.ts +++ b/web/src/pages/dashboard/reviews.test.ts @@ -76,7 +76,7 @@ vi.mock('@/features/auth/use-auth', () => ({ const useMyNamespacesMock = vi.fn() vi.mock('@/shared/hooks/use-namespace-queries', () => ({ - useMyNamespaces: () => useMyNamespacesMock(), + useMyNamespaces: (enabled?: boolean) => useMyNamespacesMock(enabled), })) vi.mock('@/shared/components/dashboard-page-header', () => ({ @@ -194,4 +194,13 @@ describe('ReviewsPage', () => { expect(useReviewListMock).toHaveBeenCalled() expect(useReviewListMock.mock.calls.every((call) => call[5] === false)).toBe(true) }) + + it('skips namespace loading for super admins with global review access', () => { + hasRoleMock.mockImplementation((role: string) => role === 'SKILL_ADMIN' || role === 'USER_ADMIN' || role === 'SUPER_ADMIN') + userMock.platformRoles = ['SUPER_ADMIN'] + + renderToStaticMarkup(createElement(ReviewsPage)) + + expect(useMyNamespacesMock).toHaveBeenCalledWith(false) + }) }) diff --git a/web/src/pages/dashboard/reviews.tsx b/web/src/pages/dashboard/reviews.tsx index 94c0ded8..29fc40b2 100644 --- a/web/src/pages/dashboard/reviews.tsx +++ b/web/src/pages/dashboard/reviews.tsx @@ -40,7 +40,6 @@ export function ReviewsPage() { const navigate = useNavigate() const search = useSearch({ from: '/dashboard/reviews' }) const { hasRole, user } = useAuth() - const { data: myNamespaces, isLoading: isLoadingNamespaces } = useMyNamespaces() const [pages, setPages] = useState>({ PENDING: 0, APPROVED: 0, @@ -52,6 +51,7 @@ export function ReviewsPage() { const isSkillAdmin = hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN') const isUserAdmin = hasRole('USER_ADMIN') || hasRole('SUPER_ADMIN') const hasGlobalReviewAccess = canAccessGlobalReviewCenter(user?.platformRoles) + const { data: myNamespaces, isLoading: isLoadingNamespaces } = useMyNamespaces(!hasGlobalReviewAccess) const namespaceReviewEntry = getPreferredNamespaceReviewEntry(myNamespaces) const showTypeTabs = isSkillAdmin && isUserAdmin diff --git a/web/src/shared/components/user-menu.test.tsx b/web/src/shared/components/user-menu.test.tsx index fe70125e..e037dbc2 100644 --- a/web/src/shared/components/user-menu.test.tsx +++ b/web/src/shared/components/user-menu.test.tsx @@ -1,10 +1,11 @@ import type { ReactNode } from 'react' +import type { ManagedNamespace } from '@/api/types' import { renderToStaticMarkup } from 'react-dom/server' import { beforeEach, describe, expect, it, vi } from 'vitest' import * as mod from './user-menu' import { UserMenu } from './user-menu' -const useMyNamespacesMock = vi.hoisted(() => vi.fn(() => ({ data: [] }))) +const useMyNamespacesMock = vi.hoisted(() => vi.fn(() => ({ data: [] as ManagedNamespace[] }))) vi.mock('react', async () => { const actual = await vi.importActual('react') @@ -112,7 +113,41 @@ describe('UserMenu security settings visibility', () => { expect(html).not.toContain('user.menu.security') }) - it('does not fetch namespace memberships while rendering the global menu', () => { + it('shows reviews for namespace admins without platform review roles', () => { + useMyNamespacesMock.mockReturnValue({ + data: [ + { + id: 10, + slug: 'team-admin', + displayName: 'Team Admin', + type: 'TEAM', + status: 'ACTIVE', + immutable: false, + canFreeze: false, + canUnfreeze: false, + canArchive: false, + canRestore: false, + canDelete: false, + currentUserRole: 'ADMIN', + createdAt: '', + }, + ], + }) + + const html = renderToStaticMarkup( + , + ) + + expect(useMyNamespacesMock).toHaveBeenCalledWith(true) + expect(html).toContain('user.menu.reviews') + }) + + it('disables namespace membership loading while rendering the global menu for platform reviewers', () => { renderToStaticMarkup( { />, ) - expect(useMyNamespacesMock).not.toHaveBeenCalled() + expect(useMyNamespacesMock).toHaveBeenCalledWith(false) }) }) diff --git a/web/src/shared/components/user-menu.tsx b/web/src/shared/components/user-menu.tsx index 76229930..69d2800d 100644 --- a/web/src/shared/components/user-menu.tsx +++ b/web/src/shared/components/user-menu.tsx @@ -3,7 +3,8 @@ import { useTranslation } from 'react-i18next' import { Link } from '@tanstack/react-router' import { useQueryClient } from '@tanstack/react-query' import { authApi } from '@/api/client' -import { buildGlobalReviewsPath, canAccessGlobalReviewCenter } from '@/features/review/review-paths' +import { useMyNamespaces } from '@/shared/hooks/use-namespace-queries' +import { buildGlobalReviewsPath, canAccessGlobalReviewCenter, canAccessReviewCenter } from '@/features/review/review-paths' import { clearSessionScopedQueries } from '@/features/notification/notification-session' import { canViewGovernanceCenter } from '@/shared/lib/governance-access' import { cn } from '@/shared/lib/utils' @@ -35,7 +36,9 @@ export function UserMenu({ user, triggerClassName }: UserMenuProps) { const isUserAdmin = hasRole('USER_ADMIN') || hasRole('SUPER_ADMIN') const isAuditor = hasRole('AUDITOR') || hasRole('SUPER_ADMIN') const isSuperAdmin = hasRole('SUPER_ADMIN') - const reviewCenterVisible = canAccessGlobalReviewCenter(user.platformRoles) + const hasGlobalReviewAccess = canAccessGlobalReviewCenter(user.platformRoles) + const { data: myNamespaces } = useMyNamespaces(!hasGlobalReviewAccess) + const reviewCenterVisible = canAccessReviewCenter(user.platformRoles, myNamespaces) const canChangePassword = user.canChangePassword === true const open = isHovered || isClickOpen diff --git a/web/src/shared/hooks/use-namespace-queries.test.ts b/web/src/shared/hooks/use-namespace-queries.test.ts index 68b2823b..4c5268ec 100644 --- a/web/src/shared/hooks/use-namespace-queries.test.ts +++ b/web/src/shared/hooks/use-namespace-queries.test.ts @@ -1,4 +1,18 @@ -import { describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const useQueryMock = vi.hoisted(() => vi.fn()) + +vi.mock('@tanstack/react-query', () => ({ + useQuery: useQueryMock, + useMutation: vi.fn(), + useQueryClient: vi.fn(), +})) + +vi.mock('@/api/client', () => ({ + namespaceApi: { + listMine: vi.fn(), + }, +})) /** * use-namespace-queries.ts exports React hooks that wrap @tanstack/react-query @@ -10,6 +24,10 @@ import { describe, expect, it } from 'vitest' * Here we verify that all expected hooks are exported. */ describe('use-namespace-queries exports', () => { + beforeEach(() => { + useQueryMock.mockClear() + }) + it('exports all expected hook functions', async () => { const mod = await import('./use-namespace-queries') expect(typeof mod.useMyNamespaces).toBe('function') @@ -25,4 +43,15 @@ describe('use-namespace-queries exports', () => { expect(typeof mod.useArchiveNamespace).toBe('function') expect(typeof mod.useRestoreNamespace).toBe('function') }) + + it('passes the enabled flag to the my namespaces query', async () => { + const mod = await import('./use-namespace-queries') + + mod.useMyNamespaces(false) + + expect(useQueryMock).toHaveBeenCalledWith(expect.objectContaining({ + queryKey: ['namespaces', 'my'], + enabled: false, + })) + }) }) diff --git a/web/src/shared/hooks/use-namespace-queries.ts b/web/src/shared/hooks/use-namespace-queries.ts index be2d7230..7a31f66c 100644 --- a/web/src/shared/hooks/use-namespace-queries.ts +++ b/web/src/shared/hooks/use-namespace-queries.ts @@ -51,10 +51,11 @@ function invalidateNamespaceQueries(queryClient: ReturnType