Skip to content

Commit

Permalink
[Unified Histogram] [Discover] Unified Histogram solutions cleanup (e…
Browse files Browse the repository at this point in the history
…lastic#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 <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and wayneseymour committed Jan 19, 2023
1 parent 159392b commit 78b4c9b
Show file tree
Hide file tree
Showing 34 changed files with 903 additions and 418 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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: '',
Expand Down Expand Up @@ -123,6 +125,7 @@ const mountComponent = ({
setExpandedDoc: jest.fn(),
savedSearch,
savedSearchData$,
savedSearchFetch$: new Subject(),
savedSearchRefetch$: new Subject(),
stateContainer,
onFieldEdited: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ 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;
isTimeBased: boolean;
resizeRef: RefObject<HTMLDivElement>;
inspectorAdapters: InspectorAdapters;
searchSessionManager: DiscoverSearchSessionManager;
savedSearchFetch$: DataFetch$;
}

export const DiscoverHistogramLayout = ({
Expand All @@ -30,6 +32,7 @@ export const DiscoverHistogramLayout = ({
resetSavedSearch,
savedSearch,
savedSearchData$,
savedSearchFetch$,
stateContainer,
isTimeBased,
resizeRef,
Expand All @@ -51,6 +54,7 @@ export const DiscoverHistogramLayout = ({
isTimeBased,
inspectorAdapters,
searchSessionManager,
savedSearchFetch$,
...commonProps,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { dataViewWithTimefieldMock } from '../../../../__mocks__/data_view_with_
import {
AvailableFields$,
DataDocuments$,
DataFetch$,
DataMain$,
DataRefetch$,
DataTotalHits$,
Expand Down Expand Up @@ -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: '',
Expand Down Expand Up @@ -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' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export function DiscoverLayout({
onChangeDataView,
onUpdateQuery,
setExpandedDoc,
savedSearchFetch$,
savedSearchRefetch$,
resetSavedSearch,
savedSearchData$,
Expand Down Expand Up @@ -237,6 +238,7 @@ export function DiscoverLayout({
setExpandedDoc={setExpandedDoc}
savedSearch={savedSearch}
savedSearchData$={savedSearchData$}
savedSearchFetch$={savedSearchFetch$}
savedSearchRefetch$={savedSearchRefetch$}
stateContainer={stateContainer}
isTimeBased={isTimeBased}
Expand Down Expand Up @@ -270,6 +272,7 @@ export function DiscoverLayout({
resultState,
savedSearch,
savedSearchData$,
savedSearchFetch$,
savedSearchRefetch$,
searchSessionManager,
setExpandedDoc,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -29,6 +29,7 @@ export interface DiscoverLayoutProps {
setExpandedDoc: (doc?: DataTableRecord) => void;
savedSearch: SavedSearch;
savedSearchData$: SavedSearchData;
savedSearchFetch$: DataFetch$;
savedSearchRefetch$: DataRefetch$;
searchSource: ISearchSource;
stateContainer: DiscoverStateContainer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
})),
};
});

Expand Down Expand Up @@ -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;
Expand All @@ -131,6 +142,7 @@ describe('useDiscoverHistogram', () => {
inspectorAdapters?: InspectorAdapters;
totalHits$?: DataTotalHits$;
main$?: DataMain$;
savedSearchFetch$?: DataFetch$;
} = {}) => {
mockStorage = storage;
mockCanVisualize = canVisualize;
Expand Down Expand Up @@ -161,6 +173,7 @@ describe('useDiscoverHistogram', () => {
const initialProps = {
stateContainer,
savedSearchData$,
savedSearchFetch$,
dataView: dataViewWithTimefieldMock,
savedSearch: savedSearchMock,
isTimeBased,
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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();
});
});
});
Loading

0 comments on commit 78b4c9b

Please sign in to comment.