fix(editor): Fix table pagination state handling and adding more tests (#16986)

packages/frontend/@n8n/i18n/src/locales/en.json

@@ -1965,6 +1965,7 @@
 	"settings.users.inviteUrlCreated.message": "Send the invite link to your invitee for activation",
 	"settings.users.passwordResetUrlCreated": "Password reset link copied to clipboard",
 	"settings.users.passwordResetUrlCreated.message": "Send the reset link to your user for them to reset their password",
+	"settings.users.passwordResetLinkError": "Could not retrieve password reset link",
 	"settings.users.allowSSOManualLogin": "Manual Login Allowed",
 	"settings.users.allowSSOManualLogin.message": "User can now login manually and through SSO",
 	"settings.users.disallowSSOManualLogin": "Manual Login Disallowed",
@@ -1996,6 +1997,7 @@
 	"settings.users.userRoleUpdated": "Changes saved",
 	"settings.users.userRoleUpdated.message": "{user} has been successfully updated to a {role}",
 	"settings.users.userRoleUpdatedError": "Unable to updated role",
+	"settings.users.table.update.error": "Failed to update table",
 	"settings.users.table.header.user": "@:_reusableBaseText.user",
 	"settings.users.table.header.accountType": "Account Type",
 	"settings.users.table.header.2fa": "2FA",

packages/frontend/editor-ui/src/components/SettingsUsers/SettingsUsersTable.test.ts

@@ -8,22 +8,14 @@ import SettingsUsersTable from '@/components/SettingsUsers/SettingsUsersTable.vu
 import { createComponentRenderer } from '@/__tests__/render';
 import { useEmitters } from '@/__tests__/utils';
 import type { IUser } from '@/Interface';
-import type { PermissionType, PermissionTypeOptions } from '@/types/rbac';
 
 const { emitters, addEmitter } = useEmitters<
 	'settingsUsersRoleCell' | 'settingsUsersActionsCell' | 'n8nDataTableServer'
 >();
 
-// Mock child components and composables
+const hasPermission = vi.fn(() => true);
-const hasPermission = vi.fn(
-	(permissionNames: PermissionType[], options?: Partial<PermissionTypeOptions>) =>
-		!!(permissionNames || options || []),
-);
 vi.mock('@/utils/rbac/permissions', () => ({
-	hasPermission: (
+	hasPermission: () => hasPermission(),
-		permissionNames: PermissionType[],
-		options?: Partial<PermissionTypeOptions>,
-	): boolean => hasPermission(permissionNames, options),
 }));
 
 vi.mock('@/components/SettingsUsers/SettingsUsersRoleCell.vue', () => ({

packages/frontend/editor-ui/src/components/SettingsUsers/SettingsUsersTable.vue

@@ -32,6 +32,10 @@ const emit = defineEmits<{
 	action: [value: { action: string; userId: string }];
 }>();
 
+const tableOptions = defineModel<TableOptions>('tableOptions', {
+	default: () => ({}),
+});
+
 const rows = computed(() => props.data.items);
 const headers = ref<Array<TableHeader<Item>>>([
 	{
@@ -133,6 +137,9 @@ const onRoleChange = ({ role, userId }: { role: string; userId: string }) => {
 <template>
 	<div>
 		<N8nDataTableServer
+			v-model:sort-by="tableOptions.sortBy"
+			v-model:page="tableOptions.page"
+			v-model:items-per-page="tableOptions.itemsPerPage"
 			:headers="headers"
 			:items="rows"
 			:items-length="data.count"

packages/frontend/editor-ui/src/views/SettingsUsersView.test.ts

@@ -3,7 +3,8 @@ import { createTestingPinia } from '@pinia/testing';
 import { screen, waitFor } from '@testing-library/vue';
 import userEvent from '@testing-library/user-event';
 import { vi } from 'vitest';
-import { type FrontendSettings, ROLE, type UsersList } from '@n8n/api-types';
+import { type FrontendSettings, type Role, ROLE, type UsersList } from '@n8n/api-types';
+import type { IUser } from '@/Interface';
 import {
 	INVITE_USER_MODAL_KEY,
 	DELETE_USER_MODAL_KEY,
@@ -25,7 +26,7 @@ import { useSSOStore } from '@/stores/sso.store';
 
 const { emitters, addEmitter } = useEmitters<'settingsUsersTable'>();
 
-// Mock the SettingsUsersTable component to emit events when clicked
+// Mock the SettingsUsersTable component to emit events
 vi.mock('@/components/SettingsUsers/SettingsUsersTable.vue', () => ({
 	default: defineComponent({
 		setup(_, { emit }) {
@@ -66,10 +67,6 @@ vi.mock('@/composables/usePageRedirectionHelper', () => ({
 	usePageRedirectionHelper: vi.fn(() => mockPageRedirectionHelper),
 }));
 
-vi.mock('@/utils/rbac/permissions', () => ({
-	hasPermission: vi.fn(() => false),
-}));
-
 const mockUsersList: UsersList = {
 	items: [
 		{
@@ -142,8 +139,10 @@ describe('SettingsUsersView', () => {
 			.mockResolvedValue({ link: 'https://example.com/reset/123' });
 		usersStore.updateOtherUserSettings = vi.fn().mockResolvedValue(undefined);
 		usersStore.updateGlobalRole = vi.fn().mockResolvedValue(undefined);
+		usersStore.updateEnforceMfa = vi.fn().mockResolvedValue(undefined);
 
 		settingsStore.isSmtpSetup = true;
+		settingsStore.isMFAEnforced = false;
 		settingsStore.settings.enterprise = {
 			mfaEnforcement: true,
 		} as FrontendSettings['enterprise'];
@@ -156,7 +155,7 @@ describe('SettingsUsersView', () => {
 		vi.clearAllMocks();
 	});
 
-	it('turn enforcing mfa on', async () => {
+	it('should turn enforcing mfa on', async () => {
 		const { getByTestId } = renderComponent();
 
 		const actionSwitch = getByTestId('enable-force-mfa');
@@ -167,7 +166,7 @@ describe('SettingsUsersView', () => {
 		expect(usersStore.updateEnforceMfa).toHaveBeenCalledWith(true);
 	});
 
-	it('turn enforcing mfa off', async () => {
+	it('should turn enforcing mfa off', async () => {
 		settingsStore.isMFAEnforced = true;
 		const { getByTestId } = renderComponent();
 
@@ -179,6 +178,32 @@ describe('SettingsUsersView', () => {
 		expect(usersStore.updateEnforceMfa).toHaveBeenCalledWith(false);
 	});
 
+	it('should handle MFA enforcement error', async () => {
+		usersStore.updateEnforceMfa = vi.fn().mockRejectedValue(new Error('MFA update failed'));
+
+		const { getByTestId } = renderComponent();
+
+		const actionSwitch = getByTestId('enable-force-mfa');
+		await userEvent.click(actionSwitch);
+
+		await waitFor(() => {
+			expect(mockToast.showError).toHaveBeenCalledWith(expect.any(Error), expect.any(String));
+		});
+	});
+
+	it('should handle missing user settings in SSO actions', async () => {
+		// Remove settings from user
+		usersStore.usersList.state.items[1].settings = undefined;
+
+		renderComponent();
+
+		emitters.settingsUsersTable.emit('action', { action: 'allowSSOManualLogin', userId: '2' });
+
+		expect(usersStore.updateOtherUserSettings).toHaveBeenCalledWith('2', {
+			allowSSOManualLogin: true,
+		});
+	});
+
 	it('should render correctly with users list', () => {
 		renderComponent();
 
@@ -274,6 +299,118 @@ describe('SettingsUsersView', () => {
 		expect(getByTestId('settings-users-table')).toBeInTheDocument();
 	});
 
+	describe('search functionality', () => {
+		it('should handle empty search', async () => {
+			renderComponent();
+
+			const searchInput = screen.getByTestId('users-list-search');
+			await userEvent.clear(searchInput);
+
+			await waitFor(() => {
+				expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+					skip: 0,
+					take: 10,
+					sortBy: ['firstName:asc', 'lastName:asc', 'email:asc'],
+					expand: ['projectRelations'],
+					filter: {
+						fullText: '',
+					},
+				});
+			});
+		});
+
+		it('should handle search with special characters', async () => {
+			renderComponent();
+
+			const searchInput = screen.getByTestId('users-list-search');
+			await userEvent.type(searchInput, '@#$%^&*()');
+
+			await waitFor(() => {
+				expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+					skip: 0,
+					take: 10,
+					sortBy: ['firstName:asc', 'lastName:asc', 'email:asc'],
+					expand: ['projectRelations'],
+					filter: {
+						fullText: '@#$%^&*()',
+					},
+				});
+			});
+		});
+
+		it('should reset to first page when searching', async () => {
+			renderComponent();
+
+			// Set table to page 2
+			emitters.settingsUsersTable.emit('update:options', {
+				page: 2,
+				itemsPerPage: 10,
+				sortBy: [{ id: 'email', desc: false }],
+			});
+
+			// Now search - should reset to page 0
+			const searchInput = screen.getByTestId('users-list-search');
+			await userEvent.type(searchInput, 'test');
+
+			await waitFor(() => {
+				expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+					skip: 0, // Should be 0, not 20 (page 2)
+					take: 10,
+					sortBy: ['email:asc'],
+					expand: ['projectRelations'],
+					filter: {
+						fullText: 'test',
+					},
+				});
+			});
+		});
+
+		it('should handle search with whitespace only', async () => {
+			renderComponent();
+
+			const searchInput = screen.getByTestId('users-list-search');
+			await userEvent.type(searchInput, '   ');
+
+			await waitFor(() => {
+				expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+					skip: 0,
+					take: 10,
+					sortBy: ['firstName:asc', 'lastName:asc', 'email:asc'],
+					expand: ['projectRelations'],
+					filter: {
+						fullText: '', // Should be trimmed to empty string
+					},
+				});
+			});
+		});
+	});
+
+	describe('component lifecycle', () => {
+		it('should not load users data when showing UM setup warning', async () => {
+			usersStore.currentUser = { isDefaultUser: true } as IUser;
+			renderComponent();
+
+			// Should not call execute when showing setup warning
+			expect(usersStore.usersList.execute).not.toHaveBeenCalled();
+		});
+
+		it('should load users data on mount when not showing setup warning', async () => {
+			renderComponent();
+
+			await waitFor(() => {
+				expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+					skip: 0,
+					take: 10,
+					sortBy: ['firstName:asc', 'lastName:asc', 'email:asc'],
+					expand: ['projectRelations'],
+					filter: {
+						fullText: '',
+					},
+				});
+			});
+		});
+	});
+
 	describe('user actions', () => {
 		it('should handle delete user action', async () => {
 			renderComponent();
@@ -386,6 +523,71 @@ describe('SettingsUsersView', () => {
 				expect(mockToast.showError).toHaveBeenCalledWith(expect.any(Error), expect.any(String));
 			});
 		});
+
+		it('should handle copy invite link with missing inviteAcceptUrl', async () => {
+			// Remove invite URL from user
+			usersStore.usersList.state.items[2].inviteAcceptUrl = undefined;
+
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('action', { action: 'copyInviteLink', userId: '3' });
+
+			// Should not call clipboard.copy or show toast
+			expect(mockClipboard.copy).not.toHaveBeenCalled();
+			expect(mockToast.showToast).not.toHaveBeenCalled();
+		});
+
+		it('should handle copy password reset link error', async () => {
+			usersStore.getUserPasswordResetLink = vi
+				.fn()
+				.mockRejectedValue(new Error('Reset link failed'));
+
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('action', { action: 'copyPasswordResetLink', userId: '2' });
+
+			await waitFor(() => {
+				expect(usersStore.getUserPasswordResetLink).toHaveBeenCalled();
+				expect(mockClipboard.copy).not.toHaveBeenCalled();
+				expect(mockToast.showToast).not.toHaveBeenCalled();
+			});
+		});
+
+		it('should handle reinvite with invalid role', async () => {
+			// Set user with invalid role
+			usersStore.usersList.state.items[2].role = 'invalid-role' as Role;
+
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('action', { action: 'reinvite', userId: '3' });
+
+			// Should not call reinviteUser with invalid role
+			expect(usersStore.reinviteUser).not.toHaveBeenCalled();
+		});
+
+		it('should handle reinvite with missing user data', async () => {
+			// Remove email from user
+			usersStore.usersList.state.items[2].email = undefined;
+
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('action', { action: 'reinvite', userId: '3' });
+
+			// Should not call reinviteUser
+			expect(usersStore.reinviteUser).not.toHaveBeenCalled();
+		});
+
+		it('should handle disallow SSO manual login with missing settings', async () => {
+			// Remove settings from user
+			usersStore.usersList.state.items[1].settings = undefined;
+
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('action', { action: 'disallowSSOManualLogin', userId: '2' });
+
+			// Should not call updateOtherUserSettings
+			expect(usersStore.updateOtherUserSettings).not.toHaveBeenCalled();
+		});
 	});
 
 	describe('role updates', () => {
@@ -417,6 +619,37 @@ describe('SettingsUsersView', () => {
 				expect(mockToast.showError).toHaveBeenCalledWith(expect.any(Error), expect.any(String));
 			});
 		});
+
+		it('should handle role update for non-existent user', async () => {
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('update:role', { role: ROLE.Admin, userId: 'non-existent' });
+
+			await waitFor(() => {
+				expect(mockToast.showError).toHaveBeenCalledWith(expect.any(Error), expect.any(String));
+			});
+
+			// Should not call updateGlobalRole
+			expect(usersStore.updateGlobalRole).not.toHaveBeenCalled();
+		});
+
+		it('should use email as fallback when user has no first/last name', async () => {
+			// Set user with no first/last name
+			usersStore.usersList.state.items[1].firstName = '';
+			usersStore.usersList.state.items[1].lastName = '';
+
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('update:role', { role: ROLE.Admin, userId: '2' });
+
+			await waitFor(() => {
+				expect(mockToast.showToast).toHaveBeenCalledWith({
+					type: 'success',
+					title: expect.any(String),
+					message: expect.stringContaining('member@example.com'),
+				});
+			});
+		});
 	});
 
 	describe('table functionality', () => {
@@ -479,5 +712,233 @@ describe('SettingsUsersView', () => {
 				},
 			});
 		});
+
+		it('should handle multiple mixed sort keys', async () => {
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('update:options', {
+				page: 0,
+				itemsPerPage: 10,
+				sortBy: [
+					{ id: 'name', desc: false },
+					{ id: 'email', desc: true },
+					{ id: 'invalidKey', desc: false },
+					{ id: 'role', desc: true },
+				],
+			});
+
+			expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+				skip: 0,
+				take: 10,
+				sortBy: ['firstName:asc', 'lastName:asc', 'email:asc', 'email:desc', 'role:desc'],
+				expand: ['projectRelations'],
+				filter: {
+					fullText: '',
+				},
+			});
+		});
+
+		it('should handle pagination correctly', async () => {
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('update:options', {
+				page: 3,
+				itemsPerPage: 25,
+				sortBy: [{ id: 'email', desc: false }],
+			});
+
+			expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+				skip: 75, // page 3 * 25 items per page
+				take: 25,
+				sortBy: ['email:asc'],
+				expand: ['projectRelations'],
+				filter: {
+					fullText: '',
+				},
+			});
+		});
+	});
+
+	describe('upgrade functionality', () => {
+		it('should handle upgrade banner click', async () => {
+			usersStore.usersLimitNotReached = false;
+
+			renderComponent();
+
+			const upgradeButton = screen.getByRole('button', { name: /view plans/i });
+			await userEvent.click(upgradeButton);
+
+			expect(mockPageRedirectionHelper.goToUpgrade).toHaveBeenCalledWith(
+				'settings-users',
+				'upgrade-users',
+			);
+		});
+
+		it('should handle advanced permissions upgrade click', async () => {
+			settingsStore.isEnterpriseFeatureEnabled[EnterpriseEditionFeature.AdvancedPermissions] =
+				false;
+
+			renderComponent();
+
+			const upgradeLink = screen.getByTestId('upgrade-permissions-link');
+			await userEvent.click(upgradeLink);
+
+			expect(mockPageRedirectionHelper.goToUpgrade).toHaveBeenCalledWith(
+				'settings-users',
+				'upgrade-advanced-permissions',
+			);
+		});
+	});
+
+	describe('edge cases', () => {
+		it('should handle empty users list', () => {
+			usersStore.usersList.state = {
+				items: [],
+				count: 0,
+			};
+
+			renderComponent();
+
+			expect(screen.getByTestId('settings-users-table')).toBeInTheDocument();
+		});
+
+		it('should handle loading state', () => {
+			usersStore.usersList.isLoading = true;
+
+			const { getByTestId } = renderComponent();
+
+			expect(getByTestId('settings-users-table')).toBeInTheDocument();
+		});
+
+		it('should handle user with missing email', async () => {
+			usersStore.usersList.state.items[1].email = undefined;
+
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('update:role', { role: ROLE.Admin, userId: '2' });
+
+			await waitFor(() => {
+				expect(mockToast.showToast).toHaveBeenCalledWith({
+					type: 'success',
+					title: expect.any(String),
+					message: expect.stringContaining(''),
+				});
+			});
+		});
+
+		it('should handle SSO actions with user not found', async () => {
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('action', {
+				action: 'allowSSOManualLogin',
+				userId: 'non-existent',
+			});
+
+			// Should not call updateOtherUserSettings
+			expect(usersStore.updateOtherUserSettings).not.toHaveBeenCalled();
+		});
+
+		it('should handle copy password reset link with user not found', async () => {
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('action', {
+				action: 'copyPasswordResetLink',
+				userId: 'non-existent',
+			});
+
+			// Should not call getUserPasswordResetLink
+			expect(usersStore.getUserPasswordResetLink).not.toHaveBeenCalled();
+		});
+
+		it('should handle table data update with network error', async () => {
+			usersStore.usersList.execute = vi.fn().mockRejectedValue(new Error('Network error'));
+
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('update:options', {
+				page: 0,
+				itemsPerPage: 10,
+				sortBy: [{ id: 'email', desc: false }],
+			});
+
+			await waitFor(() => {
+				expect(usersStore.usersList.execute).toHaveBeenCalled();
+			});
+		});
+
+		it('should handle sort by multiple fields with same direction', async () => {
+			renderComponent();
+
+			emitters.settingsUsersTable.emit('update:options', {
+				page: 0,
+				itemsPerPage: 10,
+				sortBy: [
+					{ id: 'email', desc: false },
+					{ id: 'firstName', desc: false },
+					{ id: 'lastName', desc: false },
+				],
+			});
+
+			expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+				skip: 0,
+				take: 10,
+				sortBy: ['email:asc', 'firstName:asc', 'lastName:asc'],
+				expand: ['projectRelations'],
+				filter: {
+					fullText: '',
+				},
+			});
+		});
+
+		it('should handle MFA enforcement with partial success', async () => {
+			let callCount = 0;
+			usersStore.updateEnforceMfa = vi.fn().mockImplementation(async () => {
+				callCount++;
+				if (callCount === 1) {
+					throw Error('First attempt failed');
+				}
+				return await Promise.resolve();
+			});
+
+			const { getByTestId } = renderComponent();
+
+			const actionSwitch = getByTestId('enable-force-mfa');
+			await userEvent.click(actionSwitch);
+
+			await waitFor(() => {
+				expect(mockToast.showError).toHaveBeenCalledWith(expect.any(Error), expect.any(String));
+			});
+
+			// Try again
+			await userEvent.click(actionSwitch);
+
+			await waitFor(() => {
+				expect(usersStore.updateEnforceMfa).toHaveBeenCalledTimes(2);
+			});
+		});
+
+		it('should handle rapid consecutive search inputs', async () => {
+			renderComponent();
+
+			const searchInput = screen.getByTestId('users-list-search');
+
+			// Type rapidly
+			await userEvent.type(searchInput, 'a');
+			await userEvent.type(searchInput, 'b');
+			await userEvent.type(searchInput, 'c');
+
+			await waitFor(() => {
+				expect(usersStore.usersList.execute).toHaveBeenCalledTimes(2); // Once on mount, once on search because of debounce
+				expect(usersStore.usersList.execute).toHaveBeenCalledWith(0, {
+					skip: 0,
+					take: 10,
+					sortBy: ['firstName:asc', 'lastName:asc', 'email:asc'],
+					expand: ['projectRelations'],
+					filter: {
+						fullText: 'abc',
+					},
+				});
+			});
+		});
 	});
 });

packages/frontend/editor-ui/src/views/SettingsUsersView.vue

@@ -158,12 +158,12 @@ async function onDelete(userId: string) {
 	});
 }
 async function onReinvite(userId: string) {
+	try {
 		const user = usersStore.usersList.state.items.find((u) => u.id === userId);
 		if (user?.email && user?.role) {
 			if (!['global:admin', 'global:member'].includes(user.role)) {
 				throw new Error('Invalid role name on reinvite');
 			}
-		try {
 			await usersStore.reinviteUser({
 				email: user.email,
 				role: user.role as InvitableRoleName,
@@ -175,11 +175,11 @@ async function onReinvite(userId: string) {
 					interpolate: { email: user.email ?? '' },
 				}),
 			});
+		}
 	} catch (e) {
 		showError(e, i18n.baseText('settings.users.userReinviteError'));
 	}
 }
-}
 async function onCopyInviteLink(userId: string) {
 	const user = usersStore.usersList.state.items.find((u) => u.id === userId);
 	if (user?.inviteAcceptUrl) {
@@ -193,6 +193,7 @@ async function onCopyInviteLink(userId: string) {
 	}
 }
 async function onCopyPasswordResetLink(userId: string) {
+	try {
 		const user = usersStore.usersList.state.items.find((u) => u.id === userId);
 		if (user) {
 			const url = await usersStore.getUserPasswordResetLink(user);
@@ -204,6 +205,9 @@ async function onCopyPasswordResetLink(userId: string) {
 				message: i18n.baseText('settings.users.passwordResetUrlCreated.message'),
 			});
 		}
+	} catch (error) {
+		showError(error, i18n.baseText('settings.users.passwordResetLinkError'));
+	}
 }
 async function onAllowSSOManualLogin(userId: string) {
 	const user = usersStore.usersList.state.items.find((u) => u.id === userId);
@@ -281,6 +285,7 @@ const isValidSortKey = (key: string): key is UsersListSortOptions =>
 	(USERS_LIST_SORT_OPTIONS as readonly string[]).includes(key);
 
 const updateUsersTableData = async ({ page, itemsPerPage, sortBy }: TableOptions) => {
+	try {
 		usersTableState.value = {
 			page,
 			itemsPerPage,
@@ -309,9 +314,13 @@ const updateUsersTableData = async ({ page, itemsPerPage, sortBy }: TableOptions
 				fullText: search.value.trim(),
 			},
 		});
+	} catch (error) {
+		showError(error, i18n.baseText('settings.users.table.update.error'));
+	}
 };
 
 const debouncedUpdateUsersTableData = useDebounceFn(() => {
+	usersTableState.value.page = 0; // Reset to first page on search
 	void updateUsersTableData(usersTableState.value);
 }, 300);
 
@@ -442,6 +451,7 @@ async function onUpdateMfaEnforced(value: boolean) {
 			:class="$style.usersContainer"
 		>
 			<SettingsUsersTable
+				v-model:table-options="usersTableState"
 				data-test-id="settings-users-table"
 				:data="usersStore.usersList.state"
 				:loading="usersStore.usersList.isLoading"