feat: enforce env change request on project create (#9646)

src/lib/features/project-environments/environment-service.test.ts

@@ -78,10 +78,19 @@ test('Can manage required approvals', async () => {
     const groupRetrieved = (await service.getAll()).find(
         (env) => env.name === 'approval_env',
     );
+    const changeRequestEnvs =
+        await db.stores.environmentStore.getChangeRequestEnvironments([
+            'approval_env',
+            'default',
+            'other',
+        ]);
 
     expect(retrieved).toEqual(created);
     expect(updated).toEqual({ ...created, requiredApprovals: 2 });
     expect(groupRetrieved).toMatchObject({ ...created, requiredApprovals: 2 });
+    expect(changeRequestEnvs).toEqual([
+        { name: 'approval_env', requiredApprovals: 2 },
+    ]);
 });
 
 test('Can connect environment to project', async () => {

src/lib/features/project-environments/environment-store-type.ts

@@ -30,4 +30,7 @@ export interface IEnvironmentStore extends Store<IEnvironment, string> {
         projectId: string,
         query?: Object,
     ): Promise<IProjectEnvironment[]>;
+    getChangeRequestEnvironments(
+        environments: string[],
+    ): Promise<{ name: string; requiredApprovals: number }[]>;
 }

src/lib/features/project-environments/environment-store.ts

@@ -237,6 +237,21 @@ export default class EnvironmentStore implements IEnvironmentStore {
         return rows.map(mapRowWithCounts);
     }
 
+    async getChangeRequestEnvironments(
+        environments: string[],
+    ): Promise<{ name: string; requiredApprovals: number }[]> {
+        const stopTimer = this.timer('getChangeRequestEnvironments');
+        const rows = await this.db<IEnvironmentsTable>(TABLE)
+            .select('name', 'required_approvals')
+            .whereIn('name', environments)
+            .andWhere('required_approvals', '>', 0);
+        stopTimer();
+        return rows.map((row) => ({
+            name: row.name,
+            requiredApprovals: row.required_approvals || 1,
+        }));
+    }
+
     async getProjectEnvironments(
         projectId: string,
         query?: Object,

src/lib/features/project-environments/fake-environment-store.ts

@@ -140,6 +140,23 @@ export default class FakeEnvironmentStore implements IEnvironmentStore {
         return Promise.resolve(this.environments);
     }
 
+    async getChangeRequestEnvironments(
+        environments: string[],
+    ): Promise<{ name: string; requiredApprovals: number }[]> {
+        const filteredEnvironments = this.environments
+            .filter(
+                (env) =>
+                    environments.includes(env.name) &&
+                    env.requiredApprovals &&
+                    env.requiredApprovals > 0,
+            )
+            .map((env) => ({
+                name: env.name,
+                requiredApprovals: env.requiredApprovals || 1,
+            }));
+        return Promise.resolve(filteredEnvironments);
+    }
+
     async getProjectEnvironments(
         // eslint-disable-next-line @typescript-eslint/no-unused-vars
         projectId: string,

src/lib/features/project/createProjectService.ts

@@ -156,9 +156,7 @@ export const createProjectService = (
     );
 };
 
-export const createFakeProjectService = (
+export const createFakeProjectService = (config: IUnleashConfig) => {
-    config: IUnleashConfig,
-): ProjectService => {
     const { getLogger } = config;
     const eventStore = new FakeEventStore();
     const projectOwnersReadModel = new FakeProjectOwnersReadModel();
@@ -201,7 +199,7 @@ export const createFakeProjectService = (
 
     const onboardingReadModel = createFakeOnboardingReadModel();
 
-    return new ProjectService(
+    const projectService = new ProjectService(
         {
             projectStore,
             projectOwnersReadModel,
@@ -224,4 +222,11 @@ export const createFakeProjectService = (
         privateProjectChecker,
         apiTokenService,
     );
+    return {
+        projectService,
+        environmentStore,
+        accessService,
+        eventService,
+        projectStore,
+    };
 };

src/lib/features/project/project-service.limit.test.ts

@@ -11,7 +11,7 @@ const alwaysOnFlagResolver = {
 
 test('Should not allow to exceed project limit on create', async () => {
     const LIMIT = 1;
-    const projectService = createFakeProjectService({
+    const { projectService } = createFakeProjectService({
         ...createTestConfig(),
         flagResolver: alwaysOnFlagResolver,
         resourceLimits: { projects: LIMIT },
@@ -32,7 +32,7 @@ test('Should not allow to exceed project limit on create', async () => {
 
 test('Should not allow to exceed project limit on revive', async () => {
     const LIMIT = 1;
-    const projectService = createFakeProjectService({
+    const { projectService } = createFakeProjectService({
         ...createTestConfig(),
         flagResolver: alwaysOnFlagResolver,
         resourceLimits: { projects: LIMIT },

src/lib/features/project/project-service.test.ts

@@ -1,21 +1,32 @@
 import { createTestConfig } from '../../../test/config/test-config';
 import { BadDataError } from '../../error';
-import { type IBaseEvent, RoleName, TEST_AUDIT_USER } from '../../types';
+import {
+    type IFlagResolver,
+    type ProjectCreated,
+    RoleName,
+    TEST_AUDIT_USER,
+} from '../../types';
 import { createFakeProjectService } from './createProjectService';
 
 describe('enterprise extension: enable change requests', () => {
-    const createService = () => {
+    const createService = (mode: 'oss' | 'enterprise' = 'enterprise') => {
         const config = createTestConfig();
-        const service = createFakeProjectService(config);
+        config.isEnterprise = mode === 'enterprise';
-        // @ts-expect-error: we're setting this up to test the change request
+        const alwaysOnFlagResolver = {
-        service.flagResolver = {
+            isEnabled() {
-            isEnabled: () => true,
+                return true;
-        };
+            },
-        // @ts-expect-error: we're setting this up to test the change request
+        } as unknown as IFlagResolver;
-        service.isEnterprise = true;
+        config.flagResolver = alwaysOnFlagResolver;
+        const {
+            projectService,
+            accessService,
+            environmentStore,
+            eventService,
+            projectStore,
+        } = createFakeProjectService(config);
 
-        // @ts-expect-error: if we don't set this up, the tests will fail due to a missing role.
+        accessService.createRole(
-        service.accessService.createRole(
             {
                 name: RoleName.OWNER,
                 description: 'Project owner',
@@ -24,12 +35,17 @@ describe('enterprise extension: enable change requests', () => {
             TEST_AUDIT_USER,
         );
 
-        return service;
+        return {
+            service: projectService,
+            environmentStore,
+            eventService,
+            projectStore,
+        };
     };
 
     test('it calls the change request enablement function on enterprise after creating the project', async () => {
         expect.assertions(1);
-        const service = createService();
+        const { service, projectStore } = createService();
 
         const projectId = 'fake-project-id';
         await service.createProject(
@@ -45,9 +61,7 @@ describe('enterprise extension: enable change requests', () => {
             },
             TEST_AUDIT_USER,
             async () => {
-                // @ts-expect-error: we want to verify that the project /has/
+                const project = await projectStore.get(projectId);
-                // been created when calling the function.
-                const project = await service.projectStore.get(projectId);
 
                 expect(project).toBeTruthy();
 
@@ -57,9 +71,7 @@ describe('enterprise extension: enable change requests', () => {
     });
 
     test("it does not call the change request enablement function if we're not enterprise", async () => {
-        const service = createService();
+        const { service } = createService('oss');
-        // @ts-expect-error
-        service.isEnterprise = false;
 
         const fn = jest.fn();
 
@@ -83,14 +95,7 @@ describe('enterprise extension: enable change requests', () => {
 
     test('the emitted event contains an empty list of changeRequestEnvironments if the input had none', async () => {
         expect.assertions(1);
-        const service = createService();
+        const { service, eventService } = createService();
-
-        const storeEvent = async (e: IBaseEvent) => {
-            expect(e.data.changeRequestEnvironments).toStrictEqual([]);
-        };
-
-        // @ts-expect-error: for testing purposes
-        service.eventService.storeEvent = storeEvent;
 
         const projectId = 'fake-project-id';
         await service.createProject(
@@ -105,20 +110,18 @@ describe('enterprise extension: enable change requests', () => {
             },
             TEST_AUDIT_USER,
         );
+
+        const { events } = await eventService.getEvents();
+        expect(events).toMatchObject([
+            {
+                type: 'project-created',
+                data: { changeRequestEnvironments: [] },
+            },
+        ]);
     });
 
     test('the emitted event contains no changeRequestEnvironments if we are not on enterprise', async () => {
-        expect.assertions(1);
+        const { service, eventService } = createService('oss');
-        const service = createService();
-        // @ts-expect-error
-        service.isEnterprise = false;
-
-        const storeEvent = async (e: IBaseEvent) => {
-            expect('changeRequestEnvironments' in e.data).toBeFalsy();
-        };
-
-        // @ts-expect-error: for testing purposes
-        service.eventService.storeEvent = storeEvent;
 
         const projectId = 'fake-project-id';
         await service.createProject(
@@ -134,24 +137,23 @@ describe('enterprise extension: enable change requests', () => {
             TEST_AUDIT_USER,
             async () => [],
         );
+
+        const { events } = await eventService.getEvents();
+        expect(events[0].data.changeRequestEnvironments).toBeUndefined();
     });
 
     test('the emitted event contains the list of change request envs returned from the extension function, *not* what was passed in', async () => {
-        expect.assertions(1);
+        const { service, environmentStore, eventService } = createService();
-        const service = createService();
+        await environmentStore.create({
-
+            name: 'dev',
-        // @ts-expect-error: we want this to pass to test the functionality
+            type: 'production',
-        service.environmentStore.exists = () => true;
+            enabled: true,
+            protected: false,
+            sortOrder: 0,
+        });
 
         const crEnvs = [{ name: 'prod', requiredApprovals: 10 }];
 
-        const storeEvent = async (e: IBaseEvent) => {
-            expect(e.data.changeRequestEnvironments).toMatchObject(crEnvs);
-        };
-
-        // @ts-expect-error: for testing purposes
-        service.eventService.storeEvent = storeEvent;
-
         const projectId = 'fake-project-id';
         await service.createProject(
             {
@@ -169,16 +171,93 @@ describe('enterprise extension: enable change requests', () => {
             TEST_AUDIT_USER,
             async () => crEnvs,
         );
+
+        const { events } = await eventService.getEvents();
+        expect(events).toMatchObject([
+            {
+                type: 'project-created',
+                data: {
+                    changeRequestEnvironments: [
+                        { name: 'prod', requiredApprovals: 10 },
+                    ],
+                },
+            },
+        ]);
+    });
+
+    test('prefer env approval settings over explicit user approvals', async () => {
+        const { service, environmentStore, eventService } = createService();
+        await environmentStore.create({
+            name: 'dev',
+            type: 'production',
+            enabled: true,
+            protected: false,
+            sortOrder: 0,
+        });
+        await environmentStore.create({
+            name: 'stage',
+            type: 'production',
+            enabled: true,
+            protected: false,
+            sortOrder: 0,
+            requiredApprovals: 2,
+        });
+        await environmentStore.create({
+            name: 'prod',
+            type: 'production',
+            enabled: true,
+            protected: false,
+            sortOrder: 0,
+            requiredApprovals: 3,
+        });
+
+        const projectId = 'fake-project-id';
+        await service.createProject(
+            {
+                id: projectId,
+                name: 'fake-project-name',
+                environments: ['prod', 'stage'], // stage selected but no approvals set by user
+                changeRequestEnvironments: [
+                    { name: 'dev', requiredApprovals: 1 },
+                    { name: 'prod', requiredApprovals: 10 }, // ignored in favor of env config
+                ],
+            },
+            {
+                id: 5,
+                permissions: [],
+                isAPI: false,
+            },
+            TEST_AUDIT_USER,
+            async (envs) => envs as ProjectCreated['changeRequestEnvironments'],
+        );
+
+        const { events } = await eventService.getEvents();
+        expect(events).toMatchObject([
+            {
+                type: 'project-created',
+                data: {
+                    changeRequestEnvironments: [
+                        { name: 'dev', requiredApprovals: 1 },
+                        { name: 'stage', requiredApprovals: 2 },
+                        { name: 'prod', requiredApprovals: 3 },
+                    ],
+                },
+            },
+        ]);
     });
 
     test('the create project function returns the list of change request envs returned from the extension function, *not* what was passed in', async () => {
-        const service = createService();
+        const { service, environmentStore } = createService();
+        await environmentStore.create({
+            name: 'dev',
+            type: 'production',
+            enabled: true,
+            protected: false,
+            sortOrder: 0,
+        });
 
         const crEnvs = [{ name: 'prod', requiredApprovals: 10 }];
 
-        // @ts-expect-error: we want this to pass to test the functionality
-        service.environmentStore.exists = () => true;
-
         const projectId = 'fake-project-id';
         const result = await service.createProject(
             {
@@ -201,7 +280,7 @@ describe('enterprise extension: enable change requests', () => {
     });
 
     test('the create project function defaults to returning an empty list if the input had no cr envs', async () => {
-        const service = createService();
+        const { service } = createService();
 
         const projectId = 'fake-project-id';
         const result = await service.createProject(
@@ -222,9 +301,7 @@ describe('enterprise extension: enable change requests', () => {
     });
 
     test('the create project function does not return a list of change request envs if we are not on enterprise', async () => {
-        const service = createService();
+        const { service } = createService('oss');
-        // @ts-expect-error
-        service.isEnterprise = false;
 
         const crEnvs = [{ name: 'prod', requiredApprovals: 10 }];
 
@@ -250,9 +327,7 @@ describe('enterprise extension: enable change requests', () => {
     });
 
     test("it throws an error if you provide it with environments that don't exist", async () => {
-        const service = createService();
+        const { service } = createService();
-        // @ts-expect-error
-        service.environmentStore.exists = () => false;
 
         const projectId = 'fake-project-id';
         expect(
@@ -275,11 +350,7 @@ describe('enterprise extension: enable change requests', () => {
     });
 
     test("it does not throw if an error if you provide it with environments that don't exist but aren't on enterprise", async () => {
-        const service = createService();
+        const { service } = createService('oss');
-        // @ts-expect-error
-        service.isEnterprise = false;
-        // @ts-expect-error
-        service.environmentStore.exists = () => false;
 
         const projectId = 'fake-project-id';
         expect(

src/lib/features/project/project-service.ts

@@ -346,6 +346,27 @@ export default class ProjectService {
         return generateUniqueId();
     }
 
+    async getAllChangeRequestEnvironments(
+        newProject: CreateProject,
+    ): Promise<CreateProject['changeRequestEnvironments']> {
+        const predefinedChangeRequestEnvironments =
+            await this.environmentStore.getChangeRequestEnvironments(
+                newProject.environments || [],
+            );
+        const userSelectedChangeRequestEnvironments =
+            newProject.changeRequestEnvironments || [];
+        const allChangeRequestEnvironments = [
+            ...userSelectedChangeRequestEnvironments.filter(
+                (userEnv) =>
+                    !predefinedChangeRequestEnvironments.find(
+                        (predefinedEnv) => predefinedEnv.name === userEnv.name,
+                    ),
+            ),
+            ...predefinedChangeRequestEnvironments,
+        ];
+        return allChangeRequestEnvironments;
+    }
+
     async createProject(
         newProject: CreateProject,
         user: IUser,
@@ -398,12 +419,25 @@ export default class ProjectService {
                 await this.validateEnvironmentsExist(
                     newProject.changeRequestEnvironments.map((env) => env.name),
                 );
-                const changeRequestEnvironments =
+                const globalChangeRequestConfigEnabled =
-                    await enableChangeRequestsForSpecifiedEnvironments(
+                    this.flagResolver.isEnabled('globalChangeRequestConfig');
-                        newProject.changeRequestEnvironments,
+                if (globalChangeRequestConfigEnabled) {
-                    );
+                    const allChangeRequestEnvironments =
+                        await this.getAllChangeRequestEnvironments(newProject);
+                    const changeRequestEnvironments =
+                        await enableChangeRequestsForSpecifiedEnvironments(
+                            allChangeRequestEnvironments,
+                        );
 
-                data.changeRequestEnvironments = changeRequestEnvironments;
+                    data.changeRequestEnvironments = changeRequestEnvironments;
+                } else {
+                    const changeRequestEnvironments =
+                        await enableChangeRequestsForSpecifiedEnvironments(
+                            newProject.changeRequestEnvironments,
+                        );
+
+                    data.changeRequestEnvironments = changeRequestEnvironments;
+                }
             } else {
                 data.changeRequestEnvironments = [];
             }

src/lib/services/index.ts

@@ -308,10 +308,12 @@ export const createServices = (
     const favoritesService = new FavoritesService(stores, config, eventService);
     const projectService = db
         ? createProjectService(db, config)
-        : createFakeProjectService(config);
+        : createFakeProjectService(config).projectService;
     const transactionalProjectService = db
         ? withTransactional((db: Db) => createProjectService(db, config), db)
-        : withFakeTransactional(createFakeProjectService(config));
+        : withFakeTransactional(
+              createFakeProjectService(config).projectService,
+          );
     const projectInsightsService = db
         ? createProjectInsightsService(db, config)
         : createFakeProjectInsightsService().projectInsightsService;