From 67e63f7642f5c64d7f5a88e9608bbeab00a20f9a Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Wed, 22 Dec 2021 17:10:19 +0100 Subject: [PATCH 1/7] Displays assignable artifact list and allow selection. Also displays empty state message --- .../policy/view/event_filters/flyout/index.ts | 8 + .../flyout/policy_event_filters_flyout.tsx | 223 ++++++++++++++++++ .../pages/policy/view/event_filters/hooks.ts | 42 +++- .../layout/policy_event_filters_layout.tsx | 30 ++- 4 files changed, 297 insertions(+), 6 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/index.ts create mode 100644 x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/index.ts b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/index.ts new file mode 100644 index 0000000000000..ae6861787044d --- /dev/null +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/index.ts @@ -0,0 +1,8 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export { PolicyEventFiltersFlyout } from './policy_event_filters_flyout'; diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx new file mode 100644 index 0000000000000..f37cddbf1085f --- /dev/null +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx @@ -0,0 +1,223 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React, { useCallback, useMemo, useState } from 'react'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n-react'; +import { isEmpty, without } from 'lodash/fp'; +import { + EuiTitle, + EuiFlyout, + EuiSpacer, + EuiFlyoutHeader, + EuiFlyoutBody, + EuiFlyoutFooter, + EuiFlexGroup, + EuiFlexItem, + EuiButtonEmpty, + EuiButton, + EuiCallOut, + EuiEmptyPrompt, +} from '@elastic/eui'; +import { SearchExceptions } from '../../../../../components/search_exceptions'; +import { ImmutableObject, PolicyData } from '../../../../../../../common/endpoint/types'; +import { EventFiltersHttpService } from '../../../../event_filters/service'; +import { useHttp } from '../../../../../../common/lib/kibana'; +import { useSearchNotAssignedEventFilters, useGetAllAssignedEventFilters } from '../hooks'; +import { PolicyArtifactsAssignableList } from '../../artifacts/assignable'; + +interface PolicyEventFiltersFlyoutProps { + policyItem: ImmutableObject; + onClose: () => void; +} + +const MAX_ALLOWED_RESULTS = 100; + +export const PolicyEventFiltersFlyout = React.memo( + ({ policyItem, onClose }) => { + const [selectedArtifactIds, setSelectedArtifactIds] = useState([]); + const [currentFilter, setCurrentFilter] = useState(''); + const http = useHttp(); + const eventFiltersService = useMemo(() => new EventFiltersHttpService(http), [http]); + + const handleOnSearch = useCallback((query) => { + setSelectedArtifactIds([]); + setCurrentFilter(query); + }, []); + const handleOnConfirmAction = useCallback(() => {}, []); + + const handleSelectArtifacts = (artifactId: string, selected: boolean) => { + setSelectedArtifactIds((currentSelectedArtifactIds) => + selected + ? [...currentSelectedArtifactIds, artifactId] + : without([artifactId], currentSelectedArtifactIds) + ); + }; + + const { data: eventFilters, isLoading: isLoadingEventFilters } = + useSearchNotAssignedEventFilters(eventFiltersService, policyItem.id, { + filter: currentFilter, + }); + + const { data: allAssigned, isLoading: isLoadingAllAssigned } = useGetAllAssignedEventFilters( + policyItem.id, + currentFilter !== '' && eventFilters?.total === 0, + 'assignmentFlyout' + ); + + const searchWarningMessage = useMemo( + () => ( + <> + + {i18n.translate( + 'xpack.securitySolution.endpoint.policy.eventFilters.layout.flyout.searchWarning.text', + { + defaultMessage: + 'Only the first 100 event filters are displayed. Please use the search bar to refine the results.', + } + )} + + + + ), + [] + ); + + const noItemsMessage = useMemo(() => { + if (isLoadingEventFilters || isLoadingAllAssigned) { + return null; + } + + // there are no event filters assignable to this policy + if (allAssigned?.total === 0 || (eventFilters?.total === 0 && currentFilter === '')) { + return ( + + } + /> + ); + } + + // there are no results for the current search + if (eventFilters?.total === 0) { + return ( + + } + /> + ); + } + }, [ + allAssigned?.total, + currentFilter, + eventFilters?.total, + isLoadingAllAssigned, + isLoadingEventFilters, + ]); + + return ( + + + +

+ +

