feat: include CR envs enabled on creation in event and update validation (#6931)

This PR improves the handling of change request enables on project
creation in two ways:

1. We now verify that the envs you try to enable CRs for exist before
passing them on to the enterprise functionality.
2. We include data about environments and change request environments in
the project created events.

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

@@ -1,15 +1,20 @@
 import { createTestConfig } from '../../../test/config/test-config';
-import { RoleName, TEST_AUDIT_USER } from '../../types';
+import { BadDataError } from '../../error';
+import { type IBaseEvent, RoleName, TEST_AUDIT_USER } from '../../types';
 import { createFakeProjectService } from './createProjectService';
 
 describe('enterprise extension: enable change requests', () => {
-    test('it calls the change request enablement function', async () => {
-        expect.assertions(1);
-
+    const createService = () => {
         const config = createTestConfig();
         const service = createFakeProjectService(config);
+        // @ts-expect-error: we're setting this up to test the change request
+        service.flagResolver = {
+            isEnabled: () => true,
+        };
+        // @ts-expect-error: we're setting this up to test the change request
+        service.isEnterprise = true;
 
-        // @ts-expect-error: if we don't set this up, the test will fail due to a missing role.
+        // @ts-expect-error: if we don't set this up, the tests will fail due to a missing role.
         service.accessService.createRole(
             {
                 name: RoleName.OWNER,
@@ -19,11 +24,19 @@ describe('enterprise extension: enable change requests', () => {
             TEST_AUDIT_USER,
         );
 
+        return service;
+    };
+
+    test('it calls the change request enablement function on enterprise after creating the project', async () => {
+        expect.assertions(1);
+        const service = createService();
+
         const projectId = 'fake-project-id';
         await service.createProject(
             {
                 id: projectId,
                 name: 'fake-project-name',
+                changeRequestEnvironments: [],
             },
             {
                 id: 5,
@@ -40,4 +53,255 @@ 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();
+        // @ts-expect-error
+        service.isEnterprise = false;
+
+        const fn = jest.fn();
+
+        const projectId = 'fake-project-id';
+        await service.createProject(
+            {
+                id: projectId,
+                name: 'fake-project-name',
+            },
+            {
+                id: 5,
+                permissions: [],
+                isAPI: false,
+            },
+            TEST_AUDIT_USER,
+            fn,
+        );
+
+        expect(fn).not.toHaveBeenCalled();
+    });
+
+    test('the emitted event contains an empty list of changeRequestEnvironments if the input had none', async () => {
+        expect.assertions(1);
+        const service = 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(
+            {
+                id: projectId,
+                name: 'fake-project-name',
+            },
+            {
+                id: 5,
+                permissions: [],
+                isAPI: false,
+            },
+            TEST_AUDIT_USER,
+        );
+    });
+
+    test('the emitted event contains no changeRequestEnvironments if we are not on enterprise', async () => {
+        expect.assertions(1);
+        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(
+            {
+                id: projectId,
+                name: 'fake-project-name',
+            },
+            {
+                id: 5,
+                permissions: [],
+                isAPI: false,
+            },
+            TEST_AUDIT_USER,
+            async () => {
+                [];
+            },
+        );
+    });
+
+    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 = createService();
+
+        // @ts-expect-error: we want this to pass to test the functionality
+        service.environmentStore.exists = () => true;
+
+        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(
+            {
+                id: projectId,
+                name: 'fake-project-name',
+                changeRequestEnvironments: [
+                    { name: 'dev', requiredApprovals: 1 },
+                ],
+            },
+            {
+                id: 5,
+                permissions: [],
+                isAPI: false,
+            },
+            TEST_AUDIT_USER,
+            async () => crEnvs,
+        );
+    });
+
+    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 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(
+            {
+                id: projectId,
+                name: 'fake-project-name',
+                changeRequestEnvironments: [
+                    { name: 'dev', requiredApprovals: 1 },
+                ],
+            },
+            {
+                id: 5,
+                permissions: [],
+                isAPI: false,
+            },
+            TEST_AUDIT_USER,
+            async () => crEnvs,
+        );
+
+        expect(result.changeRequestEnvironments).toStrictEqual(crEnvs);
+    });
+
+    test('the create project function defaults to returning an empty list if the input had no cr envs', async () => {
+        const service = createService();
+
+        const projectId = 'fake-project-id';
+        const result = await service.createProject(
+            {
+                id: projectId,
+                name: 'fake-project-name',
+            },
+            {
+                id: 5,
+                permissions: [],
+                isAPI: false,
+            },
+            TEST_AUDIT_USER,
+            async () => {
+                return;
+            },
+        );
+
+        expect(result.changeRequestEnvironments).toStrictEqual([]);
+    });
+
+    test('the create project function does not return a list of change request envs if we are not on enterprise', async () => {
+        const service = createService();
+        // @ts-expect-error
+        service.isEnterprise = false;
+
+        const crEnvs = [{ name: 'prod', requiredApprovals: 10 }];
+
+        const projectId = 'fake-project-id';
+        const result = await service.createProject(
+            {
+                id: projectId,
+                name: 'fake-project-name',
+                changeRequestEnvironments: [
+                    { name: 'dev', requiredApprovals: 1 },
+                ],
+            },
+            {
+                id: 5,
+                permissions: [],
+                isAPI: false,
+            },
+            TEST_AUDIT_USER,
+            async () => {
+                crEnvs;
+            },
+        );
+
+        expect('changeRequestEnvironments' in result).toBeFalsy();
+    });
+
+    test("it throws an error if you provide it with environments that don't exist", async () => {
+        const service = createService();
+        // @ts-expect-error
+        service.environmentStore.exists = () => false;
+
+        const projectId = 'fake-project-id';
+        expect(
+            service.createProject(
+                {
+                    id: projectId,
+                    name: 'fake-project-name',
+                    changeRequestEnvironments: [
+                        { name: 'dev', requiredApprovals: 1 },
+                    ],
+                },
+                {
+                    id: 5,
+                    permissions: [],
+                    isAPI: false,
+                },
+                TEST_AUDIT_USER,
+            ),
+        ).rejects.toThrow(BadDataError);
+    });
+
+    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();
+        // @ts-expect-error
+        service.isEnterprise = false;
+        // @ts-expect-error
+        service.environmentStore.exists = () => false;
+
+        const projectId = 'fake-project-id';
+        expect(
+            service.createProject(
+                {
+                    id: projectId,
+                    name: 'fake-project-name',
+                    changeRequestEnvironments: [
+                        { name: 'dev', requiredApprovals: 1 },
+                    ],
+                },
+                {
+                    id: 5,
+                    permissions: [],
+                    isAPI: false,
+                },
+                TEST_AUDIT_USER,
+            ),
+        ).resolves.toBeTruthy();
+    });
 });

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

@@ -248,6 +248,27 @@ export default class ProjectService {
         return featureNaming;
     };
 
+    private async validateEnvironmentsExist(environments: string[]) {
+        const projectsAndExistence = await Promise.all(
+            environments.map(async (env) => [
+                env,
+                await this.environmentStore.exists(env),
+            ]),
+        );
+
+        const invalidEnvs = projectsAndExistence
+            .filter(([_, exists]) => !exists)
+            .map(([env]) => env);
+
+        if (invalidEnvs.length > 0) {
+            throw new BadDataError(
+                `These environments do not exist: ${invalidEnvs
+                    .map((env) => `'${env}'`)
+                    .join(', ')}.`,
+            );
+        }
+    }
+
     async validateProjectEnvironments(environments: string[] | undefined) {
         if (
             this.flagResolver.isEnabled('createProjectWithEnvironmentConfig') &&
@@ -259,24 +280,7 @@ export default class ProjectService {
                 );
             }
 
-            const projectsAndExistence = await Promise.all(
-                environments.map(async (env) => [
-                    env,
-                    await this.environmentStore.exists(env),
-                ]),
-            );
-
-            const invalidEnvs = projectsAndExistence
-                .filter(([_, exists]) => !exists)
-                .map(([env]) => env);
-
-            if (invalidEnvs.length > 0) {
-                throw new BadDataError(
-                    `These environments do not exist and can not be selected for the project: ${invalidEnvs
-                        .map((env) => `'${env}'`)
-                        .join(', ')}.`,
-                );
-            }
+            await this.validateEnvironmentsExist(environments);
         }
     }
 
@@ -284,12 +288,18 @@ export default class ProjectService {
         newProject: CreateProject,
         user: IUser,
         auditUser: IAuditUser,
-        enableChangeRequestsForSpecifiedEnvironments: () => Promise<void> = async () => {},
+        enableChangeRequestsForSpecifiedEnvironments: (
+            environments: CreateProject['changeRequestEnvironments'],
+        ) => Promise<
+            void | ProjectCreated['changeRequestEnvironments']
+        > = async () => {
+            return;
+        },
     ): Promise<ProjectCreated> {
         await this.validateProjectEnvironments(newProject.environments);
 
         const validatedData = await projectSchema.validateAsync(newProject);
-        const data = this.removeModeForNonEnterprise(validatedData);
+        const data = this.removePropertiesForNonEnterprise(validatedData);
         await this.validateUniqueId(data.id);
 
         await this.projectStore.create(data);
@@ -310,7 +320,22 @@ export default class ProjectService {
             }),
         );
 
