From d2a4763eaa5268406c416e30dd716815357e5380 Mon Sep 17 00:00:00 2001
From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
Date: Wed, 26 Jul 2023 16:08:11 +0200
Subject: [PATCH] fix: frontend variant weights distribution (#4347)

## About the changes
Unit-tested way of distributing weights between variants.
---
 frontend/src/component/common/util.test.ts    | 117 ++++++++++++++++++
 frontend/src/component/common/util.ts         |  15 ++-
 .../StrategyTypes/StrategyVariants.test.tsx   |   2 +-
 frontend/src/interfaces/featureToggle.ts      |   2 +-
 4 files changed, 129 insertions(+), 7 deletions(-)
 create mode 100644 frontend/src/component/common/util.test.ts

diff --git a/frontend/src/component/common/util.test.ts b/frontend/src/component/common/util.test.ts
new file mode 100644
index 0000000000..aee2de4e11
--- /dev/null
+++ b/frontend/src/component/common/util.test.ts
@@ -0,0 +1,117 @@
+import { updateWeightEdit } from './util';
+
+const variantTemplate = {
+    id: '0',
+    name: 'A',
+    weight: 0,
+    weightType: 'variable' as const,
+    stickiness: 'default',
+    isValid: true,
+    new: false,
+};
+
+describe('updateWeightEdit', () => {
+    it('can assign weight to one only variant', () => {
+        const variants = [variantTemplate];
+        expect(updateWeightEdit(variants, 100)).toMatchInlineSnapshot(`
+          [
+            {
+              "id": "0",
+              "isValid": true,
+              "name": "A",
+              "new": false,
+              "stickiness": "default",
+              "weight": 100,
+              "weightType": "variable",
+            },
+          ]
+        `);
+    });
+
+    it('can distribute weight between 2 variants evenly', () => {
+        const variants = [
+            variantTemplate,
+            { ...variantTemplate, id: '2', name: 'B' },
+        ];
+        updateWeightEdit(variants, 100).forEach(variant => {
+            expect(variant).toHaveProperty('weight', 50);
+        });
+    });
+
+    it('can distribute weight between 8 variants evenly', () => {
+        const variants = Array.from({ length: 8 }, (_, i) => ({
+            ...variantTemplate,
+            id: `${i}`,
+            name: `${i}`,
+            weight: i,
+        }));
+        updateWeightEdit(variants, 1000).forEach(variant => {
+            expect(variant).toHaveProperty('weight', 125);
+        });
+    });
+
+    it('can distribute weight between 8 variants evenly and assign the remainder to the last variant', () => {
+        const variants = Array.from({ length: 8 }, (_, i) => ({
+            ...variantTemplate,
+            id: `${i}`,
+            name: `${i}`,
+            weight: i,
+        }));
+        const weights = updateWeightEdit(variants, 100).map(
+            variant => variant.weight
+        );
+        expect(weights).toEqual([13, 12, 13, 12, 13, 12, 13, 12]);
+    });
+
+    it('can adjust variable weight to get correct sum', () => {
+        const variants = [
+            { ...variantTemplate, weightType: 'fix' as const, weight: 333 },
+            { ...variantTemplate, id: '2', name: 'B' },
+        ];
+        const weights = updateWeightEdit(variants, 1000).map(
+            variant => variant.weight
+        );
+        expect(weights).toEqual([333, 667]);
+    });
+
+    it('can deal with complex example', () => {
+        const variants = [
+            { ...variantTemplate, weightType: 'fix' as const, weight: 333 },
+            { ...variantTemplate, id: '2', name: 'B' },
+            { ...variantTemplate, id: '3', name: 'C' },
+            { ...variantTemplate, id: '4', name: 'D' },
+            { ...variantTemplate, id: '5', name: 'E' },
+            {
+                ...variantTemplate,
+                id: '6',
+                name: 'F',
+                weightType: 'fix' as const,
+                weight: 111,
+            },
+            { ...variantTemplate, id: '7', name: 'G' },
+            { ...variantTemplate, id: '8', name: 'H' },
+        ];
+        const weights = updateWeightEdit(variants, 1000).map(
+            variant => variant.weight
+        );
+        expect(weights).toEqual([333, 93, 93, 93, 92, 111, 93, 92]);
+    });
+
+    it('can deal with 0-weight variable variant', () => {
+        const variants = [
+            { ...variantTemplate, weightType: 'fix' as const, weight: 500 },
+            {
+                ...variantTemplate,
+                weightType: 'fix' as const,
+                weight: 500,
+                id: '2',
+                name: 'B',
+            },
+            { ...variantTemplate, id: '3', name: 'C' },
+        ];
+        const weights = updateWeightEdit(variants, 1000).map(
+            variant => variant.weight
+        );
+        expect(weights).toEqual([500, 500, 0]);
+    });
+});
diff --git a/frontend/src/component/common/util.ts b/frontend/src/component/common/util.ts
index ce6348664a..97178f6371 100644
--- a/frontend/src/component/common/util.ts
+++ b/frontend/src/component/common/util.ts
@@ -98,11 +98,12 @@ export function updateWeightEdit(
     if (variants.length === 0) {
         return [];
     }
-    const { remainingPercentage, variableVariantCount } = variants.reduce(
+    let { remainingPercentage, variableVariantCount } = variants.reduce(
         ({ remainingPercentage, variableVariantCount }, variant) => {
             if (variant.weight && variant.weightType === weightTypes.FIX) {
                 remainingPercentage -= Number(variant.weight);
-            } else {
+            }
+            if (variant.weightType === weightTypes.VARIABLE) {
                 variableVariantCount += 1;
             }
             return {
@@ -113,14 +114,18 @@ export function updateWeightEdit(
         { remainingPercentage: totalWeight, variableVariantCount: 0 }
     );
 
-    const percentage = parseInt(
-        String(remainingPercentage / variableVariantCount)
-    );
+    const getPercentage = () =>
+        Math.round(remainingPercentage / variableVariantCount);
 
     return variants.map(variant => {
         if (variant.weightType !== weightTypes.FIX) {
+            const percentage = getPercentage(); // round "as we go" - clean best effort approach
+            remainingPercentage -= percentage;
+            variableVariantCount -= 1;
+
             variant.weight = percentage;
         }
+
         return variant;
     });
 }
diff --git a/frontend/src/component/feature/StrategyTypes/StrategyVariants.test.tsx b/frontend/src/component/feature/StrategyTypes/StrategyVariants.test.tsx
index 57b2961226..e91be57d68 100644
--- a/frontend/src/component/feature/StrategyTypes/StrategyVariants.test.tsx
+++ b/frontend/src/component/feature/StrategyTypes/StrategyVariants.test.tsx
@@ -17,7 +17,7 @@ test('should render variants', async () => {
                 name: 'variantName',
                 stickiness: 'default',
                 weight: 1000,
-                weightType: 'variable',
+                weightType: 'variable' as const,
                 payload: {
                     type: 'string',
                     value: 'variantValue',
diff --git a/frontend/src/interfaces/featureToggle.ts b/frontend/src/interfaces/featureToggle.ts
index adfa3b8580..c654d80273 100644
--- a/frontend/src/interfaces/featureToggle.ts
+++ b/frontend/src/interfaces/featureToggle.ts
@@ -52,7 +52,7 @@ export interface IFeatureVariant {
     name: string;
     stickiness: string;
     weight: number;
-    weightType: string;
+    weightType: 'fix' | 'variable';
     overrides?: IOverride[];
     payload?: IPayload;
 }