diff --git a/packages/@n8n/api-types/src/dto/index.ts b/packages/@n8n/api-types/src/dto/index.ts index 681a087cfd..f9988a8915 100644 --- a/packages/@n8n/api-types/src/dto/index.ts +++ b/packages/@n8n/api-types/src/dto/index.ts @@ -80,6 +80,8 @@ export { export { UpdateRoleDto } from './roles/update-role.dto'; export { CreateRoleDto } from './roles/create-role.dto'; +export { RoleListQueryDto } from './roles/role-list-query.dto'; +export { RoleGetQueryDto } from './roles/role-get-query.dto'; export { OidcConfigDto } from './oidc/config.dto'; diff --git a/packages/@n8n/api-types/src/dto/roles/__tests__/role-get-query.dto.test.ts b/packages/@n8n/api-types/src/dto/roles/__tests__/role-get-query.dto.test.ts new file mode 100644 index 0000000000..b28919d8e3 --- /dev/null +++ b/packages/@n8n/api-types/src/dto/roles/__tests__/role-get-query.dto.test.ts @@ -0,0 +1,68 @@ +import { RoleGetQueryDto } from '../role-get-query.dto'; + +describe('RoleGetQueryDto', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'with "true"', + request: { + withUsageCount: 'true', + }, + }, + { + name: 'with "false"', + request: { + withUsageCount: 'false', + }, + }, + { + name: 'without withUsageCount (uses default)', + request: {}, + }, + ])('should pass validation for withUsageCount $name', ({ request }) => { + const result = RoleGetQueryDto.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'with number', + request: { + withUsageCount: 1, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with boolean (true)', + request: { + withUsageCount: true, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with boolean (false)', + request: { + withUsageCount: false, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with invalid string', + request: { + withUsageCount: 'invalid', + }, + expectedErrorPath: ['withUsageCount'], + }, + ])('should fail validation for withUsageCount $name', ({ request, expectedErrorPath }) => { + const result = RoleGetQueryDto.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/roles/__tests__/role-list-query.dto.test.ts b/packages/@n8n/api-types/src/dto/roles/__tests__/role-list-query.dto.test.ts new file mode 100644 index 0000000000..6d1f3677ca --- /dev/null +++ b/packages/@n8n/api-types/src/dto/roles/__tests__/role-list-query.dto.test.ts @@ -0,0 +1,68 @@ +import { RoleListQueryDto } from '../role-list-query.dto'; + +describe('RoleListQueryDto', () => { + describe('Valid requests', () => { + test.each([ + { + name: 'with "true"', + request: { + withUsageCount: 'true', + }, + }, + { + name: 'with "false"', + request: { + withUsageCount: 'false', + }, + }, + { + name: 'without withUsageCount (uses default)', + request: {}, + }, + ])('should pass validation for withUsageCount $name', ({ request }) => { + const result = RoleListQueryDto.safeParse(request); + expect(result.success).toBe(true); + }); + }); + + describe('Invalid requests', () => { + test.each([ + { + name: 'with number', + request: { + withUsageCount: 1, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with boolean (true)', + request: { + withUsageCount: true, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with boolean (false)', + request: { + withUsageCount: false, + }, + expectedErrorPath: ['withUsageCount'], + }, + { + name: 'with invalid string', + request: { + withUsageCount: 'invalid', + }, + expectedErrorPath: ['withUsageCount'], + }, + ])('should fail validation for withUsageCount $name', ({ request, expectedErrorPath }) => { + const result = RoleListQueryDto.safeParse(request); + + expect(result.success).toBe(false); + + if (expectedErrorPath) { + expect(result.error?.issues[0].path).toEqual(expectedErrorPath); + } + }); + }); +}); diff --git a/packages/@n8n/api-types/src/dto/roles/role-get-query.dto.ts b/packages/@n8n/api-types/src/dto/roles/role-get-query.dto.ts new file mode 100644 index 0000000000..b115b6521d --- /dev/null +++ b/packages/@n8n/api-types/src/dto/roles/role-get-query.dto.ts @@ -0,0 +1,10 @@ +import { Z } from 'zod-class'; + +import { booleanFromString } from '../../schemas/boolean-from-string'; + +/** + * Query DTO for retrieving a single role with optional usage count + */ +export class RoleGetQueryDto extends Z.class({ + withUsageCount: booleanFromString.optional().default('false'), +}) {} diff --git a/packages/@n8n/api-types/src/dto/roles/role-list-query.dto.ts b/packages/@n8n/api-types/src/dto/roles/role-list-query.dto.ts new file mode 100644 index 0000000000..9ed2060eec --- /dev/null +++ b/packages/@n8n/api-types/src/dto/roles/role-list-query.dto.ts @@ -0,0 +1,10 @@ +import { Z } from 'zod-class'; + +import { booleanFromString } from '../../schemas/boolean-from-string'; + +/** + * Query DTO for listing roles with optional usage count + */ +export class RoleListQueryDto extends Z.class({ + withUsageCount: booleanFromString.optional().default('false'), +}) {} diff --git a/packages/@n8n/db/src/entities/role.ts b/packages/@n8n/db/src/entities/role.ts index 7db4d1bb15..0dfaa660d9 100644 --- a/packages/@n8n/db/src/entities/role.ts +++ b/packages/@n8n/db/src/entities/role.ts @@ -1,12 +1,13 @@ import { Column, Entity, JoinTable, ManyToMany, OneToMany, PrimaryColumn } from '@n8n/typeorm'; +import { WithTimestamps } from './abstract-entity'; import type { ProjectRelation } from './project-relation'; import { Scope } from './scope'; @Entity({ name: 'role', }) -export class Role { +export class Role extends WithTimestamps { @PrimaryColumn({ type: String, name: 'slug', diff --git a/packages/@n8n/db/src/migrations/common/1756906557570-AddTimestampsToRoleAndRoleIndexes.ts b/packages/@n8n/db/src/migrations/common/1756906557570-AddTimestampsToRoleAndRoleIndexes.ts new file mode 100644 index 0000000000..4a8e0d44d6 --- /dev/null +++ b/packages/@n8n/db/src/migrations/common/1756906557570-AddTimestampsToRoleAndRoleIndexes.ts @@ -0,0 +1,46 @@ +import { Column } from '../dsl/column'; +import type { IrreversibleMigration, MigrationContext } from '../migration-types'; + +const ROLE_TABLE_NAME = 'role'; +const PROJECT_RELATION_TABLE_NAME = 'project_relation'; +const USER_TABLE_NAME = 'user'; +const PROJECT_RELATION_ROLE_IDX_NAME = 'project_relation_role_idx'; +const PROJECT_RELATION_ROLE_PROJECT_IDX_NAME = 'project_relation_role_project_idx'; +const USER_ROLE_IDX_NAME = 'user_role_idx'; + +export class AddTimestampsToRoleAndRoleIndexes1756906557570 implements IrreversibleMigration { + async up({ schemaBuilder, queryRunner, tablePrefix }: MigrationContext) { + // This loads the table metadata from the database and + // feeds the query runners cache with the table metadata + // Not doing this, seems to get TypeORM to wrongfully try to + // add the columns twice in the same statement. + await queryRunner.getTable(`${tablePrefix}${USER_TABLE_NAME}`); + + await schemaBuilder.addColumns(ROLE_TABLE_NAME, [ + new Column('createdAt').timestampTimezone().notNull.default('NOW()'), + new Column('updatedAt').timestampTimezone().notNull.default('NOW()'), + ]); + + // This index should allow us to efficiently query project relations by their role + // This will be used for counting how many users have a specific project role + await schemaBuilder.createIndex( + PROJECT_RELATION_TABLE_NAME, + ['role'], + false, + PROJECT_RELATION_ROLE_IDX_NAME, + ); + + // This index should allow us to efficiently query project relations by their role and project + // This will be used for counting how many users in a specific project have a specific project role + await schemaBuilder.createIndex( + PROJECT_RELATION_TABLE_NAME, + ['projectId', 'role'], + false, + PROJECT_RELATION_ROLE_PROJECT_IDX_NAME, + ); + + // This index should allow us to efficiently query users by their role slug + // This will be used for counting how many users have a specific global role + await schemaBuilder.createIndex(USER_TABLE_NAME, ['roleSlug'], false, USER_ROLE_IDX_NAME); + } +} diff --git a/packages/@n8n/db/src/migrations/mysqldb/index.ts b/packages/@n8n/db/src/migrations/mysqldb/index.ts index b5028d6665..0d6b731a19 100644 --- a/packages/@n8n/db/src/migrations/mysqldb/index.ts +++ b/packages/@n8n/db/src/migrations/mysqldb/index.ts @@ -96,6 +96,7 @@ import { RemoveOldRoleColumn1750252139170 } from '../common/1750252139170-Remove import { AddInputsOutputsToTestCaseExecution1752669793000 } from '../common/1752669793000-AddInputsOutputsToTestCaseExecution'; import { CreateDataStoreTables1754475614601 } from '../common/1754475614601-CreateDataStoreTables'; import { ReplaceDataStoreTablesWithDataTables1754475614602 } from '../common/1754475614602-ReplaceDataStoreTablesWithDataTables'; +import { AddTimestampsToRoleAndRoleIndexes1756906557570 } from '../common/1756906557570-AddTimestampsToRoleAndRoleIndexes'; import type { Migration } from '../migration-types'; import { UpdateParentFolderIdColumn1740445074052 } from '../mysqldb/1740445074052-UpdateParentFolderIdColumn'; @@ -199,4 +200,5 @@ export const mysqlMigrations: Migration[] = [ RemoveOldRoleColumn1750252139170, ReplaceDataStoreTablesWithDataTables1754475614602, LinkRoleToProjectRelationTable1753953244168, + AddTimestampsToRoleAndRoleIndexes1756906557570, ]; diff --git a/packages/@n8n/db/src/migrations/postgresdb/index.ts b/packages/@n8n/db/src/migrations/postgresdb/index.ts index 79db0086c8..9358259fc5 100644 --- a/packages/@n8n/db/src/migrations/postgresdb/index.ts +++ b/packages/@n8n/db/src/migrations/postgresdb/index.ts @@ -96,6 +96,7 @@ import { LinkRoleToUserTable1750252139168 } from '../common/1750252139168-LinkRo import { RemoveOldRoleColumn1750252139170 } from '../common/1750252139170-RemoveOldRoleColumn'; import { CreateDataStoreTables1754475614601 } from '../common/1754475614601-CreateDataStoreTables'; import { ReplaceDataStoreTablesWithDataTables1754475614602 } from '../common/1754475614602-ReplaceDataStoreTablesWithDataTables'; +import { AddTimestampsToRoleAndRoleIndexes1756906557570 } from '../common/1756906557570-AddTimestampsToRoleAndRoleIndexes'; import type { Migration } from '../migration-types'; export const postgresMigrations: Migration[] = [ @@ -197,4 +198,5 @@ export const postgresMigrations: Migration[] = [ RemoveOldRoleColumn1750252139170, ReplaceDataStoreTablesWithDataTables1754475614602, LinkRoleToProjectRelationTable1753953244168, + AddTimestampsToRoleAndRoleIndexes1756906557570, ]; diff --git a/packages/@n8n/db/src/migrations/sqlite/index.ts b/packages/@n8n/db/src/migrations/sqlite/index.ts index d095abc9d7..a457b6cb07 100644 --- a/packages/@n8n/db/src/migrations/sqlite/index.ts +++ b/packages/@n8n/db/src/migrations/sqlite/index.ts @@ -92,6 +92,7 @@ import { RemoveOldRoleColumn1750252139170 } from '../common/1750252139170-Remove import { AddInputsOutputsToTestCaseExecution1752669793000 } from '../common/1752669793000-AddInputsOutputsToTestCaseExecution'; import { CreateDataStoreTables1754475614601 } from '../common/1754475614601-CreateDataStoreTables'; import { ReplaceDataStoreTablesWithDataTables1754475614602 } from '../common/1754475614602-ReplaceDataStoreTablesWithDataTables'; +import { AddTimestampsToRoleAndRoleIndexes1756906557570 } from '../common/1756906557570-AddTimestampsToRoleAndRoleIndexes'; import type { Migration } from '../migration-types'; import { LinkRoleToProjectRelationTable1753953244168 } from './../common/1753953244168-LinkRoleToProjectRelationTable'; @@ -191,6 +192,7 @@ const sqliteMigrations: Migration[] = [ RemoveOldRoleColumn1750252139170, ReplaceDataStoreTablesWithDataTables1754475614602, LinkRoleToProjectRelationTable1753953244168, + AddTimestampsToRoleAndRoleIndexes1756906557570, ]; export { sqliteMigrations }; diff --git a/packages/@n8n/db/src/repositories/role.repository.ts b/packages/@n8n/db/src/repositories/role.repository.ts index 8ad461eae2..72f269c8e2 100644 --- a/packages/@n8n/db/src/repositories/role.repository.ts +++ b/packages/@n8n/db/src/repositories/role.repository.ts @@ -3,7 +3,7 @@ import { Service } from '@n8n/di'; import { DataSource, EntityManager, In, Repository } from '@n8n/typeorm'; import { UserError } from 'n8n-workflow'; -import { Role } from '../entities'; +import { ProjectRelation, Role, User } from '../entities'; @Service() export class RoleRepository extends Repository { @@ -18,6 +18,51 @@ export class RoleRepository extends Repository { return await this.find({ relations: ['scopes'] }); } + async countUsersWithRole(role: Role): Promise { + if (role.roleType === 'global') { + return await this.manager.getRepository(User).count({ + where: { + role: { + slug: role.slug, + }, + }, + }); + } else if (role.roleType === 'project') { + return await this.manager.getRepository(ProjectRelation).count({ + where: { role: { slug: role.slug } }, + }); + } + + return 0; + } + + async findAllRoleCounts() { + const userCount = await this.manager + .createQueryBuilder(User, 'user') + .select('user.roleSlug', 'roleSlug') + .addSelect('COUNT(user.id)', 'count') + .groupBy('user.roleSlug') + .getRawMany<{ roleSlug: string; count: string }>(); + + const projectCount = await this.manager + .createQueryBuilder(ProjectRelation, 'projectRelation') + .select('projectRelation.role', 'roleSlug') + .addSelect('COUNT(projectRelation.user)', 'count') + .groupBy('projectRelation.role') + .getRawMany<{ roleSlug: string; count: string }>(); + + return userCount.concat(projectCount).reduce( + (acc, { roleSlug, count }) => { + if (!acc[roleSlug]) { + acc[roleSlug] = 0; + } + acc[roleSlug] += parseInt(count, 10); + return acc; + }, + {} as Record, + ); + } + async findBySlug(slug: string) { return await this.findOne({ where: { slug }, diff --git a/packages/@n8n/permissions/src/schemas.ee.ts b/packages/@n8n/permissions/src/schemas.ee.ts index ea97f23110..e763b72340 100644 --- a/packages/@n8n/permissions/src/schemas.ee.ts +++ b/packages/@n8n/permissions/src/schemas.ee.ts @@ -62,6 +62,9 @@ export const roleSchema = z.object({ roleType: roleNamespaceSchema, licensed: z.boolean(), scopes: z.array(scopeSchema), + createdAt: z.date().optional(), + updatedAt: z.date().optional(), + usedByUsers: z.number().optional(), }); export type Role = z.infer; diff --git a/packages/cli/src/controllers/role.controller.ts b/packages/cli/src/controllers/role.controller.ts index 2821a0afb6..39af0a4a9c 100644 --- a/packages/cli/src/controllers/role.controller.ts +++ b/packages/cli/src/controllers/role.controller.ts @@ -1,4 +1,4 @@ -import { CreateRoleDto, UpdateRoleDto } from '@n8n/api-types'; +import { CreateRoleDto, RoleGetQueryDto, RoleListQueryDto, UpdateRoleDto } from '@n8n/api-types'; import { LICENSE_FEATURES } from '@n8n/constants'; import { AuthenticatedRequest } from '@n8n/db'; import { @@ -10,6 +10,7 @@ import { Param, Patch, Post, + Query, RestController, } from '@n8n/decorators'; import { Role as RoleDTO } from '@n8n/permissions'; @@ -21,8 +22,12 @@ export class RoleController { constructor(private readonly roleService: RoleService) {} @Get('/') - async getAllRoles(): Promise> { - const allRoles = await this.roleService.getAllRoles(); + async getAllRoles( + _req: AuthenticatedRequest, + _res: Response, + @Query query: RoleListQueryDto, + ): Promise> { + const allRoles = await this.roleService.getAllRoles(query.withUsageCount); return { global: allRoles.filter((r) => r.roleType === 'global'), project: allRoles.filter((r) => r.roleType === 'project'), @@ -36,8 +41,9 @@ export class RoleController { _req: AuthenticatedRequest, _res: Response, @Param('slug') slug: string, + @Query query: RoleGetQueryDto, ): Promise { - return await this.roleService.getRole(slug); + return await this.roleService.getRole(slug, query.withUsageCount); } @Patch('/:slug') diff --git a/packages/cli/src/services/role.service.ts b/packages/cli/src/services/role.service.ts index 74e7f9e296..6f37ca2851 100644 --- a/packages/cli/src/services/role.service.ts +++ b/packages/cli/src/services/role.service.ts @@ -38,23 +38,37 @@ export class RoleService { private readonly scopeRepository: ScopeRepository, ) {} - private dbRoleToRoleDTO(role: Role): RoleDTO { + private dbRoleToRoleDTO(role: Role, usedByUsers?: number): RoleDTO { return { ...role, scopes: role.scopes.map((s) => s.slug), licensed: this.isRoleLicensed(role.slug), + usedByUsers, }; } - async getAllRoles(): Promise { + async getAllRoles(withCount: boolean = false): Promise { const roles = await this.roleRepository.findAll(); - return roles.map((r) => this.dbRoleToRoleDTO(r)); + + if (!withCount) { + return roles.map((r) => this.dbRoleToRoleDTO(r)); + } + + const roleCounts = await this.roleRepository.findAllRoleCounts(); + + return roles.map((role) => { + const usedByUsers = roleCounts[role.slug] ?? 0; + return this.dbRoleToRoleDTO(role, usedByUsers); + }); } - async getRole(slug: string): Promise { + async getRole(slug: string, withCount: boolean = false): Promise { const role = await this.roleRepository.findBySlug(slug); if (role) { - return this.dbRoleToRoleDTO(role); + const usedByUsers = withCount + ? await this.roleRepository.countUsersWithRole(role) + : undefined; + return this.dbRoleToRoleDTO(role, usedByUsers); } throw new NotFoundError('Role not found'); } diff --git a/packages/cli/test/integration/controllers/role.controller-db.test.ts b/packages/cli/test/integration/controllers/role.controller-db.test.ts index 44572f2232..b7670ebd84 100644 --- a/packages/cli/test/integration/controllers/role.controller-db.test.ts +++ b/packages/cli/test/integration/controllers/role.controller-db.test.ts @@ -59,6 +59,8 @@ describe('RoleController - Integration Tests', () => { roleType: role.roleType, scopes: role.scopes.map((scope) => scope.slug).sort(), licensed: expect.any(Boolean), + createdAt: expect.any(String), + updatedAt: expect.any(String), }, }); }, @@ -90,6 +92,8 @@ describe('RoleController - Integration Tests', () => { slug: expect.any(String), licensed: expect.any(Boolean), systemRole: false, + createdAt: expect.any(String), + updatedAt: expect.any(String), }, }); @@ -102,6 +106,8 @@ describe('RoleController - Integration Tests', () => { slug: response.body.data.slug, licensed: expect.any(Boolean), systemRole: false, + createdAt: expect.any(String), + updatedAt: expect.any(String), }, }); }); @@ -147,6 +153,8 @@ describe('RoleController - Integration Tests', () => { roleType: 'project', licensed: expect.any(Boolean), systemRole: false, + createdAt: expect.any(String), + updatedAt: expect.any(String), }, }); @@ -161,6 +169,8 @@ describe('RoleController - Integration Tests', () => { roleType: 'project', licensed: expect.any(Boolean), systemRole: false, + createdAt: expect.any(String), + updatedAt: expect.any(String), }, }); }); diff --git a/packages/cli/test/integration/controllers/role.controller.test.ts b/packages/cli/test/integration/controllers/role.controller.test.ts index 44ca87bceb..82b6c66092 100644 --- a/packages/cli/test/integration/controllers/role.controller.test.ts +++ b/packages/cli/test/integration/controllers/role.controller.test.ts @@ -106,6 +106,251 @@ describe('RoleController', () => { expect(roleService.getAllRoles).toHaveBeenCalledTimes(1); }); + describe('GET /roles with withUsageCount parameter', () => { + it('should pass withUsageCount=true to service and include usage counts in response', async () => { + // + // ARRANGE + // + const mockRolesWithUsage: Role[] = [ + { + slug: 'global:admin', + displayName: 'Global Admin', + description: 'Global administrator', + systemRole: true, + roleType: 'global', + scopes: ['user:manage', 'workflow:create'], + licensed: true, + usedByUsers: 5, + }, + { + slug: 'project:editor', + displayName: 'Project Editor', + description: 'Project editor role', + systemRole: true, + roleType: 'project', + scopes: ['workflow:create', 'workflow:edit'], + licensed: true, + usedByUsers: 12, + }, + ]; + + roleService.getAllRoles.mockResolvedValue(mockRolesWithUsage); + + // + // ACT + // + const response = await memberAgent.get('/roles?withUsageCount=true').expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ + data: { + global: [mockRolesWithUsage[0]], // global:admin with usedByUsers + project: [mockRolesWithUsage[1]], // project:editor with usedByUsers + credential: [], + workflow: [], + }, + }); + + expect(roleService.getAllRoles).toHaveBeenCalledWith(true); + }); + + it('should pass withUsageCount=false to service and exclude usage counts', async () => { + // + // ARRANGE + // + const mockRolesWithoutUsage: Role[] = [ + { + slug: 'global:admin', + displayName: 'Global Admin', + description: 'Global administrator', + systemRole: true, + roleType: 'global', + scopes: ['user:manage', 'workflow:create'], + licensed: true, + }, + { + slug: 'project:editor', + displayName: 'Project Editor', + description: 'Project editor role', + systemRole: true, + roleType: 'project', + scopes: ['workflow:create', 'workflow:edit'], + licensed: true, + }, + ]; + + roleService.getAllRoles.mockResolvedValue(mockRolesWithoutUsage); + + // + // ACT + // + const response = await memberAgent.get('/roles?withUsageCount=false').expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ + data: { + global: [mockRolesWithoutUsage[0]], // global:admin without usedByUsers + project: [mockRolesWithoutUsage[1]], // project:editor without usedByUsers + credential: [], + workflow: [], + }, + }); + + expect(roleService.getAllRoles).toHaveBeenCalledWith(false); + }); + + it('should default to withUsageCount=false when parameter is omitted', async () => { + // + // ARRANGE + // + const mockRoles: Role[] = [ + { + slug: 'global:admin', + displayName: 'Global Admin', + description: 'Global administrator', + systemRole: true, + roleType: 'global', + scopes: ['user:manage', 'workflow:create'], + licensed: true, + }, + ]; + + roleService.getAllRoles.mockResolvedValue(mockRoles); + + // + // ACT + // + const response = await memberAgent.get('/roles').expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ + data: { + global: [mockRoles[0]], // global:admin without usedByUsers + project: [], + credential: [], + workflow: [], + }, + }); + + expect(roleService.getAllRoles).toHaveBeenCalledWith(false); + }); + + it('should maintain grouped response structure with usage counts for all role types', async () => { + // + // ARRANGE + // + const mockRolesWithUsageAllTypes: Role[] = [ + { + slug: 'global:admin', + displayName: 'Global Admin', + description: 'Global administrator', + systemRole: true, + roleType: 'global', + scopes: ['user:manage', 'workflow:create'], + licensed: true, + usedByUsers: 3, + }, + { + slug: 'project:editor', + displayName: 'Project Editor', + description: 'Project editor role', + systemRole: true, + roleType: 'project', + scopes: ['workflow:create', 'workflow:edit'], + licensed: true, + usedByUsers: 8, + }, + { + slug: 'credential:owner', + displayName: 'Credential Owner', + description: 'Credential owner', + systemRole: true, + roleType: 'credential', + scopes: ['credential:read', 'credential:write'], + licensed: true, + usedByUsers: 15, + }, + { + slug: 'workflow:editor', + displayName: 'Workflow Editor', + description: 'Workflow editor', + systemRole: true, + roleType: 'workflow', + scopes: ['workflow:read', 'workflow:edit'], + licensed: true, + usedByUsers: 7, + }, + ]; + + roleService.getAllRoles.mockResolvedValue(mockRolesWithUsageAllTypes); + + // + // ACT + // + const response = await memberAgent.get('/roles?withUsageCount=true').expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ + data: { + global: [mockRolesWithUsageAllTypes[0]], // global:admin with usedByUsers: 3 + project: [mockRolesWithUsageAllTypes[1]], // project:editor with usedByUsers: 8 + credential: [mockRolesWithUsageAllTypes[2]], // credential:owner with usedByUsers: 15 + workflow: [mockRolesWithUsageAllTypes[3]], // workflow:editor with usedByUsers: 7 + }, + }); + + expect(roleService.getAllRoles).toHaveBeenCalledWith(true); + }); + + it('should handle invalid withUsageCount parameter values gracefully', async () => { + // + // ARRANGE & ACT & ASSERT + // + // Should return 400 for invalid parameter values due to DTO validation + await memberAgent.get('/roles?withUsageCount=invalid').expect(400); + + // Service should not be called when validation fails + expect(roleService.getAllRoles).not.toHaveBeenCalled(); + }); + + it('should work with both member and owner agents when withUsageCount=true', async () => { + // + // ARRANGE + // + const mockRolesWithUsage: Role[] = [ + { + slug: 'project:admin', + displayName: 'Project Admin', + description: 'Project administrator', + systemRole: true, + roleType: 'project', + scopes: ['project:manage'], + licensed: true, + usedByUsers: 4, + }, + ]; + + roleService.getAllRoles.mockResolvedValue(mockRolesWithUsage); + + // + // ACT & ASSERT + // + await ownerAgent.get('/roles?withUsageCount=true').expect(200); + await memberAgent.get('/roles?withUsageCount=true').expect(200); + + expect(roleService.getAllRoles).toHaveBeenNthCalledWith(1, true); + expect(roleService.getAllRoles).toHaveBeenNthCalledWith(2, true); + }); + }); + it('should return empty categories when no roles exist', async () => { // // ARRANGE @@ -250,6 +495,253 @@ describe('RoleController', () => { expect(response.body).toEqual({ data: mockRole }); // Parameter verification skipped - test framework issue }); + + describe('GET /roles/:slug with withUsageCount parameter', () => { + it('should pass withUsageCount=true to service and include usage count in response', async () => { + // + // ARRANGE + // + const roleSlug = 'project:admin'; + const mockRoleWithUsage: Role = { + slug: roleSlug, + displayName: 'Project Admin', + description: 'Project administrator role', + systemRole: true, + roleType: 'project', + scopes: ['project:manage', 'workflow:create'], + licensed: true, + usedByUsers: 8, + }; + + roleService.getRole.mockResolvedValue(mockRoleWithUsage); + + // + // ACT + // + const response = await memberAgent + .get(`/roles/${roleSlug}?withUsageCount=true`) + .expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ data: mockRoleWithUsage }); + expect(roleService.getRole).toHaveBeenCalledTimes(1); + expect(roleService.getRole).toHaveBeenCalledWith(roleSlug, true); + }); + + it('should pass withUsageCount=false to service and exclude usage count', async () => { + // + // ARRANGE + // + const roleSlug = 'project:admin'; + const mockRoleWithoutUsage: Role = { + slug: roleSlug, + displayName: 'Project Admin', + description: 'Project administrator role', + systemRole: true, + roleType: 'project', + scopes: ['project:manage', 'workflow:create'], + licensed: true, + }; + + roleService.getRole.mockResolvedValue(mockRoleWithoutUsage); + + // + // ACT + // + const response = await memberAgent + .get(`/roles/${roleSlug}?withUsageCount=false`) + .expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ data: mockRoleWithoutUsage }); + expect(roleService.getRole).toHaveBeenCalledTimes(1); + expect(roleService.getRole).toHaveBeenCalledWith(roleSlug, false); + }); + + it('should default to withUsageCount=false when parameter is omitted', async () => { + // + // ARRANGE + // + const roleSlug = 'project:admin'; + const mockRoleWithoutUsage: Role = { + slug: roleSlug, + displayName: 'Project Admin', + description: 'Project administrator role', + systemRole: true, + roleType: 'project', + scopes: ['project:manage', 'workflow:create'], + licensed: true, + }; + + roleService.getRole.mockResolvedValue(mockRoleWithoutUsage); + + // + // ACT + // + const response = await memberAgent.get(`/roles/${roleSlug}`).expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ data: mockRoleWithoutUsage }); + expect(roleService.getRole).toHaveBeenCalledTimes(1); + expect(roleService.getRole).toHaveBeenCalledWith(roleSlug, false); + }); + + it('should include usage count in response when withUsageCount=true', async () => { + // + // ARRANGE + // + const roleSlug = 'project:editor'; + const mockRoleWithUsage: Role = { + slug: roleSlug, + displayName: 'Project Editor', + description: 'Project editor role', + systemRole: true, + roleType: 'project', + scopes: ['workflow:create', 'workflow:edit'], + licensed: true, + usedByUsers: 15, + }; + + roleService.getRole.mockResolvedValue(mockRoleWithUsage); + + // + // ACT + // + const response = await memberAgent + .get(`/roles/${roleSlug}?withUsageCount=true`) + .expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ data: mockRoleWithUsage }); + expect(response.body.data.usedByUsers).toBe(15); + expect(roleService.getRole).toHaveBeenCalledTimes(1); + expect(roleService.getRole).toHaveBeenCalledWith(roleSlug, true); + }); + + it('should work with URL-encoded slugs and withUsageCount parameter', async () => { + // + // ARRANGE + // + const roleSlug = 'project:custom-role'; + const encodedSlug = encodeURIComponent(roleSlug); + const mockRoleWithUsage: Role = { + slug: roleSlug, + displayName: 'Custom Role', + description: 'A custom project role', + systemRole: false, + roleType: 'project', + scopes: ['workflow:read'], + licensed: true, + usedByUsers: 3, + }; + + roleService.getRole.mockResolvedValue(mockRoleWithUsage); + + // + // ACT + // + const response = await memberAgent + .get(`/roles/${encodedSlug}?withUsageCount=true`) + .expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ data: mockRoleWithUsage }); + expect(response.body.data.usedByUsers).toBe(3); + expect(roleService.getRole).toHaveBeenCalledTimes(1); + expect(roleService.getRole).toHaveBeenCalledWith(roleSlug, true); + }); + + it('should handle invalid withUsageCount parameter values gracefully', async () => { + // + // ARRANGE + // + const roleSlug = 'project:admin'; + + // + // ACT & ASSERT + // + // Should return 400 for invalid parameter values due to DTO validation + await memberAgent.get(`/roles/${roleSlug}?withUsageCount=invalid`).expect(400); + + // Service should not be called when validation fails + expect(roleService.getRole).not.toHaveBeenCalled(); + }); + + it('should work with both member and owner agents when withUsageCount=true', async () => { + // + // ARRANGE + // + const roleSlug = 'project:viewer'; + const mockRoleWithUsage: Role = { + slug: roleSlug, + displayName: 'Project Viewer', + description: 'Project viewer role', + systemRole: true, + roleType: 'project', + scopes: ['workflow:read'], + licensed: true, + usedByUsers: 22, + }; + + roleService.getRole.mockResolvedValue(mockRoleWithUsage); + + // + // ACT & ASSERT + // + await ownerAgent.get(`/roles/${roleSlug}?withUsageCount=true`).expect(200); + await memberAgent.get(`/roles/${roleSlug}?withUsageCount=true`).expect(200); + + expect(roleService.getRole).toHaveBeenCalledTimes(2); + expect(roleService.getRole).toHaveBeenNthCalledWith(1, roleSlug, true); + expect(roleService.getRole).toHaveBeenNthCalledWith(2, roleSlug, true); + }); + + it('should maintain single role response structure with usage count', async () => { + // + // ARRANGE + // + const roleSlug = 'credential:owner'; + const mockRoleWithUsage: Role = { + slug: roleSlug, + displayName: 'Credential Owner', + description: 'Credential owner role', + systemRole: true, + roleType: 'credential', + scopes: ['credential:read', 'credential:write'], + licensed: true, + usedByUsers: 7, + }; + + roleService.getRole.mockResolvedValue(mockRoleWithUsage); + + // + // ACT + // + const response = await memberAgent + .get(`/roles/${roleSlug}?withUsageCount=true`) + .expect(200); + + // + // ASSERT + // + expect(response.body).toEqual({ data: mockRoleWithUsage }); + expect(response.body.data).toHaveProperty('slug', roleSlug); + expect(response.body.data).toHaveProperty('displayName', 'Credential Owner'); + expect(response.body.data).toHaveProperty('usedByUsers', 7); + expect(roleService.getRole).toHaveBeenCalledTimes(1); + expect(roleService.getRole).toHaveBeenCalledWith(roleSlug, true); + }); + }); }); describe('POST /roles', () => { diff --git a/packages/cli/test/integration/database/repositories/role.repository.test.ts b/packages/cli/test/integration/database/repositories/role.repository.test.ts index 6bb7fd8228..4c7c380e22 100644 --- a/packages/cli/test/integration/database/repositories/role.repository.test.ts +++ b/packages/cli/test/integration/database/repositories/role.repository.test.ts @@ -1,6 +1,6 @@ -import { testDb } from '@n8n/backend-test-utils'; +import { testDb, linkUserToProject, createTeamProject } from '@n8n/backend-test-utils'; import { GlobalConfig } from '@n8n/config'; -import { RoleRepository, ScopeRepository } from '@n8n/db'; +import { AuthRolesService, RoleRepository, ScopeRepository } from '@n8n/db'; import { Container } from '@n8n/di'; import { @@ -9,6 +9,7 @@ import { createCustomRoleWithScopes, createTestScopes, } from '../../shared/db/roles'; +import { createUser } from '../../shared/db/users'; describe('RoleRepository', () => { let roleRepository: RoleRepository; @@ -24,7 +25,7 @@ describe('RoleRepository', () => { // Truncate in the correct order to respect foreign key constraints // user table references role via roleSlug // ProjectRelation references role - await testDb.truncate(['User', 'ProjectRelation', 'Role', 'Scope']); + await testDb.truncate(['User', 'ProjectRelation', 'Project', 'Role', 'Scope']); }); afterAll(async () => { @@ -641,4 +642,186 @@ describe('RoleRepository', () => { }); }); }); + + describe('countUsersWithRole()', () => { + beforeEach(async () => { + // make sure to initalize the default roles for user creation + await Container.get(AuthRolesService).init(); + }); + + describe('global roles', () => { + it('should return 0 when no users have the global role', async () => { + // + // ARRANGE + // + const globalRole = await createRole({ + slug: 'global-empty-role', + displayName: 'Global Empty Role', + roleType: 'global', + }); + + // + // ACT + // + const count = await roleRepository.countUsersWithRole(globalRole); + + // + // ASSERT + // + expect(count).toBe(0); + }); + + it('should return correct count when multiple users have the global role', async () => { + // + // ARRANGE + // + const globalRole = await createRole({ + slug: 'global-multi-role', + displayName: 'Global Multi Role', + roleType: 'global', + }); + + await createUser({ role: globalRole }); + await createUser({ role: globalRole }); + await createUser({ role: globalRole }); + + // Create user with different role to ensure isolation + const otherRole = await createRole({ + slug: 'other-global-role', + displayName: 'Other Global Role', + roleType: 'global', + }); + await createUser({ role: otherRole }); + + // + // ACT + // + const count = await roleRepository.countUsersWithRole(globalRole); + + // + // ASSERT + // + expect(count).toBe(3); + }); + }); + + describe('project roles', () => { + it('should return 0 when no project relations exist for the project role', async () => { + // + // ARRANGE + // + const projectRole = await createRole({ + slug: 'project-empty-role', + displayName: 'Project Empty Role', + roleType: 'project', + }); + + // + // ACT + // + const count = await roleRepository.countUsersWithRole(projectRole); + + // + // ASSERT + // + expect(count).toBe(0); + }); + + it('should return correct count when multiple users have the project role', async () => { + // + // ARRANGE + // + const projectRole = await createRole({ + slug: 'project-multi-role', + displayName: 'Project Multi Role', + roleType: 'project', + }); + + // Create users and projects + const user1 = await createUser(); + const user2 = await createUser(); + const user3 = await createUser(); + const project1 = await createTeamProject('Test Project 1'); + const project2 = await createTeamProject('Test Project 2'); + + // Link users to projects with the target role + await linkUserToProject(user1, project1, projectRole.slug); + await linkUserToProject(user2, project1, projectRole.slug); + await linkUserToProject(user3, project2, projectRole.slug); + + // + // ACT + // + const count = await roleRepository.countUsersWithRole(projectRole); + + // + // ASSERT + // + expect(count).toBe(3); + }); + + it('should only count users with the specific project role slug', async () => { + // + // ARRANGE + // + const targetRole = await createRole({ + slug: 'project-target-role', + displayName: 'Project Target Role', + roleType: 'project', + }); + + const otherRole = await createRole({ + slug: 'project-other-role', + displayName: 'Project Other Role', + roleType: 'project', + }); + + const user1 = await createUser(); + const user2 = await createUser(); + const user3 = await createUser(); + const project = await createTeamProject('Test Project'); + + // Link users with different roles + await linkUserToProject(user1, project, targetRole.slug as any); + await linkUserToProject(user2, project, targetRole.slug as any); + await linkUserToProject(user3, project, otherRole.slug as any); + + // + // ACT + // + const count = await roleRepository.countUsersWithRole(targetRole); + + // + // ASSERT + // + expect(count).toBe(2); + }); + }); + + describe('edge cases', () => { + it('should handle project roles when query returns null count', async () => { + // + // ARRANGE + // + const projectRole = await createRole({ + slug: 'project-null-count-role', + displayName: 'Project Null Count Role', + roleType: 'project', + }); + + // Create a project role but don't link any users to it + // This ensures the query returns a row but with null/0 count + + // + // ACT + // + const count = await roleRepository.countUsersWithRole(projectRole); + + // + // ASSERT + // + expect(count).toBe(0); + }); + }); + }); }); diff --git a/packages/cli/test/integration/role.api.test.ts b/packages/cli/test/integration/role.api.test.ts index 3a926ac5fd..5a54b5584c 100644 --- a/packages/cli/test/integration/role.api.test.ts +++ b/packages/cli/test/integration/role.api.test.ts @@ -23,7 +23,11 @@ function checkForRole(role: Role, roles: Role[]) { role.scopes.sort(); returnedRole!.scopes.sort(); returnedRole!.licensed = role.licensed; - expect(returnedRole).toEqual(role); + expect(returnedRole).toEqual({ + ...role, + createdAt: expect.any(String), + updatedAt: expect.any(String), + }); } beforeAll(async () => { diff --git a/packages/cli/test/integration/services/role.service.test.ts b/packages/cli/test/integration/services/role.service.test.ts index 4d7530599b..1e447aabb8 100644 --- a/packages/cli/test/integration/services/role.service.test.ts +++ b/packages/cli/test/integration/services/role.service.test.ts @@ -136,6 +136,307 @@ describe('RoleService', () => { }); }); + describe('getAllRoles with usage counting', () => { + it('should return roles without usage counts when withCount=false', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const customRole = await createCustomRoleWithScopes( + [testScopes.readScope, testScopes.writeScope], + { + displayName: 'Custom Test Role', + description: 'A custom role for testing', + }, + ); + const systemRole = await createSystemRole({ + displayName: 'System Test Role', + }); + + const mockfindAllRoleCounts = jest.spyOn(roleRepository, 'findAllRoleCounts'); + + // + // ACT + // + const roles = await roleService.getAllRoles(false); + + // + // ASSERT + // + expect(roles).toBeDefined(); + expect(Array.isArray(roles)).toBe(true); + + // Find our test roles + const returnedCustomRole = roles.find((r) => r.slug === customRole.slug); + const returnedSystemRole = roles.find((r) => r.slug === systemRole.slug); + + expect(returnedCustomRole).toBeDefined(); + expect(returnedSystemRole).toBeDefined(); + + // Verify usedByUsers is undefined when withCount=false + expect(returnedCustomRole?.usedByUsers).toBeUndefined(); + expect(returnedSystemRole?.usedByUsers).toBeUndefined(); + + expect(mockfindAllRoleCounts).not.toHaveBeenCalled(); + mockfindAllRoleCounts.mockRestore(); + + // Verify other properties are correct + expect(returnedCustomRole).toMatchObject({ + slug: customRole.slug, + displayName: customRole.displayName, + description: customRole.description, + systemRole: false, + roleType: customRole.roleType, + scopes: expect.any(Array), + licensed: expect.any(Boolean), + }); + }); + + it('should return roles with usage counts when withCount=true', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const customRole = await createCustomRoleWithScopes( + [testScopes.readScope, testScopes.writeScope], + { + displayName: 'Custom Role With Usage', + description: 'A custom role for usage testing', + }, + ); + const systemRole = await createSystemRole({ + displayName: 'System Role With Usage', + }); + + // Mock roleRepository.findAllRoleCounts to return predictable usage counts + const mockfindAllRoleCounts = jest.spyOn(roleRepository, 'findAllRoleCounts'); + mockfindAllRoleCounts.mockResolvedValue({ + [customRole.slug]: 3, + [systemRole.slug]: 1, + }); + + // + // ACT + // + const roles = await roleService.getAllRoles(true); + + // + // ASSERT + // + expect(roles).toBeDefined(); + expect(Array.isArray(roles)).toBe(true); + + // Find our test roles + const returnedCustomRole = roles.find((r) => r.slug === customRole.slug); + const returnedSystemRole = roles.find((r) => r.slug === systemRole.slug); + + expect(returnedCustomRole).toBeDefined(); + expect(returnedSystemRole).toBeDefined(); + + // Verify usedByUsers is included when withCount=true + expect(returnedCustomRole?.usedByUsers).toBe(3); + expect(returnedSystemRole?.usedByUsers).toBe(1); + + // Verify other properties are preserved + expect(returnedCustomRole).toMatchObject({ + slug: customRole.slug, + displayName: customRole.displayName, + description: customRole.description, + systemRole: false, + roleType: customRole.roleType, + scopes: expect.any(Array), + licensed: expect.any(Boolean), + }); + + // Verify findAllRoleCounts was called only once + expect(mockfindAllRoleCounts).toBeCalledTimes(1); + + mockfindAllRoleCounts.mockRestore(); + }); + + it('should return roles with zero usage count', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const unusedRole = await createCustomRoleWithScopes([testScopes.readScope], { + displayName: 'Unused Role', + description: 'A role with no users', + }); + + // Mock roleRepository.findAllRoleCounts to return 0 for all roles + const mockfindAllRoleCounts = jest.spyOn(roleRepository, 'findAllRoleCounts'); + mockfindAllRoleCounts.mockResolvedValue({ [unusedRole.slug]: 0 }); + + // + // ACT + // + const roles = await roleService.getAllRoles(true); + + // + // ASSERT + // + const returnedRole = roles.find((r) => r.slug === unusedRole.slug); + + expect(returnedRole).toBeDefined(); + expect(returnedRole?.usedByUsers).toBe(0); + + mockfindAllRoleCounts.mockRestore(); + }); + + it('should handle mixed system and custom roles with different usage counts', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const customRole1 = await createCustomRoleWithScopes([testScopes.readScope], { + displayName: 'Custom Role 1', + }); + const customRole2 = await createCustomRoleWithScopes([testScopes.writeScope], { + displayName: 'Custom Role 2', + }); + const systemRole = await createSystemRole({ + displayName: 'System Role', + }); + + // Mock different usage counts for each role + const mockfindAllRoleCounts = jest.spyOn(roleRepository, 'findAllRoleCounts'); + mockfindAllRoleCounts.mockResolvedValue({ + [customRole1.slug]: 5, + [customRole2.slug]: 2, + [systemRole.slug]: 10, + }); + + // + // ACT + // + const roles = await roleService.getAllRoles(true); + + // + // ASSERT + // + const returnedCustomRole1 = roles.find((r) => r.slug === customRole1.slug); + const returnedCustomRole2 = roles.find((r) => r.slug === customRole2.slug); + const returnedSystemRole = roles.find((r) => r.slug === systemRole.slug); + + expect(returnedCustomRole1?.usedByUsers).toBe(5); + expect(returnedCustomRole2?.usedByUsers).toBe(2); + expect(returnedSystemRole?.usedByUsers).toBe(10); + + // Verify role types are preserved + expect(returnedCustomRole1?.systemRole).toBe(false); + expect(returnedCustomRole2?.systemRole).toBe(false); + expect(returnedSystemRole?.systemRole).toBe(true); + + expect(mockfindAllRoleCounts).toHaveBeenCalledTimes(1); + mockfindAllRoleCounts.mockRestore(); + }); + + it('should preserve complete role structure when adding usage counts', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const fullRole = await createCustomRoleWithScopes( + [testScopes.readScope, testScopes.writeScope, testScopes.deleteScope], + { + displayName: 'Complete Role', + description: 'A role with full properties', + }, + ); + + // Mock usage count + const mockfindAllRoleCounts = jest.spyOn(roleRepository, 'findAllRoleCounts'); + mockfindAllRoleCounts.mockResolvedValue({ + [fullRole.slug]: 7, + }); + + // + // ACT + // + const roles = await roleService.getAllRoles(true); + + // + // ASSERT + // + const returnedRole = roles.find((r) => r.slug === fullRole.slug); + + expect(returnedRole).toBeDefined(); + expect(returnedRole).toMatchObject({ + slug: fullRole.slug, + displayName: fullRole.displayName, + description: fullRole.description, + systemRole: false, + roleType: fullRole.roleType, + scopes: expect.arrayContaining([ + testScopes.readScope.slug, + testScopes.writeScope.slug, + testScopes.deleteScope.slug, + ]), + licensed: expect.any(Boolean), + usedByUsers: 7, + createdAt: expect.any(Date), + updatedAt: expect.any(Date), + }); + + // Verify all scopes are correctly converted to slugs + expect(returnedRole?.scopes).toHaveLength(3); + + mockfindAllRoleCounts.mockRestore(); + }); + + it('should verify repository findAllRoleCounts is called correctly', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const role1 = await createCustomRoleWithScopes([testScopes.readScope]); + const role2 = await createSystemRole(); + + const mockfindAllRoleCounts = jest.spyOn(roleRepository, 'findAllRoleCounts'); + mockfindAllRoleCounts.mockResolvedValue({ + [role1.slug]: 4, + [role2.slug]: 6, + }); + + // + // ACT + // + await roleService.getAllRoles(true); + + // + // ASSERT + // + // Verify findAllRoleCounts was called only once + expect(mockfindAllRoleCounts).toHaveBeenCalledTimes(1); + + mockfindAllRoleCounts.mockRestore(); + }); + + it('should not call findAllRoleCounts when withCount=false', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + await createCustomRoleWithScopes([testScopes.readScope]); + + const mockfindAllRoleCounts = jest.spyOn(roleRepository, 'findAllRoleCounts'); + + // + // ACT + // + await roleService.getAllRoles(false); + + // + // ASSERT + // + // Verify findAllRoleCounts was never called + expect(mockfindAllRoleCounts).not.toHaveBeenCalled(); + + mockfindAllRoleCounts.mockRestore(); + }); + }); + describe('getRole', () => { it('should return role with licensing information when role exists', async () => { // @@ -180,6 +481,297 @@ describe('RoleService', () => { }); }); + describe('getRole with usage counting', () => { + it('should return role without usage count when withCount=false', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const customRole = await createCustomRoleWithScopes( + [testScopes.readScope, testScopes.writeScope], + { + displayName: 'Custom Test Role', + description: 'A custom role for testing without usage count', + }, + ); + + // + // ACT + // + const result = await roleService.getRole(customRole.slug, false); + + // + // ASSERT + // + expect(result).toMatchObject({ + slug: customRole.slug, + displayName: customRole.displayName, + description: customRole.description, + systemRole: false, + roleType: customRole.roleType, + scopes: expect.arrayContaining([testScopes.readScope.slug, testScopes.writeScope.slug]), + licensed: expect.any(Boolean), + }); + + // Verify usedByUsers is undefined when withCount=false + expect(result.usedByUsers).toBeUndefined(); + }); + + it('should return role with accurate usage count when withCount=true', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const customRole = await createCustomRoleWithScopes([testScopes.adminScope], { + displayName: 'Role With Usage Count', + description: 'A custom role for usage counting testing', + }); + + // Mock roleRepository.countUsersWithRole to return predictable count + const mockCountUsersWithRole = jest.spyOn(roleRepository, 'countUsersWithRole'); + mockCountUsersWithRole.mockResolvedValue(5); + + // + // ACT + // + const result = await roleService.getRole(customRole.slug, true); + + // + // ASSERT + // + expect(result).toMatchObject({ + slug: customRole.slug, + displayName: customRole.displayName, + description: customRole.description, + systemRole: false, + roleType: customRole.roleType, + scopes: expect.arrayContaining([testScopes.adminScope.slug]), + licensed: expect.any(Boolean), + usedByUsers: 5, + }); + + // Verify countUsersWithRole was called with the correct role + expect(mockCountUsersWithRole).toHaveBeenCalledWith( + expect.objectContaining({ slug: customRole.slug }), + ); + + mockCountUsersWithRole.mockRestore(); + }); + + it('should throw NotFoundError regardless of withCount parameter', async () => { + // + // ARRANGE + // + const nonExistentSlug = 'non-existent-role-for-usage-test'; + + // + // ACT & ASSERT + // + await expect(roleService.getRole(nonExistentSlug, false)).rejects.toThrow(NotFoundError); + await expect(roleService.getRole(nonExistentSlug, false)).rejects.toThrow('Role not found'); + + await expect(roleService.getRole(nonExistentSlug, true)).rejects.toThrow(NotFoundError); + await expect(roleService.getRole(nonExistentSlug, true)).rejects.toThrow('Role not found'); + }); + + it('should work with system roles and usage counting', async () => { + // + // ARRANGE + // + const systemRole = await createSystemRole({ + displayName: 'System Role With Usage', + description: 'A system role for usage testing', + }); + + // Mock higher usage count for system role + const mockCountUsersWithRole = jest.spyOn(roleRepository, 'countUsersWithRole'); + mockCountUsersWithRole.mockResolvedValue(12); + + // + // ACT + // + const result = await roleService.getRole(systemRole.slug, true); + + // + // ASSERT + // + expect(result).toMatchObject({ + slug: systemRole.slug, + displayName: systemRole.displayName, + description: systemRole.description, + systemRole: true, + roleType: systemRole.roleType, + scopes: expect.any(Array), + licensed: expect.any(Boolean), + usedByUsers: 12, + }); + + // Verify system role properties are preserved + expect(result.systemRole).toBe(true); + + mockCountUsersWithRole.mockRestore(); + }); + + it('should return role with zero usage count', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const unusedRole = await createCustomRoleWithScopes([testScopes.readScope], { + displayName: 'Unused Role', + description: 'A role with no assigned users', + }); + + // Mock countUsersWithRole to return 0 + const mockCountUsersWithRole = jest.spyOn(roleRepository, 'countUsersWithRole'); + mockCountUsersWithRole.mockResolvedValue(0); + + // + // ACT + // + const result = await roleService.getRole(unusedRole.slug, true); + + // + // ASSERT + // + expect(result).toMatchObject({ + slug: unusedRole.slug, + displayName: unusedRole.displayName, + description: unusedRole.description, + systemRole: false, + roleType: unusedRole.roleType, + scopes: expect.arrayContaining([testScopes.readScope.slug]), + licensed: expect.any(Boolean), + usedByUsers: 0, + }); + + mockCountUsersWithRole.mockRestore(); + }); + + it('should preserve complete role structure with usage count', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const fullRole = await createCustomRoleWithScopes( + [testScopes.readScope, testScopes.writeScope, testScopes.deleteScope], + { + displayName: 'Complete Role Structure', + description: 'A role with full properties for structure verification', + }, + ); + + // Mock usage count + const mockCountUsersWithRole = jest.spyOn(roleRepository, 'countUsersWithRole'); + mockCountUsersWithRole.mockResolvedValue(8); + + // + // ACT + // + const result = await roleService.getRole(fullRole.slug, true); + + // + // ASSERT + // + expect(result).toMatchObject({ + slug: fullRole.slug, + displayName: fullRole.displayName, + description: fullRole.description, + systemRole: false, + roleType: fullRole.roleType, + scopes: expect.arrayContaining([ + testScopes.readScope.slug, + testScopes.writeScope.slug, + testScopes.deleteScope.slug, + ]), + licensed: expect.any(Boolean), + usedByUsers: 8, + createdAt: expect.any(Date), + updatedAt: expect.any(Date), + }); + + // Verify all scopes are included + expect(result.scopes).toHaveLength(3); + + mockCountUsersWithRole.mockRestore(); + }); + + it('should verify countUsersWithRole is called only when withCount=true', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const testRole = await createCustomRoleWithScopes([testScopes.readScope]); + + const mockCountUsersWithRole = jest.spyOn(roleRepository, 'countUsersWithRole'); + mockCountUsersWithRole.mockResolvedValue(3); + + // + // ACT + // + // Test with withCount=false + await roleService.getRole(testRole.slug, false); + + // Test with withCount=true + await roleService.getRole(testRole.slug, true); + + // + // ASSERT + // + // Verify countUsersWithRole was called only once (for withCount=true) + expect(mockCountUsersWithRole).toHaveBeenCalledTimes(1); + expect(mockCountUsersWithRole).toHaveBeenCalledWith( + expect.objectContaining({ slug: testRole.slug }), + ); + + mockCountUsersWithRole.mockRestore(); + }); + + it('should verify repository integration with different usage counts', async () => { + // + // ARRANGE + // + const testScopes = await createTestScopes(); + const role1 = await createCustomRoleWithScopes([testScopes.readScope], { + displayName: 'Role One', + }); + const role2 = await createCustomRoleWithScopes([testScopes.writeScope], { + displayName: 'Role Two', + }); + + // Mock different usage counts for different roles + const mockCountUsersWithRole = jest.spyOn(roleRepository, 'countUsersWithRole'); + mockCountUsersWithRole.mockImplementation(async (role) => { + if (role.slug === role1.slug) return 15; + if (role.slug === role2.slug) return 3; + return 0; + }); + + // + // ACT + // + const result1 = await roleService.getRole(role1.slug, true); + const result2 = await roleService.getRole(role2.slug, true); + + // + // ASSERT + // + expect(result1.usedByUsers).toBe(15); + expect(result2.usedByUsers).toBe(3); + + // Verify each role was queried correctly + expect(mockCountUsersWithRole).toHaveBeenCalledTimes(2); + expect(mockCountUsersWithRole).toHaveBeenCalledWith( + expect.objectContaining({ slug: role1.slug }), + ); + expect(mockCountUsersWithRole).toHaveBeenCalledWith( + expect.objectContaining({ slug: role2.slug }), + ); + + mockCountUsersWithRole.mockRestore(); + }); + }); + describe('createCustomRole', () => { it('should create custom role with valid data', async () => { //