From 2ee2bd8345601aaee0fe556743394b99a939d254 Mon Sep 17 00:00:00 2001 From: Pablo Neves Machado Date: Fri, 30 Sep 2022 17:13:57 +0200 Subject: [PATCH 1/4] Refactor global_query_string to move reusabel code to helper --- .../utils/global_query_string/helpers.ts | 62 ++++++++++++++- .../utils/global_query_string/index.test.tsx | 30 +++++++- .../common/utils/global_query_string/index.ts | 75 +++++++------------ 3 files changed, 116 insertions(+), 51 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts b/x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts index 63684f1985049..3287683e30f29 100644 --- a/x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts +++ b/x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts @@ -5,8 +5,12 @@ * 2.0. */ -import { parse } from 'query-string'; import { decode, encode } from 'rison-node'; +import type { ParsedQuery } from 'query-string'; +import { parse, stringify } from 'query-string'; +import { url } from '@kbn/kibana-utils-plugin/public'; +import { useHistory } from 'react-router-dom'; +import { useCallback, useMemo } from 'react'; import { SecurityPageName } from '../../../app/types'; export const isDetectionsPages = (pageName: string) => @@ -40,3 +44,59 @@ export const getParamFromQueryString = ( return Array.isArray(queryParam) ? queryParam[0] : queryParam; }; + +/** + * + * Gets the value of the URL param from the query string. + * It doesn't update when the URL changes. + * + */ +export const useInitialUrlParamValue = (urlParamKey: string) => { + // window.location.search provides the most updated representation of the url search. + // It also guarantees that we don't overwrite URL param managed outside react-router. + const result = useMemo(() => { + const param = getParamFromQueryString( + getQueryStringFromLocation(window.location.search), + urlParamKey + ); + + const decodedParam = decodeRisonUrlState(param ?? undefined); + + return { param, decodedParam }; + }, [urlParamKey]); + + return result; +}; + +export const encodeQueryString = (urlParams: ParsedQuery): string => + stringify(url.encodeQuery(urlParams), { sort: false, encode: false }); + +export const useReplaceUrlParams = () => { + const history = useHistory(); + + const replaceUrlParams = useCallback( + (params: Array<{ key: string; value: string | null }>) => { + // window.location.search provides the most updated representation of the url search. + // It prevents unnecessary re-renders which useLocation would create because 'replaceUrlParams' does update the location. + // window.location.search also guarantees that we don't overwrite URL param managed outside react-router. + const search = window.location.search; + const urlParams = parse(search, { sort: false }); + + params.forEach(({ key, value }) => { + if (value == null || value === '') { + delete urlParams[key]; + } else { + urlParams[key] = value; + } + }); + + const newSearch = encodeQueryString(urlParams); + + if (getQueryStringFromLocation(search) !== newSearch) { + history.replace({ search: newSearch }); + } + }, + [history] + ); + return replaceUrlParams; +}; diff --git a/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.test.tsx b/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.test.tsx index a400ab24e06dd..dede5125775c4 100644 --- a/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.test.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { renderHook } from '@testing-library/react-hooks'; +import { act, renderHook } from '@testing-library/react-hooks'; import { useInitializeUrlParam, useGlobalQueryString, @@ -296,5 +296,33 @@ describe('global query string', () => { expect(mockHistory.replace).not.toHaveBeenCalledWith(); }); + + it('deletes unregistered URL params', async () => { + const urlParamKey = 'testKey'; + const value = '123'; + window.location.search = `?${urlParamKey}=${value}`; + const globalUrlParam = { + [urlParamKey]: value, + }; + const store = makeStore(globalUrlParam); + + const { waitForNextUpdate } = renderHook(() => useSyncGlobalQueryString(), { + wrapper: ({ children }: { children: React.ReactElement }) => ( + {children} + ), + }); + + mockHistory.replace.mockClear(); + + act(() => { + store.dispatch(globalUrlParamActions.deregisterUrlParam({ key: urlParamKey })); + }); + + waitForNextUpdate(); + + expect(mockHistory.replace).toHaveBeenCalledWith({ + search: ``, + }); + }); }); }); diff --git a/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts b/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts index 09588c7298c09..101f83f1a27a5 100644 --- a/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts +++ b/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts @@ -5,20 +5,15 @@ * 2.0. */ -import type * as H from 'history'; -import type { ParsedQuery } from 'query-string'; -import { parse, stringify } from 'query-string'; import { useCallback, useEffect, useMemo } from 'react'; - -import { url } from '@kbn/kibana-utils-plugin/public'; -import { isEmpty, pickBy } from 'lodash/fp'; -import { useHistory } from 'react-router-dom'; +import { difference, isEmpty, pickBy } from 'lodash/fp'; import { useDispatch } from 'react-redux'; +import usePrevious from 'react-use/lib/usePrevious'; import { - decodeRisonUrlState, + encodeQueryString, encodeRisonUrlState, - getParamFromQueryString, - getQueryStringFromLocation, + useInitialUrlParamValue, + useReplaceUrlParams, } from './helpers'; import { useShallowEqualSelector } from '../../hooks/use_selector'; import { globalUrlParamActions, globalUrlParamSelectors } from '../../store/global_url_param'; @@ -42,15 +37,10 @@ export const useInitializeUrlParam = ( onInitialize: (state: State | null) => void ) => { const dispatch = useDispatch(); + const { param: initialValue, decodedParam: decodedInitialValue } = + useInitialUrlParamValue(urlParamKey); useEffect(() => { - // window.location.search provides the most updated representation of the url search. - // It also guarantees that we don't overwrite URL param managed outside react-router. - const initialValue = getParamFromQueryString( - getQueryStringFromLocation(window.location.search), - urlParamKey - ); - dispatch( globalUrlParamActions.registerUrlParam({ key: urlParamKey, @@ -59,7 +49,7 @@ export const useInitializeUrlParam = ( ); // execute consumer initialization - onInitialize(decodeRisonUrlState(initialValue ?? undefined)); + onInitialize(decodedInitialValue); return () => { dispatch(globalUrlParamActions.deregisterUrlParam({ key: urlParamKey })); @@ -103,9 +93,16 @@ export const useGlobalQueryString = (): string => { * - It updates the URL when globalUrlParam store updates. */ export const useSyncGlobalQueryString = () => { - const history = useHistory(); const [{ pageName }] = useRouteSpy(); const globalUrlParam = useShallowEqualSelector(globalUrlParamSelectors.selectGlobalUrlParam); + const previousGlobalUrlParams = usePrevious(globalUrlParam); + const replaceUrlParams = useReplaceUrlParams(); + + // Url params that got deleted from GlobalUrlParams + const unregisteredKeys = useMemo( + () => difference(Object.keys(previousGlobalUrlParams ?? {}), Object.keys(globalUrlParam)), + [previousGlobalUrlParams, globalUrlParam] + ); useEffect(() => { const linkInfo = getLinkInfo(pageName) ?? { skipUrlState: true }; @@ -114,36 +111,16 @@ export const useSyncGlobalQueryString = () => { value: linkInfo.skipUrlState ? null : value, })); - if (params.length > 0) { - // window.location.search provides the most updated representation of the url search. - // It prevents unnecessary re-renders which useLocation would create because 'replaceUrlParams' does update the location. - // window.location.search also guarantees that we don't overwrite URL param managed outside react-router. - replaceUrlParams(params, history, window.location.search); - } - }, [globalUrlParam, pageName, history]); -}; - -const encodeQueryString = (urlParams: ParsedQuery): string => - stringify(url.encodeQuery(urlParams), { sort: false, encode: false }); - -const replaceUrlParams = ( - params: Array<{ key: string; value: string | null }>, - history: H.History, - search: string -) => { - const urlParams = parse(search, { sort: false }); + // Delete unregistered Url params + unregisteredKeys.forEach((key) => { + params.push({ + key, + value: null, + }); + }); - params.forEach(({ key, value }) => { - if (value == null || value === '') { - delete urlParams[key]; - } else { - urlParams[key] = value; + if (params.length > 0) { + replaceUrlParams(params); } - }); - - const newSearch = encodeQueryString(urlParams); - - if (getQueryStringFromLocation(search) !== newSearch) { - history.replace({ search: newSearch }); - } + }, [globalUrlParam, pageName, unregisteredKeys, replaceUrlParams]); }; From 05fbdbfe775d96a4919dccf55afa7b41a9ba0e37 Mon Sep 17 00:00:00 2001 From: Pablo Neves Machado Date: Fri, 30 Sep 2022 17:14:36 +0200 Subject: [PATCH 2/4] Add Url state parameter for external alerts checkbox --- .../events_tab/events_query_tab_body.tsx | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx b/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx index db663c1f0fc6d..25ec0f59a0b70 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx @@ -42,6 +42,11 @@ import { useLicense } from '../../hooks/use_license'; import { useUiSetting$ } from '../../lib/kibana'; import { defaultAlertsFilters } from '../events_viewer/external_alerts_filter'; +import { + useInitialUrlParamValue, + useReplaceUrlParams, +} from '../../utils/global_query_string/helpers'; + export const ALERTS_EVENTS_HISTOGRAM_ID = 'alertsOrEventsHistogramQuery'; type QueryTabBodyProps = UserQueryTabBodyProps | HostQueryTabBodyProps | NetworkQueryTabBodyProps; @@ -55,6 +60,8 @@ export type EventsQueryTabBodyComponentProps = QueryTabBodyProps & { timelineId: TimelineId; }; +const EXTERNAL_ALERTS_URL_PARAM = 'onlyExternalAlerts'; + const EventsQueryTabBodyComponent: React.FC = ({ deleteQuery, endDate, @@ -70,13 +77,41 @@ const EventsQueryTabBodyComponent: React.FC = const { globalFullScreen } = useGlobalFullScreen(); const tGridEnabled = useIsExperimentalFeatureEnabled('tGridEnabled'); const [defaultNumberFormat] = useUiSetting$(DEFAULT_NUMBER_FORMAT); - const [showExternalAlerts, setShowExternalAlerts] = useState(false); const isEnterprisePlus = useLicense().isEnterprise(); const ACTION_BUTTON_COUNT = isEnterprisePlus ? 5 : 4; const leadingControlColumns = useMemo( () => getDefaultControlColumn(ACTION_BUTTON_COUNT), [ACTION_BUTTON_COUNT] ); + const replaceUrlParams = useReplaceUrlParams(); + + const { decodedParam: showExternalAlertsInitialUrlState } = + useInitialUrlParamValue(EXTERNAL_ALERTS_URL_PARAM); + + const [showExternalAlerts, setShowExternalAlerts] = useState( + showExternalAlertsInitialUrlState ?? false + ); + + useEffect(() => { + replaceUrlParams([ + { + key: EXTERNAL_ALERTS_URL_PARAM, + value: showExternalAlerts ? 'true' : null, + }, + ]); + }, [showExternalAlerts, replaceUrlParams]); + + useEffect(() => { + // Only called on component unmount + return () => { + replaceUrlParams([ + { + key: EXTERNAL_ALERTS_URL_PARAM, + value: null, + }, + ]); + }; + }, [replaceUrlParams]); const toggleExternalAlerts = useCallback(() => setShowExternalAlerts((s) => !s), []); const getHistogramSubtitle = useMemo( From bcbebc1437bb54092bba161a9942607a8b2f8e65 Mon Sep 17 00:00:00 2001 From: Pablo Neves Machado Date: Mon, 3 Oct 2022 12:14:13 +0200 Subject: [PATCH 3/4] Fix bug found by integration test and improve code --- .../events_tab/events_query_tab_body.tsx | 67 ++++++++++++------- .../utils/global_query_string/helpers.ts | 8 +-- .../common/utils/global_query_string/index.ts | 8 ++- 3 files changed, 52 insertions(+), 31 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx b/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx index 25ec0f59a0b70..07c9995ddbd9e 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx @@ -43,7 +43,7 @@ import { useUiSetting$ } from '../../lib/kibana'; import { defaultAlertsFilters } from '../events_viewer/external_alerts_filter'; import { - useInitialUrlParamValue, + useGetInitialUrlParamValue, useReplaceUrlParams, } from '../../utils/global_query_string/helpers'; @@ -83,35 +83,14 @@ const EventsQueryTabBodyComponent: React.FC = () => getDefaultControlColumn(ACTION_BUTTON_COUNT), [ACTION_BUTTON_COUNT] ); - const replaceUrlParams = useReplaceUrlParams(); - const { decodedParam: showExternalAlertsInitialUrlState } = - useInitialUrlParamValue(EXTERNAL_ALERTS_URL_PARAM); + const showExternalAlertsInitialUrlState = useExternalAlertsInitialUrlState(); const [showExternalAlerts, setShowExternalAlerts] = useState( showExternalAlertsInitialUrlState ?? false ); - useEffect(() => { - replaceUrlParams([ - { - key: EXTERNAL_ALERTS_URL_PARAM, - value: showExternalAlerts ? 'true' : null, - }, - ]); - }, [showExternalAlerts, replaceUrlParams]); - - useEffect(() => { - // Only called on component unmount - return () => { - replaceUrlParams([ - { - key: EXTERNAL_ALERTS_URL_PARAM, - value: null, - }, - ]); - }; - }, [replaceUrlParams]); + useSyncExternalAlertsUrlState(showExternalAlerts); const toggleExternalAlerts = useCallback(() => setShowExternalAlerts((s) => !s), []); const getHistogramSubtitle = useMemo( @@ -213,3 +192,43 @@ EventsQueryTabBodyComponent.displayName = 'EventsQueryTabBodyComponent'; export const EventsQueryTabBody = React.memo(EventsQueryTabBodyComponent); EventsQueryTabBody.displayName = 'EventsQueryTabBody'; + +const useExternalAlertsInitialUrlState = () => { + const replaceUrlParams = useReplaceUrlParams(); + + const getInitialUrlParamValue = useGetInitialUrlParamValue(EXTERNAL_ALERTS_URL_PARAM); + + const { decodedParam: showExternalAlertsInitialUrlState } = useMemo( + () => getInitialUrlParamValue(), + [getInitialUrlParamValue] + ); + + useEffect(() => { + // Only called on component unmount + return () => { + replaceUrlParams([ + { + key: EXTERNAL_ALERTS_URL_PARAM, + value: null, + }, + ]); + }; + }, [replaceUrlParams]); + + return showExternalAlertsInitialUrlState; +}; + +/** + * Update URL state when showExternalAlerts value changes + */ +const useSyncExternalAlertsUrlState = (showExternalAlerts: boolean) => { + const replaceUrlParams = useReplaceUrlParams(); + useEffect(() => { + replaceUrlParams([ + { + key: EXTERNAL_ALERTS_URL_PARAM, + value: showExternalAlerts ? 'true' : null, + }, + ]); + }, [showExternalAlerts, replaceUrlParams]); +}; diff --git a/x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts b/x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts index 3287683e30f29..a5f7e93146750 100644 --- a/x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts +++ b/x-pack/plugins/security_solution/public/common/utils/global_query_string/helpers.ts @@ -10,7 +10,7 @@ import type { ParsedQuery } from 'query-string'; import { parse, stringify } from 'query-string'; import { url } from '@kbn/kibana-utils-plugin/public'; import { useHistory } from 'react-router-dom'; -import { useCallback, useMemo } from 'react'; +import { useCallback } from 'react'; import { SecurityPageName } from '../../../app/types'; export const isDetectionsPages = (pageName: string) => @@ -51,10 +51,10 @@ export const getParamFromQueryString = ( * It doesn't update when the URL changes. * */ -export const useInitialUrlParamValue = (urlParamKey: string) => { +export const useGetInitialUrlParamValue = (urlParamKey: string) => { // window.location.search provides the most updated representation of the url search. // It also guarantees that we don't overwrite URL param managed outside react-router. - const result = useMemo(() => { + const getInitialUrlParamValue = useCallback(() => { const param = getParamFromQueryString( getQueryStringFromLocation(window.location.search), urlParamKey @@ -65,7 +65,7 @@ export const useInitialUrlParamValue = (urlParamKey: string) => { return { param, decodedParam }; }, [urlParamKey]); - return result; + return getInitialUrlParamValue; }; export const encodeQueryString = (urlParams: ParsedQuery): string => diff --git a/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts b/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts index 101f83f1a27a5..96834d39fd644 100644 --- a/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts +++ b/x-pack/plugins/security_solution/public/common/utils/global_query_string/index.ts @@ -12,7 +12,7 @@ import usePrevious from 'react-use/lib/usePrevious'; import { encodeQueryString, encodeRisonUrlState, - useInitialUrlParamValue, + useGetInitialUrlParamValue, useReplaceUrlParams, } from './helpers'; import { useShallowEqualSelector } from '../../hooks/use_selector'; @@ -37,10 +37,12 @@ export const useInitializeUrlParam = ( onInitialize: (state: State | null) => void ) => { const dispatch = useDispatch(); - const { param: initialValue, decodedParam: decodedInitialValue } = - useInitialUrlParamValue(urlParamKey); + + const getInitialUrlParamValue = useGetInitialUrlParamValue(urlParamKey); useEffect(() => { + const { param: initialValue, decodedParam: decodedInitialValue } = getInitialUrlParamValue(); + dispatch( globalUrlParamActions.registerUrlParam({ key: urlParamKey, From dae05dfed589501314dbaa66ae2d62122e217d14 Mon Sep 17 00:00:00 2001 From: Pablo Neves Machado Date: Mon, 3 Oct 2022 13:20:06 +0200 Subject: [PATCH 4/4] Fix unit test --- .../components/events_tab/events_query_tab_body.test.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.test.tsx b/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.test.tsx index 0b13363a653a0..cb12aaa7f0e79 100644 --- a/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.test.tsx @@ -15,6 +15,7 @@ import { EventsQueryTabBody, ALERTS_EVENTS_HISTOGRAM_ID } from './events_query_t import { useGlobalFullScreen } from '../../containers/use_full_screen'; import * as tGridActions from '@kbn/timelines-plugin/public/store/t_grid/actions'; import { licenseService } from '../../hooks/use_license'; +import { mockHistory } from '../../mock/router'; const mockGetDefaultControlColumn = jest.fn(); jest.mock('../../../timelines/components/timeline/body/control_columns', () => ({ @@ -39,6 +40,11 @@ jest.mock('../../lib/kibana', () => { }; }); +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), + useHistory: () => mockHistory, +})); + const FakeStatefulEventsViewer = ({ additionalFilters }: { additionalFilters: JSX.Element }) => (
{additionalFilters}