1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-07-26 13:48:33 +02:00

fix: broken UI after import (#2447)

fix: broken UI when importing features into environments which are not linked to the feature's project

## Related to
- PR: https://github.com/Unleash/unleash/pull/2209
- Issue: https://github.com/Unleash/unleash/issues/2186
- Issue: https://github.com/Unleash/unleash/issues/2193

## Expected behaviour:
After importing we should see:

![image](https://user-images.githubusercontent.com/455064/202149719-fa74b3b7-3936-443b-9d0e-8f1ca2e779f4.png)

## About the changes
**The problem:** when we import we have projects, features and
environments. Each feature belongs to a project (this is by default and
the imported file enforces that). The links between projects and
features, or projects and environments, depend on us creating those
relationships. When we add a feature to an environment we're not
validating that the project and the environment are connected. Because
of that, in some situations (like in this test), we can end up with a
project with features but no environment.

This breaks a weak constraint we had which is that all projects should
have at least one environment.

**This PR makes the following assumption when importing**: _if a feature
is added to an environment, and that environment is still not linked to
the project that feature belongs to, then the project and environments
have to be linked_. The rationale behind this is that the user couldn't
have generated this export file without the project and environment
being linked together.
This commit is contained in:
Gastón Fournier 2022-11-17 14:05:57 +01:00 committed by GitHub
parent 726ede5cbe
commit dc08f1dadd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 77 additions and 22 deletions

View File

@ -217,11 +217,17 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore {
async connectProject(
environment: string,
projectId: string,
idempotent?: boolean, // default false to respect old behavior
): Promise<void> {
await this.db('project_environments').insert({
const query = this.db('project_environments').insert({
environment_name: environment,
project_id: projectId,
});
if (idempotent) {
await query.onConflict(['environment_name', 'project_id']).ignore();
} else {
await query;
}
}
async connectFeatures(
@ -258,6 +264,7 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore {
async connectFeatureToEnvironmentsForProject(
featureName: string,
projectId: string,
enabledIn: { [environment: string]: boolean } = {},
): Promise<void> {
const environmentsToEnable = await this.db('project_environments')
.select('environment_name')
@ -268,7 +275,7 @@ export class FeatureEnvironmentStore implements IFeatureEnvironmentStore {
.insert({
environment: env.environment_name,
feature_name: featureName,
enabled: false,
enabled: enabledIn[env.environment_name] || false,
})
.onConflict(['environment', 'feature_name'])
.ignore();

View File

@ -200,9 +200,19 @@ export default class StateService {
dropBeforeImport,
keepExisting,
});
await this.importFeatureEnvironments({
featureEnvironments,
});
if (featureEnvironments) {
// make sure the project and environment are connected
// before importing featureEnvironments
await this.linkFeatureEnvironments({
features,
featureEnvironments,
});
await this.importFeatureEnvironments({
featureEnvironments,
});
}
await this.importFeatureStrategies({
featureStrategies,
dropBeforeImport,
@ -256,6 +266,35 @@ export default class StateService {
}
}
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
async linkFeatureEnvironments({
features,
featureEnvironments,
}): Promise<void> {
const linkTasks = featureEnvironments.map(async (fe) => {
const project = features.find(
(f) => f.project && f.name === fe.featureName,
).project;
if (project) {
return this.featureEnvironmentStore.connectProject(
fe.environment,
project,
true, // make it idempotent
);
}
});
await Promise.all(linkTasks);
}
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
enabledInConfiguration(feature: string, env) {
const config = {};
env.filter((e) => e.featureName === feature).forEach((e) => {
config[e.environment] = e.enabled || false;
});
return config;
}
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
async importFeatureEnvironments({ featureEnvironments }): Promise<void> {
await Promise.all(
@ -266,6 +305,10 @@ export default class StateService {
this.featureEnvironmentStore.connectFeatureToEnvironmentsForProject(
env.featureName,
id,
this.enabledInConfiguration(
env.featureName,
featureEnvironments,
),
),
),
),

View File

@ -44,9 +44,14 @@ export interface IFeatureEnvironmentStore
connectFeatureToEnvironmentsForProject(
featureName: string,
projectId: string,
enabledIn?: { [environment: string]: boolean },
): Promise<void>;
connectProject(environment: string, projectId: string): Promise<void>;
connectProject(
environment: string,
projectId: string,
idempotent?: boolean,
): Promise<void>;
disconnectProject(environment: string, projectId: string): Promise<void>;
copyEnvironmentFeaturesByProjects(
sourceEnvironment: string,

View File

@ -452,15 +452,15 @@ test(`should not show environment on feature toggle, when environment is disable
.attach('file', 'src/test/examples/import-state.json')
.expect(202);
await app.request
.post('/api/admin/projects/default/environments')
.send({ environment: 'state-visible-environment' })
.expect(200);
const { body } = await app.request
.get('/api/admin/projects/default/features/my-feature')
.expect(200);
expect(body.environments).toHaveLength(1);
expect(body.environments[0].name).toBe('state-visible-environment');
// sort to have predictable test results
const result = body.environments.sort((e1, e2) => e1.name < e2.name);
expect(result).toHaveLength(2);
expect(result[0].name).toBe('development');
expect(result[0].enabled).toBeTruthy();
expect(result[1].name).toBe('production');
expect(result[1].enabled).toBeFalsy();
});

View File

@ -17,7 +17,7 @@
"id": "2ea91298-4565-4db2-8a23-50757001a076",
"featureName": "my-feature",
"projectId": "default",
"environment": "state-visible-environment",
"environment": "development",
"strategyName": "gradualRolloutRandom",
"parameters": {
"percentage": "100"
@ -28,26 +28,26 @@
],
"environments": [
{
"name": "state-visible-environment",
"type": "production",
"displayName": "Visible"
"name": "development",
"type": "development",
"displayName": "Dev"
},
{
"name": "state-hidden-environment",
"name": "production",
"type": "production",
"displayName": "Hidden"
"displayName": "Prod"
}
],
"featureEnvironments": [
{
"enabled": true,
"featureName": "my-feature",
"environment": "state-visible-environment"
"environment": "development"
},
{
"enabled": false,
"featureName": "my-feature",
"environment": "state-hidden-environment"
"environment": "production"
}
]
}

View File

@ -123,7 +123,7 @@ export default class FakeFeatureEnvironmentStore
// eslint-disable-next-line @typescript-eslint/no-unused-vars
projectId: string,
): Promise<void> {
return Promise.reject(new Error('Not implemented'));
return Promise.resolve(undefined);
}
async connectFeatures(