From 0effc36f45bf91fb51cabb80df11865023786dd1 Mon Sep 17 00:00:00 2001 From: yyh <92089059+lyzno1@users.noreply.github.com> Date: Wed, 30 Sep 2026 04:35:54 +0000 Subject: [PATCH] refactor(access-config): scope app permissions to the page query (#43259) --- .../[appId]/access-config/page.tsx | 16 ++- .../access-config/__tests__/index.spec.tsx | 107 +++++++++++++----- .../components/app/access-config/index.tsx | 41 +++---- 3 files changed, 113 insertions(+), 51 deletions(-) diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/access-config/page.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/access-config/page.tsx index 92dcff4064a..71121c09faa 100644 --- a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/access-config/page.tsx +++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/access-config/page.tsx @@ -1,5 +1,7 @@ import type { Locale } from '@/i18n' +import { Suspense } from 'react' import AppAccessConfigPage from '@/app/components/app/access-config' +import { SkeletonContainer, SkeletonRectangle } from '@/app/components/base/skeleton' export type AccessConfigPageProps = { params: Promise<{ locale: Locale; appId: string }> @@ -10,7 +12,19 @@ const AccessConfig = async (props: AccessConfigPageProps) => { const { appId } = params - return + return ( + + + + + + } + > + + + ) } export default AccessConfig diff --git a/web/app/components/app/access-config/__tests__/index.spec.tsx b/web/app/components/app/access-config/__tests__/index.spec.tsx index 2986b56b77e..f77c906df52 100644 --- a/web/app/components/app/access-config/__tests__/index.spec.tsx +++ b/web/app/components/app/access-config/__tests__/index.spec.tsx @@ -1,13 +1,15 @@ import type { AccessRulesEditorProps } from '@/app/components/access-rules-editor' -import { act, screen } from '@testing-library/react' -import { useStore } from '@/app/components/app/store' +import { act, screen, waitFor } from '@testing-library/react' +import ErrorBoundary from '@/app/components/base/error-boundary' import { useAppAccessRules, useAppResourceWhitelist, useAppResourceWhitelistConfig, useAppUserAccessSettings, } from '@/service/access-control/use-app-access-config' -import { renderWithConsoleQuery } from '@/test/console/query-data' +import { consoleQuery } from '@/service/console' +import { createConsoleQueryClient, renderWithConsoleQuery } from '@/test/console/query-data' +import { createAppDetailFixture } from '@/test/fixtures/app' import { AppModeEnum } from '@/types/app' import { AppACLPermission } from '@/utils/permission' import AppAccessConfigPage from '../index' @@ -18,12 +20,20 @@ const mockConsoleState = vi.hoisted(() => ({ })) let mockIsRbacEnabled = true +let appDetail = createAppDetailFixture() -const render = (ui: Parameters[0]) => - renderWithConsoleQuery(ui, { +const render = (ui: Parameters[0]) => { + const queryClient = createConsoleQueryClient() + queryClient.setQueryData( + consoleQuery.apps.byAppId.get.queryKey({ input: { params: { app_id: appDetail.id } } }), + appDetail, + ) + return renderWithConsoleQuery(ui, { + queryClient, accountProfile: mockConsoleState.userProfile, systemFeatures: { rbac_enabled: mockIsRbacEnabled }, }) +} const mockAppAccessRules = vi.hoisted(() => ({ items: [] as AccessRulesEditorProps['rules'], @@ -143,12 +153,10 @@ describe('AppAccessConfigPage', () => { mockMutations.isUpdatingAutomaticIncludeWorkspaceMembers = false mockMutations.removeMemberBindingsAsync.mockResolvedValue(undefined) mockAccessRulesEditor.props = null - useStore.setState({ - appDetail: { - id: 'app-1', - maintainer: 'account-1', - permission_keys: [AppACLPermission.AccessConfig], - } as unknown as NonNullable['appDetail']>, + appDetail = createAppDetailFixture({ + id: 'app-1', + maintainer: 'account-1', + permission_keys: [AppACLPermission.AccessConfig], }) }) @@ -341,12 +349,10 @@ describe('AppAccessConfigPage', () => { }) it('should not mount access config data hooks when access permission is missing', () => { - useStore.setState({ - appDetail: { - id: 'app-1', - maintainer: 'account-1', - permission_keys: [AppACLPermission.ViewLayout], - } as NonNullable['appDetail']>, + appDetail = createAppDetailFixture({ + id: 'app-1', + maintainer: 'account-1', + permission_keys: [AppACLPermission.ViewLayout], }) render() @@ -367,13 +373,11 @@ describe('AppAccessConfigPage', () => { }) it('should not mount access config data hooks for Agent apps', () => { - useStore.setState({ - appDetail: { - id: 'app-1', - mode: AppModeEnum.AGENT, - maintainer: 'account-1', - permission_keys: [AppACLPermission.AccessConfig], - } as unknown as NonNullable['appDetail']>, + appDetail = createAppDetailFixture({ + id: 'app-1', + mode: AppModeEnum.AGENT, + maintainer: 'account-1', + permission_keys: [AppACLPermission.AccessConfig], }) render() @@ -386,16 +390,59 @@ describe('AppAccessConfigPage', () => { it('should allow the maintainer with app management workspace permission', () => { mockConsoleState.userProfile = { id: 'account-1' } mockConsoleState.workspacePermissionKeys = ['app.create_and_management'] - useStore.setState({ - appDetail: { - id: 'app-1', - maintainer: 'account-1', - permission_keys: [], - } as unknown as NonNullable['appDetail']>, + appDetail = createAppDetailFixture({ + id: 'app-1', + maintainer: 'account-1', + permission_keys: [], }) render() expect(screen.getByTestId('access-rules-editor')).toBeInTheDocument() }) + + it('keeps the current member page on metadata refresh and stops rendering after permission revocation', async () => { + appDetail = createAppDetailFixture({ permission_keys: [AppACLPermission.AccessConfig] }) + const { queryClient } = render() + const queryKey = consoleQuery.apps.byAppId.get.queryKey({ + input: { params: { app_id: 'app-1' } }, + }) + act(() => mockAccessRulesEditor.props?.onPageChange?.(3)) + expect(useAppUserAccessSettings).toHaveBeenLastCalledWith('app-1', expect.any(String), 3, 10) + act(() => { + queryClient.setQueryData(queryKey, { ...appDetail, maintainer: 'new-maintainer' }) + }) + await waitFor(() => expect(mockAccessRulesEditor.props?.maintainerId).toBe('new-maintainer')) + expect(useAppUserAccessSettings).toHaveBeenLastCalledWith('app-1', expect.any(String), 3, 10) + act(() => { + queryClient.setQueryData(queryKey, { ...appDetail, permission_keys: [] }) + }) + await waitFor(() => expect(screen.queryByTestId('access-rules-editor')).not.toBeInTheDocument()) + }) + + it('surfaces an initial detail failure without mounting member queries', async () => { + const queryClient = createConsoleQueryClient() + const queryKey = consoleQuery.apps.byAppId.get.queryKey({ + input: { params: { app_id: 'unavailable-app' } }, + }) + queryClient.setQueryDefaults(queryKey, { retryOnMount: false }) + await queryClient + .query({ queryKey, queryFn: () => Promise.reject(new Error('Detail unavailable')) }) + .catch(() => {}) + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + renderWithConsoleQuery( + + + , + { queryClient }, + ) + expect( + await screen.findByRole('button', { name: 'common.errorBoundary.tryAgain' }), + ).toBeInTheDocument() + expect(useAppAccessRules).not.toHaveBeenCalled() + } finally { + consoleError.mockRestore() + } + }) }) diff --git a/web/app/components/app/access-config/index.tsx b/web/app/components/app/access-config/index.tsx index 7e19c81a65a..1088e44fbfe 100644 --- a/web/app/components/app/access-config/index.tsx +++ b/web/app/components/app/access-config/index.tsx @@ -1,13 +1,13 @@ 'use client' +import type { AppDetailWithSite } from '@dify/contracts/api/console/apps/types.gen' import type { AccessPolicyMemberBindingRemoval } from '@/app/components/access-rules-editor' -import { useSuspenseQuery } from '@tanstack/react-query' +import { useSuspenseQueries } from '@tanstack/react-query' import { useAtomValue } from 'jotai' import { useCallback, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import { useLocale } from '#i18n' import AccessRulesEditor from '@/app/components/access-rules-editor' -import { useStore } from '@/app/components/app/store' import { workspacePermissionKeysAtom } from '@/context/permission-state' import { userProfileQueryOptions } from '@/features/account-profile/client' import { systemFeaturesQueryOptions } from '@/features/system-features/client' @@ -22,6 +22,7 @@ import { useUpdateAppAutomaticIncludeWorkspaceMembers, useUpdateAppUserAccessSettings, } from '@/service/access-control/use-app-access-config' +import { consoleQuery } from '@/service/console' import { AppModeEnum } from '@/types/app' import { getAppACLCapabilities } from '@/utils/permission' @@ -31,7 +32,7 @@ type AppAccessConfigPageProps = { type AppAccessConfigContentProps = { appId: string - maintainerId?: string | null + maintainerId: AppDetailWithSite['maintainer'] } const AppAccessConfigContent = ({ appId, maintainerId }: AppAccessConfigContentProps) => { @@ -233,38 +234,38 @@ const AppAccessConfigContent = ({ appId, maintainerId }: AppAccessConfigContentP } const AppAccessConfigPage = ({ appId }: AppAccessConfigPageProps) => { - const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions()) - const { data: currentUserId } = useSuspenseQuery({ - ...userProfileQueryOptions(), - select: (data) => data.profile.id, - }) + const [{ data: systemFeatures }, { data: userProfile }, { data: appDetail }] = useSuspenseQueries( + { + queries: [ + systemFeaturesQueryOptions(), + userProfileQueryOptions(), + consoleQuery.apps.byAppId.get.queryOptions({ + input: { params: { app_id: appId } }, + }), + ], + }, + ) + const currentUserId = userProfile.profile.id const workspacePermissionKeys = useAtomValue(workspacePermissionKeysAtom) const isRbacEnabled = systemFeatures.rbac_enabled - const appDetail = useStore((state) => state.appDetail) const appACLCapabilities = useMemo( () => - getAppACLCapabilities(appDetail?.permission_keys, { + getAppACLCapabilities(appDetail.permission_keys, { currentUserId, - resourceMaintainer: appDetail?.maintainer, + resourceMaintainer: appDetail.maintainer, workspacePermissionKeys, isRbacEnabled, }), [ - appDetail?.maintainer, - appDetail?.permission_keys, + appDetail.maintainer, + appDetail.permission_keys, currentUserId, isRbacEnabled, workspacePermissionKeys, ], ) - if ( - !appDetail || - appDetail.id !== appId || - appDetail.mode === AppModeEnum.AGENT || - !appACLCapabilities.canAccessConfig - ) - return null + if (appDetail.mode === AppModeEnum.AGENT || !appACLCapabilities.canAccessConfig) return null return }