From 8618cec83219ac39dbddec4942d61bb354b04763 Mon Sep 17 00:00:00 2001 From: sellinjaanus <107852002+sellinjaanus@users.noreply.github.com> Date: Wed, 19 Oct 2022 15:05:07 +0300 Subject: [PATCH] Import of feature still showing env on feature, when environment is disabled on project (#2209) * Import state test * Update importer Co-authored-by: sjaanus --- src/lib/db/project-store.ts | 14 ++++- src/lib/services/feature-toggle-service.ts | 1 - src/lib/services/state-service.ts | 43 +++++++++------ src/lib/types/stores/project-store.ts | 7 ++- src/test/e2e/api/admin/state.e2e.test.ts | 41 ++++++++++---- src/test/examples/import-state.json | 53 +++++++++++++++++++ .../fake-feature-environment-store.ts | 2 +- src/test/fixtures/fake-project-store.ts | 13 ++++- 8 files changed, 139 insertions(+), 35 deletions(-) create mode 100644 src/test/examples/import-state.json diff --git a/src/lib/db/project-store.ts b/src/lib/db/project-store.ts index efddf733a6..1c661c0462 100644 --- a/src/lib/db/project-store.ts +++ b/src/lib/db/project-store.ts @@ -2,7 +2,7 @@ import { Knex } from 'knex'; import { Logger, LogProvider } from '../logger'; import NotFoundError from '../error/notfound-error'; -import { IProject, IProjectWithCount } from '../types/model'; +import { IEnvironment, IProject, IProjectWithCount } from '../types/model'; import { IProjectHealthUpdate, IProjectInsert, @@ -169,7 +169,10 @@ class ProjectStore implements IProjectStore { } } - async importProjects(projects: IProjectInsert[]): Promise { + async importProjects( + projects: IProjectInsert[], + environments?: IEnvironment[], + ): Promise { const rows = await this.db(TABLE) .insert(projects.map(this.fieldToRow)) .returning(COLUMNS) @@ -177,6 +180,13 @@ class ProjectStore implements IProjectStore { .ignore(); if (rows.length > 0) { await this.addDefaultEnvironment(rows); + environments + ?.filter((env) => env.name !== DEFAULT_ENV) + .forEach((env) => { + projects.forEach((project) => { + this.addEnvironmentToProject(project.id, env.name); + }); + }); return rows.map(this.mapRow); } return []; diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 43c32b98ad..9eb72be567 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -1006,7 +1006,6 @@ class FeatureToggleService { const defaultEnv = environments.find((e) => e.name === DEFAULT_ENV); const strategies = defaultEnv?.strategies || []; const enabled = defaultEnv?.enabled || false; - return { ...legacyFeature, enabled, strategies }; } diff --git a/src/lib/services/state-service.ts b/src/lib/services/state-service.ts index d0de790505..4737c549f7 100644 --- a/src/lib/services/state-service.ts +++ b/src/lib/services/state-service.ts @@ -164,8 +164,9 @@ export default class StateService { } const importData = await stateSchema.validateAsync(data); + let importedEnvironments: IEnvironment[] = []; if (importData.environments) { - await this.importEnvironments({ + importedEnvironments = await this.importEnvironments({ environments: data.environments, userName, dropBeforeImport, @@ -173,6 +174,16 @@ export default class StateService { }); } + if (importData.projects) { + await this.importProjects({ + projects: data.projects, + importedEnvironments, + userName, + dropBeforeImport, + keepExisting, + }); + } + if (importData.features) { let projectData; if (!importData.version || importData.version === 1) { @@ -208,15 +219,6 @@ export default class StateService { }); } - if (importData.projects) { - await this.importProjects({ - projects: data.projects, - userName, - dropBeforeImport, - keepExisting, - }); - } - if (importData.tagTypes && importData.tags) { await this.importTagData({ tagTypes: data.tagTypes, @@ -258,11 +260,14 @@ export default class StateService { async importFeatureEnvironments({ featureEnvironments }): Promise { await Promise.all( featureEnvironments.map((env) => - this.featureEnvironmentStore.addEnvironmentToFeature( - env.featureName, - env.environment, - env.enabled, - ), + this.toggleStore + .getProjectId(env.featureName) + .then((id) => + this.featureEnvironmentStore.connectFeatureToEnvironmentsForProject( + env.featureName, + id, + ), + ), ), ); } @@ -410,7 +415,7 @@ export default class StateService { userName, dropBeforeImport, keepExisting, - }): Promise { + }): Promise { this.logger.info(`Import ${environments.length} projects`); const oldEnvs = dropBeforeImport ? [] @@ -427,8 +432,9 @@ export default class StateService { const envsImport = environments.filter((env) => keepExisting ? !oldEnvs.some((old) => old.name === env.name) : true, ); + let importedEnvs = []; if (envsImport.length > 0) { - const importedEnvs = await this.environmentStore.importEnvironments( + importedEnvs = await this.environmentStore.importEnvironments( envsImport, ); const importedEnvironmentEvents = importedEnvs.map((env) => ({ @@ -447,11 +453,13 @@ export default class StateService { this.apiTokenStore.delete(apiToken.secret), ); } + return importedEnvs; } // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types async importProjects({ projects, + importedEnvironments, userName, dropBeforeImport, keepExisting, @@ -477,6 +485,7 @@ export default class StateService { if (projectsToImport.length > 0) { const importedProjects = await this.projectStore.importProjects( projectsToImport, + importedEnvironments, ); const importedProjectEvents = importedProjects.map((project) => ({ type: PROJECT_IMPORT, diff --git a/src/lib/types/stores/project-store.ts b/src/lib/types/stores/project-store.ts index cdacb37337..56834ac937 100644 --- a/src/lib/types/stores/project-store.ts +++ b/src/lib/types/stores/project-store.ts @@ -2,7 +2,7 @@ import { IEnvironmentProjectLink, IProjectMembersCount, } from '../../db/project-store'; -import { IProject, IProjectWithCount } from '../model'; +import { IEnvironment, IProject, IProjectWithCount } from '../model'; import { Store } from './store'; export interface IProjectInsert { @@ -31,7 +31,10 @@ export interface IProjectStore extends Store { updateHealth(healthUpdate: IProjectHealthUpdate): Promise; create(project: IProjectInsert): Promise; update(update: IProjectInsert): Promise; - importProjects(projects: IProjectInsert[]): Promise; + importProjects( + projects: IProjectInsert[], + environments?: IEnvironment[], + ): Promise; addEnvironmentToProject(id: string, environment: string): Promise; deleteEnvironmentForProject(id: string, environment: string): Promise; getEnvironmentsForProject(id: string): Promise; diff --git a/src/test/e2e/api/admin/state.e2e.test.ts b/src/test/e2e/api/admin/state.e2e.test.ts index 7ccf1856c7..9cd69cab13 100644 --- a/src/test/e2e/api/admin/state.e2e.test.ts +++ b/src/test/e2e/api/admin/state.e2e.test.ts @@ -260,14 +260,7 @@ test('Roundtrip with strategies in multiple environments works', async () => { id: projectId, description: 'Project for export', }); - await app.services.environmentService.addEnvironmentToProject( - environment, - projectId, - ); - await app.services.environmentService.addEnvironmentToProject( - DEFAULT_ENV, - projectId, - ); + await app.services.featureToggleServiceV2.createFeatureToggle( projectId, { @@ -277,6 +270,15 @@ test('Roundtrip with strategies in multiple environments works', async () => { }, userName, ); + await app.services.environmentService.addEnvironmentToProject( + environment, + projectId, + ); + + await app.services.environmentService.addEnvironmentToProject( + DEFAULT_ENV, + projectId, + ); await app.services.featureToggleServiceV2.createStrategy( { name: 'default', @@ -307,7 +309,7 @@ test('Roundtrip with strategies in multiple environments works', async () => { userName: 'export-tester', }); const f = await app.services.featureToggleServiceV2.getFeature(featureName); - expect(f.environments).toHaveLength(2); + expect(f.environments).toHaveLength(4); }); test(`Importing version 2 replaces :global: environment with 'default'`, async () => { @@ -320,7 +322,7 @@ test(`Importing version 2 replaces :global: environment with 'default'`, async ( const feature = await app.services.featureToggleServiceV2.getFeatureToggle( 'this-is-fun', ); - expect(feature.environments).toHaveLength(1); + expect(feature.environments).toHaveLength(4); expect(feature.environments[0].name).toBe(DEFAULT_ENV); }); @@ -437,3 +439,22 @@ test(`should clean apitokens for not existing environment after import with drop const apiTokens = await app.services.apiTokenService.getAllTokens(); expect(apiTokens.length).toEqual(0); }); + +test(`should not show environment on feature toggle, when environment is disabled`, async () => { + await app.request + .post('/api/admin/state/import?drop=true') + .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'); +}); diff --git a/src/test/examples/import-state.json b/src/test/examples/import-state.json new file mode 100644 index 0000000000..0360445bc5 --- /dev/null +++ b/src/test/examples/import-state.json @@ -0,0 +1,53 @@ +{ + "version": 2, + "features": [ + { + "name": "my-feature", + "description": "", + "type": "release", + "project": "default", + "stale": false, + "variants": [], + "createdAt": "2021-09-17T07:06:40.925Z", + "lastSeenAt": null + } + ], + "featureStrategies": [ + { + "id": "2ea91298-4565-4db2-8a23-50757001a076", + "featureName": "my-feature", + "projectId": "default", + "environment": "state-visible-environment", + "strategyName": "gradualRolloutRandom", + "parameters": { + "percentage": "100" + }, + "constraints": [], + "createdAt": "2021-09-17T07:23:39.374Z" + } + ], + "environments": [ + { + "name": "state-visible-environment", + "type": "production", + "displayName": "Visible" + }, + { + "name": "state-hidden-environment", + "type": "production", + "displayName": "Hidden" + } + ], + "featureEnvironments": [ + { + "enabled": true, + "featureName": "my-feature", + "environment": "state-visible-environment" + }, + { + "enabled": false, + "featureName": "my-feature", + "environment": "state-hidden-environment" + } + ] +} diff --git a/src/test/fixtures/fake-feature-environment-store.ts b/src/test/fixtures/fake-feature-environment-store.ts index d14db3aba0..197032c0ff 100644 --- a/src/test/fixtures/fake-feature-environment-store.ts +++ b/src/test/fixtures/fake-feature-environment-store.ts @@ -150,7 +150,7 @@ export default class FakeFeatureEnvironmentStore // eslint-disable-next-line @typescript-eslint/no-unused-vars projectId: string, ): Promise { - return Promise.reject(new Error('Not implemented')); + return Promise.resolve(); } disableEnvironmentIfNoStrategies( diff --git a/src/test/fixtures/fake-project-store.ts b/src/test/fixtures/fake-project-store.ts index 7cf6635377..a447ef8fca 100644 --- a/src/test/fixtures/fake-project-store.ts +++ b/src/test/fixtures/fake-project-store.ts @@ -3,7 +3,11 @@ import { IProjectInsert, IProjectStore, } from '../../lib/types/stores/project-store'; -import { IProject, IProjectWithCount } from '../../lib/types/model'; +import { + IEnvironment, + IProject, + IProjectWithCount, +} from '../../lib/types/model'; import NotFoundError from '../../lib/error/notfound-error'; import { IEnvironmentProjectLink, @@ -110,7 +114,12 @@ export default class FakeProjectStore implements IProjectStore { return this.exists(id); } - async importProjects(projects: IProjectInsert[]): Promise { + async importProjects( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + projects: IProjectInsert[], + // eslint-disable-next-line @typescript-eslint/no-unused-vars + environments?: IEnvironment[], + ): Promise { return projects.map((p) => this.createInternal(p)); }