fix: make new variant api validate name uniqueness (#1126)

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;
         }

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) {

src/lib/routes/admin-api/project/variants.ts

@@ -43,7 +43,7 @@ export default class VariantsController extends Controller {
     ): Promise<void> {
         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(

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()

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/,
+            );
+        });
+});