fix: add symbols as constraint ids (#5913)

This PR adds uuids as ids using a symbol in order to make sure we only
use this to keep internal order in the viritual DOM. This makes us able
to have predictable mutable lists on the frontend, and makes it easy to
not pass this property along to the backend.

frontend/src/component/changeRequest/ChangeRequest/Changes/Change/NewEditChange.tsx

@@ -16,6 +16,8 @@ import { useChangeRequestsEnabled } from 'hooks/useChangeRequestsEnabled';
 import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
 import { comparisonModerator } from 'component/feature/FeatureStrategy/featureStrategy.utils';
 import {
+    ChangeRequestAddStrategy,
+    ChangeRequestEditStrategy,
     IChangeRequestAddStrategy,
     IChangeRequestUpdateStrategy,
 } from 'component/changeRequest/changeRequest.types';
@@ -25,6 +27,8 @@ import { useUiFlag } from 'hooks/useUiFlag';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { NewFeatureStrategyForm } from 'component/feature/FeatureStrategy/FeatureStrategyForm/NewFeatureStrategyForm';
 import { NewStrategyVariants } from 'component/feature/StrategyTypes/NewStrategyVariants';
+import { constraintId } from 'component/common/ConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
+import { v4 as uuidv4 } from 'uuid';
 
 interface IEditChangeProps {
     change: IChangeRequestAddStrategy | IChangeRequestUpdateStrategy;
@@ -36,6 +40,16 @@ interface IEditChangeProps {
     onClose: () => void;
 }
 
+const addIdSymbolToConstraints = (
+    strategy?: ChangeRequestAddStrategy | ChangeRequestEditStrategy,
+) => {
+    if (!strategy) return;
+
+    return strategy?.constraints.map((constraint) => {
+        return { ...constraint, [constraintId]: uuidv4() };
+    });
+};
+
 export const NewEditChange = ({
     change,
     changeRequestId,
@@ -50,9 +64,12 @@ export const NewEditChange = ({
     const [tab, setTab] = useState(0);
     const newStrategyConfiguration = useUiFlag('newStrategyConfiguration');
 
-    const [strategy, setStrategy] = useState<Partial<IFeatureStrategy>>(
-        change.payload,
-    );
+    const constraintsWithId = addIdSymbolToConstraints(change.payload);
+
+    const [strategy, setStrategy] = useState<Partial<IFeatureStrategy>>({
+        ...change.payload,
+        constraints: constraintsWithId,
+    });
 
     const { segments: allSegments } = useSegments();
     const strategySegments = (allSegments || []).filter((segment) => {

frontend/src/component/changeRequest/changeRequest.types.ts

@@ -216,7 +216,7 @@ type ChangeRequestEnabled = { enabled: boolean };
 
 type ChangeRequestAddDependency = { feature: string };
 
-type ChangeRequestAddStrategy = Pick<
+export type ChangeRequestAddStrategy = Pick<
     IFeatureStrategy,
     | 'parameters'
     | 'constraints'
@@ -226,7 +226,9 @@ type ChangeRequestAddStrategy = Pick<
     | 'variants'
 > & { name: string };
 
-type ChangeRequestEditStrategy = ChangeRequestAddStrategy & { id: string };
+export type ChangeRequestEditStrategy = ChangeRequestAddStrategy & {
+    id: string;
+};
 
 type ChangeRequestDeleteStrategy = {
     id: string;

frontend/src/component/common/ConstraintAccordion/ConstraintAccordionList/createEmptyConstraint.ts

@@ -2,6 +2,9 @@ import { dateOperators } from 'constants/operators';
 import { IConstraint } from 'interfaces/strategy';
 import { oneOf } from 'utils/oneOf';
 import { operatorsForContext } from 'utils/operatorsForContext';
+import { v4 as uuidv4 } from 'uuid';
+
+export const constraintId = Symbol('id');
 
 export const createEmptyConstraint = (contextName: string): IConstraint => {
     const operator = operatorsForContext(contextName)[0];
@@ -17,5 +20,6 @@ export const createEmptyConstraint = (contextName: string): IConstraint => {
         values: [],
         caseInsensitive: false,
         inverted: false,
+        [constraintId]: uuidv4(),
     };
 };

frontend/src/component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList.tsx

@@ -1,16 +1,14 @@
-import React, {
-    forwardRef,
-    Fragment,
-    RefObject,
-    useImperativeHandle,
-} from 'react';
-import { Button, styled, Tooltip } from '@mui/material';
+import React, { forwardRef, Fragment, useImperativeHandle } from 'react';
+import { styled, Tooltip } from '@mui/material';
 import { HelpOutline } from '@mui/icons-material';
 import { IConstraint } from 'interfaces/strategy';
 import produce from 'immer';
 import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
 import { IUseWeakMap, useWeakMap } from 'hooks/useWeakMap';
-import { createEmptyConstraint } from 'component/common/ConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
+import {
+    constraintId,
+    createEmptyConstraint,
+} from 'component/common/ConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
 import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator';
 import { NewConstraintAccordion } from 'component/common/NewConstraintAccordion/NewConstraintAccordion';
@@ -162,26 +160,30 @@ export const NewConstraintAccordionList = forwardRef<
 
     return (
         <StyledContainer id={constraintAccordionListId}>
-            {constraints.map((constraint, index) => (
+            {constraints.map((constraint, index) => {
                 // biome-ignore lint: reason=objectId would change every time values change - this is no different than using index
-                <Fragment key={index}>
-                    <ConditionallyRender
-                        condition={index > 0}
-                        show={<StrategySeparator text='AND' />}
-                    />
-
-                    <NewConstraintAccordion
-                        constraint={constraint}
-                        onEdit={onEdit?.bind(null, constraint)}
-                        onCancel={onCancel.bind(null, index)}
-                        onDelete={onRemove?.bind(null, index)}
-                        onSave={onSave?.bind(null, index)}
-                        onAutoSave={onAutoSave?.bind(null, index)}
-                        editing={Boolean(state.get(constraint)?.editing)}
-                        compact
-                    />
-                </Fragment>
-            ))}
+                const id = constraint[constraintId];
+
+                return (
+                    <Fragment key={id}>
+                        <ConditionallyRender
+                            condition={index > 0}
+                            show={<StrategySeparator text='AND' />}
+                        />
+
+                        <NewConstraintAccordion
+                            constraint={constraint}
+                            onEdit={onEdit?.bind(null, constraint)}
+                            onCancel={onCancel.bind(null, index)}
+                            onDelete={onRemove?.bind(null, index)}
+                            onSave={onSave?.bind(null, index)}
+                            onAutoSave={onAutoSave?.bind(null, index)}
+                            editing={Boolean(state.get(constraint)?.editing)}
+                            compact
+                        />
+                    </Fragment>
+                );
+            })}
         </StyledContainer>
     );
 });

frontend/src/component/feature/FeatureStrategy/NewFeatureStrategyEdit/NewFeatureStrategyEdit.tsx

@@ -28,6 +28,8 @@ import { usePendingChangeRequests } from 'hooks/api/getters/usePendingChangeRequ
 import { usePlausibleTracker } from 'hooks/usePlausibleTracker';
 import { NewFeatureStrategyForm } from 'component/feature/FeatureStrategy/FeatureStrategyForm/NewFeatureStrategyForm';
 import { NewStrategyVariants } from 'component/feature/StrategyTypes/NewStrategyVariants';
+import { constraintId } from 'component/common/ConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
+import { v4 as uuidv4 } from 'uuid';
 
 const useTitleTracking = () => {
     const [previousTitle, setPreviousTitle] = useState<string>('');
@@ -75,6 +77,14 @@ const useTitleTracking = () => {
     };
 };
 
+const addIdSymbolToConstraints = (strategy?: IFeatureStrategy) => {
+    if (!strategy) return;
+
+    return strategy?.constraints.map((constraint) => {
+        return { ...constraint, [constraintId]: uuidv4() };
+    });
+};
+
 export const NewFeatureStrategyEdit = () => {
     const projectId = useRequiredPathParam('projectId');
     const featureId = useRequiredPathParam('featureId');
@@ -133,7 +143,15 @@ export const NewFeatureStrategyEdit = () => {
         const savedStrategy = data?.environments
             .flatMap((environment) => environment.strategies)
             .find((strategy) => strategy.id === strategyId);
-        setStrategy((prev) => ({ ...prev, ...savedStrategy }));
+
+        const constraintsWithId = addIdSymbolToConstraints(savedStrategy);
+
+        const formattedStrategy = {
+            ...savedStrategy,
+            constraints: constraintsWithId,
+        };
+
+        setStrategy((prev) => ({ ...prev, ...formattedStrategy }));
         setPreviousTitle(savedStrategy?.title || '');
     }, [strategyId, data]);
 

frontend/src/interfaces/strategy.ts

@@ -1,5 +1,6 @@
 import { Operator } from 'constants/operators';
 import { IFeatureVariant } from './featureToggle';
+import { constraintId } from 'component/common/ConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
 
 export interface IFeatureStrategy {
     id: string;
@@ -61,6 +62,7 @@ export interface IConstraint {
     caseInsensitive?: boolean;
     operator: Operator;
     contextName: string;
+    [constraintId]?: string;
 }
 
 export interface IFeatureStrategySortOrder {