From 4c16eb507bde2b9a0f1c65f819190060361c94bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Thu, 5 Jan 2023 11:30:07 +0100 Subject: [PATCH] fix: export features with variants event when feature is disabled (#2824) ## About the changes When exporting features one is normally also interested in disabled features, so they are also included in the export file, while the variants are not. I do not see a good reason for that, so this PR removes the check and exports the variants then as well. I could also add an option as well, but as long as there is no good reason for ignoring the variants I would just export them with the features. This PR adds tests on #2719 Closes #2719 Co-authored-by: Martin Joehren --- src/lib/services/state-service.test.ts | 152 ++++++++++++++----------- src/lib/services/state-service.ts | 2 +- 2 files changed, 85 insertions(+), 69 deletions(-) diff --git a/src/lib/services/state-service.test.ts b/src/lib/services/state-service.test.ts index 4232fb06b19..a09867820a3 100644 --- a/src/lib/services/state-service.test.ts +++ b/src/lib/services/state-service.test.ts @@ -28,77 +28,43 @@ function getSetup() { async function setupV3VariantsCompatibilityScenario( variantsPerEnvironment: boolean, + envs = [ + { name: 'env-2', enabled: true }, + { name: 'env-3', enabled: true }, + { name: 'env-1', enabled: true }, + ], ) { const stores = createStores(); - await stores.environmentStore.create({ - name: 'env-1', - type: 'production', - sortOrder: 3, - }); - await stores.environmentStore.create({ - name: 'env-2', - type: 'production', - sortOrder: 1, - }); - await stores.environmentStore.create({ - name: 'env-3', - type: 'production', - sortOrder: 2, - }); await stores.featureToggleStore.create('some-project', { name: 'Feature-with-variants', }); - await stores.featureEnvironmentStore.addEnvironmentToFeature( - 'Feature-with-variants', - 'env-1', - true, - ); - await stores.featureEnvironmentStore.addEnvironmentToFeature( - 'Feature-with-variants', - 'env-2', - true, - ); - await stores.featureEnvironmentStore.addEnvironmentToFeature( - 'Feature-with-variants', - 'env-3', - true, - ); - await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( - 'Feature-with-variants', - 'env-1', - [ - { - name: 'env-1-variant', - stickiness: 'default', - weight: 1000, - weightType: 'variable', - }, - ], - ); - await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( - 'Feature-with-variants', - 'env-2', - [ - { - name: 'env-2-variant', - stickiness: 'default', - weight: 1000, - weightType: 'variable', - }, - ], - ); - await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( - 'Feature-with-variants', - 'env-3', - [ - { - name: 'env-3-variant', - stickiness: 'default', - weight: 1000, - weightType: 'variable', - }, - ], - ); + let sortOrder = 1; + envs.forEach(async (env) => { + await stores.environmentStore.create({ + name: env.name, + type: 'production', + sortOrder: sortOrder++, + }); + await stores.featureEnvironmentStore.addEnvironmentToFeature( + 'Feature-with-variants', + env.name, + env.enabled, + ); + await stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + 'Feature-with-variants', + env.name, + [ + { + name: variantsPerEnvironment + ? `${env.name}-variant` + : 'variant-name', + stickiness: 'default', + weight: 1000, + weightType: 'variable', + }, + ], + ); + }); return { stateService: new StateService(stores, { getLogger, @@ -671,10 +637,60 @@ test('exporting variants to v3 format should pick variants from the correct env' const exported = await stateService.export({}); expect(exported.features).toHaveLength(1); - // env 2 has the lowest sortOrder so we expect env-2-variant to be selected + + expect(exported.features[0].variants).toStrictEqual([ + { + name: 'variant-name', + stickiness: 'default', + weight: 1000, + weightType: 'variable', + }, + ]); + exported.featureEnvironments.forEach((fe) => + expect(fe.variants).toBeUndefined(), + ); + expect(exported.environments).toHaveLength(3); +}); + +// This test should be removed as soon as variants per environment is GA +test('exporting variants to v3 format when some envs are disabled should export variants', async () => { + const { stateService } = await setupV3VariantsCompatibilityScenario(false, [ + { name: 'env-2', enabled: false }, + { name: 'env-3', enabled: false }, + { name: 'env-1', enabled: true }, + ]); + + const exported = await stateService.export({}); + expect(exported.features).toHaveLength(1); + + expect(exported.features[0].variants).toStrictEqual([ + { + name: 'variant-name', + stickiness: 'default', + weight: 1000, + weightType: 'variable', + }, + ]); + exported.featureEnvironments.forEach((fe) => + expect(fe.variants).toBeUndefined(), + ); + expect(exported.environments).toHaveLength(3); +}); + +// This test should be removed as soon as variants per environment is GA +test('exporting variants to v3 format when all envs are disabled should export variants', async () => { + const { stateService } = await setupV3VariantsCompatibilityScenario(false, [ + { name: 'env-2', enabled: false }, + { name: 'env-3', enabled: false }, + { name: 'env-1', enabled: false }, + ]); + + const exported = await stateService.export({}); + expect(exported.features).toHaveLength(1); + expect(exported.features[0].variants).toStrictEqual([ { - name: 'env-2-variant', + name: 'variant-name', stickiness: 'default', weight: 1000, weightType: 'variable', diff --git a/src/lib/services/state-service.ts b/src/lib/services/state-service.ts index 73ef43b6e5f..6a187639c65 100644 --- a/src/lib/services/state-service.ts +++ b/src/lib/services/state-service.ts @@ -743,7 +743,7 @@ export default class StateService { ); v4.features = v4.features.map((f) => { const variants = featureEnvs.find( - (fe) => fe.enabled !== false && fe.featureName === f.name, + (fe) => fe.featureName === f.name, )?.variants; return { ...f, variants }; });