From bddc50858260e2ca3958633e620e62722d392071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Thu, 29 Feb 2024 12:56:48 +0000 Subject: [PATCH] chore: actions filter constraints (#6389) https://linear.app/unleash/issue/2-1952/adapt-actions-form-ui-to-unleash-constraint-operators Implements the new action filters UI, now powered by constraint operators. This PR goes through some effort to not touch existing components too much, especially since they are critical for activation strategies. Instead, the new feature tries to adapt to the existing components and styling them appropriately, while still re-using them. We can refactor this at a later stage if needed. This UI will face some more drastic changes in the near future due to the feature rename, so I wanted to keep this PR mostly scoped to the constraint operators before proceeding with more changes. ![image](https://github.com/Unleash/unleash/assets/14320932/f4bef4e0-33bd-463c-a252-e1cc0c22e843) ![image](https://github.com/Unleash/unleash/assets/14320932/192a3bd4-f11d-4619-b826-bbf5df80050c) ![image](https://github.com/Unleash/unleash/assets/14320932/fcfc0239-a28f-446d-a901-2e73f0add1a2) As always, did some manual tests and it seems to be working great! --- .../FreeTextInput/FreeTextInput.tsx | 4 +- .../ResolveInput/ResolveInput.tsx | 4 +- .../useConstraintInput/useConstraintInput.tsx | 4 +- .../CaseSensitiveButton.tsx | 2 +- .../InvertedOperatorButton.tsx | 2 +- .../ProjectActionsFiltersCell.tsx | 28 +- .../ProjectActionsFilterItem.tsx | 283 +++++++++++++++--- .../ProjectActionsForm/ProjectActionsForm.tsx | 42 ++- .../useProjectActionsForm.ts | 26 +- .../ProjectActionsModal.tsx | 20 +- frontend/src/interfaces/action.ts | 9 +- 11 files changed, 352 insertions(+), 72 deletions(-) diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/FreeTextInput/FreeTextInput.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/FreeTextInput/FreeTextInput.tsx index 53569a1740..deb08c4977 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/FreeTextInput/FreeTextInput.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/FreeTextInput/FreeTextInput.tsx @@ -87,7 +87,7 @@ export const FreeTextInput = ({ }; return ( -
+ <> Set values (maximum 100 char length per value) @@ -125,7 +125,7 @@ export const FreeTextInput = ({ removeValue={removeValue} />
- + ); }; diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx index 8b617feda8..e417a2c650 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput.tsx @@ -20,8 +20,8 @@ import { import React from 'react'; interface IResolveInputProps { - contextDefinition: IUnleashContextDefinition; - localConstraint: IConstraint; + contextDefinition: Pick; + localConstraint: Pick; constraintValues: string[]; constraintValue: string; setValue: (value: string) => void; diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/useConstraintInput/useConstraintInput.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/useConstraintInput/useConstraintInput.tsx index bd1c82279a..3df3d08a83 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/useConstraintInput/useConstraintInput.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/useConstraintInput/useConstraintInput.tsx @@ -20,8 +20,8 @@ import { import { nonEmptyArray } from 'utils/nonEmptyArray'; interface IUseConstraintInputProps { - contextDefinition: IUnleashContextDefinition; - localConstraint: IConstraint; + contextDefinition: Pick; + localConstraint: Pick; } interface IUseConstraintOutput { diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/CaseSensitiveButton/CaseSensitiveButton.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/CaseSensitiveButton/CaseSensitiveButton.tsx index 568296017e..54a29fc0d9 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/CaseSensitiveButton/CaseSensitiveButton.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/CaseSensitiveButton/CaseSensitiveButton.tsx @@ -9,7 +9,7 @@ import { ConditionallyRender } from '../../../../ConditionallyRender/Conditional import { IConstraint } from 'interfaces/strategy'; interface CaseSensitiveButtonProps { - localConstraint: IConstraint; + localConstraint: Pick; setCaseInsensitive: () => void; } diff --git a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/InvertedOperatorButton/InvertedOperatorButton.tsx b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/InvertedOperatorButton/InvertedOperatorButton.tsx index 31f523fc86..0a31a1964b 100644 --- a/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/InvertedOperatorButton/InvertedOperatorButton.tsx +++ b/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/InvertedOperatorButton/InvertedOperatorButton.tsx @@ -9,7 +9,7 @@ import { import { ConditionallyRender } from '../../../../ConditionallyRender/ConditionallyRender'; interface InvertedOperatorButtonProps { - localConstraint: IConstraint; + localConstraint: Pick; setInvertedOperator: () => void; } diff --git a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsFiltersCell.tsx b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsFiltersCell.tsx index 0d3bbf2be6..fe84ca4347 100644 --- a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsFiltersCell.tsx +++ b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsFiltersCell.tsx @@ -26,11 +26,29 @@ export const ProjectActionsFiltersCell = ({ - {filters.map(([parameter, value]) => ( - - {parameter}: {value} - - ))} + {filters.map( + ([ + parameter, + { + inverted, + operator, + caseInsensitive, + value, + values, + }, + ]) => ( + + {parameter}{' '} + {inverted ? 'NOT' : ''} {operator}{' '} + {caseInsensitive + ? '(case insensitive)' + : ''}{' '} + + {values ? values.join(', ') : value} + + + ), + )} } > diff --git a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/ProjectActionsFilterItem.tsx b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/ProjectActionsFilterItem.tsx index e0949f3c6a..cc96a69490 100644 --- a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/ProjectActionsFilterItem.tsx +++ b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/ProjectActionsFilterItem.tsx @@ -1,10 +1,73 @@ -import { Badge, IconButton, Tooltip, styled } from '@mui/material'; +import { IconButton, Tooltip, styled } from '@mui/material'; import { ActionsFilterState } from './useProjectActionsForm'; import { Delete } from '@mui/icons-material'; import Input from 'component/common/Input/Input'; import { ProjectActionsFormItem } from './ProjectActionsFormItem'; +import { ConstraintOperatorSelect } from 'component/common/ConstraintAccordion/ConstraintOperatorSelect'; +import { + Operator, + allOperators, + dateOperators, + inOperators, + stringOperators, +} from 'constants/operators'; +import { useEffect, useState } from 'react'; +import { oneOf } from 'utils/oneOf'; +import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; +import { CaseSensitiveButton } from 'component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/CaseSensitiveButton/CaseSensitiveButton'; +import { InvertedOperatorButton } from 'component/common/NewConstraintAccordion/ConstraintAccordionEdit/StyledToggleButton/InvertedOperatorButton/InvertedOperatorButton'; +import { ResolveInput } from 'component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/ResolveInput/ResolveInput'; +import { useConstraintInput } from 'component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditBody/useConstraintInput/useConstraintInput'; +import { useUiFlag } from 'hooks/useUiFlag'; + +const StyledDeleteButton = styled(IconButton)({ + marginRight: '-6px', +}); + +const StyledFilter = styled('div')({ + display: 'flex', + flexDirection: 'column', + width: '100%', +}); + +const StyledFilterHeader = styled('div')(({ theme }) => ({ + display: 'flex', + alignItems: 'center', + gap: theme.spacing(1), + [theme.breakpoints.down('sm')]: { + flexDirection: 'column', + alignItems: 'start', + gap: theme.spacing(2), + }, +})); + +const StyledOperatorOptions = styled('div')(({ theme }) => ({ + width: '100%', + display: 'inline-flex', + flex: 1, + gap: theme.spacing(1), +})); + +const StyledOperatorSelectWrapper = styled('div')(({ theme }) => ({ + width: '100%', + '&&& > div': { + width: '100%', + }, +})); + +const StyledOperatorButtonWrapper = styled('div')(({ theme }) => ({ + display: 'inline-flex', + flex: 1, + '&&& button': { + marginRight: 0, + '&:not(.operator-is-active)': { + backgroundColor: theme.palette.background.elevation2, + }, + }, +})); const StyledInputContainer = styled('div')({ + width: '100%', flex: 1, }); @@ -12,64 +75,208 @@ const StyledInput = styled(Input)({ width: '100%', }); -const StyledBadge = styled(Badge)(({ theme }) => ({ - margin: theme.spacing(0, 1), - fontSize: theme.fontSizes.mainHeader, +const StyledResolveInputWrapper = styled('div')(({ theme }) => ({ + '& > h3': { + margin: theme.spacing(1, 0, 0, 0), + fontSize: theme.fontSizes.smallBody, + }, + '& > div': { + margin: 0, + maxWidth: 'unset', + '& > div': { + flex: 1, + }, + '& .MuiFormControl-root': { + margin: theme.spacing(0.5, 0, 0, 0), + }, + '&:not(:first-of-type)': { + marginTop: theme.spacing(1), + }, + }, })); +interface IProjectActionsFilterItemProps { + filter: ActionsFilterState; + index: number; + stateChanged: (updatedFilter: ActionsFilterState) => void; + onDelete: () => void; +} + export const ProjectActionsFilterItem = ({ filter, index, stateChanged, onDelete, -}: { - filter: ActionsFilterState; - index: number; - stateChanged: (updatedFilter: ActionsFilterState) => void; - onDelete: () => void; -}) => { - const { parameter, value } = filter; +}: IProjectActionsFilterItemProps) => { + const { parameter, inverted, operator, caseInsensitive, value, values } = + filter; const header = ( <> Filter {index + 1}
- + - +
); + // Adapted from `/frontend/src/component/common/NewConstraintAccordion/ConstraintAccordionEdit/ConstraintAccordionEditHeader/ConstraintAccordionEditHeader.tsx` + const [showCaseSensitiveButton, setShowCaseSensitiveButton] = + useState(false); + + const caseInsensitiveInOperators = useUiFlag('caseInsensitiveInOperators'); + + const validOperators = allOperators.filter( + (operator) => !oneOf(dateOperators, operator), + ); + + const { input, validator, setError, error } = useConstraintInput({ + contextDefinition: { legalValues: [] }, + localConstraint: { operator, value, values }, + }); + + const validate = () => { + stateChanged({ + ...filter, + error: undefined, + }); + const [typeValidatorResult, err] = validator(); + if (!typeValidatorResult) { + setError(err); + stateChanged({ + ...filter, + error: err, + }); + } + }; + + useEffect(() => { + validate(); + }, [value, error]); + + useEffect(() => { + if ( + oneOf(stringOperators, operator) || + (oneOf(inOperators, operator) && caseInsensitiveInOperators) + ) { + setShowCaseSensitiveButton(true); + } else { + setShowCaseSensitiveButton(false); + } + }, [operator, caseInsensitiveInOperators]); + + const onOperatorChange = (operator: Operator) => { + if ( + oneOf(stringOperators, operator) || + (oneOf(inOperators, operator) && caseInsensitiveInOperators) + ) { + setShowCaseSensitiveButton(true); + } else { + setShowCaseSensitiveButton(false); + } + + stateChanged({ + ...filter, + operator, + }); + }; + + const setValue = (value: string) => { + stateChanged({ + ...filter, + values: undefined, + value, + }); + }; + + const setValues = (values: string[]) => { + stateChanged({ + ...filter, + value: undefined, + values, + }); + }; + + const removeValue = (index: number) => { + const newValues = values?.filter((_, i) => i !== index) || []; + setValues(newValues); + }; + return ( - - - stateChanged({ - ...filter, - parameter: e.target.value, - }) - } - /> - - = - - - stateChanged({ - ...filter, - value: e.target.value, - }) - } - /> - + + + + + stateChanged({ + ...filter, + parameter: e.target.value, + }) + } + /> + + + + + stateChanged({ + ...filter, + inverted: !inverted || undefined, + }) + } + /> + + + + + + + stateChanged({ + ...filter, + caseInsensitive: + !caseInsensitive || + undefined, + }) + } + /> + + } + /> + + + + + + ); }; diff --git a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/ProjectActionsForm.tsx b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/ProjectActionsForm.tsx index 0ff3f6784b..d4519d20f2 100644 --- a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/ProjectActionsForm.tsx +++ b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/ProjectActionsForm.tsx @@ -18,6 +18,8 @@ import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; import { ProjectActionsActionItem } from './ProjectActionsActionItem'; import { ProjectActionsFilterItem } from './ProjectActionsFilterItem'; import { ProjectActionsFormStep } from './ProjectActionsFormStep'; +import { IN } from 'constants/operators'; +import { HelpIcon } from 'component/common/HelpIcon/HelpIcon'; const StyledServiceAccountAlert = styled(Alert)(({ theme }) => ({ marginBottom: theme.spacing(4), @@ -43,15 +45,10 @@ const StyledInput = styled(Input)(() => ({ width: '100%', })); -const StyledSecondaryDescription = styled('p')(({ theme }) => ({ - display: 'flex', - color: theme.palette.text.secondary, - fontSize: theme.fontSizes.smallBody, - marginBottom: theme.spacing(1), -})); - const StyledButtonContainer = styled('div')(({ theme }) => ({ + display: 'flex', marginTop: theme.spacing(1), + gap: theme.spacing(1), })); const StyledDivider = styled(Divider)(({ theme }) => ({ @@ -59,6 +56,12 @@ const StyledDivider = styled(Divider)(({ theme }) => ({ marginBottom: theme.spacing(2), })); +const StyledTooltip = styled('div')(({ theme }) => ({ + display: 'flex', + flexDirection: 'column', + gap: theme.spacing(1), +})); + interface IProjectActionsFormProps { enabled: boolean; setEnabled: React.Dispatch>; @@ -111,7 +114,7 @@ export const ProjectActionsForm = ({ { id, parameter: '', - value: '', + operator: IN, }, ]); }; @@ -232,10 +235,6 @@ export const ProjectActionsForm = ({ - - If no filters are defined then the action will be triggered - every time the incoming webhook is called. - {filters.map((filter, index) => ( Add filter + +

+ Filters allow you to add conditions to the + execution of the actions based on the source + payload. +

+

+ If no filters are defined then the action + will always be triggered from the selected + source, no matter the payload. +

+ + } + /> diff --git a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/useProjectActionsForm.ts b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/useProjectActionsForm.ts index 155fa11221..92b59602a5 100644 --- a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/useProjectActionsForm.ts +++ b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsForm/useProjectActionsForm.ts @@ -1,5 +1,5 @@ import { useActions } from 'hooks/api/getters/useActions/useActions'; -import { IAction, IActionSet } from 'interfaces/action'; +import { IAction, IActionSet, ParameterMatch } from 'interfaces/action'; import { useEffect, useState } from 'react'; import { v4 as uuidv4 } from 'uuid'; import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; @@ -7,14 +7,15 @@ import { useRequiredPathParam } from 'hooks/useRequiredPathParam'; enum ErrorField { NAME = 'name', TRIGGER = 'trigger', + FILTERS = 'filters', ACTOR = 'actor', ACTIONS = 'actions', } -export type ActionsFilterState = { +export type ActionsFilterState = ParameterMatch & { id: string; parameter: string; - value: string; + error?: string; }; export type ActionsActionState = Omit< @@ -27,6 +28,7 @@ export type ActionsActionState = Omit< const DEFAULT_PROJECT_ACTIONS_FORM_ERRORS = { [ErrorField.NAME]: undefined, [ErrorField.TRIGGER]: undefined, + [ErrorField.FILTERS]: undefined, [ErrorField.ACTOR]: undefined, [ErrorField.ACTIONS]: undefined, }; @@ -50,10 +52,17 @@ export const useProjectActionsForm = (action?: IActionSet) => { setSourceId(action?.match?.sourceId ?? 0); setFilters( Object.entries(action?.match?.payload ?? {}).map( - ([parameter, value]) => ({ + ([ + parameter, + { inverted, operator, caseInsensitive, value, values }, + ]) => ({ id: uuidv4(), parameter, - value: value as string, + inverted, + operator, + caseInsensitive, + value, + values, }), ), ); @@ -87,6 +96,13 @@ export const useProjectActionsForm = (action?: IActionSet) => { setErrors((errors) => ({ ...errors, [field]: error })); }; + useEffect(() => { + clearError(ErrorField.FILTERS); + if (filters.some(({ error }) => error)) { + setError(ErrorField.FILTERS, 'One or more filters have errors.'); + } + }, [filters]); + const isEmpty = (value: string) => !value.length; const isNameNotUnique = (value: string) => diff --git a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsModal.tsx b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsModal.tsx index 1920e94c78..2e3e38c2e4 100644 --- a/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsModal.tsx +++ b/frontend/src/component/project/Project/ProjectSettings/ProjectActions/ProjectActionsTable/ProjectActionsModal/ProjectActionsModal.tsx @@ -100,9 +100,25 @@ export const ProjectActionsModal = ({ payload: filters .filter((f) => f.parameter.length > 0) .reduce( - (acc, filter) => ({ + ( + acc, + { + parameter, + inverted, + operator, + caseInsensitive, + value, + values, + }, + ) => ({ ...acc, - [filter.parameter]: filter.value, + [parameter]: { + inverted, + operator, + caseInsensitive, + value, + values, + }, }), {}, ), diff --git a/frontend/src/interfaces/action.ts b/frontend/src/interfaces/action.ts index bb0094f70b..e0b3acdc04 100644 --- a/frontend/src/interfaces/action.ts +++ b/frontend/src/interfaces/action.ts @@ -1,3 +1,5 @@ +import { IConstraint } from './strategy'; + export interface IActionSet { id: number; enabled: boolean; @@ -12,10 +14,15 @@ export interface IActionSet { type MatchSource = 'incoming-webhook'; +export type ParameterMatch = Pick< + IConstraint, + 'inverted' | 'operator' | 'caseInsensitive' | 'value' | 'values' +>; + export interface IMatch { source: MatchSource; sourceId: number; - payload: Record; + payload: Record; } export interface IAction {