diff --git a/src/lib/db/feature-strategy-store.ts b/src/lib/db/feature-strategy-store.ts index 8484de0eb0..fd2d7c9ee7 100644 --- a/src/lib/db/feature-strategy-store.ts +++ b/src/lib/db/feature-strategy-store.ts @@ -286,6 +286,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { ); return e; }); + featureToggle.variants = featureToggle.variants || []; featureToggle.archived = archived; return featureToggle; } diff --git a/src/lib/db/feature-toggle-store.ts b/src/lib/db/feature-toggle-store.ts index a9d5bb0fbc..9958f6e9d0 100644 --- a/src/lib/db/feature-toggle-store.ts +++ b/src/lib/db/feature-toggle-store.ts @@ -161,7 +161,7 @@ export default class FeatureToggleStore implements IFeatureToggleStore { type: row.type, project: row.project, stale: row.stale, - variants: row.variants as unknown as IVariant[], + variants: (row.variants as unknown as IVariant[]) || [], createdAt: row.created_at, lastSeenAt: row.last_seen_at, }; @@ -182,7 +182,9 @@ export default class FeatureToggleStore implements IFeatureToggleStore { project, archived: data.archived || false, stale: data.stale, - variants: data.variants ? JSON.stringify(data.variants) : null, + variants: data.variants + ? JSON.stringify(data.variants) + : JSON.stringify([]), created_at: data.createdAt, }; if (!row.created_at) { diff --git a/src/lib/routes/admin-api/project/variants.ts b/src/lib/routes/admin-api/project/variants.ts index cc88c303ff..a73e076821 100644 --- a/src/lib/routes/admin-api/project/variants.ts +++ b/src/lib/routes/admin-api/project/variants.ts @@ -43,7 +43,7 @@ export default class VariantsController extends Controller { ): Promise { const { featureName } = req.params; const variants = await this.featureService.getVariants(featureName); - res.status(200).json({ version: '1', variants }); + res.status(200).json({ version: '1', variants: variants || [] }); } async patchVariants( diff --git a/src/lib/schema/feature-schema.ts b/src/lib/schema/feature-schema.ts index 4ea4e1b6c5..f2501b0492 100644 --- a/src/lib/schema/feature-schema.ts +++ b/src/lib/schema/feature-schema.ts @@ -42,7 +42,11 @@ export const variantsSchema = joi.object().keys({ ), }); -export const variantsArraySchema = joi.array().min(0).items(variantsSchema); +export const variantsArraySchema = joi + .array() + .min(0) + .items(variantsSchema) + .unique((a, b) => a.name === b.name); export const featureMetadataSchema = joi .object() 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 de39df76d0..cf25eceaf0 100644 --- a/src/test/e2e/api/admin/project/variants.e2e.test.ts +++ b/src/test/e2e/api/admin/project/variants.e2e.test.ts @@ -643,3 +643,74 @@ test('If sum of fixed variant weight equals 1000 variable variants gets weight 0 ).toEqual(0); }); }); + +test('PATCH endpoint validates uniqueness of variant names', async () => { + const featureName = 'variants-uniqueness-names'; + await db.stores.featureToggleStore.create('default', { + name: featureName, + variants: [ + { + name: 'variant1', + weight: 1000, + weightType: WeightType.VARIABLE, + stickiness: 'default', + }, + ], + }); + + const newVariants: IVariant[] = []; + + const observer = jsonpatch.observe(newVariants); + newVariants.push({ + name: 'variant1', + weight: 550, + weightType: WeightType.VARIABLE, + stickiness: 'default', + }); + newVariants.push({ + name: 'variant2', + weight: 550, + weightType: WeightType.VARIABLE, + stickiness: 'default', + }); + const patch = jsonpatch.generate(observer); + await app.request + .patch(`/api/admin/projects/default/features/${featureName}/variants`) + .send(patch) + .expect(400) + .expect((res) => { + expect(res.body.details[0].message).toMatch( + /contains a duplicate value/, + ); + }); +}); + +test('PUT endpoint validates uniqueness of variant names', async () => { + const featureName = 'variants-put-uniqueness-names'; + await db.stores.featureToggleStore.create('default', { + name: featureName, + variants: [], + }); + await app.request + .put(`/api/admin/projects/default/features/${featureName}/variants`) + .send([ + { + name: 'variant1', + weightType: 'variable', + weight: 500, + stickiness: 'default', + }, + { + name: 'variant1', + weightType: 'variable', + weight: 500, + stickiness: 'default', + }, + ]) + .expect(400) + .expect((res) => { + expect(res.body.details[0].message).toMatch( + /contains a duplicate value/, + ); + }); +});