From 07e448d34393ca85916eec23ecfcb52e3969aadb Mon Sep 17 00:00:00 2001 From: oatkiller Date: Tue, 18 Feb 2020 17:00:16 -0500 Subject: [PATCH 01/23] add a flyout --- x-pack/plugins/endpoint/common/types.ts | 2 +- .../endpoint/store/alerts/middleware.ts | 32 +++++ .../endpoint/store/alerts/selectors.ts | 84 +++++++++-- .../public/applications/endpoint/types.ts | 7 + .../endpoint/view/alerts/index.tsx | 130 ++++++++++++++---- 5 files changed, 212 insertions(+), 43 deletions(-) diff --git a/x-pack/plugins/endpoint/common/types.ts b/x-pack/plugins/endpoint/common/types.ts index f0fd9dc610e4e..78be98b7805ba 100644 --- a/x-pack/plugins/endpoint/common/types.ts +++ b/x-pack/plugins/endpoint/common/types.ts @@ -71,7 +71,7 @@ export interface EndpointResultList { } export interface AlertData { - '@timestamp': Date; + '@timestamp': string; agent: { id: string; version: string; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts index 059507c8f0658..015b9644ac25d 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts @@ -15,10 +15,42 @@ export const alertMiddlewareFactory: MiddlewareFactory = coreSta next(action); const state = api.getState(); if (action.type === 'userChangedUrl' && isOnAlertPage(state)) { + const response: AlertResultList = { + total: 1, + request_page_size: 1, + request_page_index: 0, + result_from_index: 0, + alerts: [ + { + '@timestamp': new Date().toString(), + agent: { id: '', version: '' }, + event: { + action: '', + }, + file_classification: { + malware_classification: { + score: 1, + }, + }, + host: { + hostname: '', + ip: '', + os: { + name: '', + }, + }, + thread: {}, + }, + ], + }; + api.dispatch({ type: 'serverReturnedAlertsData', payload: response }); + /* + * TODO dont commit this file const response: AlertResultList = await coreStart.http.get(`/api/endpoint/alerts`, { query: paginationDataFromUrl(state) as HttpFetchQuery, }); api.dispatch({ type: 'serverReturnedAlertsData', payload: response }); + */ } }; }; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts index 6ad053888729c..1afaab7b59607 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts @@ -5,7 +5,9 @@ */ import qs from 'querystring'; -import { AlertListState } from '../../types'; +import { createSelector } from 'reselect'; +import { Immutable } from '../../../../../common/types'; +import { AlertListState, AlertIndexQueryParams } from '../../types'; /** * Returns the Alert Data array from state @@ -32,20 +34,41 @@ export const isOnAlertPage = (state: AlertListState): boolean => { }; /** - * Returns the query object received from parsing the URL query params + * Returns the query object received from parsing the URL query params. + * The query value passed when requesting alert list data from the server. + * Also used to get new client side URLs. */ -export const paginationDataFromUrl = (state: AlertListState): qs.ParsedUrlQuery => { - if (state.location) { - // Removes the `?` from the beginning of query string if it exists - const query = qs.parse(state.location.search.slice(1)); - return { - ...(query.page_size ? { page_size: query.page_size } : {}), - ...(query.page_index ? { page_index: query.page_index } : {}), - }; - } else { - return {}; +// TODO rename to alertIndexQueryParams +export const paginationDataFromUrl: ( + state: AlertListState +) => Immutable = createSelector( + (state: AlertListState) => state.location, + (location: AlertListState['location']) => { + const data: AlertIndexQueryParams = {}; + if (location) { + // Removes the `?` from the beginning of query string if it exists + const query = qs.parse(location.search.slice(1)); + if (typeof query.page_size === 'string') { + data.page_size = query.page_size; + } else if (Array.isArray(query.page_size)) { + data.page_size = query.page_size[query.page_size.length - 1]; + } + + if (typeof query.page_index === 'string') { + data.page_index = query.page_index; + } else if (Array.isArray(query.page_index)) { + data.page_index = query.page_index[query.page_index.length - 1]; + } + + if (typeof query.selected_alert === 'string') { + data.selected_alert = query.selected_alert; + } else if (Array.isArray(query.selected_alert)) { + data.selected_alert = query.selected_alert[query.selected_alert.length - 1]; + } + } + return data; } -}; +); /** * Returns a function that takes in a new page size and returns a new query param string @@ -54,7 +77,7 @@ export const urlFromNewPageSizeParam: ( state: AlertListState ) => (newPageSize: number) => string = state => { return newPageSize => { - const urlPaginationData = paginationDataFromUrl(state); + const urlPaginationData: AlertIndexQueryParams = { ...paginationDataFromUrl(state) }; urlPaginationData.page_size = newPageSize.toString(); // Only set the url back to page zero if the user has changed the page index already @@ -72,8 +95,39 @@ export const urlFromNewPageIndexParam: ( state: AlertListState ) => (newPageIndex: number) => string = state => { return newPageIndex => { - const urlPaginationData = paginationDataFromUrl(state); + const urlPaginationData: AlertIndexQueryParams = { ...paginationDataFromUrl(state) }; urlPaginationData.page_index = newPageIndex.toString(); return '?' + qs.stringify(urlPaginationData); }; }; + +/** + * Returns a url like the current one, but with a new alert id. + */ +export const urlWithSelectedAlert: ( + state: AlertListState +) => (alertID: string) => string = state => { + return (alertID: string) => { + const urlPaginationData = { ...paginationDataFromUrl(state) }; + urlPaginationData.selected_alert = alertID; + return '?' + qs.stringify(urlPaginationData); + }; +}; + +/** + * Returns a url like the current one, but with no alert id + */ +export const urlWithoutSelectedAlert: (state: AlertListState) => string = createSelector( + paginationDataFromUrl, + urlPaginationData => { + // TODO, different pattern for calculating URL w/ and w/o qs values + const newUrlPaginationData = { ...urlPaginationData }; + delete newUrlPaginationData.selected_alert; + return '?' + qs.stringify(newUrlPaginationData); + } +); + +export const hasSelectedAlert: (state: AlertListState) => boolean = createSelector( + paginationDataFromUrl, + ({ selected_alert: selectedAlert }) => selectedAlert !== undefined +); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts index d07521d09a119..cc05241dd010d 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts @@ -85,3 +85,10 @@ export type AlertListData = AlertResultList; export type AlertListState = Immutable & { readonly location?: Immutable; }; + +export interface AlertIndexQueryParams { + page_size?: string; + page_index?: string; + // TODO, reference alert event id type directly + selected_alert?: string; +} diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index 045b82200b11b..e96e9acdd63a0 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -6,7 +6,19 @@ import { memo, useState, useMemo, useCallback } from 'react'; import React from 'react'; -import { EuiDataGrid, EuiDataGridColumn, EuiPage, EuiPageBody, EuiPageContent } from '@elastic/eui'; +import { + EuiLink, + EuiDataGrid, + EuiDataGridColumn, + EuiPage, + EuiPageBody, + EuiPageContent, + EuiFlyout, + EuiFlyoutHeader, + EuiFlyoutBody, + EuiTitle, + EuiBadge, +} from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { useHistory } from 'react-router-dom'; import * as selectors from '../../store/alerts/selectors'; @@ -15,7 +27,7 @@ import { useAlertListSelector } from './hooks/use_alerts_selector'; export const AlertIndex = memo(() => { const history = useHistory(); - const columns: EuiDataGridColumn[] = useMemo(() => { + const columns = useMemo((): EuiDataGridColumn[] => { return [ { id: 'alert_type', @@ -68,10 +80,14 @@ export const AlertIndex = memo(() => { ]; }, []); + // TODO consider structuredSelector const { pageIndex, pageSize, total } = useAlertListSelector(selectors.alertListPagination); const urlFromNewPageSizeParam = useAlertListSelector(selectors.urlFromNewPageSizeParam); + const urlWithSelectedAlert = useAlertListSelector(selectors.urlWithSelectedAlert); + const urlWithoutSelectedAlert = useAlertListSelector(selectors.urlWithoutSelectedAlert); const urlFromNewPageIndexParam = useAlertListSelector(selectors.urlFromNewPageIndexParam); const alertListData = useAlertListSelector(selectors.alertListData); + const hasSelectedAlert = useAlertListSelector(selectors.hasSelectedAlert); const onChangeItemsPerPage = useCallback( newPageSize => history.push(urlFromNewPageSizeParam(newPageSize)), @@ -84,6 +100,30 @@ export const AlertIndex = memo(() => { ); const [visibleColumns, setVisibleColumns] = useState(() => columns.map(({ id }) => id)); + const formatter = new Intl.DateTimeFormat(i18n.getLocale(), { + year: 'numeric', + month: '2-digit', + day: '2-digit', + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + }); + + const handleAlertClick = useCallback( + (event: React.MouseEvent) => { + if (event.target instanceof HTMLElement) { + const alertId: string | undefined = event.target.dataset.alertId; + if (alertId !== undefined) { + history.push(urlWithSelectedAlert(alertId)); + } + } + }, + [history, urlWithSelectedAlert] + ); + + const handleFlyoutClose = useCallback(() => { + history.push(urlWithoutSelectedAlert); + }, [history, urlWithoutSelectedAlert]); const renderCellValue = useMemo(() => { return ({ rowIndex, columnId }: { rowIndex: number; columnId: string }) => { @@ -94,11 +134,16 @@ export const AlertIndex = memo(() => { const row = alertListData[rowIndex % pageSize]; if (columnId === 'alert_type') { - return i18n.translate( - 'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription', - { - defaultMessage: 'Malicious File', - } + return ( + + {/* TODO populate data-alert-id with something real */} + {i18n.translate( + 'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription', + { + defaultMessage: 'Malicious File', + } + )} + ); } else if (columnId === 'event_type') { return row.event.action; @@ -109,7 +154,21 @@ export const AlertIndex = memo(() => { } else if (columnId === 'host_name') { return row.host.hostname; } else if (columnId === 'timestamp') { - return row['@timestamp']; + const date = new Date(row['@timestamp']); + if (isFinite(date.getTime())) { + return formatter.format(date); + } else { + return ( + + {i18n.translate( + 'xpack.endpoint.application.endpoint.alerts.alertDate.timestampInvalidLabel', + { + defaultMessage: 'invalid', + } + )} + + ); + } } else if (columnId === 'archived') { return null; } else if (columnId === 'malware_score') { @@ -117,7 +176,7 @@ export const AlertIndex = memo(() => { } return null; }; - }, [alertListData, pageSize, total]); + }, [alertListData, formatter, handleAlertClick, pageSize, total]); const pagination = useMemo(() => { return { @@ -130,23 +189,40 @@ export const AlertIndex = memo(() => { }, [onChangeItemsPerPage, onChangePage, pageIndex, pageSize]); return ( - - - - - - - + <> + {/* + TODO, rethink this. we may already have this in state. we still need `hasSelectedAlert`, to know to show this flyout. we should also have `selectedAlert`, which will eventually be loaded from server. */} + {hasSelectedAlert && ( + + + +

+ alert detailz + {/* TODO, make an issue to add logic to get selected alert. it might already be in state! */} +

+
+
+ hey! +
+ )} + + + + + + + + ); }); From e7c891cc84a9db4259cccddf69d95fcfdb9098a1 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Tue, 18 Feb 2020 19:53:03 -0500 Subject: [PATCH 02/23] Removing TODOs, added issue https://github.com/elastic/endpoint-app-team/issues/189 --- x-pack/plugins/endpoint/public/applications/endpoint/types.ts | 1 - .../endpoint/public/applications/endpoint/view/alerts/index.tsx | 1 - 2 files changed, 2 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts index cc05241dd010d..f5f263fa4f026 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts @@ -89,6 +89,5 @@ export type AlertListState = Immutable & { export interface AlertIndexQueryParams { page_size?: string; page_index?: string; - // TODO, reference alert event id type directly selected_alert?: string; } diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index e96e9acdd63a0..72da6d95d367f 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -136,7 +136,6 @@ export const AlertIndex = memo(() => { if (columnId === 'alert_type') { return ( - {/* TODO populate data-alert-id with something real */} {i18n.translate( 'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription', { From 3d6f4ab4d51b443ed9fa944c505ef028c06c3c2e Mon Sep 17 00:00:00 2001 From: oatkiller Date: Tue, 18 Feb 2020 19:56:16 -0500 Subject: [PATCH 03/23] Rename `queryParams` selector --- .../store/alerts/alert_list_pagination.test.ts | 14 +++++--------- .../endpoint/store/alerts/middleware.ts | 2 +- .../endpoint/store/alerts/selectors.ts | 13 ++++++------- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts index 77708a3c77e2b..d1ab77c6a437a 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts @@ -12,11 +12,7 @@ import { alertMiddlewareFactory } from './middleware'; import { AppAction } from '../action'; import { coreMock } from 'src/core/public/mocks'; import { createBrowserHistory } from 'history'; -import { - urlFromNewPageSizeParam, - paginationDataFromUrl, - urlFromNewPageIndexParam, -} from './selectors'; +import { urlFromNewPageSizeParam, queryParams, urlFromNewPageIndexParam } from './selectors'; describe('alert list pagination', () => { let store: Store; @@ -36,7 +32,7 @@ describe('alert list pagination', () => { store.dispatch({ type: 'userChangedUrl', payload: history.location }); }); it('should modify the url correctly', () => { - const actualPaginationQuery = paginationDataFromUrl(store.getState()); + const actualPaginationQuery = queryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { "page_size": "1", @@ -51,7 +47,7 @@ describe('alert list pagination', () => { store.dispatch({ type: 'userChangedUrl', payload: history.location }); }); it('should modify the url in the correct order', () => { - const actualPaginationQuery = paginationDataFromUrl(store.getState()); + const actualPaginationQuery = queryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { "page_index": "1", @@ -69,7 +65,7 @@ describe('alert list pagination', () => { store.dispatch({ type: 'userChangedUrl', payload: history.location }); }); it('should modify the url correctly', () => { - const actualPaginationQuery = paginationDataFromUrl(store.getState()); + const actualPaginationQuery = queryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { "page_index": "1", @@ -84,7 +80,7 @@ describe('alert list pagination', () => { store.dispatch({ type: 'userChangedUrl', payload: history.location }); }); it('should modify the url correctly and reset index to `0`', () => { - const actualPaginationQuery = paginationDataFromUrl(store.getState()); + const actualPaginationQuery = queryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { "page_index": "0", diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts index 015b9644ac25d..2e63100d021f4 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts @@ -8,7 +8,7 @@ import { HttpFetchQuery } from 'kibana/public'; import { AlertResultList } from '../../../../../common/types'; import { AppAction } from '../action'; import { MiddlewareFactory, AlertListState } from '../../types'; -import { isOnAlertPage, paginationDataFromUrl } from './selectors'; +import { isOnAlertPage, queryParams } from './selectors'; export const alertMiddlewareFactory: MiddlewareFactory = coreStart => { return api => next => async (action: AppAction) => { diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts index 1afaab7b59607..511d0138d4789 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts @@ -38,8 +38,7 @@ export const isOnAlertPage = (state: AlertListState): boolean => { * The query value passed when requesting alert list data from the server. * Also used to get new client side URLs. */ -// TODO rename to alertIndexQueryParams -export const paginationDataFromUrl: ( +export const queryParams: ( state: AlertListState ) => Immutable = createSelector( (state: AlertListState) => state.location, @@ -77,7 +76,7 @@ export const urlFromNewPageSizeParam: ( state: AlertListState ) => (newPageSize: number) => string = state => { return newPageSize => { - const urlPaginationData: AlertIndexQueryParams = { ...paginationDataFromUrl(state) }; + const urlPaginationData: AlertIndexQueryParams = { ...queryParams(state) }; urlPaginationData.page_size = newPageSize.toString(); // Only set the url back to page zero if the user has changed the page index already @@ -95,7 +94,7 @@ export const urlFromNewPageIndexParam: ( state: AlertListState ) => (newPageIndex: number) => string = state => { return newPageIndex => { - const urlPaginationData: AlertIndexQueryParams = { ...paginationDataFromUrl(state) }; + const urlPaginationData: AlertIndexQueryParams = { ...queryParams(state) }; urlPaginationData.page_index = newPageIndex.toString(); return '?' + qs.stringify(urlPaginationData); }; @@ -108,7 +107,7 @@ export const urlWithSelectedAlert: ( state: AlertListState ) => (alertID: string) => string = state => { return (alertID: string) => { - const urlPaginationData = { ...paginationDataFromUrl(state) }; + const urlPaginationData = { ...queryParams(state) }; urlPaginationData.selected_alert = alertID; return '?' + qs.stringify(urlPaginationData); }; @@ -118,7 +117,7 @@ export const urlWithSelectedAlert: ( * Returns a url like the current one, but with no alert id */ export const urlWithoutSelectedAlert: (state: AlertListState) => string = createSelector( - paginationDataFromUrl, + queryParams, urlPaginationData => { // TODO, different pattern for calculating URL w/ and w/o qs values const newUrlPaginationData = { ...urlPaginationData }; @@ -128,6 +127,6 @@ export const urlWithoutSelectedAlert: (state: AlertListState) => string = create ); export const hasSelectedAlert: (state: AlertListState) => boolean = createSelector( - paginationDataFromUrl, + queryParams, ({ selected_alert: selectedAlert }) => selectedAlert !== undefined ); From 4aeeba74aadeabd22927bf941b56cc9fe0d49bc5 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Tue, 18 Feb 2020 20:00:49 -0500 Subject: [PATCH 04/23] Removing TODOs, added issue https://github.com/elastic/endpoint-app-team/issues/190 --- .../applications/endpoint/store/alerts/selectors.ts | 1 - .../public/applications/endpoint/view/alerts/index.tsx | 9 ++------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts index 511d0138d4789..56bd03c7e4eed 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts @@ -119,7 +119,6 @@ export const urlWithSelectedAlert: ( export const urlWithoutSelectedAlert: (state: AlertListState) => string = createSelector( queryParams, urlPaginationData => { - // TODO, different pattern for calculating URL w/ and w/o qs values const newUrlPaginationData = { ...urlPaginationData }; delete newUrlPaginationData.selected_alert; return '?' + qs.stringify(newUrlPaginationData); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index 72da6d95d367f..ef70e0230c5d4 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -189,19 +189,14 @@ export const AlertIndex = memo(() => { return ( <> - {/* - TODO, rethink this. we may already have this in state. we still need `hasSelectedAlert`, to know to show this flyout. we should also have `selectedAlert`, which will eventually be loaded from server. */} {hasSelectedAlert && ( -

