mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-23 21:06:51 +02:00
chore(core): Ensure unique display name for roles (#20601)
This commit is contained in:
Andreas Fitzek
GitHub
parent
a74c08b012
commit
e73d0f4137
|
|
@ -24,6 +24,11 @@ Runs a single migration by name.
|
|||
**Throws:**
|
||||
- `UnexpectedError` if the migration is not found or database is not initialized
|
||||
|
||||
## `undoLastSingleMigration(): Promise<void>`
|
||||
|
||||
Undoes the last single migration.
|
||||
|
||||
|
||||
## Usage Example
|
||||
|
||||
```typescript
|
||||
|
|
|
|||
|
|
@ -90,6 +90,17 @@ export async function initDbUpToMigration(beforeMigrationName: string): Promise<
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Undo the last single migration down.
|
||||
* Useful for testing the down path of a specific migration after inserting test data.
|
||||
*/
|
||||
export async function undoLastSingleMigration(): Promise<void> {
|
||||
const dataSource = Container.get(DataSource);
|
||||
await dataSource.undoLastMigration({
|
||||
transaction: 'each',
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a single migration by name.
|
||||
* Useful for testing a specific migration after inserting test data.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,64 @@
|
|||
import type { Role } from '../../entities';
|
||||
import type { MigrationContext, ReversibleMigration } from '../migration-types';
|
||||
|
||||
export class UniqueRoleNames1760020838000 implements ReversibleMigration {
|
||||
async up({ isMysql, escape, runQuery }: MigrationContext) {
|
||||
const tableName = escape.tableName('role');
|
||||
const displayNameColumn = escape.columnName('displayName');
|
||||
const slugColumn = escape.columnName('slug');
|
||||
const createdAtColumn = escape.columnName('createdAt');
|
||||
const allRoles: Array<Pick<Role, 'slug' | 'displayName'>> = await runQuery(
|
||||
`SELECT ${slugColumn}, ${displayNameColumn} FROM ${tableName} ORDER BY ${displayNameColumn}, ${createdAtColumn} ASC`,
|
||||
);
|
||||
|
||||
// Group roles by displayName in memory
|
||||
const groupedByName = new Map<string, Array<Pick<Role, 'slug' | 'displayName'>>>();
|
||||
|
||||
for (const role of allRoles) {
|
||||
const existing = groupedByName.get(role.displayName) || [];
|
||||
existing.push(role);
|
||||
groupedByName.set(role.displayName, existing);
|
||||
}
|
||||
|
||||
for (const [_, roles] of groupedByName.entries()) {
|
||||
if (roles.length > 1) {
|
||||
const duplicates = roles.slice(1);
|
||||
let index = 2;
|
||||
for (const role of duplicates.values()) {
|
||||
let newDisplayName = `${role.displayName} ${index}`;
|
||||
while (allRoles.some((r) => r.displayName === newDisplayName)) {
|
||||
index++;
|
||||
newDisplayName = `${role.displayName} ${index}`;
|
||||
}
|
||||
await runQuery(
|
||||
`UPDATE ${tableName} SET ${displayNameColumn} = :displayName WHERE ${slugColumn} = :slug`,
|
||||
{
|
||||
displayName: newDisplayName,
|
||||
slug: role.slug,
|
||||
},
|
||||
);
|
||||
index++;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const indexName = escape.indexName('UniqueRoleDisplayName');
|
||||
// MySQL cannot create an index on a column with a type of TEXT or BLOB without a length limit
|
||||
// The (100) specifies the maximum length of the index key
|
||||
// meaning that only the first 100 characters of the displayName column will be used for indexing
|
||||
// But since in our DTOs we limit the displayName to 100 characters, we can safely use this prefix length
|
||||
await runQuery(
|
||||
isMysql
|
||||
? `CREATE UNIQUE INDEX ${indexName} ON ${tableName} (${displayNameColumn}(100))`
|
||||
: `CREATE UNIQUE INDEX ${indexName} ON ${tableName} (${displayNameColumn})`,
|
||||
);
|
||||
}
|
||||
|
||||
async down({ isMysql, escape, runQuery }: MigrationContext) {
|
||||
const tableName = escape.tableName('role');
|
||||
const indexName = escape.indexName('UniqueRoleDisplayName');
|
||||
await runQuery(
|
||||
isMysql ? `ALTER TABLE ${tableName} DROP INDEX ${indexName}` : `DROP INDEX ${indexName}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -101,6 +101,7 @@ import { ReplaceDataStoreTablesWithDataTables1754475614602 } from '../common/175
|
|||
import { AddTimestampsToRoleAndRoleIndexes1756906557570 } from '../common/1756906557570-AddTimestampsToRoleAndRoleIndexes';
|
||||
import { ChangeValueTypesForInsights1759399811000 } from '../common/1759399811000-ChangeValueTypesForInsights';
|
||||
import { CreateChatHubTables1760019379982 } from '../common/1760019379982-CreateChatHubTables';
|
||||
import { UniqueRoleNames1760020838000 } from '../common/1760020838000-UniqueRoleNames';
|
||||
import type { Migration } from '../migration-types';
|
||||
import { UpdateParentFolderIdColumn1740445074052 } from '../mysqldb/1740445074052-UpdateParentFolderIdColumn';
|
||||
|
||||
|
|
@ -209,4 +210,5 @@ export const mysqlMigrations: Migration[] = [
|
|||
AddAudienceColumnToApiKeys1758731786132,
|
||||
ChangeValueTypesForInsights1759399811000,
|
||||
CreateChatHubTables1760019379982,
|
||||
UniqueRoleNames1760020838000,
|
||||
];
|
||||
|
|
|
|||
|
|
@ -101,6 +101,7 @@ import { AddTimestampsToRoleAndRoleIndexes1756906557570 } from '../common/175690
|
|||
import { AddAudienceColumnToApiKeys1758731786132 } from '../common/1758731786132-AddAudienceColumnToApiKey';
|
||||
import { ChangeValueTypesForInsights1759399811000 } from '../common/1759399811000-ChangeValueTypesForInsights';
|
||||
import { CreateChatHubTables1760019379982 } from '../common/1760019379982-CreateChatHubTables';
|
||||
import { UniqueRoleNames1760020838000 } from '../common/1760020838000-UniqueRoleNames';
|
||||
import type { Migration } from '../migration-types';
|
||||
|
||||
export const postgresMigrations: Migration[] = [
|
||||
|
|
@ -207,4 +208,5 @@ export const postgresMigrations: Migration[] = [
|
|||
AddAudienceColumnToApiKeys1758731786132,
|
||||
ChangeValueTypesForInsights1759399811000,
|
||||
CreateChatHubTables1760019379982,
|
||||
UniqueRoleNames1760020838000,
|
||||
];
|
||||
|
|
|
|||
|
|
@ -95,6 +95,7 @@ import { CreateDataStoreTables1754475614601 } from '../common/1754475614601-Crea
|
|||
import { ReplaceDataStoreTablesWithDataTables1754475614602 } from '../common/1754475614602-ReplaceDataStoreTablesWithDataTables';
|
||||
import { AddTimestampsToRoleAndRoleIndexes1756906557570 } from '../common/1756906557570-AddTimestampsToRoleAndRoleIndexes';
|
||||
import { ChangeValueTypesForInsights1759399811000 } from '../common/1759399811000-ChangeValueTypesForInsights';
|
||||
import { UniqueRoleNames1760020838000 } from '../common/1760020838000-UniqueRoleNames';
|
||||
import type { Migration } from '../migration-types';
|
||||
import { LinkRoleToProjectRelationTable1753953244168 } from './../common/1753953244168-LinkRoleToProjectRelationTable';
|
||||
import { AddProjectIdToVariableTable1758794506893 } from './1758794506893-AddProjectIdToVariableTable';
|
||||
|
|
@ -201,6 +202,7 @@ const sqliteMigrations: Migration[] = [
|
|||
AddAudienceColumnToApiKeys1758731786132,
|
||||
ChangeValueTypesForInsights1759399811000,
|
||||
CreateChatHubTables1760019379982,
|
||||
UniqueRoleNames1760020838000,
|
||||
];
|
||||
|
||||
export { sqliteMigrations };
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ import { UnexpectedError, UserError } from 'n8n-workflow';
|
|||
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||
import { NotFoundError } from '@/errors/response-errors/not-found.error';
|
||||
import { RoleCacheService } from './role-cache.service';
|
||||
import { isUniqueConstraintError } from '@/response-helper';
|
||||
|
||||
@Service()
|
||||
export class RoleService {
|
||||
|
|
@ -143,6 +144,11 @@ export class RoleService {
|
|||
if (error instanceof UserError && error.message === 'Cannot update system roles') {
|
||||
throw new BadRequestError('Cannot update system roles');
|
||||
}
|
||||
|
||||
if (error instanceof Error && isUniqueConstraintError(error)) {
|
||||
throw new BadRequestError(`A role with the name "${displayName}" already exists`);
|
||||
}
|
||||
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
|
@ -159,12 +165,20 @@ export class RoleService {
|
|||
role.systemRole = false;
|
||||
role.roleType = newRole.roleType;
|
||||
role.slug = `${newRole.roleType}:${newRole.displayName.toLowerCase().replace(/[^a-z0-9]+/g, '-')}-${Math.random().toString(36).substring(2, 8)}`;
|
||||
const createdRole = await this.roleRepository.save(role);
|
||||
|
||||
// Invalidate cache after role creation
|
||||
await this.roleCacheService.invalidateCache();
|
||||
try {
|
||||
const createdRole = await this.roleRepository.save(role);
|
||||
|
||||
return this.dbRoleToRoleDTO(createdRole);
|
||||
// Invalidate cache after role creation
|
||||
await this.roleCacheService.invalidateCache();
|
||||
|
||||
return this.dbRoleToRoleDTO(createdRole);
|
||||
} catch (error) {
|
||||
if (error instanceof Error && isUniqueConstraintError(error)) {
|
||||
throw new BadRequestError(`A role with the name "${newRole.displayName}" already exists`);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
async checkRolesExist(
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import {
|
|||
randomCredentialPayload,
|
||||
createWorkflow,
|
||||
mockInstance,
|
||||
testDb,
|
||||
} from '@n8n/backend-test-utils';
|
||||
import type { Project, User, Role } from '@n8n/db';
|
||||
|
||||
|
|
@ -126,6 +127,7 @@ describe('Cross-Project Access Control Tests', () => {
|
|||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await testDb.truncate(['User']);
|
||||
await cleanupRolesAndScopes();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -166,6 +166,7 @@ describe('Custom Role Functionality Tests', () => {
|
|||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await testDb.truncate(['User']);
|
||||
await cleanupRolesAndScopes();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -135,6 +135,7 @@ describe('Resource Access Control Matrix Tests', () => {
|
|||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await testDb.truncate(['User']);
|
||||
await cleanupRolesAndScopes();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -10,7 +10,9 @@ import {
|
|||
PROJECT_EDITOR_ROLE,
|
||||
PROJECT_OWNER_ROLE,
|
||||
PROJECT_VIEWER_ROLE,
|
||||
RoleRepository,
|
||||
} from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
|
||||
describe('RoleController - Integration Tests', () => {
|
||||
const testServer = setupTestServer({ endpointGroups: ['role'] });
|
||||
|
|
@ -32,6 +34,10 @@ describe('RoleController - Integration Tests', () => {
|
|||
|
||||
afterEach(async () => {
|
||||
await cleanupRolesAndScopes();
|
||||
// Clear custom roles
|
||||
await Container.get(RoleRepository).delete({
|
||||
systemRole: false,
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
|
|
|
|||
|
|
@ -53,8 +53,8 @@ afterAll(async () => {
|
|||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await cleanupRolesAndScopes();
|
||||
await testDb.truncate(['User']);
|
||||
await cleanupRolesAndScopes();
|
||||
});
|
||||
|
||||
describe('RoleService', () => {
|
||||
|
|
@ -909,6 +909,25 @@ describe('RoleService', () => {
|
|||
expect(result.slug).toContain('role');
|
||||
expect(result.slug).toContain('name');
|
||||
});
|
||||
|
||||
it('should throw BadRequestError when a role with the same display name already exists', async () => {
|
||||
const testScopes = await createTestScopes();
|
||||
const createRoleDto: CreateRoleDto = {
|
||||
displayName: 'Existing Role',
|
||||
roleType: 'project',
|
||||
scopes: [testScopes.readScope.slug],
|
||||
};
|
||||
|
||||
await roleService.createCustomRole(createRoleDto);
|
||||
|
||||
const duplicateRoleDto: CreateRoleDto = {
|
||||
displayName: 'Existing Role',
|
||||
roleType: 'project',
|
||||
scopes: [testScopes.writeScope.slug],
|
||||
};
|
||||
|
||||
await expect(roleService.createCustomRole(duplicateRoleDto)).rejects.toThrow(BadRequestError);
|
||||
});
|
||||
});
|
||||
|
||||
describe('updateCustomRole', () => {
|
||||
|
|
@ -1032,6 +1051,25 @@ describe('RoleService', () => {
|
|||
'The following scopes are invalid: invalid:scope',
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw error when a role with the same display name already exists', async () => {
|
||||
//
|
||||
// ARRANGE
|
||||
//
|
||||
const existingRole = await createRole();
|
||||
const otherExistingRole = await createRole();
|
||||
|
||||
const updateRoleDto: UpdateRoleDto = {
|
||||
displayName: existingRole.displayName,
|
||||
};
|
||||
|
||||
//
|
||||
// ACT & ASSERT
|
||||
//
|
||||
await expect(
|
||||
roleService.updateCustomRole(otherExistingRole.slug, updateRoleDto),
|
||||
).rejects.toThrow(`A role with the name "${existingRole.displayName}" already exists`);
|
||||
});
|
||||
});
|
||||
|
||||
describe('removeCustomRole', () => {
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ export async function createRole(overrides: Partial<Role> = {}): Promise<Role> {
|
|||
|
||||
const defaultRole: Partial<Role> = {
|
||||
slug: `test-role-${Math.random().toString(36).substring(7)}`,
|
||||
displayName: 'Test Role',
|
||||
displayName: `Test Role ${Math.random().toString(36).substring(7)}`,
|
||||
description: 'A test role for integration testing',
|
||||
systemRole: false,
|
||||
roleType: 'project',
|
||||
|
|
@ -164,11 +164,7 @@ export async function cleanupRolesAndScopes(): Promise<void> {
|
|||
.getMany();
|
||||
|
||||
for (const role of testRoles) {
|
||||
try {
|
||||
await roleRepository.delete({ slug: role.slug });
|
||||
} catch (error) {
|
||||
// Ignore errors for system roles or roles with dependencies
|
||||
}
|
||||
await roleRepository.delete({ slug: role.slug });
|
||||
}
|
||||
|
||||
// Delete test scopes
|
||||
|
|
@ -178,10 +174,6 @@ export async function cleanupRolesAndScopes(): Promise<void> {
|
|||
.getMany();
|
||||
|
||||
for (const scope of testScopes) {
|
||||
try {
|
||||
await scopeRepository.delete({ slug: scope.slug });
|
||||
} catch (error) {
|
||||
// Ignore errors for scopes with dependencies
|
||||
}
|
||||
await scopeRepository.delete({ slug: scope.slug });
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1644,7 +1644,7 @@ describe('PATCH /users/:id/role', () => {
|
|||
|
||||
test('should change to existing custom role', async () => {
|
||||
const customRole = 'custom:role';
|
||||
await createRole({ slug: customRole, displayName: 'Custom Role', roleType: 'global' });
|
||||
await createRole({ slug: customRole, displayName: 'Custom Role 1', roleType: 'global' });
|
||||
const response = await ownerAgent.patch(`/users/${member.id}/role`).send({
|
||||
newRoleName: customRole,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -0,0 +1,438 @@
|
|||
import {
|
||||
createTestMigrationContext,
|
||||
initDbUpToMigration,
|
||||
runSingleMigration,
|
||||
undoLastSingleMigration,
|
||||
type TestMigrationContext,
|
||||
} from '@n8n/backend-test-utils';
|
||||
import { DbConnection } from '@n8n/db';
|
||||
import { Container } from '@n8n/di';
|
||||
import { DataSource } from '@n8n/typeorm';
|
||||
|
||||
const MIGRATION_NAME = 'UniqueRoleNames1760020838000';
|
||||
|
||||
interface RoleData {
|
||||
slug: string;
|
||||
displayName: string;
|
||||
createdAt: Date;
|
||||
systemRole?: boolean;
|
||||
roleType?: string;
|
||||
description?: string | null;
|
||||
}
|
||||
|
||||
interface RoleRow {
|
||||
slug: string;
|
||||
displayName: string;
|
||||
createdAt: Date;
|
||||
}
|
||||
|
||||
describe('UniqueRoleNames Migration', () => {
|
||||
let dataSource: DataSource;
|
||||
|
||||
beforeAll(async () => {
|
||||
// Initialize DB connection without running migrations
|
||||
const dbConnection = Container.get(DbConnection);
|
||||
await dbConnection.init();
|
||||
|
||||
dataSource = Container.get(DataSource);
|
||||
|
||||
// Run migrations up to (but not including) target migration
|
||||
await initDbUpToMigration(MIGRATION_NAME);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
const dbConnection = Container.get(DbConnection);
|
||||
await dbConnection.close();
|
||||
});
|
||||
|
||||
/**
|
||||
* Helper function to insert a test role with controlled timestamp
|
||||
*/
|
||||
async function insertTestRole(context: TestMigrationContext, roleData: RoleData): Promise<void> {
|
||||
const tableName = context.escape.tableName('role');
|
||||
const slugColumn = context.escape.columnName('slug');
|
||||
const displayNameColumn = context.escape.columnName('displayName');
|
||||
const createdAtColumn = context.escape.columnName('createdAt');
|
||||
const updatedAtColumn = context.escape.columnName('updatedAt');
|
||||
const systemRoleColumn = context.escape.columnName('systemRole');
|
||||
const roleTypeColumn = context.escape.columnName('roleType');
|
||||
const descriptionColumn = context.escape.columnName('description');
|
||||
|
||||
const systemRole = roleData.systemRole ?? false;
|
||||
const roleType = roleData.roleType ?? 'project';
|
||||
const description = roleData.description ?? null;
|
||||
|
||||
await context.queryRunner.query(
|
||||
`INSERT INTO ${tableName} (${slugColumn}, ${displayNameColumn}, ${createdAtColumn}, ${updatedAtColumn}, ${systemRoleColumn}, ${roleTypeColumn}, ${descriptionColumn}) VALUES (?, ?, ?, ?, ?, ?, ?)`,
|
||||
[
|
||||
roleData.slug,
|
||||
roleData.displayName,
|
||||
roleData.createdAt,
|
||||
roleData.createdAt,
|
||||
systemRole,
|
||||
roleType,
|
||||
description,
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper function to retrieve all roles ordered by creation date
|
||||
*/
|
||||
async function getAllRoles(context: TestMigrationContext): Promise<RoleRow[]> {
|
||||
const tableName = context.escape.tableName('role');
|
||||
const slugColumn = context.escape.columnName('slug');
|
||||
const displayNameColumn = context.escape.columnName('displayName');
|
||||
const createdAtColumn = context.escape.columnName('createdAt');
|
||||
|
||||
const roles = await context.queryRunner.query(
|
||||
`SELECT ${slugColumn} as slug, ${displayNameColumn} as displayName, ${createdAtColumn} as createdAt FROM ${tableName} ORDER BY ${createdAtColumn} ASC`,
|
||||
);
|
||||
|
||||
return roles;
|
||||
}
|
||||
|
||||
describe('Schema Migration', () => {
|
||||
it('should create unique index and correctly rename all duplicate roles', async () => {
|
||||
// Create migration context for schema queries
|
||||
const context = createTestMigrationContext(dataSource);
|
||||
|
||||
// Test Scenario 1: 3 roles with same displayName "Duplicate Name"
|
||||
await insertTestRole(context, {
|
||||
slug: 'test-role-oldest',
|
||||
displayName: 'Duplicate Name',
|
||||
createdAt: new Date('2024-01-01T00:00:00.000Z'),
|
||||
});
|
||||
await insertTestRole(context, {
|
||||
slug: 'test-role-middle',
|
||||
displayName: 'Duplicate Name',
|
||||
createdAt: new Date('2024-01-02T00:00:00.000Z'),
|
||||
});
|
||||
await insertTestRole(context, {
|
||||
slug: 'test-role-newest',
|
||||
displayName: 'Duplicate Name',
|
||||
createdAt: new Date('2024-01-03T00:00:00.000Z'),
|
||||
});
|
||||
|
||||
// Test Scenario 2: 2 duplicate "Editor" roles
|
||||
await insertTestRole(context, {
|
||||
slug: 'editor-first',
|
||||
displayName: 'Editor',
|
||||
createdAt: new Date('2025-01-01T00:00:00.000Z'),
|
||||
});
|
||||
await insertTestRole(context, {
|
||||
slug: 'editor-second',
|
||||
displayName: 'Editor',
|
||||
createdAt: new Date('2025-01-02T00:00:00.000Z'),
|
||||
});
|
||||
|
||||
// Test Scenario 3: 5 duplicate "Manager" roles
|
||||
for (let i = 1; i <= 5; i++) {
|
||||
await insertTestRole(context, {
|
||||
slug: `manager-${i}`,
|
||||
displayName: 'Manager',
|
||||
createdAt: new Date(`2025-02-0${i}T00:00:00.000Z`),
|
||||
});
|
||||
}
|
||||
|
||||
// Test Scenario 4: Multiple independent duplicate groups
|
||||
// Group 1: 3 "Admin" roles
|
||||
await insertTestRole(context, {
|
||||
slug: 'admin-1',
|
||||
displayName: 'Admin',
|
||||
createdAt: new Date('2025-03-01T00:00:00.000Z'),
|
||||
});
|
||||
await insertTestRole(context, {
|
||||
slug: 'admin-2',
|
||||
displayName: 'Admin',
|
||||
createdAt: new Date('2025-03-02T00:00:00.000Z'),
|
||||
});
|
||||
await insertTestRole(context, {
|
||||
slug: 'admin-3',
|
||||
displayName: 'Admin',
|
||||
createdAt: new Date('2025-03-03T00:00:00.000Z'),
|
||||
});
|
||||
|
||||
// Group 2: 2 "Reviewer" roles
|
||||
await insertTestRole(context, {
|
||||
slug: 'reviewer-1',
|
||||
displayName: 'Reviewer',
|
||||
createdAt: new Date('2025-03-04T00:00:00.000Z'),
|
||||
});
|
||||
await insertTestRole(context, {
|
||||
slug: 'reviewer-2',
|
||||
displayName: 'Reviewer',
|
||||
createdAt: new Date('2025-03-05T00:00:00.000Z'),
|
||||
});
|
||||
|
||||
// Check conflict with generated display name conflict
|
||||
await insertTestRole(context, {
|
||||
slug: 'reviewer-3',
|
||||
displayName: 'Reviewer 2',
|
||||
createdAt: new Date('2025-03-05T00:00:00.000Z'),
|
||||
});
|
||||
|
||||
// Group 3: 1 "Viewer" role (no duplicates)
|
||||
await insertTestRole(context, {
|
||||
slug: 'viewer-1',
|
||||
displayName: 'Viewer',
|
||||
createdAt: new Date('2025-03-06T00:00:00.000Z'),
|
||||
});
|
||||
|
||||
// Verify pre-migration state - all roles exist with original displayNames
|
||||
const beforeRoles = await getAllRoles(context);
|
||||
expect(beforeRoles.filter((r) => r.displayName === 'Duplicate Name')).toHaveLength(3);
|
||||
expect(beforeRoles.filter((r) => r.displayName === 'Editor')).toHaveLength(2);
|
||||
expect(beforeRoles.filter((r) => r.displayName === 'Manager')).toHaveLength(5);
|
||||
expect(beforeRoles.filter((r) => r.displayName === 'Admin')).toHaveLength(3);
|
||||
expect(beforeRoles.filter((r) => r.displayName === 'Reviewer')).toHaveLength(2);
|
||||
expect(beforeRoles.filter((r) => r.displayName === 'Viewer')).toHaveLength(1);
|
||||
|
||||
// Run the migration
|
||||
await runSingleMigration(MIGRATION_NAME);
|
||||
|
||||
// Release old query runner before creating new one
|
||||
await context.queryRunner.release();
|
||||
|
||||
// Create fresh context after migration
|
||||
const postMigrationContext = createTestMigrationContext(dataSource);
|
||||
|
||||
const tableName = postMigrationContext.escape.tableName('role');
|
||||
const displayNameColumn = postMigrationContext.escape.columnName('displayName');
|
||||
const slugColumn = postMigrationContext.escape.columnName('slug');
|
||||
const indexName = postMigrationContext.escape.indexName('UniqueRoleDisplayName');
|
||||
|
||||
// Verify all duplicate roles were renamed correctly
|
||||
const afterRoles = await getAllRoles(postMigrationContext);
|
||||
|
||||
// Test Scenario 1: 3 "Duplicate Name" roles
|
||||
const oldestRole = afterRoles.find((r) => r.slug === 'test-role-oldest');
|
||||
const middleRole = afterRoles.find((r) => r.slug === 'test-role-middle');
|
||||
const newestRole = afterRoles.find((r) => r.slug === 'test-role-newest');
|
||||
expect(oldestRole?.displayName).toBe('Duplicate Name'); // Oldest keeps original
|
||||
expect(middleRole?.displayName).toBe('Duplicate Name 2'); // Second gets " 2"
|
||||
expect(newestRole?.displayName).toBe('Duplicate Name 3'); // Third gets " 3"
|
||||
|
||||
// Test Scenario 2: 2 "Editor" roles
|
||||
const editorFirst = afterRoles.find((r) => r.slug === 'editor-first');
|
||||
const editorSecond = afterRoles.find((r) => r.slug === 'editor-second');
|
||||
expect(editorFirst?.displayName).toBe('Editor'); // Oldest keeps original
|
||||
expect(editorSecond?.displayName).toBe('Editor 2'); // Second gets " 2"
|
||||
|
||||
// Test Scenario 3: 5 "Manager" roles
|
||||
const manager1 = afterRoles.find((r) => r.slug === 'manager-1');
|
||||
const manager2 = afterRoles.find((r) => r.slug === 'manager-2');
|
||||
const manager3 = afterRoles.find((r) => r.slug === 'manager-3');
|
||||
const manager4 = afterRoles.find((r) => r.slug === 'manager-4');
|
||||
const manager5 = afterRoles.find((r) => r.slug === 'manager-5');
|
||||
expect(manager1?.displayName).toBe('Manager'); // Oldest keeps original
|
||||
expect(manager2?.displayName).toBe('Manager 2');
|
||||
expect(manager3?.displayName).toBe('Manager 3');
|
||||
expect(manager4?.displayName).toBe('Manager 4');
|
||||
expect(manager5?.displayName).toBe('Manager 5');
|
||||
|
||||
// Test Scenario 4: Multiple independent groups
|
||||
const admin1 = afterRoles.find((r) => r.slug === 'admin-1');
|
||||
const admin2 = afterRoles.find((r) => r.slug === 'admin-2');
|
||||
const admin3 = afterRoles.find((r) => r.slug === 'admin-3');
|
||||
expect(admin1?.displayName).toBe('Admin');
|
||||
expect(admin2?.displayName).toBe('Admin 2');
|
||||
expect(admin3?.displayName).toBe('Admin 3');
|
||||
|
||||
const reviewer1 = afterRoles.find((r) => r.slug === 'reviewer-1');
|
||||
const reviewer2 = afterRoles.find((r) => r.slug === 'reviewer-2');
|
||||
expect(reviewer1?.displayName).toBe('Reviewer');
|
||||
expect(reviewer2?.displayName).toBe('Reviewer 3');
|
||||
|
||||
const reviewer3 = afterRoles.find((r) => r.slug === 'reviewer-3');
|
||||
expect(reviewer3?.displayName).toBe('Reviewer 2');
|
||||
|
||||
const viewer1 = afterRoles.find((r) => r.slug === 'viewer-1');
|
||||
expect(viewer1?.displayName).toBe('Viewer'); // Unchanged (no duplicates)
|
||||
|
||||
// Verify unique index exists based on database type
|
||||
if (postMigrationContext.isSqlite) {
|
||||
const indexes = await postMigrationContext.queryRunner.query(
|
||||
`PRAGMA index_list(${tableName})`,
|
||||
);
|
||||
const uniqueIndex = indexes.find(
|
||||
(idx: { name: string; unique: number }) =>
|
||||
idx.name.includes('UniqueRoleDisplayName') && idx.unique === 1,
|
||||
);
|
||||
expect(uniqueIndex).toBeDefined();
|
||||
} else if (postMigrationContext.isPostgres) {
|
||||
const result = await postMigrationContext.queryRunner.query(
|
||||
`SELECT indexname FROM pg_indexes WHERE tablename = ${tableName} AND indexname = ${indexName}`,
|
||||
);
|
||||
expect(result).toHaveLength(1);
|
||||
|
||||
// Verify index is unique
|
||||
const uniqueCheck = await postMigrationContext.queryRunner.query(
|
||||
`SELECT i.relname as index_name, ix.indisunique
|
||||
FROM pg_class t
|
||||
JOIN pg_index ix ON t.oid = ix.indrelid
|
||||
JOIN pg_class i ON i.oid = ix.indexrelid
|
||||
WHERE t.relname = ${tableName} AND i.relname = ${indexName}`,
|
||||
);
|
||||
expect(uniqueCheck[0].indisunique).toBe(true);
|
||||
} else if (postMigrationContext.isMysql) {
|
||||
const result = await postMigrationContext.queryRunner.query(
|
||||
`SHOW INDEXES FROM ${tableName} WHERE Key_name = ${indexName}`,
|
||||
);
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].Non_unique).toBe(0); // 0 means unique
|
||||
}
|
||||
|
||||
// Verify index enforces uniqueness by attempting duplicate insert
|
||||
await postMigrationContext.queryRunner.query(
|
||||
`INSERT INTO ${tableName} (${slugColumn}, ${displayNameColumn}, ${postMigrationContext.escape.columnName('createdAt')}, ${postMigrationContext.escape.columnName('updatedAt')}, ${postMigrationContext.escape.columnName('systemRole')}, ${postMigrationContext.escape.columnName('roleType')}) VALUES (?, ?, ?, ?, ?, ?)`,
|
||||
['test-duplicate-attempt', 'Unique Test Name', new Date(), new Date(), false, 'project'],
|
||||
);
|
||||
|
||||
const attemptDuplicateInsert = async () => {
|
||||
return await postMigrationContext.queryRunner.query(
|
||||
`INSERT INTO ${tableName} (${slugColumn}, ${displayNameColumn}, ${postMigrationContext.escape.columnName('createdAt')}, ${postMigrationContext.escape.columnName('updatedAt')}, ${postMigrationContext.escape.columnName('systemRole')}, ${postMigrationContext.escape.columnName('roleType')}) VALUES (?, ?, ?, ?, ?, ?)`,
|
||||
[
|
||||
'test-duplicate-attempt-2',
|
||||
'Unique Test Name',
|
||||
new Date(),
|
||||
new Date(),
|
||||
false,
|
||||
'project',
|
||||
],
|
||||
);
|
||||
};
|
||||
|
||||
await expect(attemptDuplicateInsert()).rejects.toThrow();
|
||||
|
||||
// Cleanup
|
||||
await postMigrationContext.queryRunner.release();
|
||||
});
|
||||
|
||||
it('should remove unique index on rollback', async () => {
|
||||
// NOTE: This test skips duplicate scenarios since migration already ran in previous test
|
||||
// We're testing rollback functionality independently
|
||||
|
||||
// Run up() migration first (already done in test set up)
|
||||
// runSingleMigration checks if already executed and skips if needed
|
||||
await runSingleMigration(MIGRATION_NAME);
|
||||
|
||||
// Create fresh context
|
||||
const upContext = createTestMigrationContext(dataSource);
|
||||
|
||||
const tableName = upContext.escape.tableName('role');
|
||||
const indexName = upContext.escape.indexName('UniqueRoleDisplayName');
|
||||
|
||||
// Verify unique index exists
|
||||
if (upContext.isSqlite) {
|
||||
const indexes = await upContext.queryRunner.query(`PRAGMA index_list(${tableName})`);
|
||||
const uniqueIndex = indexes.find(
|
||||
(idx: { name: string; unique: number }) =>
|
||||
idx.name.includes('UniqueRoleDisplayName') && idx.unique === 1,
|
||||
);
|
||||
expect(uniqueIndex).toBeDefined();
|
||||
} else if (upContext.isPostgres) {
|
||||
const result = await upContext.queryRunner.query(
|
||||
`SELECT indexname FROM pg_indexes WHERE tablename = ${tableName} AND indexname = ${indexName}`,
|
||||
);
|
||||
expect(result).toHaveLength(1);
|
||||
} else if (upContext.isMysql) {
|
||||
const result = await upContext.queryRunner.query(
|
||||
`SHOW INDEXES FROM ${tableName} WHERE Key_name = ${indexName}`,
|
||||
);
|
||||
expect(result).toHaveLength(1);
|
||||
}
|
||||
|
||||
await upContext.queryRunner.release();
|
||||
|
||||
await undoLastSingleMigration();
|
||||
|
||||
// Create fresh context after rollback
|
||||
const postRollbackContext = createTestMigrationContext(dataSource);
|
||||
|
||||
// Verify index is removed (DB-specific queries)
|
||||
if (postRollbackContext.isSqlite) {
|
||||
const indexes = await postRollbackContext.queryRunner.query(
|
||||
`PRAGMA index_list(${tableName})`,
|
||||
);
|
||||
const uniqueIndex = indexes.find(
|
||||
(idx: { name: string; unique: number }) =>
|
||||
idx.name.includes('UniqueRoleDisplayName') && idx.unique === 1,
|
||||
);
|
||||
expect(uniqueIndex).toBeUndefined();
|
||||
} else if (postRollbackContext.isPostgres) {
|
||||
const result = await postRollbackContext.queryRunner.query(
|
||||
`SELECT indexname FROM pg_indexes WHERE tablename = ${tableName} AND indexname = ${indexName}`,
|
||||
);
|
||||
expect(result).toHaveLength(0);
|
||||
} else if (postRollbackContext.isMysql) {
|
||||
const result = await postRollbackContext.queryRunner.query(
|
||||
`SHOW INDEXES FROM ${tableName} WHERE Key_name = ${indexName}`,
|
||||
);
|
||||
expect(result).toHaveLength(0);
|
||||
}
|
||||
|
||||
// Verify duplicate displayNames can be inserted again
|
||||
// Insert 2 roles with same displayName to confirm duplicates allowed
|
||||
const slugColumn = postRollbackContext.escape.columnName('slug');
|
||||
const displayNameColumn = postRollbackContext.escape.columnName('displayName');
|
||||
const createdAtColumn = postRollbackContext.escape.columnName('createdAt');
|
||||
const updatedAtColumn = postRollbackContext.escape.columnName('updatedAt');
|
||||
const systemRoleColumn = postRollbackContext.escape.columnName('systemRole');
|
||||
const roleTypeColumn = postRollbackContext.escape.columnName('roleType');
|
||||
|
||||
await postRollbackContext.queryRunner.query(
|
||||
`INSERT INTO ${tableName} (${slugColumn}, ${displayNameColumn}, ${createdAtColumn}, ${updatedAtColumn}, ${systemRoleColumn}, ${roleTypeColumn}) VALUES (?, ?, ?, ?, ?, ?)`,
|
||||
['rollback-test-1', 'Duplicate After Rollback', new Date(), new Date(), false, 'project'],
|
||||
);
|
||||
|
||||
await postRollbackContext.queryRunner.query(
|
||||
`INSERT INTO ${tableName} (${slugColumn}, ${displayNameColumn}, ${createdAtColumn}, ${updatedAtColumn}, ${systemRoleColumn}, ${roleTypeColumn}) VALUES (?, ?, ?, ?, ?, ?)`,
|
||||
['rollback-test-2', 'Duplicate After Rollback', new Date(), new Date(), false, 'project'],
|
||||
);
|
||||
|
||||
// Verify both roles were inserted successfully
|
||||
const duplicateRoles = await postRollbackContext.queryRunner.query(
|
||||
`SELECT ${slugColumn} as slug, ${displayNameColumn} as displayName FROM ${tableName} WHERE ${displayNameColumn} = ?`,
|
||||
['Duplicate After Rollback'],
|
||||
);
|
||||
|
||||
expect(duplicateRoles).toHaveLength(2);
|
||||
|
||||
// Cleanup
|
||||
await postRollbackContext.queryRunner.release();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Post-Migration Capacity', () => {
|
||||
it('should accept unique displayNames after migration', async () => {
|
||||
const context = createTestMigrationContext(dataSource);
|
||||
|
||||
const tableName = context.escape.tableName('role');
|
||||
const slugColumn = context.escape.columnName('slug');
|
||||
const displayNameColumn = context.escape.columnName('displayName');
|
||||
const createdAtColumn = context.escape.columnName('createdAt');
|
||||
const updatedAtColumn = context.escape.columnName('updatedAt');
|
||||
const systemRoleColumn = context.escape.columnName('systemRole');
|
||||
const roleTypeColumn = context.escape.columnName('roleType');
|
||||
|
||||
// Insert role with unique displayName
|
||||
await context.queryRunner.query(
|
||||
`INSERT INTO ${tableName} (${slugColumn}, ${displayNameColumn}, ${createdAtColumn}, ${updatedAtColumn}, ${systemRoleColumn}, ${roleTypeColumn}) VALUES (?, ?, ?, ?, ?, ?)`,
|
||||
['unique-role-test', 'Unique Role Name', new Date(), new Date(), false, 'project'],
|
||||
);
|
||||
|
||||
// Verify retrieval using SQL
|
||||
const [result] = await context.queryRunner.query(
|
||||
`SELECT ${slugColumn} as slug, ${displayNameColumn} as displayName FROM ${tableName} WHERE ${slugColumn} = ?`,
|
||||
['unique-role-test'],
|
||||
);
|
||||
|
||||
expect(result).toBeDefined();
|
||||
expect(result.displayName).toBe('Unique Role Name');
|
||||
|
||||
// Cleanup
|
||||
await context.queryRunner.release();
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user