Skip to content

Commit

Permalink
fix: found an edge case exporting variants (#2900)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
Gastón Fournier committed Jan 13, 2023
1 parent 3a08122 commit 6996584
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 18 deletions.
49 changes: 31 additions & 18 deletions src/lib/services/state-service.ts
Expand Up @@ -725,26 +725,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) => {
Expand All @@ -755,6 +764,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;
Expand Down
34 changes: 34 additions & 0 deletions src/test/e2e/api/admin/state.e2e.test.ts
Expand Up @@ -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');
Expand Down Expand Up @@ -498,3 +499,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);
});
88 changes: 88 additions & 0 deletions 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": []
}

0 comments on commit 6996584

Please sign in to comment.