- alert detailz - {/* TODO, make an issue to add logic to get selected alert. it might already be in state! */} -

+

Alert Details

- hey! +
)} From 634027077eec133026eed2f4ed49a5fd0b6e7340 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Tue, 18 Feb 2020 20:03:16 -0500 Subject: [PATCH 05/23] Removed TODOs, added issue https://github.com/elastic/endpoint-app-team/issues/191 --- .../endpoint/public/applications/endpoint/view/alerts/index.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index ef70e0230c5d4..75809e088e38b 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -80,7 +80,6 @@ export const AlertIndex = memo(() => { ]; }, []); - // TODO consider structuredSelector const { pageIndex, pageSize, total } = useAlertListSelector(selectors.alertListPagination); const urlFromNewPageSizeParam = useAlertListSelector(selectors.urlFromNewPageSizeParam); const urlWithSelectedAlert = useAlertListSelector(selectors.urlWithSelectedAlert); From 26c3ca6776cc5bb3934b474d6b4e6ef46f845cf6 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Tue, 18 Feb 2020 20:06:50 -0500 Subject: [PATCH 06/23] Removed sample code And created issue: https://github.com/elastic/endpoint-app-team/issues/192 --- .../endpoint/store/alerts/middleware.ts | 34 +------------------ 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts index 2e63100d021f4..059507c8f0658 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts @@ -8,49 +8,17 @@ import { HttpFetchQuery } from 'kibana/public'; import { AlertResultList } from '../../../../../common/types'; import { AppAction } from '../action'; import { MiddlewareFactory, AlertListState } from '../../types'; -import { isOnAlertPage, queryParams } from './selectors'; +import { isOnAlertPage, paginationDataFromUrl } from './selectors'; export const alertMiddlewareFactory: MiddlewareFactory = coreStart => { return api => next => async (action: AppAction) => { next(action); const state = api.getState(); if (action.type === 'userChangedUrl' && isOnAlertPage(state)) { - const response: AlertResultList = { - total: 1, - request_page_size: 1, - request_page_index: 0, - result_from_index: 0, - alerts: [ - { - '@timestamp': new Date().toString(), - agent: { id: '', version: '' }, - event: { - action: '', - }, - file_classification: { - malware_classification: { - score: 1, - }, - }, - host: { - hostname: '', - ip: '', - os: { - name: '', - }, - }, - thread: {}, - }, - ], - }; - api.dispatch({ type: 'serverReturnedAlertsData', payload: response }); - /* - * TODO dont commit this file const response: AlertResultList = await coreStart.http.get(`/api/endpoint/alerts`, { query: paginationDataFromUrl(state) as HttpFetchQuery, }); api.dispatch({ type: 'serverReturnedAlertsData', payload: response }); - */ } }; }; From 09fe7dc043bedc2d7d32310230ce5e27332892b5 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 19 Feb 2020 11:29:11 -0500 Subject: [PATCH 07/23] DO IT --- .../endpoint/store/alerts/alert_list.test.ts | 2 +- .../alerts/alert_list_pagination.test.ts | 10 +++--- .../endpoint/store/alerts/middleware.ts | 5 ++- .../endpoint/store/alerts/selectors.ts | 36 ++++++++++++------- .../public/applications/endpoint/types.ts | 13 ++++++- 5 files changed, 44 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list.test.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list.test.ts index 6ba7a34ae81d1..cfdc00aacde5b 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list.test.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list.test.ts @@ -31,7 +31,7 @@ describe('alert list tests', () => { const response: AlertResultList = { alerts: [ { - '@timestamp': new Date(1542341895000), + '@timestamp': new Date(1542341895000).toString(), agent: { id: 'ced9c68e-b94a-4d66-bb4c-6106514f0a2f', version: '3.0.0', diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts index d1ab77c6a437a..dba08ec5b225c 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts @@ -12,7 +12,7 @@ import { alertMiddlewareFactory } from './middleware'; import { AppAction } from '../action'; import { coreMock } from 'src/core/public/mocks'; import { createBrowserHistory } from 'history'; -import { urlFromNewPageSizeParam, queryParams, urlFromNewPageIndexParam } from './selectors'; +import { urlFromNewPageSizeParam, uiQueryParams, urlFromNewPageIndexParam } from './selectors'; describe('alert list pagination', () => { let store: Store; @@ -32,7 +32,7 @@ describe('alert list pagination', () => { store.dispatch({ type: 'userChangedUrl', payload: history.location }); }); it('should modify the url correctly', () => { - const actualPaginationQuery = queryParams(store.getState()); + const actualPaginationQuery = uiQueryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { "page_size": "1", @@ -47,7 +47,7 @@ describe('alert list pagination', () => { store.dispatch({ type: 'userChangedUrl', payload: history.location }); }); it('should modify the url in the correct order', () => { - const actualPaginationQuery = queryParams(store.getState()); + const actualPaginationQuery = uiQueryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { "page_index": "1", @@ -65,7 +65,7 @@ describe('alert list pagination', () => { store.dispatch({ type: 'userChangedUrl', payload: history.location }); }); it('should modify the url correctly', () => { - const actualPaginationQuery = queryParams(store.getState()); + const actualPaginationQuery = uiQueryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { "page_index": "1", @@ -80,7 +80,7 @@ describe('alert list pagination', () => { store.dispatch({ type: 'userChangedUrl', payload: history.location }); }); it('should modify the url correctly and reset index to `0`', () => { - const actualPaginationQuery = queryParams(store.getState()); + const actualPaginationQuery = uiQueryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { "page_index": "0", diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts index 059507c8f0658..76a6867418bd8 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/middleware.ts @@ -4,11 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -import { HttpFetchQuery } from 'kibana/public'; import { AlertResultList } from '../../../../../common/types'; import { AppAction } from '../action'; import { MiddlewareFactory, AlertListState } from '../../types'; -import { isOnAlertPage, paginationDataFromUrl } from './selectors'; +import { isOnAlertPage, apiQueryParams } from './selectors'; export const alertMiddlewareFactory: MiddlewareFactory = coreStart => { return api => next => async (action: AppAction) => { @@ -16,7 +15,7 @@ export const alertMiddlewareFactory: MiddlewareFactory = coreSta const state = api.getState(); if (action.type === 'userChangedUrl' && isOnAlertPage(state)) { const response: AlertResultList = await coreStart.http.get(`/api/endpoint/alerts`, { - query: paginationDataFromUrl(state) as HttpFetchQuery, + query: apiQueryParams(state), }); api.dispatch({ type: 'serverReturnedAlertsData', payload: response }); } diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts index 56bd03c7e4eed..bd0e2278dac51 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts @@ -7,7 +7,7 @@ import qs from 'querystring'; import { createSelector } from 'reselect'; import { Immutable } from '../../../../../common/types'; -import { AlertListState, AlertIndexQueryParams } from '../../types'; +import { AlertListState, AlertingIndexUIQueryParams, AlertsAPIQueryParams } from '../../types'; /** * Returns the Alert Data array from state @@ -34,16 +34,15 @@ export const isOnAlertPage = (state: AlertListState): boolean => { }; /** - * Returns the query object received from parsing the URL query params. - * The query value passed when requesting alert list data from the server. - * Also used to get new client side URLs. + * Returns the query object received from parsing the browsers URL query params. + * Used to calculate urls for links and such. */ -export const queryParams: ( +export const uiQueryParams: ( state: AlertListState -) => Immutable = createSelector( +) => Immutable = createSelector( (state: AlertListState) => state.location, (location: AlertListState['location']) => { - const data: AlertIndexQueryParams = {}; + const data: AlertingIndexUIQueryParams = {}; if (location) { // Removes the `?` from the beginning of query string if it exists const query = qs.parse(location.search.slice(1)); @@ -69,6 +68,19 @@ export const queryParams: ( } ); +/** + * query params to use when requesting alert data. + */ +export const apiQueryParams: ( + state: AlertListState +) => Immutable = createSelector( + uiQueryParams, + ({ page_size, page_index }) => ({ + page_size, + page_index, + }) +); + /** * Returns a function that takes in a new page size and returns a new query param string */ @@ -76,7 +88,7 @@ export const urlFromNewPageSizeParam: ( state: AlertListState ) => (newPageSize: number) => string = state => { return newPageSize => { - const urlPaginationData: AlertIndexQueryParams = { ...queryParams(state) }; + const urlPaginationData: AlertingIndexUIQueryParams = { ...uiQueryParams(state) }; urlPaginationData.page_size = newPageSize.toString(); // Only set the url back to page zero if the user has changed the page index already @@ -94,7 +106,7 @@ export const urlFromNewPageIndexParam: ( state: AlertListState ) => (newPageIndex: number) => string = state => { return newPageIndex => { - const urlPaginationData: AlertIndexQueryParams = { ...queryParams(state) }; + const urlPaginationData: AlertingIndexUIQueryParams = { ...uiQueryParams(state) }; urlPaginationData.page_index = newPageIndex.toString(); return '?' + qs.stringify(urlPaginationData); }; @@ -107,7 +119,7 @@ export const urlWithSelectedAlert: ( state: AlertListState ) => (alertID: string) => string = state => { return (alertID: string) => { - const urlPaginationData = { ...queryParams(state) }; + const urlPaginationData = { ...uiQueryParams(state) }; urlPaginationData.selected_alert = alertID; return '?' + qs.stringify(urlPaginationData); }; @@ -117,7 +129,7 @@ export const urlWithSelectedAlert: ( * Returns a url like the current one, but with no alert id */ export const urlWithoutSelectedAlert: (state: AlertListState) => string = createSelector( - queryParams, + uiQueryParams, urlPaginationData => { const newUrlPaginationData = { ...urlPaginationData }; delete newUrlPaginationData.selected_alert; @@ -126,6 +138,6 @@ export const urlWithoutSelectedAlert: (state: AlertListState) => string = create ); export const hasSelectedAlert: (state: AlertListState) => boolean = createSelector( - queryParams, + uiQueryParams, ({ selected_alert: selectedAlert }) => selectedAlert !== undefined ); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts index f5f263fa4f026..e1a4d2e5fccfc 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts @@ -86,8 +86,19 @@ export type AlertListState = Immutable & { readonly location?: Immutable; }; -export interface AlertIndexQueryParams { +/** + * Gotten by parsing the URL from the browser. Used to calculate the new URL when changing views. + */ +export interface AlertingIndexUIQueryParams { page_size?: string; page_index?: string; selected_alert?: string; } + +/** + * Query params to pass to the alert API when fetching new data. + */ +export interface AlertsAPIQueryParams { + page_size?: string; + page_index?: string; +} From 517d7963e4deef1ead886140bfa55b4c1faa2778 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 19 Feb 2020 12:26:41 -0500 Subject: [PATCH 08/23] return alerts from alert api. --- .../services/endpoint/alert_query_builders.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.ts b/x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.ts index a20f2ae1cdecd..e56ae43ef5c76 100644 --- a/x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.ts +++ b/x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.ts @@ -7,9 +7,9 @@ import { KibanaRequest } from 'kibana/server'; import { EndpointAppConstants } from '../../../common/types'; import { EndpointAppContext, AlertRequestParams, JSONish } from '../../types'; -export const buildAlertListESQuery = async ( +export const buildAlertListESQuery: ( pagingProperties: Record -): Promise => { +) => JSONish = pagingProperties => { const DEFAULT_TOTAL_HITS = 10000; // Calculate minimum total hits set to indicate there's a next page @@ -22,7 +22,15 @@ export const buildAlertListESQuery = async ( body: { track_total_hits: totalHitsMin, query: { - match_all: {}, + bool: { + must: [ + { + match: { + 'event.kind': 'alert', + }, + }, + ], + }, }, sort: [ { From dc98ea04fbdc85036c85ac97a5cf0e5f7e720af9 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 19 Feb 2020 12:35:27 -0500 Subject: [PATCH 09/23] remove copy paste --- .../endpoint/store/alerts/selectors.ts | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts index bd0e2278dac51..4d739f63d511b 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts @@ -46,22 +46,23 @@ export const uiQueryParams: ( if (location) { // Removes the `?` from the beginning of query string if it exists const query = qs.parse(location.search.slice(1)); - if (typeof query.page_size === 'string') { - data.page_size = query.page_size; - } else if (Array.isArray(query.page_size)) { - data.page_size = query.page_size[query.page_size.length - 1]; - } - - if (typeof query.page_index === 'string') { - data.page_index = query.page_index; - } else if (Array.isArray(query.page_index)) { - data.page_index = query.page_index[query.page_index.length - 1]; - } - if (typeof query.selected_alert === 'string') { - data.selected_alert = query.selected_alert; - } else if (Array.isArray(query.selected_alert)) { - data.selected_alert = query.selected_alert[query.selected_alert.length - 1]; + /** + * Build an AlertingIndexUIQueryParams object with keys from the query. + * If more than one value exists for a key, use the last. + */ + const keys: Array = [ + 'page_size', + 'page_index', + 'selected_alert', + ]; + for (const key of keys) { + const value = query[key]; + if (typeof value === 'string') { + data[key] = value; + } else if (Array.isArray(value)) { + data[key] = value[value.length - 1]; + } } } return data; From f2847b4ec20607350cf29249148dd41e2c09b503 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 19 Feb 2020 12:48:39 -0500 Subject: [PATCH 10/23] Fix query builder test --- .../endpoint/alert_query_builders.test.ts | 88 ++++++++++++------- 1 file changed, 54 insertions(+), 34 deletions(-) diff --git a/x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.test.ts b/x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.test.ts index a4d7de8fdcfdb..3ef1142b9ce46 100644 --- a/x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.test.ts +++ b/x-pack/plugins/endpoint/server/services/endpoint/alert_query_builders.test.ts @@ -16,26 +16,36 @@ describe('test query builder', () => { config: () => Promise.resolve(EndpointConfigSchema.validate({})), }; const queryParams = await getPagingProperties(mockRequest, mockCtx); - const query = await buildAlertListESQuery(queryParams); + const query = buildAlertListESQuery(queryParams); - expect(query).toEqual({ - body: { - query: { - match_all: {}, - }, - sort: [ - { - '@timestamp': { - order: 'desc', + expect(query).toMatchInlineSnapshot(` + Object { + "body": Object { + "query": Object { + "bool": Object { + "must": Array [ + Object { + "match": Object { + "event.kind": "alert", + }, + }, + ], }, }, - ], - track_total_hits: 10000, - }, - from: 0, - size: 10, - index: 'my-index', - } as Record); + "sort": Array [ + Object { + "@timestamp": Object { + "order": "desc", + }, + }, + ], + "track_total_hits": 10000, + }, + "from": 0, + "index": "my-index", + "size": 10, + } + `); }); it('should adjust track_total_hits for deep pagination', async () => { const mockRequest = httpServerMock.createKibanaRequest({ @@ -49,26 +59,36 @@ describe('test query builder', () => { config: () => Promise.resolve(EndpointConfigSchema.validate({})), }; const queryParams = await getPagingProperties(mockRequest, mockCtx); - const query = await buildAlertListESQuery(queryParams); + const query = buildAlertListESQuery(queryParams); - expect(query).toEqual({ - body: { - query: { - match_all: {}, - }, - sort: [ - { - '@timestamp': { - order: 'desc', + expect(query).toMatchInlineSnapshot(` + Object { + "body": Object { + "query": Object { + "bool": Object { + "must": Array [ + Object { + "match": Object { + "event.kind": "alert", + }, + }, + ], }, }, - ], - track_total_hits: 12000, - }, - from: 10000, - size: 1000, - index: 'my-index', - } as Record); + "sort": Array [ + Object { + "@timestamp": Object { + "order": "desc", + }, + }, + ], + "track_total_hits": 12000, + }, + "from": 10000, + "index": "my-index", + "size": 1000, + } + `); }); }); }); From 7d18af0affc16e33f597cee8884d62d12765c29a Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 19 Feb 2020 13:49:14 -0500 Subject: [PATCH 11/23] GO FOR IT --- .../public/applications/endpoint/types.ts | 15 +++++++++++++++ .../applications/endpoint/view/alerts/index.tsx | 6 +++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts index e1a4d2e5fccfc..9052902de469e 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts @@ -90,8 +90,17 @@ export type AlertListState = Immutable & { * Gotten by parsing the URL from the browser. Used to calculate the new URL when changing views. */ export interface AlertingIndexUIQueryParams { + /** + * How many items to show in list. + */ page_size?: string; + /** + * Which page to show. If `page_index` is 1, show page 2. + */ page_index?: string; + /** + * If any value is present, show the alert detail view for the selected alert. Should be an ID for an alert event. + */ selected_alert?: string; } @@ -99,6 +108,12 @@ export interface AlertingIndexUIQueryParams { * Query params to pass to the alert API when fetching new data. */ export interface AlertsAPIQueryParams { + /** + * Number of results to return. + */ page_size?: string; + /** + * 0-based index of 'page' to return. + */ page_index?: string; } diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index 75809e088e38b..eef14e15cb861 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -192,7 +192,11 @@ export const AlertIndex = memo(() => { -

