Skip to content

Commit 96d6b89

Browse files
[Unified Histogram] [Discover] Unified Histogram solutions cleanup (#146352)
## 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: reactjs/rfcs#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 <[email protected]>
1 parent 74ba605 commit 96d6b89

34 files changed

+903
-418
lines changed

src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.test.tsx

+4-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ const mountComponent = ({
6969
services.data.query.timefilter.timefilter.getAbsoluteTime = () => {
7070
return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' };
7171
};
72-
72+
services.data.query.timefilter.timefilter.getTime = () => {
73+
return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' };
74+
};
7375
(services.data.query.queryString.getDefaultQuery as jest.Mock).mockReturnValue({
7476
language: 'kuery',
7577
query: '',
@@ -123,6 +125,7 @@ const mountComponent = ({
123125
setExpandedDoc: jest.fn(),
124126
savedSearch,
125127
savedSearchData$,
128+
savedSearchFetch$: new Subject(),
126129
savedSearchRefetch$: new Subject(),
127130
stateContainer,
128131
onFieldEdited: jest.fn(),

src/plugins/discover/public/application/main/components/layout/discover_histogram_layout.tsx

+4
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@ import type { DiscoverSearchSessionManager } from '../../services/discover_searc
1515
import type { InspectorAdapters } from '../../hooks/use_inspector';
1616
import { type DiscoverMainContentProps, DiscoverMainContent } from './discover_main_content';
1717
import { ResetSearchButton } from './reset_search_button';
18+
import type { DataFetch$ } from '../../hooks/use_saved_search';
1819

1920
export interface DiscoverHistogramLayoutProps extends DiscoverMainContentProps {
2021
resetSavedSearch: () => void;
2122
isTimeBased: boolean;
2223
resizeRef: RefObject<HTMLDivElement>;
2324
inspectorAdapters: InspectorAdapters;
2425
searchSessionManager: DiscoverSearchSessionManager;
26+
savedSearchFetch$: DataFetch$;
2527
}
2628

2729
export const DiscoverHistogramLayout = ({
@@ -30,6 +32,7 @@ export const DiscoverHistogramLayout = ({
3032
resetSavedSearch,
3133
savedSearch,
3234
savedSearchData$,
35+
savedSearchFetch$,
3336
stateContainer,
3437
isTimeBased,
3538
resizeRef,
@@ -51,6 +54,7 @@ export const DiscoverHistogramLayout = ({
5154
isTimeBased,
5255
inspectorAdapters,
5356
searchSessionManager,
57+
savedSearchFetch$,
5458
...commonProps,
5559
});
5660

src/plugins/discover/public/application/main/components/layout/discover_layout.test.tsx

+5-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { dataViewWithTimefieldMock } from '../../../../__mocks__/data_view_with_
2424
import {
2525
AvailableFields$,
2626
DataDocuments$,
27+
DataFetch$,
2728
DataMain$,
2829
DataRefetch$,
2930
DataTotalHits$,
@@ -58,7 +59,9 @@ function mountComponent(
5859
[SIDEBAR_CLOSED_KEY]: prevSidebarClosed,
5960
}) as unknown as Storage,
6061
} as unknown as DiscoverServices;
61-
62+
services.data.query.timefilter.timefilter.getTime = () => {
63+
return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' };
64+
};
6265
(services.data.query.queryString.getDefaultQuery as jest.Mock).mockReturnValue({
6366
language: 'kuery',
6467
query: '',
@@ -113,6 +116,7 @@ function mountComponent(
113116
resetSavedSearch: jest.fn(),
114117
savedSearch: savedSearchMock,
115118
savedSearchData$,
119+
savedSearchFetch$: new Subject() as DataFetch$,
116120
savedSearchRefetch$: new Subject() as DataRefetch$,
117121
searchSource: searchSourceMock,
118122
state: { columns: [], query, hideChart: false, interval: 'auto' },

src/plugins/discover/public/application/main/components/layout/discover_layout.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export function DiscoverLayout({
6262
onChangeDataView,
6363
onUpdateQuery,
6464
setExpandedDoc,
65+
savedSearchFetch$,
6566
savedSearchRefetch$,
6667
resetSavedSearch,
6768
savedSearchData$,
@@ -237,6 +238,7 @@ export function DiscoverLayout({
237238
setExpandedDoc={setExpandedDoc}
238239
savedSearch={savedSearch}
239240
savedSearchData$={savedSearchData$}
241+
savedSearchFetch$={savedSearchFetch$}
240242
savedSearchRefetch$={savedSearchRefetch$}
241243
stateContainer={stateContainer}
242244
isTimeBased={isTimeBased}
@@ -270,6 +272,7 @@ export function DiscoverLayout({
270272
resultState,
271273
savedSearch,
272274
savedSearchData$,
275+
savedSearchFetch$,
273276
savedSearchRefetch$,
274277
searchSessionManager,
275278
setExpandedDoc,

src/plugins/discover/public/application/main/components/layout/types.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99
import type { Query, TimeRange, AggregateQuery } from '@kbn/es-query';
1010
import type { DataView } from '@kbn/data-views-plugin/public';
1111
import type { ISearchSource } from '@kbn/data-plugin/public';
12-
import { SavedSearch } from '@kbn/saved-search-plugin/public';
13-
import { DataTableRecord } from '../../../../types';
14-
import { DiscoverStateContainer } from '../../services/discover_state';
15-
import { DataRefetch$, SavedSearchData } from '../../hooks/use_saved_search';
12+
import type { SavedSearch } from '@kbn/saved-search-plugin/public';
13+
import type { DataTableRecord } from '../../../../types';
14+
import type { DiscoverStateContainer } from '../../services/discover_state';
15+
import type { DataFetch$, DataRefetch$, SavedSearchData } from '../../hooks/use_saved_search';
1616
import type { DiscoverSearchSessionManager } from '../../services/discover_search_session';
1717
import type { InspectorAdapters } from '../../hooks/use_inspector';
1818

@@ -29,6 +29,7 @@ export interface DiscoverLayoutProps {
2929
setExpandedDoc: (doc?: DataTableRecord) => void;
3030
savedSearch: SavedSearch;
3131
savedSearchData$: SavedSearchData;
32+
savedSearchFetch$: DataFetch$;
3233
savedSearchRefetch$: DataRefetch$;
3334
searchSource: ISearchSource;
3435
stateContainer: DiscoverStateContainer;

src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx

+103-7
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@ import React, { ReactElement } from 'react';
99
import { buildDataTableRecord } from '../../../../utils/build_data_record';
1010
import { esHits } from '../../../../__mocks__/es_hits';
1111
import { act, renderHook, WrapperComponent } from '@testing-library/react-hooks';
12-
import { BehaviorSubject } from 'rxjs';
12+
import { BehaviorSubject, Subject } from 'rxjs';
1313
import { FetchStatus } from '../../../types';
1414
import {
1515
AvailableFields$,
1616
DataDocuments$,
17+
DataFetch$,
1718
DataMain$,
1819
DataTotalHits$,
1920
RecordRawType,
@@ -72,6 +73,15 @@ jest.mock('@kbn/unified-field-list-plugin/public', () => {
7273
return {
7374
...originalModule,
7475
getVisualizeInformation: jest.fn(() => Promise.resolve(mockCanVisualize)),
76+
useQuerySubscriber: jest.fn(() => ({
77+
query: {
78+
query: 'query',
79+
language: 'kuery',
80+
},
81+
filters: [],
82+
fromDate: 'now-15m',
83+
toDate: 'now',
84+
})),
7585
};
7686
});
7787

@@ -120,6 +130,7 @@ describe('useDiscoverHistogram', () => {
120130
recordRawType: isPlainRecord ? RecordRawType.PLAIN : RecordRawType.DOCUMENT,
121131
foundDocuments: true,
122132
}) as DataMain$,
133+
savedSearchFetch$ = new Subject() as DataFetch$,
123134
}: {
124135
isPlainRecord?: boolean;
125136
isTimeBased?: boolean;
@@ -131,6 +142,7 @@ describe('useDiscoverHistogram', () => {
131142
inspectorAdapters?: InspectorAdapters;
132143
totalHits$?: DataTotalHits$;
133144
main$?: DataMain$;
145+
savedSearchFetch$?: DataFetch$;
134146
} = {}) => {
135147
mockStorage = storage;
136148
mockCanVisualize = canVisualize;
@@ -161,6 +173,7 @@ describe('useDiscoverHistogram', () => {
161173
const initialProps = {
162174
stateContainer,
163175
savedSearchData$,
176+
savedSearchFetch$,
164177
dataView: dataViewWithTimefieldMock,
165178
savedSearch: savedSearchMock,
166179
isTimeBased,
@@ -188,11 +201,20 @@ describe('useDiscoverHistogram', () => {
188201
return { hook, initialProps };
189202
};
190203

191-
it('should return undefined if there is no search session', async () => {
192-
const {
193-
hook: { result },
194-
} = await renderUseDiscoverHistogram({ searchSessionId: null });
195-
expect(result.current).toBeUndefined();
204+
describe('result', () => {
205+
it('should return undefined if there is no search session', async () => {
206+
const {
207+
hook: { result },
208+
} = await renderUseDiscoverHistogram({ searchSessionId: null });
209+
expect(result.current).toBeUndefined();
210+
});
211+
212+
it('it should not return undefined if there is no search session, but isPlainRecord is true', async () => {
213+
const {
214+
hook: { result },
215+
} = await renderUseDiscoverHistogram({ searchSessionId: null, isPlainRecord: true });
216+
expect(result.current).toBeDefined();
217+
});
196218
});
197219

198220
describe('contexts', () => {
@@ -263,6 +285,21 @@ describe('useDiscoverHistogram', () => {
263285
});
264286
});
265287

288+
describe('search params', () => {
289+
it('should return the correct query, filters, and timeRange', async () => {
290+
const { hook } = await renderUseDiscoverHistogram();
291+
expect(hook.result.current?.query).toEqual({
292+
query: 'query',
293+
language: 'kuery',
294+
});
295+
expect(hook.result.current?.filters).toEqual([]);
296+
expect(hook.result.current?.timeRange).toEqual({
297+
from: 'now-15m',
298+
to: 'now',
299+
});
300+
});
301+
});
302+
266303
describe('onEditVisualization', () => {
267304
it('returns a callback for onEditVisualization when the data view can be visualized', async () => {
268305
const {
@@ -364,7 +401,7 @@ describe('useDiscoverHistogram', () => {
364401
} = await renderUseDiscoverHistogram({ inspectorAdapters });
365402
expect(inspectorAdapters.lensRequests).toBeUndefined();
366403
act(() => {
367-
result.current?.onChartLoad({ complete: true, adapters: { requests: lensRequests } });
404+
result.current?.onChartLoad({ adapters: { requests: lensRequests } });
368405
});
369406
expect(inspectorAdapters.lensRequests).toBeDefined();
370407
});
@@ -486,4 +523,63 @@ describe('useDiscoverHistogram', () => {
486523
expect(mockCheckHitCount).not.toHaveBeenCalled();
487524
});
488525
});
526+
527+
describe('refetching', () => {
528+
it("should call input$.next({ type: 'refetch' }) when savedSearchFetch$ is triggered", async () => {
529+
const savedSearchFetch$ = new BehaviorSubject({ reset: false, searchSessionId: '1234' });
530+
const { hook } = await renderUseDiscoverHistogram({ savedSearchFetch$ });
531+
const onRefetch = jest.fn();
532+
hook.result.current?.input$.subscribe(onRefetch);
533+
act(() => {
534+
savedSearchFetch$.next({ reset: false, searchSessionId: '1234' });
535+
});
536+
expect(onRefetch).toHaveBeenCalledWith({ type: 'refetch' });
537+
});
538+
539+
it("should not call input$.next({ type: 'refetch' }) when searchSessionId is not set", async () => {
540+
const savedSearchFetch$ = new BehaviorSubject({ reset: false, searchSessionId: '1234' });
541+
const { hook } = await renderUseDiscoverHistogram({
542+
savedSearchFetch$,
543+
searchSessionId: null,
544+
});
545+
const onRefetch = jest.fn();
546+
hook.result.current?.input$.subscribe(onRefetch);
547+
act(() => {
548+
savedSearchFetch$.next({ reset: false, searchSessionId: '1234' });
549+
});
550+
expect(onRefetch).not.toHaveBeenCalled();
551+
});
552+
553+
it("should call input$.next({ type: 'refetch' }) when searchSessionId is not set and isPlainRecord is true", async () => {
554+
const savedSearchFetch$ = new BehaviorSubject({ reset: false, searchSessionId: '1234' });
555+
const { hook } = await renderUseDiscoverHistogram({
556+
savedSearchFetch$,
557+
searchSessionId: null,
558+
isPlainRecord: true,
559+
});
560+
const onRefetch = jest.fn();
561+
hook.result.current?.input$.subscribe(onRefetch);
562+
act(() => {
563+
savedSearchFetch$.next({ reset: false, searchSessionId: '1234' });
564+
});
565+
expect(onRefetch).toHaveBeenCalledWith({ type: 'refetch' });
566+
});
567+
568+
it('should skip the next refetch when state.hideChart changes from true to false', async () => {
569+
const savedSearchFetch$ = new BehaviorSubject({ reset: false, searchSessionId: '1234' });
570+
const { hook } = await renderUseDiscoverHistogram({ savedSearchFetch$ });
571+
const onRefetch = jest.fn();
572+
hook.result.current?.input$.subscribe(onRefetch);
573+
act(() => {
574+
hook.result.current?.onChartHiddenChange(true);
575+
});
576+
act(() => {
577+
hook.result.current?.onChartHiddenChange(false);
578+
});
579+
act(() => {
580+
savedSearchFetch$.next({ reset: false, searchSessionId: '1234' });
581+
});
582+
expect(onRefetch).not.toHaveBeenCalled();
583+
});
584+
});
489585
});

0 commit comments

Comments
 (0)