From 5829ec7b3d4139934d97712a3f3c3dd43922bbfd Mon Sep 17 00:00:00 2001
From: Christopher Kolstad <chriswk@getunleash.ai>
Date: Fri, 26 Nov 2021 13:06:36 +0100
Subject: [PATCH] task: Sort variants by name (#1132)

---
 src/lib/db/feature-toggle-store.ts            | 17 +++-
 .../api/admin/project/variants.e2e.test.ts    | 99 +++++++++++++++++++
 2 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/src/lib/db/feature-toggle-store.ts b/src/lib/db/feature-toggle-store.ts
index 9958f6e9d0..bee2fcf52b 100644
--- a/src/lib/db/feature-toggle-store.ts
+++ b/src/lib/db/feature-toggle-store.ts
@@ -4,7 +4,7 @@ import metricsHelper from '../util/metrics-helper';
 import { DB_TIME } from '../metric-events';
 import NotFoundError from '../error/notfound-error';
 import { Logger, LogProvider } from '../logger';
-import { FeatureToggleDTO, FeatureToggle, IVariant } from '../types/model';
+import { FeatureToggle, FeatureToggleDTO, IVariant } from '../types/model';
 import { IFeatureToggleStore } from '../types/stores/feature-toggle-store';
 
 const FEATURE_COLUMNS = [
@@ -155,13 +155,15 @@ export default class FeatureToggleStore implements IFeatureToggleStore {
         if (!row) {
             throw new NotFoundError('No feature toggle found');
         }
+        const sortedVariants = (row.variants as unknown as IVariant[]) || [];
+        sortedVariants.sort((a, b) => a.name.localeCompare(b.name));
         return {
             name: row.name,
             description: row.description,
             type: row.type,
             project: row.project,
             stale: row.stale,
-            variants: (row.variants as unknown as IVariant[]) || [],
+            variants: sortedVariants,
             createdAt: row.created_at,
             lastSeenAt: row.last_seen_at,
         };
@@ -171,7 +173,10 @@ export default class FeatureToggleStore implements IFeatureToggleStore {
         if (!row) {
             throw new NotFoundError('No feature toggle found');
         }
-        return (row.variants as unknown as IVariant[]) || [];
+        const sortedVariants = (row.variants as unknown as IVariant[]) || [];
+        sortedVariants.sort((a, b) => a.name.localeCompare(b.name));
+
+        return sortedVariants;
     }
 
     dtoToRow(project: string, data: FeatureToggleDTO): FeaturesTable {
@@ -183,7 +188,11 @@ export default class FeatureToggleStore implements IFeatureToggleStore {
             archived: data.archived || false,
             stale: data.stale,
             variants: data.variants
-                ? JSON.stringify(data.variants)
+                ? JSON.stringify(
+                      data.variants.sort((a, b) =>
+                          a.name.localeCompare(b.name),
+                      ),
+                  )
                 : JSON.stringify([]),
             created_at: data.createdAt,
         };
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 cf25eceaf0..d19b98ad16 100644
--- a/src/test/e2e/api/admin/project/variants.e2e.test.ts
+++ b/src/test/e2e/api/admin/project/variants.e2e.test.ts
@@ -714,3 +714,102 @@ test('PUT endpoint validates uniqueness of variant names', async () => {
             );
         });
 });
+
+test('Variants should be sorted by their name when PUT', async () => {
+    const featureName = 'variants-sort-by-name';
+    await db.stores.featureToggleStore.create('default', {
+        name: featureName,
+    });
+
+    await app.request
+        .put(`/api/admin/projects/default/features/${featureName}/variants`)
+        .send([
+            {
+                name: 'zvariant',
+                weightType: 'variable',
+                weight: 500,
+                stickiness: 'default',
+            },
+            {
+                name: 'variant-a',
+                weightType: 'variable',
+                weight: 500,
+                stickiness: 'default',
+            },
+            {
+                name: 'g-variant',
+                weightType: 'variable',
+                weight: 500,
+                stickiness: 'default',
+            },
+            {
+                name: 'variant-g',
+                weightType: 'variable',
+                weight: 500,
+                stickiness: 'default',
+            },
+        ])
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.variants[0].name).toBe('g-variant');
+            expect(res.body.variants[1].name).toBe('variant-a');
+            expect(res.body.variants[2].name).toBe('variant-g');
+            expect(res.body.variants[3].name).toBe('zvariant');
+        });
+});
+
+test('Variants should be sorted by name when PATCHed as well', async () => {
+    const featureName = 'variants-patch-sort-by-name';
+    await db.stores.featureToggleStore.create('default', {
+        name: featureName,
+    });
+
+    const variants: IVariant[] = [];
+    const observer = jsonpatch.observe(variants);
+    variants.push({
+        name: 'g-variant',
+        weightType: 'variable',
+        weight: 500,
+        stickiness: 'default',
+    });
+    variants.push({
+        name: 'a-variant',
+        weightType: 'variable',
+        weight: 500,
+        stickiness: 'default',
+    });
+    const patch = jsonpatch.generate(observer);
+    await app.request
+        .patch(`/api/admin/projects/default/features/${featureName}/variants`)
+        .send(patch)
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.variants[0].name).toBe('a-variant');
+            expect(res.body.variants[1].name).toBe('g-variant');
+        });
+    variants.push({
+        name: '00-variant',
+        weightType: 'variable',
+        weight: 500,
+        stickiness: 'default',
+    });
+    variants.push({
+        name: 'z-variant',
+        weightType: 'variable',
+        weight: 500,
+        stickiness: 'default',
+    });
+    const secondPatch = jsonpatch.generate(observer);
+    expect(secondPatch).toHaveLength(2);
+    await app.request
+        .patch(`/api/admin/projects/default/features/${featureName}/variants`)
+        .send(secondPatch)
+        .expect(200)
+        .expect((res) => {
+            expect(res.body.variants).toHaveLength(4);
+            expect(res.body.variants[0].name).toBe('00-variant');
+            expect(res.body.variants[1].name).toBe('a-variant');
+            expect(res.body.variants[2].name).toBe('g-variant');
+            expect(res.body.variants[3].name).toBe('z-variant');
+        });
+});