-        await enableChangeRequestsForSpecifiedEnvironments();
+        if (
+            this.isEnterprise &&
+            this.flagResolver.isEnabled('createProjectWithEnvironmentConfig')
+        ) {
+            // todo: this is a workaround for backwards compatibility
+            // (i.e. not breaking enterprise tests) that we can change
+            // once these changes have been merged and enterprise
+            // updated. Instead, we can exit early if there are no cr
+            // envs
+            const crEnvs = newProject.changeRequestEnvironments || [];
+            await this.validateEnvironmentsExist(crEnvs.map((env) => env.name));
+            const changeRequestEnvironments =
+                await enableChangeRequestsForSpecifiedEnvironments(crEnvs);
+
+            data.changeRequestEnvironments = changeRequestEnvironments ?? [];
+        }
 
         await this.accessService.createDefaultProjectRoles(user, data.id);
 
@@ -1357,11 +1382,11 @@ export default class ProjectService {
     }
 
     // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
-    removeModeForNonEnterprise(data): any {
+    removePropertiesForNonEnterprise(data): any {
         if (this.isEnterprise) {
             return data;
         }
-        const { mode, ...proData } = data;
+        const { mode, changeRequestEnvironments, ...proData } = data;
         return proData;
     }
 }

src/lib/services/project-schema.ts

@@ -18,5 +18,12 @@ export const projectSchema = joi
             example: joi.string().allow(null).allow('').optional(),
             description: joi.string().allow(null).allow('').optional(),
         }),
+        environments: joi.array().items(joi.string()),
+        changeRequestEnvironments: joi.array().items(
+            joi.object({
+                name: joi.string(),
+                requiredApprovals: joi.number(),
+            }),
+        ),
     })
     .options({ allowUnknown: false, stripUnknown: true });

src/lib/types/model.ts

@@ -483,6 +483,7 @@ export type CreateProject = Pick<IProject, 'id' | 'name'> & {
     mode?: ProjectMode;
     defaultStickiness?: string;
     environments?: string[];
+    changeRequestEnvironments?: { name: string; requiredApprovals?: number }[];
 };
 
 // Create project aligns with #/components/schemas/projectCreatedSchema
@@ -496,6 +497,7 @@ export type ProjectCreated = Pick<
     | 'featureLimit'
 > & {
     environments: string[];
+    changeRequestEnvironments?: { name: string; requiredApprovals: number }[];
 };
 
 export interface IProject {