From 39b0c089d18223e3064f5d0d8e1f32278c805365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Wed, 20 Sep 2023 09:21:30 +0100 Subject: [PATCH] feat: simpler integration filters (#4766) https://linear.app/unleash/issue/2-1407/remove-the-all-checkboxes-from-project-and-environment-filters Simplifies integration event filters by removing the "ALL" checkboxes from these components. Whether you opted to check the "ALL" checkbox, or not to filter at all, the result is the same - The selected options would act as a filter. Includes some refactoring and clean up. ![image](https://github.com/Unleash/unleash/assets/14320932/2e30c5c5-12e1-4bc6-bd4a-8be4226d625d) --- .../IntegrationForm.styles.tsx | 8 +- .../IntegrationForm/IntegrationForm.tsx | 8 +- .../IntegrationMultiSelector.test.tsx | 90 +------------ .../IntegrationMultiSelector.tsx | 125 +++--------------- .../IntegrationParameterTextField.tsx | 4 +- src/lib/addons/datadog-definition.ts | 6 +- src/lib/addons/slack-definition.ts | 6 +- src/lib/addons/teams-definition.ts | 6 +- src/lib/openapi/spec/addons-schema.ts | 8 +- 9 files changed, 47 insertions(+), 214 deletions(-) diff --git a/frontend/src/component/integrations/IntegrationForm/IntegrationForm.styles.tsx b/frontend/src/component/integrations/IntegrationForm/IntegrationForm.styles.tsx index 8c7584f3da..b2de776892 100644 --- a/frontend/src/component/integrations/IntegrationForm/IntegrationForm.styles.tsx +++ b/frontend/src/component/integrations/IntegrationForm/IntegrationForm.styles.tsx @@ -1,5 +1,5 @@ import { Paper, styled } from '@mui/material'; -import { FormControlLabel, TextField, Typography } from '@mui/material'; +import { TextField, Typography } from '@mui/material'; import { forwardRef, type FC, type ReactNode, ComponentProps } from 'react'; export const StyledForm = styled('form')(({ theme }) => ({ @@ -43,12 +43,6 @@ export const StyledTextField = styled(TextField)(({ theme }) => ({ width: '100%', })); -export const StyledSelectAllFormControlLabel = styled(FormControlLabel)( - ({ theme }) => ({ - paddingBottom: theme.spacing(1), - }) -); - export const StyledTitle = forwardRef< HTMLHeadingElement, { children: ReactNode } diff --git a/frontend/src/component/integrations/IntegrationForm/IntegrationForm.tsx b/frontend/src/component/integrations/IntegrationForm/IntegrationForm.tsx index 0659eb4736..ff44a87a20 100644 --- a/frontend/src/component/integrations/IntegrationForm/IntegrationForm.tsx +++ b/frontend/src/component/integrations/IntegrationForm/IntegrationForm.tsx @@ -367,7 +367,6 @@ export const IntegrationForm: VFC = ({ selectedItems={formValues.events} onChange={setEventValues} entityName="event" - selectAllEnabled={false} error={errors.events} description="Select which events you want your integration to be notified about." required @@ -379,7 +378,8 @@ export const IntegrationForm: VFC = ({ selectedItems={formValues.projects || []} onChange={setProjects} entityName="project" - selectAllEnabled={true} + description="Selecting project(s) will filter events, so that your integration only receives events related to those specific projects." + note="If no projects are selected, the integration will receive events from all projects." />
@@ -388,8 +388,8 @@ export const IntegrationForm: VFC = ({ selectedItems={formValues.environments || []} onChange={setEnvironments} entityName="environment" - selectAllEnabled={true} - description="Global events that are not specific to an environment will still be received." + description="Selecting environment(s) will filter events, so that your integration only receives events related to those specific environments. Global events that are not specific to an environment will still be received." + note="If no environments are selected, the integration will receive events from all environments." />
diff --git a/frontend/src/component/integrations/IntegrationForm/IntegrationMultiSelector/IntegrationMultiSelector.test.tsx b/frontend/src/component/integrations/IntegrationForm/IntegrationMultiSelector/IntegrationMultiSelector.test.tsx index 93c8349feb..9802b4d616 100644 --- a/frontend/src/component/integrations/IntegrationForm/IntegrationMultiSelector/IntegrationMultiSelector.test.tsx +++ b/frontend/src/component/integrations/IntegrationForm/IntegrationMultiSelector/IntegrationMultiSelector.test.tsx @@ -1,5 +1,4 @@ import { vi } from 'vitest'; -import React from 'react'; import { screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { render } from 'utils/testRenderer'; @@ -21,8 +20,8 @@ const mockProps: IIntegrationMultiSelectorProps = { selectedItems: [], onChange, onFocus, - selectAllEnabled: true, entityName: 'project', + description: 'some description', }; const server = testServerSetup(); @@ -35,85 +34,18 @@ describe('AddonMultiSelector', () => { }); it('renders with default state', () => { - render( - - ); - - const checkbox = screen.getByLabelText( - /all current and future projects/i - ); - expect(checkbox).toBeChecked(); - - const selectInputContainer = screen.getByTestId('select-project-input'); - const input = within(selectInputContainer).getByRole('combobox'); - expect(input).toBeDisabled(); - }); - - it('can toggle "ALL" checkbox', async () => { - const user = userEvent.setup(); - const { rerender } = render( - - ); - - await user.click(screen.getByTestId('select-all-projects')); - - expect(onChange).toHaveBeenCalledWith([]); - - rerender( - - ); - - await user.click(screen.getByTestId('select-all-projects')); - - expect(onChange).toHaveBeenCalledWith(['*']); - }); - - it('renders with autocomplete enabled if default value is not a wildcard', () => { - render( - - ); - - const checkbox = screen.getByLabelText( - /all current and future projects/i - ); - expect(checkbox).not.toBeChecked(); + render(); const selectInputContainer = screen.getByTestId('select-project-input'); const input = within(selectInputContainer).getByRole('combobox'); expect(input).toBeEnabled(); }); - describe('Select/Deselect projects in dropdown', () => { - it("doesn't show up for less than 3 options", async () => { - const user = userEvent.setup(); - render( - - ); - await user.click(screen.getByLabelText('Projects')); - - const button = screen.queryByRole('button', { - name: /select all/i, - }); - expect(button).not.toBeInTheDocument(); - }); - }); - it('can filter options', async () => { const user = userEvent.setup(); render( { expect(screen.queryByText('Alpha')).not.toBeInTheDocument(); }); }); - - it('will load wildcard status from props', async () => { - const { rerender } = render( - - ); - - expect( - screen.getByLabelText(/all current and future projects/i) - ).not.toBeChecked(); - - rerender( - - ); - - expect( - screen.getByLabelText(/all current and future projects/i) - ).toBeChecked(); - }); }); diff --git a/frontend/src/component/integrations/IntegrationForm/IntegrationMultiSelector/IntegrationMultiSelector.tsx b/frontend/src/component/integrations/IntegrationForm/IntegrationMultiSelector/IntegrationMultiSelector.tsx index 0c21459b01..4e1fe8cc1d 100644 --- a/frontend/src/component/integrations/IntegrationForm/IntegrationMultiSelector/IntegrationMultiSelector.tsx +++ b/frontend/src/component/integrations/IntegrationForm/IntegrationMultiSelector/IntegrationMultiSelector.tsx @@ -1,7 +1,6 @@ -import React, { ChangeEvent, Fragment, VFC } from 'react'; +import { VFC } from 'react'; import { IAutocompleteBoxOption } from '../../../common/AutocompleteBox/AutocompleteBox'; import { - AutocompleteRenderGroupParams, AutocompleteRenderInputParams, AutocompleteRenderOptionState, } from '@mui/material/Autocomplete'; @@ -16,13 +15,7 @@ import { } from '@mui/material'; import CheckBoxOutlineBlankIcon from '@mui/icons-material/CheckBoxOutlineBlank'; import CheckBoxIcon from '@mui/icons-material/CheckBox'; -import { ConditionallyRender } from '../../../common/ConditionallyRender/ConditionallyRender'; -import { SelectAllButton } from '../../../admin/apiToken/ApiTokenForm/ProjectSelector/SelectProjectInput/SelectAllButton/SelectAllButton'; -import { - StyledHelpText, - StyledSelectAllFormControlLabel, - StyledTitle, -} from '../IntegrationForm.styles'; +import { StyledHelpText, StyledTitle } from '../IntegrationForm.styles'; export interface IIntegrationMultiSelectorProps { options: IAutocompleteBoxOption[]; @@ -31,13 +24,11 @@ export interface IIntegrationMultiSelectorProps { error?: string; onFocus?: () => void; entityName: string; - selectAllEnabled: boolean; - description?: string; + description: string; + note?: string; required?: boolean; } -const ALL_OPTIONS = '*'; - const StyledCheckbox = styled(Checkbox)(() => ({ marginRight: '0.2em', })); @@ -51,45 +42,30 @@ export const IntegrationMultiSelector: VFC = ({ error, onFocus, entityName, - selectAllEnabled = true, description, + note, required, }) => { const renderInput = (params: AutocompleteRenderInputParams) => ( + {capitalize(`${entityName}s`)} + {required ? ( + * + ) : null} + + } placeholder={`Select ${entityName}s to filter by`} onFocus={onFocus} data-testid={`select-${entityName}-input`} /> ); - const isAllSelected = - selectedItems.length > 0 && - selectedItems.length === options.length && - selectedItems[0] !== ALL_OPTIONS; - - const isWildcardSelected = selectedItems.includes(ALL_OPTIONS); - - const onAllItemsChange = ( - e: ChangeEvent, - checked: boolean - ) => { - if (checked) { - onChange([ALL_OPTIONS]); - } else { - onChange(selectedItems.includes(ALL_OPTIONS) ? [] : selectedItems); - } - }; - - const onSelectAllClick = () => { - const newItems = isAllSelected ? [] : options.map(({ value }) => value); - onChange(newItems); - }; const renderOption = ( props: object, option: IAutocompleteBoxOption, @@ -106,89 +82,30 @@ export const IntegrationMultiSelector: VFC = ({ ); }; - const renderGroup = ({ key, children }: AutocompleteRenderGroupParams) => ( - - 2 && selectAllEnabled} - show={ - - } - /> - {children} - - ); - const SelectAllFormControl = () => ( - - } - label={`ALL current and future ${entityName}s`} - /> - ); - - const DefaultHelpText = () => ( - - Selecting {entityName}(s) will filter events, so that your - integration only receives events related to those specific{' '} - {entityName}s. - - ); return ( - - - {capitalize(`${entityName}s`)} - {required ? ( - - * - - ) : null} - - } - /> - {description}} - /> - } - /> + <> + {capitalize(`${entityName}s`)} + {description} label} fullWidth - groupBy={() => 'Select/Deselect all'} - renderGroup={renderGroup} PaperComponent={CustomPaper} renderOption={renderOption} renderInput={renderInput} - value={ - isWildcardSelected - ? options - : options.filter(option => - selectedItems.includes(option.value) - ) - } + value={options.filter(option => + selectedItems.includes(option.value) + )} onChange={(_, input) => { const state = input.map(({ value }) => value); onChange(state); }} /> - + ); }; diff --git a/frontend/src/component/integrations/IntegrationForm/IntegrationParameters/IntegrationParameter/IntegrationParameterTextField.tsx b/frontend/src/component/integrations/IntegrationForm/IntegrationParameters/IntegrationParameter/IntegrationParameterTextField.tsx index dbe59e4319..d0502e8659 100644 --- a/frontend/src/component/integrations/IntegrationForm/IntegrationParameters/IntegrationParameter/IntegrationParameterTextField.tsx +++ b/frontend/src/component/integrations/IntegrationForm/IntegrationParameters/IntegrationParameter/IntegrationParameterTextField.tsx @@ -49,9 +49,7 @@ export const IntegrationParameterTextField = ({ <> {definition.displayName} {definition.required ? ( - - * - + * ) : null} } diff --git a/src/lib/addons/datadog-definition.ts b/src/lib/addons/datadog-definition.ts index 37ef23a9c9..f5c88daacf 100644 --- a/src/lib/addons/datadog-definition.ts +++ b/src/lib/addons/datadog-definition.ts @@ -54,8 +54,10 @@ const dataDogDefinition: IAddonDefinition = { { name: 'customHeaders', displayName: 'Extra HTTP Headers', - placeholder: - '{\n"ISTIO_USER_KEY": "hunter2",\n"SOME_OTHER_CUSTOM_HTTP_HEADER": "SOMEVALUE"\n}', + placeholder: `{ + "ISTIO_USER_KEY": "hunter2", + "SOME_OTHER_CUSTOM_HTTP_HEADER": "SOMEVALUE" +}`, description: '(Optional) Used to add extra HTTP Headers to the request the plugin fires off. This must be a valid json object of key-value pairs where both the key and the value are strings', required: false, diff --git a/src/lib/addons/slack-definition.ts b/src/lib/addons/slack-definition.ts index f933107289..ebbec92969 100644 --- a/src/lib/addons/slack-definition.ts +++ b/src/lib/addons/slack-definition.ts @@ -69,8 +69,10 @@ const slackDefinition: IAddonDefinition = { { name: 'customHeaders', displayName: 'Extra HTTP Headers', - placeholder: - '{\n"ISTIO_USER_KEY": "hunter2",\n"SOME_OTHER_CUSTOM_HTTP_HEADER": "SOMEVALUE"\n}', + placeholder: `{ + "ISTIO_USER_KEY": "hunter2", + "SOME_OTHER_CUSTOM_HTTP_HEADER": "SOMEVALUE" +}`, description: `(Optional) Used to add extra HTTP Headers to the request the plugin fires off. This must be a valid json object of key-value pairs where both the key and the value are strings`, required: false, sensitive: true, diff --git a/src/lib/addons/teams-definition.ts b/src/lib/addons/teams-definition.ts index 8b6e99e26d..3c888be9f6 100644 --- a/src/lib/addons/teams-definition.ts +++ b/src/lib/addons/teams-definition.ts @@ -35,8 +35,10 @@ const teamsDefinition: IAddonDefinition = { { name: 'customHeaders', displayName: 'Extra HTTP Headers', - placeholder: - '{\n"ISTIO_USER_KEY": "hunter2",\n"SOME_OTHER_CUSTOM_HTTP_HEADER": "SOMEVALUE"\n}', + placeholder: `{ + "ISTIO_USER_KEY": "hunter2", + "SOME_OTHER_CUSTOM_HTTP_HEADER": "SOMEVALUE" +}`, description: `(Optional) Used to add extra HTTP Headers to the request the plugin fires off. This must be a valid json object of key-value pairs where both the key and the value are strings`, required: false, sensitive: true, diff --git a/src/lib/openapi/spec/addons-schema.ts b/src/lib/openapi/spec/addons-schema.ts index c1d46cb7f6..d76e9f4a8e 100644 --- a/src/lib/openapi/spec/addons-schema.ts +++ b/src/lib/openapi/spec/addons-schema.ts @@ -65,8 +65,12 @@ export const addonsSchema = { { name: 'bodyTemplate', displayName: 'Body template', - placeholder: - '{\n "event": "{{event.type}}",\n "createdBy": "{{event.createdBy}}",\n "featureToggle": "{{event.data.name}}",\n "timestamp": "{{event.data.createdAt}}"\n}', + placeholder: `{ + "event": "{{event.type}}", + "createdBy": "{{event.createdBy}}", + "featureToggle": "{{event.data.name}}", + "timestamp": "{{event.data.createdAt}}" +}`, description: "(Optional) You may format the body using a mustache template. If you don't specify anything, the format will similar to the events format (https://docs.getunleash.io/reference/api/legacy/unleash/admin/events)", type: 'textfield',