Alert Details

+

+ {i18n.translate('xpack.endpoint.application.endpoint.alerts.detailsTitle', { + defaultMessage: 'Alert Details', + })} +

From 9686b88ae479c6c1596b8377eae941e7f28b3145 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 19 Feb 2020 16:46:26 -0500 Subject: [PATCH 12/23] add tests --- .../alerts/alert_list_pagination.test.ts | 1 - .../endpoint/store/alerts/selectors.ts | 71 ++++++++------- .../applications/endpoint/store/index.ts | 37 ++++---- .../public/applications/endpoint/types.ts | 5 +- .../endpoint/view/alerts/index.test.tsx | 86 +++++++++++++++++++ .../endpoint/view/alerts/index.tsx | 42 +++++---- .../resolver/view/use_camera.test.tsx | 3 - 7 files changed, 175 insertions(+), 70 deletions(-) create mode 100644 x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts index dba08ec5b225c..f659c90585d49 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts @@ -83,7 +83,6 @@ describe('alert list pagination', () => { const actualPaginationQuery = uiQueryParams(store.getState()); expect(actualPaginationQuery).toMatchInlineSnapshot(` Object { - "page_index": "0", "page_size": "1", } `); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts index 4d739f63d511b..27b2edc576c17 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts @@ -5,10 +5,19 @@ */ import qs from 'querystring'; -import { createSelector } from 'reselect'; +import { + createSelector, + createStructuredSelector as createStructuredSelectorWithBadType, +} from 'reselect'; import { Immutable } from '../../../../../common/types'; -import { AlertListState, AlertingIndexUIQueryParams, AlertsAPIQueryParams } from '../../types'; +import { + AlertListState, + AlertingIndexUIQueryParams, + AlertsAPIQueryParams, + CreateStructuredSelector, +} from '../../types'; +const createStructuredSelector: CreateStructuredSelector = createStructuredSelectorWithBadType; /** * Returns the Alert Data array from state */ @@ -17,14 +26,12 @@ export const alertListData = (state: AlertListState) => state.alerts; /** * Returns the alert list pagination data from state */ -export const alertListPagination = (state: AlertListState) => { - return { - pageIndex: state.request_page_index, - pageSize: state.request_page_size, - resultFromIndex: state.result_from_index, - total: state.total, - }; -}; +export const alertListPagination = createStructuredSelector({ + pageIndex: (state: AlertListState) => state.request_page_index, + pageSize: (state: AlertListState) => state.request_page_size, + resultFromIndex: (state: AlertListState) => state.result_from_index, + total: (state: AlertListState) => state.total, +}); /** * Returns a boolean based on whether or not the user is on the alerts page @@ -87,44 +94,46 @@ export const apiQueryParams: ( */ export const urlFromNewPageSizeParam: ( state: AlertListState -) => (newPageSize: number) => string = state => { +) => (newPageSize: number) => string = createSelector(uiQueryParams, paramData => { return newPageSize => { - const urlPaginationData: AlertingIndexUIQueryParams = { ...uiQueryParams(state) }; - urlPaginationData.page_size = newPageSize.toString(); + const queryParams: AlertingIndexUIQueryParams = { ...paramData }; + queryParams.page_size = newPageSize.toString(); - // Only set the url back to page zero if the user has changed the page index already - if (urlPaginationData.page_index !== undefined) { - urlPaginationData.page_index = '0'; + /** + * Reset the page index when changing page size. + */ + if (queryParams.page_index !== undefined) { + delete queryParams.page_index; } - return '?' + qs.stringify(urlPaginationData); + return '?' + qs.stringify(queryParams); }; -}; +}); /** * Returns a function that takes in a new page index and returns a new query param string */ export const urlFromNewPageIndexParam: ( state: AlertListState -) => (newPageIndex: number) => string = state => { +) => (newPageIndex: number) => string = createSelector(uiQueryParams, paramData => { return newPageIndex => { - const urlPaginationData: AlertingIndexUIQueryParams = { ...uiQueryParams(state) }; - urlPaginationData.page_index = newPageIndex.toString(); - return '?' + qs.stringify(urlPaginationData); + const queryParams: AlertingIndexUIQueryParams = { ...paramData }; + queryParams.page_index = newPageIndex.toString(); + return '?' + qs.stringify(queryParams); }; -}; +}); /** * Returns a url like the current one, but with a new alert id. */ export const urlWithSelectedAlert: ( state: AlertListState -) => (alertID: string) => string = state => { +) => (alertID: string) => string = createSelector(uiQueryParams, paramData => { return (alertID: string) => { - const urlPaginationData = { ...uiQueryParams(state) }; - urlPaginationData.selected_alert = alertID; - return '?' + qs.stringify(urlPaginationData); + const queryParams = { ...paramData }; + queryParams.selected_alert = alertID; + return '?' + qs.stringify(queryParams); }; -}; +}); /** * Returns a url like the current one, but with no alert id @@ -132,9 +141,9 @@ export const urlWithSelectedAlert: ( export const urlWithoutSelectedAlert: (state: AlertListState) => string = createSelector( uiQueryParams, urlPaginationData => { - const newUrlPaginationData = { ...urlPaginationData }; - delete newUrlPaginationData.selected_alert; - return '?' + qs.stringify(newUrlPaginationData); + const queryParams = { ...urlPaginationData }; + delete queryParams.selected_alert; + return '?' + qs.stringify(queryParams); } ); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts index 3aeeeaf1c09e2..65738587c1291 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts @@ -48,25 +48,30 @@ export const substateMiddlewareFactory = ( }; }; -export const appStoreFactory = (coreStart: CoreStart): Store => { +export const appStoreFactory: (coreStart: CoreStart, disableMiddleware?: boolean) => Store = ( + coreStart, + disableMiddleware = false +) => { const store = createStore( appReducer, - composeWithReduxDevTools( - applyMiddleware( - substateMiddlewareFactory( - globalState => globalState.managementList, - managementMiddlewareFactory(coreStart) - ), - substateMiddlewareFactory( - globalState => globalState.policyList, - policyListMiddlewareFactory(coreStart) - ), - substateMiddlewareFactory( - globalState => globalState.alertList, - alertMiddlewareFactory(coreStart) + disableMiddleware + ? undefined + : composeWithReduxDevTools( + applyMiddleware( + substateMiddlewareFactory( + globalState => globalState.managementList, + managementMiddlewareFactory(coreStart) + ), + substateMiddlewareFactory( + globalState => globalState.policyList, + policyListMiddlewareFactory(coreStart) + ), + substateMiddlewareFactory( + globalState => globalState.alertList, + alertMiddlewareFactory(coreStart) + ) + ) ) - ) - ) ); return store; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts index 9052902de469e..66d39b3a47fca 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts @@ -63,6 +63,10 @@ export interface GlobalState { readonly policyList: PolicyListState; } +/** + * A better type for createStructuredSelector. This doesn't support the options object. + * TODO + */ export type CreateStructuredSelector = < SelectorMap extends { [key: string]: (...args: never[]) => unknown } >( @@ -76,7 +80,6 @@ export type CreateStructuredSelector = < export interface EndpointAppLocation { pathname: string; search: string; - state: never; hash: string; key?: string; } diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx new file mode 100644 index 0000000000000..e943a9a17e847 --- /dev/null +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx @@ -0,0 +1,86 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import * as reactTestingLibrary from '@testing-library/react'; +import { Provider } from 'react-redux'; +import { I18nProvider } from '@kbn/i18n/react'; +import { AlertIndex } from './index'; +import { appStoreFactory } from '../../store'; +import { coreMock } from 'src/core/public/mocks'; +import { EndpointAppLocation } from '../../types'; +import { fireEvent } from '@testing-library/react'; +jest.mock('react-router-dom', () => { + const history = { + push: jest.fn(), + }; + return { + ...jest.requireActual('react-router-dom'), + useHistory: () => history, + }; +}); +import { useHistory } from 'react-router-dom'; + +function testSubjSelector(testSubjectID: string): string { + return `[data-test-subj="${testSubjectID}"]`; +} + +describe('when on the alerting page', () => { + let render: () => reactTestingLibrary.RenderResult; + let store: ReturnType; + beforeEach(async () => { + store = appStoreFactory(coreMock.createStart(), true); + render = () => { + return reactTestingLibrary.render( + + + + + + ); + }; + }); + it('should show a data grid', () => { + expect(render().container.querySelector(testSubjSelector('alertListGrid'))).not.toBeNull(); + }); + describe('when there is no selected alert in the url', () => { + it('should not show the flyout', async () => { + expect(render().container.querySelector(testSubjSelector('alert-detail-flyout'))).toBeNull(); + }); + }); + describe('when there is a selected alert in the url', () => { + beforeEach(() => { + reactTestingLibrary.act(() => { + const payload: EndpointAppLocation = { + pathname: '', + search: '?selected_alert=1', + hash: '', + }; + store.dispatch({ type: 'userChangedUrl', payload }); + }); + }); + it('should show the flyout', () => { + expect( + render().container.querySelector(testSubjSelector('alert-detail-flyout')) + ).not.toBeNull(); + }); + describe('when the user clicks the close button on the flyout', () => { + beforeEach(() => { + const result = render(); + const closeButton = result.container.querySelector( + testSubjSelector('euiFlyoutCloseButton') + ); + if (closeButton) { + fireEvent.click(closeButton); + } + }); + it('should push a new URL without the selected alert', () => { + expect(useHistory().push).toHaveBeenCalledTimes(1); + expect(useHistory().push).toHaveBeenLastCalledWith('?'); + }); + }); + }); +}); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index eef14e15cb861..2d293b34f7001 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -99,26 +99,29 @@ export const AlertIndex = memo(() => { ); const [visibleColumns, setVisibleColumns] = useState(() => columns.map(({ id }) => id)); - const formatter = new Intl.DateTimeFormat(i18n.getLocale(), { - year: 'numeric', - month: '2-digit', - day: '2-digit', - hour: '2-digit', - minute: '2-digit', - second: '2-digit', - }); + const formatter = useMemo( + () => + new Intl.DateTimeFormat(i18n.getLocale(), { + year: 'numeric', + month: '2-digit', + day: '2-digit', + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + }), + [] + ); - const handleAlertClick = useCallback( - (event: React.MouseEvent) => { + const handleAlertClick = useMemo(() => { + return (event: React.MouseEvent) => { if (event.target instanceof HTMLElement) { const alertId: string | undefined = event.target.dataset.alertId; if (alertId !== undefined) { history.push(urlWithSelectedAlert(alertId)); } } - }, - [history, urlWithSelectedAlert] - ); + }; + }, [history, urlWithSelectedAlert]); const handleFlyoutClose = useCallback(() => { history.push(urlWithoutSelectedAlert); @@ -189,7 +192,7 @@ export const AlertIndex = memo(() => { return ( <> {hasSelectedAlert && ( - +

@@ -209,10 +212,13 @@ export const AlertIndex = memo(() => { aria-label="Alert List" rowCount={total} columns={columns} - columnVisibility={{ - visibleColumns, - setVisibleColumns, - }} + columnVisibility={useMemo( + () => ({ + visibleColumns, + setVisibleColumns, + }), + [setVisibleColumns, visibleColumns] + )} renderCellValue={renderCellValue} pagination={pagination} data-test-subj="alertListGrid" diff --git a/x-pack/plugins/endpoint/public/embeddables/resolver/view/use_camera.test.tsx b/x-pack/plugins/endpoint/public/embeddables/resolver/view/use_camera.test.tsx index 85e1d4e694b15..f4abb51f062f2 100644 --- a/x-pack/plugins/endpoint/public/embeddables/resolver/view/use_camera.test.tsx +++ b/x-pack/plugins/endpoint/public/embeddables/resolver/view/use_camera.test.tsx @@ -4,9 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -/** - * This import must be hoisted as it uses `jest.mock`. Is there a better way? Mocking is not good. - */ import React from 'react'; import { render, act, RenderResult, fireEvent } from '@testing-library/react'; import { useCamera } from './use_camera'; From b5ce8635d0ddc6c28a3a20246c9a2b916b23da5b Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 19 Feb 2020 17:08:13 -0500 Subject: [PATCH 13/23] use davis technique --- .../public/applications/endpoint/index.tsx | 15 ++------ .../public/applications/endpoint/types.ts | 1 + .../endpoint/view/alerts/index.test.tsx | 37 +++++++++---------- .../endpoint/view/route_capture.tsx | 21 +++++++++++ 4 files changed, 42 insertions(+), 32 deletions(-) create mode 100644 x-pack/plugins/endpoint/public/applications/endpoint/view/route_capture.tsx diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/index.tsx index 8530d6206d398..c6c032c273543 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/index.tsx @@ -8,16 +8,14 @@ import * as React from 'react'; import ReactDOM from 'react-dom'; import { CoreStart, AppMountParameters } from 'kibana/public'; import { I18nProvider, FormattedMessage } from '@kbn/i18n/react'; -import { Route, Switch, BrowserRouter, useLocation } from 'react-router-dom'; -import { Provider, useDispatch } from 'react-redux'; +import { Route, Switch, BrowserRouter } from 'react-router-dom'; +import { Provider } from 'react-redux'; import { Store } from 'redux'; -import { memo } from 'react'; +import { RouteCapture } from './view/route_capture'; import { appStoreFactory } from './store'; import { AlertIndex } from './view/alerts'; import { ManagementList } from './view/managing'; import { PolicyList } from './view/policy'; -import { AppAction } from './store/action'; -import { EndpointAppLocation } from './types'; /** * This module will be loaded asynchronously to reduce the bundle size of your plugin's main bundle. @@ -33,13 +31,6 @@ export function renderApp(coreStart: CoreStart, { appBasePath, element }: AppMou }; } -const RouteCapture = memo(({ children }) => { - const location: EndpointAppLocation = useLocation(); - const dispatch: (action: AppAction) => unknown = useDispatch(); - dispatch({ type: 'userChangedUrl', payload: location }); - return <>{children}; -}); - interface RouterProps { basename: string; store: Store; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts index 66d39b3a47fca..59b69c2f6a5c7 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts @@ -10,6 +10,7 @@ import { EndpointMetadata } from '../../../common/types'; import { AppAction } from './store/action'; import { AlertResultList, Immutable } from '../../../common/types'; +export { AppAction }; export type MiddlewareFactory = ( coreStart: CoreStart ) => ( diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx index e943a9a17e847..0f17f9851e360 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx @@ -11,18 +11,10 @@ import { I18nProvider } from '@kbn/i18n/react'; import { AlertIndex } from './index'; import { appStoreFactory } from '../../store'; import { coreMock } from 'src/core/public/mocks'; -import { EndpointAppLocation } from '../../types'; import { fireEvent } from '@testing-library/react'; -jest.mock('react-router-dom', () => { - const history = { - push: jest.fn(), - }; - return { - ...jest.requireActual('react-router-dom'), - useHistory: () => history, - }; -}); -import { useHistory } from 'react-router-dom'; +import { RouteCapture } from '../route_capture'; +import { createMemoryHistory, MemoryHistory } from 'history'; +import { Router } from 'react-router-dom'; function testSubjSelector(testSubjectID: string): string { return `[data-test-subj="${testSubjectID}"]`; @@ -30,14 +22,20 @@ function testSubjSelector(testSubjectID: string): string { describe('when on the alerting page', () => { let render: () => reactTestingLibrary.RenderResult; + let history: MemoryHistory; let store: ReturnType; beforeEach(async () => { + history = createMemoryHistory(); store = appStoreFactory(coreMock.createStart(), true); render = () => { return reactTestingLibrary.render( - + + + + + ); @@ -54,12 +52,10 @@ describe('when on the alerting page', () => { describe('when there is a selected alert in the url', () => { beforeEach(() => { reactTestingLibrary.act(() => { - const payload: EndpointAppLocation = { - pathname: '', + history.push({ + ...history.location, search: '?selected_alert=1', - hash: '', - }; - store.dispatch({ type: 'userChangedUrl', payload }); + }); }); }); it('should show the flyout', () => { @@ -77,9 +73,10 @@ describe('when on the alerting page', () => { fireEvent.click(closeButton); } }); - it('should push a new URL without the selected alert', () => { - expect(useHistory().push).toHaveBeenCalledTimes(1); - expect(useHistory().push).toHaveBeenLastCalledWith('?'); + it('should no longer show the flyout', () => { + expect( + render().container.querySelector(testSubjSelector('alert-detail-flyout')) + ).toBeNull(); }); }); }); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/route_capture.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/route_capture.tsx new file mode 100644 index 0000000000000..28d2019b56888 --- /dev/null +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/route_capture.tsx @@ -0,0 +1,21 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { memo } from 'react'; +import { useLocation } from 'react-router-dom'; +import { useDispatch } from 'react-redux'; +import { EndpointAppLocation, AppAction } from '../types'; + +/** + * This component should be used above all routes, but below the Provider. + * It dispatches actions when the URL is changed. + */ +export const RouteCapture = memo(({ children }) => { + const location: EndpointAppLocation = useLocation(); + const dispatch: (action: AppAction) => unknown = useDispatch(); + dispatch({ type: 'userChangedUrl', payload: location }); + return <>{children}; +}); From eb25f76f68f17eae74fdc46067ce0d380a942ab0 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Wed, 19 Feb 2020 17:23:42 -0500 Subject: [PATCH 14/23] add tests that test it --- .../endpoint/store/alerts/alert_list.test.ts | 33 +------------- .../store/alerts/mock_alert_result_list.ts | 42 +++++++++++++++++ .../endpoint/view/alerts/index.test.tsx | 45 +++++++++++++++++-- .../endpoint/view/alerts/index.tsx | 6 ++- 4 files changed, 91 insertions(+), 35 deletions(-) create mode 100644 x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/mock_alert_result_list.ts diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list.test.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list.test.ts index cfdc00aacde5b..0aeeb6881ad96 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list.test.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list.test.ts @@ -14,6 +14,7 @@ import { coreMock } from 'src/core/public/mocks'; import { AlertResultList } from '../../../../../common/types'; import { isOnAlertPage } from './selectors'; import { createBrowserHistory } from 'history'; +import { mockAlertResultList } from './mock_alert_result_list'; describe('alert list tests', () => { let store: Store; @@ -28,37 +29,7 @@ describe('alert list tests', () => { describe('when the user navigates to the alert list page', () => { beforeEach(() => { coreStart.http.get.mockImplementation(async () => { - const response: AlertResultList = { - alerts: [ - { - '@timestamp': new Date(1542341895000).toString(), - agent: { - id: 'ced9c68e-b94a-4d66-bb4c-6106514f0a2f', - version: '3.0.0', - }, - event: { - action: 'open', - }, - file_classification: { - malware_classification: { - score: 3, - }, - }, - host: { - hostname: 'HD-c15-bc09190a', - ip: '10.179.244.14', - os: { - name: 'Windows', - }, - }, - thread: {}, - }, - ], - total: 1, - request_page_size: 10, - request_page_index: 0, - result_from_index: 0, - }; + const response: AlertResultList = mockAlertResultList(); return response; }); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/mock_alert_result_list.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/mock_alert_result_list.ts new file mode 100644 index 0000000000000..4bbb28bf5af3e --- /dev/null +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/mock_alert_result_list.ts @@ -0,0 +1,42 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { AlertResultList } from '../../../../../common/types'; + +export const mockAlertResultList: () => AlertResultList = () => { + const mock: AlertResultList = { + alerts: [ + { + '@timestamp': new Date(1542341895000).toString(), + agent: { + id: 'ced9c68e-b94a-4d66-bb4c-6106514f0a2f', + version: '3.0.0', + }, + event: { + action: 'open', + }, + file_classification: { + malware_classification: { + score: 3, + }, + }, + host: { + hostname: 'HD-c15-bc09190a', + ip: '10.179.244.14', + os: { + name: 'Windows', + }, + }, + thread: {}, + }, + ], + total: 1, + request_page_size: 10, + request_page_index: 0, + result_from_index: 0, + }; + return mock; +}; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx index 0f17f9851e360..cb4e5978653ec 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx @@ -15,6 +15,8 @@ import { fireEvent } from '@testing-library/react'; import { RouteCapture } from '../route_capture'; import { createMemoryHistory, MemoryHistory } from 'history'; import { Router } from 'react-router-dom'; +import { AppAction } from '../../types'; +import { mockAlertResultList } from '../../store/alerts/mock_alert_result_list'; function testSubjSelector(testSubjectID: string): string { return `[data-test-subj="${testSubjectID}"]`; @@ -48,6 +50,42 @@ describe('when on the alerting page', () => { it('should not show the flyout', async () => { expect(render().container.querySelector(testSubjSelector('alert-detail-flyout'))).toBeNull(); }); + describe('when data loads', () => { + beforeEach(() => { + reactTestingLibrary.act(() => { + const action: AppAction = { + type: 'serverReturnedAlertsData', + payload: mockAlertResultList(), + }; + store.dispatch(action); + }); + }); + it('should render the alert summary row in the grid', async () => { + /** + * There should be a 'row' which is the header, and + * another 'row' which is the alert summary. + */ + expect(await render().findAllByRole('row')).toHaveLength(2); + }); + describe('when the user has clicked the alert type in the grid', () => { + let renderResult: reactTestingLibrary.RenderResult; + beforeEach(() => { + renderResult = render(); + // This is the cell with the alert type, it has a link. + const alertTypeCellLink = renderResult.container.querySelector( + testSubjSelector('alert-type-cell-link') + ); + if (alertTypeCellLink) { + fireEvent.click(alertTypeCellLink); + } + }); + it('should show the flyout', () => { + expect( + renderResult.container.querySelector(testSubjSelector('alert-detail-flyout')) + ).not.toBeNull(); + }); + }); + }); }); describe('when there is a selected alert in the url', () => { beforeEach(() => { @@ -64,9 +102,10 @@ describe('when on the alerting page', () => { ).not.toBeNull(); }); describe('when the user clicks the close button on the flyout', () => { + let renderResult: reactTestingLibrary.RenderResult; beforeEach(() => { - const result = render(); - const closeButton = result.container.querySelector( + renderResult = render(); + const closeButton = renderResult.container.querySelector( testSubjSelector('euiFlyoutCloseButton') ); if (closeButton) { @@ -75,7 +114,7 @@ describe('when on the alerting page', () => { }); it('should no longer show the flyout', () => { expect( - render().container.querySelector(testSubjSelector('alert-detail-flyout')) + renderResult.container.querySelector(testSubjSelector('alert-detail-flyout')) ).toBeNull(); }); }); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index 2d293b34f7001..3305066a33c04 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -137,7 +137,11 @@ export const AlertIndex = memo(() => { if (columnId === 'alert_type') { return ( - + {i18n.translate( 'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription', { From daafb550f9f51bc1aa4457456fa165c72dd8fe61 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 20 Feb 2020 09:31:23 -0500 Subject: [PATCH 15/23] use currentTarget cause candace said so --- .../public/applications/endpoint/view/alerts/index.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index 3305066a33c04..fb50b4ae86f68 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -114,11 +114,9 @@ export const AlertIndex = memo(() => { const handleAlertClick = useMemo(() => { return (event: React.MouseEvent) => { - if (event.target instanceof HTMLElement) { - const alertId: string | undefined = event.target.dataset.alertId; - if (alertId !== undefined) { - history.push(urlWithSelectedAlert(alertId)); - } + const alertId: string | undefined = event.currentTarget.dataset.alertId; + if (alertId !== undefined) { + history.push(urlWithSelectedAlert(alertId)); } }; }, [history, urlWithSelectedAlert]); From 0d67dad1629c4d9dc4b48c19825d1fe1043063fd Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 20 Feb 2020 10:10:57 -0500 Subject: [PATCH 16/23] use react router link. its an actual link, and its easier to test w/ react router also, use `href` instead of a button --- .../endpoint/view/alerts/index.tsx | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index fb50b4ae86f68..97992a534e138 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -7,7 +7,6 @@ import { memo, useState, useMemo, useCallback } from 'react'; import React from 'react'; import { - EuiLink, EuiDataGrid, EuiDataGridColumn, EuiPage, @@ -20,7 +19,7 @@ import { EuiBadge, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import { useHistory } from 'react-router-dom'; +import { useHistory, Link } from 'react-router-dom'; import * as selectors from '../../store/alerts/selectors'; import { useAlertListSelector } from './hooks/use_alerts_selector'; @@ -112,15 +111,6 @@ export const AlertIndex = memo(() => { [] ); - const handleAlertClick = useMemo(() => { - return (event: React.MouseEvent) => { - const alertId: string | undefined = event.currentTarget.dataset.alertId; - if (alertId !== undefined) { - history.push(urlWithSelectedAlert(alertId)); - } - }; - }, [history, urlWithSelectedAlert]); - const handleFlyoutClose = useCallback(() => { history.push(urlWithoutSelectedAlert); }, [history, urlWithoutSelectedAlert]); @@ -135,18 +125,14 @@ export const AlertIndex = memo(() => { if (columnId === 'alert_type') { return ( - + {i18n.translate( 'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription', { defaultMessage: 'Malicious File', } )} - + ); } else if (columnId === 'event_type') { return row.event.action; @@ -179,7 +165,7 @@ export const AlertIndex = memo(() => { } return null; }; - }, [alertListData, formatter, handleAlertClick, pageSize, total]); + }, [alertListData, formatter, pageSize, total, urlWithSelectedAlert]); const pagination = useMemo(() => { return { From 26a5ac2fa25ac4384103efeb8c763f18ac307121 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 20 Feb 2020 12:39:22 -0500 Subject: [PATCH 17/23] rewrite the tests --- .../alerts/__snapshots__/index.test.tsx.snap | 606 ++++++++++++++++++ .../endpoint/view/alerts/index.test.tsx | 102 ++- .../endpoint/view/alerts/index.tsx | 47 +- 3 files changed, 703 insertions(+), 52 deletions(-) create mode 100644 x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/__snapshots__/index.test.tsx.snap diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/__snapshots__/index.test.tsx.snap b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/__snapshots__/index.test.tsx.snap new file mode 100644 index 0000000000000..c096ccf392910 --- /dev/null +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/__snapshots__/index.test.tsx.snap @@ -0,0 +1,606 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`when on the alerting page when there is no selected alert in the url when data loads should render the alert summary row in the grid 1`] = ` +
+
+
+
+
+
+
+

+ + Row + : + 1 + , + Column + : + 1 + : + +

+ +
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+

+ + Row + : + 1 + , + Column + : + 2 + : + +

+
+ open +
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+

+ + Row + : + 1 + , + Column + : + 3 + : + +

+
+ Windows +
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+

+ + Row + : + 1 + , + Column + : + 4 + : + +

+
+ 10.179.244.14 +
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+

+ + Row + : + 1 + , + Column + : + 5 + : + +

+
+ HD-c15-bc09190a +
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+

+ + Row + : + 1 + , + Column + : + 6 + : + +

+
+ 11/15/2018, 11:18:15 PM +
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+

+ + Row + : + 1 + , + Column + : + 7 + : + +

+
+
+
+ +
+
+
+
+
+
+
+
+
+
+
+
+

+ + Row + : + 1 + , + Column + : + 8 + : + +

+
+ 3 +
+
+
+ +
+
+
+
+
+
+
+`; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx index cb4e5978653ec..3083457cba642 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx @@ -11,25 +11,49 @@ import { I18nProvider } from '@kbn/i18n/react'; import { AlertIndex } from './index'; import { appStoreFactory } from '../../store'; import { coreMock } from 'src/core/public/mocks'; -import { fireEvent } from '@testing-library/react'; +import { fireEvent, waitForElement } from '@testing-library/react'; import { RouteCapture } from '../route_capture'; import { createMemoryHistory, MemoryHistory } from 'history'; import { Router } from 'react-router-dom'; import { AppAction } from '../../types'; import { mockAlertResultList } from '../../store/alerts/mock_alert_result_list'; -function testSubjSelector(testSubjectID: string): string { - return `[data-test-subj="${testSubjectID}"]`; -} - describe('when on the alerting page', () => { let render: () => reactTestingLibrary.RenderResult; let history: MemoryHistory; let store: ReturnType; + + /** + * @testing-library/react provides `queryByTestId`, but that uses the data attribute + * 'data-testid' whereas our FTR and EUI's tests all use 'data-test-subj'. While @testing-library/react + * could be configured to use 'data-test-subj', it is not currently configured that way. + * + * This provides an equivalent function to `queryByTestId` but that uses our 'data-test-subj' attribute. + */ + let queryByTestSubjId: ( + renderResult: reactTestingLibrary.RenderResult, + testSubjId: string + ) => Promise; + beforeEach(async () => { + /** + * Create a 'history' instance that is only in-memory and causes no side effects to the testing environment. + */ history = createMemoryHistory(); + /** + * Create a store, with the middleware disabled. We don't want side effects being created by our code in this test. + */ store = appStoreFactory(coreMock.createStart(), true); + /** + * Render the test component, use this after setting up anything in `beforeEach`. + */ render = () => { + /** + * Provide the store via `Provider`, and i18n APIs via `I18nProvider`. + * Use react-router via `Router`, passing our in-memory `history` instance. + * Use `RouteCapture` to emit url-change actions when the URL is changed. + * Finally, render the `AlertIndex` component which we are testing. + */ return reactTestingLibrary.render( @@ -42,16 +66,28 @@ describe('when on the alerting page', () => { ); }; + queryByTestSubjId = async (renderResult, testSubjId) => { + return await waitForElement( + () => renderResult.container.querySelector(`[data-test-subj="${testSubjId}"]`), + { + container: renderResult.container, + } + ); + }; }); - it('should show a data grid', () => { - expect(render().container.querySelector(testSubjSelector('alertListGrid'))).not.toBeNull(); + it('should show a data grid', async () => { + await render().findByTestId('alertListGrid'); }); describe('when there is no selected alert in the url', () => { - it('should not show the flyout', async () => { - expect(render().container.querySelector(testSubjSelector('alert-detail-flyout'))).toBeNull(); + it('should not show the flyout', () => { + expect(render().queryByTestId('alertDetailFlyout')).toBeNull(); }); describe('when data loads', () => { beforeEach(() => { + /** + * Dispatch the `serverReturnedAlertsData` action, which is normally dispatched by the middleware + * after interacting with the server. + */ reactTestingLibrary.act(() => { const action: AppAction = { type: 'serverReturnedAlertsData', @@ -61,28 +97,31 @@ describe('when on the alerting page', () => { }); }); it('should render the alert summary row in the grid', async () => { + const renderResult = render(); + const rows = await renderResult.findAllByRole('row'); + /** * There should be a 'row' which is the header, and * another 'row' which is the alert summary. */ - expect(await render().findAllByRole('row')).toHaveLength(2); + expect(rows).toHaveLength(2); + + /** + * Record the markup for the first row, to alert us in case something in the implementation changes. + */ + expect(rows[1]).toMatchSnapshot(); }); describe('when the user has clicked the alert type in the grid', () => { let renderResult: reactTestingLibrary.RenderResult; - beforeEach(() => { + beforeEach(async () => { renderResult = render(); - // This is the cell with the alert type, it has a link. - const alertTypeCellLink = renderResult.container.querySelector( - testSubjSelector('alert-type-cell-link') - ); - if (alertTypeCellLink) { - fireEvent.click(alertTypeCellLink); - } + /** + * This is the cell with the alert type, it has a link. + */ + fireEvent.click(await renderResult.findByTestId('alertTypeCellLink')); }); - it('should show the flyout', () => { - expect( - renderResult.container.querySelector(testSubjSelector('alert-detail-flyout')) - ).not.toBeNull(); + it('should show the flyout', async () => { + await renderResult.findByTestId('alertDetailFlyout'); }); }); }); @@ -96,26 +135,23 @@ describe('when on the alerting page', () => { }); }); }); - it('should show the flyout', () => { - expect( - render().container.querySelector(testSubjSelector('alert-detail-flyout')) - ).not.toBeNull(); + it('should show the flyout', async () => { + await render().findByTestId('alertDetailFlyout'); }); describe('when the user clicks the close button on the flyout', () => { let renderResult: reactTestingLibrary.RenderResult; - beforeEach(() => { + beforeEach(async () => { renderResult = render(); - const closeButton = renderResult.container.querySelector( - testSubjSelector('euiFlyoutCloseButton') - ); + /** + * Use our helper function to find the flyout's close button, as it uses a different test ID attribute. + */ + const closeButton = await queryByTestSubjId(renderResult, 'euiFlyoutCloseButton'); if (closeButton) { fireEvent.click(closeButton); } }); it('should no longer show the flyout', () => { - expect( - renderResult.container.querySelector(testSubjSelector('alert-detail-flyout')) - ).toBeNull(); + expect(render().queryByTestId('alertDetailFlyout')).toBeNull(); }); }); }); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index 97992a534e138..f236991f9f27e 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -20,6 +20,8 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { useHistory, Link } from 'react-router-dom'; +import { FormattedDate } from 'react-intl'; +import { AlertData } from '../../../../../common/types'; import * as selectors from '../../store/alerts/selectors'; import { useAlertListSelector } from './hooks/use_alerts_selector'; @@ -98,23 +100,19 @@ export const AlertIndex = memo(() => { ); const [visibleColumns, setVisibleColumns] = useState(() => columns.map(({ id }) => id)); - const formatter = useMemo( - () => - new Intl.DateTimeFormat(i18n.getLocale(), { - year: 'numeric', - month: '2-digit', - day: '2-digit', - hour: '2-digit', - minute: '2-digit', - second: '2-digit', - }), - [] - ); const handleFlyoutClose = useCallback(() => { history.push(urlWithoutSelectedAlert); }, [history, urlWithoutSelectedAlert]); + const datesForRows: Map = useMemo(() => { + return new Map( + alertListData.map(alertData => { + return [alertData, new Date(alertData['@timestamp'])]; + }) + ); + }, [alertListData]); + const renderCellValue = useMemo(() => { return ({ rowIndex, columnId }: { rowIndex: number; columnId: string }) => { if (rowIndex > total) { @@ -125,7 +123,7 @@ export const AlertIndex = memo(() => { if (columnId === 'alert_type') { return ( - + {i18n.translate( 'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription', { @@ -143,9 +141,19 @@ export const AlertIndex = memo(() => { } else if (columnId === 'host_name') { return row.host.hostname; } else if (columnId === 'timestamp') { - const date = new Date(row['@timestamp']); - if (isFinite(date.getTime())) { - return formatter.format(date); + const date = datesForRows.get(row)!; + if (date && isFinite(date.getTime())) { + return ( + + ); } else { return ( @@ -165,7 +173,7 @@ export const AlertIndex = memo(() => { } return null; }; - }, [alertListData, formatter, pageSize, total, urlWithSelectedAlert]); + }, [alertListData, datesForRows, pageSize, total, urlWithSelectedAlert]); const pagination = useMemo(() => { return { @@ -180,7 +188,7 @@ export const AlertIndex = memo(() => { return ( <> {hasSelectedAlert && ( - +

@@ -193,7 +201,7 @@ export const AlertIndex = memo(() => { )} - + { renderCellValue={renderCellValue} pagination={pagination} data-test-subj="alertListGrid" + data-testid="alertListGrid" /> From 60db6168528a4381d2d65f25be954ff5487c0ac2 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 20 Feb 2020 12:57:54 -0500 Subject: [PATCH 18/23] remove todos --- x-pack/plugins/endpoint/public/applications/endpoint/types.ts | 1 - .../endpoint/public/embeddables/resolver/store/selectors.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts index 59b69c2f6a5c7..bd4838419891d 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/types.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/types.ts @@ -66,7 +66,6 @@ export interface GlobalState { /** * A better type for createStructuredSelector. This doesn't support the options object. - * TODO */ export type CreateStructuredSelector = < SelectorMap extends { [key: string]: (...args: never[]) => unknown } diff --git a/x-pack/plugins/endpoint/public/embeddables/resolver/store/selectors.ts b/x-pack/plugins/endpoint/public/embeddables/resolver/store/selectors.ts index 4d12e656205fa..25d08a8c347ed 100644 --- a/x-pack/plugins/endpoint/public/embeddables/resolver/store/selectors.ts +++ b/x-pack/plugins/endpoint/public/embeddables/resolver/store/selectors.ts @@ -31,7 +31,6 @@ export const inverseProjectionMatrix = composeSelectors( /** * The scale by which world values are scaled when rendered. - * TODO make it a number */ export const scale = composeSelectors(cameraStateSelector, cameraSelectors.scale); From 66c45900ad22e0cb4b2e5b75ea8c28303151b47e Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 20 Feb 2020 13:05:47 -0500 Subject: [PATCH 19/23] Add comments --- .../public/applications/endpoint/store/index.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts index 65738587c1291..b95ff7cb2d45c 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/index.ts @@ -48,10 +48,16 @@ export const substateMiddlewareFactory = ( }; }; -export const appStoreFactory: (coreStart: CoreStart, disableMiddleware?: boolean) => Store = ( - coreStart, - disableMiddleware = false -) => { +export const appStoreFactory: ( + /** + * Allow middleware to communicate with Kibana core. + */ + coreStart: CoreStart, + /** + * Create the store without any middleware. This is useful for testing the store w/o side effects. + */ + disableMiddleware?: boolean +) => Store = (coreStart, disableMiddleware = false) => { const store = createStore( appReducer, disableMiddleware From 5a301684649b3447354849417909e3d39d527898 Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 20 Feb 2020 14:38:58 -0500 Subject: [PATCH 20/23] remove snapshot as it was not stable (relied on timezone) --- .../alerts/__snapshots__/index.test.tsx.snap | 606 ------------------ .../endpoint/view/alerts/index.test.tsx | 5 - 2 files changed, 611 deletions(-) delete mode 100644 x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/__snapshots__/index.test.tsx.snap diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/__snapshots__/index.test.tsx.snap b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/__snapshots__/index.test.tsx.snap deleted file mode 100644 index c096ccf392910..0000000000000 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/__snapshots__/index.test.tsx.snap +++ /dev/null @@ -1,606 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`when on the alerting page when there is no selected alert in the url when data loads should render the alert summary row in the grid 1`] = ` -
-
-
-
-
-
-
-

- - Row - : - 1 - , - Column - : - 1 - : - -

- -
-
- -
-
-
-
-
-
-
-
-
-
-
-
-

- - Row - : - 1 - , - Column - : - 2 - : - -

-
- open -
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-

- - Row - : - 1 - , - Column - : - 3 - : - -

-
- Windows -
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-

- - Row - : - 1 - , - Column - : - 4 - : - -

-
- 10.179.244.14 -
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-

- - Row - : - 1 - , - Column - : - 5 - : - -

-
- HD-c15-bc09190a -
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-

- - Row - : - 1 - , - Column - : - 6 - : - -

-
- 11/15/2018, 11:18:15 PM -
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-

- - Row - : - 1 - , - Column - : - 7 - : - -

-
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-

- - Row - : - 1 - , - Column - : - 8 - : - -

-
- 3 -
-
-
- -
-
-
-
-
-
-
-`; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx index 3083457cba642..0cf4e6efd121a 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx @@ -105,11 +105,6 @@ describe('when on the alerting page', () => { * another 'row' which is the alert summary. */ expect(rows).toHaveLength(2); - - /** - * Record the markup for the first row, to alert us in case something in the implementation changes. - */ - expect(rows[1]).toMatchSnapshot(); }); describe('when the user has clicked the alert type in the grid', () => { let renderResult: reactTestingLibrary.RenderResult; From 626e2fb0d3fdbfcb3561fa8da63d0ad1075a919b Mon Sep 17 00:00:00 2001 From: oatkiller Date: Thu, 20 Feb 2020 18:16:24 -0500 Subject: [PATCH 21/23] respect existing query params when opening or closing flyouyt disable test i broke. lets rethink this --- .../alerts/alert_list_pagination.test.ts | 52 +++++++++------- .../endpoint/store/alerts/selectors.ts | 62 +------------------ .../endpoint/view/alerts/index.tsx | 39 ++++++++---- .../view/alerts/url_from_query_params.ts | 27 ++++++++ 4 files changed, 85 insertions(+), 95 deletions(-) create mode 100644 x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts index f659c90585d49..8cb8b96dfefe5 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts @@ -7,33 +7,47 @@ import { Store, createStore, applyMiddleware } from 'redux'; import { History } from 'history'; import { alertListReducer } from './reducer'; -import { AlertListState } from '../../types'; +import { AlertListState, AlertingIndexUIQueryParams } from '../../types'; import { alertMiddlewareFactory } from './middleware'; import { AppAction } from '../action'; import { coreMock } from 'src/core/public/mocks'; import { createBrowserHistory } from 'history'; -import { urlFromNewPageSizeParam, uiQueryParams, urlFromNewPageIndexParam } from './selectors'; +import { uiQueryParams } from './selectors'; +import { urlFromQueryParams } from '../../view/alerts/url_from_query_params'; describe('alert list pagination', () => { let store: Store; let coreStart: ReturnType; let history: History; + let queryParams: () => AlertingIndexUIQueryParams; + /** + * Update the history with a new `AlertingIndexUIQueryParams` + */ + let historyPush: (params: AlertingIndexUIQueryParams) => void; beforeEach(() => { coreStart = coreMock.createStart(); history = createBrowserHistory(); + const middleware = alertMiddlewareFactory(coreStart); store = createStore(alertListReducer, applyMiddleware(middleware)); + + history.listen(location => { + store.dispatch({ type: 'userChangedUrl', payload: location }); + }); + + queryParams = () => uiQueryParams(store.getState()); + + historyPush = (nextQueryParams: AlertingIndexUIQueryParams): void => { + return history.push(urlFromQueryParams(nextQueryParams)); + }; }); describe('when the user navigates to the alert list page', () => { describe('when a new page size is passed', () => { beforeEach(() => { - const urlPageSizeSelector = urlFromNewPageSizeParam(store.getState()); - history.push(urlPageSizeSelector(1)); - store.dispatch({ type: 'userChangedUrl', payload: history.location }); + historyPush({ ...queryParams(), page_size: '1' }); }); it('should modify the url correctly', () => { - const actualPaginationQuery = uiQueryParams(store.getState()); - expect(actualPaginationQuery).toMatchInlineSnapshot(` + expect(queryParams()).toMatchInlineSnapshot(` Object { "page_size": "1", } @@ -42,13 +56,10 @@ describe('alert list pagination', () => { describe('and then a new page index is passed', () => { beforeEach(() => { - const urlPageIndexSelector = urlFromNewPageIndexParam(store.getState()); - history.push(urlPageIndexSelector(1)); - store.dispatch({ type: 'userChangedUrl', payload: history.location }); + historyPush({ ...queryParams(), page_index: '1' }); }); it('should modify the url in the correct order', () => { - const actualPaginationQuery = uiQueryParams(store.getState()); - expect(actualPaginationQuery).toMatchInlineSnapshot(` + expect(queryParams()).toMatchInlineSnapshot(` Object { "page_index": "1", "page_size": "1", @@ -60,28 +71,23 @@ describe('alert list pagination', () => { describe('when a new page index is passed', () => { beforeEach(() => { - const urlPageIndexSelector = urlFromNewPageIndexParam(store.getState()); - history.push(urlPageIndexSelector(1)); - store.dispatch({ type: 'userChangedUrl', payload: history.location }); + historyPush({ ...queryParams(), page_index: '1' }); }); it('should modify the url correctly', () => { - const actualPaginationQuery = uiQueryParams(store.getState()); - expect(actualPaginationQuery).toMatchInlineSnapshot(` + expect(queryParams()).toMatchInlineSnapshot(` Object { "page_index": "1", } `); }); - describe('and then a new page size is passed', () => { + // TODO, move this test to react land, since thats where this logic lives now. is that good? + xdescribe('and then a new page size is passed', () => { beforeEach(() => { - const urlPageSizeSelector = urlFromNewPageSizeParam(store.getState()); - history.push(urlPageSizeSelector(1)); - store.dispatch({ type: 'userChangedUrl', payload: history.location }); + historyPush({ ...queryParams(), page_size: '1' }); }); it('should modify the url correctly and reset index to `0`', () => { - const actualPaginationQuery = uiQueryParams(store.getState()); - expect(actualPaginationQuery).toMatchInlineSnapshot(` + expect(queryParams()).toMatchInlineSnapshot(` Object { "page_size": "1", } diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts index 27b2edc576c17..3a0461e06538f 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import qs from 'querystring'; +import querystring from 'querystring'; import { createSelector, createStructuredSelector as createStructuredSelectorWithBadType, @@ -52,7 +52,7 @@ export const uiQueryParams: ( const data: AlertingIndexUIQueryParams = {}; if (location) { // Removes the `?` from the beginning of query string if it exists - const query = qs.parse(location.search.slice(1)); + const query = querystring.parse(location.search.slice(1)); /** * Build an AlertingIndexUIQueryParams object with keys from the query. @@ -89,64 +89,6 @@ export const apiQueryParams: ( }) ); -/** - * Returns a function that takes in a new page size and returns a new query param string - */ -export const urlFromNewPageSizeParam: ( - state: AlertListState -) => (newPageSize: number) => string = createSelector(uiQueryParams, paramData => { - return newPageSize => { - const queryParams: AlertingIndexUIQueryParams = { ...paramData }; - queryParams.page_size = newPageSize.toString(); - - /** - * Reset the page index when changing page size. - */ - if (queryParams.page_index !== undefined) { - delete queryParams.page_index; - } - return '?' + qs.stringify(queryParams); - }; -}); - -/** - * Returns a function that takes in a new page index and returns a new query param string - */ -export const urlFromNewPageIndexParam: ( - state: AlertListState -) => (newPageIndex: number) => string = createSelector(uiQueryParams, paramData => { - return newPageIndex => { - const queryParams: AlertingIndexUIQueryParams = { ...paramData }; - queryParams.page_index = newPageIndex.toString(); - return '?' + qs.stringify(queryParams); - }; -}); - -/** - * Returns a url like the current one, but with a new alert id. - */ -export const urlWithSelectedAlert: ( - state: AlertListState -) => (alertID: string) => string = createSelector(uiQueryParams, paramData => { - return (alertID: string) => { - const queryParams = { ...paramData }; - queryParams.selected_alert = alertID; - return '?' + qs.stringify(queryParams); - }; -}); - -/** - * Returns a url like the current one, but with no alert id - */ -export const urlWithoutSelectedAlert: (state: AlertListState) => string = createSelector( - uiQueryParams, - urlPaginationData => { - const queryParams = { ...urlPaginationData }; - delete queryParams.selected_alert; - return '?' + qs.stringify(queryParams); - } -); - export const hasSelectedAlert: (state: AlertListState) => boolean = createSelector( uiQueryParams, ({ selected_alert: selectedAlert }) => selectedAlert !== undefined diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx index f236991f9f27e..6f88727575557 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx @@ -21,6 +21,7 @@ import { import { i18n } from '@kbn/i18n'; import { useHistory, Link } from 'react-router-dom'; import { FormattedDate } from 'react-intl'; +import { urlFromQueryParams } from './url_from_query_params'; import { AlertData } from '../../../../../common/types'; import * as selectors from '../../store/alerts/selectors'; import { useAlertListSelector } from './hooks/use_alerts_selector'; @@ -82,28 +83,39 @@ export const AlertIndex = memo(() => { }, []); const { pageIndex, pageSize, total } = useAlertListSelector(selectors.alertListPagination); - const urlFromNewPageSizeParam = useAlertListSelector(selectors.urlFromNewPageSizeParam); - const urlWithSelectedAlert = useAlertListSelector(selectors.urlWithSelectedAlert); - const urlWithoutSelectedAlert = useAlertListSelector(selectors.urlWithoutSelectedAlert); - const urlFromNewPageIndexParam = useAlertListSelector(selectors.urlFromNewPageIndexParam); const alertListData = useAlertListSelector(selectors.alertListData); const hasSelectedAlert = useAlertListSelector(selectors.hasSelectedAlert); + const queryParams = useAlertListSelector(selectors.uiQueryParams); const onChangeItemsPerPage = useCallback( - newPageSize => history.push(urlFromNewPageSizeParam(newPageSize)), - [history, urlFromNewPageSizeParam] + newPageSize => { + const newQueryParms = { ...queryParams }; + newQueryParms.page_size = newPageSize; + delete newQueryParms.page_index; + const relativeURL = urlFromQueryParams(newQueryParms); + return history.push(relativeURL); + }, + [history, queryParams] ); const onChangePage = useCallback( - newPageIndex => history.push(urlFromNewPageIndexParam(newPageIndex)), - [history, urlFromNewPageIndexParam] + newPageIndex => { + return history.push( + urlFromQueryParams({ + ...queryParams, + page_index: newPageIndex, + }) + ); + }, + [history, queryParams] ); const [visibleColumns, setVisibleColumns] = useState(() => columns.map(({ id }) => id)); const handleFlyoutClose = useCallback(() => { - history.push(urlWithoutSelectedAlert); - }, [history, urlWithoutSelectedAlert]); + const { selected_alert, ...paramsWithoutSelectedAlert } = queryParams; + history.push(urlFromQueryParams(paramsWithoutSelectedAlert)); + }, [history, queryParams]); const datesForRows: Map = useMemo(() => { return new Map( @@ -123,7 +135,10 @@ export const AlertIndex = memo(() => { if (columnId === 'alert_type') { return ( - + {i18n.translate( 'xpack.endpoint.application.endpoint.alerts.alertType.maliciousFileDescription', { @@ -173,7 +188,7 @@ export const AlertIndex = memo(() => { } return null; }; - }, [alertListData, datesForRows, pageSize, total, urlWithSelectedAlert]); + }, [alertListData, datesForRows, pageSize, queryParams, total]); const pagination = useMemo(() => { return { diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts new file mode 100644 index 0000000000000..6dd8edb2684c0 --- /dev/null +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import querystring from 'querystring'; +import { AlertingIndexUIQueryParams } from '../../types'; + +/** + * Return a relative URL for `AlertingIndexUIQueryParams`. + * usage: + * + * ```ts + * // Replace this with however you get state, e.g. useSelector in react + * const queryParams = selectors.uiQueryParams(store.getState()) + * + * // same as current url, but page_index is now 3 + * const relativeURL = urlFromQueryParams({ ...queryParams, page_index: 3 }) + * + * // now use relativeURL in the 'href' of a link, the 'to' of a react-router-dom 'Link' or history.push, history.replace + * ``` + */ +export function urlFromQueryParams(queryParams: AlertingIndexUIQueryParams): string { + const search = querystring.stringify(queryParams); + return search === '' ? '' : '?' + search; +} From 5b78bafdab31ed02146a868af7e99c68d34e667c Mon Sep 17 00:00:00 2001 From: oatkiller Date: Fri, 21 Feb 2020 11:30:18 -0500 Subject: [PATCH 22/23] pedro --- .../endpoint/view/alerts/url_from_query_params.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts index 6dd8edb2684c0..e037d000e6e8f 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/url_from_query_params.ts @@ -5,7 +5,7 @@ */ import querystring from 'querystring'; -import { AlertingIndexUIQueryParams } from '../../types'; +import { AlertingIndexUIQueryParams, EndpointAppLocation } from '../../types'; /** * Return a relative URL for `AlertingIndexUIQueryParams`. @@ -21,7 +21,11 @@ import { AlertingIndexUIQueryParams } from '../../types'; * // now use relativeURL in the 'href' of a link, the 'to' of a react-router-dom 'Link' or history.push, history.replace * ``` */ -export function urlFromQueryParams(queryParams: AlertingIndexUIQueryParams): string { +export function urlFromQueryParams( + queryParams: AlertingIndexUIQueryParams +): Partial { const search = querystring.stringify(queryParams); - return search === '' ? '' : '?' + search; + return { + search, + }; } From dbffe17375562b2234b69c023522ab555eab370b Mon Sep 17 00:00:00 2001 From: oatkiller Date: Fri, 21 Feb 2020 12:07:27 -0500 Subject: [PATCH 23/23] DO IT --- .../alerts/alert_list_pagination.test.ts | 14 ---- .../store/alerts/mock_alert_result_list.ts | 72 ++++++++++++------- .../endpoint/view/alerts/index.test.tsx | 42 ++++++++++- 3 files changed, 84 insertions(+), 44 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts index 8cb8b96dfefe5..5c257c3d65fdc 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/alert_list_pagination.test.ts @@ -80,20 +80,6 @@ describe('alert list pagination', () => { } `); }); - - // TODO, move this test to react land, since thats where this logic lives now. is that good? - xdescribe('and then a new page size is passed', () => { - beforeEach(() => { - historyPush({ ...queryParams(), page_size: '1' }); - }); - it('should modify the url correctly and reset index to `0`', () => { - expect(queryParams()).toMatchInlineSnapshot(` - Object { - "page_size": "1", - } - `); - }); - }); }); }); }); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/mock_alert_result_list.ts b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/mock_alert_result_list.ts index 4bbb28bf5af3e..338a1077b58a2 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/mock_alert_result_list.ts +++ b/x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/mock_alert_result_list.ts @@ -6,36 +6,54 @@ import { AlertResultList } from '../../../../../common/types'; -export const mockAlertResultList: () => AlertResultList = () => { - const mock: AlertResultList = { - alerts: [ - { - '@timestamp': new Date(1542341895000).toString(), - agent: { - id: 'ced9c68e-b94a-4d66-bb4c-6106514f0a2f', - version: '3.0.0', - }, - event: { - action: 'open', - }, - file_classification: { - malware_classification: { - score: 3, - }, +export const mockAlertResultList: (options?: { + total?: number; + request_page_size?: number; + request_page_index?: number; +}) => AlertResultList = (options = {}) => { + const { + total = 1, + request_page_size: requestPageSize = 10, + request_page_index: requestPageIndex = 0, + } = options; + + // Skip any that are before the page we're on + const numberToSkip = requestPageSize * requestPageIndex; + + // total - numberToSkip is the count of non-skipped ones, but return no more than a pageSize, and no less than 0 + const actualCountToReturn = Math.max(Math.min(total - numberToSkip, requestPageSize), 0); + + const alerts = []; + for (let index = 0; index < actualCountToReturn; index++) { + alerts.push({ + '@timestamp': new Date(1542341895000).toString(), + agent: { + id: 'ced9c68e-b94a-4d66-bb4c-6106514f0a2f', + version: '3.0.0', + }, + event: { + action: 'open', + }, + file_classification: { + malware_classification: { + score: 3, }, - host: { - hostname: 'HD-c15-bc09190a', - ip: '10.179.244.14', - os: { - name: 'Windows', - }, + }, + host: { + hostname: 'HD-c15-bc09190a', + ip: '10.179.244.14', + os: { + name: 'Windows', }, - thread: {}, }, - ], - total: 1, - request_page_size: 10, - request_page_index: 0, + thread: {}, + }); + } + const mock: AlertResultList = { + alerts, + total, + request_page_size: requestPageSize, + request_page_index: requestPageIndex, result_from_index: 0, }; return mock; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx index 0cf4e6efd121a..37847553d512a 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.test.tsx @@ -11,7 +11,7 @@ import { I18nProvider } from '@kbn/i18n/react'; import { AlertIndex } from './index'; import { appStoreFactory } from '../../store'; import { coreMock } from 'src/core/public/mocks'; -import { fireEvent, waitForElement } from '@testing-library/react'; +import { fireEvent, waitForElement, act } from '@testing-library/react'; import { RouteCapture } from '../route_capture'; import { createMemoryHistory, MemoryHistory } from 'history'; import { Router } from 'react-router-dom'; @@ -68,7 +68,10 @@ describe('when on the alerting page', () => { }; queryByTestSubjId = async (renderResult, testSubjId) => { return await waitForElement( - () => renderResult.container.querySelector(`[data-test-subj="${testSubjId}"]`), + /** + * Use document.body instead of container because EUI renders things like popover out of the DOM heirarchy. + */ + () => document.body.querySelector(`[data-test-subj="${testSubjId}"]`), { container: renderResult.container, } @@ -102,7 +105,7 @@ describe('when on the alerting page', () => { /** * There should be a 'row' which is the header, and - * another 'row' which is the alert summary. + * row which is the alert item. */ expect(rows).toHaveLength(2); }); @@ -150,4 +153,37 @@ describe('when on the alerting page', () => { }); }); }); + describe('when the url has page_size=1 and a page_index=1', () => { + beforeEach(() => { + reactTestingLibrary.act(() => { + history.push({ + ...history.location, + search: '?page_size=1&page_index=1', + }); + }); + }); + describe('when the user changes page size to 10', () => { + beforeEach(async () => { + const renderResult = render(); + const paginationButton = await queryByTestSubjId( + renderResult, + 'tablePaginationPopoverButton' + ); + if (paginationButton) { + act(() => { + fireEvent.click(paginationButton); + }); + } + const show10RowsButton = await queryByTestSubjId(renderResult, 'tablePagination-10-rows'); + if (show10RowsButton) { + act(() => { + fireEvent.click(show10RowsButton); + }); + } + }); + it('should have a page_index of 0', () => { + expect(history.location.search).toBe('?page_size=10'); + }); + }); + }); });