fix(editor): Reintroduce user deletion actions in the members table in Users and Project settings page (#19604)

This commit is contained in:
Csaba Tuncsik
2025-09-18 17:15:05 +02:00
committed by GitHub
parent f0b48733ac
commit bcedf5c76f
22 changed files with 483 additions and 191 deletions

View File

@@ -3,7 +3,6 @@ import { MainSidebar } from './sidebar/main-sidebar';
import { SettingsSidebar } from './sidebar/settings-sidebar';
import { WorkflowPage } from './workflow';
import { WorkflowsPage } from './workflows';
import { getVisiblePopper } from '../utils';
const workflowPage = new WorkflowPage();
const workflowsPage = new WorkflowsPage();
@@ -62,8 +61,8 @@ export class SettingsUsersPage extends BasePage {
}
},
opedDeleteDialog: (email: string) => {
this.getters.userRoleSelect(email).find('button').should('be.visible').click();
getVisiblePopper().find('span').contains('Remove user').click();
this.getters.userActionsToggle(email).should('be.visible').click();
this.getters.deleteUserAction().click();
this.getters.confirmDeleteModal().should('be.visible');
},
};

View File

@@ -0,0 +1,86 @@
import { createTestingPinia } from '@pinia/testing';
import { screen } from '@testing-library/vue';
import userEvent from '@testing-library/user-event';
import { vi } from 'vitest';
import type { UserAction } from '@n8n/design-system';
import ProjectMembersActionsCell from '@/components/Projects/ProjectMembersActionsCell.vue';
import { createComponentRenderer } from '@/__tests__/render';
import type { ProjectMemberData } from '@/types/projects.types';
vi.mock('@n8n/design-system', async (importOriginal) => {
const original = await importOriginal<object>();
return {
...original,
N8nActionToggle: {
name: 'N8nActionToggle',
props: {
actions: { type: Array, required: true },
},
emits: ['action'],
template: `
<div>
<button
v-for="a in actions"
:key="a.value"
:data-test-id="'action-' + a.value"
@click="$emit('action', a.value)"
>
{{ a.label }}
</button>
</div>
`,
},
};
});
const baseMember: ProjectMemberData = {
id: '1',
firstName: 'John',
lastName: 'Doe',
email: 'john@example.com',
role: 'project:editor',
};
const removeAction: UserAction<ProjectMemberData> = {
value: 'remove',
label: 'Remove user',
};
let renderComponent: ReturnType<typeof createComponentRenderer>;
describe('ProjectMembersActionsCell', () => {
beforeEach(() => {
renderComponent = createComponentRenderer(ProjectMembersActionsCell, {
pinia: createTestingPinia(),
});
});
afterEach(() => {
vi.clearAllMocks();
});
it('renders actions when allowed and emits on click', async () => {
const props = {
data: baseMember,
actions: [removeAction],
};
const { emitted } = renderComponent({ props });
const user = userEvent.setup();
await user.click(screen.getByTestId('action-remove'));
expect(emitted()).toHaveProperty('action');
expect(emitted().action[0]).toEqual([{ action: 'remove', userId: '1' }]);
});
it('does not render when actions list is empty', () => {
const props = {
data: baseMember,
actions: [],
};
const { container } = renderComponent({ props });
expect(container.querySelector('button')).toBeNull();
});
// Visibility filtering is handled by ProjectMembersTable now
});

View File

@@ -0,0 +1,27 @@
<script lang="ts" setup>
import type { ProjectMemberData } from '@/types/projects.types';
import type { UserAction } from '@n8n/design-system';
const props = defineProps<{
data: ProjectMemberData;
actions: Array<UserAction<ProjectMemberData>>;
}>();
const emit = defineEmits<{
action: [value: { action: string; userId: string }];
}>();
const onAction = (action: string) => {
emit('action', { action, userId: props.data.id });
};
</script>
<template>
<N8nActionToggle
v-if="props.actions.length > 0"
placement="bottom"
:actions="props.actions"
theme="dark"
@action="onAction"
/>
</template>

View File

