diff --git a/src/lib/db/feature-strategy-store.ts b/src/lib/db/feature-strategy-store.ts index 1015cf50c6..bb924578ff 100644 --- a/src/lib/db/feature-strategy-store.ts +++ b/src/lib/db/feature-strategy-store.ts @@ -303,7 +303,15 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { if (withEnvironmentVariants) { env.variants = variants; } - acc.variants = variants; + + // this code sets variants at the feature level (should be deprecated with variants per environment) + const currentVariants = new Map( + acc.variants?.map((v) => [v.name, v]), + ); + variants.forEach((variant) => { + currentVariants.set(variant.name, variant); + }); + acc.variants = Array.from(currentVariants.values()); env.enabled = r.enabled; env.type = r.environment_type; diff --git a/src/lib/routes/admin-api/project/variants.ts b/src/lib/routes/admin-api/project/variants.ts index 0825e36da7..d0741b2e3e 100644 --- a/src/lib/routes/admin-api/project/variants.ts +++ b/src/lib/routes/admin-api/project/variants.ts @@ -143,6 +143,11 @@ export default class VariantsController extends Controller { }); } + /** + * @deprecated - Variants should be fetched from featureService.getVariantsForEnv (since variants are now; since 4.18, connected to environments) + * @param req + * @param res + */ async getVariants( req: Request, res: Response, @@ -158,7 +163,6 @@ export default class VariantsController extends Controller { ): Promise { const { projectId, featureName } = req.params; const userName = extractUsername(req); - const updatedFeature = await this.featureService.updateVariants( featureName, projectId, diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 98dba11637..13f5df35ad 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -655,6 +655,7 @@ class FeatureToggleService { /** * GET /api/admin/projects/:project/features/:featureName/variants + * @deprecated - Variants should be fetched from FeatureEnvironmentStore (since variants are now; since 4.18, connected to environments) * @param featureName * @return The list of variants */ @@ -1250,9 +1251,22 @@ class FeatureToggleService { newVariants: Operation[], createdBy: string, ): Promise { - const oldVariants = await this.getVariants(featureName); - const { newDocument } = applyPatch(oldVariants, newVariants); - return this.saveVariants(featureName, project, newDocument, createdBy); + const ft = + await this.featureStrategiesStore.getFeatureToggleWithVariantEnvs( + featureName, + ); + const promises = ft.environments.map((env) => + this.updateVariantsOnEnv( + featureName, + project, + env.name, + newVariants, + createdBy, + ).then((resultingVariants) => (env.variants = resultingVariants)), + ); + await Promise.all(promises); + ft.variants = ft.environments[0].variants; + return ft; } async updateVariantsOnEnv( @@ -1273,6 +1287,7 @@ class FeatureToggleService { environment, newDocument, createdBy, + oldVariants, ); } @@ -1312,15 +1327,18 @@ class FeatureToggleService { environment: string, newVariants: IVariant[], createdBy: string, + oldVariants?: IVariant[], ): Promise { await variantsArraySchema.validateAsync(newVariants); const fixedVariants = this.fixVariantWeights(newVariants); - const oldVariants = ( - await this.featureEnvironmentStore.get({ - featureName, - environment, - }) - ).variants; + const theOldVariants: IVariant[] = + oldVariants || + ( + await this.featureEnvironmentStore.get({ + featureName, + environment, + }) + ).variants; await this.eventStore.store( new EnvironmentVariantEvent({ @@ -1328,7 +1346,7 @@ class FeatureToggleService { environment, project: projectId, createdBy, - oldVariants, + oldVariants: theOldVariants, newVariants: fixedVariants, }), ); diff --git a/src/test/e2e/api/admin/project/variants.e2e.test.ts b/src/test/e2e/api/admin/project/variants.e2e.test.ts index e66b3d7bfe..d3da1ece46 100644 --- a/src/test/e2e/api/admin/project/variants.e2e.test.ts +++ b/src/test/e2e/api/admin/project/variants.e2e.test.ts @@ -120,6 +120,104 @@ test('Can patch variants for a feature and get a response of new variant', async }); }); +test('Can patch variants for a feature patches all environments independently', async () => { + const featureName = 'feature-to-patch'; + const addedVariantName = 'patched-variant-name'; + const variants = (name: string) => [ + { + name, + stickiness: 'default', + weight: 1000, + weightType: WeightType.VARIABLE, + }, + ]; + + await db.stores.environmentStore.create({ + name: 'development', + type: 'development', + }); + await db.stores.environmentStore.create({ + name: 'production', + type: 'production', + }); + await db.stores.featureToggleStore.create('default', { + name: featureName, + }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'development', + true, + ); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'production', + true, + ); + await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + 'development', + variants('dev-variant'), + ); + await db.stores.featureEnvironmentStore.addVariantsToFeatureEnvironment( + featureName, + 'production', + variants('prod-variant'), + ); + + const patch = [ + { + op: 'add', + path: '/1', + value: { + name: addedVariantName, + weightType: 'fix', + weight: 50, + }, + }, + ]; + + await app.request + .patch(`/api/admin/projects/default/features/${featureName}/variants`) + .send(patch) + .expect(200) + .expect((res) => { + expect(res.body.version).toBe(1); + expect(res.body.variants).toHaveLength(2); + // it picks variants from the first environment (sorted by name) + expect(res.body.variants[0].name).toBe('dev-variant'); + expect(res.body.variants[1].name).toBe(addedVariantName); + }); + + await app.request + .get( + `/api/admin/projects/default/features/${featureName}?variantEnvironments=true`, + ) + .expect((res) => { + const environments = res.body.environments; + expect(environments).toHaveLength(2); + const developmentVariants = environments.find( + (x) => x.name === 'development', + ).variants; + const productionVariants = environments.find( + (x) => x.name === 'production', + ).variants; + expect(developmentVariants).toHaveLength(2); + expect(productionVariants).toHaveLength(2); + expect( + developmentVariants.find((x) => x.name === addedVariantName), + ).toBeTruthy(); + expect( + productionVariants.find((x) => x.name === addedVariantName), + ).toBeTruthy(); + expect( + developmentVariants.find((x) => x.name === 'dev-variant'), + ).toBeTruthy(); + expect( + productionVariants.find((x) => x.name === 'prod-variant'), + ).toBeTruthy(); + }); +}); + test('Can add variant for a feature', async () => { const featureName = 'feature-variants-patch-add'; const variantName = 'fancy-variant-patch'; @@ -315,6 +413,11 @@ test('Invalid variant in PATCH also throws 400 exception', async () => { await db.stores.featureToggleStore.create('default', { name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); const invalidPatch = `[{ "op": "add", @@ -439,6 +542,11 @@ test('PATCHING with no variable variants fails with 400', async () => { await db.stores.featureToggleStore.create('default', { name: featureName, }); + await db.stores.featureEnvironmentStore.addEnvironmentToFeature( + featureName, + 'default', + true, + ); const newVariants: IVariant[] = [];