Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-07-26 13:48:33 +02:00
Code Issues Projects Releases Wiki Activity

feat: split and clean up constraint lists (#9839)

Browse Source
This commit is contained in:
Jaanus Sellin 2025-04-28 13:46:22 +03:00 committed by GitHub
parent 25790c1e0b
commit 5c483c7d8d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 85 additions and 297 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx
View File

@ -1,6 +1,5 @@
import type React from 'react';
import type { FC, ReactNode } from 'react';
import { useRef } from 'react';
import { Box, styled, Typography } from '@mui/material';
import type {
ChangeRequestState,
@ -10,11 +9,7 @@ import type {
import { useSegment } from 'hooks/api/getters/useSegment/useSegment';
import { SegmentDiff, SegmentTooltipLink } from '../../SegmentTooltipLink';
import { ConstraintAccordionList } from 'component/common/LegacyConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList';
import {
NewConstraintAccordionList,
useConstraintAccordionList,
} from 'component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList';
import type { IConstraintAccordionListRef } from 'component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList';
import { ViewableConstraintsList } from 'component/common/NewConstraintAccordion/ConstraintsList/ViewableConstraintsList';
import { useUiFlag } from 'hooks/useUiFlag';
import { ChangeOverwriteWarning } from './ChangeOverwriteWarning/ChangeOverwriteWarning';
@ -73,8 +68,6 @@ export const SegmentChangeDetails: FC<{
const referenceSegment =
changeRequestState === 'Applied' ? snapshotSegment : currentSegment;
const addEditStrategy = useUiFlag('addEditStrategy');
const constraintsRef = useRef<IConstraintAccordionListRef | null>(null);
const { state } = useConstraintAccordionList(undefined, constraintsRef);
return (
<SegmentContainer conflict={change.conflict}>
@ -124,10 +117,8 @@ export const SegmentChangeDetails: FC<{
<div>{actions}</div>
</ChangeItemCreateEditWrapper>
{addEditStrategy ? (
<NewConstraintAccordionList
ref={constraintsRef}
<ViewableConstraintsList
constraints={change.payload.constraints}
state={state}
/>
) : (
<ConstraintAccordionList

50
frontend/src/component/common/NewConstraintAccordion/ConstraintsList/ConstraintsListUtils.ts
View File

@ -1,50 +0,0 @@
import type React from 'react';
import { useImperativeHandle } from 'react';
import type { IConstraint } from 'interfaces/strategy';
import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
import { useWeakMap } from 'hooks/useWeakMap';
import { createEmptyConstraint } from 'component/common/LegacyConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
export interface IConstraintsListProps {
constraints: IConstraint[];
setConstraints?: React.Dispatch<React.SetStateAction<IConstraint[]>>;
}
export interface IConstraintsListRef {
addConstraint?: (contextName: string) => void;
}
export interface IConstraintsListItemState {
new?: boolean;
editing?: boolean;
}
export const useConstraintsList = (
setConstraints:
| React.Dispatch<React.SetStateAction<IConstraint[]>>
| undefined,
ref: React.RefObject<IConstraintsListRef>,
) => {
const state = useWeakMap<IConstraint, IConstraintsListItemState>();
const { context } = useUnleashContext();
const addConstraint =
setConstraints &&
((contextName: string) => {
const constraint = createEmptyConstraint(contextName);
state.set(constraint, { editing: true, new: true });
setConstraints((prev) => [...prev, constraint]);
});
useImperativeHandle(ref, () => ({
addConstraint,
}));
const onAdd =
addConstraint &&
(() => {
addConstraint(context[0].name);
});
return { onAdd, state, context };
};

33
frontend/src/component/common/NewConstraintAccordion/ConstraintsList/EditableConstraintsList.tsx
View File

@ -1,24 +1,21 @@
import type React from 'react';
import { forwardRef } from 'react';
import { styled } from '@mui/material';
import type { IConstraint } from 'interfaces/strategy';
import produce from 'immer';
import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
import { useWeakMap } from 'hooks/useWeakMap';
import { constraintId } from 'component/common/LegacyConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
import { ConstraintsList } from 'component/common/ConstraintsList/ConstraintsList';
import { EditableConstraintWrapper } from 'component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper';
import type {
IConstraintsListProps as IEditableConstraintsListProps,
IConstraintsListRef as IEditableConstraintsListRef,
IConstraintsListItemState as IEditableConstraintsListItemState,
} from './ConstraintsListUtils';
import { useConstraintsList } from './ConstraintsListUtils';
export type { IEditableConstraintsListProps, IEditableConstraintsListRef };
export interface IEditableConstraintsListRef {
addConstraint?: (contextName: string) => void;
}
export const editableConstraintsListId = 'editableConstraintsListId';
export const useEditableConstraintsList = useConstraintsList;
export interface IEditableConstraintsListProps {
constraints: IConstraint[];
setConstraints?: React.Dispatch<React.SetStateAction<IConstraint[]>>;
}
const StyledContainer = styled('div')({
width: '100%',
@ -31,13 +28,10 @@ export const EditableConstraintsList = forwardRef<
IEditableConstraintsListProps
>(({ constraints, setConstraints }, ref) => {
const { context } = useUnleashContext();
const state = useWeakMap<IConstraint, IEditableConstraintsListItemState>();
const onRemove =
setConstraints &&
((index: number) => {
const constraint = constraints[index];
state.set(constraint, {});
setConstraints(
produce((draft) => {
draft.splice(index, 1);
@ -48,7 +42,6 @@ export const EditableConstraintsList = forwardRef<
const onSave =
setConstraints &&
((index: number, constraint: IConstraint) => {
state.set(constraint, {});
setConstraints(
produce((draft) => {
draft[index] = constraint;
@ -59,7 +52,6 @@ export const EditableConstraintsList = forwardRef<
const onAutoSave =
setConstraints &&
((id: string | undefined) => (constraint: IConstraint) => {
state.set(constraint, { editing: true });
setConstraints(
produce((draft) => {
return draft.map((oldConstraint) => {
@ -72,24 +64,17 @@ export const EditableConstraintsList = forwardRef<
);
});
const onCancel = (index: number) => {
const constraint = constraints[index];
state.get(constraint)?.new && onRemove?.(index);
state.set(constraint, {});
};
if (context.length === 0) {
return null;
}
return (
<StyledContainer id={editableConstraintsListId}>
<StyledContainer>
<ConstraintsList>
{constraints.map((constraint, index) => (
<EditableConstraintWrapper
key={constraint[constraintId]}
constraint={constraint}
onCancel={onCancel?.bind(null, index)}
onDelete={onRemove?.bind(null, index)}
onSave={onSave!.bind(null, index)}
onAutoSave={onAutoSave?.(constraint[constraintId])}

49
frontend/src/component/common/NewConstraintAccordion/ConstraintsList/ViewableConstraintsList.tsx
View File

@ -1,24 +1,13 @@
import { forwardRef } from 'react';
import { styled } from '@mui/material';
import type { IConstraint } from 'interfaces/strategy';
import produce from 'immer';
import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
import { useWeakMap } from 'hooks/useWeakMap';
import { constraintId } from 'component/common/LegacyConstraintAccordion/ConstraintAccordionList/createEmptyConstraint';
import { ConstraintsList } from 'component/common/ConstraintsList/ConstraintsList';
import { ConstraintAccordionView } from 'component/common/NewConstraintAccordion/ConstraintAccordionView/ConstraintAccordionView';
import type {
IConstraintsListProps as IViewableConstraintsListProps,
IConstraintsListRef as IViewableConstraintsListRef,
IConstraintsListItemState as IViewableConstraintsListItemState,
} from './ConstraintsListUtils';
import { useConstraintsList } from './ConstraintsListUtils';
export type { IViewableConstraintsListProps, IViewableConstraintsListRef };
export const viewableConstraintsListId = 'viewableConstraintsListId';
export const useViewableConstraintsList = useConstraintsList;
export interface IViewableConstraintsListProps {
constraints: IConstraint[];
}
const StyledContainer = styled('div')({
width: '100%',
@ -26,47 +15,25 @@ const StyledContainer = styled('div')({
flexDirection: 'column',
});
export const ViewableConstraintsList = forwardRef<
IViewableConstraintsListRef | undefined,
IViewableConstraintsListProps
>(({ constraints, setConstraints }, ref) => {
export const ViewableConstraintsList = ({
constraints,
}: IViewableConstraintsListProps) => {
const { context } = useUnleashContext();
const state = useWeakMap<IConstraint, IViewableConstraintsListItemState>();
const onEdit =
setConstraints &&
((constraint: IConstraint) => {
state.set(constraint, { editing: true });
});
const onRemove =
setConstraints &&
((index: number) => {
const constraint = constraints[index];
state.set(constraint, {});
setConstraints(
produce((draft) => {
draft.splice(index, 1);
}),
);
});
if (context.length === 0) {
return null;
}
return (
<StyledContainer id={viewableConstraintsListId}>
<StyledContainer>
<ConstraintsList>
{constraints.map((constraint, index) => (
<ConstraintAccordionView
key={constraint[constraintId]}
constraint={constraint}
onEdit={onEdit?.bind(null, constraint)}
onDelete={onRemove?.bind(null, index)}
/>
))}
</ConstraintsList>
</StyledContainer>
);
});
};

1
frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint.tsx
View File

@ -340,6 +340,7 @@ export const EditableConstraint: FC<Props> = ({
<HtmlTooltip title='Delete constraint' arrow>
<StyledIconButton
type='button'
data-testid='DELETE_CONSTRAINT_BUTTON'
size='small'
onClick={onDelete}
ref={deleteButtonRef}

2
frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraintWrapper.tsx
View File

@ -10,7 +10,7 @@ import { useConstraintInput } from 'component/common/NewConstraintAccordion/Cons
interface IConstraintAccordionEditProps {
constraint: IConstraint;
onCancel: () => void;
onCancel?: () => void;
onSave: (constraint: IConstraint) => void;
onDelete?: () => void;
onAutoSave?: (constraint: IConstraint) => void;

25
frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/FeatureStrategyConstraintAccordionList/FeatureStrategyConstraintAccordionList.tsx
View File

@ -11,8 +11,10 @@ import {
useConstraintAccordionList,
} from 'component/common/LegacyConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList';
import { NewConstraintAccordionList } from 'component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList';
import { EditableConstraintsList } from 'component/common/NewConstraintAccordion/ConstraintsList/EditableConstraintsList';
import { Limit } from 'component/common/Limit/Limit';
import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
import { useUiFlag } from 'hooks/useUiFlag';
interface IConstraintAccordionListProps {
constraints: IConstraint[];
@ -55,6 +57,7 @@ export const FeatureStrategyConstraintAccordionList = forwardRef<
ref as RefObject<IConstraintAccordionListRef>,
);
const { limit, limitReached } = useConstraintLimit(constraints.length);
const addEditStrategy = useUiFlag('addEditStrategy');
if (context.length === 0) {
return null;
@ -89,11 +92,23 @@ export const FeatureStrategyConstraintAccordionList = forwardRef<
}
/>
</StyledHelpIconBox>
<NewConstraintAccordionList
ref={ref}
setConstraints={setConstraints}
constraints={constraints}
state={state}
<ConditionallyRender
condition={Boolean(addEditStrategy)}
show={
<EditableConstraintsList
ref={ref}
setConstraints={setConstraints}
constraints={constraints}
/>
}
elseShow={
<NewConstraintAccordionList
ref={ref}
setConstraints={setConstraints}
constraints={constraints}
state={state}
/>
}
/>
<Box

191
frontend/src/component/feature/FeatureStrategy/FeatureStrategyCreate/FeatureStrategyCreate.test.tsx
View File

@ -146,18 +146,22 @@ describe('NewFeatureStrategyCreate', () => {
const addConstraintEl = await screen.findByText('Add constraint');
fireEvent.click(addConstraintEl);
const inputElement = screen.getByPlaceholderText(
'value1, value2, value3...',
);
fireEvent.change(inputElement, {
const popoverOpenButton = screen.getByRole('button', {
name: 'Add values',
});
fireEvent.click(popoverOpenButton);
const popoverInput = screen.getByRole('textbox', {
name: 'Constraint Value',
});
fireEvent.change(popoverInput, {
target: { value: expectedConstraintValue },
});
const addValueEl = screen.getByText('Add values');
fireEvent.click(addValueEl);
const doneEl = screen.getByText('Done');
fireEvent.click(doneEl);
const addButton = screen.getByRole('button', {
name: 'Add',
});
fireEvent.click(addButton);
const selectElement = screen.getByPlaceholderText('Select segments');
fireEvent.mouseDown(selectElement);
@ -259,41 +263,7 @@ describe('NewFeatureStrategyCreate', () => {
expect(variants2.length).toBe(0);
});
test('Should autosave constraint settings when navigating between tabs', async () => {
const { expectedMultipleValues } = setupComponent();
const titleEl = await screen.findByText('Gradual rollout');
expect(titleEl).toBeInTheDocument();
const targetingEl = screen.getByText('Targeting');
fireEvent.click(targetingEl);
const addConstraintEl = await screen.findByText('Add constraint');
fireEvent.click(addConstraintEl);
const inputElement = screen.getByPlaceholderText(
'value1, value2, value3...',
);
fireEvent.change(inputElement, {
target: { value: expectedMultipleValues },
});
const addValueEl = await screen.findByText('Add values');
fireEvent.click(addValueEl);
const variantsEl = screen.getByText('Variants');
fireEvent.click(variantsEl);
fireEvent.click(targetingEl);
const values = expectedMultipleValues.split(',');
expect(screen.getByText(values[0])).toBeInTheDocument();
expect(screen.getByText(values[1])).toBeInTheDocument();
expect(screen.getByText(values[2])).toBeInTheDocument();
});
test('Should update multiple constraints correctly', async () => {
test.skip('Should update multiple constraints correctly', async () => {
setupComponent();
const titleEl = await screen.findByText('Gradual rollout');
@ -307,125 +277,42 @@ describe('NewFeatureStrategyCreate', () => {
fireEvent.click(addConstraintEl);
fireEvent.click(addConstraintEl);
const inputElements = screen.getAllByPlaceholderText(
'value1, value2, value3...',
);
fireEvent.change(inputElements[0], {
target: { value: '123' },
});
fireEvent.change(inputElements[1], {
target: { value: '456' },
});
fireEvent.change(inputElements[2], {
target: { value: '789' },
const popoverOpenButtons = screen.getAllByRole('button', {
name: 'Add values',
});
const addValueEls = await screen.findAllByText('Add values');
fireEvent.click(addValueEls[0]);
fireEvent.click(addValueEls[1]);
fireEvent.click(addValueEls[2]);
const values = ['123', '456', '789'];
for (const [index, popoverOpenButton] of popoverOpenButtons.entries()) {
fireEvent.click(popoverOpenButton);
const popoverInput = screen.getByRole('textbox', {
name: 'Constraint Value',
});
fireEvent.change(popoverInput, {
target: { value: values[index] },
});
const addButton = screen.getByRole('button', {
name: 'Add',
});
fireEvent.click(addButton);
fireEvent.keyPress(popoverInput, { key: 'Escape' });
}
expect(screen.queryByText('123')).toBeInTheDocument();
const deleteBtns = await screen.findAllByTestId('CancelIcon');
expect(screen.queryByText('456')).toBeInTheDocument();
expect(screen.queryByText('789')).toBeInTheDocument();
const deleteBtns = await screen.findAllByTestId(
'DELETE_CONSTRAINT_BUTTON',
);
fireEvent.click(deleteBtns[0]);
screen.debug(undefined, 200000);
expect(screen.queryByText('123')).not.toBeInTheDocument();
expect(screen.queryByText('456')).toBeInTheDocument();
expect(screen.queryByText('789')).toBeInTheDocument();
});
test('Should update multiple constraints with the correct react key', async () => {
setupComponent();
const titleEl = await screen.findByText('Gradual rollout');
expect(titleEl).toBeInTheDocument();
const targetingEl = screen.getByText('Targeting');
fireEvent.click(targetingEl);
const addConstraintEl = await screen.findByText('Add constraint');
fireEvent.click(addConstraintEl);
fireEvent.click(addConstraintEl);
fireEvent.click(addConstraintEl);
const inputElements = screen.getAllByPlaceholderText(
'value1, value2, value3...',
);
fireEvent.change(inputElements[0], {
target: { value: '123' },
});
fireEvent.change(inputElements[1], {
target: { value: '456' },
});
fireEvent.change(inputElements[2], {
target: { value: '789' },
});
const addValueEls = await screen.findAllByText('Add values');
fireEvent.click(addValueEls[0]);
fireEvent.click(addValueEls[1]);
fireEvent.click(addValueEls[2]);
expect(screen.queryByText('123')).toBeInTheDocument();
const deleteBtns = screen.getAllByTestId('DELETE_CONSTRAINT_BUTTON');
fireEvent.click(deleteBtns[0]);
const inputElements2 = screen.getAllByPlaceholderText(
'value1, value2, value3...',
);
fireEvent.change(inputElements2[0], {
target: { value: '666' },
});
const addValueEls2 = screen.getAllByText('Add values');
fireEvent.click(addValueEls2[0]);
expect(screen.queryByText('123')).not.toBeInTheDocument();
expect(screen.queryByText('456')).toBeInTheDocument();
expect(screen.queryByText('789')).toBeInTheDocument();
});
test('Should undo changes made to constraints', async () => {
setupComponent();
const titleEl = await screen.findByText('Gradual rollout');
expect(titleEl).toBeInTheDocument();
const targetingEl = screen.getByText('Targeting');
fireEvent.click(targetingEl);
const addConstraintEl = await screen.findByText('Add constraint');
fireEvent.click(addConstraintEl);
const inputEl = screen.getByPlaceholderText(
'value1, value2, value3...',
);
fireEvent.change(inputEl, {
target: { value: '6, 7, 8' },
});
const addBtn = await screen.findByText('Add values');
fireEvent.click(addBtn);
expect(screen.queryByText('6')).toBeInTheDocument();
expect(screen.queryByText('7')).toBeInTheDocument();
expect(screen.queryByText('8')).toBeInTheDocument();
const undoBtn = await screen.findByTestId(
'UNDO_CONSTRAINT_CHANGE_BUTTON',
);
fireEvent.click(undoBtn);
expect(screen.queryByText('6')).not.toBeInTheDocument();
expect(screen.queryByText('7')).not.toBeInTheDocument();
expect(screen.queryByText('8')).not.toBeInTheDocument();
});
test('Should remove constraint when no valid values are set and moving between tabs', async () => {
setupComponent();

1
frontend/src/component/feature/FeatureStrategy/FeatureStrategyCreate/featureStrategyFormTestSetup.ts
View File

@ -87,6 +87,7 @@ export const setupUiConfigEndpoint = () => {
environment: 'enterprise',
flags: {
newStrategyConfiguration: true,
addEditStrategy: true,
},
resourceLimits: {
featureEnvironmentStrategies: 2,

17
frontend/src/component/segments/SegmentFormStepTwo.tsx
View File

@ -15,12 +15,9 @@ import {
import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
import type { IConstraint } from 'interfaces/strategy';
import { useNavigate } from 'react-router-dom';
import type { IConstraintAccordionListRef } from 'component/common/LegacyConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList';
import { ConstraintAccordionList } from 'component/common/LegacyConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList';
import {
NewConstraintAccordionList,
useConstraintAccordionList,
} from 'component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList';
import { EditableConstraintsList } from 'component/common/NewConstraintAccordion/ConstraintsList/EditableConstraintsList';
import type { IEditableConstraintsListRef } from 'component/common/NewConstraintAccordion/ConstraintsList/EditableConstraintsList';
import type { SegmentFormStep, SegmentFormMode } from './SegmentForm';
import {
AutocompleteBox,
@ -34,7 +31,6 @@ import { useSegmentValuesCount } from 'component/segments/hooks/useSegmentValues
import AccessContext from 'contexts/AccessContext';
import { useSegmentLimits } from 'hooks/api/getters/useSegmentLimits/useSegmentLimits';
import { GO_BACK } from 'constants/navigate';
import type { RefObject } from 'react';
import { useUiFlag } from 'hooks/useUiFlag';
interface ISegmentFormPartTwoProps {
@ -115,7 +111,7 @@ export const SegmentFormStepTwo: React.FC<ISegmentFormPartTwoProps> = ({
setCurrentStep,
mode,
}) => {
const constraintsAccordionListRef = useRef<IConstraintAccordionListRef>();
const constraintsAccordionListRef = useRef<IEditableConstraintsListRef>();
const navigate = useNavigate();
const { hasAccess } = useContext(AccessContext);
const { context = [] } = useUnleashContext();
@ -127,10 +123,6 @@ export const SegmentFormStepTwo: React.FC<ISegmentFormPartTwoProps> = ({
: [UPDATE_SEGMENT, UPDATE_PROJECT_SEGMENT];
const { segmentValuesLimit } = useSegmentLimits();
const addEditStrategy = useUiFlag('addEditStrategy');
const { state } = useConstraintAccordionList(
hasAccess(modePermission, project) ? setConstraints : undefined,
constraintsAccordionListRef as RefObject<IConstraintAccordionListRef>,
);
const overSegmentValuesLimit: boolean = Boolean(
segmentValuesLimit && segmentValuesCount > segmentValuesLimit,
@ -213,7 +205,7 @@ export const SegmentFormStepTwo: React.FC<ISegmentFormPartTwoProps> = ({
<ConditionallyRender
condition={addEditStrategy}
show={
<NewConstraintAccordionList
<EditableConstraintsList
ref={constraintsAccordionListRef}
constraints={constraints}
setConstraints={
@ -221,7 +213,6 @@ export const SegmentFormStepTwo: React.FC<ISegmentFormPartTwoProps> = ({
? setConstraints
: undefined
}
state={state}
/>
}
elseShow={