mirror of
https://github.com/Unleash/unleash.git
synced 2024-12-22 19:07:54 +01:00
chore: select enabled environments on project creation (#6869)
This PR adds functionality to the `createProject` function to choose which environments should be enabled when you create a new project. The new `environments` property is optional and omitting it will make it work exactly as it does today. The current implementation is fairly strict. We have some potential ideas to make it easier to work with, but we haven't agreed on any yet. Making it this strict means that we can always relax the rules later. The rules are (codified in tests): - If `environments` is not provided, all non-deprecated environments are enabled - If `environments` is provided, only the environments listed are enabled, regardless of whether they're deprecated or not - If `environments` is provided and is an empty array, the service throws an error. The API should dilsallow that via the schema anyway, but this catches it in case it sneaks in some other way. - If `environments` is provided and contains one or more environments that don't exist, the service throws an error. While we could ignore them, that would lead to more complexity because we'd have to also check that the at least one of the environments is valid. It also leads to silent ignoring of errors, which may or may not be good for the user experience. The API endpoint for this sits in enterprise, so no customer-facing changes are part of this.
This commit is contained in:
parent
6b5cdc2d24
commit
bda5eda224
@ -25,7 +25,7 @@ import {
|
||||
SYSTEM_USER_ID,
|
||||
} from '../../types';
|
||||
import type { User } from '../../server-impl';
|
||||
import { InvalidOperationError } from '../../error';
|
||||
import { BadDataError, InvalidOperationError } from '../../error';
|
||||
|
||||
let stores: IUnleashStores;
|
||||
let db: ITestDb;
|
||||
@ -68,6 +68,11 @@ beforeAll(async () => {
|
||||
await stores.accessStore.addUserToRole(opsUser.id, 1, '');
|
||||
const config = createTestConfig({
|
||||
getLogger,
|
||||
experimental: {
|
||||
flags: {
|
||||
createProjectWithEnvironmentConfig: true,
|
||||
},
|
||||
},
|
||||
});
|
||||
eventService = new EventService(stores, config);
|
||||
accessService = createAccessService(db.rawDatabase, config);
|
||||
@ -2481,3 +2486,95 @@ test('should get project settings with mode', async () => {
|
||||
expect(foundProjectTwo!.mode).toBe('open');
|
||||
expect(foundProjectTwo!.defaultStickiness).toBe('default');
|
||||
});
|
||||
|
||||
describe('create project with environments', () => {
|
||||
const disabledEnv = { name: 'disabled', type: 'production' };
|
||||
|
||||
const extraEnvs = [
|
||||
{ name: 'development', type: 'development' },
|
||||
{ name: 'production', type: 'production' },
|
||||
{ name: 'staging', type: 'staging' },
|
||||
{ name: 'QA', type: 'QA' },
|
||||
disabledEnv,
|
||||
];
|
||||
|
||||
const allEnabledEnvs = [
|
||||
'QA',
|
||||
'default',
|
||||
'development',
|
||||
'production',
|
||||
'staging',
|
||||
];
|
||||
|
||||
beforeEach(async () => {
|
||||
await Promise.all(
|
||||
extraEnvs.map((env) => stores.environmentStore.create(env)),
|
||||
);
|
||||
|
||||
await stores.environmentStore.disable([
|
||||
{ ...disabledEnv, enabled: true, protected: false, sortOrder: 5 },
|
||||
]);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await Promise.all(
|
||||
extraEnvs.map((env) => stores.environmentStore.delete(env.name)),
|
||||
);
|
||||
});
|
||||
|
||||
const createProjectWithEnvs = async (environments) => {
|
||||
const project = await projectService.createProject(
|
||||
{
|
||||
id: randomId(),
|
||||
name: 'New name',
|
||||
mode: 'open' as const,
|
||||
defaultStickiness: 'default',
|
||||
...(environments ? { environments } : {}),
|
||||
},
|
||||
user,
|
||||
);
|
||||
|
||||
const projectEnvs = (
|
||||
await projectService.getProjectOverview(project.id)
|
||||
).environments.map(({ environment }) => environment);
|
||||
|
||||
projectEnvs.sort();
|
||||
return projectEnvs;
|
||||
};
|
||||
|
||||
test('no environments specified means all enabled envs are enabled', async () => {
|
||||
const created = await createProjectWithEnvs(undefined);
|
||||
|
||||
expect(created).toMatchObject(allEnabledEnvs);
|
||||
});
|
||||
|
||||
test('an empty list throws an error', async () => {
|
||||
// You shouldn't be allowed to pass an empty list via the API.
|
||||
// This test checks what happens in the event that an empty
|
||||
// list manages to sneak in.
|
||||
await expect(createProjectWithEnvs([])).rejects.toThrow(BadDataError);
|
||||
});
|
||||
|
||||
test('it only enables the envs it is asked to enable', async () => {
|
||||
const selectedEnvs = ['development', 'production'];
|
||||
const created = await createProjectWithEnvs(selectedEnvs);
|
||||
|
||||
expect(created).toMatchObject(selectedEnvs);
|
||||
});
|
||||
|
||||
test('it enables deprecated environments when asked explicitly', async () => {
|
||||
const selectedEnvs = ['disabled'];
|
||||
const created = await createProjectWithEnvs(selectedEnvs);
|
||||
|
||||
expect(created).toMatchObject(selectedEnvs);
|
||||
});
|
||||
|
||||
test("envs that don't exist cause errors", async () => {
|
||||
await expect(createProjectWithEnvs(['fake-project'])).rejects.toThrow(
|
||||
BadDataError,
|
||||
);
|
||||
await expect(createProjectWithEnvs(['fake-project'])).rejects.toThrow(
|
||||
/'fake-project'/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
@ -248,27 +248,63 @@ export default class ProjectService {
|
||||
return featureNaming;
|
||||
};
|
||||
|
||||
async validateProjectEnvironments(environments: string[] | undefined) {
|
||||
if (
|
||||
this.flagResolver.isEnabled('createProjectWithEnvironmentConfig') &&
|
||||
environments
|
||||
) {
|
||||
if (environments.length === 0) {
|
||||
throw new BadDataError(
|
||||
'A project must always have at least one environment.',
|
||||
);
|
||||
}
|
||||
|
||||
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(', ')}.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async createProject(
|
||||
newProject: CreateProject,
|
||||
user: IUser,
|
||||
): Promise<IProject> {
|
||||
await this.validateProjectEnvironments(newProject.environments);
|
||||
|
||||
const validatedData = await projectSchema.validateAsync(newProject);
|
||||
const data = this.removeModeForNonEnterprise(validatedData);
|
||||
await this.validateUniqueId(data.id);
|
||||
|
||||
await this.projectStore.create(data);
|
||||
|
||||
const enabledEnvironments = await this.environmentStore.getAll({
|
||||
const envsToEnable =
|
||||
this.flagResolver.isEnabled('createProjectWithEnvironmentConfig') &&
|
||||
newProject.environments?.length
|
||||
? newProject.environments
|
||||
: (
|
||||
await this.environmentStore.getAll({
|
||||
enabled: true,
|
||||
});
|
||||
})
|
||||
).map((env) => env.name);
|
||||
|
||||
// TODO: Only if enabled!
|
||||
await Promise.all(
|
||||
enabledEnvironments.map(async (e) => {
|
||||
await this.featureEnvironmentStore.connectProject(
|
||||
e.name,
|
||||
data.id,
|
||||
);
|
||||
envsToEnable.map(async (env) => {
|
||||
await this.featureEnvironmentStore.connectProject(env, data.id);
|
||||
}),
|
||||
);
|
||||
|
||||
|
@ -471,6 +471,7 @@ export interface IImportData extends ImportCommon {
|
||||
export type CreateProject = Pick<IProject, 'id' | 'name'> & {
|
||||
mode?: ProjectMode;
|
||||
defaultStickiness?: string;
|
||||
environments?: string[];
|
||||
};
|
||||
|
||||
export interface IProject {
|
||||
|
Loading…
Reference in New Issue
Block a user