@@ -26,8 +26,8 @@ vi.mock('@n8n/design-system', async (importOriginal) => {
</div>
<ul data-test-id="dropdown-menu">
<li v-for="item in items" :key="item.id">
<button
:data-test-id="'action-' + item.id"
<button
:data-test-id="'action-' + item.id"
:disabled="item.disabled"
@click="$emit('select', item.id)"
>
@@ -73,9 +73,9 @@ vi.mock('element-plus', async (importOriginal) => {
emits: ['update:model-value'],
template: `
<label>
<input
type="radio"
:value="label"
<input
type="radio"
:value="label"
:checked="modelValue === label"
:disabled="disabled"
@change="$emit('update:model-value', label)"
@@ -130,7 +130,6 @@ const mockActions: Array<ActionDropdownItem<string>> = [
{ id: 'project:admin', label: 'Admin' },
{ id: 'project:editor', label: 'Editor' },
{ id: 'project:viewer', label: 'Viewer', disabled: true },
{ id: 'remove', label: 'Remove User', divided: true },
];
let renderComponent: ReturnType<typeof createComponentRenderer>;
@@ -224,11 +223,10 @@ describe('ProjectMembersRoleCell', () => {
it('should pass actions to dropdown component', () => {
renderComponent();
// Check that all action items are rendered
// Check that all role action items are rendered
expect(screen.getByTestId('action-project:admin')).toBeInTheDocument();
expect(screen.getByTestId('action-project:editor')).toBeInTheDocument();
expect(screen.getByTestId('action-project:viewer')).toBeInTheDocument();
expect(screen.getByTestId('action-remove')).toBeInTheDocument();
});
});
@@ -248,21 +246,6 @@ describe('ProjectMembersRoleCell', () => {
]);
});
it('should emit update:role when remove action is selected', async () => {
const { emitted } = renderComponent();
const user = userEvent.setup();
await user.click(screen.getByTestId('action-remove'));
expect(emitted()).toHaveProperty('update:role');
expect(emitted()['update:role'][0]).toEqual([
{
role: 'remove',
userId: '123',
},
]);
});
it('should emit with correct userId from data prop', async () => {
const customData = { ...mockMemberData, id: 'custom-user-id' };
const { emitted } = renderComponent({
@@ -321,13 +304,6 @@ describe('ProjectMembersRoleCell', () => {
expect(radioInputs).toHaveLength(3); // admin, editor, viewer (remove is not a radio)
});
it('should render remove option without radio button', () => {
renderComponent();
expect(screen.getByTestId('action-remove')).toBeInTheDocument();
expect(screen.getByText('Remove User')).toBeInTheDocument();
});
it('should handle disabled actions correctly', () => {
renderComponent();

View File

@@ -9,11 +9,11 @@ import type { ProjectMemberData } from '@/types/projects.types';
const props = defineProps<{
data: ProjectMemberData;
roles: Record<ProjectRole, { label: string; desc: string }>;
actions: Array<ActionDropdownItem<ProjectRole | 'remove'>>;
actions: Array<ActionDropdownItem<ProjectRole>>;
}>();
const emit = defineEmits<{
'update:role': [payload: { role: ProjectRole | 'remove'; userId: string }];
'update:role': [payload: { role: ProjectRole; userId: string }];
}>();
const selectedRole = ref<string>(props.data.role);
@@ -31,7 +31,7 @@ const roleLabel = computed(() =>
: selectedRole.value,
);
const onActionSelect = (role: ProjectRole | 'remove') => {
const onActionSelect = (role: ProjectRole) => {
emit('update:role', {
role,
userId: props.data.id,
@@ -54,11 +54,7 @@ const onActionSelect = (role: ProjectRole | 'remove') => {
</button>
</template>
<template #menuItem="item">
<N8nText v-if="item.id === 'remove'" color="text-dark" :class="$style.removeUser">{{
item.label
}}</N8nText>
<ElRadio
v-else
:model-value="selectedRole"
:label="item.id"
:disabled="item.disabled"
@@ -98,8 +94,5 @@ const onActionSelect = (role: ProjectRole | 'remove') => {
}
}
.removeUser {
display: block;
padding: var(--spacing-2xs) var(--spacing-l);
}
/* removeUser style no longer used since remove moved to actions menu */
</style>

View File

@@ -94,6 +94,19 @@ vi.mock('@/components/Projects/ProjectMembersRoleCell.vue', () => ({
},
}));
// Mock ProjectMembersActionsCell component to avoid dependence on design-system ActionToggle
vi.mock('@/components/Projects/ProjectMembersActionsCell.vue', () => ({
default: {
name: 'ProjectMembersActionsCell',
props: {
data: { type: Object, required: true },
actions: { type: Array, required: true },
},
emits: ['action'],
template: '<div :data-test-id="`actions-cell-` + data.id"></div>',
},
}));
const mockMembers: ProjectMemberData[] = [
{
id: '1',

View File

@@ -7,11 +7,12 @@ import {
N8nDataTableServer,
N8nText,
type ActionDropdownItem,
type UserAction,
} from '@n8n/design-system';
import type { TableHeader, TableOptions } from '@n8n/design-system/components/N8nDataTableServer';
import ProjectMembersRoleCell from '@/components/Projects/ProjectMembersRoleCell.vue';
import ProjectMembersActionsCell from '@/components/Projects/ProjectMembersActionsCell.vue';
import type { UsersInfoProps } from '@n8n/design-system/components/N8nUserInfo/UserInfo.vue';
import { isProjectRole } from '@/utils/typeGuards';
import type { ProjectMemberData } from '@/types/projects.types';
const i18n = useI18n();
@@ -21,11 +22,13 @@ const props = defineProps<{
loading?: boolean;
currentUserId?: string;
projectRoles: Array<{ slug: string; displayName: string; licensed: boolean }>;
actions?: Array<UserAction<ProjectMemberData>>;
}>();
const emit = defineEmits<{
'update:options': [payload: TableOptions];
'update:role': [payload: { role: ProjectRole | 'remove'; userId: string }];
'update:role': [payload: { role: ProjectRole; userId: string }];
action: [value: { action: string; userId: string }];
}>();
const tableOptions = defineModel<TableOptions>('tableOptions', {
@@ -50,6 +53,16 @@ const headers = ref<Array<TableHeader<ProjectMemberData>>>([
key: 'role',
disableSort: true,
},
{
title: '',
key: 'actions',
align: 'end',
width: 46,
disableSort: true,
value() {
return;
},
},
]);
const roles = computed<Record<ProjectRole, { label: string; desc: string }>>(() => ({
@@ -71,26 +84,23 @@ const roles = computed<Record<ProjectRole, { label: string; desc: string }>>(()
},
}));
const roleActions = computed<Array<ActionDropdownItem<ProjectRole | 'remove'>>>(() => [
const roleActions = computed<Array<ActionDropdownItem<ProjectRole>>>(() => [
...props.projectRoles.map((role) => ({
id: role.slug as ProjectRole,
label: role.displayName,
disabled: !role.licensed,
})),
{
id: 'remove',
label: i18n.baseText('projects.settings.table.row.removeUser'),
divided: true,
},
]);
const canUpdateRole = (member: ProjectMemberData): boolean => {
// User cannot change their own role or remove themselves
return member.id !== props.currentUserId;
const canUpdateRole = (member: ProjectMemberData): boolean => member.id !== props.currentUserId;
const onRoleChange = ({ role, userId }: { role: ProjectRole; userId: string }) => {
emit('update:role', { role, userId });
};
const onRoleChange = ({ role, userId }: { role: ProjectRole | 'remove'; userId: string }) => {
emit('update:role', { role, userId });
const filterActions = (member: ProjectMemberData) => {
if (member.id === props.currentUserId || member.role === 'project:personalOwner') return [];
return (props.actions ?? []).filter((action) => action.guard?.(member) ?? true);
};
</script>
@@ -120,9 +130,14 @@ const onRoleChange = ({ role, userId }: { role: ProjectRole | 'remove'; userId:
:actions="roleActions"
@update:role="onRoleChange"
/>
<N8nText v-else color="text-dark">{{
isProjectRole(item.role) ? roles[item.role]?.label || item.role : item.role
}}</N8nText>
<N8nText v-else color="text-dark">{{ roles[item.role]?.label ?? item.role }}</N8nText>
</template>
<template #[`item.actions`]="{ item }">
<ProjectMembersActionsCell
:data="item"
:actions="filterActions(item)"
@action="$emit('action', $event)"
/>
</template>
</N8nDataTableServer>
</div>

View File

@@ -38,12 +38,6 @@ describe('SettingsUsersActionsCell', () => {
vi.clearAllMocks();
});
it('should not render action toggle for an owner', () => {
const props = { data: { ...baseUser, isOwner: true }, actions: mockActions };
const { container } = renderComponent({ props });
expect(container.firstChild).toBeEmptyDOMElement();
});
it('should not render action toggle if there are no actions', () => {
const props = { data: baseUser, actions: [] };
const { container } = renderComponent({ props });

View File

@@ -23,7 +23,7 @@ const onUserAction = (action: string) => {
<template>
<div>
<N8nActionToggle
v-if="!props.data.isOwner && props.data.signInType !== 'ldap' && props.actions.length > 0"
v-if="props.data.signInType !== 'ldap' && props.actions.length > 0"
placement="bottom"
:actions="props.actions"
theme="dark"

View File

@@ -55,10 +55,9 @@ const mockRoles = {
[ROLE.Default]: { label: 'Default', desc: '' },
};
const mockActions: Array<ActionDropdownItem<Role | 'delete'>> = [
const mockActions: Array<ActionDropdownItem<Role>> = [
{ id: ROLE.Member, label: 'Member' },
{ id: ROLE.Admin, label: 'Admin' },
{ id: 'delete', label: 'Delete User', divided: true },
];
let renderComponent: ReturnType<typeof createComponentRenderer>;
@@ -106,14 +105,4 @@ describe('SettingsUsersRoleCell', () => {
expect(emitted()).toHaveProperty('update:role');
expect(emitted()['update:role'][0]).toEqual([{ role: ROLE.Admin, userId: '1' }]);
});
it('should emit "update:role" with "delete" when delete action is clicked', async () => {
const { emitted } = renderComponent();
const user = userEvent.setup();
await user.click(screen.getByTestId('action-delete'));
expect(emitted()).toHaveProperty('update:role');
expect(emitted()['update:role'][0]).toEqual([{ role: 'delete', userId: '1' }]);
});
});

View File

@@ -6,18 +6,18 @@ import { type ActionDropdownItem, N8nActionDropdown, N8nIcon } from '@n8n/design
const props = defineProps<{
data: UsersList['items'][number];
roles: Record<Role, { label: string; desc: string }>;
actions: Array<ActionDropdownItem<Role | 'delete'>>;
actions: Array<ActionDropdownItem<Role>>;
}>();
const emit = defineEmits<{
'update:role': [payload: { role: Role | 'delete'; userId: string }];
'update:role': [payload: { role: Role; userId: string }];
}>();
const selectedRole = ref<Role>(props.data.role ?? ROLE.Default);
const isEditable = computed(() => props.data.role !== ROLE.Owner);
const roleLabel = computed(() => props.roles[selectedRole.value].label);
const onActionSelect = (role: Role | 'delete') => {
const onActionSelect = (role: Role) => {
emit('update:role', {
role,
userId: props.data.id,
@@ -41,11 +41,7 @@ const onActionSelect = (role: Role | 'delete') => {
</button>
</template>
<template #menuItem="item">
<N8nText v-if="item.id === 'delete'" color="text-dark" :class="$style.removeUser">{{
item.label
}}</N8nText>
<ElRadio
v-else
:model-value="selectedRole"
:label="item.id"
@update:model-value="selectedRole = item.id as Role"

View File

@@ -184,18 +184,6 @@ describe('SettingsUsersTable', () => {
expect(emitted()['update:role'][0]).toEqual([{ role: 'global:admin', userId: '2' }]);
});
it('should emit "action" with "delete" payload when delete is selected from role change', () => {
const { emitted } = renderComponent();
emitters.settingsUsersRoleCell.emit('update:role', { role: 'delete', userId: '2' });
// It should not emit 'update:role'
expect(emitted()).not.toHaveProperty('update:role');
// It should emit 'action'
expect(emitted()).toHaveProperty('action');
expect(emitted().action[0]).toEqual([{ action: 'delete', userId: '2' }]);
});
it('should render role as plain text when user lacks permission', () => {
hasPermission.mockReturnValue(false);
renderComponent();

View File

@@ -109,7 +109,7 @@ const roles = computed<Record<Role, { label: string; desc: string }>>(() => ({
},
[ROLE.Default]: { label: i18n.baseText('auth.roles.default'), desc: '' },
}));
const roleActions = computed<Array<ActionDropdownItem<Role | 'delete'>>>(() => [
const roleActions = computed<Array<ActionDropdownItem<Role>>>(() => [
{
id: ROLE.Member,
label: i18n.baseText('auth.roles.member'),
@@ -118,11 +118,6 @@ const roleActions = computed<Array<ActionDropdownItem<Role | 'delete'>>>(() => [
id: ROLE.Admin,
label: i18n.baseText('auth.roles.admin'),
},
{
id: 'delete',
label: i18n.baseText('settings.users.table.row.deleteUser'),
divided: true,
},
]);
const canUpdateRole = computed((): boolean => {
@@ -138,11 +133,7 @@ const filterActions = (user: UsersList['items'][number]) => {
};
const onRoleChange = ({ role, userId }: { role: string; userId: string }) => {
if (role === 'delete') {
emit('action', { action: 'delete', userId });
} else {
emit('update:role', { role: role as Role, userId });
}
emit('update:role', { role: role as Role, userId });
};
</script>

View File

@@ -0,0 +1,127 @@
import { setActivePinia, createPinia } from 'pinia';
import { reactive } from 'vue';
import { vi } from 'vitest';
import { useProjectsStore } from '@/stores/projects.store';
import * as projectsApi from '@/api/projects.api';
import type { Project, ProjectListItem } from '@/types/projects.types';
import { ProjectTypes } from '@/types/projects.types';
import type { ProjectRole, Scope } from '@n8n/permissions';
// Minimal router mock to satisfy useRoute usage in the store
vi.mock('vue-router', () => ({
useRoute: () => reactive({ params: {}, query: {}, path: '' }),
}));
vi.mock('@/api/projects.api', () => ({
updateProject: vi.fn(),
getProject: vi.fn(),
}));
// Typed mocked facade for the API module
const mockedProjectsApi = vi.mocked(projectsApi);
describe('useProjectsStore.updateProject (partial payloads)', () => {
beforeEach(() => {
setActivePinia(createPinia());
vi.clearAllMocks();
});
const makeStoreWithProject = () => {
const store = useProjectsStore();
// Seed myProjects and currentProject with proper typings
const now = new Date().toISOString();
const listItem: ProjectListItem = {
id: 'p1',
name: 'A',
description: 'desc',
icon: { type: 'icon', value: 'layers' },
type: ProjectTypes.Team,
createdAt: now,
updatedAt: now,
role: 'project:admin' as ProjectRole,
scopes: [] as Scope[],
};
store.myProjects = [listItem];
const project: Project = {
id: 'p1',
name: 'A',
description: 'desc',
icon: { type: 'icon', value: 'layers' },
type: ProjectTypes.Team,
createdAt: now,
updatedAt: now,
relations: [
{ id: 'u1', email: 'x@y.z', firstName: 'X', lastName: 'Y', role: 'project:editor' },
],
scopes: ['project:read' as Scope],
};
store.currentProject = project;
return store;
};
it('updates name only when provided', async () => {
const store = makeStoreWithProject();
mockedProjectsApi.updateProject.mockResolvedValue(undefined);
await store.updateProject('p1', { name: 'B' });
expect(mockedProjectsApi.updateProject).toHaveBeenCalledWith(expect.anything(), 'p1', {
name: 'B',
});
expect(store.myProjects[0].name).toBe('B');
expect(store.myProjects[0].description).toBe('desc');
expect(store.currentProject?.name).toBe('B');
expect(store.currentProject?.description).toBe('desc');
// No relations refetch
expect(mockedProjectsApi.getProject).not.toHaveBeenCalled();
});
it('updates description only when provided', async () => {
const store = makeStoreWithProject();
mockedProjectsApi.updateProject.mockResolvedValue(undefined);
await store.updateProject('p1', { description: 'new-desc' });
expect(mockedProjectsApi.updateProject).toHaveBeenCalledWith(expect.anything(), 'p1', {
description: 'new-desc',
});
expect(store.myProjects[0].description).toBe('new-desc');
expect(store.myProjects[0].name).toBe('A');
expect(store.currentProject?.description).toBe('new-desc');
expect(store.currentProject?.name).toBe('A');
expect(mockedProjectsApi.getProject).not.toHaveBeenCalled();
});
it('refetches project when relations are provided and does not touch name/icon/description', async () => {
const store = makeStoreWithProject();
mockedProjectsApi.updateProject.mockResolvedValue(undefined);
const now = new Date().toISOString();
const serverProject: Project = {
id: 'p1',
name: 'SERVER',
description: 'SERVER',
icon: { type: 'icon', value: 'layers' },
type: ProjectTypes.Team,
createdAt: now,
updatedAt: now,
relations: [],
scopes: ['project:read' as Scope],
};
mockedProjectsApi.getProject.mockResolvedValue(serverProject);
await store.updateProject('p1', { relations: [{ userId: 'u2', role: 'project:viewer' }] });
// Ensure only relations were sent
expect(mockedProjectsApi.updateProject).toHaveBeenCalledWith(expect.anything(), 'p1', {
relations: [{ userId: 'u2', role: 'project:viewer' }],
});
// Refetch invoked
expect(mockedProjectsApi.getProject).toHaveBeenCalledWith(expect.anything(), 'p1');
// Local name/description remain unchanged eagerly; currentProject then replaced by getProject
expect(store.myProjects[0].name).toBe('A');
expect(store.myProjects[0].description).toBe('desc');
expect(store.currentProject?.name).toBe('SERVER');
expect(store.currentProject?.description).toBe('SERVER');
});
});

View File

@@ -18,7 +18,6 @@ import { useUsersStore } from '@/stores/users.store';
import { getResourcePermissions } from '@n8n/permissions';
import type { CreateProjectDto, UpdateProjectDto } from '@n8n/api-types';
import { useSourceControlStore } from '@/stores/sourceControl.store';
import type { IconOrEmoji } from '@n8n/design-system/components/N8nIconPicker/types';
export const useProjectsStore = defineStore(STORES.PROJECTS, () => {
const route = useRoute();
@@ -118,24 +117,22 @@ export const useProjectsStore = defineStore(STORES.PROJECTS, () => {
return newProject;
};
const updateProject = async (
id: Project['id'],
projectData: Required<UpdateProjectDto>,
): Promise<void> => {
const updateProject = async (id: Project['id'], projectData: UpdateProjectDto): Promise<void> => {
await projectsApi.updateProject(rootStore.restApiContext, id, projectData);
const projectIndex = myProjects.value.findIndex((p) => p.id === id);
const { name, icon, description } = projectData;
const { name, icon, description, relations } = projectData;
if (projectIndex !== -1) {
myProjects.value[projectIndex].name = name;
myProjects.value[projectIndex].icon = icon as IconOrEmoji;
myProjects.value[projectIndex].description = description;
if (typeof name !== 'undefined') myProjects.value[projectIndex].name = name;
if (typeof icon !== 'undefined') myProjects.value[projectIndex].icon = icon;
if (typeof description !== 'undefined')
myProjects.value[projectIndex].description = description;
}
if (currentProject.value) {
currentProject.value.name = name;
currentProject.value.icon = icon as IconOrEmoji;
currentProject.value.description = description;
if (typeof name !== 'undefined') currentProject.value.name = name;
if (typeof icon !== 'undefined') currentProject.value.icon = icon;
if (typeof description !== 'undefined') currentProject.value.description = description;
}
if (projectData.relations) {
if (relations) {
await getProject(id);
}
};

View File

@@ -71,8 +71,9 @@ vi.mock('@/components/Projects/ProjectMembersTable.vue', () => ({
data: { type: Object, required: true },
currentUserId: { type: String, required: false },
projectRoles: { type: Array, required: true },
actions: { type: Array, required: false },
},
emits: ['update:options', 'update:role'],
emits: ['update:options', 'update:role', 'action'],
setup(_, { emit }) {
addEmitter('projectMembersTable', emit as unknown as Emitter);
return {};
@@ -491,27 +492,31 @@ describe('ProjectSettings', () => {
);
});
it('marks member for removal, filters it out, and saves without it with telemetry', async () => {
it('removes member immediately and shows success toast with telemetry', async () => {
const updateSpy = vi.spyOn(projectsStore, 'updateProject').mockResolvedValue(undefined);
const { getByTestId, queryByTestId } = renderComponent();
await nextTick();
expect(getByTestId('members-count').textContent).toBe('1');
// Mark for removal
emitters.projectMembersTable.emit('update:role', { userId: '1', role: 'remove' });
// Remove member via inline action
emitters.projectMembersTable.emit('action', { action: 'remove', userId: '1' });
await nextTick();
// Pending removal should hide members table container entirely
expect(queryByTestId('members-count')).toBeNull();
await userEvent.click(getByTestId('project-settings-save-button'));
await nextTick();
// Members list container should now be hidden
expect(queryByTestId('members-count')).toBeNull();
expect(updateSpy).toHaveBeenCalled();
const payload = updateSpy.mock.calls[0][1];
expect(payload.relations).toEqual([]);
expect(mockShowMessage).toHaveBeenCalled();
expect(mockTrack).toHaveBeenCalledWith(
'User removed member from project',
expect.objectContaining({ project_id: '123', target_user_id: '1' }),
);
// Save should not re-add removed user
await userEvent.click(getByTestId('project-settings-save-button'));
await nextTick();
expect(queryByTestId('members-count')).toBeNull();
});
it('prevents saving when invalid role selected', async () => {

View File

@@ -22,6 +22,8 @@ import { useDocumentTitle } from '@/composables/useDocumentTitle';
import ProjectHeader from '@/components/Projects/ProjectHeader.vue';
import { isIconOrEmoji, type IconOrEmoji } from '@n8n/design-system/components/N8nIconPicker/types';
import type { TableOptions } from '@n8n/design-system/components/N8nDataTableServer';
import type { UserAction } from '@n8n/design-system';
import type { ProjectMemberData } from '@/types/projects.types';
import { isProjectRole } from '@/utils/typeGuards';
type FormDataDiff = {
@@ -53,6 +55,8 @@ const formData = ref<Pick<Project, 'name' | 'description' | 'relations'>>({
description: '',
relations: [],
});
// Used to skip one watcher sync after targeted server updates (e.g., immediate removal)
const suppressNextSync = ref(false);
const projectRoleTranslations = ref<{ [key: string]: string }>({
'project:viewer': i18n.baseText('projects.settings.role.viewer'),
@@ -77,8 +81,6 @@ const membersTableState = ref<TableOptions>({
],
});
const pendingRemovals = ref<Set<string>>(new Set());
const usersList = computed(() =>
usersStore.allUsers.filter((user: IUser) => {
const isAlreadySharedWithUser = (formData.value.relations || []).find(
@@ -102,6 +104,15 @@ const projectRoles = computed(() =>
);
const firstLicensedRole = computed(() => projectRoles.value.find((role) => role.licensed)?.slug);
const projectMembersActions = computed<Array<UserAction<ProjectMemberData>>>(() => [
{
label: i18n.baseText('projects.settings.table.row.removeUser'),
value: 'remove',
guard: (member) =>
member.id !== usersStore.currentUser?.id && member.role !== 'project:personalOwner',
},
]);
const onAddMember = (userId: string) => {
isDirty.value = true;
const user = usersStore.usersById[userId];
@@ -117,20 +128,7 @@ const onAddMember = (userId: string) => {
formData.value.relations.push(relation);
};
const onUpdateMemberRole = async ({
userId,
role,
}: {
userId: string;
role: ProjectRole | 'remove';
}) => {
if (role === 'remove') {
// Mark for pending removal instead of immediate removal
pendingRemovals.value.add(userId);
isDirty.value = true;
return;
}
const onUpdateMemberRole = async ({ userId, role }: { userId: string; role: ProjectRole }) => {
if (!projectsStore.currentProject) {
return;
}
@@ -148,9 +146,6 @@ const onUpdateMemberRole = async ({
try {
await projectsStore.updateProject(projectsStore.currentProject.id, {
name: formData.value.name ?? '',
icon: projectIcon.value,
description: formData.value.description ?? '',
relations: formData.value.relations.map((r: ProjectRelation) => ({
userId: r.id,
role: r.role,
@@ -177,13 +172,59 @@ const onTextInput = () => {
isDirty.value = true;
};
async function onRemoveMember(userId: string) {
const current = projectsStore.currentProject;
if (!current) return;
const idx = formData.value.relations.findIndex((r) => r.id === userId);
if (idx === -1) return;
// Optimistically remove from UI
const removed = formData.value.relations.splice(idx, 1)[0];
// Only persist if user existed in server relations
const isPersisted = current.relations.some((r) => r.id === userId);
if (!isPersisted) return;
try {
// Prevent next sync from wiping unsaved edits
suppressNextSync.value = true;
await projectsStore.updateProject(current.id, {
relations: current.relations
.filter((r) => r.id !== userId)
.map((r) => ({ userId: r.id, role: r.role })),
});
toast.showMessage({
type: 'success',
title: i18n.baseText('projects.settings.member.removed.title'),
});
telemetry.track('User removed member from project', {
project_id: current.id,
target_user_id: userId,
});
} catch (error) {
formData.value.relations.splice(idx, 0, removed);
toast.showError(error, i18n.baseText('projects.settings.save.error.title'));
}
}
const onMembersListAction = async ({ action, userId }: { action: string; userId: string }) => {
switch (action) {
case 'remove':
await onRemoveMember(userId);
break;
default:
// no-op for now; future actions can be added here
break;
}
};
const onCancel = () => {
formData.value.relations = projectsStore.currentProject?.relations
? deepCopy(projectsStore.currentProject.relations)
: [];
formData.value.name = projectsStore.currentProject?.name ?? '';
formData.value.description = projectsStore.currentProject?.description ?? '';
pendingRemovals.value.clear();
isDirty.value = false;
};
@@ -265,24 +306,15 @@ const updateProject = async () => {
throw new Error('Invalid role selected for this project.');
}
// Remove pending removal members from relations before saving
const relationsToSave = formData.value.relations.filter(
(r: ProjectRelation) => !pendingRemovals.value.has(r.id),
);
await projectsStore.updateProject(projectsStore.currentProject.id, {
name: formData.value.name ?? '',
icon: projectIcon.value,
description: formData.value.description ?? '',
relations: relationsToSave.map((r: ProjectRelation) => ({
relations: formData.value.relations.map((r: ProjectRelation) => ({
userId: r.id,
role: r.role,
})),
});
// After successful save, actually remove pending members from formData
formData.value.relations = relationsToSave;
pendingRemovals.value.clear();
isDirty.value = false;
} catch (error) {
toast.showError(error, i18n.baseText('projects.settings.save.error.title'));
@@ -356,9 +388,14 @@ const onIconUpdated = async () => {
});
};
// Skip one sync after targeted updates (e.g. removal) to preserve unsaved edits
watch(
() => projectsStore.currentProject,
async () => {
if (suppressNextSync.value) {
suppressNextSync.value = false;
return;
}
formData.value.name = projectsStore.currentProject?.name ?? '';
formData.value.description = projectsStore.currentProject?.description ?? '';
formData.value.relations = projectsStore.currentProject?.relations
@@ -375,30 +412,27 @@ watch(
// Add users property to the relation objects,
// So that the table has access to the full user data
// Filter out users marked for pending removal
const relationUsers = computed(() =>
formData.value.relations
.filter((relation: ProjectRelation) => !pendingRemovals.value.has(relation.id))
.map((relation: ProjectRelation) => {
const user = usersStore.usersById[relation.id];
// Ensure type safety for UI display while preserving original role in formData
const safeRole: ProjectRole = isProjectRole(relation.role) ? relation.role : 'project:viewer';
formData.value.relations.map((relation: ProjectRelation) => {
const user = usersStore.usersById[relation.id];
// Ensure type safety for UI display while preserving original role in formData
const safeRole: ProjectRole = isProjectRole(relation.role) ? relation.role : 'project:viewer';
if (!user) {
return {
...relation,
role: safeRole,
firstName: null,
lastName: null,
email: null,
};
}
if (!user) {
return {
...user,
...relation,
role: safeRole,
firstName: null,
lastName: null,
email: null,
};
}),
}
return {
...user,
...relation,
role: safeRole,
};
}),
);
const membersTableData = computed(() => ({
@@ -526,8 +560,10 @@ onMounted(() => {
:data="filteredMembersData"
:current-user-id="usersStore.currentUser?.id"
:project-roles="projectRoles"
:actions="projectMembersActions"
@update:options="onUpdateMembersTableOptions"
@update:role="onUpdateMemberRole"
@action="onMembersListAction"
/>
</div>
</fieldset>

View File

@@ -18,16 +18,32 @@ import { useUsersStore } from '@/stores/users.store';
import { useUIStore } from '@/stores/ui.store';
import { useSettingsStore } from '@/stores/settings.store';
import { useSSOStore } from '@/stores/sso.store';
import * as permissions from '@/utils/rbac/permissions';
const { emitters, addEmitter } = useEmitters<'settingsUsersTable'>();
// Mock the SettingsUsersTable component to emit events
// Mock the SettingsUsersTable component to emit events and render an accessible actions list per user
vi.mock('@/components/SettingsUsers/SettingsUsersTable.vue', () => ({
default: defineComponent({
name: 'SettingsUsersTableStub',
props: {
data: { type: Object, required: false },
actions: { type: Array, required: false },
},
setup(_, { emit }) {
addEmitter('settingsUsersTable', emit);
},
template: '<div />',
template: `
<div data-test-id="settings-users-table">
<div v-for="u in (data?.items || [])" :key="u.id" :data-test-id="'user-' + u.id">
<ul :aria-label="'Actions for user ' + u.id" role="list" :data-test-id="'actions-for-' + u.id">
<li v-for="a in (actions || []).filter(act => !act?.guard || act.guard(u))" :key="a.value" role="listitem">
<button type="button" :data-test-id="'action-' + a.value + '-' + u.id">{{ a.label || a.value }}</button>
</li>
</ul>
</div>
</div>
`,
}),
}));
@@ -292,6 +308,19 @@ describe('SettingsUsersView', () => {
expect(getByTestId('settings-users-table')).toBeInTheDocument();
});
it('should include delete action and pass guard for non-current user', () => {
const spy = vi
.spyOn(permissions, 'hasPermission')
.mockImplementation((features: string[]) => features.includes('rbac'));
renderComponent();
// ensure the member (id: 2) has Delete action rendered in the accessible list
const actionsList = screen.getByTestId('actions-for-2');
expect(actionsList).toBeInTheDocument();
expect(screen.getByTestId('action-delete-2')).toBeInTheDocument();
spy.mockRestore();
});
describe('search functionality', () => {
it('should handle empty search', async () => {
renderComponent();

View File

@@ -80,6 +80,13 @@ const usersListActions = computed((): Array<UserAction<IUser>> => {
guard: (user) =>
usersStore.usersLimitNotReached && !user.firstName && settingsStore.isSmtpSetup,
},
{
label: i18n.baseText('settings.users.actions.delete'),
value: 'delete',
guard: (user) =>
hasPermission(['rbac'], { rbac: { scope: 'user:delete' } }) &&
user.id !== usersStore.currentUserId,
},
{
label: i18n.baseText('settings.users.actions.copyPasswordResetLink'),
value: 'copyPasswordResetLink',

View File

@@ -18,4 +18,21 @@ export abstract class BasePage {
protected async clickButtonByName(name: string) {
await this.page.getByRole('button', { name }).click();
}
protected async waitForRestResponse(
url: string | RegExp,
method?: 'GET' | 'POST' | 'PATCH' | 'PUT' | 'DELETE',
) {
if (typeof url === 'string') {
return await this.page.waitForResponse((res) => {
const matches = res.url().includes(url);
return matches && (method ? res.request().method() === method : true);
});
}
return await this.page.waitForResponse((res) => {
const matches = url.test(res.url());
return matches && (method ? res.request().method() === method : true);
});
}
}

View File

@@ -15,7 +15,10 @@ export class ProjectSettingsPage extends BasePage {
}
async clickSaveButton() {
await this.clickButtonByName('Save');
await Promise.all([
this.waitForRestResponse(/\/rest\/projects\/[^/]+$/, 'PATCH'),
this.clickButtonByName('Save'),
]);
}
async clickCancelButton() {
@@ -76,4 +79,8 @@ export class ProjectSettingsPage extends BasePage {
const select = this.page.getByTestId('project-members-select');
await expect(select).toBeVisible();
}
async waitForProjectSettingsRestResponse() {
await this.waitForRestResponse(/\/rest\/projects\/[^/]+$/, 'GET');
}
}

View File

@@ -112,7 +112,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Verify basic project settings form elements are visible (inner controls)
await expect(n8n.projectSettings.getNameInput()).toBeVisible();
@@ -137,7 +137,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Update project name
const newName = 'Updated Project Name';
@@ -166,7 +166,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
const table = n8n.projectSettings.getMembersTable();
@@ -192,7 +192,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Current user (owner) should not have a role dropdown
const currentUserRow = n8n.page.locator('tbody tr').first();
@@ -208,7 +208,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Verify search input is visible
const searchInput = n8n.page.getByTestId('project-members-search');
@@ -232,7 +232,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Clear the project name (required field)
await n8n.projectSettings.fillProjectName('');
@@ -254,7 +254,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Initially, save and cancel buttons should be disabled (no changes)
await expect(n8n.page.getByTestId('project-settings-save-button')).toBeDisabled();
@@ -284,7 +284,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Scroll to bottom to see delete section
await n8n.page
@@ -308,7 +308,7 @@ test.describe('Projects', () => {
// Navigate to project settings
await n8n.page.goto(`/projects/${projectId}/settings`);
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Update project details
const projectName = 'Persisted Project Name';
@@ -325,7 +325,7 @@ test.describe('Projects', () => {
// Reload the page
await n8n.page.reload();
await n8n.page.waitForLoadState('domcontentloaded');
await n8n.projectSettings.waitForProjectSettingsRestResponse();
// Verify data persisted
await n8n.projectSettings.expectProjectNameValue(projectName);