diff --git a/cypress/e2e/30-editor-after-route-changes.cy.ts b/cypress/e2e/30-editor-after-route-changes.cy.ts index 6b069dfbf1..4313c5fdec 100644 --- a/cypress/e2e/30-editor-after-route-changes.cy.ts +++ b/cypress/e2e/30-editor-after-route-changes.cy.ts @@ -50,7 +50,7 @@ describe('Editor zoom should work after route changes', () => { it('after switching between Editor and Workflow history and Workflow list', () => { cy.intercept('GET', '/rest/workflow-history/workflow/*/version/*').as('getVersion'); cy.intercept('GET', '/rest/workflow-history/workflow/*').as('getHistory'); - cy.intercept('GET', '/rest/users').as('getUsers'); + cy.intercept('GET', '/rest/users?*').as('getUsers'); cy.intercept('GET', '/rest/workflows?*').as('getWorkflows'); cy.intercept('GET', '/rest/active-workflows').as('getActiveWorkflows'); cy.intercept('GET', '/rest/projects').as('getProjects'); diff --git a/packages/@n8n/api-types/src/dto/pagination/pagination.dto.ts b/packages/@n8n/api-types/src/dto/pagination/pagination.dto.ts index 0c2ce7ab69..ee3d39f641 100644 --- a/packages/@n8n/api-types/src/dto/pagination/pagination.dto.ts +++ b/packages/@n8n/api-types/src/dto/pagination/pagination.dto.ts @@ -14,7 +14,7 @@ const skipValidator = z message: 'Param `skip` must be a non-negative integer', }); -export const createTakeValidator = (maxItems: number) => +export const createTakeValidator = (maxItems: number, allowInfinity: boolean = false) => z .string() .optional() @@ -22,9 +22,15 @@ export const createTakeValidator = (maxItems: number) => .refine((val) => !isNaN(val) && Number.isInteger(val), { message: 'Param `take` must be a valid integer', }) - .refine((val) => val >= 0, { - message: 'Param `take` must be a non-negative integer', - }) + .refine( + (val) => { + if (!allowInfinity) return val >= 0; + return true; + }, + { + message: 'Param `take` must be a non-negative integer', + }, + ) .transform((val) => Math.min(val, maxItems)); export const paginationSchema = { diff --git a/packages/@n8n/api-types/src/dto/user/__tests__/users-list-filter.dto.test.ts b/packages/@n8n/api-types/src/dto/user/__tests__/users-list-filter.dto.test.ts index 368900fe9c..23c8752174 100644 --- a/packages/@n8n/api-types/src/dto/user/__tests__/users-list-filter.dto.test.ts +++ b/packages/@n8n/api-types/src/dto/user/__tests__/users-list-filter.dto.test.ts @@ -14,14 +14,19 @@ describe('UsersListFilterDto', () => { parsedResult: { skip: 5, take: 20 }, }, { - name: 'sort by name ascending', - request: { sortBy: 'name:asc' }, - parsedResult: { skip: 0, take: 10, sortBy: 'name:asc' }, + name: 'sort by firstName ascending', + request: { sortBy: ['firstName:asc'] }, + parsedResult: { skip: 0, take: 10, sortBy: ['firstName:asc'] }, }, { - name: 'sort by last active descending and pagination', - request: { skip: '5', take: '20', sortBy: 'lastActive:desc' }, - parsedResult: { skip: 5, take: 20, sortBy: 'lastActive:desc' }, + name: 'sort by lastName ascending', + request: { sortBy: ['lastName:asc'] }, + parsedResult: { skip: 0, take: 10, sortBy: ['lastName:asc'] }, + }, + { + name: 'sort by role descending and pagination', + request: { skip: '5', take: '20', sortBy: ['role:desc'] }, + parsedResult: { skip: 5, take: 20, sortBy: ['role:desc'] }, }, ])('should validate $name', ({ request, parsedResult }) => { const result = UsersListFilterDto.safeParse(request); diff --git a/packages/@n8n/api-types/src/dto/user/users-list-filter.dto.ts b/packages/@n8n/api-types/src/dto/user/users-list-filter.dto.ts index fd1ed35051..1626754695 100644 --- a/packages/@n8n/api-types/src/dto/user/users-list-filter.dto.ts +++ b/packages/@n8n/api-types/src/dto/user/users-list-filter.dto.ts @@ -1,25 +1,75 @@ +import { jsonParse } from 'n8n-workflow'; import { z } from 'zod'; import { Z } from 'zod-class'; -import { paginationSchema } from '../pagination/pagination.dto'; +import { createTakeValidator, paginationSchema } from '../pagination/pagination.dto'; const USERS_LIST_SORT_OPTIONS = [ - 'name:asc', - 'name:desc', + 'firstName:asc', + 'firstName:desc', + 'lastName:asc', + 'lastName:desc', 'role:asc', // ascending order by role is Owner, Admin, Member 'role:desc', - 'lastActive:asc', - 'lastActive:desc', + // 'lastActive:asc', + // 'lastActive:desc', ] as const; const usersListSortByValidator = z - .enum(USERS_LIST_SORT_OPTIONS, { - message: `sortBy must be one of: ${USERS_LIST_SORT_OPTIONS.join(', ')}`, - }) + .array( + z.enum(USERS_LIST_SORT_OPTIONS, { + message: `sortBy must be one of: ${USERS_LIST_SORT_OPTIONS.join(', ')}`, + }), + ) .optional(); +const userSelectSchema = z.array( + z.enum(['id', 'firstName', 'lastName', 'email', 'disabled', 'mfaEnabled', 'role']), +); + +const userFilterSchema = z.object({ + isOwner: z.boolean().optional(), + firstName: z.string().optional(), + lastName: z.string().optional(), + email: z.string().optional(), + fullText: z.string().optional(), // Full text search across firstName, lastName, and email +}); + +const filterValidatorSchema = z + .string() + .optional() + .transform((val, ctx) => { + if (!val) return undefined; + try { + const parsed: unknown = jsonParse(val); + try { + return userFilterSchema.parse(parsed); + } catch (e) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Invalid filter fields', + path: ['filter'], + }); + return z.NEVER; + } + } catch (e) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: 'Invalid filter format', + path: ['filter'], + }); + return z.NEVER; + } + }); + +const userExpandSchema = z.array(z.enum(['projectRelations'])); + export class UsersListFilterDto extends Z.class({ ...paginationSchema, + take: createTakeValidator(50, true), // Limit to 50 items per page, and allow infinity for pagination + select: userSelectSchema.optional(), + filter: filterValidatorSchema.optional(), + expand: userExpandSchema.optional(), // Default sort order is role:asc, secondary sort criteria is name:asc sortBy: usersListSortByValidator, }) {} diff --git a/packages/@n8n/api-types/src/index.ts b/packages/@n8n/api-types/src/index.ts index 161041444f..d3fdf99b2b 100644 --- a/packages/@n8n/api-types/src/index.ts +++ b/packages/@n8n/api-types/src/index.ts @@ -41,5 +41,7 @@ export { export { ROLE, type Role, + type User, type UsersList, + usersListSchema, } from './schemas/user.schema'; diff --git a/packages/@n8n/api-types/src/schemas/__tests__/user.schema.test.ts b/packages/@n8n/api-types/src/schemas/__tests__/user.schema.test.ts index 744c6b59c4..3ed2487d21 100644 --- a/packages/@n8n/api-types/src/schemas/__tests__/user.schema.test.ts +++ b/packages/@n8n/api-types/src/schemas/__tests__/user.schema.test.ts @@ -31,16 +31,11 @@ describe('user.schema', () => { isValid: true, }, { - name: 'user with null fields', + name: 'user with undefined fields', data: { id: '123', - firstName: null, - lastName: null, - email: null, role: 'global:member', isPending: false, - lastActive: null, - projects: null, }, isValid: true, }, @@ -96,7 +91,7 @@ describe('user.schema', () => { name: 'valid users list', data: { count: 2, - data: [ + items: [ { id: '123', firstName: 'John', @@ -104,7 +99,6 @@ describe('user.schema', () => { email: 'johndoe@example.com', role: 'global:member', isPending: false, - lastActive: '2023-10-01T12:00:00Z', projects: ['project1', 'project2'], }, { @@ -114,7 +108,6 @@ describe('user.schema', () => { email: 'janedoe@example.com', role: 'global:admin', isPending: true, - lastActive: '2023-10-02T12:00:00Z', projects: null, }, ], @@ -125,14 +118,14 @@ describe('user.schema', () => { name: 'empty users list', data: { count: 0, - data: [], + items: [], }, isValid: true, }, { name: 'missing count', data: { - data: [], + items: [], }, isValid: false, }, @@ -147,7 +140,7 @@ describe('user.schema', () => { name: 'invalid user in list', data: { count: 1, - data: [ + items: [ { id: '123', firstName: 'John', diff --git a/packages/@n8n/api-types/src/schemas/user-settings.schema.ts b/packages/@n8n/api-types/src/schemas/user-settings.schema.ts new file mode 100644 index 0000000000..c39e63dc07 --- /dev/null +++ b/packages/@n8n/api-types/src/schemas/user-settings.schema.ts @@ -0,0 +1,27 @@ +import { z } from 'zod'; + +export const npsSurveyRespondedSchema = z.object({ + lastShownAt: z.number(), + responded: z.literal(true), +}); + +export const npsSurveyWaitingSchema = z.object({ + lastShownAt: z.number(), + waitingForResponse: z.literal(true), + ignoredCount: z.number(), +}); + +export const npsSurveySchema = z.union([npsSurveyRespondedSchema, npsSurveyWaitingSchema]); + +export const userSettingsSchema = z.object({ + isOnboarded: z.boolean().optional(), + firstSuccessfulWorkflowId: z.string().optional(), + userActivated: z.boolean().optional(), + userActivatedAt: z.number().optional(), + allowSSOManualLogin: z.boolean().optional(), + npsSurvey: npsSurveySchema.optional(), + easyAIWorkflowOnboarded: z.boolean().optional(), + userClaimedAiCredits: z.boolean().optional(), + dismissedCallouts: z.record(z.boolean()).optional(), +}); +export type UserSettings = z.infer; diff --git a/packages/@n8n/api-types/src/schemas/user.schema.ts b/packages/@n8n/api-types/src/schemas/user.schema.ts index 5bd2301b82..3b84c4144e 100644 --- a/packages/@n8n/api-types/src/schemas/user.schema.ts +++ b/packages/@n8n/api-types/src/schemas/user.schema.ts @@ -1,5 +1,8 @@ +import { projectRoleSchema } from '@n8n/permissions'; import { z } from 'zod'; +import { userSettingsSchema } from './user-settings.schema'; + export const ROLE = { Owner: 'global:owner', Member: 'global:member', @@ -13,20 +16,31 @@ export type Role = (typeof ROLE)[keyof typeof ROLE]; const roleValuesForSchema = Object.values(ROLE) as [Role, ...Role[]]; export const roleSchema = z.enum(roleValuesForSchema); +export const userProjectSchema = z.object({ + id: z.string(), + role: projectRoleSchema, + name: z.string(), +}); + export const userListItemSchema = z.object({ id: z.string(), - firstName: z.string().nullable(), - lastName: z.string().nullable(), - email: z.string().email().nullable(), - role: roleSchema, - isPending: z.boolean(), - lastActive: z.string().nullable(), - projects: z.array(z.string()).nullable(), // Can be null if the user is the owner or is an admin + firstName: z.string().nullable().optional(), + lastName: z.string().nullable().optional(), + email: z.string().email().nullable().optional(), + role: roleSchema.optional(), + isPending: z.boolean().optional(), + isOwner: z.boolean().optional(), + signInType: z.string().optional(), + settings: userSettingsSchema.nullable().optional(), + personalizationAnswers: z.object({}).passthrough().nullable().optional(), + lastActive: z.string().optional(), + projectRelations: z.array(userProjectSchema).nullable().optional(), }); export const usersListSchema = z.object({ count: z.number(), - data: z.array(userListItemSchema), + items: z.array(userListItemSchema), }); +export type User = z.infer; export type UsersList = z.infer; diff --git a/packages/@n8n/db/package.json b/packages/@n8n/db/package.json index 0601f890df..4622b8cadf 100644 --- a/packages/@n8n/db/package.json +++ b/packages/@n8n/db/package.json @@ -21,6 +21,7 @@ "dist/**/*" ], "dependencies": { + "@n8n/api-types": "workspace:^", "@n8n/backend-common": "workspace:^", "@n8n/config": "workspace:^", "@n8n/constants": "workspace:^", @@ -42,4 +43,4 @@ "@n8n/typescript-config": "workspace:*", "@types/lodash": "catalog:" } -} +} \ No newline at end of file diff --git a/packages/@n8n/db/src/repositories/user.repository.ts b/packages/@n8n/db/src/repositories/user.repository.ts index b952c83f30..42c639da3b 100644 --- a/packages/@n8n/db/src/repositories/user.repository.ts +++ b/packages/@n8n/db/src/repositories/user.repository.ts @@ -1,10 +1,10 @@ +import type { UsersListFilterDto } from '@n8n/api-types'; import { Service } from '@n8n/di'; import type { GlobalRole } from '@n8n/permissions'; -import type { DeepPartial, EntityManager, FindManyOptions } from '@n8n/typeorm'; -import { DataSource, In, IsNull, Not, Repository } from '@n8n/typeorm'; +import type { DeepPartial, EntityManager, SelectQueryBuilder } from '@n8n/typeorm'; +import { Brackets, DataSource, In, IsNull, Not, Repository } from '@n8n/typeorm'; import { Project, ProjectRelation, User } from '../entities'; -import type { ListQuery } from '../entities/types-db'; @Service() export class UserRepository extends Repository { @@ -75,41 +75,6 @@ export class UserRepository extends Repository { ); } - async toFindManyOptions(listQueryOptions?: ListQuery.Options) { - const findManyOptions: FindManyOptions = {}; - - if (!listQueryOptions) { - findManyOptions.relations = ['authIdentities']; - return findManyOptions; - } - - const { filter, select, take, skip } = listQueryOptions; - - if (select) findManyOptions.select = select; - if (take) findManyOptions.take = take; - if (skip) findManyOptions.skip = skip; - - if (take && !select) { - findManyOptions.relations = ['authIdentities']; - } - - if (take && select && !select?.id) { - findManyOptions.select = { ...findManyOptions.select, id: true }; // pagination requires id - } - - if (filter) { - const { isOwner, ...otherFilters } = filter; - - findManyOptions.where = otherFilters; - - if (isOwner !== undefined) { - findManyOptions.where.role = isOwner ? 'global:owner' : Not('global:owner'); - } - } - - return findManyOptions; - } - /** * Get emails of users who have completed setup, by user IDs. */ @@ -181,4 +146,141 @@ export class UserRepository extends Repository { }, }); } + + private applyUserListSelect( + queryBuilder: SelectQueryBuilder, + select: Array | undefined, + ): SelectQueryBuilder { + if (select !== undefined) { + if (!select.includes('id')) { + select.unshift('id'); // Ensure id is always selected + } + queryBuilder.select(select.map((field) => `user.${field}`)); + } + return queryBuilder; + } + + private applyUserListFilter( + queryBuilder: SelectQueryBuilder, + filter: UsersListFilterDto['filter'], + ): SelectQueryBuilder { + if (filter?.email !== undefined) { + queryBuilder.andWhere('user.email = :email', { + email: filter.email, + }); + } + + if (filter?.firstName !== undefined) { + queryBuilder.andWhere('user.firstName = :firstName', { + firstName: filter.firstName, + }); + } + + if (filter?.lastName !== undefined) { + queryBuilder.andWhere('user.lastName = :lastName', { + lastName: filter.lastName, + }); + } + + if (filter?.isOwner !== undefined) { + if (filter.isOwner) { + queryBuilder.andWhere('user.role = :role', { + role: 'global:owner', + }); + } else { + queryBuilder.andWhere('user.role <> :role', { + role: 'global:owner', + }); + } + } + + if (filter?.fullText !== undefined) { + const fullTextFilter = `%${filter.fullText}%`; + queryBuilder.andWhere( + new Brackets((qb) => { + qb.where('LOWER(user.firstName) like LOWER(:firstNameFullText)', { + firstNameFullText: fullTextFilter, + }) + .orWhere('LOWER(user.lastName) like LOWER(:lastNameFullText)', { + lastNameFullText: fullTextFilter, + }) + .orWhere('LOWER(user.email) like LOWER(:email)', { + email: fullTextFilter, + }); + }), + ); + } + + return queryBuilder; + } + + private applyUserListExpand( + queryBuilder: SelectQueryBuilder, + expand: UsersListFilterDto['expand'], + ): SelectQueryBuilder { + if (expand?.includes('projectRelations')) { + queryBuilder.leftJoinAndSelect( + 'user.projectRelations', + 'projectRelations', + 'projectRelations.role <> :projectRole', + { + projectRole: 'project:personalOwner', // Exclude personal project relations + }, + ); + queryBuilder.leftJoinAndSelect('projectRelations.project', 'project'); + } + + return queryBuilder; + } + + private applyUserListSort( + queryBuilder: SelectQueryBuilder, + sortBy: UsersListFilterDto['sortBy'], + ): SelectQueryBuilder { + if (sortBy) { + for (const sort of sortBy) { + const [field, order] = sort.split(':'); + if (field === 'firstName' || field === 'lastName') { + queryBuilder.addOrderBy(`user.${field}`, order.toUpperCase() as 'ASC' | 'DESC'); + } else if (field === 'role') { + queryBuilder.addOrderBy( + "CASE WHEN user.role='global:owner' THEN 0 WHEN user.role='global:admin' THEN 1 ELSE 2 END", + order.toUpperCase() as 'ASC' | 'DESC', + ); + } + } + } + + return queryBuilder; + } + + private applyUserListPagination( + queryBuilder: SelectQueryBuilder, + take: number, + skip: number | undefined, + ): SelectQueryBuilder { + if (take >= 0) queryBuilder.limit(take); + if (skip) queryBuilder.offset(skip); + + return queryBuilder; + } + + buildUserQuery(listQueryOptions?: UsersListFilterDto): SelectQueryBuilder { + const queryBuilder = this.createQueryBuilder('user'); + + queryBuilder.leftJoinAndSelect('user.authIdentities', 'authIdentities'); + + if (listQueryOptions === undefined) { + return queryBuilder; + } + const { filter, select, take, skip, expand, sortBy } = listQueryOptions; + + this.applyUserListSelect(queryBuilder, select as Array); + this.applyUserListFilter(queryBuilder, filter); + this.applyUserListExpand(queryBuilder, expand); + this.applyUserListPagination(queryBuilder, take, skip); + this.applyUserListSort(queryBuilder, sortBy); + + return queryBuilder; + } } diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index 2379b9a708..55ec9aa83e 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -1,4 +1,9 @@ -import { RoleChangeRequestDto, SettingsUpdateRequestDto } from '@n8n/api-types'; +import { + RoleChangeRequestDto, + SettingsUpdateRequestDto, + UsersListFilterDto, + usersListSchema, +} from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import type { PublicUser } from '@n8n/db'; import { @@ -19,6 +24,7 @@ import { Licensed, Body, Param, + Query, } from '@n8n/decorators'; import { Response } from 'express'; @@ -29,8 +35,7 @@ import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { EventService } from '@/events/event.service'; import { ExternalHooks } from '@/external-hooks'; -import { listQueryMiddleware } from '@/middlewares'; -import { ListQuery, AuthenticatedRequest, UserRequest } from '@/requests'; +import { AuthenticatedRequest, UserRequest } from '@/requests'; import { FolderService } from '@/services/folder.service'; import { ProjectService } from '@/services/project.service.ee'; import { UserService } from '@/services/user.service'; @@ -64,20 +69,16 @@ export class UsersController { private removeSupplementaryFields( publicUsers: Array>, - listQueryOptions: ListQuery.Options, + listQueryOptions: UsersListFilterDto, ) { - const { take, select, filter } = listQueryOptions; + const { select } = listQueryOptions; // remove fields added to satisfy query - if (take && select && !select?.id) { + if (select !== undefined && !select.includes('id')) { for (const user of publicUsers) delete user.id; } - if (filter?.isOwner) { - for (const user of publicUsers) delete user.role; - } - // remove computed fields (unselectable) if (select) { @@ -91,25 +92,40 @@ export class UsersController { return publicUsers; } - @Get('/', { middlewares: listQueryMiddleware }) + @Get('/') @GlobalScope('user:list') - async listUsers(req: ListQuery.Request) { - const { listQueryOptions } = req; + async listUsers( + req: AuthenticatedRequest, + _res: Response, + @Query listQueryOptions: UsersListFilterDto, + ) { + const userQuery = this.userRepository.buildUserQuery(listQueryOptions); - const findManyOptions = await this.userRepository.toFindManyOptions(listQueryOptions); + const response = await userQuery.getManyAndCount(); - const users = await this.userRepository.find(findManyOptions); + const [users, count] = response; - const publicUsers: Array> = await Promise.all( - users.map( - async (u) => - await this.userService.toPublic(u, { withInviteUrl: true, inviterId: req.user.id }), - ), + const publicUsers = await Promise.all( + users.map(async (u) => { + const user = await this.userService.toPublic(u, { + withInviteUrl: true, + inviterId: req.user.id, + }); + return { + ...user, + projectRelations: u.projectRelations?.map((pr) => ({ + id: pr.projectId, + role: pr.role, // normalize role for frontend + name: pr.project.name, + })), + }; + }), ); - return listQueryOptions - ? this.removeSupplementaryFields(publicUsers, listQueryOptions) - : publicUsers; + return usersListSchema.parse({ + count, + items: this.removeSupplementaryFields(publicUsers, listQueryOptions), + }); } @Get('/:id/password-reset-link') diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index 6e07991348..ea3eee4bbc 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -99,6 +99,7 @@ export const setupTestServer = ({ const app = express(); app.use(rawBodyReader); app.use(cookieParser()); + app.set('query parser', 'extended'); app.use((req: APIRequest, _, next) => { req.browserId = browserId; next(); diff --git a/packages/cli/test/integration/users.api.test.ts b/packages/cli/test/integration/users.api.test.ts index f16a637241..46a76ad87e 100644 --- a/packages/cli/test/integration/users.api.test.ts +++ b/packages/cli/test/integration/users.api.test.ts @@ -22,7 +22,7 @@ import { shareCredentialWithUsers, } from './shared/db/credentials'; import { createTeamProject, getPersonalProject, linkUserToProject } from './shared/db/projects'; -import { createAdmin, createMember, createOwner, getUserById } from './shared/db/users'; +import { createAdmin, createMember, createOwner, createUser, getUserById } from './shared/db/users'; import { createWorkflow, getWorkflowById, shareWorkflowWithUsers } from './shared/db/workflows'; import { randomCredentialPayload } from './shared/random'; import * as testDb from './shared/test-db'; @@ -41,15 +41,39 @@ const testServer = utils.setupTestServer({ describe('GET /users', () => { let owner: User; - let member: User; + let member1: User; let ownerAgent: SuperAgentTest; + let userRepository: UserRepository; beforeAll(async () => { await testDb.truncate(['User']); - owner = await createOwner(); - member = await createMember(); - await createMember(); + userRepository = Container.get(UserRepository); + + owner = await createUser({ + role: 'global:owner', + email: 'owner@n8n.io', + firstName: 'OwnerFirstName', + lastName: 'OwnerLastName', + }); + member1 = await createUser({ + role: 'global:member', + email: 'member1@n8n.io', + firstName: 'Member1FirstName', + lastName: 'Member1LastName', + }); + await createUser({ + role: 'global:member', + email: 'member2@n8n.io', + firstName: 'Member2FirstName', + lastName: 'Member2LastName', + }); + await createUser({ + role: 'global:admin', + email: 'admin@n8n.io', + firstName: 'AdminFirstName', + lastName: 'AdminLastName', + }); ownerAgent = testServer.authAgentFor(owner); }); @@ -57,9 +81,11 @@ describe('GET /users', () => { test('should return all users', async () => { const response = await ownerAgent.get('/users').expect(200); - expect(response.body.data).toHaveLength(3); + expect(response.body.data).toHaveProperty('count', 4); + expect(response.body.data).toHaveProperty('items'); + expect(response.body.data.items).toHaveLength(4); - response.body.data.forEach(validateUser); + response.body.data.items.forEach(validateUser); }); describe('list query options', () => { @@ -67,61 +93,85 @@ describe('GET /users', () => { test('should filter users by field: email', async () => { const response = await ownerAgent .get('/users') - .query(`filter={ "email": "${member.email}" }`) + .query(`filter={ "email": "${member1.email}" }`) .expect(200); - expect(response.body.data).toHaveLength(1); + expect(response.body.data).toEqual({ + count: 1, + items: expect.arrayContaining([]), + }); + expect(response.body.data.items).toHaveLength(1); - const [user] = response.body.data; + const [user] = response.body.data.items; - expect(user.email).toBe(member.email); + expect(user.email).toBe(member1.email); const _response = await ownerAgent .get('/users') .query('filter={ "email": "non@existing.com" }') .expect(200); - expect(_response.body.data).toHaveLength(0); + expect(_response.body.data).toEqual({ + count: 0, + items: expect.arrayContaining([]), + }); + expect(_response.body.data.items).toHaveLength(0); }); test('should filter users by field: firstName', async () => { const response = await ownerAgent .get('/users') - .query(`filter={ "firstName": "${member.firstName}" }`) + .query(`filter={ "firstName": "${member1.firstName}" }`) .expect(200); - expect(response.body.data).toHaveLength(1); + expect(response.body.data).toEqual({ + count: 1, + items: expect.arrayContaining([]), + }); + expect(response.body.data.items).toHaveLength(1); - const [user] = response.body.data; + const [user] = response.body.data.items; - expect(user.email).toBe(member.email); + expect(user.email).toBe(member1.email); const _response = await ownerAgent .get('/users') .query('filter={ "firstName": "Non-Existing" }') .expect(200); - expect(_response.body.data).toHaveLength(0); + expect(_response.body.data).toEqual({ + count: 0, + items: expect.arrayContaining([]), + }); + expect(_response.body.data.items).toHaveLength(0); }); test('should filter users by field: lastName', async () => { const response = await ownerAgent .get('/users') - .query(`filter={ "lastName": "${member.lastName}" }`) + .query(`filter={ "lastName": "${member1.lastName}" }`) .expect(200); - expect(response.body.data).toHaveLength(1); + expect(response.body.data).toEqual({ + count: 1, + items: expect.arrayContaining([]), + }); + expect(response.body.data.items).toHaveLength(1); - const [user] = response.body.data; + const [user] = response.body.data.items; - expect(user.email).toBe(member.email); + expect(user.email).toBe(member1.email); const _response = await ownerAgent .get('/users') .query('filter={ "lastName": "Non-Existing" }') .expect(200); - expect(_response.body.data).toHaveLength(0); + expect(_response.body.data).toEqual({ + count: 0, + items: expect.arrayContaining([]), + }); + expect(_response.body.data.items).toHaveLength(0); }); test('should filter users by computed field: isOwner', async () => { @@ -130,9 +180,13 @@ describe('GET /users', () => { .query('filter={ "isOwner": true }') .expect(200); - expect(response.body.data).toHaveLength(1); + expect(response.body.data).toEqual({ + count: 1, + items: expect.arrayContaining([]), + }); + expect(response.body.data.items).toHaveLength(1); - const [user] = response.body.data; + const [user] = response.body.data.items; expect(user.isOwner).toBe(true); @@ -141,60 +195,183 @@ describe('GET /users', () => { .query('filter={ "isOwner": false }') .expect(200); - expect(_response.body.data).toHaveLength(2); + expect(_response.body.data).toEqual({ + count: 3, + items: expect.arrayContaining([]), + }); + expect(_response.body.data.items).toHaveLength(3); - const [_user] = _response.body.data; + const [_user] = _response.body.data.items; expect(_user.isOwner).toBe(false); }); + + test('should filter users by field: fullText', async () => { + const response = await ownerAgent + .get('/users') + .query('filter={ "fullText": "member1" }') + .expect(200); + + expect(response.body.data).toEqual({ + count: 1, + items: expect.arrayContaining([]), + }); + expect(response.body.data.items).toHaveLength(1); + + const [user] = response.body.data.items; + + expect(user.email).toBe(member1.email); + + const _response = await ownerAgent + .get('/users') + .query('filter={ "fullText": "Non-Existing" }') + .expect(200); + + expect(_response.body.data).toEqual({ + count: 0, + items: expect.arrayContaining([]), + }); + expect(_response.body.data.items).toHaveLength(0); + }); }); describe('select', () => { test('should select user field: id', async () => { - const response = await ownerAgent.get('/users').query('select=["id"]').expect(200); + const response = await ownerAgent.get('/users').query('select[]=id').expect(200); expect(response.body).toEqual({ - data: [ - { id: expect.any(String) }, - { id: expect.any(String) }, - { id: expect.any(String) }, - ], + data: { + count: 4, + items: [ + { id: expect.any(String) }, + { id: expect.any(String) }, + { id: expect.any(String) }, + { id: expect.any(String) }, + ], + }, }); }); test('should select user field: email', async () => { - const response = await ownerAgent.get('/users').query('select=["email"]').expect(200); + const response = await ownerAgent.get('/users').query('select[]=email').expect(200); expect(response.body).toEqual({ - data: [ - { email: expect.any(String) }, - { email: expect.any(String) }, - { email: expect.any(String) }, - ], + data: { + count: 4, + items: [ + { + id: expect.any(String), + email: expect.any(String), + }, + { + id: expect.any(String), + email: expect.any(String), + }, + { + id: expect.any(String), + email: expect.any(String), + }, + { + id: expect.any(String), + email: expect.any(String), + }, + ], + }, }); }); test('should select user field: firstName', async () => { - const response = await ownerAgent.get('/users').query('select=["firstName"]').expect(200); + const response = await ownerAgent.get('/users').query('select[]=firstName').expect(200); expect(response.body).toEqual({ - data: [ - { firstName: expect.any(String) }, - { firstName: expect.any(String) }, - { firstName: expect.any(String) }, - ], + data: { + count: 4, + items: [ + { + id: expect.any(String), + firstName: expect.any(String), + }, + { + id: expect.any(String), + firstName: expect.any(String), + }, + { + id: expect.any(String), + firstName: expect.any(String), + }, + { + id: expect.any(String), + firstName: expect.any(String), + }, + ], + }, }); }); test('should select user field: lastName', async () => { - const response = await ownerAgent.get('/users').query('select=["lastName"]').expect(200); + const response = await ownerAgent.get('/users').query('select[]=lastName').expect(200); expect(response.body).toEqual({ - data: [ - { lastName: expect.any(String) }, - { lastName: expect.any(String) }, - { lastName: expect.any(String) }, - ], + data: { + count: 4, + items: [ + { + id: expect.any(String), + lastName: expect.any(String), + }, + { + id: expect.any(String), + lastName: expect.any(String), + }, + { + id: expect.any(String), + lastName: expect.any(String), + }, + { + id: expect.any(String), + lastName: expect.any(String), + }, + ], + }, + }); + }); + + test('should select multiple user fields: email, firstName, lastName', async () => { + const response = await ownerAgent + .get('/users') + .query('select[]=email&select[]=firstName&select[]=lastName') + .expect(200); + + expect(response.body).toEqual({ + data: { + count: 4, + items: [ + { + id: expect.any(String), + email: expect.any(String), + firstName: expect.any(String), + lastName: expect.any(String), + }, + { + id: expect.any(String), + email: expect.any(String), + firstName: expect.any(String), + lastName: expect.any(String), + }, + { + id: expect.any(String), + email: expect.any(String), + firstName: expect.any(String), + lastName: expect.any(String), + }, + { + id: expect.any(String), + email: expect.any(String), + firstName: expect.any(String), + lastName: expect.any(String), + }, + ], + }, }); }); }); @@ -203,23 +380,48 @@ describe('GET /users', () => { test('should return n users or less, without skip', async () => { const response = await ownerAgent.get('/users').query('take=2').expect(200); - expect(response.body.data).toHaveLength(2); + expect(response.body.data.count).toBe(4); + expect(response.body.data.items).toHaveLength(2); - response.body.data.forEach(validateUser); + response.body.data.items.forEach(validateUser); const _response = await ownerAgent.get('/users').query('take=1').expect(200); - expect(_response.body.data).toHaveLength(1); + expect(_response.body.data.items).toHaveLength(1); - _response.body.data.forEach(validateUser); + _response.body.data.items.forEach(validateUser); }); test('should return n users or less, with skip', async () => { const response = await ownerAgent.get('/users').query('take=1&skip=1').expect(200); - expect(response.body.data).toHaveLength(1); + expect(response.body.data.count).toBe(4); + expect(response.body.data.items).toHaveLength(1); - response.body.data.forEach(validateUser); + response.body.data.items.forEach(validateUser); + }); + + test('should return all users with negative take', async () => { + const users: User[] = []; + + for (let i = 0; i < 100; i++) { + users.push(await createMember()); + } + const response = await ownerAgent.get('/users').query('take=-1').expect(200); + + expect(response.body.data.items).toHaveLength(104); + + response.body.data.items.forEach(validateUser); + + for (const user of users) { + await userRepository.delete({ id: user.id }); + } + + const _response = await ownerAgent.get('/users').query('take=-1').expect(200); + + expect(_response.body.data.items).toHaveLength(4); + + _response.body.data.items.forEach(validateUser); }); }); @@ -232,10 +434,189 @@ describe('GET /users', () => { test('should support options that require auxiliary fields', async () => { const response = await ownerAgent .get('/users') - .query('filter={ "isOwner": true }&select=["firstName"]&take=1') + .query('filter={ "isOwner": true }&select[]=firstName&take=1') .expect(200); - expect(response.body).toEqual({ data: [{ firstName: expect.any(String) }] }); + expect(response.body).toEqual({ + data: { + count: 1, + items: [ + { + id: expect.any(String), + firstName: expect.any(String), + }, + ], + }, + }); + }); + }); + + describe('expand', () => { + test('should expand on team projects', async () => { + const project = await createTeamProject('Test Project'); + await linkUserToProject(member1, project, 'project:admin'); + + const response = await ownerAgent + .get('/users') + .query( + `filter={ "email": "${member1.email}" }&select[]=firstName&take=1&expand[]=projectRelations&sortBy[]=role:asc`, + ) + .expect(200); + + expect(response.body).toEqual({ + data: { + count: 1, + items: [ + { + id: expect.any(String), + firstName: expect.any(String), + projectRelations: [ + { + id: project.id, + role: 'project:admin', + name: project.name, // Ensure the project name is included + }, + ], + }, + ], + }, + }); + + expect(response.body.data.items[0].projectRelations).toHaveLength(1); + }); + + test('should expand on projects and hide personal projects', async () => { + const response = await ownerAgent + .get('/users') + .query( + 'filter={ "isOwner": true }&select[]=firstName&take=1&expand[]=projectRelations&sortBy[]=role:asc', + ) + .expect(200); + + expect(response.body).toEqual({ + data: { + count: 1, + items: [ + { + id: expect.any(String), + firstName: expect.any(String), + projectRelations: expect.arrayContaining([]), + }, + ], + }, + }); + + expect(response.body.data.items[0].projectRelations).toHaveLength(0); + }); + }); + + describe('sortBy', () => { + test('should sort by role:asc', async () => { + const response = await ownerAgent.get('/users').query('sortBy[]=role:asc').expect(200); + + expect(response.body.data.items).toHaveLength(4); + expect(response.body.data.items[0].role).toBe('global:owner'); + expect(response.body.data.items[1].role).toBe('global:admin'); + expect(response.body.data.items[2].role).toBe('global:member'); + expect(response.body.data.items[3].role).toBe('global:member'); + }); + + test('should sort by role:desc', async () => { + const response = await ownerAgent.get('/users').query('sortBy[]=role:desc').expect(200); + + expect(response.body.data.items).toHaveLength(4); + expect(response.body.data.items[0].role).toBe('global:member'); + expect(response.body.data.items[1].role).toBe('global:member'); + expect(response.body.data.items[2].role).toBe('global:admin'); + expect(response.body.data.items[3].role).toBe('global:owner'); + }); + + test('should sort by firstName:asc', async () => { + const response = await ownerAgent.get('/users').query('sortBy[]=firstName:asc').expect(200); + + expect(response.body.data.items).toHaveLength(4); + expect(response.body.data.items[0].firstName).toBe('AdminFirstName'); + expect(response.body.data.items[1].firstName).toBe('Member1FirstName'); + expect(response.body.data.items[2].firstName).toBe('Member2FirstName'); + expect(response.body.data.items[3].firstName).toBe('OwnerFirstName'); + }); + + test('should sort by firstName:desc', async () => { + const response = await ownerAgent + .get('/users') + .query('sortBy[]=firstName:desc') + .expect(200); + + expect(response.body.data.items).toHaveLength(4); + expect(response.body.data.items[0].firstName).toBe('OwnerFirstName'); + expect(response.body.data.items[1].firstName).toBe('Member2FirstName'); + expect(response.body.data.items[2].firstName).toBe('Member1FirstName'); + expect(response.body.data.items[3].firstName).toBe('AdminFirstName'); + }); + + test('should sort by lastName:asc', async () => { + const response = await ownerAgent.get('/users').query('sortBy[]=lastName:asc').expect(200); + + expect(response.body.data.items).toHaveLength(4); + expect(response.body.data.items[0].lastName).toBe('AdminLastName'); + expect(response.body.data.items[1].lastName).toBe('Member1LastName'); + expect(response.body.data.items[2].lastName).toBe('Member2LastName'); + expect(response.body.data.items[3].lastName).toBe('OwnerLastName'); + }); + + test('should sort by lastName:desc', async () => { + const response = await ownerAgent.get('/users').query('sortBy[]=lastName:desc').expect(200); + + expect(response.body.data.items).toHaveLength(4); + expect(response.body.data.items[0].lastName).toBe('OwnerLastName'); + expect(response.body.data.items[1].lastName).toBe('Member2LastName'); + expect(response.body.data.items[2].lastName).toBe('Member1LastName'); + expect(response.body.data.items[3].lastName).toBe('AdminLastName'); + }); + + test('should sort by firstName and lastName combined', async () => { + const user1 = await createUser({ + role: 'global:member', + email: 'memberz1@n8n.io', + firstName: 'ZZZFirstName', + lastName: 'ZZZLastName', + }); + + const user2 = await createUser({ + role: 'global:member', + email: 'memberz2@n8n.io', + firstName: 'ZZZFirstName', + lastName: 'ZZYLastName', + }); + + const response = await ownerAgent + .get('/users') + .query('sortBy[]=firstName:desc&sortBy[]=lastName:desc') + .expect(200); + + expect(response.body.data.items).toHaveLength(6); + expect(response.body.data.items[0].id).toBe(user1.id); + expect(response.body.data.items[1].id).toBe(user2.id); + expect(response.body.data.items[2].lastName).toBe('OwnerLastName'); + expect(response.body.data.items[3].lastName).toBe('Member2LastName'); + expect(response.body.data.items[4].lastName).toBe('Member1LastName'); + expect(response.body.data.items[5].lastName).toBe('AdminLastName'); + + const response1 = await ownerAgent + .get('/users') + .query('sortBy[]=firstName:asc&sortBy[]=lastName:asc') + .expect(200); + + expect(response1.body.data.items).toHaveLength(6); + expect(response1.body.data.items[5].id).toBe(user1.id); + expect(response1.body.data.items[4].id).toBe(user2.id); + expect(response1.body.data.items[3].lastName).toBe('OwnerLastName'); + expect(response1.body.data.items[2].lastName).toBe('Member2LastName'); + expect(response1.body.data.items[1].lastName).toBe('Member1LastName'); + expect(response1.body.data.items[0].lastName).toBe('AdminLastName'); + + await userRepository.delete({ id: user1.id }); + await userRepository.delete({ id: user2.id }); }); }); }); diff --git a/packages/frontend/editor-ui/src/Interface.ts b/packages/frontend/editor-ui/src/Interface.ts index fba42dcf2c..fd32a187a9 100644 --- a/packages/frontend/editor-ui/src/Interface.ts +++ b/packages/frontend/editor-ui/src/Interface.ts @@ -8,6 +8,7 @@ import type { IVersionNotificationSettings, ROLE, Role, + User, } from '@n8n/api-types'; import type { Scope } from '@n8n/permissions'; import type { NodeCreatorTag } from '@n8n/design-system'; @@ -53,7 +54,6 @@ import type { Cloud, InstanceUsage } from '@n8n/rest-api-client/api/cloudPlans'; import type { AI_NODE_CREATOR_VIEW, CREDENTIAL_EDIT_MODAL_KEY, - SignInType, TRIGGER_NODE_CREATOR_VIEW, REGULAR_NODE_CREATOR_VIEW, AI_OTHERS_NODE_CREATOR_VIEW, @@ -571,18 +571,10 @@ export type IPersonalizationSurveyVersions = export type InvitableRoleName = (typeof ROLE)['Member' | 'Admin']; -export interface IUserResponse { - id: string; - firstName?: string; - lastName?: string; - email?: string; - createdAt?: string; - role?: Role; +export interface IUserResponse extends User { globalScopes?: Scope[]; personalizationAnswers?: IPersonalizationSurveyVersions | null; - isPending: boolean; - signInType?: SignInType; - settings?: IUserSettings; + settings?: IUserSettings | null; } export interface CurrentUserResponse extends IUserResponse { diff --git a/packages/frontend/editor-ui/src/api/users.ts b/packages/frontend/editor-ui/src/api/users.ts index e4cc851f36..694906c891 100644 --- a/packages/frontend/editor-ui/src/api/users.ts +++ b/packages/frontend/editor-ui/src/api/users.ts @@ -2,6 +2,8 @@ import type { LoginRequestDto, PasswordUpdateRequestDto, SettingsUpdateRequestDto, + UsersList, + UsersListFilterDto, UserUpdateRequestDto, } from '@n8n/api-types'; import type { @@ -126,8 +128,11 @@ export async function deleteUser( await makeRestApiRequest(context, 'DELETE', `/users/${id}`, transferId ? { transferId } : {}); } -export async function getUsers(context: IRestApiContext): Promise { - return await makeRestApiRequest(context, 'GET', '/users'); +export async function getUsers( + context: IRestApiContext, + filter?: UsersListFilterDto, +): Promise { + return await makeRestApiRequest(context, 'GET', '/users', filter); } export async function getInviteLink( diff --git a/packages/frontend/editor-ui/src/stores/npsSurvey.store.ts b/packages/frontend/editor-ui/src/stores/npsSurvey.store.ts index 92c0e6e048..889391d95d 100644 --- a/packages/frontend/editor-ui/src/stores/npsSurvey.store.ts +++ b/packages/frontend/editor-ui/src/stores/npsSurvey.store.ts @@ -28,7 +28,7 @@ export const useNpsSurveyStore = defineStore('npsSurvey', () => { const currentUserId = ref(); const promptsData = ref(); - function setupNpsSurveyOnLogin(userId: string, settings?: IUserSettings): void { + function setupNpsSurveyOnLogin(userId: string, settings?: IUserSettings | null): void { currentUserId.value = userId; if (settings) { diff --git a/packages/frontend/editor-ui/src/stores/posthog.test.ts b/packages/frontend/editor-ui/src/stores/posthog.test.ts index b97610db44..615c049695 100644 --- a/packages/frontend/editor-ui/src/stores/posthog.test.ts +++ b/packages/frontend/editor-ui/src/stores/posthog.test.ts @@ -36,7 +36,6 @@ function setCurrentUser() { { id: CURRENT_USER_ID, isPending: false, - createdAt: '2023-03-17T14:01:36.432Z', }, ]); @@ -115,7 +114,6 @@ describe('Posthog store', () => { const userId = `${CURRENT_INSTANCE_ID}#${CURRENT_USER_ID}`; expect(window.posthog?.identify).toHaveBeenCalledWith(userId, { - created_at_timestamp: 1679061696432, instance_id: CURRENT_INSTANCE_ID, }); }); diff --git a/packages/frontend/editor-ui/src/stores/users.store.ts b/packages/frontend/editor-ui/src/stores/users.store.ts index 958d17c304..67db319d23 100644 --- a/packages/frontend/editor-ui/src/stores/users.store.ts +++ b/packages/frontend/editor-ui/src/stores/users.store.ts @@ -3,6 +3,7 @@ import { type PasswordUpdateRequestDto, type SettingsUpdateRequestDto, type UserUpdateRequestDto, + type User, ROLE, } from '@n8n/api-types'; import type { UpdateGlobalRolePayload } from '@/api/users'; @@ -119,8 +120,8 @@ export const useUsersStore = defineStore(STORES.USERS, () => { // Methods - const addUsers = (newUsers: IUserResponse[]) => { - newUsers.forEach((userResponse: IUserResponse) => { + const addUsers = (newUsers: User[]) => { + newUsers.forEach((userResponse) => { const prevUser = usersById.value[userResponse.id] || {}; const updatedUser = { ...prevUser, @@ -309,8 +310,8 @@ export const useUsersStore = defineStore(STORES.USERS, () => { }; const fetchUsers = async () => { - const users = await usersApi.getUsers(rootStore.restApiContext); - addUsers(users); + const { items } = await usersApi.getUsers(rootStore.restApiContext, { take: -1, skip: 0 }); + addUsers(items); }; const inviteUsers = async (params: Array<{ email: string; role: InvitableRoleName }>) => { diff --git a/packages/frontend/editor-ui/src/views/WorkflowsView.test.ts b/packages/frontend/editor-ui/src/views/WorkflowsView.test.ts index 284c787316..d9a2524fe7 100644 --- a/packages/frontend/editor-ui/src/views/WorkflowsView.test.ts +++ b/packages/frontend/editor-ui/src/views/WorkflowsView.test.ts @@ -360,7 +360,10 @@ describe('WorkflowsView', () => { workflowsStore.fetchActiveWorkflows.mockResolvedValue([]); }); it('should reinitialize on source control pullWorkfolder', async () => { - vi.spyOn(usersApi, 'getUsers').mockResolvedValue([]); + vi.spyOn(usersApi, 'getUsers').mockResolvedValue({ + count: 0, + items: [], + }); const userStore = mockedStore(useUsersStore); const sourceControl = useSourceControlStore(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 62959185d7..6b78f0c7da 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -541,6 +541,9 @@ importers: packages/@n8n/db: dependencies: + '@n8n/api-types': + specifier: workspace:^ + version: link:../api-types '@n8n/backend-common': specifier: workspace:^ version: link:../backend-common