[Security Solution] Resolve JS warnings triggered by incorrect state …

…changed (elastic#148552) ## Summary Found these js warning after replacing charts with Lens in elastic#148519: <img width="1671" alt="Screenshot 2023-01-09 at 15 31 17" src="https://user-images.githubusercontent.com/6295984/211345750-8c4e67ee-bf96-49d2-8bb2-0f71e5f9bcd2.png"> Wrap `search.session.start()` with useEffect to avoid incorrect state changed. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Steph Milovic <stephanie.milovic@elastic.co>
jennypavlova · Jan 13, 2023 · 1c7d335 · 1c7d335
1 parent e0c4700
commit 1c7d335
Show file tree

Hide file tree

Showing 23 changed files with 102 additions and 89 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -496,6 +496,7 @@
 /x-pack/plugins/security_solution/public/common/components/navigation @elastic/security-threat-hunting-explore
 /x-pack/plugins/security_solution/public/common/components/news_feed @elastic/security-threat-hunting-explore
 /x-pack/plugins/security_solution/public/common/components/overview_description_list @elastic/security-threat-hunting-explore
+/x-pack/plugins/security_solution/public/common/components/page @elastic/security-threat-hunting-explore
 /x-pack/plugins/security_solution/public/common/components/sidebar_header @elastic/security-threat-hunting-explore
 /x-pack/plugins/security_solution/public/common/components/tables @elastic/security-threat-hunting-explore
 /x-pack/plugins/security_solution/public/common/components/top_n @elastic/security-threat-hunting-explore

diff --git a/x-pack/plugins/security_solution/public/common/components/page/manage_query.tsx b/x-pack/plugins/security_solution/public/common/components/page/manage_query.tsx
@@ -7,8 +7,10 @@
 
 import type { Position } from '@elastic/charts';
 import { omit } from 'lodash/fp';
+import type { MutableRefObject } from 'react';
 import React, { useEffect } from 'react';
 
+import type { ISessionService } from '@kbn/data-plugin/public';
 import type { inputsModel } from '../../store';
 import type { GlobalTimeArgs } from '../../containers/use_global_time';
 import type { InputsModelId } from '../../store/inputs/constants';
@@ -21,22 +23,22 @@ export interface OwnProps extends Pick<GlobalTimeArgs, 'deleteQuery' | 'setQuery
   legendPosition?: Position;
   loading: boolean;
   refetch: inputsModel.Refetch;
-  searchSessionId?: string;
+  session?: MutableRefObject<ISessionService>;
 }
 
 export function manageQuery<T>(
   WrappedComponent: React.ComponentClass<T> | React.ComponentType<T>
 ): React.FC<OwnProps & T> {
   const ManageQuery = (props: OwnProps & T) => {
-    const { deleteQuery, id, inspect = null, loading, refetch, setQuery, searchSessionId } = props;
+    const { deleteQuery, id, inspect = null, loading, refetch, setQuery, session } = props;
 
     useQueryInspector({
       deleteQuery,
       inspect,
       loading,
       queryId: id,
       refetch,
-      searchSessionId,
+      session,
       setQuery,
     });
 
@@ -54,7 +56,7 @@ interface UseQueryInspectorTypes extends Pick<GlobalTimeArgs, 'deleteQuery' | 's
   loading: boolean;
   refetch: inputsModel.Refetch;
   inspect?: inputsModel.InspectQuery | null;
-  searchSessionId?: string;
+  session?: MutableRefObject<ISessionService>;
 }
 
 export const useQueryInspector = ({
@@ -64,11 +66,17 @@ export const useQueryInspector = ({
   inspect,
   loading,
   queryId,
-  searchSessionId,
+  session,
 }: UseQueryInspectorTypes) => {
   useEffect(() => {
-    setQuery({ id: queryId, inspect: inspect ?? null, loading, refetch, searchSessionId });
-  }, [deleteQuery, setQuery, queryId, refetch, inspect, loading, searchSessionId]);
+    setQuery({
+      id: queryId,
+      inspect: inspect ?? null,
+      loading,
+      refetch,
+      searchSessionId: session?.current.start(),
+    });
+  }, [deleteQuery, setQuery, queryId, refetch, inspect, loading, session]);
 
   useEffect(() => {
     return () => {

diff --git a/...k/plugins/security_solution/public/common/components/page/use_refetch_by_session.test.tsx b/...k/plugins/security_solution/public/common/components/page/use_refetch_by_session.test.tsx
@@ -4,6 +4,7 @@
  * 2.0; you may not use this file except in compliance with the Elastic License
  * 2.0.
  */
+import type { MutableRefObject } from 'react';
 import React from 'react';
 import type { RenderHookResult } from '@testing-library/react-hooks';
 import { renderHook } from '@testing-library/react-hooks';
@@ -21,6 +22,7 @@ import { InputsModelId } from '../../store/inputs/constants';
 import { useRefetchByRestartingSession } from './use_refetch_by_session';
 import { inputsActions } from '../../store/actions';
 import type { Refetch } from '../../store/inputs/model';
+import type { ISessionService } from '@kbn/data-plugin/public';
 
 const state: State = mockGlobalState;
 
@@ -60,14 +62,11 @@ describe(`useRefetchByRestartingSession`, () => {
       children: React.ReactNode;
     },
     {
-      searchSessionId: string | undefined;
+      session: MutableRefObject<ISessionService>;
       refetchByRestartingSession: Refetch;
     }
   >;
-  const mockSessionStart = jest
-    .fn()
-    .mockReturnValueOnce('mockSessionId')
-    .mockReturnValue('mockSessionId1');
+  const mockSessionStart = jest.fn().mockReturnValue('mockSessionId');
   const mockSession = {
     start: mockSessionStart,
   };
@@ -97,20 +96,15 @@ describe(`useRefetchByRestartingSession`, () => {
     );
   });
 
-  it('should start a session', () => {
-    expect(mockSessionStart).toHaveBeenCalledTimes(1);
-    expect(res.result.current.searchSessionId).toBe('mockSessionId');
-  });
-
   it('should start a session when clicking refetchByRestartingSession', () => {
     res.result.current.refetchByRestartingSession();
-    expect(mockSessionStart).toHaveBeenCalledTimes(2);
+    expect(mockSessionStart).toHaveBeenCalledTimes(1);
     expect(inputsActions.setInspectionParameter).toHaveBeenCalledWith({
       id: 'test',
       selectedInspectIndex: 0,
       isInspected: false,
       inputId: InputsModelId.global,
-      searchSessionId: 'mockSessionId1',
+      searchSessionId: 'mockSessionId',
     });
   });
 });
diff --git a/x-pack/plugins/security_solution/public/common/components/page/use_refetch_by_session.tsx b/x-pack/plugins/security_solution/public/common/components/page/use_refetch_by_session.tsx
@@ -5,8 +5,10 @@
  * 2.0.
  */
 
-import { useCallback, useRef, useMemo } from 'react';
+import type { MutableRefObject } from 'react';
+import { useCallback, useRef } from 'react';
 import { useDispatch } from 'react-redux';
+import type { ISessionService } from '@kbn/data-plugin/public';
 import { useDeepEqualSelector } from '../../hooks/use_selector';
 import { useKibana } from '../../lib/kibana';
 import { inputsSelectors } from '../../store';
@@ -25,7 +27,7 @@ export const useRefetchByRestartingSession = ({
   queryId,
   skip,
 }: UseRefetchByRestartingSessionProps): {
-  searchSessionId: string | undefined;
+  session: MutableRefObject<ISessionService>;
   refetchByRestartingSession: Refetch;
 } => {
   const dispatch = useDispatch();
@@ -35,16 +37,10 @@ export const useRefetchByRestartingSession = ({
 
   const getGlobalQuery = inputsSelectors.globalQueryByIdSelector();
   const getTimelineQuery = inputsSelectors.timelineQueryByIdSelector();
-  const { selectedInspectIndex, searchSessionId: existingSearchSessionId } = useDeepEqualSelector(
-    (state) =>
-      inputId === InputsModelId.global
-        ? getGlobalQuery(state, queryId)
-        : getTimelineQuery(state, queryId)
-  );
-
-  const searchSessionId = useMemo(
-    () => (skip ? undefined : existingSearchSessionId ?? session.current.start()),
-    [existingSearchSessionId, skip]
+  const { selectedInspectIndex } = useDeepEqualSelector((state) =>
+    inputId === InputsModelId.global
+      ? getGlobalQuery(state, queryId)
+      : getTimelineQuery(state, queryId)
   );
 
   const refetchByRestartingSession = useCallback(() => {
@@ -63,5 +59,8 @@ export const useRefetchByRestartingSession = ({
     );
   }, [dispatch, queryId, selectedInspectIndex, skip]);
 
-  return { searchSessionId, refetchByRestartingSession };
+  return {
+    session,
+    refetchByRestartingSession,
+  };
 };
diff --git a/.../plugins/security_solution/public/explore/hosts/components/kpi_hosts/hosts/index.test.tsx b/.../plugins/security_solution/public/explore/hosts/components/kpi_hosts/hosts/index.test.tsx
@@ -35,6 +35,7 @@ describe('KPI Hosts', () => {
   const MockKpiBaseComponentManage = KpiBaseComponentManage as jest.Mock;
   const mockRefetchByRestartingSession = jest.fn();
   const mockRefetch = jest.fn();
+  const mockSession = { current: { start: jest.fn(() => 'mockNewSearchSessionId') } };
   const defaultProps = {
     from: '2019-06-25T04:31:59.345Z',
     to: '2019-06-25T06:31:59.345Z',
@@ -58,6 +59,7 @@ describe('KPI Hosts', () => {
     ]);
     (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(false);
     (useRefetchByRestartingSession as jest.Mock).mockReturnValue({
+      session: mockSession,
       searchSessionId: 'mockSearchSessionId',
       refetchByRestartingSession: mockRefetchByRestartingSession,
     });
@@ -90,7 +92,7 @@ describe('KPI Hosts', () => {
       </TestProviders>
     );
     expect(MockKpiBaseComponentManage.mock.calls[0][0].refetch).toEqual(mockRefetch);
-    expect(MockKpiBaseComponentManage.mock.calls[0][0].searchSessionId).toBeUndefined();
+    expect(MockKpiBaseComponentManage.mock.calls[0][0].session).toBeUndefined();
   });
   it('Refetch by restarting search session ID if isChartEmbeddablesEnabled = true', () => {
     (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(true);
@@ -104,8 +106,6 @@ describe('KPI Hosts', () => {
     expect(MockKpiBaseComponentManage.mock.calls[0][0].refetch).toEqual(
       mockRefetchByRestartingSession
     );
-    expect(MockKpiBaseComponentManage.mock.calls[0][0].searchSessionId).toEqual(
-      'mockSearchSessionId'
-    );
+    expect(MockKpiBaseComponentManage.mock.calls[0][0].session).toEqual(mockSession);
   });
 });
diff --git a/x-pack/plugins/security_solution/public/explore/hosts/components/kpi_hosts/hosts/index.tsx b/x-pack/plugins/security_solution/public/explore/hosts/components/kpi_hosts/hosts/index.tsx
@@ -63,7 +63,7 @@ const HostsKpiHostsComponent: React.FC<HostsKpiProps> = ({
     skip: querySkip || isChartEmbeddablesEnabled,
   });
 
-  const { searchSessionId, refetchByRestartingSession } = useRefetchByRestartingSession({
+  const { session, refetchByRestartingSession } = useRefetchByRestartingSession({
     inputId: InputsModelId.global,
     queryId: id,
   });
@@ -81,7 +81,7 @@ const HostsKpiHostsComponent: React.FC<HostsKpiProps> = ({
       refetch={isChartEmbeddablesEnabled ? refetchByRestartingSession : refetch}
       setQuery={setQuery}
       setQuerySkip={setQuerySkip}
-      searchSessionId={isChartEmbeddablesEnabled ? searchSessionId : undefined}
+      session={isChartEmbeddablesEnabled ? session : undefined}
     />
   );
 };

diff --git a/...ins/security_solution/public/explore/hosts/components/kpi_hosts/unique_ips/index.test.tsx b/...ins/security_solution/public/explore/hosts/components/kpi_hosts/unique_ips/index.test.tsx
@@ -34,6 +34,7 @@ describe('KPI Unique IPs', () => {
   const mockUseQueryToggle = useQueryToggle as jest.Mock;
   const MockKpiBaseComponentManage = KpiBaseComponentManage as jest.Mock;
   const mockRefetchByRestartingSession = jest.fn();
+  const mockSession = { current: { start: jest.fn(() => 'mockNewSearchSessionId') } };
   const mockRefetch = jest.fn();
   const defaultProps = {
     from: '2019-06-25T04:31:59.345Z',
@@ -58,6 +59,7 @@ describe('KPI Unique IPs', () => {
     ]);
     (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(false);
     (useRefetchByRestartingSession as jest.Mock).mockReturnValue({
+      session: mockSession,
       searchSessionId: 'mockSearchSessionId',
       refetchByRestartingSession: mockRefetchByRestartingSession,
     });
@@ -90,7 +92,7 @@ describe('KPI Unique IPs', () => {
       </TestProviders>
     );
     expect(MockKpiBaseComponentManage.mock.calls[0][0].refetch).toEqual(mockRefetch);
-    expect(MockKpiBaseComponentManage.mock.calls[0][0].searchSessionId).toBeUndefined();
+    expect(MockKpiBaseComponentManage.mock.calls[0][0].session).toBeUndefined();
   });
   it('Refetch by restarting search session ID if isChartEmbeddablesEnabled = true', () => {
     (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(true);
@@ -104,8 +106,6 @@ describe('KPI Unique IPs', () => {
     expect(MockKpiBaseComponentManage.mock.calls[0][0].refetch).toEqual(
       mockRefetchByRestartingSession
     );
-    expect(MockKpiBaseComponentManage.mock.calls[0][0].searchSessionId).toEqual(
-      'mockSearchSessionId'
-    );
+    expect(MockKpiBaseComponentManage.mock.calls[0][0].session).toEqual(mockSession);
   });
 });
diff --git a/.../plugins/security_solution/public/explore/hosts/components/kpi_hosts/unique_ips/index.tsx b/.../plugins/security_solution/public/explore/hosts/components/kpi_hosts/unique_ips/index.tsx
@@ -78,7 +78,7 @@ const HostsKpiUniqueIpsComponent: React.FC<HostsKpiProps> = ({
     skip: querySkip || isChartEmbeddablesEnabled,
   });
 
-  const { searchSessionId, refetchByRestartingSession } = useRefetchByRestartingSession({
+  const { session, refetchByRestartingSession } = useRefetchByRestartingSession({
     inputId: InputsModelId.global,
     queryId: id,
   });
@@ -96,7 +96,7 @@ const HostsKpiUniqueIpsComponent: React.FC<HostsKpiProps> = ({
       refetch={isChartEmbeddablesEnabled ? refetchByRestartingSession : refetch}
       setQuery={setQuery}
       setQuerySkip={setQuerySkip}
-      searchSessionId={isChartEmbeddablesEnabled ? searchSessionId : undefined}
+      session={isChartEmbeddablesEnabled ? session : undefined}
     />
   );
 };

diff --git a/...lugins/security_solution/public/explore/network/components/kpi_network/dns/index.test.tsx b/...lugins/security_solution/public/explore/network/components/kpi_network/dns/index.test.tsx
@@ -34,6 +34,7 @@ describe('DNS KPI', () => {
   const mockUseQueryToggle = useQueryToggle as jest.Mock;
   const MockKpiBaseComponentManage = KpiBaseComponentManage as jest.Mock;
   const mockRefetchByRestartingSession = jest.fn();
+  const mockSession = { current: { start: jest.fn(() => 'mockNewSearchSessionId') } };
   const mockRefetch = jest.fn();
   const defaultProps = {
     from: '2019-06-25T04:31:59.345Z',
@@ -59,6 +60,7 @@ describe('DNS KPI', () => {
     ]);
     (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(false);
     (useRefetchByRestartingSession as jest.Mock).mockReturnValue({
+      session: mockSession,
       searchSessionId: 'mockSearchSessionId',
       refetchByRestartingSession: mockRefetchByRestartingSession,
     });
@@ -91,7 +93,7 @@ describe('DNS KPI', () => {
       </TestProviders>
     );
     expect(MockKpiBaseComponentManage.mock.calls[0][0].refetch).toEqual(mockRefetch);
-    expect(MockKpiBaseComponentManage.mock.calls[0][0].searchSessionId).toBeUndefined();
+    expect(MockKpiBaseComponentManage.mock.calls[0][0].session).toBeUndefined();
   });
   it('Refetch by restarting search session ID if isChartEmbeddablesEnabled = true', () => {
     (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(true);
@@ -105,8 +107,6 @@ describe('DNS KPI', () => {
     expect(MockKpiBaseComponentManage.mock.calls[0][0].refetch).toEqual(
       mockRefetchByRestartingSession
     );
-    expect(MockKpiBaseComponentManage.mock.calls[0][0].searchSessionId).toEqual(
-      'mockSearchSessionId'
-    );
+    expect(MockKpiBaseComponentManage.mock.calls[0][0].session).toEqual(mockSession);
   });
 });
diff --git a/x-pack/plugins/security_solution/public/explore/network/components/kpi_network/dns/index.tsx b/x-pack/plugins/security_solution/public/explore/network/components/kpi_network/dns/index.tsx
@@ -58,7 +58,7 @@ const NetworkKpiDnsComponent: React.FC<NetworkKpiProps> = ({
     skip: querySkip || isChartEmbeddablesEnabled,
   });
 
-  const { searchSessionId, refetchByRestartingSession } = useRefetchByRestartingSession({
+  const { session, refetchByRestartingSession } = useRefetchByRestartingSession({
     inputId: InputsModelId.global,
     queryId: id,
   });
@@ -76,7 +76,7 @@ const NetworkKpiDnsComponent: React.FC<NetworkKpiProps> = ({
       refetch={isChartEmbeddablesEnabled ? refetchByRestartingSession : refetch}
       setQuery={setQuery}
       setQuerySkip={setQuerySkip}
-      searchSessionId={isChartEmbeddablesEnabled ? searchSessionId : undefined}
+      session={isChartEmbeddablesEnabled ? session : undefined}
     />
   );
 };

diff --git a/...rity_solution/public/explore/network/components/kpi_network/network_events/index.test.tsx b/...rity_solution/public/explore/network/components/kpi_network/network_events/index.test.tsx
@@ -34,6 +34,7 @@ describe('Network Events KPI', () => {
   const mockUseQueryToggle = useQueryToggle as jest.Mock;
   const MockKpiBaseComponentManage = KpiBaseComponentManage as jest.Mock;
   const mockRefetchByRestartingSession = jest.fn();
+  const mockSession = { current: { start: jest.fn(() => 'mockNewSearchSessionId') } };
   const mockRefetch = jest.fn();
   const defaultProps = {
     from: '2019-06-25T04:31:59.345Z',
@@ -58,6 +59,7 @@ describe('Network Events KPI', () => {
     ]);
     (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(false);
     (useRefetchByRestartingSession as jest.Mock).mockReturnValue({
+      session: mockSession,
       searchSessionId: 'mockSearchSessionId',
       refetchByRestartingSession: mockRefetchByRestartingSession,
     });
@@ -90,7 +92,7 @@ describe('Network Events KPI', () => {
       </TestProviders>
     );
     expect(MockKpiBaseComponentManage.mock.calls[0][0].refetch).toEqual(mockRefetch);
-    expect(MockKpiBaseComponentManage.mock.calls[0][0].searchSessionId).toBeUndefined();
+    expect(MockKpiBaseComponentManage.mock.calls[0][0].session).toBeUndefined();
   });
   it('Refetch by restarting search session ID if isChartEmbeddablesEnabled = true', () => {
     (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(true);
@@ -104,8 +106,6 @@ describe('Network Events KPI', () => {
     expect(MockKpiBaseComponentManage.mock.calls[0][0].refetch).toEqual(
       mockRefetchByRestartingSession
     );
-    expect(MockKpiBaseComponentManage.mock.calls[0][0].searchSessionId).toEqual(
-      'mockSearchSessionId'
-    );
+    expect(MockKpiBaseComponentManage.mock.calls[0][0].session).toEqual(mockSession);
   });
 });
diff --git a/.../security_solution/public/explore/network/components/kpi_network/network_events/index.tsx b/.../security_solution/public/explore/network/components/kpi_network/network_events/index.tsx
@@ -62,7 +62,7 @@ const NetworkKpiNetworkEventsComponent: React.FC<NetworkKpiProps> = ({
     skip: querySkip || isChartEmbeddablesEnabled,
   });
 
-  const { searchSessionId, refetchByRestartingSession } = useRefetchByRestartingSession({
+  const { session, refetchByRestartingSession } = useRefetchByRestartingSession({
     inputId: InputsModelId.global,
     queryId: id,
   });
@@ -80,7 +80,7 @@ const NetworkKpiNetworkEventsComponent: React.FC<NetworkKpiProps> = ({
       refetch={isChartEmbeddablesEnabled ? refetchByRestartingSession : refetch}
       setQuery={setQuery}
       setQuerySkip={setQuerySkip}
-      searchSessionId={isChartEmbeddablesEnabled ? searchSessionId : undefined}
+      session={isChartEmbeddablesEnabled ? session : undefined}
     />
   );
 };