+ + + + + + {(eventFilters?.total || 0) > MAX_ALLOWED_RESULTS ? searchWarningMessage : null} + + + + + + {noItemsMessage} + + + + + + + + + + + + + + + + + ); + } +); + +PolicyEventFiltersFlyout.displayName = 'PolicyEventFiltersFlyout'; diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts index 91297a1119e5b..3eb33ba047105 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts @@ -13,13 +13,15 @@ import { parseQueryFilterToKQL } from '../../../../common/utils'; import { SEARCHABLE_FIELDS } from '../../../event_filters/constants'; export function useGetAllAssignedEventFilters( - policyId?: string + policyId: string, + enabled: boolean = true, + customQueryId: string = '' ): QueryObserverResult { const http = useHttp(); const eventFiltersService = new EventFiltersHttpService(http); return useQuery( - ['eventFilters', 'assigned', policyId], + ['eventFilters', 'assigned', policyId, customQueryId], () => { return eventFiltersService.getList({ filter: `(exception-list-agnostic.attributes.tags:"policy:${policyId}" OR exception-list-agnostic.attributes.tags:"policy:all")`, @@ -28,8 +30,7 @@ export function useGetAllAssignedEventFilters( { refetchIntervalInBackground: false, refetchOnWindowFocus: false, - enabled: !!policyId, - refetchOnMount: true, + enabled, } ); } @@ -68,6 +69,39 @@ export function useSearchAssignedEventFilters( ); } +export function useSearchNotAssignedEventFilters( + eventFiltersService: EventFiltersHttpService, + policyId: string, + options: { filter?: string } +): QueryObserverResult { + return useQuery( + ['eventFilters', 'notAssigned', policyId, options], + () => { + const { filter } = options; + const kuery = [ + `((exception-list-agnostic.attributes.tags:"policy:${policyId}") OR (exception-list-agnostic.attributes.tags:"policy:all"))`, + ]; + + if (filter) { + const filterKuery = parseQueryFilterToKQL(filter, SEARCHABLE_FIELDS) || undefined; + if (filterKuery) { + kuery.push(filterKuery); + } + } + + return eventFiltersService.getList({ + filter: kuery.join(' AND '), + }); + }, + { + refetchIntervalInBackground: false, + refetchOnWindowFocus: false, + enabled: !!policyId, + refetchOnMount: true, + } + ); +} + export function useGetAllEventFilters(): QueryObserverResult< FoundExceptionListItemSchema, ServerApiError diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/layout/policy_event_filters_layout.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/layout/policy_event_filters_layout.tsx index d03832e233b30..61970c74a4969 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/layout/policy_event_filters_layout.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/layout/policy_event_filters_layout.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useMemo } from 'react'; +import React, { useMemo, useCallback } from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; import { @@ -15,6 +15,7 @@ import { EuiText, EuiSpacer, EuiLink, + EuiButton, } from '@elastic/eui'; import { useAppUrl } from '../../../../../../common/lib/kibana'; import { APP_UI_ID } from '../../../../../../../common/constants'; @@ -23,6 +24,9 @@ import { getEventFiltersListPath } from '../../../../../common/routing'; import { useGetAllAssignedEventFilters, useGetAllEventFilters } from '../hooks'; import { ManagementPageLoader } from '../../../../../components/management_page_loader'; import { PolicyEventFiltersEmptyUnassigned, PolicyEventFiltersEmptyUnexisting } from '../empty'; +import { usePolicyDetailsSelector } from '../../policy_hooks'; +import { getCurrentArtifactsLocation } from '../../../store/policy_details/selectors'; +import { PolicyEventFiltersFlyout } from '../flyout'; interface PolicyEventFiltersLayoutProps { policyItem?: ImmutableObject | undefined; @@ -30,12 +34,13 @@ interface PolicyEventFiltersLayoutProps { export const PolicyEventFiltersLayout = React.memo( ({ policyItem }) => { const { getAppUrl } = useAppUrl(); + const urlParams = usePolicyDetailsSelector(getCurrentArtifactsLocation); const { data: allAssigned, isLoading: isLoadingAllAssigned, isRefetching: isRefetchingAllAssigned, - } = useGetAllAssignedEventFilters(policyItem?.id); + } = useGetAllAssignedEventFilters(policyItem?.id || '', !!policyItem?.id); const { data: allEventFilters, @@ -43,6 +48,9 @@ export const PolicyEventFiltersLayout = React.memo {}, []); + const handleOnCloseFlyout = useCallback(() => {}, []); + const aboutInfo = useMemo(() => { const link = ( {aboutInfo}

+ + + {i18n.translate( + 'xpack.securitySolution.endpoint.policy.eventFilters.layout.assignToPolicy', + { + defaultMessage: 'Assign event filters to policy', + } + )} + + + {urlParams.show === 'list' && ( + + )} ); } From 5e6479a78c75dc18d40a3e64c921e2baede6d35f Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 23 Dec 2021 12:40:08 +0100 Subject: [PATCH 2/7] Adds mutation to update event filters with new tags --- .../pages/event_filters/service/index.ts | 13 ++++ .../pages/event_filters/store/middleware.ts | 14 ---- .../flyout/policy_event_filters_flyout.tsx | 49 +++++++++----- .../pages/policy/view/event_filters/hooks.ts | 67 +++++++++++++++---- 4 files changed, 100 insertions(+), 43 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/event_filters/service/index.ts b/x-pack/plugins/security_solution/public/management/pages/event_filters/service/index.ts index 05729c6d1c14d..70ac198dd3478 100644 --- a/x-pack/plugins/security_solution/public/management/pages/event_filters/service/index.ts +++ b/x-pack/plugins/security_solution/public/management/pages/event_filters/service/index.ts @@ -90,6 +90,19 @@ export class EventFiltersHttpService implements EventFiltersService { async updateOne( exception: Immutable ): Promise { + // Clean unnecessary fields for update action + [ + 'created_at', + 'created_by', + 'created_at', + 'created_by', + 'list_id', + 'tie_breaker_id', + 'updated_at', + 'updated_by', + ].forEach((field) => { + delete exception[field as keyof UpdateExceptionListItemSchema]; + }); return (await this.httpWrapper()).put(EXCEPTION_LIST_ITEM_URL, { body: JSON.stringify(exception), }); diff --git a/x-pack/plugins/security_solution/public/management/pages/event_filters/store/middleware.ts b/x-pack/plugins/security_solution/public/management/pages/event_filters/store/middleware.ts index d1693d24363f2..e1bc2b18b15f0 100644 --- a/x-pack/plugins/security_solution/public/management/pages/event_filters/store/middleware.ts +++ b/x-pack/plugins/security_solution/public/management/pages/event_filters/store/middleware.ts @@ -136,20 +136,6 @@ const eventFiltersUpdate = async ( getNewComment(store.getState()) ) as UpdateExceptionListItemSchema; - // Clean unnecessary fields for update action - [ - 'created_at', - 'created_by', - 'created_at', - 'created_by', - 'list_id', - 'tie_breaker_id', - 'updated_at', - 'updated_by', - ].forEach((field) => { - delete updatedCommentsEntry[field as keyof UpdateExceptionListItemSchema]; - }); - updatedCommentsEntry.comments = updatedCommentsEntry.comments?.map((comment) => ({ comment: comment.comment, id: comment.id, diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx index f37cddbf1085f..52fb228907107 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx @@ -9,6 +9,7 @@ import React, { useCallback, useMemo, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; import { isEmpty, without } from 'lodash/fp'; +import { ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; import { EuiTitle, EuiFlyout, @@ -27,7 +28,11 @@ import { SearchExceptions } from '../../../../../components/search_exceptions'; import { ImmutableObject, PolicyData } from '../../../../../../../common/endpoint/types'; import { EventFiltersHttpService } from '../../../../event_filters/service'; import { useHttp } from '../../../../../../common/lib/kibana'; -import { useSearchNotAssignedEventFilters, useGetAllAssignedEventFilters } from '../hooks'; +import { + useSearchNotAssignedEventFilters, + useGetAllAssignedEventFilters, + useBulkUpdateEventFilters, +} from '../hooks'; import { PolicyArtifactsAssignableList } from '../../artifacts/assignable'; interface PolicyEventFiltersFlyoutProps { @@ -44,11 +49,37 @@ export const PolicyEventFiltersFlyout = React.memo new EventFiltersHttpService(http), [http]); + const bulkUpdateMutation = useBulkUpdateEventFilters(); + const { data: eventFilters, isLoading: isLoadingEventFilters } = + useSearchNotAssignedEventFilters(eventFiltersService, policyItem.id, { + filter: currentFilter, + }); + + const { data: allAssigned, isLoading: isLoadingAllAssigned } = useGetAllAssignedEventFilters( + policyItem.id, + currentFilter !== '' && eventFilters?.total === 0, + 'assignmentFlyout' + ); + const handleOnSearch = useCallback((query) => { setSelectedArtifactIds([]); setCurrentFilter(query); }, []); - const handleOnConfirmAction = useCallback(() => {}, []); + + const handleOnConfirmAction = useCallback(() => { + if (!eventFilters) { + return; + } + const eventFiltersToUpdate: ExceptionListItemSchema[] = []; + selectedArtifactIds.forEach((selectedId) => { + const eventFilter = eventFilters.data.find((current) => current.id === selectedId); + if (eventFilter) { + eventFilter.tags = [...eventFilter.tags, `policy:${policyItem.id}`]; + eventFiltersToUpdate.push(eventFilter); + } + }); + bulkUpdateMutation.mutate(eventFiltersToUpdate); + }, [bulkUpdateMutation, eventFilters, policyItem.id, selectedArtifactIds]); const handleSelectArtifacts = (artifactId: string, selected: boolean) => { setSelectedArtifactIds((currentSelectedArtifactIds) => @@ -58,17 +89,6 @@ export const PolicyEventFiltersFlyout = React.memo ( <> @@ -199,8 +219,7 @@ export const PolicyEventFiltersFlyout = React.memo diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts index 3eb33ba047105..0a755e27c10dc 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts @@ -4,12 +4,16 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { FoundExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; -import { QueryObserverResult, useQuery } from 'react-query'; +import pMap from 'p-map'; +import { + ExceptionListItemSchema, + FoundExceptionListItemSchema, +} from '@kbn/securitysolution-io-ts-list-types'; +import { QueryObserverResult, useMutation, useQuery, useQueryClient } from 'react-query'; import { ServerApiError } from '../../../../../common/types'; import { useHttp } from '../../../../../common/lib/kibana/hooks'; import { EventFiltersHttpService } from '../../../event_filters/service'; -import { parseQueryFilterToKQL } from '../../../../common/utils'; +import { parsePoliciesAndFilterToKql, parseQueryFilterToKQL } from '../../../../common/utils'; import { SEARCHABLE_FIELDS } from '../../../event_filters/constants'; export function useGetAllAssignedEventFilters( @@ -78,19 +82,12 @@ export function useSearchNotAssignedEventFilters( ['eventFilters', 'notAssigned', policyId, options], () => { const { filter } = options; - const kuery = [ - `((exception-list-agnostic.attributes.tags:"policy:${policyId}") OR (exception-list-agnostic.attributes.tags:"policy:all"))`, - ]; - - if (filter) { - const filterKuery = parseQueryFilterToKQL(filter, SEARCHABLE_FIELDS) || undefined; - if (filterKuery) { - kuery.push(filterKuery); - } - } return eventFiltersService.getList({ - filter: kuery.join(' AND '), + filter: parsePoliciesAndFilterToKql({ + excludedPolicies: [policyId, 'all'], + kuery: parseQueryFilterToKQL(filter || '', SEARCHABLE_FIELDS), + }), }); }, { @@ -102,6 +99,48 @@ export function useSearchNotAssignedEventFilters( ); } +export function useBulkUpdateEventFilters( + callbacks: { + onUpdateSuccess?: () => void; + onUpdateError?: () => void; + onSettledCallback?: () => void; + } = {} +) { + const http = useHttp(); + const eventFiltersService = new EventFiltersHttpService(http); + const queryClient = useQueryClient(); + + const { + onUpdateSuccess = () => {}, + onUpdateError = () => {}, + onSettledCallback = () => {}, + } = callbacks; + + return useMutation( + (eventFilters: ExceptionListItemSchema[]) => { + return pMap( + eventFilters, + (eventFilter) => { + return eventFiltersService.updateOne(eventFilter); + }, + { + concurrency: 5, + } + ); + }, + { + onSuccess: onUpdateSuccess, + onError: onUpdateError, + onSettled: () => { + queryClient.invalidateQueries(['notAssigned']); + queryClient.invalidateQueries(['assigned']); + queryClient.invalidateQueries(['eventFilters', 'all']); + onSettledCallback(); + }, + } + ); +} + export function useGetAllEventFilters(): QueryObserverResult< FoundExceptionListItemSchema, ServerApiError From d512fe786c2b516fc0ab2f4fcb7c48ebbace4a3b Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 23 Dec 2021 13:58:13 +0100 Subject: [PATCH 3/7] Shows confirm toast when action has ben done correctly --- .../flyout/policy_event_filters_flyout.tsx | 35 +++++++++++++++++-- .../pages/policy/view/event_filters/hooks.ts | 2 +- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx index 52fb228907107..0cfab821ba8aa 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx @@ -27,7 +27,7 @@ import { import { SearchExceptions } from '../../../../../components/search_exceptions'; import { ImmutableObject, PolicyData } from '../../../../../../../common/endpoint/types'; import { EventFiltersHttpService } from '../../../../event_filters/service'; -import { useHttp } from '../../../../../../common/lib/kibana'; +import { useHttp, useToasts } from '../../../../../../common/lib/kibana'; import { useSearchNotAssignedEventFilters, useGetAllAssignedEventFilters, @@ -44,12 +44,43 @@ const MAX_ALLOWED_RESULTS = 100; export const PolicyEventFiltersFlyout = React.memo( ({ policyItem, onClose }) => { + const toasts = useToasts(); const [selectedArtifactIds, setSelectedArtifactIds] = useState([]); const [currentFilter, setCurrentFilter] = useState(''); const http = useHttp(); const eventFiltersService = useMemo(() => new EventFiltersHttpService(http), [http]); - const bulkUpdateMutation = useBulkUpdateEventFilters(); + const bulkUpdateMutation = useBulkUpdateEventFilters({ + onUpdateSuccess: (updatedExceptions: ExceptionListItemSchema[]) => { + toasts.addSuccess({ + title: i18n.translate( + 'xpack.securitySolution.endpoint.policy.eventFilters.layout.flyout.toastSuccess.title', + { + defaultMessage: 'Success', + } + ), + text: + updatedExceptions.length > 1 + ? i18n.translate( + 'xpack.securitySolution.endpoint.policy.eventFilters.layout.flyout.toastSuccess.textMultiples', + { + defaultMessage: '{count} event filters have been added to your list.', + values: { count: updatedExceptions.length }, + } + ) + : i18n.translate( + 'xpack.securitySolution.endpoint.policy.eventFilters.layout.flyout.toastSuccess.textSingle', + { + defaultMessage: '"{name}" has been added to your event filters list.', + values: { name: updatedExceptions[0].name }, + } + ), + }); + }, + onUpdateError: () => {}, + onSettledCallback: () => {}, + }); + const { data: eventFilters, isLoading: isLoadingEventFilters } = useSearchNotAssignedEventFilters(eventFiltersService, policyItem.id, { filter: currentFilter, diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts index 0a755e27c10dc..9f65b4bc87d0f 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts @@ -101,7 +101,7 @@ export function useSearchNotAssignedEventFilters( export function useBulkUpdateEventFilters( callbacks: { - onUpdateSuccess?: () => void; + onUpdateSuccess?: (updatedExceptions: ExceptionListItemSchema[]) => void; onUpdateError?: () => void; onSettledCallback?: () => void; } = {} From 3fa7fdf53496a6133440ffef5cb649b6260e60b6 Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 23 Dec 2021 16:04:25 +0100 Subject: [PATCH 4/7] Fix ui/ux loading states and added unit test for flyout --- .../policy_artifacts_assignable_list.test.tsx | 2 +- .../policy_artifacts_assignable_list.tsx | 11 +- .../policy_event_filters_flyout.test.tsx | 271 ++++++++++++++++++ .../flyout/policy_event_filters_flyout.tsx | 60 ++-- .../pages/policy/view/event_filters/hooks.ts | 25 +- .../layout/policy_event_filters_layout.tsx | 42 ++- 6 files changed, 347 insertions(+), 64 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.test.tsx diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/artifacts/assignable/policy_artifacts_assignable_list.test.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/artifacts/assignable/policy_artifacts_assignable_list.test.tsx index a93ae878eb6dd..d10b44e5b2640 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/artifacts/assignable/policy_artifacts_assignable_list.test.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/artifacts/assignable/policy_artifacts_assignable_list.test.tsx @@ -41,7 +41,7 @@ describe('Policy artifacts list', () => { selectedArtifactsUpdated: selectedArtifactsUpdatedMock, }); - expect(component.getByTestId('loading-spinner')).not.toBeNull(); + expect(component.getByTestId('artifactsAssignableListLoader')).not.toBeNull(); }); it('should artifacts list without data', async () => { diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/artifacts/assignable/policy_artifacts_assignable_list.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/artifacts/assignable/policy_artifacts_assignable_list.tsx index 6b21f92f9ed87..ec2ec70b81dab 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/artifacts/assignable/policy_artifacts_assignable_list.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/artifacts/assignable/policy_artifacts_assignable_list.tsx @@ -5,10 +5,10 @@ * 2.0. */ +import { EuiProgress } from '@elastic/eui'; import { FoundExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; import React, { useMemo } from 'react'; import { GetTrustedAppsListResponse, Immutable } from '../../../../../../../common/endpoint/types'; -import { Loader } from '../../../../../../common/components/loader'; import { AnyArtifact, ArtifactEntryCardMinified, @@ -56,7 +56,14 @@ export const PolicyArtifactsAssignableList = React.memo :
{assignableList}
; + return ( + <> + {isListLoading && ( + + )} +
{assignableList}
+ + ); } ); diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.test.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.test.tsx new file mode 100644 index 0000000000000..dbc0d7eb638b3 --- /dev/null +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.test.tsx @@ -0,0 +1,271 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { act, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; +import uuid from 'uuid'; +import { getFoundExceptionListItemSchemaMock } from '../../../../../../../../lists/common/schemas/response/found_exception_list_item_schema.mock'; +import { getExceptionListItemSchemaMock } from '../../../../../../../../lists/common/schemas/response/exception_list_item_schema.mock'; +import { + AppContextTestRender, + createAppRootMockRenderer, +} from '../../../../../../common/mock/endpoint'; +import { EndpointDocGenerator } from '../../../../../../../common/endpoint/generate_data'; +import { PolicyData } from '../../../../../../../common/endpoint/types'; +import { getPolicyEventFiltersPath } from '../../../../../common/routing'; +import { eventFiltersListQueryHttpMock } from '../../../../event_filters/test_utils'; +import { PolicyEventFiltersFlyout } from './policy_event_filters_flyout'; +import { parseQueryFilterToKQL, parsePoliciesAndFilterToKql } from '../../../../../common/utils'; +import { SEARCHABLE_FIELDS } from '../../../../event_filters/constants'; +import { FoundExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; + +const endpointGenerator = new EndpointDocGenerator('seed'); +const getDefaultQueryParameters = (customFilter: string | undefined = '') => ({ + path: '/api/exception_lists/items/_find', + query: { + filter: customFilter, + list_id: ['endpoint_event_filters'], + namespace_type: ['agnostic'], + page: undefined, + per_page: 100, + sort_field: undefined, + sort_order: undefined, + }, +}); +const emptyList = { + data: [], + page: 1, + per_page: 10, + total: 0, +}; + +describe('Policy details event filters flyout', () => { + let render: () => Promise>; + let renderResult: ReturnType; + let history: AppContextTestRender['history']; + let mockedContext: AppContextTestRender; + let mockedApi: ReturnType; + let policy: PolicyData; + let onCloseMock: jest.Mock; + + beforeEach(() => { + policy = endpointGenerator.generatePolicyPackagePolicy(); + mockedContext = createAppRootMockRenderer(); + mockedApi = eventFiltersListQueryHttpMock(mockedContext.coreStart.http); + onCloseMock = jest.fn(); + ({ history } = mockedContext); + render = async () => { + await act(async () => { + renderResult = mockedContext.render( + + ); + await waitFor(mockedApi.responseProvider.eventFiltersList); + }); + return renderResult; + }; + + history.push(getPolicyEventFiltersPath(policy.id)); + }); + + it('should render a list of assignable policies and searchbar', async () => { + mockedApi.responseProvider.eventFiltersList.mockImplementation(() => { + return getFoundExceptionListItemSchemaMock(1); + }); + await render(); + expect(mockedApi.responseProvider.eventFiltersList).toHaveBeenLastCalledWith( + getDefaultQueryParameters( + parsePoliciesAndFilterToKql({ + excludedPolicies: [policy.id, 'all'], + kuery: parseQueryFilterToKQL('', SEARCHABLE_FIELDS), + }) + ) + ); + expect(await renderResult.findByTestId('artifactsList')).toBeTruthy(); + expect(renderResult.getByTestId('searchField')).toBeTruthy(); + }); + + it('should render "no items found" when searched for a term without data', async () => { + // first render + mockedApi.responseProvider.eventFiltersList.mockImplementationOnce(() => { + return getFoundExceptionListItemSchemaMock(1); + }); + await render(); + expect(await renderResult.findByTestId('artifactsList')).toBeTruthy(); + + // results for search + mockedApi.responseProvider.eventFiltersList.mockImplementationOnce(() => emptyList); + + // do a search + userEvent.type(renderResult.getByTestId('searchField'), 'no results with this{enter}'); + + await waitFor(() => { + expect(mockedApi.responseProvider.eventFiltersList).toHaveBeenCalledWith( + getDefaultQueryParameters( + parsePoliciesAndFilterToKql({ + excludedPolicies: [policy.id, 'all'], + kuery: parseQueryFilterToKQL('no results with this', SEARCHABLE_FIELDS), + }) + ) + ); + expect(renderResult.getByTestId('eventFilters-no-items-found')).toBeTruthy(); + }); + }); + + it('should render "not assignable items" when no possible exceptions can be assigned', async () => { + // both exceptions list requests will return no results + mockedApi.responseProvider.eventFiltersList.mockImplementation(() => emptyList); + await render(); + expect(await renderResult.findByTestId('eventFilters-no-assignable-items')).toBeTruthy(); + }); + + it('should disable the submit button if no exceptions are selected', async () => { + mockedApi.responseProvider.eventFiltersList.mockImplementationOnce(() => { + return getFoundExceptionListItemSchemaMock(1); + }); + await render(); + expect(await renderResult.findByTestId('artifactsList')).toBeTruthy(); + expect(renderResult.getByTestId('eventFilters-assign-confirm-button')).toBeDisabled(); + }); + + it('should enable the submit button if an exception is selected', async () => { + const exceptions = getFoundExceptionListItemSchemaMock(1); + const firstOneName = exceptions.data[0].name; + mockedApi.responseProvider.eventFiltersList.mockImplementation(() => exceptions); + + await render(); + expect(await renderResult.findByTestId('artifactsList')).toBeTruthy(); + + // click the first item + userEvent.click(renderResult.getByTestId(`${firstOneName}_checkbox`)); + + expect(renderResult.getByTestId('eventFilters-assign-confirm-button')).toBeEnabled(); + }); + + it('should warn the user when there are over 100 results in the flyout', async () => { + mockedApi.responseProvider.eventFiltersList.mockImplementation(() => { + return { + ...getFoundExceptionListItemSchemaMock(1), + total: 120, + }; + }); + await render(); + expect(await renderResult.findByTestId('artifactsList')).toBeTruthy(); + expect(renderResult.getByTestId('eventFilters-too-many-results')).toBeTruthy(); + }); + + describe('when submitting the form', () => { + const FIRST_ONE_NAME = uuid.v4(); + const SECOND_ONE_NAME = uuid.v4(); + const testTags = ['policy:1234', 'non-policy-tag', 'policy:4321']; + let exceptions: FoundExceptionListItemSchema; + + beforeEach(async () => { + exceptions = { + ...emptyList, + total: 2, + data: [ + getExceptionListItemSchemaMock({ + name: FIRST_ONE_NAME, + id: uuid.v4(), + tags: testTags, + }), + getExceptionListItemSchemaMock({ + name: SECOND_ONE_NAME, + id: uuid.v4(), + tags: testTags, + }), + ], + }; + mockedApi.responseProvider.eventFiltersList.mockImplementation(() => exceptions); + + await render(); + // wait fo the list to render + expect(await renderResult.findByTestId('artifactsList')).toBeTruthy(); + }); + + it('should submit the exception when submit is pressed (1 exception), display a toast and close the flyout', async () => { + mockedApi.responseProvider.eventFiltersUpdateOne.mockImplementation(() => exceptions.data[0]); + // click the first item + userEvent.click(renderResult.getByTestId(`${FIRST_ONE_NAME}_checkbox`)); + // submit the form + userEvent.click(renderResult.getByTestId('eventFilters-assign-confirm-button')); + + // verify the request with the new tag + await waitFor(() => { + expect(mockedApi.responseProvider.eventFiltersUpdateOne).toHaveBeenCalledWith({ + body: JSON.stringify({ + ...exceptions.data[0], + tags: [...testTags, `policy:${policy.id}`], + }), + path: '/api/exception_lists/items', + }); + }); + + await waitFor(() => { + expect(mockedContext.coreStart.notifications.toasts.addSuccess).toHaveBeenCalledWith({ + text: `"${FIRST_ONE_NAME}" has been added to your event filters list.`, + title: 'Success', + }); + }); + expect(onCloseMock).toHaveBeenCalled(); + }); + + it('should submit the exception when submit is pressed (2 exceptions), display a toast and close the flyout', async () => { + // click the first two items + userEvent.click(renderResult.getByTestId(`${FIRST_ONE_NAME}_checkbox`)); + userEvent.click(renderResult.getByTestId(`${SECOND_ONE_NAME}_checkbox`)); + // submit the form + userEvent.click(renderResult.getByTestId('eventFilters-assign-confirm-button')); + + // verify the request with the new tag + await waitFor(() => { + // first exception + expect(mockedApi.responseProvider.eventFiltersUpdateOne).toHaveBeenCalledWith({ + body: JSON.stringify({ + ...exceptions.data[0], + tags: [...testTags, `policy:${policy.id}`], + }), + path: '/api/exception_lists/items', + }); + // second exception + expect(mockedApi.responseProvider.eventFiltersUpdateOne).toHaveBeenCalledWith({ + body: JSON.stringify({ + ...exceptions.data[0], + tags: [...testTags, `policy:${policy.id}`], + }), + path: '/api/exception_lists/items', + }); + }); + + await waitFor(() => { + expect(mockedContext.coreStart.notifications.toasts.addSuccess).toHaveBeenCalledWith({ + text: '2 event filters have been added to your list.', + title: 'Success', + }); + }); + expect(onCloseMock).toHaveBeenCalled(); + }); + + it('should show a toast error when the request fails and close the flyout', async () => { + mockedApi.responseProvider.eventFiltersUpdateOne.mockImplementation(() => { + throw new Error('the server is too far away'); + }); + // click first item + userEvent.click(renderResult.getByTestId(`${FIRST_ONE_NAME}_checkbox`)); + // submit the form + userEvent.click(renderResult.getByTestId('eventFilters-assign-confirm-button')); + + await waitFor(() => { + expect(mockedContext.coreStart.notifications.toasts.addDanger).toHaveBeenCalledWith( + 'An error occurred updating artifacts' + ); + expect(onCloseMock).toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx index 0cfab821ba8aa..5c97b914bb1ca 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.tsx @@ -26,13 +26,8 @@ import { } from '@elastic/eui'; import { SearchExceptions } from '../../../../../components/search_exceptions'; import { ImmutableObject, PolicyData } from '../../../../../../../common/endpoint/types'; -import { EventFiltersHttpService } from '../../../../event_filters/service'; -import { useHttp, useToasts } from '../../../../../../common/lib/kibana'; -import { - useSearchNotAssignedEventFilters, - useGetAllAssignedEventFilters, - useBulkUpdateEventFilters, -} from '../hooks'; +import { useToasts } from '../../../../../../common/lib/kibana'; +import { useSearchNotAssignedEventFilters, useBulkUpdateEventFilters } from '../hooks'; import { PolicyArtifactsAssignableList } from '../../artifacts/assignable'; interface PolicyEventFiltersFlyoutProps { @@ -47,8 +42,6 @@ export const PolicyEventFiltersFlyout = React.memo([]); const [currentFilter, setCurrentFilter] = useState(''); - const http = useHttp(); - const eventFiltersService = useMemo(() => new EventFiltersHttpService(http), [http]); const bulkUpdateMutation = useBulkUpdateEventFilters({ onUpdateSuccess: (updatedExceptions: ExceptionListItemSchema[]) => { @@ -77,20 +70,32 @@ export const PolicyEventFiltersFlyout = React.memo {}, - onSettledCallback: () => {}, + onUpdateError: () => { + toasts.addDanger( + i18n.translate( + 'xpack.securitySolution.endpoint.policy.eventFilters.layout.flyout.toastError.text', + { + defaultMessage: `An error occurred updating artifacts`, + } + ) + ); + }, + onSettledCallback: onClose, }); - const { data: eventFilters, isLoading: isLoadingEventFilters } = - useSearchNotAssignedEventFilters(eventFiltersService, policyItem.id, { - filter: currentFilter, - }); + const { + data: eventFilters, + isLoading: isLoadingEventFilters, + isRefetching: isRefetchingEventFilters, + } = useSearchNotAssignedEventFilters(policyItem.id, { + perPage: MAX_ALLOWED_RESULTS, + filter: currentFilter, + }); - const { data: allAssigned, isLoading: isLoadingAllAssigned } = useGetAllAssignedEventFilters( - policyItem.id, - currentFilter !== '' && eventFilters?.total === 0, - 'assignmentFlyout' - ); + const { data: allNotAssigned, isLoading: isLoadingAllNotAssigned } = + useSearchNotAssignedEventFilters(policyItem.id, { + enabled: currentFilter !== '' && eventFilters?.total === 0, + }); const handleOnSearch = useCallback((query) => { setSelectedArtifactIds([]); @@ -150,12 +155,12 @@ export const PolicyEventFiltersFlyout = React.memo { - if (isLoadingEventFilters || isLoadingAllAssigned) { + if (isLoadingEventFilters || isRefetchingEventFilters || isLoadingAllNotAssigned) { return null; } // there are no event filters assignable to this policy - if (allAssigned?.total === 0 || (eventFilters?.total === 0 && currentFilter === '')) { + if (allNotAssigned?.total === 0 || (eventFilters?.total === 0 && currentFilter === '')) { return ( @@ -250,7 +256,9 @@ export const PolicyEventFiltersFlyout = React.memo diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts index 6b7c862aa892c..ce19e0d4cfcc0 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts @@ -38,7 +38,9 @@ export function useGetAllAssignedEventFilters( { refetchIntervalInBackground: false, refetchOnWindowFocus: false, + refetchOnMount: true, enabled, + keepPreviousData: true, } ); } @@ -72,27 +74,29 @@ export function useSearchAssignedEventFilters( } export function useSearchNotAssignedEventFilters( - eventFiltersService: EventFiltersHttpService, policyId: string, - options: { filter?: string } + options: { filter?: string; perPage?: number; enabled?: boolean } ): QueryObserverResult { + const service = useGetEventFiltersService(); return useQuery( ['eventFilters', 'notAssigned', policyId, options], () => { - const { filter } = options; + const { filter, perPage } = options; - return eventFiltersService.getList({ + return service.getList({ filter: parsePoliciesAndFilterToKql({ excludedPolicies: [policyId, 'all'], kuery: parseQueryFilterToKQL(filter || '', SEARCHABLE_FIELDS), }), + perPage, }); }, { refetchIntervalInBackground: false, refetchOnWindowFocus: false, - enabled: !!policyId, refetchOnMount: true, + keepPreviousData: true, + enabled: options.enabled ?? true, } ); } @@ -104,8 +108,7 @@ export function useBulkUpdateEventFilters( onSettledCallback?: () => void; } = {} ) { - const http = useHttp(); - const eventFiltersService = new EventFiltersHttpService(http); + const service = useGetEventFiltersService(); const queryClient = useQueryClient(); const { @@ -119,7 +122,7 @@ export function useBulkUpdateEventFilters( return pMap( eventFilters, (eventFilter) => { - return eventFiltersService.updateOne(eventFilter); + return service.updateOne(eventFilter); }, { concurrency: 5, @@ -130,9 +133,8 @@ export function useBulkUpdateEventFilters( onSuccess: onUpdateSuccess, onError: onUpdateError, onSettled: () => { - queryClient.invalidateQueries(['notAssigned']); - queryClient.invalidateQueries(['assigned']); - queryClient.invalidateQueries(['eventFilters', 'all']); + queryClient.invalidateQueries(['eventFilters', 'notAssigned']); + queryClient.invalidateQueries(['eventFilters', 'assigned']); onSettledCallback(); }, } @@ -153,6 +155,7 @@ export function useGetAllEventFilters(): QueryObserverResult< refetchIntervalInBackground: false, refetchOnWindowFocus: false, refetchOnMount: true, + keepPreviousData: true, } ); } diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/layout/policy_event_filters_layout.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/layout/policy_event_filters_layout.tsx index b14d04227d882..a7091f6d92229 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/layout/policy_event_filters_layout.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/layout/policy_event_filters_layout.tsx @@ -25,7 +25,10 @@ import { getEventFiltersListPath } from '../../../../../common/routing'; import { useGetAllAssignedEventFilters, useGetAllEventFilters } from '../hooks'; import { ManagementPageLoader } from '../../../../../components/management_page_loader'; import { PolicyEventFiltersEmptyUnassigned, PolicyEventFiltersEmptyUnexisting } from '../empty'; -import { usePolicyDetailsSelector } from '../../policy_hooks'; +import { + usePolicyDetailsSelector, + usePolicyDetailsEventFiltersNavigateCallback, +} from '../../policy_hooks'; import { getCurrentArtifactsLocation } from '../../../store/policy_details/selectors'; import { PolicyEventFiltersFlyout } from '../flyout'; import { PolicyEventFiltersList } from '../list'; @@ -36,22 +39,22 @@ interface PolicyEventFiltersLayoutProps { export const PolicyEventFiltersLayout = React.memo( ({ policyItem }) => { const { getAppUrl } = useAppUrl(); + const navigateCallback = usePolicyDetailsEventFiltersNavigateCallback(); const urlParams = usePolicyDetailsSelector(getCurrentArtifactsLocation); - const { - data: allAssigned, - isLoading: isLoadingAllAssigned, - isRefetching: isRefetchingAllAssigned, - } = useGetAllAssignedEventFilters(policyItem?.id || '', !!policyItem?.id); + const { data: allAssigned, isLoading: isLoadingAllAssigned } = useGetAllAssignedEventFilters( + policyItem?.id || '', + !!policyItem?.id + ); - const { - data: allEventFilters, - isLoading: isLoadingAllEventFilters, - isRefetching: isRefetchingAllEventFilters, - } = useGetAllEventFilters(); + const { data: allEventFilters, isLoading: isLoadingAllEventFilters } = useGetAllEventFilters(); - const handleOnClickAssignButton = useCallback(() => {}, []); - const handleOnCloseFlyout = useCallback(() => {}, []); + const handleOnClickAssignButton = useCallback(() => { + navigateCallback({ show: 'list' }); + }, [navigateCallback]); + const handleOnCloseFlyout = useCallback(() => { + navigateCallback({ show: undefined }); + }, [navigateCallback]); const aboutInfo = useMemo(() => { const link = ( @@ -79,17 +82,8 @@ export const PolicyEventFiltersLayout = React.memo - isLoadingAllAssigned || - isRefetchingAllAssigned || - isLoadingAllEventFilters || - isRefetchingAllEventFilters, - [ - isLoadingAllAssigned, - isLoadingAllEventFilters, - isRefetchingAllAssigned, - isRefetchingAllEventFilters, - ] + () => isLoadingAllAssigned || isLoadingAllEventFilters, + [isLoadingAllAssigned, isLoadingAllEventFilters] ); const isEmptyState = useMemo(() => allAssigned && allAssigned.total === 0, [allAssigned]); From 776ed8e554cb7ace4ac0cb146bcebf8619041951 Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Thu, 23 Dec 2021 16:09:51 +0100 Subject: [PATCH 5/7] Remove customqueryId --- .../management/pages/policy/view/event_filters/hooks.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts index ce19e0d4cfcc0..571dd8779d093 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/hooks.ts @@ -24,12 +24,11 @@ export function useGetEventFiltersService() { export function useGetAllAssignedEventFilters( policyId: string, - enabled: boolean = true, - customQueryId: string = '' + enabled: boolean = true ): QueryObserverResult { const service = useGetEventFiltersService(); return useQuery( - ['eventFilters', 'assigned', policyId, customQueryId], + ['eventFilters', 'assigned', policyId], () => { return service.getList({ filter: parsePoliciesAndFilterToKql({ policies: [...(policyId ? [policyId] : []), 'all'] }), From ddf4d36db2c2eaf88b4c0f0172710fba5e60576f Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Tue, 4 Jan 2022 09:57:15 +0100 Subject: [PATCH 6/7] Use new object reference befor update it --- .../public/management/pages/event_filters/service/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/event_filters/service/index.ts b/x-pack/plugins/security_solution/public/management/pages/event_filters/service/index.ts index 70ac198dd3478..a4ac378610e49 100644 --- a/x-pack/plugins/security_solution/public/management/pages/event_filters/service/index.ts +++ b/x-pack/plugins/security_solution/public/management/pages/event_filters/service/index.ts @@ -90,6 +90,7 @@ export class EventFiltersHttpService implements EventFiltersService { async updateOne( exception: Immutable ): Promise { + const exceptionToUpdateCleaned = { ...exception }; // Clean unnecessary fields for update action [ 'created_at', @@ -101,10 +102,10 @@ export class EventFiltersHttpService implements EventFiltersService { 'updated_at', 'updated_by', ].forEach((field) => { - delete exception[field as keyof UpdateExceptionListItemSchema]; + delete exceptionToUpdateCleaned[field as keyof UpdateExceptionListItemSchema]; }); return (await this.httpWrapper()).put(EXCEPTION_LIST_ITEM_URL, { - body: JSON.stringify(exception), + body: JSON.stringify(exceptionToUpdateCleaned), }); } From 6a23f6db92f25938c66b6b793464c7ab85c5b582 Mon Sep 17 00:00:00 2001 From: David Sanchez Soler Date: Tue, 4 Jan 2022 11:05:56 +0100 Subject: [PATCH 7/7] Fixes unit test --- .../policy_event_filters_flyout.test.tsx | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.test.tsx b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.test.tsx index dbc0d7eb638b3..092a787d1f7e0 100644 --- a/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.test.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/policy/view/event_filters/flyout/policy_event_filters_flyout.test.tsx @@ -22,7 +22,10 @@ import { eventFiltersListQueryHttpMock } from '../../../../event_filters/test_ut import { PolicyEventFiltersFlyout } from './policy_event_filters_flyout'; import { parseQueryFilterToKQL, parsePoliciesAndFilterToKql } from '../../../../../common/utils'; import { SEARCHABLE_FIELDS } from '../../../../event_filters/constants'; -import { FoundExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types'; +import { + FoundExceptionListItemSchema, + UpdateExceptionListItemSchema, +} from '@kbn/securitysolution-io-ts-list-types'; const endpointGenerator = new EndpointDocGenerator('seed'); const getDefaultQueryParameters = (customFilter: string | undefined = '') => ({ @@ -44,6 +47,31 @@ const emptyList = { total: 0, }; +const getCleanedExceptionWithNewTags = ( + exception: UpdateExceptionListItemSchema, + testTags: string[], + policy: PolicyData +) => { + const exceptionToUpdateCleaned = { + ...exception, + tags: [...testTags, `policy:${policy.id}`], + }; + // Clean unnecessary fields for update action + [ + 'created_at', + 'created_by', + 'created_at', + 'created_by', + 'list_id', + 'tie_breaker_id', + 'updated_at', + 'updated_by', + ].forEach((field) => { + delete exceptionToUpdateCleaned[field as keyof UpdateExceptionListItemSchema]; + }); + return exceptionToUpdateCleaned; +}; + describe('Policy details event filters flyout', () => { let render: () => Promise>; let renderResult: ReturnType; @@ -198,10 +226,9 @@ describe('Policy details event filters flyout', () => { // verify the request with the new tag await waitFor(() => { expect(mockedApi.responseProvider.eventFiltersUpdateOne).toHaveBeenCalledWith({ - body: JSON.stringify({ - ...exceptions.data[0], - tags: [...testTags, `policy:${policy.id}`], - }), + body: JSON.stringify( + getCleanedExceptionWithNewTags(exceptions.data[0], testTags, policy) + ), path: '/api/exception_lists/items', }); }); @@ -226,18 +253,16 @@ describe('Policy details event filters flyout', () => { await waitFor(() => { // first exception expect(mockedApi.responseProvider.eventFiltersUpdateOne).toHaveBeenCalledWith({ - body: JSON.stringify({ - ...exceptions.data[0], - tags: [...testTags, `policy:${policy.id}`], - }), + body: JSON.stringify( + getCleanedExceptionWithNewTags(exceptions.data[0], testTags, policy) + ), path: '/api/exception_lists/items', }); // second exception expect(mockedApi.responseProvider.eventFiltersUpdateOne).toHaveBeenCalledWith({ - body: JSON.stringify({ - ...exceptions.data[0], - tags: [...testTags, `policy:${policy.id}`], - }), + body: JSON.stringify( + getCleanedExceptionWithNewTags(exceptions.data[0], testTags, policy) + ), path: '/api/exception_lists/items', }); });