From fa3e52fa9e4114e7500083485bc15a55c60eb9d2 Mon Sep 17 00:00:00 2001 From: Stephen Zhou <38493346+hyoban@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:46:05 +0800 Subject: [PATCH] refactor(knowledge-fs): reorganize settings ownership --- .../new-rag/settings/__tests__/form.spec.tsx | 164 +- .../new-rag/settings/__tests__/page.spec.tsx | 293 ++- .../__tests__/state-boundary.spec.tsx | 93 + .../new-rag/settings/basic-information.tsx | 454 +++++ .../settings/capability-return-controller.tsx | 32 + .../new-rag/settings/delete-knowledge.tsx | 131 ++ .../new-rag/settings/external-access.tsx | 158 ++ web/features/new-rag/settings/form.tsx | 1585 +---------------- web/features/new-rag/settings/model.ts | 132 ++ .../new-rag/settings/navigation-guard.tsx | 177 ++ web/features/new-rag/settings/page.tsx | 238 +-- .../new-rag/settings/retrieval-settings.tsx | 657 +++++++ .../new-rag/settings/settings-field-row.tsx | 12 + .../new-rag/settings/state/boundary.tsx | 29 + web/features/new-rag/settings/state/inputs.ts | 5 + .../new-rag/settings/state/queries.ts | 88 + .../new-rag/settings/state/workflow.ts | 47 + 17 files changed, 2283 insertions(+), 2012 deletions(-) create mode 100644 web/features/new-rag/settings/__tests__/state-boundary.spec.tsx create mode 100644 web/features/new-rag/settings/basic-information.tsx create mode 100644 web/features/new-rag/settings/capability-return-controller.tsx create mode 100644 web/features/new-rag/settings/delete-knowledge.tsx create mode 100644 web/features/new-rag/settings/external-access.tsx create mode 100644 web/features/new-rag/settings/model.ts create mode 100644 web/features/new-rag/settings/navigation-guard.tsx create mode 100644 web/features/new-rag/settings/retrieval-settings.tsx create mode 100644 web/features/new-rag/settings/settings-field-row.tsx create mode 100644 web/features/new-rag/settings/state/boundary.tsx create mode 100644 web/features/new-rag/settings/state/inputs.ts create mode 100644 web/features/new-rag/settings/state/queries.ts create mode 100644 web/features/new-rag/settings/state/workflow.ts diff --git a/web/features/new-rag/settings/__tests__/form.spec.tsx b/web/features/new-rag/settings/__tests__/form.spec.tsx index cbe0e12ee63..0ff17540160 100644 --- a/web/features/new-rag/settings/__tests__/form.spec.tsx +++ b/web/features/new-rag/settings/__tests__/form.spec.tsx @@ -7,13 +7,16 @@ import type { Member } from '@/models/common' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { act, fireEvent, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' +import { queryClientAtom } from 'jotai-tanstack-query' +import { useHydrateAtoms } from 'jotai/utils' import { render } from '@/test/console/render' import { createSystemFeaturesFixture } from '@/test/console/system-features' -import { KnowledgeSettingsForm } from '../form' +import { KnowledgeSettingsPage } from '../page' const serviceMock = vi.hoisted(() => ({ deleteSpace: vi.fn(), getMigration: vi.fn(), + getSpace: vi.fn(), patchExternalAccess: vi.fn(), patchSettings: vi.fn(), patchSpace: vi.fn(), @@ -30,6 +33,20 @@ const routerMock = vi.hoisted(() => ({ replace: vi.fn(), })) +const knowledgeQueryMock = vi.hoisted(() => ({ + externalAccess: undefined as unknown, + permissions: { data: [] } as unknown, + settings: undefined as unknown, + space: undefined as unknown, +})) + +const membersQueryMock = vi.hoisted(() => ({ + data: { accounts: [] as Member[] }, + isError: false, + isPending: false, + refetch: vi.fn(() => Promise.resolve()), +})) + const queryKeys = { accountProfile: [['console', 'account', 'profile', 'get'], { type: 'query' }], externalAccess: ['knowledge-fs', 'external-access'], @@ -41,6 +58,11 @@ const queryKeys = { vi.mock('@/next/navigation', () => ({ useRouter: () => routerMock, + useSearchParams: () => new URLSearchParams(), +})) + +vi.mock('@/service/use-common', () => ({ + useMembers: () => membersQueryMock, })) vi.mock('@/service/client', () => ({ @@ -57,12 +79,26 @@ vi.mock('@/service/client', () => ({ mutationOptions: () => ({ mutationFn: serviceMock.deleteSpace }), }, externalAccess: { - get: { key: () => queryKeys.externalAccess }, + get: { + key: () => queryKeys.externalAccess, + queryOptions: () => ({ + queryFn: () => Promise.resolve(knowledgeQueryMock.externalAccess), + queryKey: queryKeys.externalAccess, + staleTime: Infinity, + }), + }, put: { mutationOptions: () => ({ mutationFn: serviceMock.patchExternalAccess }), }, }, - get: { key: () => queryKeys.space }, + get: { + key: () => queryKeys.space, + queryOptions: () => ({ + queryFn: () => serviceMock.getSpace(), + queryKey: queryKeys.space, + staleTime: Infinity, + }), + }, members: { put: { mutationOptions: () => ({ mutationFn: serviceMock.replaceMembers }), @@ -72,10 +108,24 @@ vi.mock('@/service/client', () => ({ mutationOptions: () => ({ mutationFn: serviceMock.patchSpace }), }, permissions: { - get: { key: () => queryKeys.permissions }, + get: { + key: () => queryKeys.permissions, + queryOptions: () => ({ + queryFn: () => Promise.resolve(knowledgeQueryMock.permissions), + queryKey: queryKeys.permissions, + staleTime: Infinity, + }), + }, }, settings: { - get: { key: () => queryKeys.settings }, + get: { + key: () => queryKeys.settings, + queryOptions: () => ({ + queryFn: () => Promise.resolve(knowledgeQueryMock.settings), + queryKey: queryKeys.settings, + staleTime: Infinity, + }), + }, migrations: { byMigrationId: { get: { @@ -261,10 +311,7 @@ function renderForm({ accountProfile = {}, externalAccess: externalAccessOverride = externalAccess, members = [], - onDraftFinish, - onDraftStart, queryClient: queryClientOverride, - serverConflict, settings: settingsOverride = settings, space: spaceOverride = space, }: { @@ -279,10 +326,7 @@ function renderForm({ }> externalAccess?: typeof externalAccess members?: Member[] - onDraftFinish?: () => void - onDraftStart?: () => void queryClient?: QueryClient - serverConflict?: boolean settings?: KnowledgeFsSettingsResponse space?: KnowledgeFsSpaceDetailResponse } = {}) { @@ -308,28 +352,30 @@ function renderForm({ }, }) queryClient.setQueryData(queryKeys.systemFeatures, createSystemFeaturesFixture()) - const Wrapper = ({ children }: { children: ReactNode }) => ( - {children} - ) - return render( - , - { wrapper: Wrapper }, - ) + knowledgeQueryMock.externalAccess = externalAccessOverride + knowledgeQueryMock.permissions = { data: [] } + knowledgeQueryMock.settings = settingsOverride + knowledgeQueryMock.space = spaceOverride + membersQueryMock.data = { accounts: members } + queryClient.setQueryData(queryKeys.externalAccess, externalAccessOverride) + queryClient.setQueryData(queryKeys.permissions, { data: [] }) + queryClient.setQueryData(queryKeys.settings, settingsOverride) + queryClient.setQueryData(queryKeys.space, spaceOverride) + const Wrapper = ({ children }: { children: ReactNode }) => { + useHydrateAtoms([[queryClientAtom, queryClient]], { dangerouslyForceHydrate: true }) + return {children} + } + return { + queryClient, + ...render(, { wrapper: Wrapper }), + } } -describe('KnowledgeSettingsForm', () => { +describe('KnowledgeSettingsPage workflows', () => { beforeEach(() => { vi.clearAllMocks() serviceMock.deleteSpace.mockResolvedValue(undefined) + serviceMock.getSpace.mockResolvedValue(space) serviceMock.patchExternalAccess.mockResolvedValue(externalAccess) serviceMock.patchSettings.mockResolvedValue({ settings }) serviceMock.patchSpace.mockResolvedValue(space) @@ -382,9 +428,8 @@ describe('KnowledgeSettingsForm', () => { expect(toastMock.success).toHaveBeenCalledWith('common.api.actionSuccess') }) - it('finishes the basic info draft before refreshing saved server data', async () => { + it('keeps basic fields locked while refreshing saved server data', async () => { const user = userEvent.setup() - const onDraftFinish = vi.fn() let finishRefresh!: () => void const refreshPromise = new Promise((resolve) => { finishRefresh = resolve @@ -395,8 +440,8 @@ describe('KnowledgeSettingsForm', () => { queries: { retry: false }, }, }) - vi.spyOn(queryClient, 'invalidateQueries').mockReturnValue(refreshPromise) - renderForm({ onDraftFinish, queryClient }) + serviceMock.getSpace.mockReturnValueOnce(refreshPromise.then(() => space)) + renderForm({ queryClient }) const nameInput = screen.getByRole('textbox', { name: 'datasetSettings.form.name' }) await user.clear(nameInput) @@ -408,7 +453,6 @@ describe('KnowledgeSettingsForm', () => { ) await waitFor(() => expect(serviceMock.patchSpace).toHaveBeenCalledOnce()) - expect(onDraftFinish).toHaveBeenCalledOnce() expect(nameInput).toBeDisabled() finishRefresh() await waitFor(() => expect(nameInput).toBeEnabled()) @@ -1168,8 +1212,6 @@ describe('KnowledgeSettingsForm', () => { it('requires a rerank model for a legacy knowledge base and saves it as enabled', async () => { const user = userEvent.setup() - const onDraftFinish = vi.fn() - const onDraftStart = vi.fn() serviceMock.patchSettings.mockResolvedValueOnce({ migration: { changed_kind: 'retrieval', @@ -1194,8 +1236,6 @@ describe('KnowledgeSettingsForm', () => { updated_at: '2026-07-28T00:01:00Z', }) renderForm({ - onDraftFinish, - onDraftStart, settings: { ...settings, configuration_state: 'setup-required', @@ -1244,10 +1284,9 @@ describe('KnowledgeSettingsForm', () => { }, expect.anything(), ) - expect(onDraftStart).toHaveBeenCalledOnce() expect(screen.queryByText('dataset.newKnowledge.settings.rerankModelRequired')).toBeNull() await waitFor(() => expect(serviceMock.getMigration).toHaveBeenCalledOnce()) - await waitFor(() => expect(onDraftFinish).toHaveBeenCalledOnce()) + await waitFor(() => expect(toastMock.success).toHaveBeenCalledWith('common.api.actionSuccess')) }) it('saves a reasoning model selection immediately without enabling the basic info save', async () => { @@ -1277,9 +1316,7 @@ describe('KnowledgeSettingsForm', () => { serviceMock.patchSettings .mockReturnValueOnce(firstSave) .mockResolvedValueOnce({ settings: { ...settings, revision: 7 } }) - const onDraftFinish = vi.fn() - const onDraftStart = vi.fn() - renderForm({ onDraftFinish, onDraftStart }) + renderForm() const reasoningSelector = screen.getByRole('button', { name: 'dataset.newKnowledge.settings.systemReasoningModelLabel', @@ -1298,10 +1335,6 @@ describe('KnowledgeSettingsForm', () => { ).not.toHaveAttribute('aria-disabled', 'true') await user.click(rerankSelector) - expect(onDraftStart).toHaveBeenCalledTimes(2) - expect(onDraftStart.mock.invocationCallOrder[0]).toBeLessThan( - serviceMock.patchSettings.mock.invocationCallOrder[0]!, - ) expect(serviceMock.patchSettings).toHaveBeenCalledOnce() expect(serviceMock.patchSettings).toHaveBeenNthCalledWith( 1, @@ -1340,7 +1373,7 @@ describe('KnowledgeSettingsForm', () => { }, expect.anything(), ) - await waitFor(() => expect(onDraftFinish).toHaveBeenCalledOnce()) + await waitFor(() => expect(toastMock.success).toHaveBeenCalledWith('common.api.actionSuccess')) }) it('continues a queued model selection only after its preceding migration succeeds', async () => { @@ -1496,35 +1529,23 @@ describe('KnowledgeSettingsForm', () => { it('blocks a stale draft after the server baseline changes and restores the latest value', async () => { const user = userEvent.setup() - const onDraftFinish = vi.fn() - const onDraftStart = vi.fn() - const renderWithName = (name: string, serverConflict = false) => ( - - ) - const view = renderForm({ onDraftFinish, onDraftStart }) + const { queryClient } = renderForm() const nameInput = screen.getByRole('textbox', { name: 'datasetSettings.form.name' }) await user.clear(nameInput) await user.type(nameInput, 'Version B') - expect(onDraftStart).toHaveBeenCalled() - view.rerender(renderWithName('Version C', true)) + act(() => { + queryClient.setQueryData(queryKeys.space, { + ...space, + resource_version: space.resource_version + 1, + technical_summary: { + ...space.technical_summary, + name: 'Version C', + }, + }) + }) expect(nameInput).toHaveValue('Version B') - expect(screen.getByRole('alert')).toHaveTextContent( + expect(await screen.findByRole('alert')).toHaveTextContent( 'dataset.newKnowledge.settings.serverConflict', ) expect( @@ -1536,7 +1557,6 @@ describe('KnowledgeSettingsForm', () => { await user.click(screen.getByRole('button', { name: 'common.operation.cancel' })) expect(nameInput).toHaveValue('Version C') - expect(onDraftFinish).toHaveBeenCalledOnce() expect(serviceMock.patchSpace).not.toHaveBeenCalled() }) diff --git a/web/features/new-rag/settings/__tests__/page.spec.tsx b/web/features/new-rag/settings/__tests__/page.spec.tsx index e7d37e5bff7..f4e19ac74c4 100644 --- a/web/features/new-rag/settings/__tests__/page.spec.tsx +++ b/web/features/new-rag/settings/__tests__/page.spec.tsx @@ -2,123 +2,90 @@ import type { ReactNode } from 'react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { act, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' +import { queryClientAtom } from 'jotai-tanstack-query' +import { useHydrateAtoms } from 'jotai/utils' import { render } from '@/test/console/render' import { KnowledgeSettingsPage } from '../page' -const useQueryOptionsMock = vi.hoisted(() => vi.fn()) const navigationMock = vi.hoisted(() => ({ replace: vi.fn(), searchParams: new URLSearchParams(), })) +const serviceMock = vi.hoisted(() => ({ + getExternalAccess: vi.fn(), + getPermissions: vi.fn(), + getSettings: vi.fn(), + getSpace: vi.fn(), + queryOptions: vi.fn(), +})) + vi.mock('@/next/navigation', () => ({ useRouter: () => ({ replace: navigationMock.replace }), useSearchParams: () => navigationMock.searchParams, })) -vi.mock('@tanstack/react-query', async (importOriginal) => { - const actual = await importOriginal() - return { - ...actual, - useQuery: (options: Parameters[0]) => { - useQueryOptionsMock(options) - return actual.useQuery(options) - }, - } -}) - -const membersQueryMock = vi.hoisted(() => ({ - data: undefined as { accounts: [] } | undefined, - isError: false, - isPending: true, - refetch: vi.fn(() => Promise.resolve()), -})) - -vi.mock('@/service/use-common', () => ({ - useMembers: () => membersQueryMock, -})) - vi.mock('../form', () => ({ - KnowledgeSettingsForm: ({ - onDraftFinish, - onDraftStart, - serverConflict, - }: { - onDraftFinish: () => void - onDraftStart: () => void - serverConflict: boolean - }) => ( -
- settings-form - {serverConflict ? 'server-conflict' : 'no-conflict'} - - -
- ), + KnowledgeSettingsForm: () =>
settings-form
, })) -const queryData = vi.hoisted(() => ({ - settings: { - active_profile_available: true, - active_profile_revisions: { embedding: 1, retrieval: 1 }, - capabilities: { - deep: true, - index: true, - ingest: true, - query: true, - research: true, - source_sync: true, - }, - configuration_state: 'active', - embedding: null, - issues: [], - retrieval: null, - revision: 1, +const queryKeys = { + externalAccess: ['knowledge-fs', 'external-access'], + permissions: ['knowledge-fs', 'permissions'], + settings: ['knowledge-fs', 'settings'], + space: ['knowledge-fs', 'space'], +} + +const space = { + control_space_id: 'space-1', + created_at: '2026-07-28T00:00:00Z', + knowledge_space_id: 'knowledge-1', + owner_account_id: 'owner-1', + permission_keys: ['knowledge_space_access_config', 'knowledge_space_edit'], + resource_version: 1, + state: 'active', + technical_status: 'available', + technical_summary: null, + updated_at: '2026-07-28T00:00:00Z', + visibility: 'only_me', +} + +const settings = { + active_profile_available: true, + active_profile_revisions: { embedding: 1, retrieval: 1 }, + capabilities: { + deep: true, + index: true, + ingest: true, + query: true, + research: true, + source_sync: true, }, - space: { - control_space_id: 'space-1', - created_at: '2026-07-28T00:00:00Z', - knowledge_space_id: 'knowledge-1', - owner_account_id: 'owner-1', - permission_keys: ['knowledge_space_access_config', 'knowledge_space_edit'], - resource_version: 1, - state: 'active', - technical_status: 'available', - technical_summary: null, - updated_at: '2026-07-28T00:00:00Z', - visibility: 'only_me', - }, -})) + configuration_state: 'active', + embedding: null, + issues: [], + retrieval: null, + revision: 1, +} vi.mock('@/service/client', () => { - const query = (key: string, data: unknown) => ({ - key: () => ['knowledge-fs', key], - queryOptions: () => ({ - queryFn: () => Promise.resolve(data), - queryKey: ['knowledge-fs', key], - }), + const query = (key: keyof typeof queryKeys, queryFn: () => Promise) => ({ + queryOptions: () => { + const options = { queryFn, queryKey: queryKeys[key] } + serviceMock.queryOptions(options) + return options + }, }) + return { consoleQuery: { knowledgeFs: { spaces: { byControlSpaceId: { - externalAccess: { - get: query('external-access', { - agent_enabled: false, - mcp_enabled: false, - revision: 1, - service_api_enabled: false, - workflow_enabled: false, - }), - }, - get: query('space', queryData.space), - permissions: { get: query('permissions', { data: [] }) }, - settings: { get: query('settings', queryData.settings) }, + externalAccess: { get: query('externalAccess', serviceMock.getExternalAccess) }, + get: query('space', serviceMock.getSpace), + permissions: { get: query('permissions', serviceMock.getPermissions) }, + settings: { get: query('settings', serviceMock.getSettings) }, }, }, }, @@ -126,15 +93,32 @@ vi.mock('@/service/client', () => { } }) -function renderPage() { +function renderPage({ + seed = false, + settingsData = settings, +}: { + seed?: boolean + settingsData?: typeof settings +} = {}) { const queryClient = new QueryClient({ - defaultOptions: { - queries: { retry: false }, - }, + defaultOptions: { queries: { retry: false } }, }) - const Wrapper = ({ children }: { children: ReactNode }) => ( - {children} - ) + if (seed) { + queryClient.setQueryData(queryKeys.space, space) + queryClient.setQueryData(queryKeys.settings, settingsData) + queryClient.setQueryData(queryKeys.permissions, { data: [] }) + queryClient.setQueryData(queryKeys.externalAccess, { + agent_enabled: false, + mcp_enabled: false, + revision: 1, + service_api_enabled: false, + workflow_enabled: false, + }) + } + const Wrapper = ({ children }: { children: ReactNode }) => { + useHydrateAtoms([[queryClientAtom, queryClient]], { dangerouslyForceHydrate: true }) + return {children} + } return { queryClient, ...render(, { wrapper: Wrapper }), @@ -143,114 +127,69 @@ function renderPage() { describe('KnowledgeSettingsPage', () => { beforeEach(() => { - membersQueryMock.data = undefined - membersQueryMock.isError = false - membersQueryMock.isPending = true - membersQueryMock.refetch.mockClear() - useQueryOptionsMock.mockClear() - navigationMock.replace.mockClear() + vi.clearAllMocks() navigationMock.searchParams = new URLSearchParams() - queryData.settings.capabilities.ingest = true + serviceMock.getExternalAccess.mockResolvedValue({ + agent_enabled: false, + mcp_enabled: false, + revision: 1, + service_api_enabled: false, + workflow_enabled: false, + }) + serviceMock.getPermissions.mockResolvedValue({ data: [] }) + serviceMock.getSettings.mockResolvedValue(settings) + serviceMock.getSpace.mockResolvedValue(space) }) - it('keeps the settings form gated while workspace members are loading', async () => { + it('shows the page skeleton while its server graph is loading', () => { + serviceMock.getSpace.mockReturnValue(new Promise(() => {})) renderPage() - await waitFor(() => { - expect(screen.queryByText('settings-form')).not.toBeInTheDocument() - expect(screen.getByRole('status')).toHaveTextContent('common.loading') - expect(screen.getByRole('status')).not.toHaveTextContent( - 'dataset.newKnowledge.settings.basicInfo', - ) - expect(screen.getByRole('status')).not.toHaveTextContent( - 'dataset.newKnowledge.settings.dangerZone', - ) - expect(screen.getByText('dataset.newKnowledge.settings.basicInfo')).toBeInTheDocument() - expect(screen.getByText('dataset.newKnowledge.settings.dangerZone')).toBeInTheDocument() - }) + expect(screen.getByRole('status')).toHaveTextContent('common.loading') + expect(screen.queryByText('settings-form')).not.toBeInTheDocument() + expect(screen.getByText('dataset.newKnowledge.settings.basicInfo')).toBeInTheDocument() + expect(screen.getByText('dataset.newKnowledge.settings.dangerZone')).toBeInTheDocument() }) - it('shows the page error state and retries the members request', async () => { + it('shows the query error state and retries the failed server graph', async () => { const user = userEvent.setup() - membersQueryMock.isPending = false - membersQueryMock.isError = true + serviceMock.getSpace.mockRejectedValueOnce(new Error('unavailable')) renderPage() expect(await screen.findByRole('alert')).toBeInTheDocument() await user.click(screen.getByRole('button', { name: 'common.operation.retry' })) - expect(membersQueryMock.refetch).toHaveBeenCalledOnce() - expect(screen.queryByText('settings-form')).not.toBeInTheDocument() + expect(serviceMock.getSpace).toHaveBeenCalledTimes(2) + expect(await screen.findByText('settings-form')).toBeInTheDocument() }) - it('does not poll settings after model selections are saved', async () => { - membersQueryMock.data = { accounts: [] } - membersQueryMock.isPending = false - renderPage() + it('renders the form from cached queries without polling settings', async () => { + renderPage({ seed: true }) expect(await screen.findByText('settings-form')).toBeInTheDocument() - - const settingsOptions = useQueryOptionsMock.mock.calls + const settingsOptions = serviceMock.queryOptions.mock.calls .map(([options]) => options) - .find((options) => options.queryKey?.[1] === 'settings') - + .find((options) => options.queryKey === queryKeys.settings) expect(settingsOptions).not.toHaveProperty('refetchInterval') }) - it('keeps an active draft mounted and reports a conflict when the server version changes', async () => { - const user = userEvent.setup() - membersQueryMock.data = { accounts: [] } - membersQueryMock.isPending = false - const { queryClient } = renderPage() - - expect(await screen.findByText('no-conflict')).toBeInTheDocument() - await user.click(screen.getByRole('button', { name: 'start-draft' })) - act(() => { - queryClient.setQueryData(['knowledge-fs', 'space'], { - ...queryData.space, - resource_version: 2, - }) - }) - - expect(await screen.findByText('server-conflict')).toBeInTheDocument() - await user.click(screen.getByRole('button', { name: 'finish-draft' })) - expect(await screen.findByText('no-conflict')).toBeInTheDocument() - }) - - it('does not report a basic info conflict when immediate retrieval settings refresh', async () => { - const user = userEvent.setup() - membersQueryMock.data = { accounts: [] } - membersQueryMock.isPending = false - const { queryClient } = renderPage() - - expect(await screen.findByText('no-conflict')).toBeInTheDocument() - await user.click(screen.getByRole('button', { name: 'start-draft' })) - act(() => { - queryClient.setQueryData(['knowledge-fs', 'settings'], { - ...queryData.settings, - revision: 2, - }) - }) - - expect(await screen.findByText('no-conflict')).toBeInTheDocument() - }) - it('returns to a validated source page when its blocked capability becomes available', async () => { - membersQueryMock.data = { accounts: [] } - membersQueryMock.isPending = false navigationMock.searchParams = new URLSearchParams({ capability: 'ingest', returnTo: '/datasets/new/space-1/documents', }) - queryData.settings.capabilities.ingest = false - const { queryClient } = renderPage() + const blockedSettings = { + ...settings, + capabilities: { ...settings.capabilities, ingest: false }, + } + const { queryClient } = renderPage({ seed: true, settingsData: blockedSettings }) expect(await screen.findByText('settings-form')).toBeInTheDocument() expect(navigationMock.replace).not.toHaveBeenCalled() act(() => { - queryClient.setQueryData(['knowledge-fs', 'settings'], { - ...queryData.settings, - capabilities: { ...queryData.settings.capabilities, ingest: true }, + queryClient.setQueryData(queryKeys.settings, { + ...blockedSettings, + capabilities: { ...blockedSettings.capabilities, ingest: true }, }) }) await waitFor(() => diff --git a/web/features/new-rag/settings/__tests__/state-boundary.spec.tsx b/web/features/new-rag/settings/__tests__/state-boundary.spec.tsx new file mode 100644 index 00000000000..d6d5559a409 --- /dev/null +++ b/web/features/new-rag/settings/__tests__/state-boundary.spec.tsx @@ -0,0 +1,93 @@ +import { screen, within } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { atom, createStore, Provider, useAtomValue, useSetAtom } from 'jotai' +import { render } from '@/test/console/render' +import { KnowledgeSettingsStateBoundary } from '../state/boundary' +import { knowledgeSettingsSpaceIdAtom } from '../state/inputs' +import { + knowledgeSettingsHasUnsavedWorkAtom, + startKnowledgeSettingsBasicDraftAtom, +} from '../state/workflow' + +const globalProbeAtom = atom('global-before') + +function WorkflowProbe({ name }: { name: string }) { + const globalValue = useAtomValue(globalProbeAtom) + const hasUnsavedWork = useAtomValue(knowledgeSettingsHasUnsavedWorkAtom) + const knowledgeSpaceId = useAtomValue(knowledgeSettingsSpaceIdAtom) + const startDraft = useSetAtom(startKnowledgeSettingsBasicDraftAtom) + + return ( +
+ {globalValue} + {hasUnsavedWork ? 'dirty' : 'clean'} + {knowledgeSpaceId} + +
+ ) +} + +function GlobalProbeControl() { + const setGlobalValue = useSetAtom(globalProbeAtom) + + return ( + + ) +} + +describe('KnowledgeSettingsStateBoundary', () => { + it('keeps parent atoms visible while isolating workflow state between instances', async () => { + const user = userEvent.setup() + const store = createStore() + + render( + + + + + + + + + , + ) + + const first = within(screen.getByRole('region', { name: 'first settings instance' })) + const second = within(screen.getByRole('region', { name: 'second settings instance' })) + + await user.click(first.getByRole('button', { name: 'start draft' })) + expect(first.getByText('dirty')).toBeInTheDocument() + expect(second.getByText('clean')).toBeInTheDocument() + + await user.click(screen.getByRole('button', { name: 'update global' })) + expect(first.getByText('global-after')).toBeInTheDocument() + expect(second.getByText('global-after')).toBeInTheDocument() + }) + + it('resets scoped workflow state when the route identity changes', async () => { + const user = userEvent.setup() + const store = createStore() + const tree = (knowledgeSpaceId: string) => ( + + + + + + ) + const rendered = render(tree('space-1')) + const instance = within(screen.getByRole('region', { name: 'settings instance' })) + + await user.click(instance.getByRole('button', { name: 'start draft' })) + expect(instance.getByText('dirty')).toBeInTheDocument() + + rendered.rerender(tree('space-2')) + + const resetInstance = within(screen.getByRole('region', { name: 'settings instance' })) + expect(resetInstance.getByText('clean')).toBeInTheDocument() + expect(resetInstance.getByText('space-2')).toBeInTheDocument() + }) +}) diff --git a/web/features/new-rag/settings/basic-information.tsx b/web/features/new-rag/settings/basic-information.tsx new file mode 100644 index 00000000000..3828e2e4cfb --- /dev/null +++ b/web/features/new-rag/settings/basic-information.tsx @@ -0,0 +1,454 @@ +'use client' + +import type { + KnowledgeFsControlSpaceVisibility, + KnowledgeFsPermissionResponse, + KnowledgeFsSpaceDetailResponse, +} from '@dify/contracts/api/console/knowledge-fs/types.gen' +import { Button } from '@langgenius/dify-ui/button' +import { cn } from '@langgenius/dify-ui/cn' +import { Form } from '@langgenius/dify-ui/form' +import { Input } from '@langgenius/dify-ui/input' +import { Textarea } from '@langgenius/dify-ui/textarea' +import { toast } from '@langgenius/dify-ui/toast' +import { useMutation } from '@tanstack/react-query' +import { useAtomValue, useSetAtom } from 'jotai' +import { useRef, useState } from 'react' +import { useTranslation } from 'react-i18next' +import AppIconPicker from '@/app/components/base/app-icon-picker' +import { SkeletonRectangle } from '@/app/components/base/skeleton' +import { consoleQuery } from '@/service/client' +import { useMembers } from '@/service/use-common' +import { + DEFAULT_KNOWLEDGE_SPACE_ICON_BACKGROUND, + KnowledgeSpaceIcon, +} from '../components/knowledge-space-icon' +import { KNOWLEDGE_DESCRIPTION_MAX_LENGTH, KNOWLEDGE_NAME_MAX_LENGTH } from '../constants' +import { KnowledgeSettingsMembers } from './members' +import { SettingsFieldRow } from './settings-field-row' +import { + invalidateKnowledgeSettingsAtom, + knowledgeSettingsPermissionsAtom, + knowledgeSettingsSpaceAtom, +} from './state/queries' +import { + finishKnowledgeSettingsBasicDraftAtom, + setKnowledgeSettingsSavePendingAtom, + startKnowledgeSettingsBasicDraftAtom, +} from './state/workflow' + +const NAME_ERROR_ID = 'knowledge-name-error' +const DESCRIPTION_ERROR_ID = 'knowledge-description-error' +type BasicSaveSlice = 'members' | 'space' + +type BasicDraft = { + description: string + icon: string + iconBackground: string + name: string + selectedMemberIds: string[] + visibility: KnowledgeFsControlSpaceVisibility +} + +function sortedIds(ids: string[]) { + return [...ids].sort().join(':') +} + +function draftFromServer( + space: KnowledgeFsSpaceDetailResponse, + permissions: KnowledgeFsPermissionResponse[], +): BasicDraft { + return { + description: space.technical_summary?.description ?? '', + icon: space.technical_summary?.icon ?? '๐Ÿ“™', + iconBackground: + space.technical_summary?.icon_background ?? DEFAULT_KNOWLEDGE_SPACE_ICON_BACKGROUND, + name: space.technical_summary?.name ?? '', + selectedMemberIds: permissions + .filter( + (permission) => + permission.status === 'active' && permission.account_id !== space.owner_account_id, + ) + .map((permission) => permission.account_id), + visibility: space.visibility, + } +} + +function draftsMatch(left: BasicDraft, right: BasicDraft) { + return ( + left.name === right.name && + left.description === right.description && + left.icon === right.icon && + left.iconBackground === right.iconBackground && + left.visibility === right.visibility && + sortedIds(left.selectedMemberIds) === sortedIds(right.selectedMemberIds) + ) +} + +function BasicInformationSkeleton() { + const { t } = useTranslation('dataset') + const { t: tSettings } = useTranslation('datasetSettings') + + return ( +
+

+ {t(($) => $['newKnowledge.settings.basicInfo'])} +

+ {[ + tSettings(($) => $['form.nameAndIcon']), + tSettings(($) => $['form.desc']), + tSettings(($) => $['form.permissions']), + ].map((label) => ( + + + + ))} +
+ ) +} + +export function BasicInformationSection() { + const { t } = useTranslation('dataset') + const { t: tCommon } = useTranslation('common') + const { t: tSettings } = useTranslation('datasetSettings') + const { t: tWorkflow } = useTranslation('workflow') + const space = useAtomValue(knowledgeSettingsSpaceAtom) + const permissions = useAtomValue(knowledgeSettingsPermissionsAtom) + const startDraft = useSetAtom(startKnowledgeSettingsBasicDraftAtom) + const finishDraft = useSetAtom(finishKnowledgeSettingsBasicDraftAtom) + const setSavePending = useSetAtom(setKnowledgeSettingsSavePendingAtom) + const invalidateSettings = useSetAtom(invalidateKnowledgeSettingsAtom) + const membersQuery = useMembers() + const [draft, setDraft] = useState() + const [nameTouched, setNameTouched] = useState(false) + const [isRefreshing, setIsRefreshing] = useState(false) + const [iconPickerOpen, setIconPickerOpen] = useState(false) + const draftRef = useRef(undefined) + const draftBaseVersionRef = useRef(undefined) + const completedSaveFingerprintsRef = useRef>>({}) + const spaceMutation = useMutation( + consoleQuery.knowledgeFs.spaces.byControlSpaceId.patch.mutationOptions(), + ) + const membersMutation = useMutation( + consoleQuery.knowledgeFs.spaces.byControlSpaceId.members.put.mutationOptions(), + ) + + if (!space) return null + if (membersQuery.isPending) return + if (membersQuery.isError) { + return ( +
+ +

+ {tCommon(($) => $['api.actionFailed'])} +

+ +
+ ) + } + + const serverDraft = draftFromServer(space, permissions) + const serverVersion = [ + space.control_space_id, + space.resource_version, + permissions + .map((permission) => `${permission.account_id}:${permission.revision}`) + .sort() + .join('|'), + ].join(':') + const current = draft ?? serverDraft + const serverConflict = + draft !== undefined && + draftBaseVersionRef.current !== undefined && + draftBaseVersionRef.current !== serverVersion + const canEdit = space.permission_keys.includes('knowledge_space_edit') + const canManageAccess = space.permission_keys.includes('knowledge_space_access_config') + const basicDirty = !draftsMatch(current, serverDraft) + const spaceDirty = + current.name !== serverDraft.name || + current.description !== serverDraft.description || + current.icon !== serverDraft.icon || + current.iconBackground !== serverDraft.iconBackground || + current.visibility !== serverDraft.visibility + const membersDirty = + sortedIds(current.selectedMemberIds) !== sortedIds(serverDraft.selectedMemberIds) + const nameInvalid = !current.name.trim() + const descriptionInvalid = + Array.from(current.description).length > KNOWLEDGE_DESCRIPTION_MAX_LENGTH + const membersInvalid = + canManageAccess && + current.visibility === 'partial_members' && + current.selectedMemberIds.length === 0 + const isSaving = spaceMutation.isPending || membersMutation.isPending || isRefreshing + const fieldsDisabled = !canEdit || isSaving + const saveDisabled = + !basicDirty || nameInvalid || descriptionInvalid || membersInvalid || serverConflict + + const updateDraft = (update: (value: BasicDraft) => BasicDraft) => { + const next = update(draftRef.current ?? current) + if (draftsMatch(next, serverDraft)) { + draftRef.current = undefined + draftBaseVersionRef.current = undefined + setDraft(undefined) + finishDraft() + return + } + draftRef.current = next + draftBaseVersionRef.current ??= serverVersion + startDraft() + setDraft(next) + } + + const resetDraft = () => { + draftRef.current = undefined + draftBaseVersionRef.current = undefined + setDraft(undefined) + setNameTouched(false) + finishDraft() + } + + const showSaveError = (error?: unknown) => + toast.error( + error instanceof Response && error.status === 403 + ? t(($) => $['newKnowledge.permissionRestricted']) + : t(($) => $['newKnowledge.settings.saveFailed']), + ) + + const performSave = async () => { + if (saveDisabled || isSaving || !canEdit) return + setSavePending({ owner: 'basic', pending: true }) + try { + const saveSlice = async ( + slice: BasicSaveSlice, + payload: unknown, + save: () => Promise, + ) => { + const fingerprint = JSON.stringify(payload) + if (completedSaveFingerprintsRef.current[slice] === fingerprint) return + await save() + completedSaveFingerprintsRef.current[slice] = fingerprint + } + + if (spaceDirty) { + const body = { + ...(current.description !== serverDraft.description + ? { description: current.description } + : {}), + ...(current.icon !== serverDraft.icon ? { icon: current.icon } : {}), + ...(current.iconBackground !== serverDraft.iconBackground + ? { icon_background: current.iconBackground } + : {}), + ...(current.name !== serverDraft.name ? { name: current.name.trim() } : {}), + ...(current.visibility !== serverDraft.visibility + ? { visibility: current.visibility } + : {}), + } + await saveSlice('space', body, () => + spaceMutation.mutateAsync({ + body, + params: { control_space_id: space.control_space_id }, + }), + ) + } + if (membersDirty && canManageAccess) { + const roleByAccountId = new Map( + permissions.map((permission) => [permission.account_id, permission.role]), + ) + const body = { + members: current.selectedMemberIds.map((accountId) => ({ + account_id: accountId, + role: roleByAccountId.get(accountId) ?? 'viewer', + })), + } + await saveSlice('members', body, () => + membersMutation.mutateAsync({ + body, + params: { control_space_id: space.control_space_id }, + }), + ) + } + completedSaveFingerprintsRef.current = {} + toast.success(tCommon(($) => $['api.actionSuccess'])) + setIsRefreshing(true) + draftBaseVersionRef.current = undefined + finishDraft() + void invalidateSettings().then( + () => { + draftRef.current = undefined + setDraft(undefined) + setIsRefreshing(false) + setSavePending({ owner: 'basic', pending: false }) + }, + () => { + setIsRefreshing(false) + setSavePending({ owner: 'basic', pending: false }) + }, + ) + } catch (error) { + setSavePending({ owner: 'basic', pending: false }) + showSaveError(error) + } + } + + return ( + <> + {serverConflict && ( +
+ + + {t(($) => $['newKnowledge.settings.serverConflict'])} + +
+ )} + +
{ + event.preventDefault() + setNameTouched(true) + void performSave() + }} + > +

+ {t(($) => $['newKnowledge.settings.basicInfo'])} +

+ + $['form.nameAndIcon'])}> +
+ +
+ $['form.name'])} + aria-describedby={nameTouched && nameInvalid ? NAME_ERROR_ID : undefined} + aria-invalid={nameTouched && nameInvalid} + autoComplete="off" + name="knowledge-name" + value={current.name} + maxLength={KNOWLEDGE_NAME_MAX_LENGTH} + disabled={fieldsDisabled} + className={cn(nameTouched && nameInvalid && 'ring-1 ring-text-destructive')} + onBlur={() => setNameTouched(true)} + onChange={(event) => + updateDraft((value) => ({ + ...value, + name: event.target.value.slice(0, KNOWLEDGE_NAME_MAX_LENGTH), + })) + } + /> + {nameTouched && nameInvalid && ( + + )} + {current.name.length >= KNOWLEDGE_NAME_MAX_LENGTH * 0.9 && ( +

+ {current.name.length} / {KNOWLEDGE_NAME_MAX_LENGTH} +

+ )} +
+
+
+ + $['form.desc'])}> +
+