fix(editor): Reassign users when deleting a custom instance role (#34642)

This commit is contained in:
Yuliia Pominchuk
2026-07-21 14:02:57 +00:00
committed by GitHub
parent 29f2facb48
commit fa0abd8ca8
20 changed files with 950 additions and 89 deletions
+1
View File
@@ -187,6 +187,7 @@ export {
} from './roles/list-role-mapping-rule-query.dto';
export { RoleListQueryDto } from './roles/role-list-query.dto';
export { RoleGetQueryDto } from './roles/role-get-query.dto';
export { RoleDeleteQueryDto } from './roles/role-delete-query.dto';
export {
RoleAssignmentsResponseDto,
type RoleProjectAssignment,
@@ -0,0 +1,11 @@
import { z } from 'zod';
import { Z } from '../../zod-class';
/**
* Query DTO for deleting a role. When the role still has users assigned,
* `reassignRoleSlug` selects the role those users are moved to before deletion.
*/
export class RoleDeleteQueryDto extends Z.class({
reassignRoleSlug: z.string().min(1).optional(),
}) {}
@@ -196,6 +196,35 @@ export class RoleRepository extends Repository<Role> {
}
}
/**
* Reassign every user currently holding `role` to `toSlug`, then delete `role`,
* all within a single transaction so users are never left orphaned by a partial failure.
*/
async reassignUsersAndRemove(role: Role, toSlug: string) {
await this.manager.transaction(async (trx) => {
if (role.roleType === 'global') {
await trx
.createQueryBuilder()
.update(User)
.set({ role: { slug: toSlug } })
.where('roleSlug = :fromSlug', { fromSlug: role.slug })
.execute();
} else if (role.roleType === 'project') {
await trx
.createQueryBuilder()
.update(ProjectRelation)
.set({ role: { slug: toSlug } })
.where('role = :fromSlug', { fromSlug: role.slug })
.execute();
}
const result = await trx.delete(Role, { slug: role.slug });
if (result.affected !== 1) {
throw new Error(`Failed to delete role "${role.slug}"`);
}
});
}
private async updateEntityWithManager(
entityManager: EntityManager,
slug: string,
@@ -69,13 +69,25 @@ describe('RoleController', () => {
slug: 'custom-editor',
} as Role);
await controller.deleteRole(request, mock(), 'custom-editor');
await controller.deleteRole(request, mock(), 'custom-editor', mock());
expect(eventService.emit).toHaveBeenCalledWith('custom-role-deleted', {
userId: '123',
roleSlug: 'custom-editor',
});
});
it('should pass the reassignment role through to the service', async () => {
const request = managerRequest();
roleService.getRole.mockResolvedValue({ roleType: 'global' } as Role);
roleService.removeCustomRole.mockResolvedValue({ slug: 'custom-editor' } as Role);
await controller.deleteRole(request, mock(), 'custom-editor', {
reassignRoleSlug: 'global:member',
});
expect(roleService.removeCustomRole).toHaveBeenCalledWith('custom-editor', 'global:member');
});
});
});
});
@@ -1,6 +1,7 @@
import {
CreateRoleDto,
RoleAssignmentsResponseDto,
RoleDeleteQueryDto,
RoleGetQueryDto,
RoleListQueryDto,
RoleMembersResponseDto,
@@ -132,10 +133,11 @@ export class RoleController {
req: AuthenticatedRequest,
_res: Response,
@Param('slug') slug: string,
@Query query: RoleDeleteQueryDto,
): Promise<RoleDTO> {
const role = await this.roleService.getRole(slug);
this.assertCanManageRoleType(req.user, role.roleType);
const result = await this.roleService.removeCustomRole(slug);
const result = await this.roleService.removeCustomRole(slug, query.reassignRoleSlug);
this.eventService.emit('custom-role-deleted', {
userId: req.user.id,
roleSlug: result.slug,
+23 -3
View File
@@ -139,7 +139,7 @@ export class RoleService {
return { members };
}
async removeCustomRole(slug: string) {
async removeCustomRole(slug: string, reassignRoleSlug?: string) {
const role = await this.roleRepository.findBySlug(slug);
if (!role) {
throw new NotFoundError('Role not found');
@@ -150,7 +150,7 @@ export class RoleService {
// Check if any users is globally or project assigned to the role
const usersWithRole = await this.roleRepository.countUsersWithRole(role);
if (usersWithRole > 0) {
if (usersWithRole > 0 && !reassignRoleSlug) {
throw new BadRequestError('Cannot delete role assigned to users');
}
@@ -161,7 +161,11 @@ export class RoleService {
throw new BadRequestError(`Cannot delete role: ${blockers.join('; ')}`);
}
await this.roleRepository.removeBySlug(slug);
if (usersWithRole > 0 && reassignRoleSlug) {
await this.reassignUsersAndRemoveRole(role, reassignRoleSlug);
} else {
await this.roleRepository.removeBySlug(slug);
}
// Invalidate cache after role deletion
await this.roleCacheService.invalidateCache();
@@ -169,6 +173,22 @@ export class RoleService {
return this.dbRoleToRoleDTO(role);
}
private async reassignUsersAndRemoveRole(role: Role, reassignRoleSlug: string) {
if (reassignRoleSlug === role.slug) {
throw new BadRequestError('Cannot reassign users to the role being deleted');
}
const reassignRole = await this.roleRepository.findBySlug(reassignRoleSlug);
if (!reassignRole) {
throw new BadRequestError(`Reassignment role "${reassignRoleSlug}" does not exist`);
}
if (reassignRole.roleType !== role.roleType) {
throw new BadRequestError('Reassignment role must be of the same type as the deleted role');
}
await this.roleRepository.reassignUsersAndRemove(role, reassignRoleSlug);
}
private async resolveScopes(
scopeSlugs: string[] | undefined,
roleType: 'project' | 'global',
@@ -1128,6 +1128,32 @@ describe('RoleController', () => {
expect(roleService.removeCustomRole).toHaveBeenCalledTimes(1);
});
it('should forward the reassignRoleSlug query param to the service', async () => {
//
// ARRANGE
//
const roleSlug = 'global:test-role';
roleService.removeCustomRole.mockResolvedValue({
slug: roleSlug,
displayName: 'Deleted Role',
description: null,
systemRole: false,
roleType: 'global',
scopes: [],
licensed: true,
});
//
// ACT
//
await ownerAgent.delete(`/roles/${roleSlug}?reassignRoleSlug=global:member`).expect(200);
//
// ASSERT
//
expect(roleService.removeCustomRole).toHaveBeenCalledWith(roleSlug, 'global:member');
});
it('should handle service errors gracefully', async () => {
//
// ARRANGE
@@ -1371,6 +1371,113 @@ describe('RoleService', () => {
);
});
it('should reassign globally assigned users to another role, then delete', async () => {
//
// ARRANGE
//
const testScopes = await createTestScopes();
const roleInUse = await createCustomRoleWithScopes([testScopes.readScope], {
displayName: 'Role In Use',
roleType: 'global',
});
const targetRole = await createRole({
displayName: 'Target Role',
roleType: 'global',
systemRole: false,
});
const user = await createMember();
user.role = roleInUse;
await userRepository.save(user);
//
// ACT
//
const result = await roleService.removeCustomRole(roleInUse.slug, targetRole.slug);
//
// ASSERT
//
expect(result.slug).toBe(roleInUse.slug);
expect(await roleRepository.findBySlug(roleInUse.slug)).toBeNull();
const reassignedUser = await userRepository.findOneOrFail({
where: { id: user.id },
relations: ['role'],
});
expect(reassignedUser.role.slug).toBe(targetRole.slug);
});
it('should throw when the reassignment role does not exist', async () => {
const testScopes = await createTestScopes();
const roleInUse = await createCustomRoleWithScopes([testScopes.readScope], {
displayName: 'Role In Use',
roleType: 'global',
});
const user = await createMember();
user.role = roleInUse;
await userRepository.save(user);
await expect(
roleService.removeCustomRole(roleInUse.slug, 'global:does-not-exist'),
).rejects.toThrow('Reassignment role "global:does-not-exist" does not exist');
// Role is preserved when reassignment fails.
expect(await roleRepository.findBySlug(roleInUse.slug)).not.toBeNull();
});
it('should throw when the reassignment role is of a different type', async () => {
const testScopes = await createTestScopes();
const roleInUse = await createCustomRoleWithScopes([testScopes.readScope], {
displayName: 'Global Role In Use',
roleType: 'global',
});
const projectTarget = await createCustomRoleWithScopes([testScopes.readScope], {
displayName: 'Project Target',
roleType: 'project',
});
const user = await createMember();
user.role = roleInUse;
await userRepository.save(user);
await expect(
roleService.removeCustomRole(roleInUse.slug, projectTarget.slug),
).rejects.toThrow('Reassignment role must be of the same type as the deleted role');
});
it('should throw when reassigning users to the role being deleted', async () => {
const testScopes = await createTestScopes();
const roleInUse = await createCustomRoleWithScopes([testScopes.readScope], {
displayName: 'Role In Use',
roleType: 'global',
});
const user = await createMember();
user.role = roleInUse;
await userRepository.save(user);
await expect(roleService.removeCustomRole(roleInUse.slug, roleInUse.slug)).rejects.toThrow(
'Cannot reassign users to the role being deleted',
);
});
it('should ignore the reassignment role when no users are assigned', async () => {
const targetRole = await createRole({
displayName: 'Target Role',
roleType: 'global',
systemRole: false,
});
const unusedRole = await createRole({
displayName: 'Unused Role',
roleType: 'global',
systemRole: false,
});
const result = await roleService.removeCustomRole(unusedRole.slug, targetRole.slug);
expect(result.slug).toBe(unusedRole.slug);
expect(await roleRepository.findBySlug(unusedRole.slug)).toBeNull();
});
describe('when referenced by an SSO role mapping rule', () => {
let roleMappingRuleRepository: RoleMappingRuleRepository;
@@ -2911,6 +2911,13 @@
"roles.instance.action.fetch.error": "Error fetching role",
"roles.instance.action.delete.warning": "Deleting this role is permanent and cannot be undone.",
"roles.instance.action.delete.button": "Delete role",
"roles.instance.action.delete.reassign.title": "Delete {roleName} role",
"roles.instance.action.delete.reassign.userCount": "{count} user | {count} users",
"roles.instance.action.delete.reassign.description": "are currently assigned to this role. Choose a role to reassign them to before deleting.",
"roles.instance.action.delete.reassign.label": "Reassign users to",
"roles.instance.action.delete.reassign.placeholder": "Select a role",
"roles.instance.action.delete.reassign.confirm": "Delete and reassign users",
"roles.instance.action.delete.reassign.confirmWithRole": "Delete role and reassign users to {roleName}",
"roles.instance.action.create.success": "Role created successfully",
"roles.instance.action.create.error": "Couldn't create role",
"roles.create.validation.nameMinLength": "Enter a name of at least {min} characters",
@@ -33,8 +33,13 @@ export const updateRole = async (
return await makeRestApiRequest(context, 'PATCH', `/roles/${slug}`, body);
};
export const deleteRole = async (context: IRestApiContext, slug: string): Promise<Role> => {
return await makeRestApiRequest(context, 'DELETE', `/roles/${slug}`);
export const deleteRole = async (
context: IRestApiContext,
slug: string,
reassignRoleSlug?: string,
): Promise<Role> => {
const query = reassignRoleSlug ? `?reassignRoleSlug=${encodeURIComponent(reassignRoleSlug)}` : '';
return await makeRestApiRequest(context, 'DELETE', `/roles/${slug}${query}`);
};
export const getRoleAssignments = async (
@@ -101,8 +101,8 @@ export const useRolesStore = defineStore('roles', () => {
return await rolesApi.getRoleBySlug(rootStore.restApiContext, payload);
};
const deleteRole = async (slug: string): Promise<Role> => {
return await rolesApi.deleteRole(rootStore.restApiContext, slug);
const deleteRole = async (slug: string, reassignRoleSlug?: string): Promise<Role> => {
return await rolesApi.deleteRole(rootStore.restApiContext, slug, reassignRoleSlug);
};
const updateRole = async (slug: string, body: UpdateRoleDto): Promise<Role> => {
@@ -0,0 +1,201 @@
import { createTestingPinia } from '@pinia/testing';
import { mount } from '@vue/test-utils';
import { defineComponent } from 'vue';
import { useRolesStore } from '@/app/stores/roles.store';
import { mockedStore, waitAllPromises, type MockedStore } from '@/__tests__/utils';
import type { Role } from '@n8n/permissions';
import { useRoleDeletion } from './useRoleDeletion';
const mockShowError = vi.fn();
const mockShowMessage = vi.fn();
const mockConfirm = vi.fn();
const mockTrack = vi.fn();
const mockPush = vi.fn();
vi.mock('@/app/composables/useToast', () => ({
useToast: () => ({ showError: mockShowError, showMessage: mockShowMessage }),
}));
vi.mock('@/app/composables/useMessage', () => ({
useMessage: () => ({ confirm: mockConfirm }),
}));
vi.mock('@/app/composables/useTelemetry', () => ({
useTelemetry: () => ({ track: mockTrack }),
}));
vi.mock('vue-router', async () => {
const actual = await vi.importActual('vue-router');
return {
...actual,
useRouter: () => ({ push: mockPush }),
};
});
function withSetup() {
let result!: ReturnType<typeof useRoleDeletion>;
const comp = defineComponent({
setup() {
result = useRoleDeletion();
return () => null;
},
});
const wrapper = mount(comp);
return { result, unmount: () => wrapper.unmount() };
}
const mockRole: Role = {
displayName: 'Support',
slug: 'support',
description: 'A custom instance role',
scopes: ['user:read'],
licensed: true,
systemRole: false,
roleType: 'global',
usedByUsers: 3,
};
let rolesStore: MockedStore<typeof useRolesStore>;
describe('useRoleDeletion', () => {
beforeEach(() => {
vi.clearAllMocks();
createTestingPinia();
rolesStore = mockedStore(useRolesStore);
});
describe('reassign-and-delete flow', () => {
it('opens the reassign modal instead of deleting immediately when a global role has assigned users', async () => {
rolesStore.fetchRoleBySlug.mockResolvedValue({ ...mockRole, usedByUsers: 3 });
const { result, unmount } = withSetup();
await result.requestDelete(mockRole, { roleType: 'global' });
expect(result.reassignState.value).toEqual({
role: mockRole,
userCount: 3,
roleType: 'global',
redirectTo: undefined,
});
expect(rolesStore.deleteRole).not.toHaveBeenCalled();
unmount();
});
it('calls deleteRole with both the deleted role slug and the chosen reassignment slug', async () => {
rolesStore.fetchRoleBySlug.mockResolvedValue({ ...mockRole, usedByUsers: 3 });
rolesStore.deleteRole.mockResolvedValue(mockRole);
rolesStore.roles.global = [mockRole];
const { result, unmount } = withSetup();
await result.requestDelete(mockRole, { roleType: 'global' });
result.confirmReassignDelete('global:member');
await waitAllPromises();
expect(rolesStore.deleteRole).toHaveBeenCalledWith('support', 'global:member');
unmount();
});
it('clears the pending reassign state as soon as a target role is selected', async () => {
rolesStore.fetchRoleBySlug.mockResolvedValue({ ...mockRole, usedByUsers: 3 });
rolesStore.deleteRole.mockResolvedValue(mockRole);
rolesStore.roles.global = [mockRole];
const { result, unmount } = withSetup();
await result.requestDelete(mockRole, { roleType: 'global' });
expect(result.reassignState.value).not.toBeNull();
result.confirmReassignDelete('global:member');
expect(result.reassignState.value).toBeNull();
unmount();
});
it('on success: removes the role locally, refetches roles, shows a success toast, tracks telemetry, and redirects', async () => {
rolesStore.fetchRoleBySlug.mockResolvedValue({ ...mockRole, usedByUsers: 3 });
rolesStore.deleteRole.mockResolvedValue(mockRole);
rolesStore.roles.global = [mockRole];
rolesStore.fetchRoles.mockResolvedValue();
const redirectTo = { name: 'roles-settings' };
const { result, unmount } = withSetup();
await result.requestDelete(mockRole, { roleType: 'global', redirectTo });
result.confirmReassignDelete('global:member');
await waitAllPromises();
expect(rolesStore.roles.global).toEqual([]);
// Reassignment shifts the target role's member count, so roles are refetched.
expect(rolesStore.fetchRoles).toHaveBeenCalled();
expect(mockShowMessage).toHaveBeenCalledWith({
title: 'Role deleted',
type: 'success',
});
expect(mockTrack).toHaveBeenCalledWith(
'User successfully deleted role',
expect.objectContaining({ role_id: mockRole.slug, role_name: mockRole.displayName }),
);
expect(mockPush).toHaveBeenCalledWith(redirectTo);
unmount();
});
it('on failure: shows an error toast and does not remove the role locally', async () => {
rolesStore.fetchRoleBySlug.mockResolvedValue({ ...mockRole, usedByUsers: 3 });
const error = new Error('boom');
rolesStore.deleteRole.mockRejectedValue(error);
rolesStore.roles.global = [mockRole];
const { result, unmount } = withSetup();
await result.requestDelete(mockRole, { roleType: 'global' });
result.confirmReassignDelete('global:member');
await waitAllPromises();
expect(rolesStore.roles.global).toEqual([mockRole]);
expect(mockShowError).toHaveBeenCalledWith(error, 'Error deleting role');
expect(mockShowMessage).not.toHaveBeenCalled();
unmount();
});
});
describe('standard delete flow (no assigned users / project roles)', () => {
it('deletes immediately without a reassignment slug once confirmed', async () => {
const projectRole = { ...mockRole, roleType: 'project' as const, usedByUsers: undefined };
mockConfirm.mockResolvedValue('confirm');
rolesStore.deleteRole.mockResolvedValue(projectRole);
rolesStore.roles.project = [projectRole];
const { result, unmount } = withSetup();
await result.requestDelete(projectRole, { roleType: 'project' });
expect(rolesStore.deleteRole).toHaveBeenCalledWith('support', undefined);
expect(result.reassignState.value).toBeNull();
unmount();
});
it('does not delete when the confirmation dialog is dismissed', async () => {
mockConfirm.mockResolvedValue('cancel');
const projectRole = { ...mockRole, roleType: 'project' as const, usedByUsers: undefined };
const { result, unmount } = withSetup();
await result.requestDelete(projectRole, { roleType: 'project' });
expect(rolesStore.deleteRole).not.toHaveBeenCalled();
unmount();
});
});
describe('onBeforeDelete guard', () => {
it('aborts the deletion when the guard returns false', async () => {
const onBeforeDelete = vi.fn().mockResolvedValue(false);
const { result, unmount } = withSetup();
await result.requestDelete(mockRole, { roleType: 'global', onBeforeDelete });
expect(onBeforeDelete).toHaveBeenCalledWith(mockRole);
expect(rolesStore.fetchRoleBySlug).not.toHaveBeenCalled();
expect(rolesStore.deleteRole).not.toHaveBeenCalled();
unmount();
});
});
});
@@ -0,0 +1,153 @@
import { useMessage } from '@/app/composables/useMessage';
import { useTelemetry } from '@/app/composables/useTelemetry';
import { useToast } from '@/app/composables/useToast';
import { MODAL_CONFIRM } from '@/app/constants';
import { useRolesStore } from '@/app/stores/roles.store';
import { useI18n } from '@n8n/i18n';
import type { Role } from '@n8n/permissions';
import { ref } from 'vue';
import { useRouter, type RouteLocationRaw } from 'vue-router';
type RoleType = 'global' | 'project';
interface RequestDeleteOptions {
roleType: RoleType;
/** Where to navigate after a successful deletion (e.g. the detail view returns to the list). */
redirectTo?: RouteLocationRaw;
/**
* Guard run before anything else. Return `false` to abort the deletion
* (e.g. when the role is still in use and can't be deleted).
*/
onBeforeDelete?: (role: Role) => Promise<boolean>;
}
/** A pending reassign-then-delete, carrying what the modal needs plus the follow-up. */
interface ReassignState extends Pick<RequestDeleteOptions, 'roleType' | 'redirectTo'> {
role: Role;
userCount: number;
}
/**
* Encapsulates the full role-deletion UX so every entry point (roles list, role detail)
* behaves identically: guard → for instance roles with assigned users open the
* reassign-then-delete modal, otherwise confirm and delete.
*/
export function useRoleDeletion() {
const rolesStore = useRolesStore();
const { showError, showMessage } = useToast();
const message = useMessage();
const i18n = useI18n();
const telemetry = useTelemetry();
const router = useRouter();
/** When set, an instance role with assigned users is awaiting reassignment in the modal. */
const reassignState = ref<ReassignState | null>(null);
/**
* Fetch the current number of users assigned to a role. Returns `null` (and surfaces
* an error toast) if the lookup fails, so callers can abort rather than act on a
* stale, page-load snapshot.
*/
async function resolveAssignedUserCount(slug: string): Promise<number | null> {
try {
const role = await rolesStore.fetchRoleBySlug({ slug });
return role.usedByUsers ?? 0;
} catch (error) {
showError(error, i18n.baseText('roles.action.delete.error'));
return null;
}
}
async function performRoleDeletion(options: {
role: Role;
roleType: RoleType;
reassignRoleSlug?: string;
redirectTo?: RouteLocationRaw;
}): Promise<void> {
const { role, roleType, reassignRoleSlug, redirectTo } = options;
try {
await rolesStore.deleteRole(role.slug, reassignRoleSlug);
const index = rolesStore.roles[roleType].findIndex((r) => r.slug === role.slug);
if (index !== -1) {
rolesStore.roles[roleType].splice(index, 1);
}
// Reassignment shifts the target role's member count, so refetch to reconcile.
if (reassignRoleSlug) {
void rolesStore.fetchRoles();
}
showMessage({ title: i18n.baseText('roles.action.delete.success'), type: 'success' });
telemetry.track('User successfully deleted role', {
role_id: role.slug,
role_name: role.displayName,
role_type: roleType === 'global' ? 'instance' : 'project',
permissions: role.scopes,
});
if (redirectTo) {
void router.push(redirectTo);
}
} catch (error) {
showError(error, i18n.baseText('roles.action.delete.error'));
}
}
/**
* Entry point for deleting a role. Instance roles with assigned users open the
* reassign modal (resolved later via {@link confirmReassignDelete}); everything else
* goes through a confirmation dialog and is deleted immediately.
*/
async function requestDelete(role: Role, options: RequestDeleteOptions): Promise<void> {
const { roleType, redirectTo, onBeforeDelete } = options;
if (onBeforeDelete) {
const proceed = await onBeforeDelete(role);
if (!proceed) return;
}
if (roleType === 'global') {
const userCount = await resolveAssignedUserCount(role.slug);
if (userCount === null) return;
if (userCount > 0) {
reassignState.value = { role, userCount, roleType, redirectTo };
return;
}
}
const confirmed = await message.confirm(
i18n.baseText('roles.action.delete.text', { interpolate: { roleName: role.displayName } }),
i18n.baseText('roles.action.delete.title', { interpolate: { roleName: role.displayName } }),
{
type: 'warning',
confirmButtonText: i18n.baseText('roles.action.delete'),
cancelButtonText: i18n.baseText('roles.action.cancel'),
},
);
if (confirmed !== MODAL_CONFIRM) return;
await performRoleDeletion({ role, roleType, redirectTo });
}
/** Complete a pending reassign-then-delete with the chosen replacement role. */
function confirmReassignDelete(reassignRoleSlug: string): void {
const state = reassignState.value;
if (!state) return;
reassignState.value = null;
void performRoleDeletion({
role: state.role,
roleType: state.roleType,
reassignRoleSlug,
redirectTo: state.redirectTo,
});
}
function cancelReassign(): void {
reassignState.value = null;
}
return { reassignState, requestDelete, confirmReassignDelete, cancelReassign };
}
@@ -1,11 +1,10 @@
import { useMessage } from '@/app/composables/useMessage';
import { useTelemetry } from '@/app/composables/useTelemetry';
import { useToast } from '@/app/composables/useToast';
import { MODAL_CONFIRM } from '@/app/constants';
import { useRolesStore } from '@/app/stores/roles.store';
import { useI18n } from '@n8n/i18n';
import type { Role } from '@n8n/permissions';
import { useRouter } from 'vue-router';
import { useRoleDeletion } from './useRoleDeletion';
export interface RoleListViews {
/** Route name for read-only system-role detail page. */
@@ -31,12 +30,10 @@ export function useRolesListActions({
}: UseRolesListActionsOptions) {
const { showError, showMessage } = useToast();
const rolesStore = useRolesStore();
const message = useMessage();
const i18n = useI18n();
const telemetry = useTelemetry();
const router = useRouter();
const storeKey = roleType === 'project' ? 'project' : 'global';
const { reassignState, requestDelete, confirmReassignDelete, cancelReassign } = useRoleDeletion();
async function duplicateRole(item: Role): Promise<Role | undefined> {
try {
@@ -51,7 +48,7 @@ export function useRolesListActions({
});
// Optimistic update — background fetch reconciles with server state.
rolesStore.roles[storeKey].push(role);
rolesStore.roles[roleType].push(role);
void rolesStore.fetchRoles();
telemetry.track('User duplicated role', {
@@ -75,39 +72,7 @@ export function useRolesListActions({
}
async function deleteRole(item: Role): Promise<void> {
if (onBeforeDelete) {
const proceed = await onBeforeDelete(item);
if (!proceed) return;
}
const confirmed = await message.confirm(
i18n.baseText('roles.action.delete.text', {
interpolate: { roleName: item.displayName },
}),
i18n.baseText('roles.action.delete.title', {
interpolate: { roleName: item.displayName },
}),
{
type: 'warning',
confirmButtonText: i18n.baseText('roles.action.delete'),
cancelButtonText: i18n.baseText('roles.action.cancel'),
},
);
if (confirmed !== MODAL_CONFIRM) return;
try {
await rolesStore.deleteRole(item.slug);
const index = rolesStore.roles[storeKey].findIndex((r) => r.slug === item.slug);
if (index !== -1) {
rolesStore.roles[storeKey].splice(index, 1);
}
showMessage({ title: i18n.baseText('roles.action.delete.success'), type: 'success' });
} catch (error) {
showError(error, i18n.baseText('roles.action.delete.error'));
}
await requestDelete(item, { roleType, onBeforeDelete });
}
const actions = { duplicate: duplicateRole, delete: deleteRole } as const;
@@ -132,5 +97,15 @@ export function useRolesListActions({
});
}
return { duplicateRole, deleteRole, actions, rowActions, handleAction, handleRowClick };
return {
duplicateRole,
deleteRole,
actions,
rowActions,
handleAction,
handleRowClick,
reassignState,
confirmReassignDelete,
cancelReassign,
};
}
@@ -10,8 +10,10 @@ import { computed, toRaw } from 'vue';
import { useRouter } from 'vue-router';
import RoleEditorLayout, { type RoleEditorLabels } from '../components/RoleEditorLayout.vue';
import { useRoleDeletion } from '../composables/useRoleDeletion';
import { useRoleEditorForm } from '../composables/useRoleEditorForm';
import InstanceRoleAssignmentsTab from './InstanceRoleAssignmentsTab.vue';
import DeleteInstanceRoleModal from './components/DeleteInstanceRoleModal.vue';
import ScopeGroupSelector from './components/ScopeGroupSelector.vue';
import { ALL_INSTANCE_SCOPES } from './instanceRoleScopes';
@@ -21,6 +23,7 @@ const { showMessage, showError } = useToast();
const i18n = useI18n();
const message = useMessage();
const telemetry = useTelemetry();
const { reassignState, requestDelete, confirmReassignDelete, cancelReassign } = useRoleDeletion();
const props = defineProps<{ roleSlug?: string }>();
@@ -61,6 +64,10 @@ const editorLabels = computed<RoleEditorLabels>(() => ({
// System roles populate the preset buttons (clicking copies their scopes).
const presetRoles = computed(() => rolesStore.processedInstanceRoles.filter((r) => r.systemRole));
const reassignTargetRoles = computed(() =>
rolesStore.processedInstanceRoles.filter((r) => r.slug !== reassignState.value?.role.slug),
);
function onBackClick() {
void router.push({ name: VIEWS.ROLES_SETTINGS, query: { tab: 'instance' } });
}
@@ -180,42 +187,10 @@ async function handleSubmit() {
async function deleteRole() {
if (!initialState.value) return;
const deleteConfirmed = await message.confirm(
i18n.baseText('roles.action.delete.text', {
interpolate: { roleName: initialState.value.displayName },
}),
i18n.baseText('roles.action.delete.title', {
interpolate: { roleName: initialState.value.displayName },
}),
{
type: 'warning',
confirmButtonText: i18n.baseText('roles.action.delete'),
cancelButtonText: i18n.baseText('roles.action.cancel'),
},
);
if (deleteConfirmed !== MODAL_CONFIRM) return;
try {
await rolesStore.deleteRole(initialState.value.slug);
const index = rolesStore.roles.global.findIndex(
(role) => role.slug === initialState.value?.slug,
);
if (index !== -1) {
rolesStore.roles.global.splice(index, 1);
}
showMessage({ title: i18n.baseText('roles.action.delete.success'), type: 'success' });
telemetry.track('User successfully deleted role', {
role_id: initialState.value.slug,
role_name: initialState.value.displayName,
permissions: initialState.value.scopes,
});
void router.push({ name: VIEWS.ROLES_SETTINGS, query: { tab: 'instance' } });
} catch (error) {
showError(error, i18n.baseText('roles.action.delete.error'));
}
await requestDelete(initialState.value, {
roleType: 'global',
redirectTo: { name: VIEWS.ROLES_SETTINGS, query: { tab: 'instance' } },
});
}
</script>
@@ -278,6 +253,15 @@ async function deleteRole() {
<div v-if="roleSlug && activeTab === 'assignments'">
<InstanceRoleAssignmentsTab :role-slug="roleSlug" />
</div>
<DeleteInstanceRoleModal
:model-value="reassignState !== null"
:role="reassignState?.role ?? null"
:user-count="reassignState?.userCount ?? 0"
:available-roles="reassignTargetRoles"
@confirm="confirmReassignDelete"
@update:model-value="(open?: boolean) => !open && cancelReassign()"
/>
</RoleEditorLayout>
</template>
@@ -23,12 +23,26 @@ vi.mock('@/app/composables/useMessage', () => ({
useMessage: () => ({ confirm: vi.fn() }),
}));
// Render the dialog only when open so presence assertions reflect the modal's visibility.
const ElDialogStub = {
props: ['modelValue'],
template: `
<div v-if="modelValue" role="dialog">
<slot name="header" />
<slot />
<slot name="footer" />
</div>
`,
};
const renderComponent = createComponentRenderer(InstanceRolesView, {
global: {
stubs: {
RouterLink: {
template: '<router-link-stub v-bind="$attrs"><slot /></router-link-stub>',
},
ElDialog: ElDialogStub,
N8nSelect: true,
},
},
});
@@ -128,4 +142,35 @@ describe('InstanceRolesView', () => {
expect.objectContaining({ roleType: 'global', scopes: [] }),
);
});
it('should open the reassign modal when deleting a custom role with assigned users', async () => {
const customRole = { ...mockCustomRoles[0], usedByUsers: 3 };
rolesStore.processedInstanceRoles = [...mockSystemRoles, customRole];
rolesStore.roles.global = [...mockSystemRoles, customRole];
// The delete flow fetches the current count before deciding.
rolesStore.fetchRoleBySlug.mockResolvedValue({ ...customRole, usedByUsers: 3 });
const { getByTestId, getAllByTestId } = renderComponent();
await userEvent.click(within(getAllByTestId('action-toggle')[0]).getByRole('button'));
await userEvent.click(getByTestId('action-delete'));
expect(rolesStore.fetchRoleBySlug).toHaveBeenCalledWith({ slug: customRole.slug });
expect(getByTestId('delete-instance-role-modal')).toBeInTheDocument();
// No deletion happens just from opening the modal.
expect(rolesStore.deleteRole).not.toHaveBeenCalled();
});
it('should not open the reassign modal when the custom role has no assigned users', async () => {
const customRole = { ...mockCustomRoles[0], usedByUsers: 0 };
rolesStore.processedInstanceRoles = [customRole];
rolesStore.roles.global = [customRole];
rolesStore.fetchRoleBySlug.mockResolvedValue({ ...customRole, usedByUsers: 0 });
const { queryByTestId, getByTestId, getAllByTestId } = renderComponent();
await userEvent.click(within(getAllByTestId('action-toggle')[0]).getByRole('button'));
await userEvent.click(getByTestId('action-delete'));
// Falls back to the standard confirm dialog rather than the reassign modal.
expect(queryByTestId('delete-instance-role-modal')).not.toBeInTheDocument();
});
});
@@ -1,19 +1,34 @@
<script setup lang="ts">
import { computed } from 'vue';
import { VIEWS } from '@/app/constants';
import { useRolesStore } from '@/app/stores/roles.store';
import { useSettingsStore } from '@/app/stores/settings.store';
import { useI18n } from '@n8n/i18n';
import RolesTable from '../components/RolesTable.vue';
import { useRolesListActions } from '../composables/useRolesListActions';
import DeleteInstanceRoleModal from './components/DeleteInstanceRoleModal.vue';
const rolesStore = useRolesStore();
const settingsStore = useSettingsStore();
const i18n = useI18n();
const { rowActions, handleAction, handleRowClick } = useRolesListActions({
const {
rowActions,
handleAction,
handleRowClick,
reassignState,
confirmReassignDelete,
cancelReassign,
} = useRolesListActions({
roleType: 'global',
views: { viewRoute: VIEWS.INSTANCE_ROLE_VIEW, editRoute: VIEWS.INSTANCE_ROLE_SETTINGS },
});
const reassignTargetRoles = computed(() =>
reassignState.value
? rolesStore.processedInstanceRoles.filter((r) => r.slug !== reassignState.value?.role.slug)
: [],
);
</script>
<template>
@@ -27,5 +42,14 @@ const { rowActions, handleAction, handleRowClick } = useRolesListActions({
@action="handleAction"
@row-click="handleRowClick"
/>
<DeleteInstanceRoleModal
:model-value="reassignState !== null"
:role="reassignState?.role ?? null"
:user-count="reassignState?.userCount ?? 0"
:available-roles="reassignTargetRoles"
@confirm="confirmReassignDelete"
@update:model-value="(open?: boolean) => !open && cancelReassign()"
/>
</div>
</template>
@@ -0,0 +1,117 @@
import { createComponentRenderer } from '@/__tests__/render';
import { createPinia, setActivePinia } from 'pinia';
import { waitFor } from '@testing-library/vue';
import userEvent from '@testing-library/user-event';
import type { Role } from '@n8n/permissions';
import DeleteInstanceRoleModal from './DeleteInstanceRoleModal.vue';
// The real ElDialog teleports to #app-modals; stub it so the modal content renders inline.
const ElDialogStub = {
props: ['modelValue'],
template: `
<div v-if="modelValue" role="dialog">
<slot name="header" />
<slot />
<slot name="footer" />
</div>
`,
};
const role: Role = {
displayName: 'Support Agent',
slug: 'global:support-agent',
description: null,
scopes: [],
licensed: true,
systemRole: false,
roleType: 'global',
};
const availableRoles: Role[] = [
{
displayName: 'Admin',
slug: 'global:admin',
description: null,
scopes: [],
licensed: true,
systemRole: true,
roleType: 'global',
},
{
displayName: 'Member',
slug: 'global:member',
description: null,
scopes: [],
licensed: true,
systemRole: true,
roleType: 'global',
},
];
const renderComponent = createComponentRenderer(DeleteInstanceRoleModal, {
props: { modelValue: true, role, userCount: 3, availableRoles },
global: {
stubs: {
ElDialog: ElDialogStub,
},
},
});
describe('DeleteInstanceRoleModal', () => {
beforeEach(() => {
vi.clearAllMocks();
setActivePinia(createPinia());
});
it('should show the role name, user count and reassignment prompt', () => {
const { getByText } = renderComponent();
expect(getByText('Delete Support Agent role')).toBeInTheDocument();
expect(getByText('3 users')).toBeInTheDocument();
expect(getByText(/are currently assigned to this role/)).toBeInTheDocument();
});
it('should use the singular form for a single assigned user', () => {
const { getByText } = renderComponent({
props: { modelValue: true, role, userCount: 1, availableRoles },
});
expect(getByText('1 user')).toBeInTheDocument();
});
it('should disable confirm until a role is picked, with a neutral default label', () => {
const { getByTestId } = renderComponent();
const confirmButton = getByTestId('confirm-delete-reassign-role');
expect(confirmButton).toBeDisabled();
expect(confirmButton).toHaveTextContent('Delete and reassign users');
});
it('should reflect the chosen role in the confirm label and emit on confirm', async () => {
const { getByTestId, getByText, emitted } = renderComponent();
// N8nSelect (ElSelect): open the dropdown, then pick the option.
await userEvent.click(getByTestId('reassign-role-select'));
await waitFor(() => expect(getByText('Admin')).toBeInTheDocument());
await userEvent.click(getByText('Admin'));
const confirmButton = getByTestId('confirm-delete-reassign-role');
await waitFor(() =>
expect(confirmButton).toHaveTextContent('Delete role and reassign users to Admin'),
);
expect(confirmButton).toBeEnabled();
await userEvent.click(confirmButton);
expect(emitted().confirm).toEqual([['global:admin']]);
});
it('should close without emitting confirm when cancelled', async () => {
const { getByTestId, emitted } = renderComponent();
await userEvent.click(getByTestId('cancel-delete-role'));
expect(emitted().confirm).toBeUndefined();
expect(emitted()['update:modelValue']).toEqual([[false]]);
});
});
@@ -0,0 +1,142 @@
<script lang="ts" setup>
import { computed, ref, watch } from 'vue';
import { ElDialog } from 'element-plus';
import { N8nButton, N8nHeading, N8nOption, N8nSelect, N8nText } from '@n8n/design-system';
import { useI18n } from '@n8n/i18n';
import type { Role } from '@n8n/permissions';
import { APP_MODALS_ELEMENT_ID } from '@/app/constants';
const props = defineProps<{
/** The role being deleted. */
role: Role | null;
/** Number of users currently assigned to the role. */
userCount: number;
/** Roles the assigned users can be reassigned to (excludes the role being deleted). */
availableRoles: Role[];
}>();
const emit = defineEmits<{
confirm: [reassignRoleSlug: string];
}>();
const visible = defineModel<boolean>();
const i18n = useI18n();
// Intentionally starts empty so the admin has to make an active choice.
const selectedRoleSlug = ref<string | undefined>(undefined);
// Reset the selection every time the modal is (re)opened.
watch(visible, (isOpen) => {
if (isOpen) selectedRoleSlug.value = undefined;
});
const selectedRole = computed(() =>
props.availableRoles.find((r) => r.slug === selectedRoleSlug.value),
);
const userCountText = computed(() =>
i18n.baseText('roles.instance.action.delete.reassign.userCount', {
adjustToNumber: props.userCount,
interpolate: { count: props.userCount },
}),
);
const confirmLabel = computed(() =>
selectedRole.value
? i18n.baseText('roles.instance.action.delete.reassign.confirmWithRole', {
interpolate: { roleName: selectedRole.value.displayName },
})
: i18n.baseText('roles.instance.action.delete.reassign.confirm'),
);
function onCancel() {
visible.value = false;
}
function onConfirm() {
if (!selectedRoleSlug.value) return;
emit('confirm', selectedRoleSlug.value);
}
</script>
<template>
<ElDialog
v-model="visible"
width="540"
:show-close="true"
:append-to="`#${APP_MODALS_ELEMENT_ID}`"
data-test-id="delete-instance-role-modal"
>
<template #header>
<N8nHeading tag="h2" size="xlarge" :bold="true">
{{
i18n.baseText('roles.instance.action.delete.reassign.title', {
interpolate: { roleName: role?.displayName ?? '' },
})
}}
</N8nHeading>
</template>
<div :class="$style.content">
<N8nText tag="p" size="medium" color="text-base">
<strong>{{ userCountText }}</strong>
{{ i18n.baseText('roles.instance.action.delete.reassign.description') }}
</N8nText>
<div :class="$style.field">
<N8nText tag="label" size="medium" color="text-dark">
{{ i18n.baseText('roles.instance.action.delete.reassign.label') }}
</N8nText>
<N8nSelect
v-model="selectedRoleSlug"
:placeholder="i18n.baseText('roles.instance.action.delete.reassign.placeholder')"
size="large"
data-test-id="reassign-role-select"
>
<N8nOption
v-for="option in availableRoles"
:key="option.slug"
:label="option.displayName"
:value="option.slug"
/>
</N8nSelect>
</div>
</div>
<template #footer>
<div :class="$style.footer">
<N8nButton variant="outline" data-test-id="cancel-delete-role" @click="onCancel">
{{ i18n.baseText('roles.action.cancel') }}
</N8nButton>
<N8nButton
variant="destructive"
:disabled="!selectedRoleSlug"
data-test-id="confirm-delete-reassign-role"
@click="onConfirm"
>
{{ confirmLabel }}
</N8nButton>
</div>
</template>
</ElDialog>
</template>
<style lang="scss" module>
.content {
display: flex;
flex-direction: column;
gap: var(--spacing--lg);
}
.field {
display: flex;
flex-direction: column;
gap: var(--spacing--2xs);
}
.footer {
display: flex;
justify-content: flex-end;
gap: var(--spacing--2xs);
}
</style>
@@ -326,7 +326,7 @@ describe('ProjectRolesView', () => {
},
);
expect(rolesStore.deleteRole).toHaveBeenCalledWith(mockRole.slug);
expect(rolesStore.deleteRole).toHaveBeenCalledWith(mockRole.slug, undefined);
expect(rolesStore.roles.project).not.toContain(mockRole);
expect(mockShowMessage).toHaveBeenCalledWith({ title: 'Role deleted', type: 'success' });
});
@@ -371,7 +371,7 @@ describe('ProjectRolesView', () => {
const deleteButton = getByTestId('action-delete');
await userEvent.click(deleteButton);
expect(rolesStore.deleteRole).toHaveBeenCalledWith(mockRole.slug);
expect(rolesStore.deleteRole).toHaveBeenCalledWith(mockRole.slug, undefined);
expect(mockShowError).toHaveBeenCalledWith(error, 'Error deleting role');
expect(rolesStore.roles.project).toHaveLength(1);
expect(rolesStore.roles.project[0]).toStrictEqual(mockRole);
@@ -397,7 +397,7 @@ describe('ProjectRolesView', () => {
const deleteButton = getByTestId('action-delete');
await userEvent.click(deleteButton);
expect(rolesStore.deleteRole).toHaveBeenCalledWith('non-existent-role');
expect(rolesStore.deleteRole).toHaveBeenCalledWith('non-existent-role', undefined);
expect(mockShowMessage).toHaveBeenCalledWith({ title: 'Role deleted', type: 'success' });
// Store should remain unchanged since role wasn't found
expect(rolesStore.roles.project).toHaveLength(1);