From e7ac42080d96d5c7d0b215ca35c09209443e1d5f Mon Sep 17 00:00:00 2001 From: Simon Hornby Date: Wed, 5 Mar 2025 10:09:25 +0200 Subject: [PATCH] feat: project environments include visible property (#9427) --- src/lib/error/from-legacy-error.ts | 2 - src/lib/error/index.ts | 2 - src/lib/error/unleash-error.ts | 1 - .../environment-service.test.ts | 51 +++++++++++++++++++ .../environment-service.ts | 44 +++++++++------- .../environments.e2e.test.ts | 16 ------ .../fake-environment-store.ts | 7 ++- .../spec/environment-project-schema.ts | 6 +++ src/lib/types/model.ts | 4 ++ 9 files changed, 91 insertions(+), 42 deletions(-) diff --git a/src/lib/error/from-legacy-error.ts b/src/lib/error/from-legacy-error.ts index 99c656db94..da70055cdc 100644 --- a/src/lib/error/from-legacy-error.ts +++ b/src/lib/error/from-legacy-error.ts @@ -19,8 +19,6 @@ const getStatusCode = (errorName: string): number => { return 400; case 'PasswordUndefinedError': return 400; - case 'MinimumOneEnvironmentError': - return 400; case 'InvalidTokenError': return 401; case 'UsedTokenError': diff --git a/src/lib/error/index.ts b/src/lib/error/index.ts index e9137b90f3..6400ed93f2 100644 --- a/src/lib/error/index.ts +++ b/src/lib/error/index.ts @@ -5,7 +5,6 @@ import FeatureHasTagError from './feature-has-tag-error'; import IncompatibleProjectError from './incompatible-project-error'; import InvalidOperationError from './invalid-operation-error'; import InvalidTokenError from './invalid-token-error'; -import MinimumOneEnvironmentError from './minimum-one-environment-error'; import NameExistsError from './name-exists-error'; import PermissionError from './permission-error'; import { OperationDeniedError } from './operation-denied-error'; @@ -27,7 +26,6 @@ export { IncompatibleProjectError, InvalidOperationError, InvalidTokenError, - MinimumOneEnvironmentError, NameExistsError, PermissionError, ForbiddenError, diff --git a/src/lib/error/unleash-error.ts b/src/lib/error/unleash-error.ts index 7fbda0e58b..0136c472d0 100644 --- a/src/lib/error/unleash-error.ts +++ b/src/lib/error/unleash-error.ts @@ -8,7 +8,6 @@ export const UnleashApiErrorTypes = [ 'IncompatibleProjectError', 'InvalidOperationError', 'InvalidTokenError', - 'MinimumOneEnvironmentError', 'NameExistsError', 'NoAccessError', 'NotFoundError', diff --git a/src/lib/features/project-environments/environment-service.test.ts b/src/lib/features/project-environments/environment-service.test.ts index 1a5f8ea81c..5273b95a7a 100644 --- a/src/lib/features/project-environments/environment-service.test.ts +++ b/src/lib/features/project-environments/environment-service.test.ts @@ -338,3 +338,54 @@ test('Override works correctly when enabling default and disabling prod and dev' expect(targetedEnvironment?.enabled).toBe(true); expect(allOtherEnvironments.every((x) => !x)).toBe(true); }); + +test('getProjectEnvironments also includes whether or not a given project is visible on a given environment', async () => { + const assertContains = (environments, envName, visible) => { + const env = environments.find((e) => e.name === envName); + expect(env).toBeDefined(); + expect(env.visible).toBe(visible); + }; + + const assertContainsVisible = (environments, envName) => { + assertContains(environments, envName, true); + }; + + const assertContainsNotVisible = (environments, envName) => { + assertContains(environments, envName, false); + }; + + const projectId = 'default'; + const firstEnvTest = 'some-connected-environment'; + const secondEnvTest = 'some-also-connected-environment'; + await db.stores.environmentStore.create({ + name: firstEnvTest, + type: 'production', + }); + await db.stores.environmentStore.create({ + name: secondEnvTest, + type: 'production', + }); + + await service.addEnvironmentToProject( + firstEnvTest, + projectId, + SYSTEM_USER_AUDIT, + ); + await service.addEnvironmentToProject( + secondEnvTest, + projectId, + SYSTEM_USER_AUDIT, + ); + let environments = await service.getProjectEnvironments(projectId); + assertContainsVisible(environments, firstEnvTest); + assertContainsVisible(environments, secondEnvTest); + + await service.removeEnvironmentFromProject( + firstEnvTest, + projectId, + SYSTEM_USER_AUDIT, + ); + environments = await service.getProjectEnvironments(projectId); + assertContainsNotVisible(environments, firstEnvTest); + assertContainsVisible(environments, secondEnvTest); +}); diff --git a/src/lib/features/project-environments/environment-service.ts b/src/lib/features/project-environments/environment-service.ts index 308d279d68..51e0040193 100644 --- a/src/lib/features/project-environments/environment-service.ts +++ b/src/lib/features/project-environments/environment-service.ts @@ -5,7 +5,7 @@ import { type IEnvironmentStore, type IFeatureEnvironmentStore, type IFeatureStrategiesStore, - type IProjectEnvironment, + type IProjectsAvailableOnEnvironment, type ISortOrder, type IUnleashConfig, type IUnleashStores, @@ -19,7 +19,6 @@ import NameExistsError from '../../error/name-exists-error'; import { sortOrderSchema } from '../../services/sort-order-schema'; import NotFoundError from '../../error/notfound-error'; import type { IProjectStore } from '../../features/project/project-store-type'; -import MinimumOneEnvironmentError from '../../error/minimum-one-environment-error'; import type { IFlagResolver } from '../../types/experimental'; import type { CreateFeatureStrategySchema } from '../../openapi'; import type EventService from '../events/event-service'; @@ -77,8 +76,24 @@ export default class EnvironmentService { async getProjectEnvironments( projectId: string, - ): Promise { - return this.environmentStore.getProjectEnvironments(projectId); + ): Promise { + // This function produces an object for every environment, in that object is a boolean + // describing whether or not that environment is enabled - aka not deprecated + const environments = + await this.projectStore.getEnvironmentsForProject(projectId); + const environmentsOnProject = new Set( + environments.map((env) => env.environment), + ); + + const allEnvironments = + await this.environmentStore.getProjectEnvironments(projectId); + + return allEnvironments.map((env) => { + return { + ...env, + visible: environmentsOnProject.has(env.name), + }; + }); } async updateSortOrder(sortOrder: ISortOrder): Promise { @@ -254,22 +269,13 @@ export default class EnvironmentService { const projectEnvs = await this.projectStore.getEnvironmentsForProject(projectId); - if (projectEnvs.length > 1) { - await this.forceRemoveEnvironmentFromProject( + await this.forceRemoveEnvironmentFromProject(environment, projectId); + await this.eventService.storeEvent( + new ProjectEnvironmentRemoved({ + project: projectId, environment, - projectId, - ); - await this.eventService.storeEvent( - new ProjectEnvironmentRemoved({ - project: projectId, - environment, - auditUser, - }), - ); - return; - } - throw new MinimumOneEnvironmentError( - 'You must always have one active environment', + auditUser, + }), ); } } diff --git a/src/lib/features/project-environments/environments.e2e.test.ts b/src/lib/features/project-environments/environments.e2e.test.ts index 9c401c29df..19f90c3cff 100644 --- a/src/lib/features/project-environments/environments.e2e.test.ts +++ b/src/lib/features/project-environments/environments.e2e.test.ts @@ -98,22 +98,6 @@ test('Should remove environment from project', async () => { expect(envs).toHaveLength(1); }); -test('Should not remove environment from project if project only has one environment enabled', async () => { - await app.request - .delete(`/api/admin/projects/default/environments/default`) - .expect(400) - .expect((r) => { - expect(r.body.details[0].message).toBe( - 'You must always have one active environment', - ); - }); - - const envs = - await db.stores.projectStore.getEnvironmentsForProject('default'); - - expect(envs).toHaveLength(1); -}); - test('Should add default strategy to environment', async () => { const defaultStrategy = { name: 'flexibleRollout', diff --git a/src/lib/features/project-environments/fake-environment-store.ts b/src/lib/features/project-environments/fake-environment-store.ts index 2c0aaab7d0..e898a6b528 100644 --- a/src/lib/features/project-environments/fake-environment-store.ts +++ b/src/lib/features/project-environments/fake-environment-store.ts @@ -1,4 +1,7 @@ -import type { IEnvironment, IProjectEnvironment } from '../../types/model'; +import type { + IEnvironment, + IProjectsAvailableOnEnvironment, +} from '../../types/model'; import NotFoundError from '../../error/notfound-error'; import type { IEnvironmentStore } from './environment-store-type'; @@ -140,7 +143,7 @@ export default class FakeEnvironmentStore implements IEnvironmentStore { async getProjectEnvironments( // eslint-disable-next-line @typescript-eslint/no-unused-vars projectId: string, - ): Promise { + ): Promise { return Promise.reject(new Error('Not implemented')); } diff --git a/src/lib/openapi/spec/environment-project-schema.ts b/src/lib/openapi/spec/environment-project-schema.ts index 23bbfb2d98..3d10863545 100644 --- a/src/lib/openapi/spec/environment-project-schema.ts +++ b/src/lib/openapi/spec/environment-project-schema.ts @@ -57,6 +57,12 @@ export const environmentProjectSchema = { 'The strategy configuration to add when enabling a feature environment by default', $ref: '#/components/schemas/createFeatureStrategySchema', }, + visible: { + type: 'boolean', + example: true, + description: + 'Indicates whether the environment can be enabled for feature flags in the project', + }, }, components: { schemas: { diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index cfcf0fdeb6..533b3bbd6f 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -212,6 +212,10 @@ export interface IProjectEnvironment extends IEnvironment { defaultStrategy?: CreateFeatureStrategySchema; } +export interface IProjectsAvailableOnEnvironment extends IProjectEnvironment { + visible: boolean; +} + export interface IEnvironmentCreate { name: string; type: string;