From a676b1bc2010b8408762068d94ef5ab5b8b7dfaa Mon Sep 17 00:00:00 2001
From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
Date: Fri, 2 Aug 2024 11:11:58 +0200
Subject: [PATCH] fix: strategy parameters UI (#7713)

Old versions of Unleash allow for creating "Gradual Rollout" strategies
without `groupId` or `stickiness`. UI will now populate those fields,
not getting stuck when editing strategies without said fields.
---
 .../FeatureStrategyForm/FeatureStrategyForm.tsx    | 10 ++++------
 .../FeatureStrategyType/FeatureStrategyType.tsx    |  1 +
 .../FlexibleStrategy/FlexibleStrategy.test.tsx     |  2 +-
 .../FlexibleStrategy/FlexibleStrategy.tsx          | 14 ++++++++++----
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyForm.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyForm.tsx
index a8583bb127..213f02d9d1 100644
--- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyForm.tsx
+++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyForm/FeatureStrategyForm.tsx
@@ -345,8 +345,9 @@ export const FeatureStrategyForm = ({
         return constraintCount + segmentCount;
     };
 
-    const showVariants =
-        strategy.parameters && 'stickiness' in strategy.parameters;
+    const showVariants = Boolean(
+        strategy.parameters && 'stickiness' in strategy.parameters,
+    );
 
     return (
         <>
@@ -527,10 +528,7 @@ export const FeatureStrategyForm = ({
                     condition={tab === 2}
                     show={
                         <ConditionallyRender
-                            condition={
-                                strategy.parameters != null &&
-                                'stickiness' in strategy.parameters
-                            }
+                            condition={showVariants}
                             show={StrategyVariants}
                         />
                     }
diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyType/FeatureStrategyType.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyType/FeatureStrategyType.tsx
index fcaadc6a8b..89c18dfc18 100644
--- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyType/FeatureStrategyType.tsx
+++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyType/FeatureStrategyType.tsx
@@ -49,6 +49,7 @@ export const FeatureStrategyType = ({
                     parameters={strategy.parameters ?? {}}
                     updateParameter={updateParameter}
                     editable={hasAccess}
+                    errors={errors}
                 />
             );
         case 'userWithId':
diff --git a/frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.test.tsx b/frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.test.tsx
index 80b0a8a2af..89a4694fcc 100644
--- a/frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.test.tsx
+++ b/frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.test.tsx
@@ -29,7 +29,7 @@ test('manipulates the rollout slider', async () => {
         return (
             <Routes>
                 <Route
-                    path='/projects/:projectId/features/:featureName'
+                    path='/projects/:projectId/features/:featureId'
                     element={
                         <FlexibleStrategy
                             parameters={parameters}
diff --git a/frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.tsx b/frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.tsx
index 2829a12d8a..f3b4236740 100644
--- a/frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.tsx
+++ b/frontend/src/component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy.tsx
@@ -16,12 +16,14 @@ import Loader from '../../../common/Loader/Loader';
 import { useEffect, useMemo } from 'react';
 import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
 import { useLocation } from 'react-router';
+import type { IFormErrors } from 'hooks/useFormErrors';
 
 interface IFlexibleStrategyProps {
     parameters: IFeatureStrategyParameters;
     updateParameter: (field: string, value: string) => void;
     context: any;
     editable: boolean;
+    errors?: IFormErrors;
 }
 
 const StyledBox = styled(Box)(({ theme }) => ({
@@ -53,8 +55,10 @@ const FlexibleStrategy = ({
     updateParameter,
     parameters,
     editable = true,
+    errors,
 }: IFlexibleStrategyProps) => {
     const projectId = useRequiredPathParam('projectId');
+    const featureId = useRequiredPathParam('featureId');
     const { defaultStickiness, loading } = useDefaultProjectSettings(projectId);
     const { pathname } = useLocation();
 
@@ -77,7 +81,7 @@ const FlexibleStrategy = ({
             return defaultStickiness;
         }
 
-        return parseParameterString(parameters.stickiness);
+        return parseParameterString(parameters.stickiness || defaultStickiness);
     }, [loading, parameters.stickiness]);
 
     if (parameters.stickiness === '') {
@@ -85,10 +89,10 @@ const FlexibleStrategy = ({
     }
 
     useEffect(() => {
-        if (isDefaultStrategyEdit && !parameters.groupId) {
-            onUpdate('groupId')('');
+        if (!parameters.groupId) {
+            onUpdate('groupId')(isDefaultStrategyEdit ? '' : featureId);
         }
-    }, [isDefaultStrategyEdit]);
+    }, [isDefaultStrategyEdit, featureId]);
 
     if (loading) {
         return <Loader />;
@@ -121,6 +125,8 @@ const FlexibleStrategy = ({
                         disabled={!editable}
                         onChange={(e) => onUpdate('groupId')(e.target.value)}
                         data-testid={FLEXIBLE_STRATEGY_GROUP_ID}
+                        error={Boolean(errors?.getFormError('groupId'))}
+                        helperText={errors?.getFormError('groupId')}
                     />
                 </StyledInnerBox2>
             </StyledOuterBox>