From ba24745c07213cdea255855145a81ad5f32d0462 Mon Sep 17 00:00:00 2001 From: yyh <92089059+lyzno1@users.noreply.github.com> Date: Mon, 20 Jul 2026 14:30:09 +0800 Subject: [PATCH] fix(web): restore permission selector semantics (#39269) Signed-off-by: yyh --- oxlint-suppressions.json | 32 - .../base/permission-selector/index.tsx | 295 ------- .../base/permission-selector/member-item.tsx | 51 -- .../permission-selector/permission-item.tsx | 24 - .../__tests__/index.spec.tsx | 748 +++++------------- .../settings/permission-selector/index.tsx | 500 ++++++------ .../permission-selector/member-item.tsx | 38 +- .../permission-selector/permission-item.tsx | 51 +- .../__tests__/api-key-modal.spec.tsx | 45 +- .../plugin-auth/authorize/api-key-modal.tsx | 31 +- .../authorize/permission-selector.tsx | 115 +++ 11 files changed, 673 insertions(+), 1257 deletions(-) delete mode 100644 web/app/components/base/permission-selector/index.tsx delete mode 100644 web/app/components/base/permission-selector/member-item.tsx delete mode 100644 web/app/components/base/permission-selector/permission-item.tsx create mode 100644 web/app/components/plugins/plugin-auth/authorize/permission-selector.tsx diff --git a/oxlint-suppressions.json b/oxlint-suppressions.json index cd155c48af6..eadb4e44d93 100644 --- a/oxlint-suppressions.json +++ b/oxlint-suppressions.json @@ -1889,22 +1889,6 @@ "count": 2 } }, - "web/app/components/base/permission-selector/member-item.tsx": { - "jsx_a11y/click-events-have-key-events": { - "count": 1 - }, - "jsx_a11y/no-static-element-interactions": { - "count": 1 - } - }, - "web/app/components/base/permission-selector/permission-item.tsx": { - "jsx_a11y/click-events-have-key-events": { - "count": 1 - }, - "jsx_a11y/no-static-element-interactions": { - "count": 1 - } - }, "web/app/components/base/prompt-editor/index.stories.tsx": { "jsx_a11y/label-has-associated-control": { "count": 4 @@ -2956,22 +2940,6 @@ "count": 1 } }, - "web/app/components/datasets/settings/permission-selector/member-item.tsx": { - "jsx_a11y/click-events-have-key-events": { - "count": 1 - }, - "jsx_a11y/no-static-element-interactions": { - "count": 1 - } - }, - "web/app/components/datasets/settings/permission-selector/permission-item.tsx": { - "jsx_a11y/click-events-have-key-events": { - "count": 1 - }, - "jsx_a11y/no-static-element-interactions": { - "count": 1 - } - }, "web/app/components/develop/code.tsx": { "typescript/no-explicit-any": { "count": 6 diff --git a/web/app/components/base/permission-selector/index.tsx b/web/app/components/base/permission-selector/index.tsx deleted file mode 100644 index 39c75099a2f..00000000000 --- a/web/app/components/base/permission-selector/index.tsx +++ /dev/null @@ -1,295 +0,0 @@ -import type { Member } from '@/models/common' -import { Avatar } from '@langgenius/dify-ui/avatar' -import { cn } from '@langgenius/dify-ui/cn' -import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' -import { RiArrowDownSLine, RiGroup2Line, RiLock2Line } from '@remixicon/react' -import { useDebounceFn } from 'ahooks' -import { useAtomValue } from 'jotai' -import * as React from 'react' -import { useCallback, useMemo, useState } from 'react' -import { useTranslation } from 'react-i18next' -// oxlint-disable-next-line no-restricted-imports -- This legacy selector still relies on showLeftIcon/showClearIcon props from the old input. -import Input from '@/app/components/base/input' -import { userProfileAtom } from '@/context/account-state' -import { PermissionLevel } from '@/models/permission' -import MemberItem from './member-item' -import Item from './permission-item' - -type PermissionSelectorProps = { - disabled?: boolean - permission?: PermissionLevel - value: string[] - memberList: Member[] - onChange: (permission?: PermissionLevel) => void - onMemberSelect: (v: string[]) => void - /** i18n namespace for label strings (defaults to datasetSettings for backward compat) */ - i18nNamespace?: 'datasetSettings' - /** - * Hide the "Partial members" option. Useful for surfaces (e.g. plugin - * credential creation) where partial-member access is delegated to RBAC - * and the picker should only expose only_me / all_team_members. - */ - hidePartialMembers?: boolean -} - -const PermissionSelector = ({ - disabled, - permission, - value, - memberList, - onChange, - onMemberSelect, - i18nNamespace = 'datasetSettings', - hidePartialMembers = false, -}: PermissionSelectorProps) => { - const { t } = useTranslation() - const userProfile = useAtomValue(userProfileAtom) - const [open, setOpen] = useState(false) - - const [keywords, setKeywords] = useState('') - const [searchKeywords, setSearchKeywords] = useState('') - const { run: handleSearch } = useDebounceFn( - () => { - setSearchKeywords(keywords) - }, - { wait: 500 }, - ) - const handleKeywordsChange = (value: string) => { - setKeywords(value) - handleSearch() - } - const selectMember = useCallback( - (member: Member) => { - if (value.includes(member.id)) onMemberSelect(value.filter((v) => v !== member.id)) - else onMemberSelect([...value, member.id]) - }, - [value, onMemberSelect], - ) - - const selectedMembers = useMemo(() => { - return [ - userProfile, - ...memberList - .filter((member) => member.id !== userProfile.id) - .filter((member) => value.includes(member.id)), - ] - }, [userProfile, value, memberList]) - - const showMe = useMemo(() => { - return ( - (userProfile.name ?? '').includes(searchKeywords) || - (userProfile.email ?? '').includes(searchKeywords) - ) - }, [searchKeywords, userProfile]) - - const filteredMemberList = useMemo(() => { - return memberList.filter( - (member) => - (member.name.includes(searchKeywords) || member.email.includes(searchKeywords)) && - member.id !== userProfile.id && - ['owner', 'admin', 'editor', 'dataset_operator'].includes(member.role), - ) - }, [memberList, searchKeywords, userProfile]) - - const onSelectOnlyMe = useCallback(() => { - onChange(PermissionLevel.onlyMe) - setOpen(false) - }, [onChange]) - - const onSelectAllMembers = useCallback(() => { - onChange(PermissionLevel.allTeamMembers) - setOpen(false) - }, [onChange]) - - const onSelectPartialMembers = useCallback(() => { - onChange(PermissionLevel.partialMembers) - onMemberSelect([userProfile.id]) - }, [onChange, onMemberSelect, userProfile]) - - const isOnlyMe = permission === PermissionLevel.onlyMe - const isAllTeamMembers = permission === PermissionLevel.allTeamMembers - const isPartialMembers = permission === PermissionLevel.partialMembers - const selectedMemberNames = selectedMembers.map((member) => member.name).join(', ') - - return ( - -
- - } - > - {isOnlyMe && ( - <> -
- -
-
- {t(($) => $['form.permissionsOnlyMe'], { ns: i18nNamespace })} -
- - )} - {isAllTeamMembers && ( - <> -
- -
-
- {t(($) => $['form.permissionsAllMember'], { ns: i18nNamespace })} -
- - )} - {isPartialMembers && ( - <> -
- {selectedMembers.length === 1 && ( - - )} - {selectedMembers.length >= 2 && ( - <> - - - - )} -
-
- {selectedMemberNames} -
- - )} - - - -
- {/* Only me */} - - } - text={t(($) => $['form.permissionsOnlyMe'], { ns: i18nNamespace })} - onClick={onSelectOnlyMe} - isSelected={isOnlyMe} - /> - {/* All team members */} - - -
- } - text={t(($) => $['form.permissionsAllMember'], { ns: i18nNamespace })} - onClick={onSelectAllMembers} - isSelected={isAllTeamMembers} - /> - {/* Partial members */} - {!hidePartialMembers && ( - - -
- } - text={t(($) => $['form.permissionsInvitedMembers'], { ns: i18nNamespace })} - onClick={onSelectPartialMembers} - isSelected={isPartialMembers} - /> - )} - - {!hidePartialMembers && isPartialMembers && ( -
-
- handleKeywordsChange(e.target.value)} - onClear={() => handleKeywordsChange('')} - /> -
-
- {showMe && ( - - } - name={userProfile.name} - email={userProfile.email} - isSelected - isMe - i18nNamespace={i18nNamespace} - /> - )} - {filteredMemberList.map((member) => ( - - } - name={member.name} - email={member.email} - isSelected={value.includes(member.id)} - onClick={selectMember.bind(null, member)} - i18nNamespace={i18nNamespace} - /> - ))} - {!showMe && filteredMemberList.length === 0 && ( -
- {t(($) => $['form.onSearchResults'], { ns: i18nNamespace })} -
- )} -
-
- )} - - - - ) -} - -export default PermissionSelector diff --git a/web/app/components/base/permission-selector/member-item.tsx b/web/app/components/base/permission-selector/member-item.tsx deleted file mode 100644 index b8f28a697ae..00000000000 --- a/web/app/components/base/permission-selector/member-item.tsx +++ /dev/null @@ -1,51 +0,0 @@ -import { cn } from '@langgenius/dify-ui/cn' -import { RiCheckLine } from '@remixicon/react' -import * as React from 'react' -import { useTranslation } from 'react-i18next' - -type MemberItemProps = { - leftIcon: React.ReactNode - name: string - email: string - isSelected: boolean - isMe?: boolean - onClick?: () => void - i18nNamespace?: 'datasetSettings' -} - -const MemberItem = ({ - leftIcon, - name, - email, - isSelected, - isMe = false, - onClick, - i18nNamespace = 'datasetSettings', -}: MemberItemProps) => { - const { t } = useTranslation() - - return ( -
- {leftIcon} -
-
- {name} - {isMe && ( - - {t(($) => $['form.me'], { ns: i18nNamespace })} - - )} -
-
{email}
-
- {isSelected && ( - - )} -
- ) -} - -export default React.memo(MemberItem) diff --git a/web/app/components/base/permission-selector/permission-item.tsx b/web/app/components/base/permission-selector/permission-item.tsx deleted file mode 100644 index 0cea3a79707..00000000000 --- a/web/app/components/base/permission-selector/permission-item.tsx +++ /dev/null @@ -1,24 +0,0 @@ -import { RiCheckLine } from '@remixicon/react' -import * as React from 'react' - -type PermissionItemProps = { - leftIcon: React.ReactNode - text: string - onClick: () => void - isSelected: boolean -} - -const PermissionItem = ({ leftIcon, text, onClick, isSelected }: PermissionItemProps) => { - return ( -
- {leftIcon} -
{text}
- {isSelected && } -
- ) -} - -export default React.memo(PermissionItem) diff --git a/web/app/components/datasets/settings/permission-selector/__tests__/index.spec.tsx b/web/app/components/datasets/settings/permission-selector/__tests__/index.spec.tsx index cca0fe8f85d..34639684b2e 100644 --- a/web/app/components/datasets/settings/permission-selector/__tests__/index.spec.tsx +++ b/web/app/components/datasets/settings/permission-selector/__tests__/index.spec.tsx @@ -1,574 +1,228 @@ +import type { ComponentProps, ReactNode } from 'react' import type { Member } from '@/models/common' -import { fireEvent, screen, waitFor } from '@testing-library/react' +import { render, screen, waitFor, within } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { Provider } from 'jotai' +import { userProfileQueryOptions } from '@/features/account-profile/client' +import { systemFeaturesQueryOptions } from '@/features/system-features/client' +import { defaultSystemFeatures } from '@/features/system-features/config' import { DatasetPermission } from '@/models/datasets' -import { renderWithConsoleQuery } from '@/test/console/query-data' +import { createQueryAtomTestStore } from '@/test/query-atom' import PermissionSelector from '../index' -const mockConsoleState = vi.hoisted(() => ({ - userProfile: { - id: 'user-1', - name: 'Current User', - email: 'current@example.com', +const currentUser = { + id: 'user-1', + name: 'Current User', + email: 'current@example.com', + avatar: '', + avatar_url: null, + is_password_set: true, + timezone: 'UTC', +} + +const memberList: Member[] = [ + { + ...currentUser, avatar_url: '', role: 'owner', + roles: [], + last_login_at: '', + created_at: '', + status: 'active', }, -})) + { + id: 'user-2', + name: 'John Doe', + email: 'john@example.com', + avatar: '', + avatar_url: '', + role: 'admin', + roles: [], + last_login_at: '', + created_at: '', + status: 'active', + }, + { + id: 'user-3', + name: 'Jane Smith', + email: 'jane@example.com', + avatar: '', + avatar_url: '', + role: 'normal', + roles: [], + last_login_at: '', + created_at: '', + status: 'active', + }, +] -let mockIsRbacEnabled = false +const defaultProps: ComponentProps = { + permission: DatasetPermission.onlyMe, + value: ['user-1'], + memberList, + onChange: vi.fn(), + onMemberSelect: vi.fn(), +} -vi.mock('@/context/account-state', async () => { - const { createAccountStateModuleMock } = await import('@/test/console/state-fixture') +const renderSelector = ( + props: Partial> = {}, + options: { rbacEnabled?: boolean } = {}, +) => { + const { queryClient, store } = createQueryAtomTestStore() + queryClient.setQueryData(userProfileQueryOptions().queryKey, { + profile: currentUser, + meta: { currentVersion: null, currentEnv: null }, + }) + queryClient.setQueryData(systemFeaturesQueryOptions().queryKey, { + ...defaultSystemFeatures, + rbac_enabled: options.rbacEnabled ?? false, + }) + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ) - return createAccountStateModuleMock(() => mockConsoleState) -}) -vi.mock('@/context/workspace-state', async () => { - const { createWorkspaceStateModuleMock } = await import('@/test/console/state-fixture') - - return createWorkspaceStateModuleMock(() => mockConsoleState) -}) -vi.mock('@/context/permission-state', async () => { - const { createPermissionStateModuleMock } = await import('@/test/console/state-fixture') - - return createPermissionStateModuleMock(() => mockConsoleState) -}) -vi.mock('@/context/system-features-state', async () => { - const { createSystemFeaturesStateModuleMock } = await import('@/test/console/state-fixture') - - return createSystemFeaturesStateModuleMock(() => ({ - ...(() => mockConsoleState)(), - datasetRbacEnabled: (() => ({ - isRbacEnabled: mockIsRbacEnabled, - }))().isRbacEnabled, - })) -}) + return render(, { wrapper }) +} describe('PermissionSelector', () => { - const mockMemberList: Member[] = [ - { - id: 'user-1', - name: 'Current User', - email: 'current@example.com', - avatar: '', - avatar_url: '', - role: 'owner', - roles: [], - last_login_at: '', - created_at: '', - status: 'active', - }!, - { - id: 'user-2', - name: 'John Doe', - email: 'john@example.com', - avatar: '', - avatar_url: '', - role: 'admin', - roles: [], - last_login_at: '', - created_at: '', - status: 'active', - }!, - { - id: 'user-3', - name: 'Jane Smith', - email: 'jane@example.com', - avatar: '', - avatar_url: '', - role: 'editor', - roles: [], - last_login_at: '', - created_at: '', - status: 'active', - }!, - { - id: 'user-4', - name: 'Dataset Operator', - email: 'operator@example.com', - avatar: '', - avatar_url: '', - role: 'dataset_operator', - roles: [], - last_login_at: '', - created_at: '', - status: 'active', - }!, - ] - - const defaultProps = { - permission: DatasetPermission.onlyMe, - value: ['user-1'!], - memberList: mockMemberList, - onChange: vi.fn(), - onMemberSelect: vi.fn(), - } - beforeEach(() => { vi.clearAllMocks() - mockIsRbacEnabled = false }) - describe('Rendering', () => { - it('should render Only Me option when permission is onlyMe', () => { - renderWithConsoleQuery( - , - ) - expect(screen.getByText(/form\.permissionsOnlyMe/))!.toBeInTheDocument() + it('opens from its native button with the keyboard', async () => { + const user = userEvent.setup() + renderSelector() + + const trigger = screen.getByRole('button', { name: /permissionsOnlyMe/ }) + expect(trigger).toHaveAttribute('type', 'button') + + await user.tab() + expect(trigger).toHaveFocus() + await user.keyboard('{Enter}') + + const dialog = screen.getByRole('dialog', { name: /form.permissions/ }) + expect(within(dialog).getByRole('radiogroup', { name: /form.permissions/ })).toBeInTheDocument() + expect(within(dialog).getByRole('radio', { name: /permissionsOnlyMe/ })).toBeChecked() + }) + + it.each([ + ['the disabled prop', { disabled: true }, false], + ['dataset RBAC', {}, true], + ])('uses a disabled native trigger for %s', async (_, props, rbacEnabled) => { + const user = userEvent.setup() + renderSelector(props, { rbacEnabled }) + + const trigger = rbacEnabled + ? screen.getByRole('button', { name: /permissionsAccessConfig/ }) + : screen.getByRole('button', { name: /permissionsOnlyMe/ }) + expect(trigger).toBeDisabled() + + await user.click(trigger) + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + }) + + it.each([ + [DatasetPermission.onlyMe, /permissionsOnlyMe/], + [DatasetPermission.allTeamMembers, /permissionsAllMember/], + ])('selects %s and closes the popover', async (permission, optionName) => { + const user = userEvent.setup() + const onChange = vi.fn() + renderSelector({ + permission: + permission === DatasetPermission.onlyMe + ? DatasetPermission.allTeamMembers + : DatasetPermission.onlyMe, + onChange, }) - it('should render All Team Members option when permission is allTeamMembers', () => { - renderWithConsoleQuery( - , - ) - expect(screen.getByText(/form\.permissionsAllMember/))!.toBeInTheDocument() + await user.click( + screen.getByRole('button', { + name: + permission === DatasetPermission.onlyMe ? /permissionsAllMember/ : /permissionsOnlyMe/, + }), + ) + const popover = screen.getByRole('dialog', { name: /form.permissions/ }) + await user.click(within(popover).getByRole('radio', { name: optionName })) + + expect(onChange).toHaveBeenCalledWith(permission) + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()) + }) + + it('resets partial access to the current user and keeps the popover open', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + const onMemberSelect = vi.fn() + renderSelector({ onChange, onMemberSelect }) + + await user.click(screen.getByRole('button', { name: /permissionsOnlyMe/ })) + const popover = screen.getByRole('dialog', { name: /form.permissions/ }) + await user.click(within(popover).getByRole('radio', { name: /permissionsInvitedMembers/ })) + + expect(onChange).toHaveBeenCalledWith(DatasetPermission.partialMembers) + expect(onMemberSelect).toHaveBeenCalledWith(['user-1']) + expect(screen.getByRole('dialog', { name: /form.permissions/ })).toBeInTheDocument() + }) + + it('uses radio keyboard navigation without closing the popover', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + renderSelector({ onChange }) + + await user.click(screen.getByRole('button', { name: /permissionsOnlyMe/ })) + const dialog = screen.getByRole('dialog', { name: /form.permissions/ }) + const onlyMe = within(dialog).getByRole('radio', { name: /permissionsOnlyMe/ }) + onlyMe.focus() + await user.keyboard('{ArrowDown}') + + expect(onChange).toHaveBeenCalledWith(DatasetPermission.allTeamMembers) + expect(dialog).toBeInTheDocument() + }) + + it.each([ + [['user-1'], ['user-1', 'user-2']], + [['user-1', 'user-2'], ['user-1']], + ])('toggles a member using a native button', async (value, expectedValue) => { + const user = userEvent.setup() + const onMemberSelect = vi.fn() + renderSelector({ + permission: DatasetPermission.partialMembers, + value, + onMemberSelect, }) - it('should render selected member names when permission is partialMembers', () => { - renderWithConsoleQuery( - , - ) - // Should show member names - // Should show member names - expect(screen.getByTitle(/Current User/))!.toBeInTheDocument() + await user.click(screen.getByRole('button', { name: /Current User/ })) + await user.click(within(screen.getByRole('dialog')).getByRole('button', { name: /John Doe/ })) + + expect(onMemberSelect).toHaveBeenCalledWith(expectedValue) + }) + + it('filters members after the search debounce and clears the query', async () => { + const user = userEvent.setup() + renderSelector({ permission: DatasetPermission.partialMembers }) + + await user.click(screen.getByRole('button', { name: /Current User/ })) + const search = screen.getByRole('textbox', { name: /operation.search/ }) + await user.type(search, 'Jane') + + await waitFor(() => { + expect(screen.getByRole('button', { name: /Jane Smith/ })).toBeInTheDocument() + expect(screen.queryByRole('button', { name: /John Doe/ })).not.toBeInTheDocument() + }) + + await user.click(screen.getByRole('button', { name: /operation.clear/ })) + expect(search).toHaveValue('') + await waitFor(() => { + expect(screen.getByRole('button', { name: /John Doe/ })).toBeInTheDocument() }) }) - describe('Dropdown Toggle', () => { - it('should open dropdown when clicked', async () => { - renderWithConsoleQuery() + it('shows the empty state when no member matches', async () => { + const user = userEvent.setup() + renderSelector({ permission: DatasetPermission.partialMembers }) - const trigger = screen.getByText(/form\.permissionsOnlyMe/) - fireEvent.click(trigger) + await user.click(screen.getByRole('button', { name: /Current User/ })) + await user.type(screen.getByRole('textbox', { name: /operation.search/ }), 'Nobody') - await waitFor(() => { - // Should show all permission options in dropdown - expect(screen.getAllByText(/form\.permissionsOnlyMe/).length).toBeGreaterThanOrEqual(1) - }) - }) - - it('should not open dropdown when disabled', () => { - renderWithConsoleQuery() - - const trigger = screen.getByText(/form\.permissionsOnlyMe/) - fireEvent.click(trigger) - - // Dropdown should not open - only the trigger text should be visible - expect(screen.getAllByText(/form\.permissionsOnlyMe/).length).toBe(1) - }) - }) - - describe('Permission Selection', () => { - it('should call onChange with onlyMe when Only Me is selected', async () => { - const handleChange = vi.fn() - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByText(/form\.permissionsAllMember/) - fireEvent.click(trigger) - - await waitFor(() => { - const onlyMeOptions = screen.getAllByText(/form\.permissionsOnlyMe/) - fireEvent.click(onlyMeOptions[0]!) - }) - - expect(handleChange).toHaveBeenCalledWith(DatasetPermission.onlyMe) - }) - - it('should call onChange with allTeamMembers when All Team Members is selected', async () => { - const handleChange = vi.fn() - renderWithConsoleQuery() - - const trigger = screen.getByText(/form\.permissionsOnlyMe/) - fireEvent.click(trigger) - - await waitFor(() => { - const allMemberOptions = screen.getAllByText(/form\.permissionsAllMember/) - fireEvent.click(allMemberOptions[0]!) - }) - - expect(handleChange).toHaveBeenCalledWith(DatasetPermission.allTeamMembers) - }) - - it('should call onChange with partialMembers when Invited Members is selected', async () => { - const handleChange = vi.fn() - const handleMemberSelect = vi.fn() - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByText(/form\.permissionsOnlyMe/) - fireEvent.click(trigger) - - await waitFor(() => { - const invitedOptions = screen.getAllByText(/form\.permissionsInvitedMembers/) - fireEvent.click(invitedOptions[0]!) - }) - - expect(handleChange).toHaveBeenCalledWith(DatasetPermission.partialMembers) - expect(handleMemberSelect).toHaveBeenCalledWith(['user-1']) - }) - }) - - describe('Member Selection', () => { - it('should show member list when partialMembers is selected', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - await waitFor(() => { - // Should show member list - // Should show member list - expect(screen.getByText('John Doe'))!.toBeInTheDocument() - expect(screen.getByText('Jane Smith'))!.toBeInTheDocument() - }) - }) - - it('should call onMemberSelect when a member is clicked', async () => { - const handleMemberSelect = vi.fn() - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - await waitFor(() => { - const johnDoe = screen.getByText('John Doe') - fireEvent.click(johnDoe) - }) - - expect(handleMemberSelect).toHaveBeenCalledWith(['user-1', 'user-2']) - }) - - it('should deselect member when clicked again', async () => { - const handleMemberSelect = vi.fn() - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - await waitFor(() => { - const johnDoe = screen.getByText('John Doe') - fireEvent.click(johnDoe) - }) - - expect(handleMemberSelect).toHaveBeenCalledWith(['user-1']) - }) - }) - - describe('Search Functionality', () => { - it('should allow typing in search input', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - // Wait for dropdown to open - const searchInput = await screen.findByRole('textbox') - - // Type in search - fireEvent.change(searchInput, { target: { value: 'John' } }) - expect(searchInput)!.toHaveValue('John') - }) - - it('should render search input in partial members mode', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - // Wait for dropdown to open and search input to be available - const searchInput = await screen.findByRole('textbox') - expect(searchInput)!.toBeInTheDocument() - }) - - it('should filter members after debounce completes', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - // Wait for dropdown to open - const searchInput = await screen.findByRole('textbox') - - // Type in search - fireEvent.change(searchInput, { target: { value: 'John' } }) - - // Wait for debounce (500ms) + buffer - await waitFor( - () => { - expect(screen.getByText('John Doe'))!.toBeInTheDocument() - }, - { timeout: 1000 }, - ) - }) - - it('should handle clear search functionality', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - // Wait for dropdown to open - const searchInput = await screen.findByRole('textbox') - - // Type in search - fireEvent.change(searchInput, { target: { value: 'test' } }) - expect(searchInput)!.toHaveValue('test') - - const clearButton = screen.getByRole('button', { name: 'common.operation.clear' }) - fireEvent.click(clearButton) - - // After clicking clear, input should be empty - await waitFor(() => { - expect(searchInput)!.toHaveValue('') - }) - }) - - it('should filter members by email', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - // Wait for dropdown to open - const searchInput = await screen.findByRole('textbox') - - // Search by email - fireEvent.change(searchInput, { target: { value: 'john@example' } }) - - // Wait for debounce - await waitFor( - () => { - expect(screen.getByText('John Doe'))!.toBeInTheDocument() - }, - { timeout: 1000 }, - ) - }) - - it('should show no results message when search matches nothing', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - // Wait for dropdown to open - const searchInput = await screen.findByRole('textbox') - - // Search for non-existent member - fireEvent.change(searchInput, { target: { value: 'nonexistent12345' } }) - - // Wait for debounce and no results message - await waitFor( - () => { - expect(screen.getByText(/form\.onSearchResults/))!.toBeInTheDocument() - }, - { timeout: 1000 }, - ) - }) - - it('should show current user when search matches user name', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - // Wait for dropdown to open - const searchInput = await screen.findByRole('textbox') - - // Search for current user by name - partial match - fireEvent.change(searchInput, { target: { value: 'Current' } }) - - // Current user (showMe) should remain visible based on name match - // The component uses useMemo to check if userProfile.name.includes(searchKeywords) - // Current user (showMe) should remain visible based on name match - // The component uses useMemo to check if userProfile.name.includes(searchKeywords) - expect(searchInput)!.toHaveValue('Current') - // Current User label appears multiple times (trigger + member list) - expect(screen.getAllByText('Current User').length).toBeGreaterThanOrEqual(1) - }) - - it('should show current user when search matches user email', async () => { - renderWithConsoleQuery( - , - ) - - const trigger = screen.getByTitle(/Current User/) - fireEvent.click(trigger) - - // Wait for dropdown to open - const searchInput = await screen.findByRole('textbox') - - // Search for current user by email - fireEvent.change(searchInput, { target: { value: 'current@' } }) - - // The component checks userProfile.email.includes(searchKeywords) - // The component checks userProfile.email.includes(searchKeywords) - expect(searchInput)!.toHaveValue('current@') - // Current User should remain visible based on email match - expect(screen.getAllByText('Current User').length).toBeGreaterThanOrEqual(1) - }) - }) - - describe('Disabled State', () => { - it('should apply disabled styles when disabled', () => { - const { container } = renderWithConsoleQuery( - , - ) - // When disabled, the component has cursor-not-allowed! class (escaped in Tailwind) - const triggerElement = container.querySelector('[class*="cursor-not-allowed"]') - expect(triggerElement)!.toBeInTheDocument() - }) - - it('should show access config hint and remain closed when RBAC is enabled', () => { - mockIsRbacEnabled = true - - renderWithConsoleQuery(, { - systemFeatures: { - rbac_enabled: true, - }, - }) - - const trigger = screen.getByText(/form\.permissionsAccessConfig/) - fireEvent.click(trigger) - - expect(screen.getByText(/form\.permissionsAccessConfig/))!.toBeInTheDocument() - expect(screen.queryByText(/form\.permissionsOnlyMe/))!.not.toBeInTheDocument() - }) - }) - - describe('Display Variations', () => { - it('should display single avatar when only one member selected', () => { - renderWithConsoleQuery( - , - ) - - // Should display single avatar - // Should display single avatar - expect(screen.getByTitle(/Current User/))!.toBeInTheDocument() - }) - - it('should display two avatars when two or more members selected', () => { - renderWithConsoleQuery( - , - ) - - // Should display member names - // Should display member names - expect(screen.getByTitle(/Current User, John Doe/))!.toBeInTheDocument() - }) - }) - - describe('Edge Cases', () => { - it('should handle empty member list', () => { - renderWithConsoleQuery() - - expect(screen.getByText(/form\.permissionsOnlyMe/))!.toBeInTheDocument() - }) - - it('should handle member list with only current user', () => { - renderWithConsoleQuery( - , - ) - - expect(screen.getByText(/form\.permissionsOnlyMe/))!.toBeInTheDocument() - }) - - it('should only show members with allowed roles', () => { - // The component filters members by role in useMemo - // Allowed roles are: owner, admin, editor, dataset_operator - // This is tested indirectly through the memberList filtering - const memberListWithNormalUser: Member[] = [ - ...mockMemberList, - { - id: 'user-5', - name: 'Normal User', - email: 'normal@example.com', - avatar: '', - avatar_url: '', - role: 'normal', - roles: [], - last_login_at: '', - created_at: '', - status: 'active', - }, - ] - - renderWithConsoleQuery( - , - ) - - // The component renders - the filtering logic is internal - // The component renders - the filtering logic is internal - expect(screen.getByTitle(/Current User/))!.toBeInTheDocument() - }) - }) - - describe('Props', () => { - it('should update when permission prop changes', () => { - const { rerender } = renderWithConsoleQuery( - , - ) - - expect(screen.getByText(/form\.permissionsOnlyMe/))!.toBeInTheDocument() - - rerender( - , - ) - - expect(screen.getByText(/form\.permissionsAllMember/))!.toBeInTheDocument() - }) + expect(await screen.findByText(/form.onSearchResults/)).toBeInTheDocument() }) }) diff --git a/web/app/components/datasets/settings/permission-selector/index.tsx b/web/app/components/datasets/settings/permission-selector/index.tsx index c6424bda5ca..c492ce7b7d1 100644 --- a/web/app/components/datasets/settings/permission-selector/index.tsx +++ b/web/app/components/datasets/settings/permission-selector/index.tsx @@ -2,24 +2,25 @@ import type { Member } from '@/models/common' import { Avatar } from '@langgenius/dify-ui/avatar' import { cn } from '@langgenius/dify-ui/cn' import { Input } from '@langgenius/dify-ui/input' -import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' +import { Popover, PopoverContent, PopoverTitle, PopoverTrigger } from '@langgenius/dify-ui/popover' +import { RadioGroup } from '@langgenius/dify-ui/radio' import { useDebounceFn } from 'ahooks' import { useAtomValue } from 'jotai' -import { useCallback, useMemo, useState } from 'react' +import { useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import { userProfileAtom } from '@/context/account-state' import { datasetRbacEnabledAtom } from '@/context/system-features-state' import { DatasetPermission } from '@/models/datasets' import MemberItem from './member-item' -import Item from './permission-item' +import PermissionItem from './permission-item' -type RoleSelectorProps = { +type PermissionSelectorProps = { disabled?: boolean permission?: DatasetPermission value: string[] memberList: Member[] onChange: (permission?: DatasetPermission) => void - onMemberSelect: (v: string[]) => void + onMemberSelect: (value: string[]) => void } const PermissionSelector = ({ @@ -29,283 +30,272 @@ const PermissionSelector = ({ memberList, onChange, onMemberSelect, -}: RoleSelectorProps) => { +}: PermissionSelectorProps) => { const { t } = useTranslation() const userProfile = useAtomValue(userProfileAtom) const isRbacEnabled = useAtomValue(datasetRbacEnabledAtom) - const [open, setOpen] = useState(false) - const [keywords, setKeywords] = useState('') const [searchKeywords, setSearchKeywords] = useState('') const { run: handleSearch } = useDebounceFn( - () => { - setSearchKeywords(keywords) + (nextKeywords: string) => { + setSearchKeywords(nextKeywords) }, { wait: 500 }, ) - const handleKeywordsChange = (value: string) => { - setKeywords(value) - handleSearch() + const handleKeywordsChange = (nextKeywords: string) => { + setKeywords(nextKeywords) + handleSearch(nextKeywords) + } + const selectMember = (member: Member) => { + if (value.includes(member.id)) onMemberSelect(value.filter((id) => id !== member.id)) + else onMemberSelect([...value, member.id]) } - const selectMember = useCallback( - (member: Member) => { - if (value.includes(member.id)) onMemberSelect(value.filter((v) => v !== member.id)) - else onMemberSelect([...value, member.id]) - }, - [value, onMemberSelect], - ) - const selectedMembers = useMemo(() => { - return [ + const selectedMembers = useMemo( + () => [ userProfile, - ...memberList - .filter((member) => member.id !== userProfile.id) - .filter((member) => value.includes(member.id)), - ] - }, [userProfile, value, memberList]) - - const showMe = useMemo(() => { - return userProfile.name.includes(searchKeywords) || userProfile.email.includes(searchKeywords) - }, [searchKeywords, userProfile]) - - const filteredMemberList = useMemo(() => { - return memberList.filter( - (member) => - (member.name.includes(searchKeywords) || member.email.includes(searchKeywords)) && - member.id !== userProfile.id, - ) - }, [memberList, searchKeywords, userProfile]) - - const onSelectOnlyMe = useCallback(() => { - onChange(DatasetPermission.onlyMe) - setOpen(false) - }, [onChange]) - - const onSelectAllMembers = useCallback(() => { - onChange(DatasetPermission.allTeamMembers) - setOpen(false) - }, [onChange]) - - const onSelectPartialMembers = useCallback(() => { - onChange(DatasetPermission.partialMembers) - onMemberSelect([userProfile.id]) - }, [onChange, onMemberSelect, userProfile]) + ...memberList.filter((member) => member.id !== userProfile.id && value.includes(member.id)), + ], + [memberList, userProfile, value], + ) + const filteredMemberList = useMemo( + () => + memberList.filter( + (member) => + member.id !== userProfile.id && + (member.name.includes(searchKeywords) || member.email.includes(searchKeywords)), + ), + [memberList, searchKeywords, userProfile.id], + ) const isOnlyMe = permission === DatasetPermission.onlyMe const isAllTeamMembers = permission === DatasetPermission.allTeamMembers const isPartialMembers = permission === DatasetPermission.partialMembers + const showMe = + userProfile.name.includes(searchKeywords) || userProfile.email.includes(searchKeywords) const selectedMemberNames = selectedMembers.map((member) => member.name).join(', ') - const isDisabledByRBAC = isRbacEnabled - const isDisabled = disabled || isDisabledByRBAC + const isDisabledByRbac = isRbacEnabled + const isDisabled = disabled || isDisabledByRbac + const permissionLabel = t(($) => $['form.permissions'], { ns: 'datasetSettings' }) return ( - { - if (isDisabled) return - setOpen(nextOpen) - }} - > -
- - {isDisabledByRBAC && ( - <> -
- -
-
- {t(($) => $['form.permissionsAccessConfig'], { ns: 'datasetSettings' })} -
- - )} - {!isDisabledByRBAC && isOnlyMe && ( - <> -
- -
-
- {t(($) => $['form.permissionsOnlyMe'], { ns: 'datasetSettings' })} -
- - )} - {!isDisabledByRBAC && isAllTeamMembers && ( - <> -
- -
-
- {t(($) => $['form.permissionsAllMember'], { ns: 'datasetSettings' })} -
- - )} - {!isDisabledByRBAC && isPartialMembers && ( - <> -
- {selectedMembers.length === 1 && ( - - )} - {selectedMembers.length >= 2 && ( - <> - - - - )} -
-
- {selectedMemberNames} -
- - )} - + + + {isDisabledByRbac && ( + <> +
+
- } - /> - -
-
- {/* Only me */} - + {t(($) => $['form.permissionsAccessConfig'], { ns: 'datasetSettings' })} +
+ + )} + {!isDisabledByRbac && isOnlyMe && ( + <> +
+ +
+
+ {t(($) => $['form.permissionsOnlyMe'], { ns: 'datasetSettings' })} +
+ + )} + {!isDisabledByRbac && isAllTeamMembers && ( + <> +
+
+
+ {t(($) => $['form.permissionsAllMember'], { ns: 'datasetSettings' })} +
+ + )} + {!isDisabledByRbac && isPartialMembers && ( + <> +
+ {selectedMembers.length === 1 && ( + + )} + {selectedMembers.length >= 2 && ( + <> - } - text={t(($) => $['form.permissionsOnlyMe'], { ns: 'datasetSettings' })} - onClick={onSelectOnlyMe} - isSelected={isOnlyMe} - /> - {/* All team members */} - - -
- } - text={t(($) => $['form.permissionsAllMember'], { ns: 'datasetSettings' })} - onClick={onSelectAllMembers} - isSelected={isAllTeamMembers} - /> - {/* Partial members */} - - -
- } - text={t(($) => $['form.permissionsInvitedMembers'], { ns: 'datasetSettings' })} - onClick={onSelectPartialMembers} - isSelected={isPartialMembers} - /> + + + )}
- {isPartialMembers && ( -
-
-
- - $['operation.search'], { ns: 'common' }) || ''} - onChange={(e) => handleKeywordsChange(e.target.value)} - /> - {!!keywords && ( - - )} -
+
+ {selectedMemberNames} +
+ + )} +
+
+ {showMe && ( + + } + name={userProfile.name} + email={userProfile.email} + isSelected + isMe + /> + )} + {filteredMemberList.map((member) => ( + + } + name={member.name} + email={member.email} + isSelected={value.includes(member.id)} + onClick={() => selectMember(member)} + /> + ))} + {!showMe && filteredMemberList.length === 0 && ( +
+ {t(($) => $['form.onSearchResults'], { ns: 'datasetSettings' })} +
+ )} +
+
+ )} + + ) } diff --git a/web/app/components/datasets/settings/permission-selector/member-item.tsx b/web/app/components/datasets/settings/permission-selector/member-item.tsx index 30c44d81f68..2a2e4d2fa06 100644 --- a/web/app/components/datasets/settings/permission-selector/member-item.tsx +++ b/web/app/components/datasets/settings/permission-selector/member-item.tsx @@ -1,10 +1,9 @@ +import type { ReactNode } from 'react' import { cn } from '@langgenius/dify-ui/cn' -import { RiCheckLine } from '@remixicon/react' -import * as React from 'react' import { useTranslation } from 'react-i18next' type MemberItemProps = { - leftIcon: React.ReactNode + leftIcon: ReactNode name: string email: string isSelected: boolean @@ -22,13 +21,10 @@ const MemberItem = ({ }: MemberItemProps) => { const { t } = useTranslation() - return ( -
+ const content = ( + <> {leftIcon} -
+
{name} {isMe && ( @@ -40,10 +36,28 @@ const MemberItem = ({
{email}
{isSelected && ( - +
+ + ) + + if (isMe) { + return
{content}
+ } + + return ( + ) } -export default React.memo(MemberItem) +export default MemberItem diff --git a/web/app/components/datasets/settings/permission-selector/permission-item.tsx b/web/app/components/datasets/settings/permission-selector/permission-item.tsx index 0cea3a79707..9c017b30257 100644 --- a/web/app/components/datasets/settings/permission-selector/permission-item.tsx +++ b/web/app/components/datasets/settings/permission-selector/permission-item.tsx @@ -1,24 +1,49 @@ -import { RiCheckLine } from '@remixicon/react' -import * as React from 'react' +import type { ReactNode } from 'react' +import type { DatasetPermission } from '@/models/datasets' +import { PopoverClose } from '@langgenius/dify-ui/popover' +import { RadioItem } from '@langgenius/dify-ui/radio' type PermissionItemProps = { - leftIcon: React.ReactNode + value: DatasetPermission + leftIcon: ReactNode text: string - onClick: () => void isSelected: boolean + closeOnSelect?: boolean } -const PermissionItem = ({ leftIcon, text, onClick, isSelected }: PermissionItemProps) => { - return ( -
+const className = + 'flex w-full touch-manipulation cursor-pointer items-center gap-x-1 rounded-lg border-none bg-transparent px-2 py-1 text-left outline-hidden hover:bg-state-base-hover focus-visible:ring-2 focus-visible:ring-state-accent-solid' + +const PermissionItem = ({ + value, + leftIcon, + text, + isSelected, + closeOnSelect = false, +}: PermissionItemProps) => { + const content = ( + <> {leftIcon}
{text}
- {isSelected && } -
+ {isSelected && ( +
)} diff --git a/web/app/components/plugins/plugin-auth/authorize/permission-selector.tsx b/web/app/components/plugins/plugin-auth/authorize/permission-selector.tsx new file mode 100644 index 00000000000..9292ebad2e0 --- /dev/null +++ b/web/app/components/plugins/plugin-auth/authorize/permission-selector.tsx @@ -0,0 +1,115 @@ +import { Avatar } from '@langgenius/dify-ui/avatar' +import { cn } from '@langgenius/dify-ui/cn' +import { + Popover, + PopoverClose, + PopoverContent, + PopoverTitle, + PopoverTrigger, +} from '@langgenius/dify-ui/popover' +import { RadioGroup, RadioItem } from '@langgenius/dify-ui/radio' +import { useAtomValue } from 'jotai' +import { useTranslation } from 'react-i18next' +import { userProfileAtom } from '@/context/account-state' +import { PermissionLevel } from '@/models/permission' + +export type CredentialPermission = + | typeof PermissionLevel.onlyMe + | typeof PermissionLevel.allTeamMembers + +type PermissionSelectorProps = { + disabled?: boolean + permission: CredentialPermission + onChange: (permission: CredentialPermission) => void +} + +const optionClassName = + 'flex w-full touch-manipulation cursor-pointer items-center gap-x-1 rounded-lg border-none bg-transparent px-2 py-1 text-left outline-hidden hover:bg-state-base-hover focus-visible:ring-2 focus-visible:ring-state-accent-solid' + +const PermissionSelector = ({ disabled, permission, onChange }: PermissionSelectorProps) => { + const { t } = useTranslation() + const userProfile = useAtomValue(userProfileAtom) + const isOnlyMe = permission === PermissionLevel.onlyMe + const isAllTeamMembers = permission === PermissionLevel.allTeamMembers + const permissionLabel = t(($) => $['auth.whoCanUse'], { ns: 'plugin' }) + + return ( + + + {isOnlyMe && ( + <> +
+ +
+
+ {t(($) => $['form.permissionsOnlyMe'], { ns: 'datasetSettings' })} +
+ + )} + {isAllTeamMembers && ( + <> +
+
+
+ {t(($) => $['form.permissionsAllMember'], { ns: 'datasetSettings' })} +
+ + )} + + + {permissionLabel} + + value={permission} + onValueChange={onChange} + aria-label={permissionLabel} + className="flex-col items-stretch gap-0 p-1" + > + value={PermissionLevel.onlyMe} />} + className={optionClassName} + > + +
+ {t(($) => $['form.permissionsOnlyMe'], { ns: 'datasetSettings' })} +
+ {isOnlyMe && ( + + value={PermissionLevel.allTeamMembers} />} + className={optionClassName} + > +
+
+
+ {t(($) => $['form.permissionsAllMember'], { ns: 'datasetSettings' })} +
+ {isAllTeamMembers && ( + + + + + ) +} + +export default PermissionSelector