From 78b4c9b9781cc34e7493b8293ce5fb921e7b39cf Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Wed, 18 Jan 2023 09:56:43 -0400 Subject: [PATCH] [Unified Histogram] [Discover] Unified Histogram solutions cleanup (#146352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This PR includes a number of updates to Unified Histogram to prepare for sharing with solutions, as well as updating its usage in Discover: Unified Histogram: - Adds a `disableAutoFetching` prop, similar to Unified Field List, to disable automatic Unified Histogram refetching based on prop changes. - Accepts an `input$` observable prop that can be used to control manual refetches. - Removes `refetchId` used internally and replaces it with an observables based approach to simplify refetch logic. - Introduces a `use_stable_callback` utility hook to create callback functions with stable identities which simplifies `useCallback` logic — should be replaced with `useEvent` or whatever the React team comes up with to solve this specific issue when available: https://github.com/reactjs/rfcs/pull/220. - Eliminates debouncing logic in Unified Histogram since it was hacky — manual refetching should be used instead if debouncing is needed. - Accepts `query`, `filters`, and `timeRange` props to remove dependencies on specific services. - Updates `use_request_params` to export `getTimeRange` and `updateTimeRange` functions for easier absolute time range handling. - Exposes some Lens embeddable props to allow further customizing Unified Histogram behaviour. Discover: - Exports a `fetch$` observable from `use_saved_search` to allow listening for when data fetches should be triggered. - Uses new manual refetching support in Unified Histogram to simplify refetching logic. - Passes `query`, `filters`, and `timeRange` props to Unified Histogram. ### Checklist - [ ] ~Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~ - [ ] ~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials~ - [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 - [ ] ~Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/))~ - [ ] ~Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~ - [ ] ~If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~ - [ ] ~This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~ - [ ] ~This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~ ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../layout/discover_histogram_layout.test.tsx | 5 +- .../layout/discover_histogram_layout.tsx | 4 + .../layout/discover_layout.test.tsx | 6 +- .../components/layout/discover_layout.tsx | 3 + .../main/components/layout/types.ts | 9 +- .../layout/use_discover_histogram.test.tsx | 110 +++++++++++++++-- .../layout/use_discover_histogram.ts | 102 +++++++++++----- .../main/discover_main_app.test.tsx | 4 + .../application/main/discover_main_app.tsx | 2 + .../main/hooks/use_discover_state.ts | 3 +- .../main/hooks/use_saved_search.ts | 77 ++++++++---- src/plugins/unified_histogram/kibana.json | 2 +- .../public/chart/chart.test.tsx | 14 +-- .../unified_histogram/public/chart/chart.tsx | 68 +++++++---- .../unified_histogram/public/chart/consts.ts | 9 -- .../public/chart/histogram.test.tsx | 66 +++-------- .../public/chart/histogram.tsx | 112 +++++++----------- .../public/chart/use_lens_props.test.ts | 93 +++++++++++++++ .../public/chart/use_lens_props.ts | 74 ++++++++++++ .../public/chart/use_refetch.test.ts | 85 +++++++++++++ .../{use_refetch_id.ts => use_refetch.ts} | 45 ++++--- .../public/chart/use_refetch_id.test.ts | 68 ----------- .../public/chart/use_request_params.test.ts | 48 ++++++++ .../public/chart/use_request_params.tsx | 57 ++++----- .../public/chart/use_stable_callback.test.ts | 19 +++ .../public/chart/use_stable_callback.ts | 23 ++++ .../public/chart/use_total_hits.test.ts | 30 ++--- .../public/chart/use_total_hits.ts | 65 +++++----- src/plugins/unified_histogram/public/index.ts | 3 + .../public/layout/layout.test.tsx | 17 +-- .../public/layout/layout.tsx | 60 +++++++++- src/plugins/unified_histogram/public/types.ts | 22 +++- src/plugins/unified_histogram/tsconfig.json | 1 - .../discover/group1/_discover_histogram.ts | 15 +++ 34 files changed, 903 insertions(+), 418 deletions(-) delete mode 100644 src/plugins/unified_histogram/public/chart/consts.ts create mode 100644 src/plugins/unified_histogram/public/chart/use_lens_props.test.ts create mode 100644 src/plugins/unified_histogram/public/chart/use_lens_props.ts create mode 100644 src/plugins/unified_histogram/public/chart/use_refetch.test.ts rename src/plugins/unified_histogram/public/chart/{use_refetch_id.ts => use_refetch.ts} (79%) delete mode 100644 src/plugins/unified_histogram/public/chart/use_refetch_id.test.ts create mode 100644 src/plugins/unified_histogram/public/chart/use_request_params.test.ts create mode 100644 src/plugins/unified_histogram/public/chart/use_stable_callback.test.ts create mode 100644 src/plugins/unified_histogram/public/chart/use_stable_callback.ts diff --git a/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx index 48eeaaaef4ac8..d99340da89857 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx @@ -69,7 +69,9 @@ const mountComponent = ({ services.data.query.timefilter.timefilter.getAbsoluteTime = () => { return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; }; - + services.data.query.timefilter.timefilter.getTime = () => { + return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; + }; (services.data.query.queryString.getDefaultQuery as jest.Mock).mockReturnValue({ language: 'kuery', query: '', @@ -123,6 +125,7 @@ const mountComponent = ({ setExpandedDoc: jest.fn(), savedSearch, savedSearchData$, + savedSearchFetch$: new Subject(), savedSearchRefetch$: new Subject(), stateContainer, onFieldEdited: jest.fn(), diff --git a/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx index 7c948d8a29f60..aa4027c22aedc 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx @@ -15,6 +15,7 @@ import type { DiscoverSearchSessionManager } from '../../services/discover_searc import type { InspectorAdapters } from '../../hooks/use_inspector'; import { type DiscoverMainContentProps, DiscoverMainContent } from './discover_main_content'; import { ResetSearchButton } from './reset_search_button'; +import type { DataFetch$ } from '../../hooks/use_saved_search'; export interface DiscoverHistogramLayoutProps extends DiscoverMainContentProps { resetSavedSearch: () => void; @@ -22,6 +23,7 @@ export interface DiscoverHistogramLayoutProps extends DiscoverMainContentProps { resizeRef: RefObject; inspectorAdapters: InspectorAdapters; searchSessionManager: DiscoverSearchSessionManager; + savedSearchFetch$: DataFetch$; } export const DiscoverHistogramLayout = ({ @@ -30,6 +32,7 @@ export const DiscoverHistogramLayout = ({ resetSavedSearch, savedSearch, savedSearchData$, + savedSearchFetch$, stateContainer, isTimeBased, resizeRef, @@ -51,6 +54,7 @@ export const DiscoverHistogramLayout = ({ isTimeBased, inspectorAdapters, searchSessionManager, + savedSearchFetch$, ...commonProps, }); diff --git a/src/plugins/discover/public/application/main/components/layout/discover_layout.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_layout.test.tsx index 6a9bd2d381d98..2d6998df4898f 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_layout.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_layout.test.tsx @@ -24,6 +24,7 @@ import { dataViewWithTimefieldMock } from '../../../../__mocks__/data_view_with_ import { AvailableFields$, DataDocuments$, + DataFetch$, DataMain$, DataRefetch$, DataTotalHits$, @@ -58,7 +59,9 @@ function mountComponent( [SIDEBAR_CLOSED_KEY]: prevSidebarClosed, }) as unknown as Storage, } as unknown as DiscoverServices; - + services.data.query.timefilter.timefilter.getTime = () => { + return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; + }; (services.data.query.queryString.getDefaultQuery as jest.Mock).mockReturnValue({ language: 'kuery', query: '', @@ -113,6 +116,7 @@ function mountComponent( resetSavedSearch: jest.fn(), savedSearch: savedSearchMock, savedSearchData$, + savedSearchFetch$: new Subject() as DataFetch$, savedSearchRefetch$: new Subject() as DataRefetch$, searchSource: searchSourceMock, state: { columns: [], query, hideChart: false, interval: 'auto' }, diff --git a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx index c9f52bd548f25..f93bb7b9ff8ea 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_layout.tsx @@ -62,6 +62,7 @@ export function DiscoverLayout({ onChangeDataView, onUpdateQuery, setExpandedDoc, + savedSearchFetch$, savedSearchRefetch$, resetSavedSearch, savedSearchData$, @@ -237,6 +238,7 @@ export function DiscoverLayout({ setExpandedDoc={setExpandedDoc} savedSearch={savedSearch} savedSearchData$={savedSearchData$} + savedSearchFetch$={savedSearchFetch$} savedSearchRefetch$={savedSearchRefetch$} stateContainer={stateContainer} isTimeBased={isTimeBased} @@ -270,6 +272,7 @@ export function DiscoverLayout({ resultState, savedSearch, savedSearchData$, + savedSearchFetch$, savedSearchRefetch$, searchSessionManager, setExpandedDoc, diff --git a/src/plugins/discover/public/application/main/components/layout/types.ts b/src/plugins/discover/public/application/main/components/layout/types.ts index f2a8ebe9269e8..34b8650ffc153 100644 --- a/src/plugins/discover/public/application/main/components/layout/types.ts +++ b/src/plugins/discover/public/application/main/components/layout/types.ts @@ -9,10 +9,10 @@ import type { Query, TimeRange, AggregateQuery } from '@kbn/es-query'; import type { DataView } from '@kbn/data-views-plugin/public'; import type { ISearchSource } from '@kbn/data-plugin/public'; -import { SavedSearch } from '@kbn/saved-search-plugin/public'; -import { DataTableRecord } from '../../../../types'; -import { DiscoverStateContainer } from '../../services/discover_state'; -import { DataRefetch$, SavedSearchData } from '../../hooks/use_saved_search'; +import type { SavedSearch } from '@kbn/saved-search-plugin/public'; +import type { DataTableRecord } from '../../../../types'; +import type { DiscoverStateContainer } from '../../services/discover_state'; +import type { DataFetch$, DataRefetch$, SavedSearchData } from '../../hooks/use_saved_search'; import type { DiscoverSearchSessionManager } from '../../services/discover_search_session'; import type { InspectorAdapters } from '../../hooks/use_inspector'; @@ -29,6 +29,7 @@ export interface DiscoverLayoutProps { setExpandedDoc: (doc?: DataTableRecord) => void; savedSearch: SavedSearch; savedSearchData$: SavedSearchData; + savedSearchFetch$: DataFetch$; savedSearchRefetch$: DataRefetch$; searchSource: ISearchSource; stateContainer: DiscoverStateContainer; diff --git a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx index 950b2d4571a40..89059db91e7f5 100644 --- a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx @@ -9,11 +9,12 @@ import React, { ReactElement } from 'react'; import { buildDataTableRecord } from '../../../../utils/build_data_record'; import { esHits } from '../../../../__mocks__/es_hits'; import { act, renderHook, WrapperComponent } from '@testing-library/react-hooks'; -import { BehaviorSubject } from 'rxjs'; +import { BehaviorSubject, Subject } from 'rxjs'; import { FetchStatus } from '../../../types'; import { AvailableFields$, DataDocuments$, + DataFetch$, DataMain$, DataTotalHits$, RecordRawType, @@ -72,6 +73,15 @@ jest.mock('@kbn/unified-field-list-plugin/public', () => { return { ...originalModule, getVisualizeInformation: jest.fn(() => Promise.resolve(mockCanVisualize)), + useQuerySubscriber: jest.fn(() => ({ + query: { + query: 'query', + language: 'kuery', + }, + filters: [], + fromDate: 'now-15m', + toDate: 'now', + })), }; }); @@ -120,6 +130,7 @@ describe('useDiscoverHistogram', () => { recordRawType: isPlainRecord ? RecordRawType.PLAIN : RecordRawType.DOCUMENT, foundDocuments: true, }) as DataMain$, + savedSearchFetch$ = new Subject() as DataFetch$, }: { isPlainRecord?: boolean; isTimeBased?: boolean; @@ -131,6 +142,7 @@ describe('useDiscoverHistogram', () => { inspectorAdapters?: InspectorAdapters; totalHits$?: DataTotalHits$; main$?: DataMain$; + savedSearchFetch$?: DataFetch$; } = {}) => { mockStorage = storage; mockCanVisualize = canVisualize; @@ -161,6 +173,7 @@ describe('useDiscoverHistogram', () => { const initialProps = { stateContainer, savedSearchData$, + savedSearchFetch$, dataView: dataViewWithTimefieldMock, savedSearch: savedSearchMock, isTimeBased, @@ -188,11 +201,20 @@ describe('useDiscoverHistogram', () => { return { hook, initialProps }; }; - it('should return undefined if there is no search session', async () => { - const { - hook: { result }, - } = await renderUseDiscoverHistogram({ searchSessionId: null }); - expect(result.current).toBeUndefined(); + describe('result', () => { + it('should return undefined if there is no search session', async () => { + const { + hook: { result }, + } = await renderUseDiscoverHistogram({ searchSessionId: null }); + expect(result.current).toBeUndefined(); + }); + + it('it should not return undefined if there is no search session, but isPlainRecord is true', async () => { + const { + hook: { result }, + } = await renderUseDiscoverHistogram({ searchSessionId: null, isPlainRecord: true }); + expect(result.current).toBeDefined(); + }); }); describe('contexts', () => { @@ -263,6 +285,21 @@ describe('useDiscoverHistogram', () => { }); }); + describe('search params', () => { + it('should return the correct query, filters, and timeRange', async () => { + const { hook } = await renderUseDiscoverHistogram(); + expect(hook.result.current?.query).toEqual({ + query: 'query', + language: 'kuery', + }); + expect(hook.result.current?.filters).toEqual([]); + expect(hook.result.current?.timeRange).toEqual({ + from: 'now-15m', + to: 'now', + }); + }); + }); + describe('onEditVisualization', () => { it('returns a callback for onEditVisualization when the data view can be visualized', async () => { const { @@ -364,7 +401,7 @@ describe('useDiscoverHistogram', () => { } = await renderUseDiscoverHistogram({ inspectorAdapters }); expect(inspectorAdapters.lensRequests).toBeUndefined(); act(() => { - result.current?.onChartLoad({ complete: true, adapters: { requests: lensRequests } }); + result.current?.onChartLoad({ adapters: { requests: lensRequests } }); }); expect(inspectorAdapters.lensRequests).toBeDefined(); }); @@ -486,4 +523,63 @@ describe('useDiscoverHistogram', () => { expect(mockCheckHitCount).not.toHaveBeenCalled(); }); }); + + describe('refetching', () => { + it("should call input$.next({ type: 'refetch' }) when savedSearchFetch$ is triggered", async () => { + const savedSearchFetch$ = new BehaviorSubject({ reset: false, searchSessionId: '1234' }); + const { hook } = await renderUseDiscoverHistogram({ savedSearchFetch$ }); + const onRefetch = jest.fn(); + hook.result.current?.input$.subscribe(onRefetch); + act(() => { + savedSearchFetch$.next({ reset: false, searchSessionId: '1234' }); + }); + expect(onRefetch).toHaveBeenCalledWith({ type: 'refetch' }); + }); + + it("should not call input$.next({ type: 'refetch' }) when searchSessionId is not set", async () => { + const savedSearchFetch$ = new BehaviorSubject({ reset: false, searchSessionId: '1234' }); + const { hook } = await renderUseDiscoverHistogram({ + savedSearchFetch$, + searchSessionId: null, + }); + const onRefetch = jest.fn(); + hook.result.current?.input$.subscribe(onRefetch); + act(() => { + savedSearchFetch$.next({ reset: false, searchSessionId: '1234' }); + }); + expect(onRefetch).not.toHaveBeenCalled(); + }); + + it("should call input$.next({ type: 'refetch' }) when searchSessionId is not set and isPlainRecord is true", async () => { + const savedSearchFetch$ = new BehaviorSubject({ reset: false, searchSessionId: '1234' }); + const { hook } = await renderUseDiscoverHistogram({ + savedSearchFetch$, + searchSessionId: null, + isPlainRecord: true, + }); + const onRefetch = jest.fn(); + hook.result.current?.input$.subscribe(onRefetch); + act(() => { + savedSearchFetch$.next({ reset: false, searchSessionId: '1234' }); + }); + expect(onRefetch).toHaveBeenCalledWith({ type: 'refetch' }); + }); + + it('should skip the next refetch when state.hideChart changes from true to false', async () => { + const savedSearchFetch$ = new BehaviorSubject({ reset: false, searchSessionId: '1234' }); + const { hook } = await renderUseDiscoverHistogram({ savedSearchFetch$ }); + const onRefetch = jest.fn(); + hook.result.current?.input$.subscribe(onRefetch); + act(() => { + hook.result.current?.onChartHiddenChange(true); + }); + act(() => { + hook.result.current?.onChartHiddenChange(false); + }); + act(() => { + savedSearchFetch$.next({ reset: false, searchSessionId: '1234' }); + }); + expect(onRefetch).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts index 53ddc74ad99d6..d380244fe875b 100644 --- a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts +++ b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts @@ -8,20 +8,22 @@ import type { DataView, DataViewField } from '@kbn/data-views-plugin/common'; import type { SavedSearch } from '@kbn/saved-search-plugin/public'; -import { getVisualizeInformation } from '@kbn/unified-field-list-plugin/public'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { getVisualizeInformation, useQuerySubscriber } from '@kbn/unified-field-list-plugin/public'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { UnifiedHistogramFetchStatus, UnifiedHistogramHitsContext, + UnifiedHistogramInputMessage, } from '@kbn/unified-histogram-plugin/public'; import type { UnifiedHistogramChartLoadEvent } from '@kbn/unified-histogram-plugin/public'; import useObservable from 'react-use/lib/useObservable'; import type { TypedLensByValueInput } from '@kbn/lens-plugin/public'; +import { Subject } from 'rxjs'; import { useAppStateSelector } from '../../services/discover_app_state_container'; import { getUiActions } from '../../../../kibana_services'; import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import { useDataState } from '../../hooks/use_data_state'; -import type { SavedSearchData } from '../../hooks/use_saved_search'; +import type { DataFetch$, SavedSearchData } from '../../hooks/use_saved_search'; import type { DiscoverStateContainer } from '../../services/discover_state'; import { FetchStatus } from '../../../types'; import type { DiscoverSearchSessionManager } from '../../services/discover_search_session'; @@ -41,6 +43,7 @@ export interface UseDiscoverHistogramProps { isPlainRecord: boolean; inspectorAdapters: InspectorAdapters; searchSessionManager: DiscoverSearchSessionManager; + savedSearchFetch$: DataFetch$; } export const useDiscoverHistogram = ({ @@ -52,6 +55,7 @@ export const useDiscoverHistogram = ({ isPlainRecord, inspectorAdapters, searchSessionManager, + savedSearchFetch$, }: UseDiscoverHistogramProps) => { const { storage, data, lens } = useDiscoverServices(); const [hideChart, interval, breakdownField] = useAppStateSelector((state) => [ @@ -124,21 +128,6 @@ export const useDiscoverHistogram = ({ [stateContainer] ); - /** - * Request - */ - - // The searchSessionId will be updated whenever a new search - // is started and will trigger a unified histogram refetch - const searchSessionId = useObservable(searchSessionManager.searchSessionId$); - const request = useMemo( - () => ({ - searchSessionId, - adapter: inspectorAdapters.requests, - }), - [inspectorAdapters.requests, searchSessionId] - ); - /** * Total hits */ @@ -216,32 +205,23 @@ export const useDiscoverHistogram = ({ [inspectorAdapters] ); - const [chartHidden, setChartHidden] = useState(hideChart); const chart = useMemo( () => isPlainRecord || !isTimeBased ? undefined : { - hidden: chartHidden, + hidden: hideChart, timeInterval: interval, }, - [chartHidden, interval, isPlainRecord, isTimeBased] + [hideChart, interval, isPlainRecord, isTimeBased] ); // Clear the Lens request adapter when the chart is hidden useEffect(() => { - if (chartHidden || !chart) { + if (hideChart || !chart) { inspectorAdapters.lensRequests = undefined; } - }, [chart, chartHidden, inspectorAdapters]); - - // state.chartHidden is updated before searchSessionId, which can trigger duplicate - // requests, so instead of using state.chartHidden directly, we update chartHidden - // when searchSessionId changes - useEffect(() => { - setChartHidden(hideChart); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [searchSessionId]); + }, [chart, hideChart, inspectorAdapters]); /** * Breakdown @@ -264,18 +244,78 @@ export const useDiscoverHistogram = ({ [field, isPlainRecord, isTimeBased] ); + /** + * Search params + */ + + const { query, filters, fromDate: from, toDate: to } = useQuerySubscriber({ data }); + const timeRange = useMemo( + () => (from && to ? { from, to } : data.query.timefilter.timefilter.getTimeDefaults()), + [data.query.timefilter.timefilter, from, to] + ); + + /** + * Request + */ + + // The searchSessionId will be updated whenever a new search is started + const searchSessionId = useObservable(searchSessionManager.searchSessionId$); + const request = useMemo( + () => ({ + searchSessionId, + adapter: inspectorAdapters.requests, + }), + [inspectorAdapters.requests, searchSessionId] + ); + + /** + * Data fetching + */ + + const input$ = useMemo(() => new Subject(), []); + // Initialized when the first search has been requested or // when in SQL mode since search sessions are not supported const isInitialized = Boolean(searchSessionId) || isPlainRecord; + const skipRefetch = useRef(); + + // Skip refetching when showing the chart since Lens will + // automatically fetch when the chart is shown + useEffect(() => { + if (skipRefetch.current === undefined) { + skipRefetch.current = false; + } else { + skipRefetch.current = !hideChart; + } + }, [hideChart]); + + // Trigger a unified histogram refetch when savedSearchFetch$ is triggered + useEffect(() => { + const subscription = savedSearchFetch$.subscribe(() => { + if (isInitialized && !skipRefetch.current) { + input$.next({ type: 'refetch' }); + } + skipRefetch.current = false; + }); + + return () => { + subscription.unsubscribe(); + }; + }, [input$, isInitialized, savedSearchFetch$]); // Don't render the unified histogram layout until initialized return isInitialized ? { + query, + filters, + timeRange, topPanelHeight, request, hits, chart, breakdown, + disableAutoFetching: true, + input$, onEditVisualization: canVisualize ? onEditVisualization : undefined, onTopPanelHeightChange, onChartHiddenChange, diff --git a/src/plugins/discover/public/application/main/discover_main_app.test.tsx b/src/plugins/discover/public/application/main/discover_main_app.test.tsx index 7ea5e944cc6a7..f3cc7f9cee43d 100644 --- a/src/plugins/discover/public/application/main/discover_main_app.test.tsx +++ b/src/plugins/discover/public/application/main/discover_main_app.test.tsx @@ -25,6 +25,10 @@ import { DiscoverMainProvider } from './services/discover_state_provider'; setHeaderActionMenuMounter(jest.fn()); setUrlTracker(urlTrackerMock); +discoverServiceMock.data.query.timefilter.timefilter.getTime = () => { + return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; +}; + describe('DiscoverMainApp', () => { test('renders', async () => { const dataViewList = [dataViewMock].map((ip) => { diff --git a/src/plugins/discover/public/application/main/discover_main_app.tsx b/src/plugins/discover/public/application/main/discover_main_app.tsx index a91ed42a55a62..92f155f57bafc 100644 --- a/src/plugins/discover/public/application/main/discover_main_app.tsx +++ b/src/plugins/discover/public/application/main/discover_main_app.tsx @@ -55,6 +55,7 @@ export function DiscoverMainApp(props: DiscoverMainProps) { onUpdateQuery, persistDataView, updateAdHocDataViewId, + fetch$, refetch$, resetSavedSearch, searchSource, @@ -118,6 +119,7 @@ export function DiscoverMainApp(props: DiscoverMainProps) { navigateTo={navigateTo} savedSearch={savedSearch} savedSearchData$={data$} + savedSearchFetch$={fetch$} savedSearchRefetch$={refetch$} searchSource={searchSource} stateContainer={stateContainer} diff --git a/src/plugins/discover/public/application/main/hooks/use_discover_state.ts b/src/plugins/discover/public/application/main/hooks/use_discover_state.ts index 6eefb25580969..a8ff93ef94547 100644 --- a/src/plugins/discover/public/application/main/hooks/use_discover_state.ts +++ b/src/plugins/discover/public/application/main/hooks/use_discover_state.ts @@ -121,7 +121,7 @@ export function useDiscoverState({ /** * Data fetching logic */ - const { data$, refetch$, reset, inspectorAdapters } = useSavedSearchData({ + const { data$, fetch$, refetch$, reset, inspectorAdapters } = useSavedSearchData({ initialFetchStatus, searchSessionManager, savedSearch, @@ -322,6 +322,7 @@ export function useDiscoverState({ return { data$, inspectorAdapters, + fetch$, refetch$, resetSavedSearch, onChangeDataView, diff --git a/src/plugins/discover/public/application/main/hooks/use_saved_search.ts b/src/plugins/discover/public/application/main/hooks/use_saved_search.ts index 7f7e4700925a6..1b1a917740d43 100644 --- a/src/plugins/discover/public/application/main/hooks/use_saved_search.ts +++ b/src/plugins/discover/public/application/main/hooks/use_saved_search.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ import { useCallback, useEffect, useMemo, useRef } from 'react'; -import { BehaviorSubject, Subject } from 'rxjs'; +import { BehaviorSubject, filter, map, Observable, share, Subject, tap } from 'rxjs'; import type { AutoRefreshDoneFn } from '@kbn/data-plugin/public'; import { ISearchSource } from '@kbn/data-plugin/public'; import { RequestAdapter } from '@kbn/inspector-plugin/public'; @@ -38,7 +38,10 @@ export type DataMain$ = BehaviorSubject; export type DataDocuments$ = BehaviorSubject; export type DataTotalHits$ = BehaviorSubject; export type AvailableFields$ = BehaviorSubject; - +export type DataFetch$ = Observable<{ + reset: boolean; + searchSessionId: string; +}>; export type DataRefetch$ = Subject; export interface UseSavedSearch { @@ -151,46 +154,70 @@ export const useSavedSearch = ({ }>({}); /** - * This part takes care of triggering the data fetching by creating and subscribing - * to an observable of various possible changes in state + * handler emitted by `timefilter.getAutoRefreshFetch$()` + * to notify when data completed loading and to start a new autorefresh loop */ - useEffect(() => { - /** - * handler emitted by `timefilter.getAutoRefreshFetch$()` - * to notify when data completed loading and to start a new autorefresh loop - */ - const setAutoRefreshDone = (fn: AutoRefreshDoneFn | undefined) => { - refs.current.autoRefreshDone = fn; - }; - const fetch$ = getFetch$({ - setAutoRefreshDone, + const setAutoRefreshDone = useCallback((fn: AutoRefreshDoneFn | undefined) => { + refs.current.autoRefreshDone = fn; + }, []); + + /** + * Observable that allows listening for when fetches are triggered + */ + const fetch$ = useMemo( + () => + getFetch$({ + setAutoRefreshDone, + data, + main$, + refetch$, + searchSessionManager, + searchSource, + initialFetchStatus, + }).pipe( + filter(() => validateTimeRange(timefilter.getTime(), services.toastNotifications)), + tap(() => inspectorAdapters.requests.reset()), + map((val) => ({ + reset: val === 'reset', + searchSessionId: searchSessionManager.getNextSearchSessionId(), + })), + share() + ), + [ data, + initialFetchStatus, + inspectorAdapters.requests, main$, refetch$, searchSessionManager, searchSource, - initialFetchStatus, - }); - let abortController: AbortController; + services.toastNotifications, + setAutoRefreshDone, + timefilter, + ] + ); - const subscription = fetch$.subscribe(async (val) => { - if (!validateTimeRange(timefilter.getTime(), services.toastNotifications)) { - return; - } - inspectorAdapters.requests.reset(); + /** + * This part takes care of triggering the data fetching by creating and subscribing + * to an observable of various possible changes in state + */ + useEffect(() => { + let abortController: AbortController; + const subscription = fetch$.subscribe(async ({ reset, searchSessionId }) => { abortController?.abort(); abortController = new AbortController(); + const autoRefreshDone = refs.current.autoRefreshDone; - await fetchAll(dataSubjects, searchSource, val === 'reset', { + await fetchAll(dataSubjects, searchSource, reset, { abortController, appStateContainer: stateContainer.appState, data, initialFetchStatus, inspectorAdapters, savedSearch, - searchSessionId: searchSessionManager.getNextSearchSessionId(), + searchSessionId, services, useNewFieldsApi, }); @@ -213,6 +240,7 @@ export const useSavedSearch = ({ data, data.query.queryString, dataSubjects, + fetch$, filterManager, initialFetchStatus, inspectorAdapters, @@ -235,6 +263,7 @@ export const useSavedSearch = ({ ); return { + fetch$, refetch$, data$: dataSubjects, reset, diff --git a/src/plugins/unified_histogram/kibana.json b/src/plugins/unified_histogram/kibana.json index 5be043637ed33..aa9e22fab2a08 100755 --- a/src/plugins/unified_histogram/kibana.json +++ b/src/plugins/unified_histogram/kibana.json @@ -11,5 +11,5 @@ "ui": true, "requiredPlugins": [], "optionalPlugins": [], - "requiredBundles": ["data", "dataViews", "embeddable", "kibanaUtils", "inspector"] + "requiredBundles": ["data", "dataViews", "embeddable", "inspector"] } diff --git a/src/plugins/unified_histogram/public/chart/chart.test.tsx b/src/plugins/unified_histogram/public/chart/chart.test.tsx index 21682cb919d3c..bc4115590b4e2 100644 --- a/src/plugins/unified_histogram/public/chart/chart.test.tsx +++ b/src/plugins/unified_histogram/public/chart/chart.test.tsx @@ -39,20 +39,18 @@ async function mountComponent({ dataView?: DataView; onEditVisualization?: null | (() => void); } = {}) { - const services = unifiedHistogramServicesMock; - services.data.query.timefilter.timefilter.getAbsoluteTime = () => { - return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; - }; - (services.data.query.queryString.getDefaultQuery as jest.Mock).mockReturnValue({ - language: 'kuery', - query: '', - }); (searchSourceInstanceMock.fetch$ as jest.Mock).mockImplementation( jest.fn().mockReturnValue(of({ rawResponse: { hits: { total: noHits ? 0 : 2 } } })) ); const props = { dataView, + query: { + language: 'kuery', + query: '', + }, + filters: [], + timeRange: { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }, services: unifiedHistogramServicesMock, hits: noHits ? undefined diff --git a/src/plugins/unified_histogram/public/chart/chart.tsx b/src/plugins/unified_histogram/public/chart/chart.tsx index b1970cd26e365..ccfadcaa07e97 100644 --- a/src/plugins/unified_histogram/public/chart/chart.tsx +++ b/src/plugins/unified_histogram/public/chart/chart.tsx @@ -18,7 +18,9 @@ import { } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { DataView, DataViewField, DataViewType } from '@kbn/data-views-plugin/public'; -import type { TypedLensByValueInput } from '@kbn/lens-plugin/public'; +import type { LensEmbeddableInput, TypedLensByValueInput } from '@kbn/lens-plugin/public'; +import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query'; +import { Subject } from 'rxjs'; import { HitsCounter } from '../hits_counter'; import { Histogram } from './histogram'; import { useChartPanels } from './use_chart_panels'; @@ -30,26 +32,34 @@ import type { UnifiedHistogramChartLoadEvent, UnifiedHistogramRequestContext, UnifiedHistogramServices, + UnifiedHistogramInput$, + UnifiedHistogramInputMessage, } from '../types'; import { BreakdownFieldSelector } from './breakdown_field_selector'; import { useTotalHits } from './use_total_hits'; import { useRequestParams } from './use_request_params'; import { useChartStyles } from './use_chart_styles'; import { useChartActions } from './use_chart_actions'; -import { useRefetchId } from './use_refetch_id'; import { getLensAttributes } from './get_lens_attributes'; +import { useRefetch } from './use_refetch'; export interface ChartProps { className?: string; services: UnifiedHistogramServices; dataView: DataView; - lastReloadRequestTime?: number; + query?: Query | AggregateQuery; + filters?: Filter[]; + timeRange?: TimeRange; request?: UnifiedHistogramRequestContext; hits?: UnifiedHistogramHitsContext; chart?: UnifiedHistogramChartContext; breakdown?: UnifiedHistogramBreakdownContext; appendHitsCounter?: ReactElement; appendHistogram?: ReactElement; + disableAutoFetching?: boolean; + disableTriggers?: LensEmbeddableInput['disableTriggers']; + disabledActions?: LensEmbeddableInput['disabledActions']; + input$?: UnifiedHistogramInput$; onEditVisualization?: (lensAttributes: TypedLensByValueInput['attributes']) => void; onResetChartHeight?: () => void; onChartHiddenChange?: (chartHidden: boolean) => void; @@ -57,6 +67,8 @@ export interface ChartProps { onBreakdownFieldChange?: (breakdownField: DataViewField | undefined) => void; onTotalHitsChange?: (status: UnifiedHistogramFetchStatus, result?: number | Error) => void; onChartLoad?: (event: UnifiedHistogramChartLoadEvent) => void; + onFilter?: LensEmbeddableInput['onFilter']; + onBrushEnd?: LensEmbeddableInput['onBrushEnd']; } const HistogramMemoized = memo(Histogram); @@ -65,13 +77,19 @@ export function Chart({ className, services, dataView, - lastReloadRequestTime, + query: originalQuery, + filters: originalFilters, + timeRange: originalTimeRange, request, hits, chart, breakdown, appendHitsCounter, appendHistogram, + disableAutoFetching, + disableTriggers, + disabledActions, + input$: originalInput$, onEditVisualization: originalOnEditVisualization, onResetChartHeight, onChartHiddenChange, @@ -79,6 +97,8 @@ export function Chart({ onBreakdownFieldChange, onTotalHitsChange, onChartLoad, + onFilter, + onBrushEnd, }: ChartProps) { const { showChartOptionsPopover, @@ -107,15 +127,20 @@ export function Chart({ dataView.isTimeBased() ); - const { filters, query, relativeTimeRange } = useRequestParams({ + const input$ = useMemo( + () => originalInput$ ?? new Subject(), + [originalInput$] + ); + + const { filters, query, getTimeRange, updateTimeRange, relativeTimeRange } = useRequestParams({ services, - lastReloadRequestTime, - request, + query: originalQuery, + filters: originalFilters, + timeRange: originalTimeRange, }); - const refetchId = useRefetchId({ + const refetch$ = useRefetch({ dataView, - lastReloadRequestTime, request, hits, chart, @@ -124,28 +149,21 @@ export function Chart({ filters, query, relativeTimeRange, + disableAutoFetching, + input$, + beforeRefetch: updateTimeRange, }); - // We need to update the absolute time range whenever the refetchId changes - const timeRange = useMemo( - () => services.data.query.timefilter.timefilter.getAbsoluteTime(), - // eslint-disable-next-line react-hooks/exhaustive-deps - [services.data.query.timefilter.timefilter, refetchId] - ); - useTotalHits({ services, dataView, - lastReloadRequestTime, request, hits, - chart, chartVisible, - breakdown, filters, query, - timeRange, - refetchId, + getTimeRange, + refetch$, onTotalHitsChange, }); @@ -288,14 +306,18 @@ export function Chart({ {appendHistogram} diff --git a/src/plugins/unified_histogram/public/chart/consts.ts b/src/plugins/unified_histogram/public/chart/consts.ts deleted file mode 100644 index d2af2ed4ee33a..0000000000000 --- a/src/plugins/unified_histogram/public/chart/consts.ts +++ /dev/null @@ -1,9 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -export const REQUEST_DEBOUNCE_MS = 100; diff --git a/src/plugins/unified_histogram/public/chart/histogram.test.tsx b/src/plugins/unified_histogram/public/chart/histogram.test.tsx index 03652a63f5456..7385f20358be3 100644 --- a/src/plugins/unified_histogram/public/chart/histogram.test.tsx +++ b/src/plugins/unified_histogram/public/chart/histogram.test.tsx @@ -6,18 +6,19 @@ * Side Public License, v 1. */ import { mountWithIntl } from '@kbn/test-jest-helpers'; -import { getLensProps, Histogram } from './histogram'; +import { Histogram } from './histogram'; import React from 'react'; import { unifiedHistogramServicesMock } from '../__mocks__/services'; import { dataViewWithTimefieldMock } from '../__mocks__/data_view_with_timefield'; import { createDefaultInspectorAdapters } from '@kbn/expressions-plugin/common'; -import { UnifiedHistogramFetchStatus } from '../types'; +import { UnifiedHistogramFetchStatus, UnifiedHistogramInput$ } from '../types'; import { getLensAttributes } from './get_lens_attributes'; -import { REQUEST_DEBOUNCE_MS } from './consts'; import { act } from 'react-dom/test-utils'; import * as buildBucketInterval from './build_bucket_interval'; import * as useTimeRange from './use_time_range'; import { RequestStatus } from '@kbn/inspector-plugin/public'; +import { Subject } from 'rxjs'; +import { getLensProps } from './use_lens_props'; const mockBucketInterval = { description: '1 minute', scale: undefined, scaled: false }; jest.spyOn(buildBucketInterval, 'buildBucketInterval').mockReturnValue(mockBucketInterval); @@ -43,7 +44,7 @@ function mountComponent() { }; const timefilterUpdateHandler = jest.fn(); - + const refetch$: UnifiedHistogramInput$ = new Subject(); const props = { services: unifiedHistogramServicesMock, request: { @@ -59,11 +60,11 @@ function mountComponent() { }, timefilterUpdateHandler, dataView: dataViewWithTimefieldMock, - timeRange: { + getTimeRange: () => ({ from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590', - }, - lastReloadRequestTime: 42, + }), + refetch$, lensAttributes: getMockLensAttributes(), onTotalHitsChange: jest.fn(), onChartLoad: jest.fn(), @@ -81,28 +82,27 @@ describe('Histogram', () => { expect(component.find('[data-test-subj="unifiedHistogramChart"]').exists()).toBe(true); }); - it('should render lens.EmbeddableComponent with debounced props', async () => { + it('should only update lens.EmbeddableComponent props when refetch$ is triggered', async () => { const { component, props } = mountComponent(); const embeddable = unifiedHistogramServicesMock.lens.EmbeddableComponent; expect(component.find(embeddable).exists()).toBe(true); let lensProps = component.find(embeddable).props(); const originalProps = getLensProps({ - timeRange: props.timeRange, + searchSessionId: props.request.searchSessionId, + getTimeRange: props.getTimeRange, attributes: getMockLensAttributes(), - request: props.request, - lastReloadRequestTime: props.lastReloadRequestTime, onLoad: lensProps.onLoad, }); expect(lensProps).toEqual(originalProps); - component.setProps({ lastReloadRequestTime: 43 }).update(); + component.setProps({ request: { ...props.request, searchSessionId: '321' } }).update(); lensProps = component.find(embeddable).props(); expect(lensProps).toEqual(originalProps); await act(async () => { - await new Promise((resolve) => setTimeout(resolve, REQUEST_DEBOUNCE_MS)); + props.refetch$.next({ type: 'refetch' }); }); component.update(); lensProps = component.find(embeddable).props(); - expect(lensProps).toEqual({ ...originalProps, lastReloadRequestTime: 43 }); + expect(lensProps).toEqual({ ...originalProps, searchSessionId: '321' }); }); it('should execute onLoad correctly', async () => { @@ -165,7 +165,7 @@ describe('Histogram', () => { UnifiedHistogramFetchStatus.loading, undefined ); - expect(props.onChartLoad).toHaveBeenLastCalledWith({ complete: false, adapters: {} }); + expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters: {} }); expect(buildBucketInterval.buildBucketInterval).not.toHaveBeenCalled(); expect(useTimeRange.useTimeRange).toHaveBeenLastCalledWith( expect.objectContaining({ bucketInterval: undefined }) @@ -177,7 +177,7 @@ describe('Histogram', () => { UnifiedHistogramFetchStatus.complete, 100 ); - expect(props.onChartLoad).toHaveBeenLastCalledWith({ complete: true, adapters }); + expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters }); expect(buildBucketInterval.buildBucketInterval).toHaveBeenCalled(); expect(useTimeRange.useTimeRange).toHaveBeenLastCalledWith( expect.objectContaining({ bucketInterval: mockBucketInterval }) @@ -197,7 +197,7 @@ describe('Histogram', () => { UnifiedHistogramFetchStatus.error, undefined ); - expect(props.onChartLoad).toHaveBeenLastCalledWith({ complete: false, adapters }); + expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters }); }); it('should execute onLoad correctly when the response has shard failures', async () => { @@ -222,36 +222,6 @@ describe('Histogram', () => { UnifiedHistogramFetchStatus.error, undefined ); - expect(props.onChartLoad).toHaveBeenLastCalledWith({ complete: false, adapters }); - }); - - it('should not recreate onLoad in debounced lens props when hits.total changes', async () => { - const { component, props } = mountComponent(); - const embeddable = unifiedHistogramServicesMock.lens.EmbeddableComponent; - const onLoad = component.find(embeddable).props().onLoad; - onLoad(true, undefined); - expect(props.onTotalHitsChange).toHaveBeenLastCalledWith( - UnifiedHistogramFetchStatus.loading, - undefined - ); - component - .setProps({ - hits: { - status: UnifiedHistogramFetchStatus.complete, - total: 100, - }, - }) - .update(); - expect(component.find(embeddable).props().onLoad).toBe(onLoad); - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, REQUEST_DEBOUNCE_MS)); - }); - component.update(); - expect(component.find(embeddable).props().onLoad).toBe(onLoad); - onLoad(true, undefined); - expect(props.onTotalHitsChange).toHaveBeenLastCalledWith( - UnifiedHistogramFetchStatus.loading, - 100 - ); + expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters }); }); }); diff --git a/src/plugins/unified_histogram/public/chart/histogram.tsx b/src/plugins/unified_histogram/public/chart/histogram.tsx index c30c6a1410985..01caba4ad48ad 100644 --- a/src/plugins/unified_histogram/public/chart/histogram.tsx +++ b/src/plugins/unified_histogram/public/chart/histogram.tsx @@ -8,16 +8,15 @@ import { useEuiTheme } from '@elastic/eui'; import { css } from '@emotion/react'; -import React, { useCallback, useEffect, useRef, useState } from 'react'; +import React, { useState } from 'react'; import type { DataView } from '@kbn/data-views-plugin/public'; import type { DefaultInspectorAdapters } from '@kbn/expressions-plugin/common'; import type { IKibanaSearchResponse } from '@kbn/data-plugin/public'; import type { estypes } from '@elastic/elasticsearch'; import type { TimeRange } from '@kbn/es-query'; -import useDebounce from 'react-use/lib/useDebounce'; -import { ViewMode } from '@kbn/embeddable-plugin/public'; -import type { TypedLensByValueInput } from '@kbn/lens-plugin/public'; +import type { LensEmbeddableInput, TypedLensByValueInput } from '@kbn/lens-plugin/public'; import { RequestStatus } from '@kbn/inspector-plugin/public'; +import type { Observable } from 'rxjs'; import { UnifiedHistogramBucketInterval, UnifiedHistogramChartContext, @@ -26,53 +25,55 @@ import { UnifiedHistogramChartLoadEvent, UnifiedHistogramRequestContext, UnifiedHistogramServices, + UnifiedHistogramInputMessage, } from '../types'; import { buildBucketInterval } from './build_bucket_interval'; import { useTimeRange } from './use_time_range'; -import { REQUEST_DEBOUNCE_MS } from './consts'; +import { useStableCallback } from './use_stable_callback'; +import { useLensProps } from './use_lens_props'; export interface HistogramProps { services: UnifiedHistogramServices; dataView: DataView; - lastReloadRequestTime: number | undefined; request?: UnifiedHistogramRequestContext; hits?: UnifiedHistogramHitsContext; chart: UnifiedHistogramChartContext; - timeRange: TimeRange; + getTimeRange: () => TimeRange; + refetch$: Observable; lensAttributes: TypedLensByValueInput['attributes']; + disableTriggers?: LensEmbeddableInput['disableTriggers']; + disabledActions?: LensEmbeddableInput['disabledActions']; onTotalHitsChange?: (status: UnifiedHistogramFetchStatus, result?: number | Error) => void; onChartLoad?: (event: UnifiedHistogramChartLoadEvent) => void; + onFilter?: LensEmbeddableInput['onFilter']; + onBrushEnd?: LensEmbeddableInput['onBrushEnd']; } export function Histogram({ services: { data, lens, uiSettings }, dataView, - lastReloadRequestTime, request, hits, chart: { timeInterval }, - timeRange, + getTimeRange, + refetch$, lensAttributes: attributes, + disableTriggers, + disabledActions, onTotalHitsChange, onChartLoad, + onFilter, + onBrushEnd, }: HistogramProps) { const [bucketInterval, setBucketInterval] = useState(); const { timeRangeText, timeRangeDisplay } = useTimeRange({ uiSettings, bucketInterval, - timeRange, + timeRange: getTimeRange(), timeInterval, }); - // Keep track of previous hits in a ref to avoid recreating the - // onLoad callback when the hits change, which triggers a Lens reload - const previousHits = useRef(hits?.total); - - useEffect(() => { - previousHits.current = hits?.total; - }, [hits?.total]); - - const onLoad = useCallback( + const onLoad = useStableCallback( (isLoading: boolean, adapters: Partial | undefined) => { const lensRequest = adapters?.requests?.getRequests()[0]; const requestFailed = lensRequest?.status === RequestStatus.ERROR; @@ -86,7 +87,7 @@ export function Histogram({ // This is incorrect, so we check for request failures and shard failures here, and emit an error instead. if (requestFailed || response?._shards.failed) { onTotalHitsChange?.(UnifiedHistogramFetchStatus.error, undefined); - onChartLoad?.({ complete: false, adapters: adapters ?? {} }); + onChartLoad?.({ adapters: adapters ?? {} }); return; } @@ -94,7 +95,7 @@ export function Histogram({ onTotalHitsChange?.( isLoading ? UnifiedHistogramFetchStatus.loading : UnifiedHistogramFetchStatus.complete, - totalHits ?? previousHits.current + totalHits ?? hits?.total ); if (response) { @@ -102,18 +103,25 @@ export function Histogram({ data, dataView, timeInterval, - timeRange, + timeRange: getTimeRange(), response, }); setBucketInterval(newBucketInterval); } - onChartLoad?.({ complete: !isLoading, adapters: adapters ?? {} }); - }, - [data, dataView, onChartLoad, onTotalHitsChange, timeInterval, timeRange] + onChartLoad?.({ adapters: adapters ?? {} }); + } ); + const lensProps = useLensProps({ + request, + getTimeRange, + refetch$, + attributes, + onLoad, + }); + const { euiTheme } = useEuiTheme(); const chartCss = css` position: relative; @@ -135,58 +143,18 @@ export function Histogram({ } `; - const [debouncedProps, setDebouncedProps] = useState( - getLensProps({ - timeRange, - attributes, - request, - lastReloadRequestTime, - onLoad, - }) - ); - - useDebounce( - () => { - setDebouncedProps( - getLensProps({ timeRange, attributes, request, lastReloadRequestTime, onLoad }) - ); - }, - REQUEST_DEBOUNCE_MS, - [attributes, lastReloadRequestTime, onLoad, request, timeRange] - ); - return ( <>
- +
{timeRangeDisplay} ); } - -export const getLensProps = ({ - timeRange, - attributes, - request, - lastReloadRequestTime, - onLoad, -}: { - timeRange: TimeRange; - attributes: TypedLensByValueInput['attributes']; - request: UnifiedHistogramRequestContext | undefined; - lastReloadRequestTime: number | undefined; - onLoad: (isLoading: boolean, adapters: Partial | undefined) => void; -}) => ({ - id: 'unifiedHistogramLensComponent', - viewMode: ViewMode.VIEW, - timeRange, - attributes, - noPadding: true, - searchSessionId: request?.searchSessionId, - executionContext: { - description: 'fetch chart data and total hits', - }, - lastReloadRequestTime, - onLoad, -}); diff --git a/src/plugins/unified_histogram/public/chart/use_lens_props.test.ts b/src/plugins/unified_histogram/public/chart/use_lens_props.test.ts new file mode 100644 index 0000000000000..289536edd0232 --- /dev/null +++ b/src/plugins/unified_histogram/public/chart/use_lens_props.test.ts @@ -0,0 +1,93 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { renderHook } from '@testing-library/react-hooks'; +import { act } from 'react-test-renderer'; +import { Subject } from 'rxjs'; +import type { UnifiedHistogramInputMessage } from '../types'; +import { dataViewWithTimefieldMock } from '../__mocks__/data_view_with_timefield'; +import { getLensAttributes } from './get_lens_attributes'; +import { getLensProps, useLensProps } from './use_lens_props'; + +describe('useLensProps', () => { + it('should return lens props', () => { + const getTimeRange = jest.fn(); + const refetch$ = new Subject(); + const onLoad = jest.fn(); + const attributes = getLensAttributes({ + title: 'test', + filters: [], + query: { + language: 'kuery', + query: '', + }, + dataView: dataViewWithTimefieldMock, + timeInterval: 'auto', + breakdownField: dataViewWithTimefieldMock.getFieldByName('extension'), + }); + const lensProps = renderHook(() => { + return useLensProps({ + request: { + searchSessionId: 'id', + adapter: undefined, + }, + getTimeRange, + refetch$, + attributes, + onLoad, + }); + }); + expect(lensProps.result.current).toEqual( + getLensProps({ + searchSessionId: 'id', + getTimeRange, + attributes, + onLoad, + }) + ); + }); + + it('should only update lens props when refetch$ is triggered', () => { + const getTimeRange = jest.fn(); + const refetch$ = new Subject(); + const onLoad = jest.fn(); + const lensProps = { + request: { + searchSessionId: '123', + adapter: undefined, + }, + getTimeRange, + refetch$, + attributes: getLensAttributes({ + title: 'test', + filters: [], + query: { + language: 'kuery', + query: '', + }, + dataView: dataViewWithTimefieldMock, + timeInterval: 'auto', + breakdownField: dataViewWithTimefieldMock.getFieldByName('extension'), + }), + onLoad, + }; + const hook = renderHook( + (props) => { + return useLensProps(props); + }, + { initialProps: lensProps } + ); + const originalProps = hook.result.current; + hook.rerender({ ...lensProps, request: { searchSessionId: '456', adapter: undefined } }); + expect(hook.result.current).toEqual(originalProps); + act(() => { + refetch$.next({ type: 'refetch' }); + }); + expect(hook.result.current).not.toEqual(originalProps); + }); +}); diff --git a/src/plugins/unified_histogram/public/chart/use_lens_props.ts b/src/plugins/unified_histogram/public/chart/use_lens_props.ts new file mode 100644 index 0000000000000..976f191dba5c3 --- /dev/null +++ b/src/plugins/unified_histogram/public/chart/use_lens_props.ts @@ -0,0 +1,74 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import type { TimeRange } from '@kbn/data-plugin/common'; +import { ViewMode } from '@kbn/embeddable-plugin/public'; +import type { DefaultInspectorAdapters } from '@kbn/expressions-plugin/common'; +import type { TypedLensByValueInput } from '@kbn/lens-plugin/public'; +import { useCallback, useEffect, useState } from 'react'; +import type { Observable } from 'rxjs'; +import type { UnifiedHistogramInputMessage, UnifiedHistogramRequestContext } from '../types'; +import { useStableCallback } from './use_stable_callback'; + +export const useLensProps = ({ + request, + getTimeRange, + refetch$, + attributes, + onLoad, +}: { + request?: UnifiedHistogramRequestContext; + getTimeRange: () => TimeRange; + refetch$: Observable; + attributes: TypedLensByValueInput['attributes']; + onLoad: (isLoading: boolean, adapters: Partial | undefined) => void; +}) => { + const buildLensProps = useCallback( + () => + getLensProps({ + searchSessionId: request?.searchSessionId, + getTimeRange, + attributes, + onLoad, + }), + [attributes, getTimeRange, onLoad, request?.searchSessionId] + ); + + const [lensProps, setLensProps] = useState(buildLensProps()); + const updateLensProps = useStableCallback(() => setLensProps(buildLensProps())); + + useEffect(() => { + const subscription = refetch$.subscribe(updateLensProps); + return () => subscription.unsubscribe(); + }, [refetch$, updateLensProps]); + + return lensProps; +}; + +export const getLensProps = ({ + searchSessionId, + getTimeRange, + attributes, + onLoad, +}: { + searchSessionId?: string; + getTimeRange: () => TimeRange; + attributes: TypedLensByValueInput['attributes']; + onLoad: (isLoading: boolean, adapters: Partial | undefined) => void; +}) => ({ + id: 'unifiedHistogramLensComponent', + viewMode: ViewMode.VIEW, + timeRange: getTimeRange(), + attributes, + noPadding: true, + searchSessionId, + executionContext: { + description: 'fetch chart data and total hits', + }, + onLoad, +}); diff --git a/src/plugins/unified_histogram/public/chart/use_refetch.test.ts b/src/plugins/unified_histogram/public/chart/use_refetch.test.ts new file mode 100644 index 0000000000000..39381ea661865 --- /dev/null +++ b/src/plugins/unified_histogram/public/chart/use_refetch.test.ts @@ -0,0 +1,85 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { useRefetch } from './use_refetch'; +import { DataView } from '@kbn/data-views-plugin/common'; +import { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query'; +import { renderHook } from '@testing-library/react-hooks'; +import { + UnifiedHistogramBreakdownContext, + UnifiedHistogramChartContext, + UnifiedHistogramHitsContext, + UnifiedHistogramInput$, + UnifiedHistogramRequestContext, +} from '../types'; +import { dataViewWithTimefieldMock } from '../__mocks__/data_view_with_timefield'; +import { Subject } from 'rxjs'; + +describe('useRefetch', () => { + const getDeps: () => { + dataView: DataView; + request: UnifiedHistogramRequestContext | undefined; + hits: UnifiedHistogramHitsContext | undefined; + chart: UnifiedHistogramChartContext | undefined; + chartVisible: boolean; + breakdown: UnifiedHistogramBreakdownContext | undefined; + filters: Filter[]; + query: Query | AggregateQuery; + relativeTimeRange: TimeRange; + input$: UnifiedHistogramInput$; + beforeRefetch: () => void; + } = () => ({ + dataView: dataViewWithTimefieldMock, + request: undefined, + hits: undefined, + chart: undefined, + chartVisible: true, + breakdown: undefined, + filters: [], + query: { language: 'kuery', query: '' }, + relativeTimeRange: { from: 'now-15m', to: 'now' }, + input$: new Subject(), + beforeRefetch: () => {}, + }); + + it('should trigger the refetch observable when any of the arguments change', () => { + const originalDeps = getDeps(); + const hook = renderHook((deps) => useRefetch(deps), { + initialProps: originalDeps, + }); + const refetch = jest.fn(); + hook.result.current.subscribe(refetch); + hook.rerender({ ...originalDeps }); + expect(refetch).not.toHaveBeenCalled(); + hook.rerender({ ...originalDeps, chartVisible: false }); + expect(refetch).toHaveBeenCalledTimes(1); + }); + + it('should not trigger the refetch observable when disableAutoFetching is true', () => { + const originalDeps = { ...getDeps(), disableAutoFetching: true }; + const hook = renderHook((deps) => useRefetch(deps), { + initialProps: originalDeps, + }); + const refetch = jest.fn(); + hook.result.current.subscribe(refetch); + hook.rerender({ ...originalDeps, chartVisible: false }); + expect(refetch).not.toHaveBeenCalled(); + }); + + it('should trigger the refetch observable when the input$ observable is triggered', () => { + const originalDeps = { ...getDeps(), disableAutoFetching: true }; + const hook = renderHook((deps) => useRefetch(deps), { + initialProps: originalDeps, + }); + const refetch = jest.fn(); + hook.result.current.subscribe(refetch); + expect(refetch).not.toHaveBeenCalled(); + originalDeps.input$.next({ type: 'refetch' }); + expect(refetch).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/plugins/unified_histogram/public/chart/use_refetch_id.ts b/src/plugins/unified_histogram/public/chart/use_refetch.ts similarity index 79% rename from src/plugins/unified_histogram/public/chart/use_refetch_id.ts rename to src/plugins/unified_histogram/public/chart/use_refetch.ts index 4415be9ccd8b6..31e08f3d732e5 100644 --- a/src/plugins/unified_histogram/public/chart/use_refetch_id.ts +++ b/src/plugins/unified_histogram/public/chart/use_refetch.ts @@ -9,17 +9,18 @@ import type { DataView } from '@kbn/data-views-plugin/common'; import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query'; import { cloneDeep, isEqual } from 'lodash'; -import { useEffect, useRef, useState } from 'react'; -import type { +import { useEffect, useMemo, useRef } from 'react'; +import { filter, share, tap } from 'rxjs'; +import { UnifiedHistogramBreakdownContext, UnifiedHistogramChartContext, UnifiedHistogramHitsContext, + UnifiedHistogramInput$, UnifiedHistogramRequestContext, } from '../types'; -export const useRefetchId = ({ +export const useRefetch = ({ dataView, - lastReloadRequestTime, request, hits, chart, @@ -27,10 +28,12 @@ export const useRefetchId = ({ breakdown, filters, query, - relativeTimeRange: relativeTimeRange, + relativeTimeRange, + disableAutoFetching, + input$, + beforeRefetch, }: { dataView: DataView; - lastReloadRequestTime: number | undefined; request: UnifiedHistogramRequestContext | undefined; hits: UnifiedHistogramHitsContext | undefined; chart: UnifiedHistogramChartContext | undefined; @@ -39,17 +42,23 @@ export const useRefetchId = ({ filters: Filter[]; query: Query | AggregateQuery; relativeTimeRange: TimeRange; + disableAutoFetching?: boolean; + input$: UnifiedHistogramInput$; + beforeRefetch: () => void; }) => { const refetchDeps = useRef>(); - const [refetchId, setRefetchId] = useState(0); // When the unified histogram props change, we must compare the current subset // that should trigger a histogram refetch against the previous subset. If they // are different, we must refetch the histogram to ensure it's up to date. useEffect(() => { + // Skip if auto fetching if disabled + if (disableAutoFetching) { + return; + } + const newRefetchDeps = getRefetchDeps({ dataView, - lastReloadRequestTime, request, hits, chart, @@ -62,7 +71,7 @@ export const useRefetchId = ({ if (!isEqual(refetchDeps.current, newRefetchDeps)) { if (refetchDeps.current) { - setRefetchId((id) => id + 1); + input$.next({ type: 'refetch' }); } refetchDeps.current = newRefetchDeps; @@ -72,20 +81,28 @@ export const useRefetchId = ({ chart, chartVisible, dataView, + disableAutoFetching, filters, hits, - lastReloadRequestTime, + input$, query, - request, relativeTimeRange, + request, ]); - return refetchId; + return useMemo( + () => + input$.pipe( + filter((message) => message.type === 'refetch'), + tap(beforeRefetch), + share() + ), + [beforeRefetch, input$] + ); }; const getRefetchDeps = ({ dataView, - lastReloadRequestTime, request, hits, chart, @@ -96,7 +113,6 @@ const getRefetchDeps = ({ relativeTimeRange, }: { dataView: DataView; - lastReloadRequestTime: number | undefined; request: UnifiedHistogramRequestContext | undefined; hits: UnifiedHistogramHitsContext | undefined; chart: UnifiedHistogramChartContext | undefined; @@ -108,7 +124,6 @@ const getRefetchDeps = ({ }) => cloneDeep([ dataView.id, - lastReloadRequestTime, request?.searchSessionId, Boolean(hits), chartVisible, diff --git a/src/plugins/unified_histogram/public/chart/use_refetch_id.test.ts b/src/plugins/unified_histogram/public/chart/use_refetch_id.test.ts deleted file mode 100644 index 8835173df2599..0000000000000 --- a/src/plugins/unified_histogram/public/chart/use_refetch_id.test.ts +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -import { DataView } from '@kbn/data-views-plugin/common'; -import { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query'; -import { renderHook } from '@testing-library/react-hooks'; -import { - UnifiedHistogramBreakdownContext, - UnifiedHistogramChartContext, - UnifiedHistogramHitsContext, - UnifiedHistogramRequestContext, -} from '../types'; -import { dataViewWithTimefieldMock } from '../__mocks__/data_view_with_timefield'; -import { useRefetchId } from './use_refetch_id'; - -describe('useRefetchId', () => { - const getDeps: () => { - dataView: DataView; - lastReloadRequestTime: number | undefined; - request: UnifiedHistogramRequestContext | undefined; - hits: UnifiedHistogramHitsContext | undefined; - chart: UnifiedHistogramChartContext | undefined; - chartVisible: boolean; - breakdown: UnifiedHistogramBreakdownContext | undefined; - filters: Filter[]; - query: Query | AggregateQuery; - relativeTimeRange: TimeRange; - } = () => ({ - dataView: dataViewWithTimefieldMock, - lastReloadRequestTime: 0, - request: undefined, - hits: undefined, - chart: undefined, - chartVisible: true, - breakdown: undefined, - filters: [], - query: { language: 'kuery', query: '' }, - relativeTimeRange: { from: 'now-15m', to: 'now' }, - }); - - it('should increment the refetchId when any of the arguments change', () => { - const hook = renderHook((props) => useRefetchId(props), { initialProps: getDeps() }); - expect(hook.result.current).toBe(0); - hook.rerender(getDeps()); - expect(hook.result.current).toBe(0); - hook.rerender({ - ...getDeps(), - lastReloadRequestTime: 1, - }); - expect(hook.result.current).toBe(1); - hook.rerender({ - ...getDeps(), - lastReloadRequestTime: 1, - }); - expect(hook.result.current).toBe(1); - hook.rerender({ - ...getDeps(), - lastReloadRequestTime: 1, - query: { language: 'kuery', query: 'foo' }, - }); - expect(hook.result.current).toBe(2); - }); -}); diff --git a/src/plugins/unified_histogram/public/chart/use_request_params.test.ts b/src/plugins/unified_histogram/public/chart/use_request_params.test.ts new file mode 100644 index 0000000000000..c49bcd4ce195b --- /dev/null +++ b/src/plugins/unified_histogram/public/chart/use_request_params.test.ts @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { renderHook } from '@testing-library/react-hooks'; +import { unifiedHistogramServicesMock } from '../__mocks__/services'; + +const getUseRequestParams = async () => { + jest.doMock('@kbn/data-plugin/common', () => { + return { + ...jest.requireActual('@kbn/data-plugin/common'), + getAbsoluteTimeRange: jest.fn((range) => range), + }; + }); + return (await import('./use_request_params')).useRequestParams; +}; + +describe('useRequestParams', () => { + it('should only update getTimeRange after updateTimeRange is called', async () => { + const useRequestParams = await getUseRequestParams(); + const originalTimeRange = { + from: 'now-15m', + to: 'now', + }; + const originalProps = { + services: unifiedHistogramServicesMock, + timeRange: originalTimeRange, + filters: [], + query: { + query: '', + language: 'kuery', + }, + }; + const hook = renderHook((props) => useRequestParams(props), { + initialProps: originalProps, + }); + expect(hook.result.current.getTimeRange()).toEqual(originalTimeRange); + const newTimeRange = { from: 'now-30m', to: 'now' }; + hook.rerender({ ...originalProps, timeRange: newTimeRange }); + expect(hook.result.current.getTimeRange()).toEqual(originalTimeRange); + hook.result.current.updateTimeRange(); + expect(hook.result.current.getTimeRange()).toEqual(newTimeRange); + }); +}); diff --git a/src/plugins/unified_histogram/public/chart/use_request_params.tsx b/src/plugins/unified_histogram/public/chart/use_request_params.tsx index defa2bdd920d9..8afe803d329af 100644 --- a/src/plugins/unified_histogram/public/chart/use_request_params.tsx +++ b/src/plugins/unified_histogram/public/chart/use_request_params.tsx @@ -6,53 +6,42 @@ * Side Public License, v 1. */ -import { connectToQueryState, QueryState } from '@kbn/data-plugin/public'; -import { createStateContainer, useContainerState } from '@kbn/kibana-utils-plugin/public'; -import { useEffect, useMemo, useRef } from 'react'; -import type { UnifiedHistogramRequestContext, UnifiedHistogramServices } from '../types'; +import { getAbsoluteTimeRange } from '@kbn/data-plugin/common'; +import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query'; +import { useCallback, useMemo, useRef } from 'react'; +import type { UnifiedHistogramServices } from '../types'; +import { useStableCallback } from './use_stable_callback'; export const useRequestParams = ({ services, - lastReloadRequestTime, - request, + query: originalQuery, + filters: originalFilters, + timeRange: originalTimeRange, }: { services: UnifiedHistogramServices; - lastReloadRequestTime: number | undefined; - request?: UnifiedHistogramRequestContext; + query?: Query | AggregateQuery; + filters?: Filter[]; + timeRange?: TimeRange; }) => { const { data } = services; - const queryStateContainer = useRef( - createStateContainer({ - filters: data.query.filterManager.getFilters(), - query: data.query.queryString.getQuery(), - refreshInterval: data.query.timefilter.timefilter.getRefreshInterval(), - time: data.query.timefilter.timefilter.getTime(), - }) - ).current; - - const queryState = useContainerState(queryStateContainer); - - useEffect(() => { - return connectToQueryState(data.query, queryStateContainer, { - time: true, - query: true, - filters: true, - refreshInterval: true, - }); - }, [data.query, queryStateContainer]); - - const filters = useMemo(() => queryState.filters ?? [], [queryState.filters]); + const filters = useMemo(() => originalFilters ?? [], [originalFilters]); const query = useMemo( - () => queryState.query ?? data.query.queryString.getDefaultQuery(), - [data.query.queryString, queryState.query] + () => originalQuery ?? data.query.queryString.getDefaultQuery(), + [data.query.queryString, originalQuery] ); const relativeTimeRange = useMemo( - () => queryState.time ?? data.query.timefilter.timefilter.getTimeDefaults(), - [data.query.timefilter.timefilter, queryState.time] + () => originalTimeRange ?? data.query.timefilter.timefilter.getTimeDefaults(), + [data.query.timefilter.timefilter, originalTimeRange] ); - return { filters, query, relativeTimeRange }; + const timeRange = useRef(getAbsoluteTimeRange(relativeTimeRange)); + const getTimeRange = useCallback(() => timeRange.current, []); + const updateTimeRange = useStableCallback(() => { + timeRange.current = getAbsoluteTimeRange(relativeTimeRange); + }); + + return { filters, query, getTimeRange, updateTimeRange, relativeTimeRange }; }; diff --git a/src/plugins/unified_histogram/public/chart/use_stable_callback.test.ts b/src/plugins/unified_histogram/public/chart/use_stable_callback.test.ts new file mode 100644 index 0000000000000..2ea1be911df0e --- /dev/null +++ b/src/plugins/unified_histogram/public/chart/use_stable_callback.test.ts @@ -0,0 +1,19 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { renderHook } from '@testing-library/react-hooks'; +import { useStableCallback } from './use_stable_callback'; + +describe('useStableCallback', () => { + it('should return a stable callback', () => { + const hook = renderHook((cb) => useStableCallback(cb), { initialProps: () => {} }); + const firstCallback = hook.result.current; + hook.rerender(() => {}); + expect(hook.result.current).toBe(firstCallback); + }); +}); diff --git a/src/plugins/unified_histogram/public/chart/use_stable_callback.ts b/src/plugins/unified_histogram/public/chart/use_stable_callback.ts new file mode 100644 index 0000000000000..9b81470c5ff5c --- /dev/null +++ b/src/plugins/unified_histogram/public/chart/use_stable_callback.ts @@ -0,0 +1,23 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { useCallback, useEffect, useRef } from 'react'; + +/** + * Accepts a callback and returns a function with a stable identity + * that will always call the latest version of the callback when invoked + */ +export const useStableCallback = any>(fn: T | undefined) => { + const ref = useRef(fn); + + useEffect(() => { + ref.current = fn; + }, [fn]); + + return useCallback((...args: Parameters) => ref.current?.(...args), []); +}; diff --git a/src/plugins/unified_histogram/public/chart/use_total_hits.test.ts b/src/plugins/unified_histogram/public/chart/use_total_hits.test.ts index 4782df3683fcb..ea6ab4676e681 100644 --- a/src/plugins/unified_histogram/public/chart/use_total_hits.test.ts +++ b/src/plugins/unified_histogram/public/chart/use_total_hits.test.ts @@ -7,14 +7,14 @@ */ import { Filter } from '@kbn/es-query'; -import { UnifiedHistogramFetchStatus } from '../types'; +import { UnifiedHistogramFetchStatus, UnifiedHistogramInput$ } from '../types'; import { dataViewWithTimefieldMock } from '../__mocks__/data_view_with_timefield'; import { useTotalHits } from './use_total_hits'; import { useEffect as mockUseEffect } from 'react'; import { renderHook } from '@testing-library/react-hooks'; import { dataPluginMock } from '@kbn/data-plugin/public/mocks'; import { searchSourceInstanceMock } from '@kbn/data-plugin/common/search/search_source/mocks'; -import { of, throwError } from 'rxjs'; +import { of, Subject, throwError } from 'rxjs'; import { waitFor } from '@testing-library/dom'; import { RequestAdapter } from '@kbn/inspector-plugin/common'; import { DataViewType, SearchSourceSearchOptions } from '@kbn/data-plugin/common'; @@ -26,25 +26,21 @@ jest.mock('react-use/lib/useDebounce', () => { }); describe('useTotalHits', () => { + const timeRange = { from: 'now-15m', to: 'now' }; + const refetch$: UnifiedHistogramInput$ = new Subject(); const getDeps = () => ({ services: { data: dataPluginMock.createStartContract() } as any, dataView: dataViewWithTimefieldMock, - lastReloadRequestTime: undefined, request: undefined, hits: { status: UnifiedHistogramFetchStatus.uninitialized, total: undefined, }, - chart: { - hidden: true, - timeInterval: 'auto', - }, chartVisible: false, - breakdown: undefined, filters: [], query: { query: '', language: 'kuery' }, - timeRange: { from: 'now-15m', to: 'now' }, - refetchId: 0, + getTimeRange: () => timeRange, + refetch$, onTotalHitsChange: jest.fn(), }); @@ -68,7 +64,6 @@ describe('useTotalHits', () => { }); const setFieldSpy = jest.spyOn(searchSourceInstanceMock, 'setField').mockClear(); const data = dataPluginMock.createStartContract(); - const timeRange = { from: 'now-15m', to: 'now' }; jest .spyOn(data.query.timefilter.timefilter, 'createFilter') .mockClear() @@ -86,7 +81,7 @@ describe('useTotalHits', () => { }, query, filters, - timeRange, + refetch$, onTotalHitsChange, }) ); @@ -128,11 +123,11 @@ describe('useTotalHits', () => { expect(fetchSpy).not.toHaveBeenCalled(); }); - it('should not fetch a second time if fetchId is the same', async () => { + it('should not fetch a second time if refetch$ is not triggered', async () => { const onTotalHitsChange = jest.fn(); const fetchSpy = jest.spyOn(searchSourceInstanceMock, 'fetch$').mockClear(); const setFieldSpy = jest.spyOn(searchSourceInstanceMock, 'setField').mockClear(); - const options = { ...getDeps(), refetchId: 0, onTotalHitsChange }; + const options = { ...getDeps(), onTotalHitsChange }; const { rerender } = renderHook(() => useTotalHits(options)); expect(onTotalHitsChange).toBeCalledTimes(1); expect(setFieldSpy).toHaveBeenCalled(); @@ -146,12 +141,12 @@ describe('useTotalHits', () => { expect(fetchSpy).toHaveBeenCalledTimes(1); }); - it('should fetch a second time if fetchId is different', async () => { + it('should fetch a second time if refetch$ is triggered', async () => { const abortSpy = jest.spyOn(AbortController.prototype, 'abort').mockClear(); const onTotalHitsChange = jest.fn(); const fetchSpy = jest.spyOn(searchSourceInstanceMock, 'fetch$').mockClear(); const setFieldSpy = jest.spyOn(searchSourceInstanceMock, 'setField').mockClear(); - const options = { ...getDeps(), refetchId: 0, onTotalHitsChange }; + const options = { ...getDeps(), onTotalHitsChange }; const { rerender } = renderHook(() => useTotalHits(options)); expect(onTotalHitsChange).toBeCalledTimes(1); expect(setFieldSpy).toHaveBeenCalled(); @@ -159,7 +154,7 @@ describe('useTotalHits', () => { await waitFor(() => { expect(onTotalHitsChange).toBeCalledTimes(2); }); - options.refetchId = 1; + refetch$.next({ type: 'refetch' }); rerender(); expect(abortSpy).toHaveBeenCalled(); expect(onTotalHitsChange).toBeCalledTimes(3); @@ -190,7 +185,6 @@ describe('useTotalHits', () => { .mockClear(); const setFieldSpy = jest.spyOn(searchSourceInstanceMock, 'setField').mockClear(); const data = dataPluginMock.createStartContract(); - const timeRange = { from: 'now-15m', to: 'now' }; jest .spyOn(data.query.timefilter.timefilter, 'createFilter') .mockClear() diff --git a/src/plugins/unified_histogram/public/chart/use_total_hits.ts b/src/plugins/unified_histogram/public/chart/use_total_hits.ts index 3f24b642c81bf..e7227edd76684 100644 --- a/src/plugins/unified_histogram/public/chart/use_total_hits.ts +++ b/src/plugins/unified_histogram/public/chart/use_total_hits.ts @@ -10,68 +10,63 @@ import { isCompleteResponse } from '@kbn/data-plugin/public'; import { DataView, DataViewType } from '@kbn/data-views-plugin/public'; import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query'; import { i18n } from '@kbn/i18n'; -import { MutableRefObject, useRef } from 'react'; -import useDebounce from 'react-use/lib/useDebounce'; -import { catchError, filter, lastValueFrom, map, of } from 'rxjs'; +import { MutableRefObject, useEffect, useRef } from 'react'; +import useEffectOnce from 'react-use/lib/useEffectOnce'; +import { catchError, filter, lastValueFrom, map, Observable, of } from 'rxjs'; import { - UnifiedHistogramBreakdownContext, - UnifiedHistogramChartContext, UnifiedHistogramFetchStatus, UnifiedHistogramHitsContext, + UnifiedHistogramInputMessage, UnifiedHistogramRequestContext, UnifiedHistogramServices, } from '../types'; -import { REQUEST_DEBOUNCE_MS } from './consts'; +import { useStableCallback } from './use_stable_callback'; export const useTotalHits = ({ services, dataView, - lastReloadRequestTime, request, hits, - chart, chartVisible, - breakdown, filters, query, - timeRange, - refetchId, + getTimeRange, + refetch$, onTotalHitsChange, }: { services: UnifiedHistogramServices; dataView: DataView; - lastReloadRequestTime: number | undefined; request: UnifiedHistogramRequestContext | undefined; hits: UnifiedHistogramHitsContext | undefined; - chart: UnifiedHistogramChartContext | undefined; chartVisible: boolean; - breakdown: UnifiedHistogramBreakdownContext | undefined; filters: Filter[]; query: Query | AggregateQuery; - timeRange: TimeRange; - refetchId: number; + getTimeRange: () => TimeRange; + refetch$: Observable; onTotalHitsChange?: (status: UnifiedHistogramFetchStatus, result?: number | Error) => void; }) => { const abortController = useRef(); - - useDebounce( - () => { - fetchTotalHits({ - services, - abortController, - dataView, - request, - hits, - chartVisible, - filters, - query, - timeRange, - onTotalHitsChange, - }); - }, - REQUEST_DEBOUNCE_MS, - [onTotalHitsChange, refetchId, services] - ); + const fetch = useStableCallback(() => { + fetchTotalHits({ + services, + abortController, + dataView, + request, + hits, + chartVisible, + filters, + query, + timeRange: getTimeRange(), + onTotalHitsChange, + }); + }); + + useEffectOnce(fetch); + + useEffect(() => { + const subscription = refetch$.subscribe(fetch); + return () => subscription.unsubscribe(); + }, [fetch, refetch$]); }; const fetchTotalHits = async ({ diff --git a/src/plugins/unified_histogram/public/index.ts b/src/plugins/unified_histogram/public/index.ts index 4a8f73477c0b6..97cebb51fdc2f 100644 --- a/src/plugins/unified_histogram/public/index.ts +++ b/src/plugins/unified_histogram/public/index.ts @@ -18,6 +18,9 @@ export type { UnifiedHistogramBreakdownContext, UnifiedHistogramChartLoadEvent, UnifiedHistogramAdapters, + UnifiedHistogramRefetchMessage, + UnifiedHistogramInputMessage, + UnifiedHistogramInput$, } from './types'; export { UnifiedHistogramFetchStatus } from './types'; diff --git a/src/plugins/unified_histogram/public/layout/layout.test.tsx b/src/plugins/unified_histogram/public/layout/layout.test.tsx index d77bbfa05be30..aee62e3c7d3b8 100644 --- a/src/plugins/unified_histogram/public/layout/layout.test.tsx +++ b/src/plugins/unified_histogram/public/layout/layout.test.tsx @@ -56,14 +56,6 @@ describe('Layout', () => { hits?: UnifiedHistogramHitsContext | null; chart?: UnifiedHistogramChartContext | null; } = {}) => { - services.data.query.timefilter.timefilter.getAbsoluteTime = () => { - return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; - }; - - (services.data.query.queryString.getDefaultQuery as jest.Mock).mockReturnValue({ - language: 'kuery', - query: '', - }); (searchSourceInstanceMock.fetch$ as jest.Mock).mockImplementation( jest.fn().mockReturnValue(of({ rawResponse: { hits: { total: 2 } } })) ); @@ -75,6 +67,15 @@ describe('Layout', () => { chart={chart ?? undefined} resizeRef={resizeRef} dataView={dataViewWithTimefieldMock} + query={{ + language: 'kuery', + query: '', + }} + filters={[]} + timeRange={{ + from: '2020-05-14T11:05:13.590', + to: '2020-05-14T11:20:13.590', + }} {...rest} /> ); diff --git a/src/plugins/unified_histogram/public/layout/layout.tsx b/src/plugins/unified_histogram/public/layout/layout.tsx index 87d4170a1035f..db6a161a81b7e 100644 --- a/src/plugins/unified_histogram/public/layout/layout.tsx +++ b/src/plugins/unified_histogram/public/layout/layout.tsx @@ -12,7 +12,8 @@ import React, { useMemo } from 'react'; import { createHtmlPortalNode, InPortal, OutPortal } from 'react-reverse-portal'; import { css } from '@emotion/css'; import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; -import type { TypedLensByValueInput } from '@kbn/lens-plugin/public'; +import type { LensEmbeddableInput, TypedLensByValueInput } from '@kbn/lens-plugin/public'; +import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query'; import { Chart } from '../chart'; import { Panels, PANELS_MODE } from '../panels'; import type { @@ -23,6 +24,7 @@ import type { UnifiedHistogramFetchStatus, UnifiedHistogramRequestContext, UnifiedHistogramChartLoadEvent, + UnifiedHistogramInput$, } from '../types'; export interface UnifiedHistogramLayoutProps extends PropsWithChildren { @@ -39,9 +41,17 @@ export interface UnifiedHistogramLayoutProps extends PropsWithChildren */ dataView: DataView; /** - * Can be updated to `Date.now()` to force a refresh + * The current query */ - lastReloadRequestTime?: number; + query?: Query | AggregateQuery; + /** + * The current filters + */ + filters?: Filter[]; + /** + * The current time range + */ + timeRange?: TimeRange; /** * Context object for requests made by unified histogram components -- optional */ @@ -70,6 +80,22 @@ export interface UnifiedHistogramLayoutProps extends PropsWithChildren * Append a custom element to the right of the hits count */ appendHitsCounter?: ReactElement; + /** + * Disable automatic refetching based on props changes, and instead wait for a `refetch` message + */ + disableAutoFetching?: boolean; + /** + * Disable triggers for the Lens embeddable + */ + disableTriggers?: LensEmbeddableInput['disableTriggers']; + /** + * Disabled action IDs for the Lens embeddable + */ + disabledActions?: LensEmbeddableInput['disabledActions']; + /** + * Input observable + */ + input$?: UnifiedHistogramInput$; /** * Callback to update the topPanelHeight prop when a resize is triggered */ @@ -99,13 +125,23 @@ export interface UnifiedHistogramLayoutProps extends PropsWithChildren * Called when the histogram loading status changes */ onChartLoad?: (event: UnifiedHistogramChartLoadEvent) => void; + /** + * Callback to pass to the Lens embeddable to handle filter changes + */ + onFilter?: LensEmbeddableInput['onFilter']; + /** + * Callback to pass to the Lens embeddable to handle brush events + */ + onBrushEnd?: LensEmbeddableInput['onBrushEnd']; } export const UnifiedHistogramLayout = ({ className, services, dataView, - lastReloadRequestTime, + query, + filters, + timeRange, request, hits, chart, @@ -113,6 +149,10 @@ export const UnifiedHistogramLayout = ({ resizeRef, topPanelHeight, appendHitsCounter, + disableAutoFetching, + disableTriggers, + disabledActions, + input$, onTopPanelHeightChange, onEditVisualization, onChartHiddenChange, @@ -120,6 +160,8 @@ export const UnifiedHistogramLayout = ({ onBreakdownFieldChange, onTotalHitsChange, onChartLoad, + onFilter, + onBrushEnd, children, }: UnifiedHistogramLayoutProps) => { const topPanelNode = useMemo( @@ -167,13 +209,19 @@ export const UnifiedHistogramLayout = ({ className={chartClassName} services={services} dataView={dataView} - lastReloadRequestTime={lastReloadRequestTime} + query={query} + filters={filters} + timeRange={timeRange} request={request} hits={hits} chart={chart} breakdown={breakdown} appendHitsCounter={appendHitsCounter} appendHistogram={showFixedPanels ? : } + disableAutoFetching={disableAutoFetching} + disableTriggers={disableTriggers} + disabledActions={disabledActions} + input$={input$} onEditVisualization={onEditVisualization} onResetChartHeight={onResetChartHeight} onChartHiddenChange={onChartHiddenChange} @@ -181,6 +229,8 @@ export const UnifiedHistogramLayout = ({ onBreakdownFieldChange={onBreakdownFieldChange} onTotalHitsChange={onTotalHitsChange} onChartLoad={onChartLoad} + onFilter={onFilter} + onBrushEnd={onBrushEnd} /> {children} diff --git a/src/plugins/unified_histogram/public/types.ts b/src/plugins/unified_histogram/public/types.ts index a4b253274abde..f77bfa1bbdbee 100644 --- a/src/plugins/unified_histogram/public/types.ts +++ b/src/plugins/unified_histogram/public/types.ts @@ -14,6 +14,7 @@ import type { LensPublicStart } from '@kbn/lens-plugin/public'; import type { DataViewField } from '@kbn/data-views-plugin/public'; import type { RequestAdapter } from '@kbn/inspector-plugin/public'; import type { DefaultInspectorAdapters } from '@kbn/expressions-plugin/common'; +import type { Subject } from 'rxjs'; /** * The fetch status of a unified histogram request @@ -52,10 +53,6 @@ export type UnifiedHistogramAdapters = Partial; * Emitted when the histogram loading status changes */ export interface UnifiedHistogramChartLoadEvent { - /** - * True if loading is complete - */ - complete: boolean; /** * Inspector adapters for the request */ @@ -117,3 +114,20 @@ export interface UnifiedHistogramBreakdownContext { */ field?: DataViewField; } + +/** + * Message to refetch the chart and total hits + */ +export interface UnifiedHistogramRefetchMessage { + type: 'refetch'; +} + +/** + * Unified histogram input message + */ +export type UnifiedHistogramInputMessage = UnifiedHistogramRefetchMessage; + +/** + * Unified histogram input observable + */ +export type UnifiedHistogramInput$ = Subject; diff --git a/src/plugins/unified_histogram/tsconfig.json b/src/plugins/unified_histogram/tsconfig.json index a6bd54ed5e6ea..eb7da12b70b29 100644 --- a/src/plugins/unified_histogram/tsconfig.json +++ b/src/plugins/unified_histogram/tsconfig.json @@ -18,7 +18,6 @@ "@kbn/i18n", "@kbn/es-query", "@kbn/embeddable-plugin", - "@kbn/kibana-utils-plugin", "@kbn/core-ui-settings-browser", "@kbn/datemath", "@kbn/core-ui-settings-browser-mocks", diff --git a/test/functional/apps/discover/group1/_discover_histogram.ts b/test/functional/apps/discover/group1/_discover_histogram.ts index 70a1fba5afafc..e451eee881335 100644 --- a/test/functional/apps/discover/group1/_discover_histogram.ts +++ b/test/functional/apps/discover/group1/_discover_histogram.ts @@ -30,6 +30,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { before(async () => { await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); await esArchiver.load('test/functional/fixtures/es_archiver/long_window_logstash'); + await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover'); await kibanaServer.importExport.load( 'test/functional/fixtures/kbn_archiver/long_window_logstash_index_pattern' ); @@ -96,6 +97,20 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); }); + it('should update correctly when switching data views and brushing the histogram', async () => { + await PageObjects.common.navigateToApp('discover'); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.timePicker.setDefaultAbsoluteRange(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.discover.selectIndexPattern('logstash-*'); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.discover.selectIndexPattern('long-window-logstash-*'); + await PageObjects.discover.waitUntilSearchingHasFinished(); + await PageObjects.discover.brushHistogram(); + await PageObjects.discover.waitUntilSearchingHasFinished(); + expect(await PageObjects.discover.getHitCount()).to.be('7'); + }); + it('should update the histogram timerange when the query is resubmitted', async function () { await kibanaServer.uiSettings.update({ 'timepicker:timeDefaults': '{ "from": "2015-09-18T19:37:13.000Z", "to": "now"}',