From bde81b940cec26c7cdf4a437b528670973d45b32 Mon Sep 17 00:00:00 2001 From: Mateusz Kwasniewski Date: Fri, 9 Aug 2024 09:00:19 +0200 Subject: [PATCH] feat: prevent adding flags to archived project (#7811) --- .../feature-toggle/feature-toggle-service.ts | 11 +++++-- .../tests/feature-toggle-service.e2e.test.ts | 33 +++++++++++++++++-- .../features/project/project-store-type.ts | 2 ++ src/lib/features/project/project-store.ts | 27 ++++++++++----- src/test/fixtures/fake-project-store.ts | 14 +++++--- 5 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 08a4fa3ad8..24af698d25 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -1256,7 +1256,12 @@ class FeatureToggleService { await this.validateName(value.name); await this.validateFeatureFlagNameAgainstPattern(value.name, projectId); - const projectExists = await this.projectStore.hasProject(projectId); + let projectExists: boolean; + if (this.flagResolver.isEnabled('archiveProjects')) { + projectExists = await this.projectStore.hasActiveProject(projectId); + } else { + projectExists = await this.projectStore.hasProject(projectId); + } if (await this.projectStore.isFeatureLimitReached(projectId)) { throw new InvalidOperationError( @@ -1322,7 +1327,9 @@ class FeatureToggleService { return createdToggle; } - throw new NotFoundError(`Project with id ${projectId} does not exist`); + throw new NotFoundError( + `Active project with id ${projectId} does not exist`, + ); } async checkFeatureFlagNamesAgainstProjectPattern( diff --git a/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts b/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts index 92dce170d0..7744afdf49 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts +++ b/src/lib/features/feature-toggle/tests/feature-toggle-service.e2e.test.ts @@ -16,7 +16,12 @@ import { TEST_AUDIT_USER, } from '../../../types'; import EnvironmentService from '../../project-environments/environment-service'; -import { ForbiddenError, PatternError, PermissionError } from '../../../error'; +import { + ForbiddenError, + NotFoundError, + PatternError, + PermissionError, +} from '../../../error'; import type { ISegmentService } from '../../segment/segment-service-interface'; import { createFeatureToggleService, createSegmentService } from '../..'; import { insertLastSeenAt } from '../../../../test/e2e/helpers/test-helper'; @@ -41,7 +46,9 @@ const mockConstraints = (): IConstraint[] => { const irrelevantDate = new Date(); beforeAll(async () => { - const config = createTestConfig(); + const config = createTestConfig({ + experimental: { flags: { archiveProjects: true } }, + }); db = await dbInit( 'feature_toggle_service_v2_service_serial', config.getLogger, @@ -744,3 +751,25 @@ test('Should return "default" for stickiness when creating a flexibleRollout str strategies: [{ parameters: { stickiness: 'default' } }], }); }); + +test('Should not allow to add flags to archived projects', async () => { + const project = await stores.projectStore.create({ + id: 'archivedProject', + name: 'archivedProject', + }); + await stores.projectStore.archive(project.id); + + await expect( + service.createFeatureToggle( + project.id, + { + name: 'irrelevant', + }, + TEST_AUDIT_USER, + ), + ).rejects.toEqual( + new NotFoundError( + `Active project with id archivedProject does not exist`, + ), + ); +}); diff --git a/src/lib/features/project/project-store-type.ts b/src/lib/features/project/project-store-type.ts index 375e368ece..e89257da6d 100644 --- a/src/lib/features/project/project-store-type.ts +++ b/src/lib/features/project/project-store-type.ts @@ -67,6 +67,8 @@ export interface IProjectApplicationsSearchParams { export interface IProjectStore extends Store { hasProject(id: string): Promise; + hasActiveProject(id: string): Promise; + updateHealth(healthUpdate: IProjectHealthUpdate): Promise; create(project: IProjectInsert): Promise; diff --git a/src/lib/features/project/project-store.ts b/src/lib/features/project/project-store.ts index 127bd50a6b..eeef8d2f9d 100644 --- a/src/lib/features/project/project-store.ts +++ b/src/lib/features/project/project-store.ts @@ -102,15 +102,6 @@ class ProjectStore implements IProjectStore { destroy(): void {} - async exists(id: string): Promise { - const result = await this.db.raw( - `SELECT EXISTS(SELECT 1 FROM ${TABLE} WHERE id = ?) AS present`, - [id], - ); - const { present } = result.rows[0]; - return present; - } - async isFeatureLimitReached(id: string): Promise { const result = await this.db.raw( `SELECT EXISTS(SELECT 1 @@ -252,6 +243,15 @@ class ProjectStore implements IProjectStore { .then(this.mapRow); } + async exists(id: string): Promise { + const result = await this.db.raw( + `SELECT EXISTS(SELECT 1 FROM ${TABLE} WHERE id = ?) AS present`, + [id], + ); + const { present } = result.rows[0]; + return present; + } + async hasProject(id: string): Promise { const result = await this.db.raw( `SELECT EXISTS(SELECT 1 FROM ${TABLE} WHERE id = ?) AS present`, @@ -261,6 +261,15 @@ class ProjectStore implements IProjectStore { return present; } + async hasActiveProject(id: string): Promise { + const result = await this.db.raw( + `SELECT EXISTS(SELECT 1 FROM ${TABLE} WHERE id = ? and archived_at IS NULL) AS present`, + [id], + ); + const { present } = result.rows[0]; + return present; + } + async updateHealth(healthUpdate: IProjectHealthUpdate): Promise { await this.db(TABLE).where({ id: healthUpdate.id }).update({ health: healthUpdate.health, diff --git a/src/test/fixtures/fake-project-store.ts b/src/test/fixtures/fake-project-store.ts index a12d70e328..11d712d212 100644 --- a/src/test/fixtures/fake-project-store.ts +++ b/src/test/fixtures/fake-project-store.ts @@ -108,10 +108,6 @@ export default class FakeProjectStore implements IProjectStore { return this.projects.length; } - async exists(key: string): Promise { - return this.projects.some((project) => project.id === key); - } - async get(key: string): Promise { const project = this.projects.find((p) => p.id === key); if (project) { @@ -129,10 +125,20 @@ export default class FakeProjectStore implements IProjectStore { return Promise.resolve(0); } + async exists(key: string): Promise { + return this.projects.some((project) => project.id === key); + } + async hasProject(id: string): Promise { return this.exists(id); } + async hasActiveProject(id: string): Promise { + return this.projects.some( + (project) => project.id === id && project.archivedAt === null, + ); + } + async importProjects( // eslint-disable-next-line @typescript-eslint/no-unused-vars projects: IProjectInsert[],