From 96316ebbdea6bc79cf5a0dbbb5df5836c55c2c9c Mon Sep 17 00:00:00 2001 From: Andrew Goldstein Date: Tue, 4 Feb 2020 13:39:38 -0700 Subject: [PATCH] ## Fixes an issue where the `Signals count` spinner can spin forever Per the animated gif below, in `7.6` `BC 4`, the `Signals count` spinner on the Overview page spins forever until the signals index is created (in the current Kibana space): ![signals-count-loading-spinner](https://user-images.githubusercontent.com/4459398/73785251-2ca42000-4754-11ea-8671-daa81f351c9b.gif) The `Signals count` spinner will spin forever until the user clicks the `Detections` tab, which-in turn creates the signals index (if it doesn't exist), per the animated gif below: ![create-signals-index](https://user-images.githubusercontent.com/4459398/73785319-4ba2b200-4754-11ea-9bb0-a745a8b2be5d.gif) This behavior is an issue because: - When a fresh deployment is created on Elastic Cloud, a user won't understand why the `Signals count` widget is always spinning on the `Overview` page. (The user must click the `Detections` page to resolve this.) - In deployments where authentication is disabled, or, for _reasons_, a Detections index will never be created, the `Signals count` spinner on the Overview page will always spin. To reproduce: 1. Spin up a new `7.6` `BC 4` deployment on Elastic Cloud 2. Login to Kibana for the first time 3. Navigate to the SIEM app **Expected result** - All histograms on the Overview page eventually stop displaying their respective loading spinners **Actual result** - The `Signals count` widget spinner spins forever. (The user must click the `Detections` tab to create the signals index.) ## Deleting the signals index To reproduce the issue above when a signals index has already been created (by clicking on the Detections tab), run the following from the Kibana `Dev Tools` `Console`: ``` DELETE /.siem-signals-default-000001 ``` It is also possible to reproduce this issue by creating a new space, because it won't have a signals index. https://github.com/elastic/siem-team/issues/514 --- .../signals_histogram_panel/helpers.test.tsx | 35 +++++++++++++++++++ .../signals_histogram_panel/helpers.tsx | 14 ++++++++ .../signals_histogram_panel/index.tsx | 16 +++++---- .../detection_engine/detection_engine.tsx | 1 - 4 files changed, 59 insertions(+), 7 deletions(-) create mode 100644 x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.test.tsx diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.test.tsx b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.test.tsx new file mode 100644 index 0000000000000..2758625c0d4af --- /dev/null +++ b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.test.tsx @@ -0,0 +1,35 @@ +/* + * 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 { showInitialLoadingSpinner } from './helpers'; + +describe('helpers', () => { + describe('showInitialLoadingSpinner', () => { + test('it should (only) show the spinner during initial loading, while we are fetching data', () => { + expect(showInitialLoadingSpinner({ isInitialLoading: true, isLoadingSignals: true })).toBe( + true + ); + }); + + test('it should STOP showing the spinner (during initial loading) when the first data fetch completes', () => { + expect(showInitialLoadingSpinner({ isInitialLoading: true, isLoadingSignals: false })).toBe( + false + ); + }); + + test('it should NOT show the spinner after initial loading has completed, even if the user requests more data (e.g. by clicking Refresh)', () => { + expect(showInitialLoadingSpinner({ isInitialLoading: false, isLoadingSignals: true })).toBe( + false + ); + }); + + test('it should NOT show the spinner after initial loading has completed', () => { + expect(showInitialLoadingSpinner({ isInitialLoading: false, isLoadingSignals: false })).toBe( + false + ); + }); + }); +}); diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.tsx b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.tsx index 551850fa610db..27ee552146092 100644 --- a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.tsx +++ b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/helpers.tsx @@ -76,3 +76,17 @@ export const getSignalsHistogramQuery = ( }, }, }); + +/** + * Returns `true` when the signals histogram initial loading spinner should be shown + * + * @param isInitialLoading The loading spinner will only be displayed if this value is `true`, because after initial load, a different, non-spinner loading indicator is displayed + * @param isLoadingSignals When `true`, IO is being performed to request signals (for rendering in the histogram) + */ +export const showInitialLoadingSpinner = ({ + isInitialLoading, + isLoadingSignals, +}: { + isInitialLoading: boolean; + isLoadingSignals: boolean; +}): boolean => isInitialLoading && isLoadingSignals; diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/index.tsx b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/index.tsx index 29aaa951ff71a..4de471d6733cf 100644 --- a/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/index.tsx +++ b/x-pack/legacy/plugins/siem/public/pages/detection_engine/components/signals_histogram_panel/index.tsx @@ -23,7 +23,7 @@ import { InspectButtonContainer } from '../../../../components/inspect'; import { useQuerySignals } from '../../../../containers/detection_engine/signals/use_query'; import { MatrixLoader } from '../../../../components/matrix_histogram/matrix_loader'; -import { formatSignalsData, getSignalsHistogramQuery } from './helpers'; +import { formatSignalsData, getSignalsHistogramQuery, showInitialLoadingSpinner } from './helpers'; import * as i18n from './translations'; const DEFAULT_PANEL_HEIGHT = 300; @@ -54,7 +54,6 @@ interface SignalsHistogramPanelProps { from: number; query?: Query; legendPosition?: Position; - loadingInitial?: boolean; panelHeight?: number; signalIndexName: string | null; setQuery: (params: RegisterQuery) => void; @@ -75,7 +74,6 @@ export const SignalsHistogramPanel = memo( query, from, legendPosition = 'right', - loadingInitial = false, panelHeight = DEFAULT_PANEL_HEIGHT, setQuery, signalIndexName, @@ -86,7 +84,7 @@ export const SignalsHistogramPanel = memo( title = i18n.HISTOGRAM_HEADER, updateDateRange, }) => { - const [isInitialLoading, setIsInitialLoading] = useState(loadingInitial || true); + const [isInitialLoading, setIsInitialLoading] = useState(true); const [defaultNumberFormat] = useUiSetting$(DEFAULT_NUMBER_FORMAT); const [totalSignalsObj, setTotalSignalsObj] = useState(defaultTotalSignalsObj); const [selectedStackByOption, setSelectedStackByOption] = useState( @@ -124,10 +122,16 @@ export const SignalsHistogramPanel = memo( const formattedSignalsData = useMemo(() => formatSignalsData(signalsData), [signalsData]); useEffect(() => { - if (!loadingInitial && isInitialLoading && !isLoadingSignals && signalsData) { + let canceled = false; + + if (!canceled && !showInitialLoadingSpinner({ isInitialLoading, isLoadingSignals })) { setIsInitialLoading(false); } - }, [loadingInitial, isLoadingSignals, signalsData]); + + return () => { + canceled = true; // prevent long running data fetches from updating state after unmounting + }; + }, [isInitialLoading, isLoadingSignals, setIsInitialLoading]); useEffect(() => { return () => { diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/detection_engine.tsx b/x-pack/legacy/plugins/siem/public/pages/detection_engine/detection_engine.tsx index d854c377e6ec8..ff6722840fd6b 100644 --- a/x-pack/legacy/plugins/siem/public/pages/detection_engine/detection_engine.tsx +++ b/x-pack/legacy/plugins/siem/public/pages/detection_engine/detection_engine.tsx @@ -177,7 +177,6 @@ const DetectionEnginePageComponent: React.FC deleteQuery={deleteQuery} filters={filters} from={from} - loadingInitial={loading} query={query} setQuery={setQuery} showTotalSignalsCount={true}