fix(review): restore namespace admin review access

This commit is contained in:
dongmucat 2026-04-13 17:00:07 +08:00
parent 3d1d70ac02
commit 38757084ba
19 changed files with 700 additions and 41 deletions

View file

@ -103,6 +103,10 @@ public class SecurityAuditController extends BaseApiController {
return true;
}
Map<Long, NamespaceRole> namespaceRoles = userNsRoles != null ? userNsRoles : Map.of();
NamespaceRole namespaceRole = namespaceRoles.get(skill.getNamespaceId());
if (namespaceRole == NamespaceRole.ADMIN || namespaceRole == NamespaceRole.OWNER) {
return true;
}
return visibilityChecker.canAccess(skill, principal.userId(), namespaceRoles);
}

View file

@ -1,6 +1,8 @@
package com.iflytek.skillhub.controller.portal;
import com.iflytek.skillhub.auth.rbac.PlatformPrincipal;
import com.iflytek.skillhub.domain.namespace.NamespaceMember;
import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository;
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
import com.iflytek.skillhub.domain.security.ScannerType;
import com.iflytek.skillhub.domain.security.SecurityAudit;
@ -56,6 +58,9 @@ class SecurityAuditControllerTest {
@MockBean
private ScanTaskProducer scanTaskProducer;
@MockBean
private NamespaceMemberRepository namespaceMemberRepository;
@Test
void getSecurityAudit_returnsAuditPayload() throws Exception {
SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER);
@ -129,6 +134,29 @@ class SecurityAuditControllerTest {
.andExpect(jsonPath("$.code").value(403));
}
@Test
void getSecurityAudit_allowsNamespaceAdminForPendingUnpublishedSkill() throws Exception {
SecurityAudit audit = new SecurityAudit(42L, ScannerType.SKILL_SCANNER);
setField(audit, "id", 9L);
audit.setScanId("scan-team-admin");
audit.setVerdict(SecurityVerdict.SAFE);
audit.setIsSafe(true);
audit.setMaxSeverity("LOW");
audit.setFindingsCount(0);
given(skillVersionRepository.findById(42L)).willReturn(java.util.Optional.of(skillVersion(42L, 8L)));
given(skillRepository.findById(8L)).willReturn(java.util.Optional.of(skill(8L, "owner-1")));
given(securityAuditRepository.findLatestActiveByVersionId(42L)).willReturn(List.of(audit));
given(namespaceMemberRepository.findByUserId("team-admin"))
.willReturn(List.of(new NamespaceMember(5L, "team-admin", NamespaceRole.ADMIN)));
mockMvc.perform(get("/api/v1/skills/8/versions/42/security-audit")
.with(auth("team-admin")))
.andExpect(status().isOk())
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.data[0].id").value(9L))
.andExpect(jsonPath("$.data[0].scanId").value("scan-team-admin"));
}
private RequestPostProcessor auth(String userId) {
PlatformPrincipal principal = new PlatformPrincipal(
userId,

View file

@ -7,6 +7,7 @@ import com.iflytek.skillhub.auth.rbac.PlatformPrincipal;
import com.iflytek.skillhub.auth.rbac.RbacService;
import com.iflytek.skillhub.domain.namespace.Namespace;
import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository;
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
import com.iflytek.skillhub.domain.namespace.NamespaceRepository;
import com.iflytek.skillhub.domain.namespace.NamespaceType;
import com.iflytek.skillhub.domain.review.ReviewTask;
@ -128,6 +129,37 @@ class SkillApprovalVisibilityFlowIntegrationTest {
assertThat(indexedDocument.getTitle()).isEqualTo(graph.skill().getDisplayName());
}
@Test
void namespaceAdminCanApproveOwnTeamReview() throws Exception {
PendingSkillGraph graph = createPendingTeamSkill("team-admin");
when(namespaceMemberRepository.findByUserId("team-admin"))
.thenReturn(List.of(new com.iflytek.skillhub.domain.namespace.NamespaceMember(
graph.namespace().getId(),
"team-admin",
NamespaceRole.ADMIN
)));
when(rbacService.getUserRoleCodes("team-admin")).thenReturn(Set.of());
mockMvc.perform(post("/api/v1/reviews/" + graph.reviewTask().getId() + "/approve")
.contentType("application/json")
.content("{\"comment\":\"approved by namespace admin\"}")
.with(authentication(apiAuth("team-admin")))
.with(csrf()))
.andExpect(status().isOk())
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.data.id").value(graph.reviewTask().getId()))
.andExpect(jsonPath("$.data.status").value("APPROVED"))
.andExpect(jsonPath("$.data.reviewedBy").value("team-admin"))
.andExpect(jsonPath("$.data.reviewComment").value("approved by namespace admin"));
Skill savedSkill = skillRepository.findById(graph.skill().getId()).orElseThrow();
SkillVersion savedVersion = skillVersionRepository.findById(graph.version().getId()).orElseThrow();
assertThat(savedSkill.getLatestVersionId()).isEqualTo(graph.version().getId());
assertThat(savedVersion.getStatus()).isEqualTo(SkillVersionStatus.PUBLISHED);
assertThat(savedVersion.getPublishedAt()).isNotNull();
}
private PendingSkillGraph createPendingGlobalSkill(String ownerId) {
String suffix = UUID.randomUUID().toString().substring(0, 8);
@ -154,6 +186,31 @@ class SkillApprovalVisibilityFlowIntegrationTest {
return new PendingSkillGraph(namespace, skill, version, reviewTask);
}
private PendingSkillGraph createPendingTeamSkill(String ownerId) {
String suffix = UUID.randomUUID().toString().substring(0, 8);
Namespace namespace = new Namespace("team-approval-" + suffix, "Team Approval " + suffix, ownerId);
namespace = namespaceRepository.save(namespace);
Skill skill = new Skill(namespace.getId(), "approval-skill-" + suffix, ownerId, SkillVisibility.PUBLIC);
skill.setDisplayName("Approval Skill " + suffix);
skill.setSummary("Team namespace self-review should be allowed for namespace admins.");
skill.setCreatedBy(ownerId);
skill.setUpdatedBy(ownerId);
skill = skillRepository.save(skill);
skillRepository.flush();
SkillVersion version = new SkillVersion(skill.getId(), "1.0.0", ownerId);
version.setStatus(SkillVersionStatus.PENDING_REVIEW);
version.setRequestedVisibility(SkillVisibility.PUBLIC);
version = skillVersionRepository.save(version);
skillVersionRepository.flush();
ReviewTask reviewTask = reviewTaskJpaRepository.saveAndFlush(new ReviewTask(version.getId(), namespace.getId(), ownerId));
return new PendingSkillGraph(namespace, skill, version, reviewTask);
}
private SkillSearchDocumentEntity awaitIndexedDocument(Long skillId) throws InterruptedException {
Instant deadline = Instant.now().plus(Duration.ofSeconds(5));
Optional<SkillSearchDocumentEntity> indexed = skillSearchDocumentJpaRepository.findBySkillId(skillId);

View file

@ -28,12 +28,8 @@ public class ReviewPermissionChecker {
Map<Long, NamespaceRole> userNamespaceRoles,
Set<String> platformRoles) {
if (task.getSubmittedBy().equals(userId)) {
if (platformRoles.contains("SUPER_ADMIN")) {
return true;
}
NamespaceRole role = userNamespaceRoles.get(task.getNamespaceId());
return hasPlatformReviewRole(platformRoles)
&& (role == NamespaceRole.ADMIN || role == NamespaceRole.OWNER);
return platformRoles.contains("SUPER_ADMIN")
|| canSelfReviewNamespace(task.getNamespaceId(), namespaceType, userNamespaceRoles);
}
return canReviewNamespace(task.getNamespaceId(), namespaceType, userNamespaceRoles, platformRoles);
}
@ -140,4 +136,15 @@ public class ReviewPermissionChecker {
return platformRoles.contains("SKILL_ADMIN")
|| platformRoles.contains("SUPER_ADMIN");
}
private boolean canSelfReviewNamespace(Long namespaceId,
NamespaceType namespaceType,
Map<Long, NamespaceRole> userNamespaceRoles) {
if (namespaceType == NamespaceType.GLOBAL) {
return false;
}
NamespaceRole role = userNamespaceRoles.get(namespaceId);
return role == NamespaceRole.OWNER || role == NamespaceRole.ADMIN;
}
}

View file

@ -81,6 +81,24 @@ class ReviewPermissionCheckerTest {
NamespaceType.GLOBAL, Map.of(), Set.of("SUPER_ADMIN")));
}
@Test
void teamAdminCanReviewOwnTeamSubmission() {
String userId = "user-1";
ReviewTask task = new ReviewTask(1L, 10L, userId);
assertTrue(checker.canReview(task, userId,
NamespaceType.TEAM,
Map.of(10L, NamespaceRole.ADMIN), Set.of()));
}
@Test
void teamOwnerCanReviewOwnTeamSubmission() {
String userId = "user-1";
ReviewTask task = new ReviewTask(1L, 10L, userId);
assertTrue(checker.canReview(task, userId,
NamespaceType.TEAM,
Map.of(10L, NamespaceRole.OWNER), Set.of()));
}
@Test
void teamAdminCanReviewTeamSkill() {
ReviewTask task = new ReviewTask(1L, 10L, "user-2");

View file

@ -483,6 +483,46 @@ class ReviewServiceTest {
assertEquals(USER_ID, skill.getUpdatedBy());
}
@Test
void namespaceAdminCanApproveOwnSubmission() {
ReviewTask task = createPendingReviewTask();
Namespace ns = createTeamNamespace();
SkillVersion sv = createPendingReviewSkillVersion();
Skill skill = createSkill();
when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task));
when(namespaceRepository.findById(NAMESPACE_ID)).thenReturn(Optional.of(ns));
when(permissionChecker.canReview(
eq(task),
eq(USER_ID),
eq(ns.getType()),
eq(Map.of(NAMESPACE_ID, NamespaceRole.ADMIN)),
eq(Set.of())))
.thenReturn(true);
when(reviewTaskRepository.updateStatusWithVersion(
REVIEW_TASK_ID,
ReviewTaskStatus.APPROVED,
USER_ID,
"self approved as namespace admin",
task.getVersion()))
.thenReturn(1);
when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv));
when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill));
when(skillRepository.findByNamespaceIdAndSlug(NAMESPACE_ID, "my-skill")).thenReturn(List.of(skill));
when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task));
ReviewTask result = reviewService.approveReview(
REVIEW_TASK_ID,
USER_ID,
"self approved as namespace admin",
Map.of(NAMESPACE_ID, NamespaceRole.ADMIN),
Set.of());
assertNotNull(result);
assertEquals(SkillVersionStatus.PUBLISHED, sv.getStatus());
assertEquals(USER_ID, skill.getUpdatedBy());
}
@Test
void shouldThrowOnConcurrentModification() {
ReviewTask task = createPendingReviewTask();
@ -602,6 +642,43 @@ class ReviewServiceTest {
assertEquals(SkillVersionStatus.REJECTED, sv.getStatus());
}
@Test
void namespaceAdminCanRejectOwnSubmission() {
ReviewTask task = createPendingReviewTask();
Namespace ns = createTeamNamespace();
SkillVersion sv = createPendingReviewSkillVersion();
when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task));
when(namespaceRepository.findById(NAMESPACE_ID)).thenReturn(Optional.of(ns));
when(permissionChecker.canReview(
eq(task),
eq(USER_ID),
eq(ns.getType()),
eq(Map.of(NAMESPACE_ID, NamespaceRole.ADMIN)),
eq(Set.of())))
.thenReturn(true);
when(reviewTaskRepository.updateStatusWithVersion(
REVIEW_TASK_ID,
ReviewTaskStatus.REJECTED,
USER_ID,
"self rejected as namespace admin",
task.getVersion()))
.thenReturn(1);
when(skillVersionRepository.findById(SKILL_VERSION_ID)).thenReturn(Optional.of(sv));
when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(createSkill()));
when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task));
ReviewTask result = reviewService.rejectReview(
REVIEW_TASK_ID,
USER_ID,
"self rejected as namespace admin",
Map.of(NAMESPACE_ID, NamespaceRole.ADMIN),
Set.of());
assertNotNull(result);
assertEquals(SkillVersionStatus.REJECTED, sv.getStatus());
}
@Test
void shouldThrowOnConcurrentModification() {
ReviewTask task = createPendingReviewTask();

View file

@ -0,0 +1,73 @@
import { expect, test } from '@playwright/test'
import { setEnglishLocale } from './helpers/auth-fixtures'
import { registerSession } from './helpers/session'
import { E2eTestDataBuilder } from './helpers/test-data-builder'
test.describe('Namespace Review Detail Access (Real API)', () => {
test.beforeEach(async ({ page }, testInfo) => {
await setEnglishLocale(page)
await registerSession(page, testInfo)
})
test('opens namespace review detail from the namespace review list', async ({ page }, testInfo) => {
const builder = new E2eTestDataBuilder(page, testInfo)
await builder.init()
try {
const seeded = await builder.createReviewData()
await page.goto(`/dashboard/namespaces/${seeded.namespace.slug}/reviews`)
await expect(page.getByRole('heading', { name: 'Namespace Reviews' })).toBeVisible()
await expect(page.getByText(`${seeded.namespace.slug}/${seeded.skill.slug}`)).toBeVisible()
await page.getByRole('link', { name: 'Open review' }).first().click()
await expect(page).toHaveURL(new RegExp(`/dashboard/namespaces/${seeded.namespace.slug}/reviews/\\d+$`))
await expect(page.getByRole('heading', { name: 'Review Detail' })).toBeVisible()
await expect(page.getByText(`${seeded.namespace.slug}/${seeded.skill.slug}`).first()).toBeVisible()
} finally {
await builder.cleanup()
}
})
test('redirects /dashboard/reviews to a namespace review page for namespace operators', async ({ page }, testInfo) => {
const builder = new E2eTestDataBuilder(page, testInfo)
await builder.init()
try {
await builder.createReviewData()
await page.goto('/dashboard/reviews')
await expect(page).toHaveURL(/\/dashboard\/namespaces\/.+\/reviews$/)
await expect(page.getByRole('heading', { name: 'Namespace Reviews' })).toBeVisible()
} finally {
await builder.cleanup()
}
})
test('redirects namespace review detail opened through the global detail route', async ({ page }, testInfo) => {
const builder = new E2eTestDataBuilder(page, testInfo)
await builder.init()
try {
const seeded = await builder.createReviewData()
await page.goto(`/dashboard/namespaces/${seeded.namespace.slug}/reviews`)
const reviewLink = page.getByRole('link', { name: 'Open review' }).first()
const reviewPath = await reviewLink.getAttribute('href')
const reviewId = reviewPath?.match(/\/reviews\/(\d+)$/)?.[1]
if (!reviewId) {
throw new Error(`Failed to resolve review id from href: ${reviewPath}`)
}
await page.goto(`/dashboard/reviews/${reviewId}`)
await expect(page).toHaveURL(new RegExp(`/dashboard/namespaces/${seeded.namespace.slug}/reviews/${reviewId}$`))
await expect(page.getByRole('heading', { name: 'Review Detail' })).toBeVisible()
} finally {
await builder.cleanup()
}
})
})

View file

@ -85,22 +85,18 @@ const NamespaceReviewsPage = createLazyRouteComponent(
() => import('@/pages/dashboard/namespace-reviews'),
'NamespaceReviewsPage',
)
const GovernancePage = createLazyRouteComponent(() => import('@/pages/dashboard/governance'), 'GovernancePage')
const ReviewsPage = createRoleProtectedRouteComponent(
() => import('@/pages/dashboard/reviews'),
'ReviewsPage',
['SKILL_ADMIN', 'NAMESPACE_ADMIN', 'USER_ADMIN', 'SUPER_ADMIN'],
const NamespaceReviewDetailPage = createLazyRouteComponent(
() => import('@/pages/dashboard/review-detail'),
'NamespaceReviewDetailPage',
)
const GovernancePage = createLazyRouteComponent(() => import('@/pages/dashboard/governance'), 'GovernancePage')
const ReviewsPage = createLazyRouteComponent(() => import('@/pages/dashboard/reviews'), 'ReviewsPage')
const ReportsPage = createRoleProtectedRouteComponent(
() => import('@/pages/dashboard/reports'),
'ReportsPage',
['SKILL_ADMIN', 'SUPER_ADMIN'],
)
const ReviewDetailPage = createRoleProtectedRouteComponent(
() => import('@/pages/dashboard/review-detail'),
'ReviewDetailPage',
['SKILL_ADMIN', 'NAMESPACE_ADMIN', 'SUPER_ADMIN'],
)
const ReviewDetailPage = createLazyRouteComponent(() => import('@/pages/dashboard/review-detail'), 'ReviewDetailPage')
const PromotionsPage = createRoleProtectedRouteComponent(
() => import('@/pages/dashboard/promotions'),
'PromotionsPage',
@ -298,6 +294,13 @@ const dashboardReviewDetailRoute = createRoute({
component: ReviewDetailPage,
})
const dashboardNamespaceReviewDetailRoute = createRoute({
getParentRoute: () => rootRoute,
path: 'dashboard/namespaces/$slug/reviews/$id',
beforeLoad: requireAuth,
component: NamespaceReviewDetailPage,
})
const dashboardPromotionsRoute = createRoute({
getParentRoute: () => rootRoute,
path: 'dashboard/promotions',
@ -408,6 +411,7 @@ const routeTree = rootRoute.addChildren([
dashboardNamespacesRoute,
dashboardNamespaceMembersRoute,
dashboardNamespaceReviewsRoute,
dashboardNamespaceReviewDetailRoute,
dashboardGovernanceRoute,
dashboardReviewsRoute,
dashboardReportsRoute,

View file

@ -0,0 +1,107 @@
import { describe, expect, it } from 'vitest'
import {
buildGlobalReviewsPath,
buildNamespaceReviewDetailPath,
buildNamespaceReviewsPath,
canAccessGlobalReviewCenter,
canAccessReviewCenter,
canManageNamespaceReviews,
getPreferredNamespaceReviewEntry,
} from './review-paths'
describe('review-paths', () => {
it('builds the global reviews path', () => {
expect(buildGlobalReviewsPath()).toBe('/dashboard/reviews')
})
it('builds namespace review paths', () => {
expect(buildNamespaceReviewsPath('team alpha')).toBe('/dashboard/namespaces/team%20alpha/reviews')
expect(buildNamespaceReviewDetailPath('team alpha', 12)).toBe('/dashboard/namespaces/team%20alpha/reviews/12')
})
it('detects global review access from platform roles', () => {
expect(canAccessGlobalReviewCenter(['SKILL_ADMIN'])).toBe(true)
expect(canAccessGlobalReviewCenter(['USER_ADMIN'])).toBe(true)
expect(canAccessGlobalReviewCenter(['SUPER_ADMIN'])).toBe(true)
expect(canAccessGlobalReviewCenter(['USER'])).toBe(false)
})
it('recognizes namespace review managers', () => {
expect(canManageNamespaceReviews('OWNER')).toBe(true)
expect(canManageNamespaceReviews('ADMIN')).toBe(true)
expect(canManageNamespaceReviews('MEMBER')).toBe(false)
})
it('prefers active team namespaces for namespace review entry', () => {
expect(getPreferredNamespaceReviewEntry([
{
id: 1,
slug: 'archived-team',
displayName: 'Archived Team',
type: 'TEAM',
status: 'ARCHIVED',
immutable: false,
canFreeze: false,
canUnfreeze: false,
canArchive: false,
canRestore: false,
currentUserRole: 'ADMIN',
createdAt: '',
},
{
id: 2,
slug: 'active-team',
displayName: 'Active Team',
type: 'TEAM',
status: 'ACTIVE',
immutable: false,
canFreeze: false,
canUnfreeze: false,
canArchive: false,
canRestore: false,
currentUserRole: 'OWNER',
createdAt: '',
},
])?.slug).toBe('active-team')
})
it('returns null when no manageable namespace exists', () => {
expect(getPreferredNamespaceReviewEntry([
{
id: 1,
slug: 'member-team',
displayName: 'Member Team',
type: 'TEAM',
status: 'ACTIVE',
immutable: false,
canFreeze: false,
canUnfreeze: false,
canArchive: false,
canRestore: false,
currentUserRole: 'MEMBER',
createdAt: '',
},
])).toBeNull()
})
it('detects review center access from either platform roles or namespace roles', () => {
expect(canAccessReviewCenter(['SKILL_ADMIN'], [])).toBe(true)
expect(canAccessReviewCenter([], [
{
id: 2,
slug: 'team-admin',
displayName: 'Team Admin',
type: 'TEAM',
status: 'ACTIVE',
immutable: false,
canFreeze: false,
canUnfreeze: false,
canArchive: false,
canRestore: false,
currentUserRole: 'ADMIN',
createdAt: '',
},
])).toBe(true)
expect(canAccessReviewCenter([], [])).toBe(false)
})
})

View file

@ -0,0 +1,48 @@
import type { ManagedNamespace, NamespaceRole } from '@/api/types'
const GLOBAL_REVIEW_PLATFORM_ROLES = ['SKILL_ADMIN', 'USER_ADMIN', 'SUPER_ADMIN'] as const
export function buildGlobalReviewsPath() {
return '/dashboard/reviews'
}
export function buildNamespaceReviewsPath(slug: string) {
return `/dashboard/namespaces/${encodeURIComponent(slug)}/reviews`
}
export function buildNamespaceReviewDetailPath(slug: string, reviewId: number) {
return `/dashboard/namespaces/${encodeURIComponent(slug)}/reviews/${reviewId}`
}
export function canAccessGlobalReviewCenter(platformRoles?: readonly string[]) {
return GLOBAL_REVIEW_PLATFORM_ROLES.some((role) => platformRoles?.includes(role))
}
export function canManageNamespaceReviews(role?: NamespaceRole) {
return role === 'OWNER' || role === 'ADMIN'
}
export function getPreferredNamespaceReviewEntry(
namespaces?: readonly ManagedNamespace[],
) {
if (!namespaces?.length) {
return null
}
const manageableNamespaces = namespaces.filter((namespace) =>
namespace.type === 'TEAM' && canManageNamespaceReviews(namespace.currentUserRole),
)
if (manageableNamespaces.length === 0) {
return null
}
const activeNamespace = manageableNamespaces.find((namespace) => namespace.status === 'ACTIVE')
return activeNamespace ?? manageableNamespaces[0]
}
export function canAccessReviewCenter(
platformRoles?: readonly string[],
namespaces?: readonly ManagedNamespace[],
) {
return canAccessGlobalReviewCenter(platformRoles) || getPreferredNamespaceReviewEntry(namespaces) !== null
}

View file

@ -1035,6 +1035,7 @@
"sortLabel": "Time Order",
"sortNewest": "Newest first",
"sortOldest": "Oldest first",
"openReview": "Open review",
"pageSummary": "Total {{total}} records, page {{page}}",
"prevPage": "Previous",
"nextPage": "Next",

View file

@ -1035,6 +1035,7 @@
"sortLabel": "时间排序",
"sortNewest": "最新优先",
"sortOldest": "最早优先",
"openReview": "进入审核详情",
"pageSummary": "共 {{total}} 条记录,第 {{page}} 页",
"prevPage": "上一页",
"nextPage": "下一页",

View file

@ -3,6 +3,7 @@ import { renderToStaticMarkup } from 'react-dom/server'
import { createElement } from 'react'
vi.mock('@tanstack/react-router', () => ({
Link: ({ children, to }: { children: unknown; to: string }) => createElement('a', { href: to }, children as string),
useParams: () => ({ slug: 'test-ns' }),
}))
@ -127,6 +128,8 @@ describe('NamespaceReviewsPage', () => {
const html = renderToStaticMarkup(createElement(NamespaceReviewsPage))
expect(html).toContain('nsReviews.pageSummary')
expect(html).toContain('/dashboard/namespaces/test-ns/reviews/1')
expect(html).toContain('nsReviews.openReview')
expect(paginationProps).toHaveLength(1)
expect(paginationProps[0]?.page).toBe(0)
expect(paginationProps[0]?.totalPages).toBe(2)
@ -154,4 +157,18 @@ describe('NamespaceReviewsPage', () => {
expect(paginationProps).toHaveLength(0)
})
it('does not enable review queries before namespace detail resolves', () => {
useNamespaceDetailMock.mockReturnValue({
data: undefined,
isLoading: true,
})
renderToStaticMarkup(createElement(NamespaceReviewsPage))
expect(useReviewListMock).toHaveBeenCalled()
for (const call of useReviewListMock.mock.calls) {
expect(call[5]).toBe(false)
}
})
})

View file

@ -1,6 +1,7 @@
import { useState } from 'react'
import { useParams } from '@tanstack/react-router'
import { Link, useParams } from '@tanstack/react-router'
import { useTranslation } from 'react-i18next'
import { buildNamespaceReviewDetailPath } from '@/features/review/review-paths'
import { formatLocalDateTime } from '@/shared/lib/date-time'
import { Card } from '@/shared/ui/card'
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/shared/ui/select'
@ -15,8 +16,9 @@ type ReviewStatus = 'PENDING' | 'APPROVED' | 'REJECTED'
type TimeSortDirection = 'ASC' | 'DESC'
const PAGE_SIZE = 10
function ReviewListSection({ namespaceId }: { namespaceId?: number }) {
function ReviewListSection({ namespaceId, slug }: { namespaceId?: number; slug: string }) {
const { t, i18n } = useTranslation()
const reviewsEnabled = typeof namespaceId === 'number' && namespaceId > 0
const [pages, setPages] = useState<Record<ReviewStatus, number>>({
PENDING: 0,
APPROVED: 0,
@ -24,9 +26,9 @@ function ReviewListSection({ namespaceId }: { namespaceId?: number }) {
})
const [activeStatus, setActiveStatus] = useState<ReviewStatus>('PENDING')
const [sortDirection, setSortDirection] = useState<TimeSortDirection>('DESC')
const pending = useReviewList('PENDING', namespaceId, pages.PENDING, PAGE_SIZE, sortDirection, activeStatus === 'PENDING')
const approved = useReviewList('APPROVED', namespaceId, pages.APPROVED, PAGE_SIZE, sortDirection, activeStatus === 'APPROVED')
const rejected = useReviewList('REJECTED', namespaceId, pages.REJECTED, PAGE_SIZE, sortDirection, activeStatus === 'REJECTED')
const pending = useReviewList('PENDING', namespaceId, pages.PENDING, PAGE_SIZE, sortDirection, reviewsEnabled && activeStatus === 'PENDING')
const approved = useReviewList('APPROVED', namespaceId, pages.APPROVED, PAGE_SIZE, sortDirection, reviewsEnabled && activeStatus === 'APPROVED')
const rejected = useReviewList('REJECTED', namespaceId, pages.REJECTED, PAGE_SIZE, sortDirection, reviewsEnabled && activeStatus === 'REJECTED')
const changePage = (status: ReviewStatus, nextPage: number) => {
setPages((current) => ({ ...current, [status]: nextPage }))
@ -90,6 +92,14 @@ function ReviewListSection({ namespaceId }: { namespaceId?: number }) {
{review.reviewComment ? (
<p className="mt-3 text-sm text-muted-foreground">{review.reviewComment}</p>
) : null}
<div className="mt-4 flex justify-end">
<Link
to={buildNamespaceReviewDetailPath(slug, review.id)}
className="inline-flex items-center rounded-md border border-border/60 px-3 py-2 text-sm font-medium text-foreground transition-colors hover:bg-accent hover:text-accent-foreground"
>
{t('nsReviews.openReview')}
</Link>
</div>
</div>
))}
{query.data ? renderPagination(status, query.data.totalElements, query.data.totalPages) : null}
@ -151,7 +161,7 @@ export function NamespaceReviewsPage() {
{readOnlyMessage}
</Card>
) : null}
<ReviewListSection namespaceId={namespace?.id} />
<ReviewListSection namespaceId={namespace?.id} slug={slug} />
</div>
)
}

View file

@ -5,7 +5,11 @@ const navigateMock = vi.fn()
vi.mock('@tanstack/react-router', () => ({
useNavigate: () => navigateMock,
useParams: () => ({ id: '13' }),
useParams: (options?: { from?: string }) => (
options?.from === '/dashboard/namespaces/$slug/reviews/$id'
? { id: '13', slug: 'team-alpha' }
: { id: '13' }
),
}))
vi.mock('react-i18next', async () => {
@ -112,6 +116,11 @@ vi.mock('@/features/review/use-review-detail', () => ({
}),
}))
const userMock = { platformRoles: ['SKILL_ADMIN'] as string[] }
vi.mock('@/features/auth/use-auth', () => ({
useAuth: () => ({ user: userMock }),
}))
// Mock hooks used directly by the review-detail page for file browser sidebar
vi.mock('@/features/review/use-review-file', () => ({
useReviewFile: () => ({ data: null, isLoading: false, error: null }),
@ -122,11 +131,12 @@ vi.mock('@/api/client', () => ({
WEB_API_PREFIX: '/api/web',
}))
import { ReviewDetailPage } from './review-detail'
import { NamespaceReviewDetailPage, ReviewDetailPage } from './review-detail'
describe('ReviewDetailPage', () => {
beforeEach(() => {
navigateMock.mockReset()
userMock.platformRoles = ['SKILL_ADMIN']
useReviewDetailMock.mockReset()
useReviewSkillDetailMock.mockReset()
useReviewDetailMock.mockReturnValue({
@ -206,6 +216,81 @@ describe('ReviewDetailPage', () => {
expect(html).toContain('review.notFound')
})
it('renders namespace review detail through the namespace route wrapper', () => {
useReviewDetailMock.mockReturnValue({
data: {
id: 13,
namespace: 'team-alpha',
skillSlug: 'demo-skill',
version: '1.2.0',
status: 'PENDING',
submittedBy: 'local-admin',
submittedByName: 'Local Admin',
submittedAt: '2026-03-19T00:00:00Z',
reviewedBy: null,
reviewedByName: null,
reviewedAt: null,
reviewComment: null,
},
isLoading: false,
})
const html = renderToStaticMarkup(<NamespaceReviewDetailPage />)
expect(html).toContain('review.detail')
expect(html).toContain('demo-skill')
})
it('redirects namespace reviews opened through the global route for namespace operators', () => {
userMock.platformRoles = []
useReviewDetailMock.mockReturnValue({
data: {
id: 13,
namespace: 'team-alpha',
skillSlug: 'demo-skill',
version: '1.2.0',
status: 'PENDING',
submittedBy: 'local-admin',
submittedByName: 'Local Admin',
submittedAt: '2026-03-19T00:00:00Z',
reviewedBy: null,
reviewedByName: null,
reviewedAt: null,
reviewComment: null,
},
isLoading: false,
})
const html = renderToStaticMarkup(<ReviewDetailPage />)
expect(html).toBe('')
})
it('shows not-found state when the namespace route slug does not match the review namespace', () => {
useReviewDetailMock.mockReturnValue({
data: {
id: 13,
namespace: 'other-team',
skillSlug: 'demo-skill',
version: '1.2.0',
status: 'PENDING',
submittedBy: 'local-admin',
submittedByName: 'Local Admin',
submittedAt: '2026-03-19T00:00:00Z',
reviewedBy: null,
reviewedByName: null,
reviewedAt: null,
reviewComment: null,
},
isLoading: false,
})
const html = renderToStaticMarkup(<NamespaceReviewDetailPage />)
expect(html).toContain('review.notFound')
expect(html).toContain('review.backToList')
})
it('disables approval and shows a scanning hint while the active review version is scanning', () => {
useReviewSkillDetailMock.mockReturnValue({
data: {

View file

@ -1,7 +1,14 @@
import { useState } from 'react'
import { useEffect, useState } from 'react'
import { useNavigate, useParams } from '@tanstack/react-router'
import { useTranslation } from 'react-i18next'
import { ChevronDown, Folder } from 'lucide-react'
import { useAuth } from '@/features/auth/use-auth'
import {
buildGlobalReviewsPath,
buildNamespaceReviewDetailPath,
buildNamespaceReviewsPath,
canAccessGlobalReviewCenter,
} from '@/features/review/review-paths'
import { formatLocalDateTime } from '@/shared/lib/date-time'
import { Button } from '@/shared/ui/button'
import { Card } from '@/shared/ui/card'
@ -25,11 +32,18 @@ import { useReviewDetail, useReviewSkillDetail, useApproveReview, useRejectRevie
* interaction state because both actions depend on route-local confirmation
* dialogs, comment input, and redirect behavior after completion.
*/
export function ReviewDetailPage() {
const { id } = useParams({ from: '/dashboard/reviews/$id' })
function ReviewDetailScreen({
taskId,
backTo,
namespaceSlug,
}: {
taskId: number
backTo: string
namespaceSlug?: string
}) {
const navigate = useNavigate()
const { t, i18n } = useTranslation()
const taskId = Number(id)
const { user } = useAuth()
const { data: review, isLoading } = useReviewDetail(taskId)
const {
@ -40,7 +54,7 @@ export function ReviewDetailPage() {
const approveMutation = useApproveReview({
onSuccess: () => {
toast.success(t('review.approveSuccess'))
navigate({ to: '/dashboard/reviews' })
navigate({ to: backTo })
},
onError: (error) => {
toast.error(t('review.approveFailed'), resolveReviewActionErrorDescription(error))
@ -49,7 +63,7 @@ export function ReviewDetailPage() {
const rejectMutation = useRejectReview({
onSuccess: () => {
toast.success(t('review.rejectSuccess'))
navigate({ to: '/dashboard/reviews' })
navigate({ to: backTo })
},
onError: (error) => {
toast.error(t('review.rejectFailed'), resolveReviewActionErrorDescription(error))
@ -64,6 +78,12 @@ export function ReviewDetailPage() {
const [fileBrowserOpen, setFileBrowserOpen] = useState(true)
const [previewNode, setPreviewNode] = useState<FileTreeNode | null>(null)
const [previewDialogOpen, setPreviewDialogOpen] = useState(false)
const hasGlobalReviewAccess = canAccessGlobalReviewCenter(user?.platformRoles)
const shouldRedirectToNamespaceRoute =
!namespaceSlug &&
!!review &&
review.namespace !== 'global' &&
!hasGlobalReviewAccess
// File content for preview — uses the review-bound version via review file API
const { data: previewContent, isLoading: isLoadingPreview, error: previewError } = useReviewFile(
@ -92,6 +112,17 @@ export function ReviewDetailPage() {
return formatLocalDateTime(dateString, i18n.language)
}
useEffect(() => {
if (!shouldRedirectToNamespaceRoute || !review) {
return
}
void navigate({
to: buildNamespaceReviewDetailPath(review.namespace, review.id),
replace: true,
})
}, [navigate, review, shouldRedirectToNamespaceRoute])
const handleApprove = async () => {
approveMutation.mutate({ taskId, comment: comment || undefined })
}
@ -113,6 +144,10 @@ export function ReviewDetailPage() {
)
}
if (shouldRedirectToNamespaceRoute) {
return null
}
if (!review) {
return (
<div className="text-center py-20 animate-fade-up">
@ -121,6 +156,23 @@ export function ReviewDetailPage() {
)
}
const hasNamespaceMismatch = Boolean(namespaceSlug && review.namespace !== namespaceSlug)
if (hasNamespaceMismatch) {
return (
<div className="space-y-6 max-w-3xl animate-fade-up">
<div className="text-center py-20">
<h2 className="text-2xl font-bold font-heading mb-2">{t('review.notFound')}</h2>
</div>
<div className="flex justify-center">
<Button variant="outline" onClick={() => navigate({ to: backTo })}>
{t('review.backToList')}
</Button>
</div>
</div>
)
}
const reviewFiles = reviewSkillDetail?.files
const activeReviewVersion = reviewSkillDetail?.versions?.find(
(version) => version.version === reviewSkillDetail.activeVersion
@ -136,7 +188,7 @@ export function ReviewDetailPage() {
<h1 className="text-4xl font-bold font-heading mb-2">{t('review.detail')}</h1>
<p className="text-muted-foreground">{t('review.id')}: {review.id}</p>
</div>
<Button variant="outline" onClick={() => navigate({ to: '/dashboard/reviews' })}>
<Button variant="outline" onClick={() => navigate({ to: backTo })}>
{t('review.backToList')}
</Button>
</div>
@ -363,3 +415,26 @@ export function ReviewDetailPage() {
</div>
)
}
export function ReviewDetailPage() {
const { id } = useParams({ from: '/dashboard/reviews/$id' })
return (
<ReviewDetailScreen
taskId={Number(id)}
backTo={buildGlobalReviewsPath()}
/>
)
}
export function NamespaceReviewDetailPage() {
const { id, slug } = useParams({ from: '/dashboard/namespaces/$slug/reviews/$id' })
return (
<ReviewDetailScreen
taskId={Number(id)}
backTo={buildNamespaceReviewsPath(slug)}
namespaceSlug={slug}
/>
)
}

View file

@ -67,8 +67,14 @@ vi.mock('@/features/review/use-review-list', () => ({
}))
const hasRoleMock = vi.fn()
const userMock = { platformRoles: ['SKILL_ADMIN'] }
vi.mock('@/features/auth/use-auth', () => ({
useAuth: () => ({ hasRole: hasRoleMock }),
useAuth: () => ({ hasRole: hasRoleMock, user: userMock }),
}))
const useMyNamespacesMock = vi.fn()
vi.mock('@/shared/hooks/use-namespace-queries', () => ({
useMyNamespaces: () => useMyNamespacesMock(),
}))
vi.mock('@/shared/components/dashboard-page-header', () => ({
@ -106,7 +112,13 @@ describe('ReviewsPage', () => {
paginationProps.length = 0
hasRoleMock.mockReset()
useReviewListMock.mockReset()
useMyNamespacesMock.mockReset()
hasRoleMock.mockImplementation((role: string) => role === 'SKILL_ADMIN')
userMock.platformRoles = ['SKILL_ADMIN']
useMyNamespacesMock.mockReturnValue({
data: [],
isLoading: false,
})
useReviewListMock.mockImplementation((status: string, _namespaceId: unknown, page: number, _size: number, _sortDirection: string, enabled: boolean) => {
if (!enabled) {
return { data: null, isLoading: false }

View file

@ -1,10 +1,16 @@
import { useState } from 'react'
import { useEffect, useState } from 'react'
import { useNavigate } from '@tanstack/react-router'
import { FileCheck2 } from 'lucide-react'
import { useTranslation } from 'react-i18next'
import { useMyNamespaces } from '@/shared/hooks/use-namespace-queries'
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card'
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/shared/ui/select'
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/shared/ui/tabs'
import {
buildNamespaceReviewsPath,
canAccessGlobalReviewCenter,
getPreferredNamespaceReviewEntry,
} from '@/features/review/review-paths'
import {
Table,
TableBody,
@ -32,7 +38,8 @@ const PAGE_SIZE = 20
export function ReviewsPage() {
const { t, i18n } = useTranslation()
const navigate = useNavigate()
const { hasRole } = useAuth()
const { hasRole, user } = useAuth()
const { data: myNamespaces, isLoading: isLoadingNamespaces } = useMyNamespaces()
const [pages, setPages] = useState<Record<ReviewStatus, number>>({
PENDING: 0,
APPROVED: 0,
@ -43,14 +50,29 @@ 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 namespaceReviewEntry = getPreferredNamespaceReviewEntry(myNamespaces)
const showTypeTabs = isSkillAdmin && isUserAdmin
// Determine default top-level tab
const defaultType = isSkillAdmin ? 'skill' : 'profile'
const pendingQuery = useReviewList('PENDING', undefined, pages.PENDING, PAGE_SIZE, sortDirection, activeStatus === 'PENDING')
const approvedQuery = useReviewList('APPROVED', undefined, pages.APPROVED, PAGE_SIZE, sortDirection, activeStatus === 'APPROVED')
const rejectedQuery = useReviewList('REJECTED', undefined, pages.REJECTED, PAGE_SIZE, sortDirection, activeStatus === 'REJECTED')
useEffect(() => {
if (hasGlobalReviewAccess || isLoadingNamespaces) {
return
}
if (namespaceReviewEntry) {
void navigate({ to: buildNamespaceReviewsPath(namespaceReviewEntry.slug), replace: true })
return
}
void navigate({ to: '/dashboard', replace: true })
}, [hasGlobalReviewAccess, isLoadingNamespaces, namespaceReviewEntry, navigate])
const pendingQuery = useReviewList('PENDING', undefined, pages.PENDING, PAGE_SIZE, sortDirection, hasGlobalReviewAccess && activeStatus === 'PENDING')
const approvedQuery = useReviewList('APPROVED', undefined, pages.APPROVED, PAGE_SIZE, sortDirection, hasGlobalReviewAccess && activeStatus === 'APPROVED')
const rejectedQuery = useReviewList('REJECTED', undefined, pages.REJECTED, PAGE_SIZE, sortDirection, hasGlobalReviewAccess && activeStatus === 'REJECTED')
const formatDate = (dateString: string) => formatLocalDateTime(dateString, i18n.language)
@ -211,6 +233,17 @@ export function ReviewsPage() {
)
}
if (!hasGlobalReviewAccess) {
return (
<div className="space-y-8 animate-fade-up">
<DashboardPageHeader title={t('reviews.title')} subtitle={t('reviews.subtitle')} />
<Card className="p-8 text-center text-muted-foreground">
Loading...
</Card>
</div>
)
}
return (
<div className="space-y-8 animate-fade-up">
<DashboardPageHeader title={t('reviews.title')} subtitle={t('reviews.subtitle')} />

View file

@ -3,6 +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 { useMyNamespaces } from '@/shared/hooks/use-namespace-queries'
import { buildGlobalReviewsPath, 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'
@ -22,19 +24,19 @@ interface UserMenuProps {
export function UserMenu({ user, triggerClassName }: UserMenuProps) {
const { t } = useTranslation()
const queryClient = useQueryClient()
const { data: myNamespaces } = useMyNamespaces()
const rootRef = useRef<HTMLDivElement | null>(null)
const closeTimerRef = useRef<number | null>(null)
const [isHovered, setIsHovered] = useState(false)
const [isClickOpen, setIsClickOpen] = useState(false)
const hasRole = (role: string) => user.platformRoles?.includes(role) ?? false
const isReviewer = hasRole('SKILL_ADMIN') || hasRole('NAMESPACE_ADMIN') || hasRole('SUPER_ADMIN')
const canSeeGovernance = canViewGovernanceCenter(user.platformRoles)
const isSkillAdmin = hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN')
const isUserAdmin = hasRole('USER_ADMIN') || hasRole('SUPER_ADMIN')
const isAuditor = hasRole('AUDITOR') || hasRole('SUPER_ADMIN')
const isSuperAdmin = hasRole('SUPER_ADMIN')
const canAccessReviewCenter = isReviewer || isUserAdmin
const reviewCenterVisible = canAccessReviewCenter(user.platformRoles, myNamespaces)
const isLocalAccount = !user.oauthProvider
const open = isHovered || isClickOpen
@ -157,8 +159,8 @@ export function UserMenu({ user, triggerClassName }: UserMenuProps) {
<Link to="/dashboard/stars" className={menuItemClassName} onClick={closeMenu}>
{t('user.menu.stars')}
</Link>
{canAccessReviewCenter ? (
<Link to="/dashboard/reviews" className={menuItemClassName} onClick={closeMenu}>
{reviewCenterVisible ? (
<Link to={buildGlobalReviewsPath()} className={menuItemClassName} onClick={closeMenu}>
{t('user.menu.reviews')}
</Link>
) : null}