From 005e5b1d1508f3b585d2d82321b14f663303134b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Fournier?= Date: Fri, 13 Jan 2023 14:55:57 +0100 Subject: [PATCH] fix: found an edge case exporting variants (#2900) ## About the changes When exporting v3, for variants backward compatibility, we need to find one featureEnvironment and fetch variants from there. In cases where the default environment is disabled (therefore does not get variants per environment when added), it can be still be selected for the export process. Therefore variants don't appear in the feature when they should be there. An e2e test that fails with the previous implementation was added to validate the behavior This comes from our support ticket 404 --- src/lib/services/state-service.ts | 49 +++++++---- src/test/e2e/api/admin/state.e2e.test.ts | 34 +++++++ .../exported3-with-default-disabled.json | 88 +++++++++++++++++++ 3 files changed, 153 insertions(+), 18 deletions(-) create mode 100644 src/test/examples/exported3-with-default-disabled.json diff --git a/src/lib/services/state-service.ts b/src/lib/services/state-service.ts index b3ecd9a149..6bbc2cc056 100644 --- a/src/lib/services/state-service.ts +++ b/src/lib/services/state-service.ts @@ -712,26 +712,35 @@ export default class StateService { const v4 = await this.exportV4({ ...opts, includeEnvironments: true }); // undefined defaults to true if (opts.includeFeatureToggles !== false) { - const keepEnv = v4.environments - .filter((env) => env.enabled !== false) - .sort((e1, e2) => { - if (e1.type !== 'production' || e2.type !== 'production') { - if (e1.type === 'production') { - return -1; - } else if (e2.type === 'production') { - return 1; - } - } - return e1.sortOrder - e2.sortOrder; - })[0]; - - const featureEnvs = v4.featureEnvironments.filter( - (fE) => fE.environment === keepEnv.name, + const enabledEnvironments = v4.environments.filter( + (env) => env.enabled !== false, ); + + const featureAndEnvs = v4.featureEnvironments.map((fE) => { + const envDetails = enabledEnvironments.find( + (env) => fE.environment === env.name, + ); + return { ...fE, ...envDetails, active: fE.enabled }; + }); v4.features = v4.features.map((f) => { - const variants = featureEnvs.find( - (fe) => fe.featureName === f.name, - )?.variants; + const variants = featureAndEnvs + .sort((e1, e2) => { + if (e1.active !== e2.active) { + return e1.active ? -1 : 1; + } + if ( + e1.type !== 'production' || + e2.type !== 'production' + ) { + if (e1.type === 'production') { + return -1; + } else if (e2.type === 'production') { + return 1; + } + } + return e1.sortOrder - e2.sortOrder; + }) + .find((fe) => fe.featureName === f.name)?.variants; return { ...f, variants }; }); v4.featureEnvironments = v4.featureEnvironments.map((fe) => { @@ -742,6 +751,10 @@ export default class StateService { // only if explicitly set to false (i.e. undefined defaults to true) if (opts.includeEnvironments === false) { delete v4.environments; + } else { + if (v4.environments.length === 0) { + throw Error('Unable to export without enabled environments'); + } } v4.version = 3; return v4; diff --git a/src/test/e2e/api/admin/state.e2e.test.ts b/src/test/e2e/api/admin/state.e2e.test.ts index 5b28412e58..900a56c6eb 100644 --- a/src/test/e2e/api/admin/state.e2e.test.ts +++ b/src/test/e2e/api/admin/state.e2e.test.ts @@ -5,6 +5,7 @@ import { DEFAULT_ENV } from '../../../../lib/util/constants'; import { collectIds } from '../../../../lib/util/collect-ids'; import { ApiTokenType } from '../../../../lib/types/models/api-token'; import variantsv3 from '../../../examples/variantsexport_v3.json'; +import v3WithDefaultDisabled from '../../../examples/exported3-with-default-disabled.json'; import { StateService } from '../../../../lib/services'; const importData = require('../../../examples/import.json'); @@ -464,3 +465,36 @@ test(`should handle v3 export with variants in features`, async () => { .sort(); expect(exportedFeatures).toStrictEqual(importedFeatures); }); + +test(`should handle v3 export with variants in features and only 1 env`, async () => { + app.services.stateService = new StateService(db.stores, { + getLogger, + flagResolver: { + isEnabled: () => false, + getAll: () => ({}), + }, + }); + + await app.request + .post('/api/admin/state/import?drop=true') + .attach( + 'file', + 'src/test/examples/exported3-with-default-disabled.json', + ) + .expect(202); + + const exported = await app.services.stateService.export({}); + let exportedFeatures = exported.features + .map((f) => { + delete f.createdAt; + return f; + }) + .sort(); + let importedFeatures = v3WithDefaultDisabled.features + .map((f) => { + delete f.createdAt; + return f; + }) + .sort(); + expect(exportedFeatures).toStrictEqual(importedFeatures); +}); diff --git a/src/test/examples/exported3-with-default-disabled.json b/src/test/examples/exported3-with-default-disabled.json new file mode 100644 index 0000000000..524634f4e0 --- /dev/null +++ b/src/test/examples/exported3-with-default-disabled.json @@ -0,0 +1,88 @@ +{ + "version": 3, + "features": [ + { + "name": "test-variants-postman", + "description": "", + "type": "experiment", + "project": "default", + "stale": false, + "createdAt": "2023-01-13T09:17:43.989Z", + "lastSeenAt": null, + "impressionData": false, + "archivedAt": null, + "archived": false, + "variants": [ + { + "name": "fake", + "weight": 500, + "payload": { + "type": "string", + "value": "123" + }, + "overrides": [], + "stickiness": "default", + "weightType": "variable" + }, + { + "name": "fake2", + "weight": 500, + "payload": { + "type": "json", + "value": "{\"hello\":\"world\"}" + }, + "overrides": [], + "stickiness": "default", + "weightType": "variable" + } + ] + } + ], + "strategies": [], + "projects": [ + { + "id": "default", + "name": "Default", + "description": "Default project", + "createdAt": "2023-01-10T13:11:03.260Z", + "health": 100, + "updatedAt": "2023-01-13T09:09:12.494Z" + } + ], + "tagTypes": [], + "tags": [], + "featureTags": [], + "featureStrategies": [], + "environments": [ + { + "name": "default", + "type": "production", + "sortOrder": 1, + "enabled": false, + "protected": true + }, + { + "name": "development", + "type": "development", + "sortOrder": 100, + "enabled": true, + "protected": false + }, + { + "name": "production", + "type": "production", + "sortOrder": 200, + "enabled": true, + "protected": false + } + ], + "featureEnvironments": [ + { + "enabled": true, + "featureName": "test-variants-postman", + "environment": "development" + } + ], + "segments": [], + "featureStrategySegments": [] +}