From a9a87bc84de946f5a7a6f085711bd5aae8c6dcc4 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Thu, 27 Jun 2024 09:21:09 +0200 Subject: [PATCH] chore: change generated project id format to use incrementing numbers instead of hashes (#7456) --- .../project/project-service.e2e.test.ts | 13 +++---- .../features/project/project-service.test.ts | 27 --------------- src/lib/features/project/project-service.ts | 34 ++++++++----------- 3 files changed, 21 insertions(+), 53 deletions(-) diff --git a/src/lib/features/project/project-service.e2e.test.ts b/src/lib/features/project/project-service.e2e.test.ts index fd01f57380..5eb715a894 100644 --- a/src/lib/features/project/project-service.e2e.test.ts +++ b/src/lib/features/project/project-service.e2e.test.ts @@ -2650,10 +2650,10 @@ describe('automatic ID generation for create project', () => { auditUser, ); - expect(project.id).toMatch(/^new-name-/); + expect(project.id).toBe('new-name'); }); - test('two projects with the same name get different ids', async () => { + test('projects with the same name get ids with incrementing counters', async () => { const createProject = async () => projectService.createProject( { name: 'some name' }, @@ -2663,10 +2663,11 @@ describe('automatic ID generation for create project', () => { const project1 = await createProject(); const project2 = await createProject(); + const project3 = await createProject(); - expect(project1.id).toMatch(/^some-name-/); - expect(project2.id).toMatch(/^some-name-/); - expect(project1.id).not.toBe(project2.id); + expect(project1.id).toBe('some-name'); + expect(project2.id).toBe('some-name-1'); + expect(project3.id).toBe('some-name-2'); }); test.each(['', undefined, ' '])( @@ -2679,7 +2680,7 @@ describe('automatic ID generation for create project', () => { auditUser, ); - expect(project.id).toMatch(new RegExp(`^${name}-`)); + expect(project.id).toBe(name); }, ); diff --git a/src/lib/features/project/project-service.test.ts b/src/lib/features/project/project-service.test.ts index 3e4429c543..4ca7f09b58 100644 --- a/src/lib/features/project/project-service.test.ts +++ b/src/lib/features/project/project-service.test.ts @@ -1,4 +1,3 @@ -import fc from 'fast-check'; import { createTestConfig } from '../../../test/config/test-config'; import { BadDataError } from '../../error'; import { type IBaseEvent, RoleName, TEST_AUDIT_USER } from '../../types'; @@ -302,29 +301,3 @@ describe('enterprise extension: enable change requests', () => { ).resolves.toBeTruthy(); }); }); - -describe('project ID generation', () => { - const createService = () => { - const config = createTestConfig(); - const service = createFakeProjectService(config); - - return service; - }; - - test('the name that comes out is always url friendly', () => { - const service = createService(); - fc.assert( - fc.property(fc.string(), (name) => { - const projectId = service.generateProjectId(name); - expect(projectId).toMatch(encodeURIComponent(projectId)); - }), - ); - }); - - test('it adds a 12 character hex to the end of the id', () => { - const service = createService(); - - const projectId = service.generateProjectId('one'); - expect(projectId).toMatch(/^one-[a-f0-9]{12}$/); - }); -}); diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index ef40dc258e..e36619d429 100644 --- a/src/lib/features/project/project-service.ts +++ b/src/lib/features/project/project-service.ts @@ -1,6 +1,6 @@ import { subDays } from 'date-fns'; import { ValidationError } from 'joi'; -import slug from 'slug'; +import createSlug from 'slug'; import type { IAuditUser, IUser } from '../../types/user'; import type { AccessService, @@ -62,7 +62,7 @@ import type { import type FeatureToggleService from '../feature-toggle/feature-toggle-service'; import IncompatibleProjectError from '../../error/incompatible-project-error'; import ProjectWithoutOwnerError from '../../error/project-without-owner-error'; -import { arraysHaveSameItems, randomId } from '../../util'; +import { arraysHaveSameItems } from '../../util'; import type { GroupService } from '../../services/group-service'; import type { IGroupRole } from '../../types/group'; import type { FavoritesService } from '../../services/favorites-service'; @@ -304,21 +304,17 @@ export default class ProjectService { await this.validateEnvironmentsExist(environments); } } - - generateProjectId(name: string): string { - const urlFriendly = slug(name); - const tail = randomId().slice(-12); - const id = `${urlFriendly}-${tail}`; - return id; - } - - async generateUniqueProjectId(name: string): Promise { - const id = this.generateProjectId(name); - if (await this.projectStore.hasProject(id)) { - return await this.generateUniqueProjectId(name); - } else { - return id; - } + async generateProjectId(name: string): Promise { + const generateUniqueId = async (name: string, suffix?: number) => { + const slug = createSlug(name); + const id = suffix ? `${slug}-${suffix}` : slug; + if (await this.projectStore.hasProject(id)) { + return await generateUniqueId(name, (suffix ?? 0) + 1); + } else { + return id; + } + }; + return generateUniqueId(name); } async createProject( @@ -337,9 +333,7 @@ export default class ProjectService { await this.validateProjectEnvironments(newProject.environments); if (!newProject.id?.trim()) { - newProject.id = await this.generateUniqueProjectId( - newProject.name, - ); + newProject.id = await this.generateProjectId(newProject.name); return await projectSchema.validateAsync(newProject); } else { const validatedData =