Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-01-11 00:08:30 +01:00
Code Issues Projects Releases Wiki Activity

Fix/autosave on delete (#5899)

Browse Source 
This PR will make FeatureStrategyConstraints use the value coming from
the setState function instead of closing over a stale value.
This commit is contained in:
Fredrik Strand Oseberg 2024-01-16 09:23:35 +01:00 committed by GitHub
parent 6e234727ee
commit 9d370ad85d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 50 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEdit.tsx
View File

@ -134,10 +134,6 @@ export const ConstraintAccordionEdit = ({
if (onAutoSave) {
onAutoSave(localConstraint);
}
if (onAutoSave && localConstraint.value) {
onAutoSave(localConstraint);
}
};
const recordChange = (localConstraint: IConstraint) => {
@ -233,7 +229,7 @@ export const ConstraintAccordionEdit = ({
setValuesWithRecord(valueCopy);
},
[localConstraint, setValues],
[localConstraint, setValuesWithRecord],
);
const triggerTransition = () => {

1
frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/FreeTextInput/FreeTextInput.tsx
View File

@ -157,6 +157,7 @@ const ConstraintValueChips = ({
key={`${value}-${index}`}
onDelete={() => removeValue(index)}
className={styles.valueChip}
data-testid='CONSTRAINT_VALUES_CHIP'
/>
);
})}

75
frontend/src/component/common/NewConstraintAccordion/NewConstraintAccordionList/NewConstraintAccordionList.tsx
View File

@ -74,6 +74,7 @@ export const useConstraintAccordionList = (
| undefined,
ref: React.RefObject<IConstraintAccordionListRef>,
) => {
// Constraint metadata: This is a weak map to give a constraint an ID by using the placement in memory.
const state = useWeakMap<IConstraint, IConstraintAccordionListItemState>();
const { context } = useUnleashContext();
@ -97,80 +98,6 @@ export const useConstraintAccordionList = (
return { onAdd, state, context };
};
export const ConstraintAccordionList = forwardRef<
IConstraintAccordionListRef | undefined,
IConstraintAccordionListProps
>(
(
{ constraints, setConstraints, showCreateButton, showLabel = true },
ref,
) => {
const { onAdd, state, context } = useConstraintAccordionList(
setConstraints,
ref as RefObject<IConstraintAccordionListRef>,
);
if (context.length === 0) {
return null;
}
return (
<StyledContainer id={constraintAccordionListId}>
<ConditionallyRender
condition={
constraints && constraints.length > 0 && showLabel
}
show={
<StyledConstraintLabel>
Constraints
</StyledConstraintLabel>
}
/>
<NewConstraintAccordionList
ref={ref}
setConstraints={setConstraints}
constraints={constraints}
state={state}
/>
<ConditionallyRender
condition={Boolean(showCreateButton && onAdd)}
show={
<div>
<StyledAddCustomLabel>
<p>Add any number of constraints</p>
<StyledHelpWrapper
title='View constraints documentation'
arrow
>
<a
href={
'https://docs.getunleash.io/reference/strategy-constraints'
}
target='_blank'
rel='noopener noreferrer'
>
<StyledHelp />
</a>
</StyledHelpWrapper>
</StyledAddCustomLabel>
<Button
type='button'
onClick={onAdd}
variant='outlined'
color='primary'
data-testid='ADD_CONSTRAINT_BUTTON'
>
Add constraint
</Button>
</div>
}
/>
</StyledContainer>
);
},
);
interface IConstraintList {
constraints: IConstraint[];
setConstraints?: React.Dispatch<React.SetStateAction<IConstraint[]>>;

9
frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/FeatureStrategyConstraints.tsx
View File

@ -47,14 +47,15 @@ export const FeatureStrategyConstraints = ({
};
}, []);
const constraints = useMemo(() => {
return strategy.constraints ?? [];
}, [strategy]);
const constraints = strategy.constraints || [];
const setConstraints = (value: React.SetStateAction<IConstraint[]>) => {
setStrategy((prev) => ({
...prev,
constraints: value instanceof Function ? value(constraints) : value,
constraints:
value instanceof Function
? value(prev.constraints || [])
: value,
}));
};

42
frontend/src/component/feature/FeatureStrategy/NewFeatureStrategyCreate/NewFeatureStrategyCreate.test.tsx
View File

@ -277,6 +277,48 @@ describe('NewFeatureStrategyCreate', () => {
expect(screen.getByText(values[2])).toBeInTheDocument();
});
test('Should update multiple constraints correctly', 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 = await screen.findAllByTestId('CancelIcon');
fireEvent.click(deleteBtns[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();