From 5ab45e29551f122b4707967da1f65551eb2622bb Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 28 Oct 2022 01:19:26 +0200 Subject: [PATCH 01/36] Initial commit --- .../public/__mocks__/discover_state.mock.ts | 4 +- .../application/context/context_app.tsx | 3 +- .../field_stats_table/field_stats_table.tsx | 6 +- .../__stories__/discover_layout.stories.tsx | 5 +- .../layout/__stories__/get_layout_props.ts | 4 +- .../layout/discover_documents.test.tsx | 19 +++-- .../components/layout/discover_documents.tsx | 57 ++++++++------- .../layout/discover_layout.test.tsx | 41 ++++------- .../components/layout/discover_layout.tsx | 55 +++++++------- .../layout/discover_main_content.test.tsx | 23 +++--- .../layout/discover_main_content.tsx | 15 ++-- .../main/components/layout/types.ts | 9 +-- ...est.ts => use_discover_histogram.test.tsx} | 57 +++++++++------ .../layout/use_discover_histogram.ts | 20 +++--- .../sidebar/discover_field.test.tsx | 2 +- .../sidebar/discover_sidebar.test.tsx | 57 ++++++--------- .../discover_sidebar_responsive.test.tsx | 14 +--- .../sidebar/discover_sidebar_responsive.tsx | 6 +- .../top_nav/discover_topnav.test.tsx | 30 +++++--- .../components/top_nav/discover_topnav.tsx | 14 ++-- .../top_nav/get_top_nav_links.test.ts | 4 +- .../components/top_nav/get_top_nav_links.tsx | 8 +-- .../top_nav/on_save_search.test.tsx | 18 ++--- .../components/top_nav/on_save_search.tsx | 10 +-- .../main/discover_main_app.test.tsx | 8 ++- .../application/main/discover_main_app.tsx | 11 +-- .../application/main/discover_main_route.tsx | 10 ++- ....test.ts => use_adhoc_data_views.test.tsx} | 71 ++++++++++--------- .../main/hooks/use_adhoc_data_views.ts | 31 +++----- .../main/hooks/use_discover_state.test.tsx | 31 +++++--- .../main/hooks/use_discover_state.ts | 68 +++++++----------- ...arch.test.ts => use_saved_search.test.tsx} | 62 ++++++++++------ .../main/hooks/use_saved_search.ts | 10 +-- .../main/hooks/use_search_session.test.ts | 4 +- .../main/hooks/use_search_session.ts | 8 +-- .../use_test_based_query_language.test.ts | 12 ++-- .../hooks/use_text_based_query_language.ts | 6 +- .../discover_internal_state_container.ts | 60 ++++++++++++++++ .../main/services/discover_state.test.ts | 34 ++++----- .../main/services/discover_state.ts | 45 ++++++++++-- .../main/services/discover_state_provider.tsx | 37 ++++++++++ .../components/doc_table/actions/columns.ts | 2 +- .../hooks/use_data_grid_columns.test.tsx | 13 +--- .../public/hooks/use_data_grid_columns.ts | 37 ++++------ 44 files changed, 580 insertions(+), 461 deletions(-) rename src/plugins/discover/public/application/main/components/layout/{use_discover_histogram.test.ts => use_discover_histogram.test.tsx} (89%) rename src/plugins/discover/public/application/main/hooks/{use_adhoc_data_views.test.ts => use_adhoc_data_views.test.tsx} (68%) rename src/plugins/discover/public/application/main/hooks/{use_saved_search.test.ts => use_saved_search.test.tsx} (78%) create mode 100644 src/plugins/discover/public/application/main/services/discover_internal_state_container.ts create mode 100644 src/plugins/discover/public/application/main/services/discover_state_provider.tsx diff --git a/src/plugins/discover/public/__mocks__/discover_state.mock.ts b/src/plugins/discover/public/__mocks__/discover_state.mock.ts index 30c40ef77bfb9..7155dd33ac5c0 100644 --- a/src/plugins/discover/public/__mocks__/discover_state.mock.ts +++ b/src/plugins/discover/public/__mocks__/discover_state.mock.ts @@ -6,14 +6,14 @@ * Side Public License, v 1. */ import { createBrowserHistory } from 'history'; -import { getState } from '../application/main/services/discover_state'; +import { getDiscoverStateContainer } from '../application/main/services/discover_state'; import { savedSearchMockWithTimeField, savedSearchMock } from './saved_search'; import { discoverServiceMock } from './services'; export function getDiscoverStateMock({ isTimeBased = true }) { const history = createBrowserHistory(); history.push('/'); - return getState({ + return getDiscoverStateContainer({ savedSearch: isTimeBased ? savedSearchMockWithTimeField : savedSearchMock, services: discoverServiceMock, history, diff --git a/src/plugins/discover/public/application/context/context_app.tsx b/src/plugins/discover/public/application/context/context_app.tsx index 3146e634c4575..bbfca3b4aa977 100644 --- a/src/plugins/discover/public/application/context/context_app.tsx +++ b/src/plugins/discover/public/application/context/context_app.tsx @@ -115,9 +115,10 @@ export const ContextApp = ({ dataView, anchorId }: ContextAppProps) => { config: uiSettings, dataView, dataViews, - state: appState, useNewFieldsApi, setAppState, + columns: appState.columns, + sort: appState.sort, }); const rows = useMemo( () => [ diff --git a/src/plugins/discover/public/application/main/components/field_stats_table/field_stats_table.tsx b/src/plugins/discover/public/application/main/components/field_stats_table/field_stats_table.tsx index 742342123140f..090eae497de04 100644 --- a/src/plugins/discover/public/application/main/components/field_stats_table/field_stats_table.tsx +++ b/src/plugins/discover/public/application/main/components/field_stats_table/field_stats_table.tsx @@ -22,7 +22,7 @@ import { EuiFlexItem } from '@elastic/eui'; import { css } from '@emotion/react'; import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import { FIELD_STATISTICS_LOADED } from './constants'; -import type { GetStateReturn } from '../../services/discover_state'; +import type { DiscoverStateContainer } from '../../services/discover_state'; import { AvailableFields$, DataRefetch$, DataTotalHits$ } from '../../hooks/use_saved_search'; export interface DataVisualizerGridEmbeddableInput extends EmbeddableInput { @@ -76,7 +76,7 @@ export interface FieldStatisticsTableProps { /** * State container with persisted settings */ - stateContainer?: GetStateReturn; + stateContainer?: DiscoverStateContainer; /** * Callback to add a filter to filter bar */ @@ -118,7 +118,7 @@ export const FieldStatisticsTable = (props: FieldStatisticsTableProps) => { const showPreviewByDefault = useMemo( () => - stateContainer ? !stateContainer.appStateContainer.getState().hideAggregatedPreview : true, + stateContainer ? !stateContainer.appState.getState().hideAggregatedPreview : true, [stateContainer] ); diff --git a/src/plugins/discover/public/application/main/components/layout/__stories__/discover_layout.stories.tsx b/src/plugins/discover/public/application/main/components/layout/__stories__/discover_layout.stories.tsx index 771a3e6b1072d..6c15b0beaa8c5 100644 --- a/src/plugins/discover/public/application/main/components/layout/__stories__/discover_layout.stories.tsx +++ b/src/plugins/discover/public/application/main/components/layout/__stories__/discover_layout.stories.tsx @@ -20,7 +20,7 @@ import { DiscoverLayoutProps } from '../types'; setHeaderActionMenuMounter(() => void 0); const DiscoverLayoutStory = (layoutProps: DiscoverLayoutProps) => { - const [state, setState] = useState(layoutProps.state); + const [state, setState] = useState({}); const setAppState = (newState: Partial) => { setState((prevState) => ({ ...prevState, ...newState })); @@ -31,10 +31,9 @@ const DiscoverLayoutStory = (layoutProps: DiscoverLayoutProps) => { return ( diff --git a/src/plugins/discover/public/application/main/components/layout/__stories__/get_layout_props.ts b/src/plugins/discover/public/application/main/components/layout/__stories__/get_layout_props.ts index 56c0f349615e0..fcf69dfe226b7 100644 --- a/src/plugins/discover/public/application/main/components/layout/__stories__/get_layout_props.ts +++ b/src/plugins/discover/public/application/main/components/layout/__stories__/get_layout_props.ts @@ -25,7 +25,7 @@ import { buildDataTableRecordList } from '../../../../../utils/build_data_record import { esHits } from '../../../../../__mocks__/es_hits'; import { SavedSearch } from '../../../../..'; import { DiscoverLayoutProps } from '../types'; -import { GetStateReturn } from '../../../services/discover_state'; +import { DiscoverStateContainer } from '../../../services/discover_state'; const documentObservables = { main$: new BehaviorSubject({ @@ -112,7 +112,7 @@ const getCommonProps = (dataView: DataView) => { }), setState: action('Set app state'), }, - } as unknown as GetStateReturn, + } as unknown as DiscoverStateContainer, setExpandedDoc: action('opening an expanded doc'), }; }; diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx index dca425977b359..10aa40657b92f 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx @@ -12,7 +12,7 @@ import { mountWithIntl } from '@kbn/test-jest-helpers'; import { setHeaderActionMenuMounter } from '../../../../kibana_services'; import { esHits } from '../../../../__mocks__/es_hits'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; -import { AppState, GetStateReturn } from '../../services/discover_state'; +import { AppState, DiscoverStateContainer } from '../../services/discover_state'; import { DataDocuments$ } from '../../hooks/use_saved_search'; import { discoverServiceMock } from '../../../../__mocks__/services'; import { FetchStatus } from '../../../types'; @@ -21,6 +21,8 @@ import { dataViewMock } from '../../../../__mocks__/data_view'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { buildDataTableRecord } from '../../../../utils/build_data_record'; import { EsHitRecord } from '../../../../types'; +import { DiscoverMainProvider } from '../../services/discover_state_provider'; +import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; setHeaderActionMenuMounter(jest.fn()); @@ -34,6 +36,7 @@ function mountComponent(fetchStatus: FetchStatus, hits: EsHitRecord[]) { fetchStatus, result: hits.map((hit) => buildDataTableRecord(hit, dataViewMock)), }) as DataDocuments$; + const stateContainer = getDiscoverStateMock({}); const props = { expandedDoc: undefined, @@ -44,14 +47,16 @@ function mountComponent(fetchStatus: FetchStatus, hits: EsHitRecord[]) { searchSource: documents$, setExpandedDoc: jest.fn(), state: { columns: [] }, - stateContainer: { setAppState: () => {} } as unknown as GetStateReturn, + stateContainer, navigateTo: jest.fn(), onFieldEdited: jest.fn(), }; return mountWithIntl( - + + + ); } @@ -83,15 +88,17 @@ describe('Discover documents layout', () => { setAppState: (newState: Partial) => { state = { ...state, ...newState }; }, - } as unknown as GetStateReturn; + appState: { + getState: () => state, + }, + } as unknown as DiscoverStateContainer; onResize( { columnId: 'someField', width: 205.5435345534, }, - stateContainer, - state + stateContainer ); expect(state.grid?.columns?.someField.width).toEqual(206); diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx index 2aacf598e58c4..2ae807ad3ef05 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx @@ -16,6 +16,7 @@ import { import { FormattedMessage } from '@kbn/i18n-react'; import { DataView } from '@kbn/data-views-plugin/public'; import { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public'; +import { useAppStateSelector } from '../../services/discover_app_state_container'; import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import { DocViewFilterFn } from '../../../../services/doc_views/doc_views_types'; import { DiscoverGrid } from '../../../../components/discover_grid/discover_grid'; @@ -29,7 +30,7 @@ import { } from '../../../../../common'; import { useColumns } from '../../../../hooks/use_data_grid_columns'; import { DataDocuments$, RecordRawType } from '../../hooks/use_saved_search'; -import { AppState, GetStateReturn } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; import { useDataState } from '../../hooks/use_data_state'; import { DocTableInfinite } from '../../../../components/doc_table/doc_table_infinite'; import { DocumentExplorerCallout } from '../document_explorer_callout'; @@ -44,9 +45,9 @@ const DataGridMemoized = React.memo(DiscoverGrid); // export needs for testing export const onResize = ( colSettings: { columnId: string; width: number }, - stateContainer: GetStateReturn, - state: AppState + stateContainer: DiscoverStateContainer ) => { + const state = stateContainer.appState.getState(); const grid = { ...(state.grid || {}) }; const newColumns = { ...(grid.columns || {}) }; newColumns[colSettings.columnId] = { @@ -63,7 +64,6 @@ function DiscoverDocumentsComponent({ onAddFilter, savedSearch, setExpandedDoc, - state, stateContainer, onFieldEdited, }: { @@ -74,11 +74,14 @@ function DiscoverDocumentsComponent({ onAddFilter?: DocViewFilterFn; savedSearch: SavedSearch; setExpandedDoc: (doc?: DataTableRecord) => void; - state: AppState; - stateContainer: GetStateReturn; + stateContainer: DiscoverStateContainer; onFieldEdited?: () => void; }) { const { capabilities, dataViews, uiSettings } = useDiscoverServices(); + const [query, sort, rowHeight, rowsPerPage, grid, columns] = useAppStateSelector((state) => { + return [state.query, state.sort, state.rowHeight, state.rowsPerPage, state.grid, state.columns]; + }); + const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]); const hideAnnouncements = useMemo(() => uiSettings.get(HIDE_ANNOUNCEMENTS), [uiSettings]); const isLegacy = useMemo(() => uiSettings.get(DOC_TABLE_LEGACY), [uiSettings]); @@ -86,37 +89,41 @@ function DiscoverDocumentsComponent({ const documentState = useDataState(documents$); const isLoading = documentState.fetchStatus === FetchStatus.LOADING; - const isPlainRecord = useMemo( - () => getRawRecordType(state.query) === RecordRawType.PLAIN, - [state.query] - ); + const isPlainRecord = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]); const rows = useMemo(() => documentState.result || [], [documentState.result]); - const { columns, onAddColumn, onRemoveColumn, onMoveColumn, onSetColumns } = useColumns({ + const { + columns: currentColumns, + onAddColumn, + onRemoveColumn, + onMoveColumn, + onSetColumns, + } = useColumns({ capabilities, config: uiSettings, dataView, dataViews, setAppState: stateContainer.setAppState, - state, useNewFieldsApi, + columns, + sort, }); const onResizeDataGrid = useCallback( - (colSettings) => onResize(colSettings, stateContainer, state), - [stateContainer, state] + (colSettings) => onResize(colSettings, stateContainer), + [stateContainer] ); const onUpdateRowsPerPage = useCallback( - (rowsPerPage: number) => { - stateContainer.setAppState({ rowsPerPage }); + (nextRowsPerPage: number) => { + stateContainer.setAppState({ rowsPerPage: nextRowsPerPage }); }, [stateContainer] ); const onSort = useCallback( - (sort: string[][]) => { - stateContainer.setAppState({ sort }); + (nextSort: string[][]) => { + stateContainer.setAppState({ sort: nextSort }); }, [stateContainer] ); @@ -162,10 +169,10 @@ function DiscoverDocumentsComponent({ <> {!hideAnnouncements && } 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 0e9c7f8449520..46912b401e201 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 @@ -16,9 +16,8 @@ import { esHits } from '../../../../__mocks__/es_hits'; import { dataViewMock } from '../../../../__mocks__/data_view'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; import { createSearchSourceMock } from '@kbn/data-plugin/common/search/search_source/mocks'; -import type { DataView } from '@kbn/data-views-plugin/public'; +import type { DataView, DataViewListItem } from '@kbn/data-views-plugin/public'; import { dataViewWithTimefieldMock } from '../../../../__mocks__/data_view_with_timefield'; -import { GetStateReturn } from '../../services/discover_state'; import { DiscoverLayoutProps } from './types'; import { AvailableFields$, @@ -36,12 +35,12 @@ import { LocalStorageMock } from '../../../../__mocks__/local_storage_mock'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { DiscoverServices } from '../../../../build_services'; import { buildDataTableRecord } from '../../../../utils/build_data_record'; -import { DiscoverAppStateProvider } from '../../services/discover_app_state_container'; import type { UnifiedHistogramChartData } from '@kbn/unified-histogram-plugin/public'; import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; import type { SearchResponse } from '@elastic/elasticsearch/lib/api/types'; import { setTimeout } from 'timers/promises'; import { act } from 'react-dom/test-utils'; +import { DiscoverMainProvider } from '../../services/discover_state_provider'; jest.mock('@kbn/unified-histogram-plugin/public', () => { const originalModule = jest.requireActual('@kbn/unified-histogram-plugin/public'); @@ -98,15 +97,6 @@ jest.mock('@kbn/unified-histogram-plugin/public', () => { }; }); -function getAppStateContainer() { - const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appStateContainer; - appStateContainer.set({ - query: { query: '', language: 'lucene' }, - filters: [], - }); - return appStateContainer; -} - setHeaderActionMenuMounter(jest.fn()); async function mountComponent( @@ -127,7 +117,8 @@ async function mountComponent( return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; }; - const dataViewList = [dataView]; + const dataViewList = [dataView] as DataViewListItem[]; + const stateContainer = getDiscoverStateMock({ isTimeBased: true }); const main$ = new BehaviorSubject({ fetchStatus: FetchStatus.COMPLETE, @@ -163,9 +154,12 @@ async function mountComponent( availableFields$, }; - const props = { + stateContainer.setAppState({ interval: 'auto', query }); + stateContainer.internalState.transitions.setDataView(dataView); + stateContainer.internalState.transitions.setDataViews(dataViewList as DataViewListItem[]); + + const props: DiscoverLayoutProps = { dataView, - dataViewList, inspectorAdapters: { requests: new RequestAdapter() }, navigateTo: jest.fn(), onChangeDataView: jest.fn(), @@ -175,26 +169,17 @@ async function mountComponent( savedSearchData$, savedSearchRefetch$: new Subject(), searchSource: searchSourceMock, - state: { columns: [], query }, - stateContainer: { - setAppState: () => {}, - appStateContainer: { - getState: () => ({ - interval: 'auto', - }), - }, - } as unknown as GetStateReturn, + stateContainer, setExpandedDoc: jest.fn(), persistDataView: jest.fn(), updateAdHocDataViewId: jest.fn(), - adHocDataViewList: [], }; const component = mountWithIntl( - - - + + + , mountOptions ); 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 a7cde67d4869b..a34bddc81b566 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 @@ -23,6 +23,8 @@ import { isOfQueryType } from '@kbn/es-query'; import classNames from 'classnames'; import { generateFilters } from '@kbn/data-plugin/public'; import { DataView, DataViewField, DataViewType } from '@kbn/data-views-plugin/public'; +import { useInternalStateSelector } from '../../services/discover_internal_state_container'; +import { useAppStateSelector } from '../../services/discover_app_state_container'; import { useInspector } from '../../hooks/use_inspector'; import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import { DiscoverNoResults } from '../no_results'; @@ -54,8 +56,6 @@ const SidebarMemoized = React.memo(DiscoverSidebarResponsive); const TopNavMemoized = React.memo(DiscoverTopNav); export function DiscoverLayout({ - dataView, - dataViewList, inspectorAdapters, expandedDoc, navigateTo, @@ -67,11 +67,9 @@ export function DiscoverLayout({ savedSearchData$, savedSearch, searchSource, - state, stateContainer, persistDataView, updateAdHocDataViewId, - adHocDataViewList, }: DiscoverLayoutProps) { const { trackUiMetric, @@ -86,12 +84,20 @@ export function DiscoverLayout({ inspector, } = useDiscoverServices(); const { main$ } = savedSearchData$; + const [viewMode, query, savedQuery, filters, columns, sort] = useAppStateSelector((state) => [ + state.viewMode, + state.query, + state.savedQuery, + state.filters, + state.columns, + state.sort, + ]); + const dataView = useInternalStateSelector((state) => state.dataView!); const dataState: DataMainMsg = useDataState(main$); - - const viewMode = useMemo(() => { + const currentViewMode = useMemo(() => { if (uiSettings.get(SHOW_FIELD_STATISTICS) !== true) return VIEW_MODE.DOCUMENT_LEVEL; - return state.viewMode ?? VIEW_MODE.DOCUMENT_LEVEL; - }, [uiSettings, state.viewMode]); + return viewMode ?? VIEW_MODE.DOCUMENT_LEVEL; + }, [uiSettings, viewMode]); const fetchCounter = useRef(0); @@ -113,10 +119,7 @@ export function DiscoverLayout({ const [isSidebarClosed, setIsSidebarClosed] = useState(initialSidebarClosed); const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]); - const isPlainRecord = useMemo( - () => getRawRecordType(state.query) === RecordRawType.PLAIN, - [state.query] - ); + const isPlainRecord = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]); const resultState = useMemo( () => getResultState(dataState.fetchStatus, dataState.foundDocuments!, isPlainRecord), [dataState.fetchStatus, dataState.foundDocuments, isPlainRecord] @@ -129,14 +132,19 @@ export function DiscoverLayout({ savedSearch, }); - const { columns, onAddColumn, onRemoveColumn } = useColumns({ + const { + columns: currentColumns, + onAddColumn, + onRemoveColumn, + } = useColumns({ capabilities, config: uiSettings, dataView, dataViews, setAppState: stateContainer.setAppState, - state, useNewFieldsApi, + columns, + sort, }); const onAddFilter = useCallback( @@ -218,9 +226,9 @@ export function DiscoverLayout({ @@ -296,8 +302,8 @@ export function DiscoverLayout({ isTimeBased={isTimeBased} data={data} error={dataState.error} - hasQuery={isOfQueryType(state.query) && !!state.query?.query} - hasFilters={hasActiveFilter(state.filters)} + hasQuery={isOfQueryType(query) && !!query?.query} + hasFilters={hasActiveFilter(filters)} onDisableFilters={onDisableFilters} /> )} @@ -316,13 +322,12 @@ export function DiscoverLayout({ savedSearch={savedSearch} savedSearchData$={savedSearchData$} savedSearchRefetch$={savedSearchRefetch$} - state={state} stateContainer={stateContainer} isTimeBased={isTimeBased} - viewMode={viewMode} + viewMode={currentViewMode} onAddFilter={onAddFilter as DocViewFilterFn} onFieldEdited={onFieldEdited} - columns={columns} + columns={currentColumns} resizeRef={resizeRef} /> )} diff --git a/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx index 54e3fa0b19ca5..d3ae7c9728c94 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_main_content.test.tsx @@ -12,7 +12,6 @@ import { findTestSubject, mountWithIntl } from '@kbn/test-jest-helpers'; import { esHits } from '../../../../__mocks__/es_hits'; import { dataViewMock } from '../../../../__mocks__/data_view'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; -import { GetStateReturn } from '../../services/discover_state'; import { AvailableFields$, DataCharts$, @@ -33,12 +32,14 @@ import { setTimeout } from 'timers/promises'; import { DocumentViewModeToggle } from '../../../../components/view_mode_toggle'; import { Storage } from '@kbn/kibana-utils-plugin/public'; import { LocalStorageMock } from '../../../../__mocks__/local_storage_mock'; +import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; import { UnifiedHistogramChartData, UnifiedHistogramLayout, } from '@kbn/unified-histogram-plugin/public'; import { HISTOGRAM_HEIGHT_KEY } from './use_discover_histogram'; import type { SearchResponse } from '@elastic/elasticsearch/lib/api/types'; +import { DiscoverMainProvider } from '../../services/discover_state_provider'; jest.mock('@kbn/unified-histogram-plugin/public', () => { const originalModule = jest.requireActual('@kbn/unified-histogram-plugin/public'); @@ -152,6 +153,12 @@ const mountComponent = async ({ charts$, availableFields$, }; + const stateContainer = getDiscoverStateMock({ isTimeBased }); + stateContainer.setAppState({ + interval: 'auto', + hideChart, + columns: [], + }); const props: DiscoverMainContentProps = { isPlainRecord, @@ -162,15 +169,7 @@ const mountComponent = async ({ savedSearch, savedSearchData$, savedSearchRefetch$: new Subject(), - state: { columns: [], hideChart }, - stateContainer: { - setAppState: () => {}, - appStateContainer: { - getState: () => ({ - interval: 'auto', - }), - }, - } as unknown as GetStateReturn, + stateContainer, isTimeBased, viewMode: VIEW_MODE.DOCUMENT_LEVEL, onAddFilter: jest.fn(), @@ -184,7 +183,9 @@ const mountComponent = async ({ const component = mountWithIntl( - + + + ); diff --git a/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx b/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx index 100412c8f7930..e5ce9bacfa5d1 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx @@ -19,11 +19,12 @@ import { DataTableRecord } from '../../../../types'; import { DocumentViewModeToggle, VIEW_MODE } from '../../../../components/view_mode_toggle'; import { DocViewFilterFn } from '../../../../services/doc_views/doc_views_types'; import { DataRefetch$, SavedSearchData } from '../../hooks/use_saved_search'; -import { AppState, GetStateReturn } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; import { FieldStatisticsTable } from '../field_stats_table'; import { DiscoverDocuments } from './discover_documents'; import { DOCUMENTS_VIEW_CLICK, FIELD_STATISTICS_VIEW_CLICK } from '../field_stats_table/constants'; import { useDiscoverHistogram } from './use_discover_histogram'; +import { useAppStateSelector } from '../../services/discover_app_state_container'; const FieldStatisticsTableMemoized = React.memo(FieldStatisticsTable); @@ -37,8 +38,7 @@ export interface DiscoverMainContentProps { savedSearch: SavedSearch; savedSearchData$: SavedSearchData; savedSearchRefetch$: DataRefetch$; - state: AppState; - stateContainer: GetStateReturn; + stateContainer: DiscoverStateContainer; isTimeBased: boolean; viewMode: VIEW_MODE; onAddFilter: DocViewFilterFn | undefined; @@ -57,7 +57,6 @@ export const DiscoverMainContent = ({ savedSearch, savedSearchData$, savedSearchRefetch$, - state, stateContainer, isTimeBased, viewMode, @@ -68,6 +67,8 @@ export const DiscoverMainContent = ({ }: DiscoverMainContentProps) => { const services = useDiscoverServices(); const { trackUiMetric } = services; + const query = useAppStateSelector((state) => state.query); + const filters = useAppStateSelector((state) => state.filters); const setDiscoverViewMode = useCallback( (mode: VIEW_MODE) => { @@ -94,7 +95,6 @@ export const DiscoverMainContent = ({ onTimeIntervalChange, } = useDiscoverHistogram({ stateContainer, - state, savedSearchData$, dataView, savedSearch, @@ -156,7 +156,6 @@ export const DiscoverMainContent = ({ onAddFilter={!isPlainRecord ? onAddFilter : undefined} savedSearch={savedSearch} setExpandedDoc={setExpandedDoc} - state={state} stateContainer={stateContainer} onFieldEdited={!isPlainRecord ? onFieldEdited : undefined} /> @@ -165,8 +164,8 @@ export const DiscoverMainContent = ({ availableFields$={savedSearchData$.availableFields$} savedSearch={savedSearch} dataView={dataView} - query={state.query} - filters={state.filters} + query={query} + filters={filters} columns={columns} stateContainer={stateContainer} onAddFilter={!isPlainRecord ? onAddFilter : undefined} 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 dfaf3e738aa53..ff0491c10dbfb 100644 --- a/src/plugins/discover/public/application/main/components/layout/types.ts +++ b/src/plugins/discover/public/application/main/components/layout/types.ts @@ -8,16 +8,15 @@ import type { Query, TimeRange, AggregateQuery } from '@kbn/es-query'; import type { DataView } from '@kbn/data-views-plugin/public'; -import { DataViewListItem, ISearchSource } from '@kbn/data-plugin/public'; +import { ISearchSource } from '@kbn/data-plugin/public'; import { RequestAdapter } from '@kbn/inspector-plugin/common'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; import { DataTableRecord } from '../../../../types'; -import { AppState, GetStateReturn } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; import { DataRefetch$, SavedSearchData } from '../../hooks/use_saved_search'; export interface DiscoverLayoutProps { dataView: DataView; - dataViewList: DataViewListItem[]; inspectorAdapters: { requests: RequestAdapter }; navigateTo: (url: string) => void; onChangeDataView: (id: string) => void; @@ -32,9 +31,7 @@ export interface DiscoverLayoutProps { savedSearchData$: SavedSearchData; savedSearchRefetch$: DataRefetch$; searchSource: ISearchSource; - state: AppState; - stateContainer: GetStateReturn; + stateContainer: DiscoverStateContainer; persistDataView: (dataView: DataView) => Promise; updateAdHocDataViewId: (dataView: DataView) => Promise; - adHocDataViewList: DataView[]; } diff --git a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.ts b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx similarity index 89% rename from src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.ts rename to src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx index 12fde6a5b1061..ca0631cb4d6b9 100644 --- a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.ts +++ b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx @@ -5,7 +5,7 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ - +import React from 'react'; import type { SearchResponse } from '@elastic/elasticsearch/lib/api/types'; import { buildDataTableRecord } from '../../../../utils/build_data_record'; import { esHits } from '../../../../__mocks__/es_hits'; @@ -20,7 +20,7 @@ import { DataTotalHits$, RecordRawType, } from '../../hooks/use_saved_search'; -import type { GetStateReturn } from '../../services/discover_state'; +import type { DiscoverStateContainer } from '../../services/discover_state'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; import type { Storage } from '@kbn/kibana-utils-plugin/public'; import { LocalStorageMock } from '../../../../__mocks__/local_storage_mock'; @@ -33,6 +33,8 @@ import { } from './use_discover_histogram'; import { setTimeout } from 'timers/promises'; import { calculateBounds } from '@kbn/data-plugin/public'; +import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; +import { DiscoverMainProvider } from '../../services/discover_state_provider'; const mockData = dataPluginMock.createStartContract(); @@ -62,19 +64,30 @@ jest.mock('@kbn/unified-field-list-plugin/public', () => { }; }); +function getStateContainer() { + const stateContainer = getDiscoverStateMock({ isTimeBased: true }); + stateContainer.setAppState({ + interval: 'auto', + hideChart: false, + }); + stateContainer.setAppState = jest.fn(); + + return stateContainer; +} + describe('useDiscoverHistogram', () => { const renderUseDiscoverHistogram = async ({ isPlainRecord = false, isTimeBased = true, canVisualize = true, storage = new LocalStorageMock({}) as unknown as Storage, - stateContainer = {}, + stateContainer = getStateContainer(), }: { isPlainRecord?: boolean; isTimeBased?: boolean; canVisualize?: boolean; storage?: Storage; - stateContainer?: unknown; + stateContainer?: DiscoverStateContainer; } = {}) => { mockStorage = storage; mockCanVisualize = canVisualize; @@ -158,17 +171,23 @@ describe('useDiscoverHistogram', () => { availableFields$, }; - const hook = renderHook(() => { - return useDiscoverHistogram({ - stateContainer: stateContainer as GetStateReturn, - state: { interval: 'auto', hideChart: false }, - savedSearchData$, - dataView: dataViewWithTimefieldMock, - savedSearch: savedSearchMock, - isTimeBased, - isPlainRecord, - }); - }); + const hook = renderHook( + () => { + return useDiscoverHistogram({ + stateContainer, + savedSearchData$, + dataView: dataViewWithTimefieldMock, + savedSearch: savedSearchMock, + isTimeBased, + isPlainRecord, + }); + }, + { + wrapper: ({ children }: { children: React.ReactElement }) => ( + {children} + ), + } + ); await act(() => setTimeout(0)); @@ -267,9 +286,7 @@ describe('useDiscoverHistogram', () => { it('should update chartHidden when onChartHiddenChange is called', async () => { const storage = new LocalStorageMock({}) as unknown as Storage; storage.set = jest.fn(); - const stateContainer = { - setAppState: jest.fn(), - }; + const stateContainer = getStateContainer(); const { result } = await renderUseDiscoverHistogram({ storage, stateContainer, @@ -282,9 +299,7 @@ describe('useDiscoverHistogram', () => { }); it('should update interval when onTimeIntervalChange is called', async () => { - const stateContainer = { - setAppState: jest.fn(), - }; + const stateContainer = getStateContainer(); const { result } = await renderUseDiscoverHistogram({ stateContainer, }); 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 3032fa13af043..f318a839f9c05 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 @@ -14,27 +14,26 @@ import { } from '@kbn/unified-field-list-plugin/public'; import { buildChartData } from '@kbn/unified-histogram-plugin/public'; import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useAppStateSelector } from '../../services/discover_app_state_container'; import { getUiActions } from '../../../../kibana_services'; import { PLUGIN_ID } from '../../../../../common'; import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import { useDataState } from '../../hooks/use_data_state'; import type { SavedSearchData } from '../../hooks/use_saved_search'; -import type { AppState, GetStateReturn } from '../../services/discover_state'; +import type { DiscoverStateContainer } from '../../services/discover_state'; export const CHART_HIDDEN_KEY = 'discover:chartHidden'; export const HISTOGRAM_HEIGHT_KEY = 'discover:histogramHeight'; export const useDiscoverHistogram = ({ stateContainer, - state, savedSearchData$, dataView, savedSearch, isTimeBased, isPlainRecord, }: { - stateContainer: GetStateReturn; - state: AppState; + stateContainer: DiscoverStateContainer; savedSearchData$: SavedSearchData; dataView: DataView; savedSearch: SavedSearch; @@ -42,6 +41,7 @@ export const useDiscoverHistogram = ({ isPlainRecord: boolean; }) => { const { storage, data } = useDiscoverServices(); + const [hideChart, interval] = useAppStateSelector((state) => [state.hideChart, state.interval]); /** * Visualize @@ -140,10 +140,10 @@ export const useDiscoverHistogram = ({ buildChartData({ data, dataView, - timeInterval: state.interval, + timeInterval: interval, response, }), - [data, dataView, response, state.interval] + [data, dataView, response, interval] ); const chart = useMemo( @@ -152,8 +152,8 @@ export const useDiscoverHistogram = ({ ? undefined : { status: chartFetchStatus, - hidden: state.hideChart, - timeInterval: state.interval, + hidden: hideChart, + timeInterval: interval, bucketInterval, data: chartData, error, @@ -165,8 +165,8 @@ export const useDiscoverHistogram = ({ error, isPlainRecord, isTimeBased, - state.hideChart, - state.interval, + hideChart, + interval, ] ); diff --git a/src/plugins/discover/public/application/main/components/sidebar/discover_field.test.tsx b/src/plugins/discover/public/application/main/components/sidebar/discover_field.test.tsx index dd27369a74a04..191349d6fc578 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/discover_field.test.tsx +++ b/src/plugins/discover/public/application/main/components/sidebar/discover_field.test.tsx @@ -133,7 +133,7 @@ async function getComponent({ fieldFormats: fieldFormatsServiceMock.createStartContract(), charts: chartPluginMock.createSetupContract(), }; - const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appStateContainer; + const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appState; appStateContainer.set({ query: { query: '', language: 'lucene' }, filters: [], diff --git a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.test.tsx b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.test.tsx index 0e7e3b22ac3b3..1e6a4527549ff 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.test.tsx +++ b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.test.tsx @@ -12,8 +12,6 @@ import { getDataTableRecords } from '../../../../__fixtures__/real_hits'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import React from 'react'; import { DiscoverSidebarProps } from './discover_sidebar'; -import { DataViewListItem } from '@kbn/data-views-plugin/public'; - import { getDefaultFieldFilter } from './lib/field_filter'; import { DiscoverSidebarComponent as DiscoverSidebar } from './discover_sidebar'; import { discoverServiceMock as mockDiscoverServices } from '../../../../__mocks__/services'; @@ -24,7 +22,7 @@ import { BehaviorSubject } from 'rxjs'; import { FetchStatus } from '../../../types'; import { AvailableFields$ } from '../../hooks/use_saved_search'; import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; -import { DiscoverAppStateProvider } from '../../services/discover_app_state_container'; +import { DiscoverMainProvider } from '../../services/discover_state_provider'; const mockGetActions = jest.fn>>, [string, { fieldName: string }]>( () => Promise.resolve([]) @@ -36,17 +34,21 @@ jest.mock('../../../../kibana_services', () => ({ }), })); +function getStateContainer() { + const state = getDiscoverStateMock({ isTimeBased: true }); + state.appState.set({ + query: { query: '', language: 'lucene' }, + filters: [], + }); + state.internalState.transitions.setDataView(stubLogstashDataView); + return state; +} + function getCompProps(): DiscoverSidebarProps { const dataView = stubLogstashDataView; dataView.toSpec = jest.fn(() => ({})); const hits = getDataTableRecords(dataView); - const dataViewList = [ - { id: '0', title: 'b' } as DataViewListItem, - { id: '1', title: 'a' } as DataViewListItem, - { id: '2', title: 'c' } as DataViewListItem, - ]; - const fieldCounts: Record = {}; for (const hit of hits) { @@ -63,7 +65,6 @@ function getCompProps(): DiscoverSidebarProps { columns: ['extension'], fieldCounts, documents: hits, - dataViewList, onChangeDataView: jest.fn(), onAddFilter: jest.fn(), onAddField: jest.fn(), @@ -82,34 +83,18 @@ function getCompProps(): DiscoverSidebarProps { }; } -function getAppStateContainer() { - const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appStateContainer; - appStateContainer.set({ - query: { query: '', language: 'lucene' }, - filters: [], - }); - return appStateContainer; -} - describe('discover sidebar', function () { let props: DiscoverSidebarProps; let comp: ReactWrapper; beforeAll(async () => { props = getCompProps(); - mockDiscoverServices.data.dataViews.getIdsWithTitle = jest - .fn() - .mockReturnValue(props.dataViewList); - mockDiscoverServices.data.dataViews.get = jest.fn().mockImplementation((id) => { - const dataView = props.dataViewList.find((d) => d.id === id); - return { ...dataView, isPersisted: () => true }; - }); comp = await mountWithIntl( - + - + ); // wait for lazy modules @@ -154,9 +139,9 @@ describe('discover sidebar', function () { it('should not render Add/Edit field buttons in viewer mode', () => { const compInViewerMode = mountWithIntl( - + - + ); const addFieldButton = findTestSubject(compInViewerMode, 'dataView-add-field_btn'); @@ -169,9 +154,9 @@ describe('discover sidebar', function () { it('should render buttons in data view picker correctly', async () => { const compWithPicker = mountWithIntl( - + - + ); // open data view picker @@ -195,14 +180,14 @@ describe('discover sidebar', function () { it('should not render buttons in data view picker when in viewer mode', async () => { const compWithPickerInViewerMode = mountWithIntl( - + - + ); // open data view picker @@ -221,9 +206,9 @@ describe('discover sidebar', function () { it('should render the Visualize in Lens button in text based languages mode', () => { const compInViewerMode = mountWithIntl( - + - + ); const visualizeField = findTestSubject(compInViewerMode, 'textBased-visualize'); diff --git a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx index 40cab06039f6e..0025c8cd18c7a 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx +++ b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.test.tsx @@ -13,7 +13,6 @@ import { getDataTableRecords } from '../../../../__fixtures__/real_hits'; import { act } from 'react-dom/test-utils'; import { mountWithIntl } from '@kbn/test-jest-helpers'; import React from 'react'; -import { DataViewListItem } from '@kbn/data-views-plugin/public'; import { DiscoverSidebarResponsive, DiscoverSidebarResponsiveProps, @@ -140,12 +139,6 @@ function getCompProps(): DiscoverSidebarResponsiveProps { const hits = getDataTableRecords(dataView); - const dataViewList = [ - { id: '0', title: 'b' } as DataViewListItem, - { id: '1', title: 'a' } as DataViewListItem, - { id: '2', title: 'c' } as DataViewListItem, - ]; - for (const hit of hits) { for (const key of Object.keys(hit.flattened)) { mockfieldCounts[key] = (mockfieldCounts[key] || 0) + 1; @@ -162,7 +155,6 @@ function getCompProps(): DiscoverSidebarResponsiveProps { fetchStatus: FetchStatus.COMPLETE, fields: [] as string[], }) as AvailableFields$, - dataViewList, onChangeDataView: jest.fn(), onAddFilter: jest.fn(), onAddField: jest.fn(), @@ -183,7 +175,7 @@ describe('discover responsive sidebar', function () { beforeAll(async () => { props = getCompProps(); await act(async () => { - const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appStateContainer; + const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appState; appStateContainer.set({ query: { query: '', language: 'lucene' }, filters: [], @@ -269,7 +261,7 @@ describe('discover responsive sidebar', function () { result: getDataTableRecords(stubLogstashDataView), }) as DataDocuments$, }; - const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appStateContainer; + const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appState; appStateContainer.set({ query: { sql: 'SELECT * FROM `index`' }, }); @@ -294,7 +286,7 @@ describe('discover responsive sidebar', function () { }, }, }; - const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appStateContainer; + const appStateContainer = getDiscoverStateMock({ isTimeBased: true }).appState; appStateContainer.set({ query: { query: '', language: 'lucene' }, filters: [], diff --git a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx index 9e8ad96f9abb6..1dfebaa7659d1 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx +++ b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx @@ -22,7 +22,7 @@ import { EuiShowFor, EuiTitle, } from '@elastic/eui'; -import type { DataView, DataViewField, DataViewListItem } from '@kbn/data-views-plugin/public'; +import type { DataView, DataViewField } from '@kbn/data-views-plugin/public'; import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import { getDefaultFieldFilter } from './lib/field_filter'; import { DiscoverSidebar } from './discover_sidebar'; @@ -47,10 +47,6 @@ export interface DiscoverSidebarResponsiveProps { * hits fetched from ES, displayed in the doc table */ documents$: DataDocuments$; - /** - * List of available data views - */ - dataViewList: DataViewListItem[]; /** * Has been toggled closed */ diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx index 13e3725177959..32b03a94dc5bf 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx @@ -7,16 +7,17 @@ */ import React from 'react'; -import { shallowWithIntl } from '@kbn/test-jest-helpers'; +import { mountWithIntl } from '@kbn/test-jest-helpers'; import { dataViewMock } from '../../../../__mocks__/data_view'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; import { DiscoverTopNav, DiscoverTopNavProps } from './discover_topnav'; -import { TopNavMenuData } from '@kbn/navigation-plugin/public'; +import { TopNavMenu, TopNavMenuData } from '@kbn/navigation-plugin/public'; import { ISearchSource } from '@kbn/data-plugin/public'; import { Query } from '@kbn/es-query'; -import { GetStateReturn } from '../../services/discover_state'; import { setHeaderActionMenuMounter } from '../../../../kibana_services'; import { discoverServiceMock } from '../../../../__mocks__/services'; +import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; +import { DiscoverMainProvider } from '../../services/discover_state_provider'; setHeaderActionMenuMounter(jest.fn()); @@ -29,9 +30,11 @@ jest.mock('@kbn/kibana-react-plugin/public', () => ({ function getProps(savePermissions = true): DiscoverTopNavProps { discoverServiceMock.capabilities.discover!.save = savePermissions; + const stateContainer = getDiscoverStateMock({ isTimeBased: true }); + stateContainer.internalState.transitions.setDataView(dataViewMock); return { - stateContainer: {} as GetStateReturn, + stateContainer, dataView: dataViewMock, savedSearch: savedSearchMock, navigateTo: jest.fn(), @@ -46,22 +49,31 @@ function getProps(savePermissions = true): DiscoverTopNavProps { isPlainRecord: false, persistDataView: jest.fn(), updateAdHocDataViewId: jest.fn(), - adHocDataViewList: [], }; } describe('Discover topnav component', () => { test('generated config of TopNavMenu config is correct when discover save permissions are assigned', () => { const props = getProps(true); - const component = shallowWithIntl(); - const topMenuConfig = component.props().config.map((obj: TopNavMenuData) => obj.id); + const component = mountWithIntl( + + + + ); + const topNavMenu = component.find(TopNavMenu); + const topMenuConfig = topNavMenu.props().config?.map((obj: TopNavMenuData) => obj.id); expect(topMenuConfig).toEqual(['options', 'new', 'open', 'share', 'inspect', 'save']); }); test('generated config of TopNavMenu config is correct when no discover save permissions are assigned', () => { const props = getProps(false); - const component = shallowWithIntl(); - const topMenuConfig = component.props().config.map((obj: TopNavMenuData) => obj.id); + const component = mountWithIntl( + + + + ); + const topNavMenu = component.find(TopNavMenu).props(); + const topMenuConfig = topNavMenu.config?.map((obj: TopNavMenuData) => obj.id); expect(topMenuConfig).toEqual(['options', 'new', 'open', 'share', 'inspect']); }); }); diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 98e7da1ddfa5f..9f8643a572ff9 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -10,12 +10,13 @@ import { useHistory } from 'react-router-dom'; import type { Query, TimeRange, AggregateQuery } from '@kbn/es-query'; import { DataViewType, type DataView } from '@kbn/data-views-plugin/public'; import type { DataViewPickerProps } from '@kbn/unified-search-plugin/public'; +import { useInternalStateSelector } from '../../services/discover_internal_state_container'; import { ENABLE_SQL } from '../../../../../common'; import { useDiscoverServices } from '../../../../hooks/use_discover_services'; import { DiscoverLayoutProps } from '../layout/types'; import { getTopNavLinks } from './get_top_nav_links'; import { getHeaderActionMenuMounter } from '../../../../kibana_services'; -import { GetStateReturn } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; import { onSaveSearch } from './on_save_search'; export type DiscoverTopNavProps = Pick< @@ -29,7 +30,7 @@ export type DiscoverTopNavProps = Pick< payload: { dateRange: TimeRange; query?: Query | AggregateQuery }, isUpdate?: boolean ) => void; - stateContainer: GetStateReturn; + stateContainer: DiscoverStateContainer; resetSavedSearch: () => void; onChangeDataView: (dataView: string) => void; isPlainRecord: boolean; @@ -37,7 +38,6 @@ export type DiscoverTopNavProps = Pick< onFieldEdited: () => Promise; persistDataView: (dataView: DataView) => Promise; updateAdHocDataViewId: (dataView: DataView) => Promise; - adHocDataViewList: DataView[]; }; export const DiscoverTopNav = ({ @@ -57,9 +57,9 @@ export const DiscoverTopNav = ({ onFieldEdited, persistDataView, updateAdHocDataViewId, - adHocDataViewList, }: DiscoverTopNavProps) => { const history = useHistory(); + const adHocDataViewList = useInternalStateSelector((state) => state.dataViewsAdHoc); const showDatePicker = useMemo( () => dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP, @@ -180,16 +180,16 @@ export const DiscoverTopNav = ({ ); const updateSavedQueryId = (newSavedQueryId: string | undefined) => { - const { appStateContainer, setAppState } = stateContainer; + const { appState, setAppState } = stateContainer; if (newSavedQueryId) { setAppState({ savedQuery: newSavedQueryId }); } else { // remove savedQueryId from state const newState = { - ...appStateContainer.getState(), + ...appState.getState(), }; delete newState.savedQuery; - appStateContainer.set(newState); + appState.set(newState); } }; const setMenuMountPoint = useMemo(() => { diff --git a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.test.ts b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.test.ts index af9c868f0ce78..f00ef96249b87 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.test.ts +++ b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.test.ts @@ -11,7 +11,7 @@ import { getTopNavLinks } from './get_top_nav_links'; import { dataViewMock } from '../../../../__mocks__/data_view'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; import { DiscoverServices } from '../../../../build_services'; -import { GetStateReturn } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; const services = { capabilities: { @@ -24,7 +24,7 @@ const services = { }, } as unknown as DiscoverServices; -const state = {} as unknown as GetStateReturn; +const state = {} as unknown as DiscoverStateContainer; test('getTopNavLinks result', () => { const topNavLinks = getTopNavLinks({ diff --git a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx index 7228049287f81..dc66022920189 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx @@ -16,7 +16,7 @@ import { showOpenSearchPanel } from './show_open_search_panel'; import { getSharingData, showPublicUrlSwitch } from '../../../../utils/get_sharing_data'; import { DiscoverServices } from '../../../../build_services'; import { onSaveSearch } from './on_save_search'; -import { GetStateReturn } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; import { openOptionsPopover } from './open_options_popover'; import { openAlertsPopover } from './open_alerts_popover'; @@ -40,7 +40,7 @@ export const getTopNavLinks = ({ navigateTo: (url: string) => void; savedSearch: SavedSearch; services: DiscoverServices; - state: GetStateReturn; + state: DiscoverStateContainer; onOpenInspector: () => void; searchSource: ISearchSource; onOpenSavedSearch: (id: string) => void; @@ -82,7 +82,7 @@ export const getTopNavLinks = ({ anchorElement, searchSource: savedSearch.searchSource, services, - savedQueryId: state.appStateContainer.getState().savedQuery, + savedQueryId: state.appState.getState().savedQuery, }); } }, @@ -161,7 +161,7 @@ export const getTopNavLinks = ({ } const sharingData = await getSharingData( searchSource, - state.appStateContainer.getState(), + state.appState.getState(), services ); diff --git a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.test.tsx b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.test.tsx index 9495fd3620dc0..f8199fadd2e61 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.test.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.test.tsx @@ -15,7 +15,7 @@ import { onSaveSearch } from './on_save_search'; import { dataViewMock } from '../../../../__mocks__/data_view'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; import { DiscoverServices } from '../../../../build_services'; -import { GetStateReturn } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; import { i18nServiceMock } from '@kbn/core/public/mocks'; import { ReactElement } from 'react'; import { discoverServiceMock } from '../../../../__mocks__/services'; @@ -30,12 +30,12 @@ describe('onSaveSearch', () => { }, } as unknown as DiscoverServices; const stateMock = { - appStateContainer: { + appState: { getState: () => ({ rowsPerPage: 250, }), }, - } as unknown as GetStateReturn; + } as unknown as DiscoverStateContainer; await onSaveSearch({ dataView: dataViewMock, @@ -52,12 +52,12 @@ describe('onSaveSearch', () => { it('should pass tags to the save modal', async () => { const serviceMock = discoverServiceMock; const stateMock = { - appStateContainer: { + appState: { getState: () => ({ rowsPerPage: 250, }), }, - } as unknown as GetStateReturn; + } as unknown as DiscoverStateContainer; let saveModal: ReactElement | undefined; jest.spyOn(savedObjectsPlugin, 'showSaveModal').mockImplementationOnce((modal) => { saveModal = modal; @@ -79,13 +79,13 @@ describe('onSaveSearch', () => { it('should update the saved search tags', async () => { const serviceMock = discoverServiceMock; const stateMock = { - appStateContainer: { + appState: { getState: () => ({ rowsPerPage: 250, }), }, resetInitialAppState: jest.fn(), - } as unknown as GetStateReturn; + } as unknown as DiscoverStateContainer; let saveModal: ReactElement | undefined; jest.spyOn(savedObjectsPlugin, 'showSaveModal').mockImplementationOnce((modal) => { saveModal = modal; @@ -123,13 +123,13 @@ describe('onSaveSearch', () => { it('should not update tags if savedObjectsTagging is undefined', async () => { const serviceMock = discoverServiceMock; const stateMock = { - appStateContainer: { + appState: { getState: () => ({ rowsPerPage: 250, }), }, resetInitialAppState: jest.fn(), - } as unknown as GetStateReturn; + } as unknown as DiscoverStateContainer; let saveModal: ReactElement | undefined; jest.spyOn(savedObjectsPlugin, 'showSaveModal').mockImplementationOnce((modal) => { saveModal = modal; diff --git a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx index d65debc53a0b8..5aa9fb6c65a5e 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/on_save_search.tsx @@ -14,7 +14,7 @@ import { SavedObjectSaveModal, showSaveModal, OnSaveProps } from '@kbn/saved-obj import { DataView } from '@kbn/data-views-plugin/public'; import { SavedSearch, SaveSavedSearchOptions } from '@kbn/saved-search-plugin/public'; import { DiscoverServices } from '../../../../build_services'; -import { GetStateReturn } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; import { setBreadcrumbsTitle } from '../../../../utils/breadcrumbs'; import { persistSavedSearch } from '../../utils/persist_saved_search'; import { DOC_TABLE_LEGACY } from '../../../../../common'; @@ -33,7 +33,7 @@ async function saveDataSource({ savedSearch: SavedSearch; saveOptions: SaveSavedSearchOptions; services: DiscoverServices; - state: GetStateReturn; + state: DiscoverStateContainer; navigateOrReloadSavedSearch: boolean; }) { const prevSavedSearchId = savedSearch.id; @@ -85,7 +85,7 @@ async function saveDataSource({ onSuccess, saveOptions, services, - state: state.appStateContainer.getState(), + state: state.appState.getState(), }); } @@ -103,7 +103,7 @@ export async function onSaveSearch({ navigateTo: (path: string) => void; savedSearch: SavedSearch; services: DiscoverServices; - state: GetStateReturn; + state: DiscoverStateContainer; updateAdHocDataViewId: (dataView: DataView) => Promise; onClose?: () => void; onSaveCb?: () => void; @@ -136,7 +136,7 @@ export async function onSaveSearch({ savedSearch.timeRestore = newTimeRestore; savedSearch.rowsPerPage = uiSettings.get(DOC_TABLE_LEGACY) ? currentRowsPerPage - : state.appStateContainer.getState().rowsPerPage; + : state.appState.getState().rowsPerPage; if (savedObjectsTagging) { savedSearch.tags = newTags; } 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 599eaa57aea3d..820d04cdddfe4 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 @@ -18,6 +18,8 @@ import { discoverServiceMock } from '../../__mocks__/services'; import { Router } from 'react-router-dom'; import { createMemoryHistory } from 'history'; import { urlTrackerMock } from '../../__mocks__/url_tracker.mock'; +import { getDiscoverStateMock } from '../../__mocks__/discover_state.mock'; +import { DiscoverMainProvider } from './services/discover_state_provider'; setHeaderActionMenuMounter(jest.fn()); setUrlTracker(urlTrackerMock); @@ -27,6 +29,8 @@ describe('DiscoverMainApp', () => { const dataViewList = [dataViewMock].map((ip) => { return { ...ip, ...{ attributes: { title: ip.title } } }; }) as unknown as DataViewListItem[]; + const stateContainer = getDiscoverStateMock({ isTimeBased: true }); + stateContainer.internalState.transitions.setDataView(dataViewMock); const props = { dataViewList, savedSearch: savedSearchMock, @@ -38,7 +42,9 @@ describe('DiscoverMainApp', () => { const component = mountWithIntl( - + + + ); 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 abd714fea8f07..9768faac5c982 100644 --- a/src/plugins/discover/public/application/main/discover_main_app.tsx +++ b/src/plugins/discover/public/application/main/discover_main_app.tsx @@ -17,7 +17,7 @@ import { useUrl } from './hooks/use_url'; import { useDiscoverServices } from '../../hooks/use_discover_services'; import { DataTableRecord } from '../../types'; import { useSavedSearchAliasMatchRedirect } from '../../hooks/saved_search_alias_match_redirect'; -import { DiscoverAppStateProvider } from './services/discover_app_state_container'; +import { DiscoverMainProvider } from './services/discover_state_provider'; const DiscoverLayoutMemoized = React.memo(DiscoverLayout); @@ -59,9 +59,7 @@ export function DiscoverMainApp(props: DiscoverMainProps) { refetch$, resetSavedSearch, searchSource, - state, stateContainer, - adHocDataViewList, } = useDiscoverState({ services, history: usedHistory, @@ -101,10 +99,9 @@ export function DiscoverMainApp(props: DiscoverMainProps) { useSavedSearchAliasMatchRedirect({ savedSearch, spaces, history }); return ( - + - + ); } diff --git a/src/plugins/discover/public/application/main/discover_main_route.tsx b/src/plugins/discover/public/application/main/discover_main_route.tsx index 4d3dccaa68001..073ae06d99e7d 100644 --- a/src/plugins/discover/public/application/main/discover_main_route.tsx +++ b/src/plugins/discover/public/application/main/discover_main_route.tsx @@ -20,7 +20,7 @@ import { getSavedSearch, getSavedSearchFullPathUrl, } from '@kbn/saved-search-plugin/public'; -import { getState } from './services/discover_state'; +import { getDiscoverStateContainer } from './services/discover_state'; import { loadDataView, resolveDataView } from './utils/resolve_data_view'; import { DiscoverMainApp } from './discover_main_app'; import { getRootBreadcrumbs, getSavedSearchBreadcrumbs } from '../../utils/breadcrumbs'; @@ -93,8 +93,12 @@ export function DiscoverMainRoute(props: Props) { return; } - const { appStateContainer } = getState({ history, savedSearch: nextSavedSearch, services }); - const { index } = appStateContainer.getState(); + const { appState } = getDiscoverStateContainer({ + history, + savedSearch: nextSavedSearch, + services, + }); + const { index } = appState.getState(); const ip = await loadDataView( data.dataViews, config, diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.ts b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx similarity index 68% rename from src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.ts rename to src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx index e14defbdeb585..86ae5f61db7f9 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.ts +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx @@ -6,15 +6,17 @@ * Side Public License, v 1. */ +import React from 'react'; import { createSearchSourceMock } from '@kbn/data-plugin/public/mocks'; import type { DataView } from '@kbn/data-views-plugin/public'; import { act, renderHook } from '@testing-library/react-hooks'; import { discoverServiceMock as mockDiscoverServices } from '../../../__mocks__/services'; -import { GetStateReturn } from '../services/discover_state'; import { useAdHocDataViews } from './use_adhoc_data_views'; import * as persistencePromptModule from '../../../hooks/use_confirm_persistence_prompt'; import { urlTrackerMock } from '../../../__mocks__/url_tracker.mock'; import { setUrlTracker } from '../../../kibana_services'; +import { getDiscoverStateMock } from '../../../__mocks__/discover_state.mock'; +import { DiscoverMainProvider } from '../services/discover_state_provider'; jest.mock('../../../hooks/use_confirm_persistence_prompt', () => { const createdDataView = { @@ -72,22 +74,24 @@ const savedSearchMock = { describe('useAdHocDataViews', () => { it('should save data view with new id and update saved search', async () => { - const hook = renderHook((d: DataView) => - useAdHocDataViews({ - dataView: mockDataView, - savedSearch: savedSearchMock, - stateContainer: { - appStateContainer: { getState: jest.fn().mockReturnValue({}) }, - replaceUrlAppState: jest.fn(), - kbnUrlStateStorage: { - kbnUrlControls: { flush: jest.fn() }, - }, - } as unknown as GetStateReturn, - setUrlTracking: jest.fn(), - dataViews: mockDiscoverServices.dataViews, - filterManager: mockDiscoverServices.filterManager, - toastNotifications: mockDiscoverServices.toastNotifications, - }) + const stateContainer = getDiscoverStateMock({ isTimeBased: true }); + + const hook = renderHook( + () => + useAdHocDataViews({ + dataView: mockDataView, + savedSearch: savedSearchMock, + stateContainer, + setUrlTracking: jest.fn(), + dataViews: mockDiscoverServices.dataViews, + filterManager: mockDiscoverServices.filterManager, + toastNotifications: mockDiscoverServices.toastNotifications, + }), + { + wrapper: ({ children }: { children: React.ReactElement }) => ( + {children} + ), + } ); const savedDataView = await hook.result.current.persistDataView(); @@ -104,22 +108,23 @@ describe('useAdHocDataViews', () => { ...mockDataView, id: 'updated-mock-id', })); - const hook = renderHook((d: DataView) => - useAdHocDataViews({ - dataView: mockDataView, - savedSearch: savedSearchMock, - stateContainer: { - appStateContainer: { getState: jest.fn().mockReturnValue({}) }, - replaceUrlAppState: jest.fn(), - kbnUrlStateStorage: { - kbnUrlControls: { flush: jest.fn() }, - }, - } as unknown as GetStateReturn, - setUrlTracking: jest.fn(), - dataViews: mockDiscoverServices.dataViews, - filterManager: mockDiscoverServices.filterManager, - toastNotifications: mockDiscoverServices.toastNotifications, - }) + const stateContainer = getDiscoverStateMock({ isTimeBased: true }); + const hook = renderHook( + () => + useAdHocDataViews({ + dataView: mockDataView, + savedSearch: savedSearchMock, + stateContainer: getDiscoverStateMock({ isTimeBased: true }), + setUrlTracking: jest.fn(), + dataViews: mockDiscoverServices.dataViews, + filterManager: mockDiscoverServices.filterManager, + toastNotifications: mockDiscoverServices.toastNotifications, + }), + { + wrapper: ({ children }: { children: React.ReactElement }) => ( + {children} + ), + } ); let updatedDataView: DataView; diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts index 89fd4477da34c..30787e84c304a 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { useCallback, useEffect, useState } from 'react'; +import { useCallback } from 'react'; import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; import { @@ -18,11 +18,10 @@ import type { FilterManager } from '@kbn/data-plugin/public'; import type { ToastsStart } from '@kbn/core-notifications-browser'; import { getUiActions } from '../../../kibana_services'; import { useConfirmPersistencePrompt } from '../../../hooks/use_confirm_persistence_prompt'; -import { GetStateReturn } from '../services/discover_state'; +import { DiscoverStateContainer } from '../services/discover_state'; import { useFiltersValidation } from './use_filters_validation'; export const useAdHocDataViews = ({ - dataView, savedSearch, stateContainer, setUrlTracking, @@ -30,27 +29,13 @@ export const useAdHocDataViews = ({ dataViews, toastNotifications, }: { - dataView: DataView; savedSearch: SavedSearch; - stateContainer: GetStateReturn; + stateContainer: DiscoverStateContainer; setUrlTracking: (dataView: DataView) => void; dataViews: DataViewsContract; filterManager: FilterManager; toastNotifications: ToastsStart; }) => { - const [adHocDataViewList, setAdHocDataViewList] = useState( - !dataView.isPersisted() ? [dataView] : [] - ); - - useEffect(() => { - if (!dataView.isPersisted()) { - setAdHocDataViewList((prev) => { - const existing = prev.find((prevDataView) => prevDataView.id === dataView.id); - return existing ? prev : [...prev, dataView]; - }); - } - }, [dataView]); - /** * Takes care of checking data view id references in filters */ @@ -62,12 +47,14 @@ export const useAdHocDataViews = ({ */ const updateAdHocDataViewId = useCallback( async (dataViewToUpdate: DataView) => { + const adHocDataViewList = stateContainer.internalState.getState().dataViewsAdHoc; const newDataView = await dataViews.create({ ...dataViewToUpdate.toSpec(), id: undefined }); dataViews.clearInstanceCache(dataViewToUpdate.id); - setAdHocDataViewList((prev) => - prev.filter((d) => d.id && dataViewToUpdate.id && d.id !== dataViewToUpdate.id) + const nextAdHocDataViewList = adHocDataViewList.filter( + (d: DataView) => d.id && dataViewToUpdate.id && d.id !== dataViewToUpdate.id ); + stateContainer.internalState.transitions.setDataViewsAdHoc(nextAdHocDataViewList); const uiActions = await getUiActions(); const trigger = uiActions.getTrigger(UPDATE_FILTER_REFERENCES_TRIGGER); @@ -97,7 +84,7 @@ export const useAdHocDataViews = ({ // update saved search with saved data view if (createdDataView && savedSearch.id) { - const currentState = stateContainer.appStateContainer.getState(); + const currentState = stateContainer.appState.getState(); await updateSavedSearch({ savedSearch, dataView: createdDataView, state: currentState }); } return createdDataView; @@ -105,5 +92,5 @@ export const useAdHocDataViews = ({ return currentDataView; }, [stateContainer, openConfirmSavePrompt, savedSearch, updateSavedSearch]); - return { adHocDataViewList, persistDataView, updateAdHocDataViewId }; + return { persistDataView, updateAdHocDataViewId }; }; diff --git a/src/plugins/discover/public/application/main/hooks/use_discover_state.test.tsx b/src/plugins/discover/public/application/main/hooks/use_discover_state.test.tsx index cdb931bbbd718..efd9454ef3bc6 100644 --- a/src/plugins/discover/public/application/main/hooks/use_discover_state.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_discover_state.test.tsx @@ -5,7 +5,7 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ - +import React from 'react'; import { renderHook } from '@testing-library/react-hooks'; import { DataViewListItem, SearchSource } from '@kbn/data-plugin/public'; import { createSearchSessionMock } from '../../../__mocks__/search_session'; @@ -15,22 +15,31 @@ import { useDiscoverState } from './use_discover_state'; import { dataViewMock } from '../../../__mocks__/data_view'; import { setUrlTracker } from '../../../kibana_services'; import { urlTrackerMock } from '../../../__mocks__/url_tracker.mock'; +import { DiscoverMainProvider } from '../services/discover_state_provider'; +import { getDiscoverStateMock } from '../../../__mocks__/discover_state.mock'; setUrlTracker(urlTrackerMock); describe('test useDiscoverState', () => { test('return is valid', async () => { const { history } = createSearchSessionMock(); + const stateContainer = getDiscoverStateMock({ isTimeBased: true }); - const { result } = renderHook(() => { - return useDiscoverState({ - services: discoverServiceMock, - history, - savedSearch: savedSearchMock, - setExpandedDoc: jest.fn(), - dataViewList: [dataViewMock as DataViewListItem], - }); - }); - expect(result.current.state.index).toBe(dataViewMock.id); + const { result } = renderHook( + () => { + return useDiscoverState({ + services: discoverServiceMock, + history, + savedSearch: savedSearchMock, + setExpandedDoc: jest.fn(), + dataViewList: [dataViewMock as DataViewListItem], + }); + }, + { + wrapper: ({ children }: { children: React.ReactElement }) => ( + {children} + ), + } + ); expect(result.current.stateContainer).toBeInstanceOf(Object); expect(result.current.searchSource).toBeInstanceOf(SearchSource); }); 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 ff448e59917a8..8210ec646bf69 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 @@ -5,7 +5,7 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { useMemo, useEffect, useState, useCallback } from 'react'; +import { useMemo, useEffect, useCallback } from 'react'; import { isEqual } from 'lodash'; import { History } from 'history'; import { DataViewListItem, DataViewType } from '@kbn/data-views-plugin/public'; @@ -13,7 +13,7 @@ import { SavedSearch, getSavedSearch } from '@kbn/saved-search-plugin/public'; import type { SortOrder } from '@kbn/saved-search-plugin/public'; import { useTextBasedQueryLanguage } from './use_text_based_query_language'; import { useUrlTracking } from './use_url_tracking'; -import { getState } from '../services/discover_state'; +import { getDiscoverStateContainer } from '../services/discover_state'; import { getStateDefaults } from '../utils/get_state_defaults'; import { DiscoverServices } from '../../../build_services'; import { loadDataView, resolveDataView } from '../utils/resolve_data_view'; @@ -57,19 +57,18 @@ export function useDiscoverState({ const { setUrlTracking } = useUrlTracking(savedSearch, dataView); - const stateContainer = useMemo( - () => - getState({ - history, - savedSearch, - services, - }), - [history, savedSearch, services] - ); - - const { appStateContainer, replaceUrlAppState } = stateContainer; + const stateContainer = useMemo(() => { + const container = getDiscoverStateContainer({ + history, + savedSearch, + services, + }); + container.internalState.transitions.setDataViews(dataViewList); + container.internalState.transitions.setDataView(dataView); + return container; + }, [dataView, dataViewList, history, savedSearch, services]); - const [state, setState] = useState(appStateContainer.getState()); + const { appState, replaceUrlAppState } = stateContainer; /** * Search session logic @@ -92,6 +91,7 @@ export function useDiscoverState({ */ const onChangeDataView = useCallback( async (id: string) => { + const state = stateContainer.appState.getState(); const nextDataView = await dataViews.get(id); if (nextDataView && dataView) { const nextAppState = getDataViewAppState( @@ -104,28 +104,18 @@ export function useDiscoverState({ state.query ); setUrlTracking(nextDataView); + stateContainer.internalState.transitions.setDataView(nextDataView); stateContainer.setAppState(nextAppState); } setExpandedDoc(undefined); }, - [ - setUrlTracking, - uiSettings, - dataView, - dataViews, - setExpandedDoc, - state.columns, - state.query, - state.sort, - stateContainer, - ] + [setUrlTracking, uiSettings, dataView, dataViews, setExpandedDoc, stateContainer] ); /** * Adhoc data views functionality */ - const { adHocDataViewList, persistDataView, updateAdHocDataViewId } = useAdHocDataViews({ - dataView, + const { persistDataView, updateAdHocDataViewId } = useAdHocDataViews({ stateContainer, savedSearch, setUrlTracking, @@ -168,8 +158,6 @@ export function useDiscoverState({ */ useEffect(() => { const stopSync = stateContainer.initializeAndSync(dataView, filterManager, data); - setState(stateContainer.appStateContainer.getState()); - return () => stopSync(); }, [stateContainer, filterManager, data, dataView]); @@ -177,8 +165,8 @@ export function useDiscoverState({ * Track state changes that should trigger a fetch */ useEffect(() => { - const unsubscribe = appStateContainer.subscribe(async (nextState) => { - const { hideChart, interval, sort, index } = state; + const unsubscribe = appState.subscribe(async (nextState) => { + const { hideChart, interval, sort, index } = stateContainer.getPreviousAppState(); // chart was hidden, now it should be displayed, so data is needed const chartDisplayChanged = nextState.hideChart !== hideChart && hideChart; const chartIntervalChanged = nextState.interval !== interval; @@ -202,6 +190,7 @@ export function useDiscoverState({ savedSearch.searchSource, services.toastNotifications ); + stateContainer.internalState.transitions.setDataView(nextDataView); // If the requested data view is not found, don't try to load it, // and instead reset the app state to the fallback data view @@ -209,7 +198,6 @@ export function useDiscoverState({ replaceUrlAppState({ index: nextDataView.id }); return; } - savedSearch.searchSource.setField('index', nextDataView); reset(); } @@ -217,18 +205,18 @@ export function useDiscoverState({ if (chartDisplayChanged || chartIntervalChanged || docTableSortChanged) { refetch$.next(undefined); } - setState(nextState); }); return () => unsubscribe(); }, [ - services, - appStateContainer, - state, + appState, refetch$, - data$, + replaceUrlAppState, reset, savedSearch.searchSource, - replaceUrlAppState, + services.dataViews, + services.toastNotifications, + services.uiSettings, + stateContainer, ]); /** @@ -256,7 +244,6 @@ export function useDiscoverState({ }); await stateContainer.replaceUrlAppState(newAppState); - setState(newAppState); }, [services, dataView, stateContainer] ); @@ -302,10 +289,7 @@ export function useDiscoverState({ onChangeDataView, onUpdateQuery, searchSource, - setState, - state, stateContainer, - adHocDataViewList, persistDataView, updateAdHocDataViewId, }; diff --git a/src/plugins/discover/public/application/main/hooks/use_saved_search.test.ts b/src/plugins/discover/public/application/main/hooks/use_saved_search.test.tsx similarity index 78% rename from src/plugins/discover/public/application/main/hooks/use_saved_search.test.ts rename to src/plugins/discover/public/application/main/hooks/use_saved_search.test.tsx index 34cdeb232be88..7fd55233ef550 100644 --- a/src/plugins/discover/public/application/main/hooks/use_saved_search.test.ts +++ b/src/plugins/discover/public/application/main/hooks/use_saved_search.test.tsx @@ -11,19 +11,21 @@ import { createSearchSessionMock } from '../../../__mocks__/search_session'; import { discoverServiceMock } from '../../../__mocks__/services'; import { savedSearchMock, savedSearchMockWithSQL } from '../../../__mocks__/saved_search'; import { RecordRawType, useSavedSearch } from './use_saved_search'; -import { getState } from '../services/discover_state'; +import { getDiscoverStateContainer } from '../services/discover_state'; import { useDiscoverState } from './use_discover_state'; import { FetchStatus } from '../../types'; import { dataViewMock } from '../../../__mocks__/data_view'; import { DataViewListItem } from '@kbn/data-views-plugin/common'; import { setUrlTracker } from '../../../kibana_services'; import { urlTrackerMock } from '../../../__mocks__/url_tracker.mock'; +import React from 'react'; +import { DiscoverMainProvider } from '../services/discover_state_provider'; setUrlTracker(urlTrackerMock); describe('test useSavedSearch', () => { test('useSavedSearch return is valid', async () => { const { history, searchSessionManager } = createSearchSessionMock(); - const stateContainer = getState({ + const stateContainer = getDiscoverStateContainer({ savedSearch: savedSearchMock, services: discoverServiceMock, history, @@ -49,7 +51,7 @@ describe('test useSavedSearch', () => { }); test('refetch$ triggers a search', async () => { const { history, searchSessionManager } = createSearchSessionMock(); - const stateContainer = getState({ + const stateContainer = getDiscoverStateContainer({ savedSearch: savedSearchMock, services: discoverServiceMock, history, @@ -59,15 +61,22 @@ describe('test useSavedSearch', () => { return { from: '2021-05-01T20:00:00Z', to: '2021-05-02T20:00:00Z' }; }); - const { result: resultState } = renderHook(() => { - return useDiscoverState({ - services: discoverServiceMock, - history, - savedSearch: savedSearchMock, - setExpandedDoc: jest.fn(), - dataViewList: [dataViewMock as DataViewListItem], - }); - }); + const { result: resultState } = renderHook( + () => { + return useDiscoverState({ + services: discoverServiceMock, + history, + savedSearch: savedSearchMock, + setExpandedDoc: jest.fn(), + dataViewList: [dataViewMock as DataViewListItem], + }); + }, + { + wrapper: ({ children }: { children: React.ReactElement }) => ( + {children} + ), + } + ); const { result, waitForValueToChange } = renderHook(() => { return useSavedSearch({ @@ -93,7 +102,7 @@ describe('test useSavedSearch', () => { test('reset sets back to initial state', async () => { const { history, searchSessionManager } = createSearchSessionMock(); - const stateContainer = getState({ + const stateContainer = getDiscoverStateContainer({ savedSearch: savedSearchMock, services: discoverServiceMock, history, @@ -103,15 +112,22 @@ describe('test useSavedSearch', () => { return { from: '2021-05-01T20:00:00Z', to: '2021-05-02T20:00:00Z' }; }); - const { result: resultState } = renderHook(() => { - return useDiscoverState({ - services: discoverServiceMock, - history, - savedSearch: savedSearchMock, - setExpandedDoc: jest.fn(), - dataViewList: [dataViewMock as DataViewListItem], - }); - }); + const { result: resultState } = renderHook( + () => { + return useDiscoverState({ + services: discoverServiceMock, + history, + savedSearch: savedSearchMock, + setExpandedDoc: jest.fn(), + dataViewList: [dataViewMock as DataViewListItem], + }); + }, + { + wrapper: ({ children }: { children: React.ReactElement }) => ( + {children} + ), + } + ); const { result, waitForValueToChange } = renderHook(() => { return useSavedSearch({ @@ -137,7 +153,7 @@ describe('test useSavedSearch', () => { test('useSavedSearch returns plain record raw type', async () => { const { history, searchSessionManager } = createSearchSessionMock(); - const stateContainer = getState({ + const stateContainer = getDiscoverStateContainer({ savedSearch: savedSearchMockWithSQL, services: discoverServiceMock, history, 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 2f097daac982d..8bec08b22c113 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 @@ -16,7 +16,7 @@ import type { SearchResponse } from '@elastic/elasticsearch/lib/api/types'; import { getRawRecordType } from '../utils/get_raw_record_type'; import { DiscoverServices } from '../../../build_services'; import { DiscoverSearchSessionManager } from '../services/discover_search_session'; -import { GetStateReturn } from '../services/discover_state'; +import { DiscoverStateContainer } from '../services/discover_state'; import { validateTimeRange } from '../utils/validate_time_range'; import { useSingleton } from './use_singleton'; import { FetchStatus } from '../../types'; @@ -109,12 +109,12 @@ export const useSavedSearch = ({ searchSessionManager: DiscoverSearchSessionManager; searchSource: ISearchSource; services: DiscoverServices; - stateContainer: GetStateReturn; + stateContainer: DiscoverStateContainer; useNewFieldsApi: boolean; }) => { const { data, filterManager } = services; const timefilter = data.query.timefilter.timefilter; - const { query } = stateContainer.appStateContainer.getState(); + const { query } = stateContainer.appState.getState(); const recordRawType = useMemo(() => getRawRecordType(query), [query]); @@ -190,7 +190,7 @@ export const useSavedSearch = ({ await fetchAll(dataSubjects, searchSource, val === 'reset', { abortController, - appStateContainer: stateContainer.appStateContainer, + appStateContainer: stateContainer.appState, data, initialFetchStatus, inspectorAdapters, @@ -229,7 +229,7 @@ export const useSavedSearch = ({ searchSource, services, services.toastNotifications, - stateContainer.appStateContainer, + stateContainer.appState, timefilter, useNewFieldsApi, ]); diff --git a/src/plugins/discover/public/application/main/hooks/use_search_session.test.ts b/src/plugins/discover/public/application/main/hooks/use_search_session.test.ts index f29d1414b51d3..931d652d031c5 100644 --- a/src/plugins/discover/public/application/main/hooks/use_search_session.test.ts +++ b/src/plugins/discover/public/application/main/hooks/use_search_session.test.ts @@ -11,12 +11,12 @@ import { renderHook } from '@testing-library/react-hooks'; import { createSearchSessionMock } from '../../../__mocks__/search_session'; import { discoverServiceMock } from '../../../__mocks__/services'; import { savedSearchMock } from '../../../__mocks__/saved_search'; -import { getState } from '../services/discover_state'; +import { getDiscoverStateContainer } from '../services/discover_state'; describe('test useSearchSession', () => { test('getting the next session id', async () => { const { history } = createSearchSessionMock(); - const stateContainer = getState({ + const stateContainer = getDiscoverStateContainer({ savedSearch: savedSearchMock, history, services: discoverServiceMock, diff --git a/src/plugins/discover/public/application/main/hooks/use_search_session.ts b/src/plugins/discover/public/application/main/hooks/use_search_session.ts index af5768d7c4752..8c89a2e92ff05 100644 --- a/src/plugins/discover/public/application/main/hooks/use_search_session.ts +++ b/src/plugins/discover/public/application/main/hooks/use_search_session.ts @@ -12,7 +12,7 @@ import { SavedSearch } from '@kbn/saved-search-plugin/public'; import { DiscoverSearchSessionManager } from '../services/discover_search_session'; import { createSearchSessionRestorationDataProvider, - GetStateReturn, + DiscoverStateContainer, } from '../services/discover_state'; import { DiscoverServices } from '../../../build_services'; @@ -23,7 +23,7 @@ export function useSearchSession({ savedSearch, }: { services: DiscoverServices; - stateContainer: GetStateReturn; + stateContainer: DiscoverStateContainer; history: History; savedSearch: SavedSearch; }) { @@ -43,7 +43,7 @@ export function useSearchSession({ useEffect(() => { data.search.session.enableStorage( createSearchSessionRestorationDataProvider({ - appStateContainer: stateContainer.appStateContainer, + appStateContainer: stateContainer.appState, data, getSavedSearch: () => savedSearch, }), @@ -61,7 +61,7 @@ export function useSearchSession({ capabilities.discover.storeSearchSession, data, savedSearch, - stateContainer.appStateContainer, + stateContainer.appState, ]); return searchSessionManager; diff --git a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts index d79004bf2d796..bd38bb7ffb70f 100644 --- a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts +++ b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts @@ -10,7 +10,7 @@ import { renderHook } from '@testing-library/react-hooks'; import { waitFor } from '@testing-library/react'; import { discoverServiceMock } from '../../../__mocks__/services'; import { useTextBasedQueryLanguage } from './use_text_based_query_language'; -import { AppState, GetStateReturn } from '../services/discover_state'; +import { AppState, DiscoverStateContainer } from '../services/discover_state'; import { BehaviorSubject } from 'rxjs'; import { FetchStatus } from '../../types'; import { DataDocuments$, RecordRawType } from './use_saved_search'; @@ -26,12 +26,12 @@ function getHookProps( ) { const stateContainer = { replaceUrlAppState, - appStateContainer: { + appState: { getState: () => { return []; }, }, - } as unknown as GetStateReturn; + } as unknown as DiscoverStateContainer; const msgLoading = { recordRawType: RecordRawType.PLAIN, @@ -210,7 +210,7 @@ describe('useTextBasedQueryLanguage', () => { test('it should not overwrite existing state columns on initial fetch', async () => { const replaceUrlAppState = jest.fn(); const props = getHookProps(replaceUrlAppState, query); - props.stateContainer.appStateContainer.getState = jest.fn(() => { + props.stateContainer.appState.getState = jest.fn(() => { return { columns: ['field1'], index: 'the-data-view-id' }; }); const { documents$ } = props; @@ -250,7 +250,7 @@ describe('useTextBasedQueryLanguage', () => { test('it should not overwrite state column when successfully fetching after an error fetch', async () => { const replaceUrlAppState = jest.fn(); const props = getHookProps(replaceUrlAppState, query); - props.stateContainer.appStateContainer.getState = jest.fn(() => { + props.stateContainer.appState.getState = jest.fn(() => { return { columns: [], index: 'the-data-view-id' }; }); const { documents$ } = props; @@ -275,7 +275,7 @@ describe('useTextBasedQueryLanguage', () => { query: { sql: 'SELECT * from the-data-view-title WHERE field1=2' }, }); await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); - props.stateContainer.appStateContainer.getState = jest.fn(() => { + props.stateContainer.appState.getState = jest.fn(() => { return { columns: ['field1', 'field2'], index: 'the-data-view-id' }; }); replaceUrlAppState.mockReset(); diff --git a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts index 1521ca1b673a0..0bde228e3c4c4 100644 --- a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts +++ b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts @@ -15,7 +15,7 @@ import { import { useCallback, useEffect, useRef } from 'react'; import { DataViewListItem, DataViewsContract } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; -import type { GetStateReturn } from '../services/discover_state'; +import type { DiscoverStateContainer } from '../services/discover_state'; import type { DataDocuments$ } from './use_saved_search'; import { FetchStatus } from '../../types'; @@ -33,7 +33,7 @@ export function useTextBasedQueryLanguage({ savedSearch, }: { documents$: DataDocuments$; - stateContainer: GetStateReturn; + stateContainer: DiscoverStateContainer; dataViews: DataViewsContract; dataViewList: DataViewListItem[]; savedSearch: SavedSearch; @@ -59,7 +59,7 @@ export function useTextBasedQueryLanguage({ if (!query || next.fetchStatus === FetchStatus.ERROR) { return; } - const { columns: stateColumns, index } = stateContainer.appStateContainer.getState(); + const { columns: stateColumns, index } = stateContainer.appState.getState(); let nextColumns: string[] = []; const isTextBasedQueryLang = recordRawType === 'plain' && isOfAggregateQueryType(query) && 'sql' in query; diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts new file mode 100644 index 0000000000000..77bc77d08f76b --- /dev/null +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -0,0 +1,60 @@ +/* + * 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 { + createStateContainer, + createStateContainerReactHelpers, + ReduxLikeStateContainer, +} from '@kbn/kibana-utils-plugin/common'; +import { DataViewListItem, DataView } from '@kbn/data-views-plugin/common'; + +export interface InternalState { + dataView: DataView | undefined; + dataViews: DataViewListItem[]; + dataViewsAdHoc: DataView[]; +} + +interface InternalStateTransitions { + setDataView: (state: InternalState) => (dataView: DataView) => InternalState; + setDataViews: (state: InternalState) => (dataViews: DataViewListItem[]) => InternalState; + setDataViewsAdHoc: (state: InternalState) => (dataViews: DataView[]) => InternalState; +} + +export type InternalStateContainer = ReduxLikeStateContainer< + InternalState, + InternalStateTransitions +>; + +export const { Provider: InternalStateProvider, useSelector: useInternalStateSelector } = + createStateContainerReactHelpers>(); + +export function getInternalStateContainer() { + return createStateContainer( + { + dataView: undefined, + dataViews: [], + dataViewsAdHoc: [], + }, + { + setDataView: (prevState: InternalState) => (nextDataView: DataView) => ({ + ...prevState, + dataView: nextDataView, + }), + setDataViews: (prevState: InternalState) => (dataViews: DataViewListItem[]) => ({ + ...prevState, + dataViews, + }), + setDataViewsAdHoc: (prevState: InternalState) => (dataViewsAdHoc: DataView[]) => ({ + ...prevState, + dataViewsAdHoc, + }), + }, + {}, + { freeze: (state) => state } + ); +} diff --git a/src/plugins/discover/public/application/main/services/discover_state.test.ts b/src/plugins/discover/public/application/main/services/discover_state.test.ts index aef263b23cec9..c29da2897bcee 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.test.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.test.ts @@ -7,8 +7,8 @@ */ import { - getState, - GetStateReturn, + getDiscoverStateContainer, + DiscoverStateContainer, createSearchSessionRestorationDataProvider, } from './discover_state'; import { createBrowserHistory, History } from 'history'; @@ -18,7 +18,7 @@ import { savedSearchMock, savedSearchMockWithTimeField } from '../../../__mocks_ import { discoverServiceMock } from '../../../__mocks__/services'; let history: History; -let state: GetStateReturn; +let state: DiscoverStateContainer; const getCurrentUrl = () => history.createHref(history.location); describe('Test discover state', () => { @@ -27,7 +27,7 @@ describe('Test discover state', () => { beforeEach(async () => { history = createBrowserHistory(); history.push('/'); - state = getState({ + state = getDiscoverStateContainer({ savedSearch: savedSearchMock, services: discoverServiceMock, history, @@ -49,7 +49,7 @@ describe('Test discover state', () => { test('changing URL to be propagated to appState', async () => { history.push('/#?_a=(index:modified)'); - expect(state.appStateContainer.getState()).toMatchInlineSnapshot(` + expect(state.appState.getState()).toMatchInlineSnapshot(` Object { "index": "modified", } @@ -58,7 +58,7 @@ describe('Test discover state', () => { test('URL navigation to url without _a, state should not change', async () => { history.push('/#?_a=(index:modified)'); history.push('/'); - expect(state.appStateContainer.getState()).toEqual({ + expect(state.appState.getState()).toEqual({ index: 'modified', }); }); @@ -72,7 +72,7 @@ describe('Test discover state', () => { test('getPreviousAppState returns the state before the current', async () => { state.setAppState({ index: 'first' }); - const stateA = state.appStateContainer.getState(); + const stateA = state.appState.getState(); state.setAppState({ index: 'second' }); expect(state.getPreviousAppState()).toEqual(stateA); }); @@ -89,41 +89,41 @@ describe('Test discover initial state sort handling', () => { history = createBrowserHistory(); history.push('/#?_a=(sort:!(!(order_date,desc)))'); - state = getState({ + state = getDiscoverStateContainer({ savedSearch: { ...savedSearchMock, ...{ sort: [['bytes', 'desc']] } }, services: discoverServiceMock, history, }); await state.replaceUrlAppState({}); const stopSync = state.startSync(); - expect(state.appStateContainer.getState().sort).toEqual([['order_date', 'desc']]); + expect(state.appState.getState().sort).toEqual([['order_date', 'desc']]); stopSync(); }); test('Empty sort in URL should use saved search sort for state', async () => { history = createBrowserHistory(); history.push('/#?_a=(sort:!())'); const nextSavedSearch = { ...savedSearchMock, ...{ sort: [['bytes', 'desc']] as SortOrder[] } }; - state = getState({ + state = getDiscoverStateContainer({ savedSearch: nextSavedSearch, services: discoverServiceMock, history, }); await state.replaceUrlAppState({}); const stopSync = state.startSync(); - expect(state.appStateContainer.getState().sort).toEqual([['bytes', 'desc']]); + expect(state.appState.getState().sort).toEqual([['bytes', 'desc']]); stopSync(); }); test('Empty sort in URL and saved search should sort by timestamp', async () => { history = createBrowserHistory(); history.push('/#?_a=(sort:!())'); - state = getState({ + state = getDiscoverStateContainer({ savedSearch: savedSearchMockWithTimeField, services: discoverServiceMock, history, }); await state.replaceUrlAppState({}); const stopSync = state.startSync(); - expect(state.appStateContainer.getState().sort).toEqual([['timestamp', 'desc']]); + expect(state.appState.getState().sort).toEqual([['timestamp', 'desc']]); stopSync(); }); }); @@ -134,12 +134,12 @@ describe('Test discover state with legacy migration', () => { history.push( "/#?_a=(query:(query_string:(analyze_wildcard:!t,query:'type:nice%20name:%22yeah%22')))" ); - state = getState({ + state = getDiscoverStateContainer({ savedSearch: savedSearchMock, services: discoverServiceMock, history, }); - expect(state.appStateContainer.getState().query).toMatchInlineSnapshot(` + expect(state.appState.getState().query).toMatchInlineSnapshot(` Object { "language": "lucene", "query": Object { @@ -158,11 +158,11 @@ describe('createSearchSessionRestorationDataProvider', () => { const mockDataPlugin = dataPluginMock.createStartContract(); const searchSessionInfoProvider = createSearchSessionRestorationDataProvider({ data: mockDataPlugin, - appStateContainer: getState({ + appStateContainer: getDiscoverStateContainer({ savedSearch: savedSearchMock, services: discoverServiceMock, history, - }).appStateContainer, + }).appState, getSavedSearch: () => mockSavedSearch, }); diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 611b59eacdc79..245f66bdf958d 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -36,6 +36,10 @@ import { } from '@kbn/data-plugin/public'; import { DataView } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; +import { + InternalStateContainer, + getInternalStateContainer, +} from './discover_internal_state_container'; import { getStateDefaults } from '../utils/get_state_defaults'; import { DiscoverServices } from '../../../build_services'; import { DiscoverGridSettings } from '../../../components/discover_grid/types'; @@ -107,7 +111,7 @@ export interface AppStateUrl extends Omit { sort?: string[][] | [string, string]; } -interface GetStateParams { +interface DiscoverStateContainerArgs { /** * Browser history */ @@ -122,7 +126,7 @@ interface GetStateParams { services: DiscoverServices; } -export interface GetStateReturn { +export interface DiscoverStateContainer { /** * kbnUrlStateStorage */ @@ -130,7 +134,11 @@ export interface GetStateReturn { /** * App state, the _a part of the URL */ - appStateContainer: ReduxLikeStateContainer; + appState: ReduxLikeStateContainer; + /** + * Internal state that's used at several places in the UI + */ + internalState: InternalStateContainer; /** * Initialize state with filters and query, start state syncing */ @@ -175,6 +183,12 @@ export interface GetStateReturn { * Pause the auto refresh interval without pushing an entry to history */ pauseAutoRefreshInterval: () => Promise; + /** + * functions executed by UI + */ + actions: { + setDataView: (dataView: DataView) => void; + }; } const APP_STATE_URL_KEY = '_a'; @@ -184,7 +198,11 @@ const GLOBAL_STATE_URL_KEY = '_g'; * Builds and returns appState and globalState containers and helper functions * Used to sync URL with UI state */ -export function getState({ history, savedSearch, services }: GetStateParams): GetStateReturn { +export function getDiscoverStateContainer({ + history, + savedSearch, + services, +}: DiscoverStateContainerArgs): DiscoverStateContainer { const storeInSessionStorage = services.uiSettings.get('state:storeInSessionStorage'); const toasts = services.core.notifications.toasts; const defaultAppState = getStateDefaults({ @@ -239,6 +257,8 @@ export function getState({ history, savedSearch, services }: GetStateParams): Ge await stateStorage.set(APP_STATE_URL_KEY, state, { replace: true }); }; + const internalStateContainer = getInternalStateContainer(); + const pauseAutoRefreshInterval = async () => { const state = stateStorage.get(GLOBAL_STATE_URL_KEY); if (state?.refreshInterval && !state.refreshInterval.pause) { @@ -250,9 +270,21 @@ export function getState({ history, savedSearch, services }: GetStateParams): Ge } }; + const setDataView = (dataView: DataView) => { + internalStateContainer.transitions.setDataView(dataView); + if (!dataView.isPersisted()) { + const adHocDataViewList = internalStateContainer.getState().dataViewsAdHoc; + const existing = adHocDataViewList.find((prevDataView) => prevDataView.id === dataView.id); + if (!existing) { + internalStateContainer.transitions.setDataViewsAdHoc([...adHocDataViewList, dataView]); + } + } + }; + return { kbnUrlStateStorage: stateStorage, - appStateContainer: appStateContainerModified, + appState: appStateContainerModified, + internalState: internalStateContainer, startSync: () => { const { start, stop } = syncAppState(); start(); @@ -328,6 +360,9 @@ export function getState({ history, savedSearch, services }: GetStateParams): Ge stop(); }; }, + actions: { + setDataView, + }, }; } diff --git a/src/plugins/discover/public/application/main/services/discover_state_provider.tsx b/src/plugins/discover/public/application/main/services/discover_state_provider.tsx new file mode 100644 index 0000000000000..8cfdc45207ec7 --- /dev/null +++ b/src/plugins/discover/public/application/main/services/discover_state_provider.tsx @@ -0,0 +1,37 @@ +/* + * 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 React from 'react'; +import { InternalStateProvider } from './discover_internal_state_container'; +import { DiscoverAppStateProvider } from './discover_app_state_container'; +import { DiscoverStateContainer } from './discover_state'; + +function createStateHelpers() { + const context = React.createContext(null); + return { + Provider: context.Provider, + }; +} + +export const { Provider: DiscoverStateProvider } = createStateHelpers(); + +export const DiscoverMainProvider = ({ + value, + children, +}: { + value: DiscoverStateContainer; + children: React.ReactElement; +}) => { + return ( + + + {children} + + + ); +}; diff --git a/src/plugins/discover/public/components/doc_table/actions/columns.ts b/src/plugins/discover/public/components/doc_table/actions/columns.ts index 36b4970b70999..b6438a2e1cdd1 100644 --- a/src/plugins/discover/public/components/doc_table/actions/columns.ts +++ b/src/plugins/discover/public/components/doc_table/actions/columns.ts @@ -9,7 +9,7 @@ import { Capabilities, IUiSettingsClient } from '@kbn/core/public'; import { DataViewsContract } from '@kbn/data-plugin/public'; import { DataView } from '@kbn/data-views-plugin/public'; import { SORT_DEFAULT_ORDER_SETTING } from '../../../../common'; -import { GetStateReturn as DiscoverGetStateReturn } from '../../../application/main/services/discover_state'; +import { DiscoverStateContainer as DiscoverGetStateReturn } from '../../../application/main/services/discover_state'; import { GetStateReturn as ContextGetStateReturn } from '../../../application/context/services/context_state'; import { popularizeField } from '../../../utils/popularize_field'; diff --git a/src/plugins/discover/public/hooks/use_data_grid_columns.test.tsx b/src/plugins/discover/public/hooks/use_data_grid_columns.test.tsx index 23c39113a0808..80e861ea99c42 100644 --- a/src/plugins/discover/public/hooks/use_data_grid_columns.test.tsx +++ b/src/plugins/discover/public/hooks/use_data_grid_columns.test.tsx @@ -11,7 +11,6 @@ import { useColumns } from './use_data_grid_columns'; import { dataViewMock } from '../__mocks__/data_view'; import { configMock } from '../__mocks__/config'; import { dataViewsMock } from '../__mocks__/data_views'; -import { AppState } from '../application/context/services/context_state'; import { Capabilities } from '@kbn/core/types'; describe('useColumns', () => { @@ -21,9 +20,7 @@ describe('useColumns', () => { dataView: dataViewMock, dataViews: dataViewsMock, setAppState: () => {}, - state: { - columns: ['Time', 'message'], - } as AppState, + columns: ['Time', 'message'], useNewFieldsApi: false, }; @@ -43,9 +40,7 @@ describe('useColumns', () => { const { result } = renderHook(() => { return useColumns({ ...defaultProps, - state: { - columns: ['Time', '_source'], - }, + columns: ['Time', '_source'], useNewFieldsApi: true, }); }); @@ -57,9 +52,7 @@ describe('useColumns', () => { const { result } = renderHook(() => { return useColumns({ ...defaultProps, - state: { - columns: [], - }, + columns: [], }); }); expect(result.current.columns).toEqual([]); diff --git a/src/plugins/discover/public/hooks/use_data_grid_columns.ts b/src/plugins/discover/public/hooks/use_data_grid_columns.ts index 530ac62ca8d06..a9540b275fb66 100644 --- a/src/plugins/discover/public/hooks/use_data_grid_columns.ts +++ b/src/plugins/discover/public/hooks/use_data_grid_columns.ts @@ -11,14 +11,8 @@ import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/public' import { Capabilities, IUiSettingsClient } from '@kbn/core/public'; import { isEqual } from 'lodash'; -import { - AppState as DiscoverState, - GetStateReturn as DiscoverGetStateReturn, -} from '../application/main/services/discover_state'; -import { - AppState as ContextState, - GetStateReturn as ContextGetStateReturn, -} from '../application/context/services/context_state'; +import { DiscoverStateContainer as DiscoverGetStateReturn } from '../application/main/services/discover_state'; +import { GetStateReturn as ContextGetStateReturn } from '../application/context/services/context_state'; import { getStateColumnActions } from '../components/doc_table/actions/columns'; interface UseColumnsProps { @@ -28,7 +22,8 @@ interface UseColumnsProps { dataViews: DataViewsContract; useNewFieldsApi: boolean; setAppState: DiscoverGetStateReturn['setAppState'] | ContextGetStateReturn['setAppState']; - state: DiscoverState | ContextState; + columns?: string[]; + sort?: string[][]; } export const useColumns = ({ @@ -37,17 +32,18 @@ export const useColumns = ({ dataView, dataViews, setAppState, - state, useNewFieldsApi, + columns, + sort, }: UseColumnsProps) => { - const [usedColumns, setUsedColumns] = useState(getColumns(state.columns, useNewFieldsApi)); + const [usedColumns, setUsedColumns] = useState(getColumns(columns, useNewFieldsApi)); useEffect(() => { - const nextColumns = getColumns(state.columns, useNewFieldsApi); + const nextColumns = getColumns(columns, useNewFieldsApi); if (isEqual(usedColumns, nextColumns)) { return; } setUsedColumns(nextColumns); - }, [state.columns, useNewFieldsApi, usedColumns]); + }, [columns, useNewFieldsApi, usedColumns]); const { onAddColumn, onRemoveColumn, onSetColumns, onMoveColumn } = useMemo( () => getStateColumnActions({ @@ -57,19 +53,10 @@ export const useColumns = ({ dataViews, setAppState, useNewFieldsApi, - columns: usedColumns, - sort: state.sort, + columns, + sort, }), - [ - capabilities, - config, - dataView, - dataViews, - setAppState, - state.sort, - useNewFieldsApi, - usedColumns, - ] + [capabilities, columns, config, dataView, dataViews, setAppState, sort, useNewFieldsApi] ); return { From 2557d207e58902b82d5d08a13138a49591b9140d Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Thu, 27 Oct 2022 23:27:23 +0000 Subject: [PATCH 02/36] [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' --- .../components/field_stats_table/field_stats_table.tsx | 3 +-- .../main/components/top_nav/get_top_nav_links.tsx | 6 +----- .../public/application/main/hooks/use_search_session.ts | 7 +------ 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/field_stats_table/field_stats_table.tsx b/src/plugins/discover/public/application/main/components/field_stats_table/field_stats_table.tsx index 090eae497de04..eb4577a559bb2 100644 --- a/src/plugins/discover/public/application/main/components/field_stats_table/field_stats_table.tsx +++ b/src/plugins/discover/public/application/main/components/field_stats_table/field_stats_table.tsx @@ -117,8 +117,7 @@ export const FieldStatisticsTable = (props: FieldStatisticsTableProps) => { const embeddableRoot: React.RefObject = useRef(null); const showPreviewByDefault = useMemo( - () => - stateContainer ? !stateContainer.appState.getState().hideAggregatedPreview : true, + () => (stateContainer ? !stateContainer.appState.getState().hideAggregatedPreview : true), [stateContainer] ); diff --git a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx index dc66022920189..16ebc461c2f98 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/get_top_nav_links.tsx @@ -159,11 +159,7 @@ export const getTopNavLinks = ({ if (!services.share || !updatedDataView) { return; } - const sharingData = await getSharingData( - searchSource, - state.appState.getState(), - services - ); + const sharingData = await getSharingData(searchSource, state.appState.getState(), services); services.share.toggleShareContextMenu({ anchorElement, diff --git a/src/plugins/discover/public/application/main/hooks/use_search_session.ts b/src/plugins/discover/public/application/main/hooks/use_search_session.ts index 8c89a2e92ff05..5592993b90d55 100644 --- a/src/plugins/discover/public/application/main/hooks/use_search_session.ts +++ b/src/plugins/discover/public/application/main/hooks/use_search_session.ts @@ -57,12 +57,7 @@ export function useSearchSession({ }, } ); - }, [ - capabilities.discover.storeSearchSession, - data, - savedSearch, - stateContainer.appState, - ]); + }, [capabilities.discover.storeSearchSession, data, savedSearch, stateContainer.appState]); return searchSessionManager; } From 67c1d23df41ee2c01eef53b0390191e143cba5f8 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 28 Oct 2022 09:05:49 +0200 Subject: [PATCH 03/36] Remove dataViews from internal state since it is not used --- .../main/components/layout/discover_layout.test.tsx | 4 +--- .../application/main/hooks/use_adhoc_data_views.test.tsx | 2 -- .../public/application/main/hooks/use_discover_state.ts | 9 ++++----- .../main/services/discover_internal_state_container.ts | 9 +-------- .../public/application/main/services/discover_state.ts | 3 +++ 5 files changed, 9 insertions(+), 18 deletions(-) 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 46912b401e201..22ff50374b7b7 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 @@ -16,7 +16,7 @@ import { esHits } from '../../../../__mocks__/es_hits'; import { dataViewMock } from '../../../../__mocks__/data_view'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; import { createSearchSourceMock } from '@kbn/data-plugin/common/search/search_source/mocks'; -import type { DataView, DataViewListItem } from '@kbn/data-views-plugin/public'; +import type { DataView } from '@kbn/data-views-plugin/public'; import { dataViewWithTimefieldMock } from '../../../../__mocks__/data_view_with_timefield'; import { DiscoverLayoutProps } from './types'; import { @@ -117,7 +117,6 @@ async function mountComponent( return { from: '2020-05-14T11:05:13.590', to: '2020-05-14T11:20:13.590' }; }; - const dataViewList = [dataView] as DataViewListItem[]; const stateContainer = getDiscoverStateMock({ isTimeBased: true }); const main$ = new BehaviorSubject({ @@ -156,7 +155,6 @@ async function mountComponent( stateContainer.setAppState({ interval: 'auto', query }); stateContainer.internalState.transitions.setDataView(dataView); - stateContainer.internalState.transitions.setDataViews(dataViewList as DataViewListItem[]); const props: DiscoverLayoutProps = { dataView, diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx index 86ae5f61db7f9..c561cb13211a5 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx @@ -79,7 +79,6 @@ describe('useAdHocDataViews', () => { const hook = renderHook( () => useAdHocDataViews({ - dataView: mockDataView, savedSearch: savedSearchMock, stateContainer, setUrlTracking: jest.fn(), @@ -112,7 +111,6 @@ describe('useAdHocDataViews', () => { const hook = renderHook( () => useAdHocDataViews({ - dataView: mockDataView, savedSearch: savedSearchMock, stateContainer: getDiscoverStateMock({ isTimeBased: true }), setUrlTracking: jest.fn(), 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 8210ec646bf69..9a71dfa316dba 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 @@ -63,10 +63,9 @@ export function useDiscoverState({ savedSearch, services, }); - container.internalState.transitions.setDataViews(dataViewList); - container.internalState.transitions.setDataView(dataView); + container.actions.setDataView(dataView); return container; - }, [dataView, dataViewList, history, savedSearch, services]); + }, [dataView, history, savedSearch, services]); const { appState, replaceUrlAppState } = stateContainer; @@ -104,7 +103,7 @@ export function useDiscoverState({ state.query ); setUrlTracking(nextDataView); - stateContainer.internalState.transitions.setDataView(nextDataView); + stateContainer.actions.setDataView(nextDataView); stateContainer.setAppState(nextAppState); } setExpandedDoc(undefined); @@ -190,7 +189,7 @@ export function useDiscoverState({ savedSearch.searchSource, services.toastNotifications ); - stateContainer.internalState.transitions.setDataView(nextDataView); + stateContainer.actions.setDataView(nextDataView); // If the requested data view is not found, don't try to load it, // and instead reset the app state to the fallback data view diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index 77bc77d08f76b..6e42b1792fefa 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -11,17 +11,15 @@ import { createStateContainerReactHelpers, ReduxLikeStateContainer, } from '@kbn/kibana-utils-plugin/common'; -import { DataViewListItem, DataView } from '@kbn/data-views-plugin/common'; +import { DataView } from '@kbn/data-views-plugin/common'; export interface InternalState { dataView: DataView | undefined; - dataViews: DataViewListItem[]; dataViewsAdHoc: DataView[]; } interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; - setDataViews: (state: InternalState) => (dataViews: DataViewListItem[]) => InternalState; setDataViewsAdHoc: (state: InternalState) => (dataViews: DataView[]) => InternalState; } @@ -37,7 +35,6 @@ export function getInternalStateContainer() { return createStateContainer( { dataView: undefined, - dataViews: [], dataViewsAdHoc: [], }, { @@ -45,10 +42,6 @@ export function getInternalStateContainer() { ...prevState, dataView: nextDataView, }), - setDataViews: (prevState: InternalState) => (dataViews: DataViewListItem[]) => ({ - ...prevState, - dataViews, - }), setDataViewsAdHoc: (prevState: InternalState) => (dataViewsAdHoc: DataView[]) => ({ ...prevState, dataViewsAdHoc, diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 245f66bdf958d..50c6a95354851 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -187,6 +187,9 @@ export interface DiscoverStateContainer { * functions executed by UI */ actions: { + /** + * Set the currently selected data view + */ setDataView: (dataView: DataView) => void; }; } From d74ab22b46e5a73fc0d80aa59edd06de1304a9d3 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Sat, 29 Oct 2022 09:36:33 +0200 Subject: [PATCH 04/36] Fix tests --- .../layout/discover_layout.test.tsx | 1 - .../components/layout/discover_layout.tsx | 20 +++++----- .../main/components/layout/types.ts | 1 - .../top_nav/discover_topnav.test.tsx | 1 - .../components/top_nav/discover_topnav.tsx | 4 +- .../main/discover_main_app.test.tsx | 4 +- .../application/main/discover_main_app.tsx | 2 - .../application/main/discover_main_route.tsx | 9 ++++- .../main/hooks/use_adhoc_data_views.ts | 4 +- .../main/hooks/use_discover_state.ts | 39 ++++++++++++------- 10 files changed, 48 insertions(+), 37 deletions(-) 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 22ff50374b7b7..4bf59527d1a87 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 @@ -157,7 +157,6 @@ async function mountComponent( stateContainer.internalState.transitions.setDataView(dataView); const props: DiscoverLayoutProps = { - dataView, inspectorAdapters: { requests: new RequestAdapter() }, navigateTo: jest.fn(), onChangeDataView: jest.fn(), 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 a34bddc81b566..bbe8a65ec5b1a 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 @@ -84,20 +84,19 @@ export function DiscoverLayout({ inspector, } = useDiscoverServices(); const { main$ } = savedSearchData$; - const [viewMode, query, savedQuery, filters, columns, sort] = useAppStateSelector((state) => [ - state.viewMode, + const [query, savedQuery, filters, columns, sort] = useAppStateSelector((state) => [ state.query, state.savedQuery, state.filters, state.columns, state.sort, ]); + const viewMode: VIEW_MODE = useAppStateSelector((state) => { + if (uiSettings.get(SHOW_FIELD_STATISTICS) !== true) return VIEW_MODE.DOCUMENT_LEVEL; + return state.viewMode ?? VIEW_MODE.DOCUMENT_LEVEL; + }); const dataView = useInternalStateSelector((state) => state.dataView!); const dataState: DataMainMsg = useDataState(main$); - const currentViewMode = useMemo(() => { - if (uiSettings.get(SHOW_FIELD_STATISTICS) !== true) return VIEW_MODE.DOCUMENT_LEVEL; - return viewMode ?? VIEW_MODE.DOCUMENT_LEVEL; - }, [uiSettings, viewMode]); const fetchCounter = useRef(0); @@ -163,7 +162,6 @@ export function DiscoverLayout({ const onFieldEdited = useCallback(async () => { if (!dataView.isPersisted()) { await updateAdHocDataViewId(dataView); - return; } savedSearchRefetch$.next('reset'); }, [dataView, savedSearchRefetch$, updateAdHocDataViewId]); @@ -186,8 +184,9 @@ export function DiscoverLayout({ if (nextDataView.id) { onChangeDataView(nextDataView.id); } + savedSearchRefetch$.next('reset'); }, - [onChangeDataView] + [onChangeDataView, savedSearchRefetch$] ); const savedSearchTitle = useRef(null); @@ -224,7 +223,6 @@ export function DiscoverLayout({ })} @@ -324,7 +322,7 @@ export function DiscoverLayout({ savedSearchRefetch$={savedSearchRefetch$} stateContainer={stateContainer} isTimeBased={isTimeBased} - viewMode={currentViewMode} + viewMode={viewMode} onAddFilter={onAddFilter as DocViewFilterFn} onFieldEdited={onFieldEdited} columns={currentColumns} 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 ff0491c10dbfb..d65f7e853ab85 100644 --- a/src/plugins/discover/public/application/main/components/layout/types.ts +++ b/src/plugins/discover/public/application/main/components/layout/types.ts @@ -16,7 +16,6 @@ import { DiscoverStateContainer } from '../../services/discover_state'; import { DataRefetch$, SavedSearchData } from '../../hooks/use_saved_search'; export interface DiscoverLayoutProps { - dataView: DataView; inspectorAdapters: { requests: RequestAdapter }; navigateTo: (url: string) => void; onChangeDataView: (id: string) => void; diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx index 32b03a94dc5bf..3cf331b4d52b1 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx @@ -35,7 +35,6 @@ function getProps(savePermissions = true): DiscoverTopNavProps { return { stateContainer, - dataView: dataViewMock, savedSearch: savedSearchMock, navigateTo: jest.fn(), query: {} as Query, diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 9f8643a572ff9..9356a1d51e4df 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -21,7 +21,7 @@ import { onSaveSearch } from './on_save_search'; export type DiscoverTopNavProps = Pick< DiscoverLayoutProps, - 'dataView' | 'navigateTo' | 'savedSearch' | 'searchSource' + 'navigateTo' | 'savedSearch' | 'searchSource' > & { onOpenInspector: () => void; query?: Query | AggregateQuery; @@ -41,7 +41,6 @@ export type DiscoverTopNavProps = Pick< }; export const DiscoverTopNav = ({ - dataView, onOpenInspector, query, savedQuery, @@ -60,6 +59,7 @@ export const DiscoverTopNav = ({ }: DiscoverTopNavProps) => { const history = useHistory(); const adHocDataViewList = useInternalStateSelector((state) => state.dataViewsAdHoc); + const dataView = useInternalStateSelector((state) => state.dataView!); const showDatePicker = useMemo( () => dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP, 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 820d04cdddfe4..b992f892ee27f 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 @@ -30,7 +30,7 @@ describe('DiscoverMainApp', () => { return { ...ip, ...{ attributes: { title: ip.title } } }; }) as unknown as DataViewListItem[]; const stateContainer = getDiscoverStateMock({ isTimeBased: true }); - stateContainer.internalState.transitions.setDataView(dataViewMock); + stateContainer.actions.setDataView(dataViewMock); const props = { dataViewList, savedSearch: savedSearchMock, @@ -50,6 +50,6 @@ describe('DiscoverMainApp', () => { ); expect(component.find(DiscoverTopNav).exists()).toBe(true); - expect(component.find(DiscoverTopNav).prop('dataView')).toEqual(dataViewMock); + expect(component.find(DiscoverTopNav).prop('savedSearch')).toEqual(savedSearchMock); }); }); 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 9768faac5c982..d9c5bd309042a 100644 --- a/src/plugins/discover/public/application/main/discover_main_app.tsx +++ b/src/plugins/discover/public/application/main/discover_main_app.tsx @@ -50,7 +50,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) { */ const { data$, - dataView, inspectorAdapters, onChangeDataView, onUpdateQuery, @@ -101,7 +100,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) { return ( (); + const [loading, setLoading] = useState(true); const [savedSearch, setSavedSearch] = useState(); - const dataView = savedSearch?.searchSource?.getField('index'); const [dataViewList, setDataViewList] = useState([]); const [hasESData, setHasESData] = useState(false); const [hasUserDataView, setHasUserDataView] = useState(false); const [showNoDataPage, setShowNoDataPage] = useState(false); const { id } = useParams(); + useEffect(() => { + setLoading(true); + }, [id]); useExecutionContext(core.executionContext, { type: 'application', @@ -160,6 +163,7 @@ export function DiscoverMainRoute(props: Props) { currentSavedSearch.id ); } + setLoading(false); } catch (e) { if (e instanceof DataViewSavedObjectConflictError) { setError(e); @@ -202,6 +206,7 @@ export function DiscoverMainRoute(props: Props) { const onDataViewCreated = useCallback( async (nextDataView: unknown) => { if (nextDataView) { + setLoading(true); setShowNoDataPage(false); setError(undefined); await loadSavedSearch(); @@ -250,7 +255,7 @@ export function DiscoverMainRoute(props: Props) { return ; } - if (!dataView || !savedSearch) { + if (loading || !savedSearch) { return ; } diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts index 30787e84c304a..8f4cfce6f903e 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts @@ -67,8 +67,8 @@ export const useAdHocDataViews = ({ toDataView: newDataView.id, usedDataViews: [], } as ActionExecutionContext); - - stateContainer.replaceUrlAppState({ index: newDataView.id }); + stateContainer.actions.setDataView(newDataView); + await stateContainer.replaceUrlAppState({ index: newDataView.id }); setUrlTracking(newDataView); return newDataView; }, 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 9a71dfa316dba..8c27164e5e5b4 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 @@ -5,7 +5,7 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { useMemo, useEffect, useCallback } from 'react'; +import { useMemo, useEffect, useState, useCallback } from 'react'; import { isEqual } from 'lodash'; import { History } from 'history'; import { DataViewListItem, DataViewType } from '@kbn/data-views-plugin/public'; @@ -55,8 +55,6 @@ export function useDiscoverState({ return savedSearch.searchSource.createChild(); }, [savedSearch, dataView]); - const { setUrlTracking } = useUrlTracking(savedSearch, dataView); - const stateContainer = useMemo(() => { const container = getDiscoverStateContainer({ history, @@ -65,10 +63,14 @@ export function useDiscoverState({ }); container.actions.setDataView(dataView); return container; - }, [dataView, history, savedSearch, services]); + }, [history, savedSearch, services, dataView]); + + const { setUrlTracking } = useUrlTracking(savedSearch, dataView); const { appState, replaceUrlAppState } = stateContainer; + const [state, setState] = useState(appState.getState()); + /** * Search session logic */ @@ -90,7 +92,6 @@ export function useDiscoverState({ */ const onChangeDataView = useCallback( async (id: string) => { - const state = stateContainer.appState.getState(); const nextDataView = await dataViews.get(id); if (nextDataView && dataView) { const nextAppState = getDataViewAppState( @@ -103,12 +104,21 @@ export function useDiscoverState({ state.query ); setUrlTracking(nextDataView); - stateContainer.actions.setDataView(nextDataView); stateContainer.setAppState(nextAppState); } setExpandedDoc(undefined); }, - [setUrlTracking, uiSettings, dataView, dataViews, setExpandedDoc, stateContainer] + [ + setUrlTracking, + uiSettings, + dataView, + dataViews, + setExpandedDoc, + state.columns, + state.query, + state.sort, + stateContainer, + ] ); /** @@ -157,6 +167,8 @@ export function useDiscoverState({ */ useEffect(() => { const stopSync = stateContainer.initializeAndSync(dataView, filterManager, data); + setState(stateContainer.appState.getState()); + return () => stopSync(); }, [stateContainer, filterManager, data, dataView]); @@ -165,7 +177,7 @@ export function useDiscoverState({ */ useEffect(() => { const unsubscribe = appState.subscribe(async (nextState) => { - const { hideChart, interval, sort, index } = stateContainer.getPreviousAppState(); + const { hideChart, interval, sort, index } = state; // chart was hidden, now it should be displayed, so data is needed const chartDisplayChanged = nextState.hideChart !== hideChart && hideChart; const chartIntervalChanged = nextState.interval !== interval; @@ -204,17 +216,18 @@ export function useDiscoverState({ if (chartDisplayChanged || chartIntervalChanged || docTableSortChanged) { refetch$.next(undefined); } + setState(nextState); }); return () => unsubscribe(); }, [ + services, appState, + state, refetch$, - replaceUrlAppState, + data$, reset, savedSearch.searchSource, - services.dataViews, - services.toastNotifications, - services.uiSettings, + replaceUrlAppState, stateContainer, ]); @@ -243,6 +256,7 @@ export function useDiscoverState({ }); await stateContainer.replaceUrlAppState(newAppState); + setState(newAppState); }, [services, dataView, stateContainer] ); @@ -281,7 +295,6 @@ export function useDiscoverState({ return { data$, - dataView, inspectorAdapters, refetch$, resetSavedSearch, From 43c06f9ba92b7cd3f397113417cb2f68c03b2711 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Sat, 29 Oct 2022 18:32:08 +0200 Subject: [PATCH 05/36] Fix more tests --- .../public/application/main/hooks/use_discover_state.ts | 4 ++-- .../public/application/main/services/discover_state.ts | 2 +- x-pack/test/functional/apps/discover/saved_searches.ts | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) 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 8c27164e5e5b4..49a7d7aa3254c 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 @@ -201,7 +201,6 @@ export function useDiscoverState({ savedSearch.searchSource, services.toastNotifications ); - stateContainer.actions.setDataView(nextDataView); // If the requested data view is not found, don't try to load it, // and instead reset the app state to the fallback data view @@ -211,6 +210,7 @@ export function useDiscoverState({ } savedSearch.searchSource.setField('index', nextDataView); reset(); + stateContainer.actions.setDataView(nextDataView); } if (chartDisplayChanged || chartIntervalChanged || docTableSortChanged) { @@ -226,7 +226,7 @@ export function useDiscoverState({ refetch$, data$, reset, - savedSearch.searchSource, + savedSearch, replaceUrlAppState, stateContainer, ]); diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 50c6a95354851..0def18e4575f2 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -319,7 +319,7 @@ export function getDiscoverStateContainer({ setState(appStateContainerModified, { index: dataView.id }); } // sync initial app filters from state to filterManager - const filters = appStateContainer.getState().filters; + const filters = appStateContainer.getState().filters || []; if (filters) { filterManager.setAppFilters(cloneDeep(filters)); } diff --git a/x-pack/test/functional/apps/discover/saved_searches.ts b/x-pack/test/functional/apps/discover/saved_searches.ts index 414dc8395f184..8d0c7917503ef 100644 --- a/x-pack/test/functional/apps/discover/saved_searches.ts +++ b/x-pack/test/functional/apps/discover/saved_searches.ts @@ -66,11 +66,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.discover.selectIndexPattern('ecommerce'); await filterBar.addFilter('category', 'is', `Men's Shoes`); await queryBar.setQuery('customer_gender:MALE'); + await queryBar.submitQuery(); await PageObjects.discover.saveSearch('test-unselect-saved-search'); - await queryBar.submitQuery(); - expect(await filterBar.hasFilter('category', `Men's Shoes`)).to.be(true); expect(await queryBar.getQueryString()).to.eql('customer_gender:MALE'); From 940480168a68e5f81a23cc39d47564e954144cbf Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 2 Nov 2022 10:25:29 +0100 Subject: [PATCH 06/36] Refactor AppState type location --- .../__stories__/discover_layout.stories.tsx | 2 +- .../layout/discover_documents.test.tsx | 3 +- .../use_test_based_query_language.test.ts | 3 +- .../services/discover_app_state_container.ts | 59 ++++++++++++++- .../main/services/discover_state.ts | 73 ++----------------- .../main/utils/cleanup_url_state.ts | 3 +- .../application/main/utils/fetch_all.test.ts | 2 +- .../application/main/utils/fetch_all.ts | 2 +- .../main/utils/fetch_chart.test.ts | 2 +- .../main/utils/get_state_defaults.ts | 2 +- .../main/utils/persist_saved_search.ts | 2 +- .../doc_table/actions/columns.test.ts | 2 +- .../discover/public/utils/get_sharing_data.ts | 3 +- 13 files changed, 78 insertions(+), 80 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/__stories__/discover_layout.stories.tsx b/src/plugins/discover/public/application/main/components/layout/__stories__/discover_layout.stories.tsx index 6c15b0beaa8c5..8392bb7c0ed49 100644 --- a/src/plugins/discover/public/application/main/components/layout/__stories__/discover_layout.stories.tsx +++ b/src/plugins/discover/public/application/main/components/layout/__stories__/discover_layout.stories.tsx @@ -9,12 +9,12 @@ import React, { useState } from 'react'; import { storiesOf } from '@storybook/react'; import { __IntlProvider as IntlProvider } from '@kbn/i18n-react'; +import { AppState } from '../../../services/discover_app_state_container'; import { getDataViewMock } from '../../../../../__mocks__/__storybook_mocks__/get_data_view_mock'; import { withDiscoverServices } from '../../../../../__mocks__/__storybook_mocks__/with_discover_services'; import { getDocumentsLayoutProps, getPlainRecordLayoutProps } from './get_layout_props'; import { DiscoverLayout } from '../discover_layout'; import { setHeaderActionMenuMounter } from '../../../../../kibana_services'; -import { AppState } from '../../../services/discover_state'; import { DiscoverLayoutProps } from '../types'; setHeaderActionMenuMounter(() => void 0); diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx index 10aa40657b92f..fac5e025888d8 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx @@ -12,7 +12,7 @@ import { mountWithIntl } from '@kbn/test-jest-helpers'; import { setHeaderActionMenuMounter } from '../../../../kibana_services'; import { esHits } from '../../../../__mocks__/es_hits'; import { savedSearchMock } from '../../../../__mocks__/saved_search'; -import { AppState, DiscoverStateContainer } from '../../services/discover_state'; +import { DiscoverStateContainer } from '../../services/discover_state'; import { DataDocuments$ } from '../../hooks/use_saved_search'; import { discoverServiceMock } from '../../../../__mocks__/services'; import { FetchStatus } from '../../../types'; @@ -23,6 +23,7 @@ import { buildDataTableRecord } from '../../../../utils/build_data_record'; import { EsHitRecord } from '../../../../types'; import { DiscoverMainProvider } from '../../services/discover_state_provider'; import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock'; +import { AppState } from '../../services/discover_app_state_container'; setHeaderActionMenuMounter(jest.fn()); diff --git a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts index bd38bb7ffb70f..659cdcb71b084 100644 --- a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts +++ b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts @@ -10,7 +10,7 @@ import { renderHook } from '@testing-library/react-hooks'; import { waitFor } from '@testing-library/react'; import { discoverServiceMock } from '../../../__mocks__/services'; import { useTextBasedQueryLanguage } from './use_text_based_query_language'; -import { AppState, DiscoverStateContainer } from '../services/discover_state'; +import { DiscoverStateContainer } from '../services/discover_state'; import { BehaviorSubject } from 'rxjs'; import { FetchStatus } from '../../types'; import { DataDocuments$, RecordRawType } from './use_saved_search'; @@ -19,6 +19,7 @@ import { AggregateQuery, Query } from '@kbn/es-query'; import { dataViewMock } from '../../../__mocks__/data_view'; import { DataViewListItem } from '@kbn/data-views-plugin/common'; import { savedSearchMock } from '../../../__mocks__/saved_search'; +import { AppState } from '../services/discover_app_state_container'; function getHookProps( replaceUrlAppState: (newState: Partial) => Promise, diff --git a/src/plugins/discover/public/application/main/services/discover_app_state_container.ts b/src/plugins/discover/public/application/main/services/discover_app_state_container.ts index 4c484698ece0f..d692730eda070 100644 --- a/src/plugins/discover/public/application/main/services/discover_app_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_app_state_container.ts @@ -10,7 +10,64 @@ import { createStateContainerReactHelpers, ReduxLikeStateContainer, } from '@kbn/kibana-utils-plugin/common'; -import { AppState } from './discover_state'; +import { AggregateQuery, Filter, Query } from '@kbn/es-query'; +import { DiscoverGridSettings } from '../../../components/discover_grid/types'; +import { VIEW_MODE } from '../../../components/view_mode_toggle'; + +export interface AppState { + /** + * Columns displayed in the table + */ + columns?: string[]; + /** + * Array of applied filters + */ + filters?: Filter[]; + /** + * Data Grid related state + */ + grid?: DiscoverGridSettings; + /** + * Hide chart + */ + hideChart?: boolean; + /** + * id of the used data view + */ + index?: string; + /** + * Used interval of the histogram + */ + interval?: string; + /** + * Lucence or KQL query + */ + query?: Query | AggregateQuery; + /** + * Array of the used sorting [[field,direction],...] + */ + sort?: string[][]; + /** + * id of the used saved query + */ + savedQuery?: string; + /** + * Table view: Documents vs Field Statistics + */ + viewMode?: VIEW_MODE; + /** + * Hide mini distribution/preview charts when in Field Statistics mode + */ + hideAggregatedPreview?: boolean; + /** + * Document explorer row height option + */ + rowHeight?: number; + /** + * Number of rows in the grid per page + */ + rowsPerPage?: number; +} export const { Provider: DiscoverAppStateProvider, useSelector: useAppStateSelector } = createStateContainerReactHelpers>(); diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 0def18e4575f2..27bde15953e20 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -9,14 +9,7 @@ import { cloneDeep, isEqual } from 'lodash'; import { i18n } from '@kbn/i18n'; import { History } from 'history'; -import { - Filter, - FilterStateStore, - compareFilters, - COMPARE_ALL_OPTIONS, - Query, - AggregateQuery, -} from '@kbn/es-query'; +import { COMPARE_ALL_OPTIONS, compareFilters, Filter, FilterStateStore } from '@kbn/es-query'; import { createKbnUrlStateStorage, createStateContainer, @@ -36,74 +29,18 @@ import { } from '@kbn/data-plugin/public'; import { DataView } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; +import { AppState } from './discover_app_state_container'; import { - InternalStateContainer, getInternalStateContainer, + InternalStateContainer, } from './discover_internal_state_container'; import { getStateDefaults } from '../utils/get_state_defaults'; import { DiscoverServices } from '../../../build_services'; -import { DiscoverGridSettings } from '../../../components/discover_grid/types'; import { handleSourceColumnState } from '../../../utils/state_helpers'; import { DISCOVER_APP_LOCATOR, DiscoverAppLocatorParams } from '../../../locator'; -import { VIEW_MODE } from '../../../components/view_mode_toggle'; import { cleanupUrlState } from '../utils/cleanup_url_state'; import { getValidFilters } from '../../../utils/get_valid_filters'; -export interface AppState { - /** - * Columns displayed in the table - */ - columns?: string[]; - /** - * Array of applied filters - */ - filters?: Filter[]; - /** - * Data Grid related state - */ - grid?: DiscoverGridSettings; - /** - * Hide chart - */ - hideChart?: boolean; - /** - * id of the used data view - */ - index?: string; - /** - * Used interval of the histogram - */ - interval?: string; - /** - * Lucence or KQL query - */ - query?: Query | AggregateQuery; - /** - * Array of the used sorting [[field,direction],...] - */ - sort?: string[][]; - /** - * id of the used saved query - */ - savedQuery?: string; - /** - * Table view: Documents vs Field Statistics - */ - viewMode?: VIEW_MODE; - /** - * Hide mini distribution/preview charts when in Field Statistics mode - */ - hideAggregatedPreview?: boolean; - /** - * Document explorer row height option - */ - rowHeight?: number; - /** - * Number of rows in the grid per page - */ - rowsPerPage?: number; -} - export interface AppStateUrl extends Omit { /** * Necessary to take care of legacy links [fieldName,direction] @@ -111,7 +48,7 @@ export interface AppStateUrl extends Omit { sort?: string[][] | [string, string]; } -interface DiscoverStateContainerArgs { +interface DiscoverStateContainerParams { /** * Browser history */ @@ -205,7 +142,7 @@ export function getDiscoverStateContainer({ history, savedSearch, services, -}: DiscoverStateContainerArgs): DiscoverStateContainer { +}: DiscoverStateContainerParams): DiscoverStateContainer { const storeInSessionStorage = services.uiSettings.get('state:storeInSessionStorage'); const toasts = services.core.notifications.toasts; const defaultAppState = getStateDefaults({ diff --git a/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts b/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts index 11f1a6d3c8ae3..c0e5e4ed8198b 100644 --- a/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts +++ b/src/plugins/discover/public/application/main/utils/cleanup_url_state.ts @@ -6,8 +6,9 @@ * Side Public License, v 1. */ import { isOfAggregateQueryType } from '@kbn/es-query'; +import { AppState } from '../services/discover_app_state_container'; import { migrateLegacyQuery } from '../../../utils/migrate_legacy_query'; -import { AppState, AppStateUrl } from '../services/discover_state'; +import { AppStateUrl } from '../services/discover_state'; /** * Takes care of the given url state, migrates legacy props and cleans up empty props diff --git a/src/plugins/discover/public/application/main/utils/fetch_all.test.ts b/src/plugins/discover/public/application/main/utils/fetch_all.test.ts index 59dbc3ffe73d8..576f06f524bb2 100644 --- a/src/plugins/discover/public/application/main/utils/fetch_all.test.ts +++ b/src/plugins/discover/public/application/main/utils/fetch_all.test.ts @@ -12,7 +12,6 @@ import { SearchSource } from '@kbn/data-plugin/public'; import { RequestAdapter } from '@kbn/inspector-plugin/common'; import { savedSearchMock } from '../../../__mocks__/saved_search'; import { ReduxLikeStateContainer } from '@kbn/kibana-utils-plugin/common'; -import { AppState } from '../services/discover_state'; import { discoverServiceMock } from '../../../__mocks__/services'; import { fetchAll } from './fetch_all'; import { @@ -31,6 +30,7 @@ import { fetchTotalHits } from './fetch_total_hits'; import { buildDataTableRecord } from '../../../utils/build_data_record'; import { dataViewMock } from '../../../__mocks__/data_view'; import type { SearchResponse } from '@elastic/elasticsearch/lib/api/types'; +import { AppState } from '../services/discover_app_state_container'; jest.mock('./fetch_documents', () => ({ fetchDocuments: jest.fn().mockResolvedValue([]), diff --git a/src/plugins/discover/public/application/main/utils/fetch_all.ts b/src/plugins/discover/public/application/main/utils/fetch_all.ts index d530da1492fac..61514f50ed58a 100644 --- a/src/plugins/discover/public/application/main/utils/fetch_all.ts +++ b/src/plugins/discover/public/application/main/utils/fetch_all.ts @@ -10,6 +10,7 @@ import { Adapters } from '@kbn/inspector-plugin/common'; import { ReduxLikeStateContainer } from '@kbn/kibana-utils-plugin/common'; import { DataViewType } from '@kbn/data-views-plugin/public'; import type { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public'; +import { AppState } from '../services/discover_app_state_container'; import { getRawRecordType } from './get_raw_record_type'; import { sendCompleteMsg, @@ -23,7 +24,6 @@ import { updateSearchSource } from './update_search_source'; import { fetchDocuments } from './fetch_documents'; import { fetchTotalHits } from './fetch_total_hits'; import { fetchChart } from './fetch_chart'; -import { AppState } from '../services/discover_state'; import { FetchStatus } from '../../types'; import { DataCharts$, diff --git a/src/plugins/discover/public/application/main/utils/fetch_chart.test.ts b/src/plugins/discover/public/application/main/utils/fetch_chart.test.ts index e1020404d3996..d53572e5f26d3 100644 --- a/src/plugins/discover/public/application/main/utils/fetch_chart.test.ts +++ b/src/plugins/discover/public/application/main/utils/fetch_chart.test.ts @@ -10,10 +10,10 @@ import { RequestAdapter } from '@kbn/inspector-plugin/common'; import { savedSearchMockWithTimeField } from '../../../__mocks__/saved_search'; import { fetchChart, updateSearchSource } from './fetch_chart'; import { ReduxLikeStateContainer } from '@kbn/kibana-utils-plugin/common'; -import { AppState } from '../services/discover_state'; import { discoverServiceMock } from '../../../__mocks__/services'; import { calculateBounds } from '@kbn/data-plugin/public'; import { FetchDeps } from './fetch_all'; +import { AppState } from '../services/discover_app_state_container'; function getDeps() { const deps = { diff --git a/src/plugins/discover/public/application/main/utils/get_state_defaults.ts b/src/plugins/discover/public/application/main/utils/get_state_defaults.ts index b8b3c3579f343..ee257c281cf38 100644 --- a/src/plugins/discover/public/application/main/utils/get_state_defaults.ts +++ b/src/plugins/discover/public/application/main/utils/get_state_defaults.ts @@ -9,6 +9,7 @@ import { cloneDeep, isEqual } from 'lodash'; import { IUiSettingsClient } from '@kbn/core/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; +import { AppState } from '../services/discover_app_state_container'; import { DiscoverServices } from '../../../build_services'; import { getDefaultSort, getSortArray } from '../../../utils/sorting'; import { @@ -18,7 +19,6 @@ import { SORT_DEFAULT_ORDER_SETTING, } from '../../../../common'; -import { AppState } from '../services/discover_state'; import { CHART_HIDDEN_KEY } from '../components/layout/use_discover_histogram'; function getDefaultColumns(savedSearch: SavedSearch, uiSettings: IUiSettingsClient) { diff --git a/src/plugins/discover/public/application/main/utils/persist_saved_search.ts b/src/plugins/discover/public/application/main/utils/persist_saved_search.ts index edc7d96f9decd..f9521fe20ba11 100644 --- a/src/plugins/discover/public/application/main/utils/persist_saved_search.ts +++ b/src/plugins/discover/public/application/main/utils/persist_saved_search.ts @@ -9,8 +9,8 @@ import { isOfAggregateQueryType } from '@kbn/es-query'; import { DataView } from '@kbn/data-views-plugin/public'; import { SavedObjectSaveOpts } from '@kbn/saved-objects-plugin/public'; import { SavedSearch, SortOrder, saveSavedSearch } from '@kbn/saved-search-plugin/public'; +import { AppState } from '../services/discover_app_state_container'; import { updateSearchSource } from './update_search_source'; -import { AppState } from '../services/discover_state'; import { DiscoverServices } from '../../../build_services'; /** * Helper function to update and persist the given savedSearch diff --git a/src/plugins/discover/public/components/doc_table/actions/columns.test.ts b/src/plugins/discover/public/components/doc_table/actions/columns.test.ts index b8879389b4af0..02956b2894ffb 100644 --- a/src/plugins/discover/public/components/doc_table/actions/columns.test.ts +++ b/src/plugins/discover/public/components/doc_table/actions/columns.test.ts @@ -11,7 +11,7 @@ import { configMock } from '../../../__mocks__/config'; import { dataViewMock } from '../../../__mocks__/data_view'; import { dataViewsMock } from '../../../__mocks__/data_views'; import { Capabilities } from '@kbn/core/types'; -import { AppState } from '../../../application/main/services/discover_state'; +import { AppState } from '../../../application/main/services/discover_app_state_container'; function getStateColumnAction(state: AppState, setAppState: (state: Partial) => void) { return getStateColumnActions({ diff --git a/src/plugins/discover/public/utils/get_sharing_data.ts b/src/plugins/discover/public/utils/get_sharing_data.ts index ace169359e318..dfbdcc49e90a4 100644 --- a/src/plugins/discover/public/utils/get_sharing_data.ts +++ b/src/plugins/discover/public/utils/get_sharing_data.ts @@ -15,13 +15,14 @@ import type { } from '@kbn/data-plugin/public'; import type { Filter } from '@kbn/es-query'; import type { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public'; +import { AppState } from '../application/main/services/discover_app_state_container'; import { getSortForSearchSource } from './sorting'; import { DOC_HIDE_TIME_COLUMN_SETTING, SEARCH_FIELDS_FROM_SOURCE, SORT_DEFAULT_ORDER_SETTING, } from '../../common'; -import { AppState, isEqualFilters } from '../application/main/services/discover_state'; +import { isEqualFilters } from '../application/main/services/discover_state'; /** * Preparing data to share the current state as link or CSV/Report From 5e784c36ab08476ba2f9bc648c90b5f20fe9c415 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Thu, 3 Nov 2022 13:35:00 +0300 Subject: [PATCH 07/36] [Discover] make adhoc data view logic more explicit --- .../components/layout/discover_layout.tsx | 5 +- .../components/top_nav/discover_topnav.tsx | 11 +++- .../main/hooks/use_adhoc_data_views.test.tsx | 4 +- .../main/hooks/use_adhoc_data_views.ts | 58 +++++++------------ .../main/hooks/use_discover_state.ts | 3 +- .../discover_internal_state_container.ts | 21 ++++++- .../main/services/discover_state.ts | 25 +++++--- .../main/utils/update_filter_references.ts | 27 +++++++++ .../use_confirm_persistence_prompt.test.tsx | 12 +++- .../hooks/use_confirm_persistence_prompt.ts | 32 ++++++---- 10 files changed, 129 insertions(+), 69 deletions(-) create mode 100644 src/plugins/discover/public/application/main/utils/update_filter_references.ts 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 bbe8a65ec5b1a..72d313d5a1888 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 @@ -181,12 +181,15 @@ export function DiscoverLayout({ const contentCentered = resultState === 'uninitialized' || resultState === 'none'; const onDataViewCreated = useCallback( (nextDataView: DataView) => { + if (!nextDataView.isPersisted()) { + stateContainer.actions.appendAdHocDataView(nextDataView); + } if (nextDataView.id) { onChangeDataView(nextDataView.id); } savedSearchRefetch$.next('reset'); }, - [onChangeDataView, savedSearchRefetch$] + [onChangeDataView, savedSearchRefetch$, stateContainer] ); const savedSearchTitle = useRef(null); diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 9356a1d51e4df..9ba3bbb7619be 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -61,6 +61,8 @@ export const DiscoverTopNav = ({ const adHocDataViewList = useInternalStateSelector((state) => state.dataViewsAdHoc); const dataView = useInternalStateSelector((state) => state.dataView!); + // eslint-disable-next-line no-console + console.log('adHocDataViewList', adHocDataViewList); const showDatePicker = useMemo( () => dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP, [dataView] @@ -128,13 +130,16 @@ export const DiscoverTopNav = ({ const createNewDataView = useCallback(() => { closeDataViewEditor.current = dataViewEditor.openEditor({ onSave: async (dataViewToSave) => { + if (!dataViewToSave.isPersisted()) { + stateContainer.actions.appendAdHocDataView(dataViewToSave); + } if (dataViewToSave.id) { onChangeDataView(dataViewToSave.id); } }, allowAdHocDataView: true, }); - }, [dataViewEditor, onChangeDataView]); + }, [dataViewEditor, onChangeDataView, stateContainer.actions]); const onCreateDefaultAdHocDataView = useCallback( async (pattern: string) => { @@ -144,9 +149,11 @@ export const DiscoverTopNav = ({ if (newDataView.fields.getByName('@timestamp')?.type === 'date') { newDataView.timeFieldName = '@timestamp'; } + + stateContainer.actions.appendAdHocDataView(newDataView); onChangeDataView(newDataView.id!); }, - [dataViews, onChangeDataView] + [dataViews, onChangeDataView, stateContainer.actions] ); const topNavMenu = useMemo( diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx index c561cb13211a5..b015085c8cf2c 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx @@ -130,9 +130,7 @@ describe('useAdHocDataViews', () => { updatedDataView = await hook.result.current.updateAdHocDataViewId(mockDataView); }); - expect(dataViewsCreateMock).toHaveBeenCalledWith({ - id: undefined, - }); + expect(mockDiscoverServices.dataViews.clearInstanceCache).toHaveBeenCalledWith(mockDataView.id); expect(updatedDataView!.id).toEqual('updated-mock-id'); }); }); diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts index 8f4cfce6f903e..63004bdee47e6 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts @@ -7,19 +7,15 @@ */ import { useCallback } from 'react'; +import uuid from 'uuid/v4'; import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; -import { - UPDATE_FILTER_REFERENCES_ACTION, - UPDATE_FILTER_REFERENCES_TRIGGER, -} from '@kbn/unified-search-plugin/public'; -import { ActionExecutionContext } from '@kbn/ui-actions-plugin/public'; import type { FilterManager } from '@kbn/data-plugin/public'; import type { ToastsStart } from '@kbn/core-notifications-browser'; -import { getUiActions } from '../../../kibana_services'; import { useConfirmPersistencePrompt } from '../../../hooks/use_confirm_persistence_prompt'; import { DiscoverStateContainer } from '../services/discover_state'; import { useFiltersValidation } from './use_filters_validation'; +import { updateFiltersReferences } from '../utils/update_filter_references'; export const useAdHocDataViews = ({ savedSearch, @@ -46,50 +42,40 @@ export const useAdHocDataViews = ({ * This is to prevent duplicate ids messing with our system */ const updateAdHocDataViewId = useCallback( - async (dataViewToUpdate: DataView) => { - const adHocDataViewList = stateContainer.internalState.getState().dataViewsAdHoc; - const newDataView = await dataViews.create({ ...dataViewToUpdate.toSpec(), id: undefined }); + async (prevDataView: DataView) => { + const newDataView = await dataViews.create({ ...prevDataView.toSpec(), id: uuid() }); + dataViews.clearInstanceCache(prevDataView.id); - dataViews.clearInstanceCache(dataViewToUpdate.id); - const nextAdHocDataViewList = adHocDataViewList.filter( - (d: DataView) => d.id && dataViewToUpdate.id && d.id !== dataViewToUpdate.id - ); - stateContainer.internalState.transitions.setDataViewsAdHoc(nextAdHocDataViewList); + updateFiltersReferences(prevDataView, newDataView); - const uiActions = await getUiActions(); - const trigger = uiActions.getTrigger(UPDATE_FILTER_REFERENCES_TRIGGER); - const action = uiActions.getAction(UPDATE_FILTER_REFERENCES_ACTION); - - // execute shouldn't be awaited, this is important for pending history push cancellation - action?.execute({ - trigger, - fromDataView: dataViewToUpdate.id, - toDataView: newDataView.id, - usedDataViews: [], - } as ActionExecutionContext); - stateContainer.actions.setDataView(newDataView); + stateContainer.actions.replaceAdHocDataViewWithId(prevDataView.id!, newDataView); await stateContainer.replaceUrlAppState({ index: newDataView.id }); + setUrlTracking(newDataView); return newDataView; }, [dataViews, setUrlTracking, stateContainer] ); - const { openConfirmSavePrompt, updateSavedSearch } = - useConfirmPersistencePrompt(updateAdHocDataViewId); + const { openConfirmSavePrompt, updateSavedSearch } = useConfirmPersistencePrompt(stateContainer); const persistDataView = useCallback(async () => { const currentDataView = savedSearch.searchSource.getField('index')!; - if (currentDataView && !currentDataView.isPersisted()) { - const createdDataView = await openConfirmSavePrompt(currentDataView); + if (!currentDataView || currentDataView.isPersisted()) { + return currentDataView; + } + const createdDataView = await openConfirmSavePrompt(currentDataView); + if (!createdDataView) { + return currentDataView; // persistance cancelled + } + + if (savedSearch.id) { // update saved search with saved data view - if (createdDataView && savedSearch.id) { - const currentState = stateContainer.appState.getState(); - await updateSavedSearch({ savedSearch, dataView: createdDataView, state: currentState }); - } - return createdDataView; + const currentState = stateContainer.appState.getState(); + await updateSavedSearch({ savedSearch, dataView: createdDataView, state: currentState }); } - return currentDataView; + + return createdDataView; }, [stateContainer, openConfirmSavePrompt, savedSearch, updateSavedSearch]); return { persistDataView, updateAdHocDataViewId }; 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 49a7d7aa3254c..37935167d9d8b 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 @@ -63,7 +63,8 @@ export function useDiscoverState({ }); container.actions.setDataView(dataView); return container; - }, [history, savedSearch, services, dataView]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [history, savedSearch, services]); const { setUrlTracking } = useUrlTracking(savedSearch, dataView); diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index 6e42b1792fefa..6ac294cbccabd 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -20,7 +20,11 @@ export interface InternalState { interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; - setDataViewsAdHoc: (state: InternalState) => (dataViews: DataView[]) => InternalState; + appendAdHocDataView: (state: InternalState) => (dataView: DataView) => InternalState; + removeAdHocDataViewById: (state: InternalState) => (id: string) => InternalState; + replaceAdHocDataViewWithId: ( + state: InternalState + ) => (id: string, dataView: DataView) => InternalState; } export type InternalStateContainer = ReduxLikeStateContainer< @@ -42,10 +46,21 @@ export function getInternalStateContainer() { ...prevState, dataView: nextDataView, }), - setDataViewsAdHoc: (prevState: InternalState) => (dataViewsAdHoc: DataView[]) => ({ + appendAdHocDataView: (prevState: InternalState) => (dataViewAdHoc: DataView) => ({ ...prevState, - dataViewsAdHoc, + dataViewsAdHoc: prevState.dataViewsAdHoc.concat(dataViewAdHoc), }), + removeAdHocDataViewById: (prevState: InternalState) => (id: string) => ({ + ...prevState, + dataViewsAdHoc: prevState.dataViewsAdHoc.filter((dataView) => dataView.id !== id), + }), + replaceAdHocDataViewWithId: + (prevState: InternalState) => (prevId: string, newDataView: DataView) => ({ + ...prevState, + dataViewsAdHoc: prevState.dataViewsAdHoc.map((dataView) => + dataView.id === prevId ? newDataView : dataView + ), + }), }, {}, { freeze: (state) => state } diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 27bde15953e20..558f18c0cc1e7 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -128,6 +128,12 @@ export interface DiscoverStateContainer { * Set the currently selected data view */ setDataView: (dataView: DataView) => void; + /** + * Manage AdHoc data views + */ + appendAdHocDataView: (dataView: DataView) => void; + removeAdHocDataViewById: (id: string) => void; + replaceAdHocDataViewWithId: (id: string, dataView: DataView) => void; }; } @@ -210,16 +216,14 @@ export function getDiscoverStateContainer({ } }; - const setDataView = (dataView: DataView) => { + const setDataView = (dataView: DataView) => internalStateContainer.transitions.setDataView(dataView); - if (!dataView.isPersisted()) { - const adHocDataViewList = internalStateContainer.getState().dataViewsAdHoc; - const existing = adHocDataViewList.find((prevDataView) => prevDataView.id === dataView.id); - if (!existing) { - internalStateContainer.transitions.setDataViewsAdHoc([...adHocDataViewList, dataView]); - } - } - }; + const appendAdHocDataView = (dataView: DataView) => + internalStateContainer.transitions.appendAdHocDataView(dataView); + const replaceAdHocDataViewWithId = (id: string, dataView: DataView) => + internalStateContainer.transitions.replaceAdHocDataViewWithId(id, dataView); + const removeAdHocDataViewById = (id: string) => + internalStateContainer.transitions.removeAdHocDataViewById(id); return { kbnUrlStateStorage: stateStorage, @@ -302,6 +306,9 @@ export function getDiscoverStateContainer({ }, actions: { setDataView, + appendAdHocDataView, + replaceAdHocDataViewWithId, + removeAdHocDataViewById, }, }; } diff --git a/src/plugins/discover/public/application/main/utils/update_filter_references.ts b/src/plugins/discover/public/application/main/utils/update_filter_references.ts new file mode 100644 index 0000000000000..8ab92d37a76f5 --- /dev/null +++ b/src/plugins/discover/public/application/main/utils/update_filter_references.ts @@ -0,0 +1,27 @@ +/* + * 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 { + UPDATE_FILTER_REFERENCES_ACTION, + UPDATE_FILTER_REFERENCES_TRIGGER, +} from '@kbn/unified-search-plugin/public'; +import { ActionExecutionContext } from '@kbn/ui-actions-plugin/public'; +import type { DataView } from '@kbn/data-views-plugin/public'; +import { getUiActions } from '../../../kibana_services'; + +export const updateFiltersReferences = (prevDataView: DataView, nextDataView: DataView) => { + const uiActions = getUiActions(); + const trigger = uiActions.getTrigger(UPDATE_FILTER_REFERENCES_TRIGGER); + const action = uiActions.getAction(UPDATE_FILTER_REFERENCES_ACTION); + action?.execute({ + trigger, + fromDataView: prevDataView.id, + toDataView: nextDataView.id, + usedDataViews: [], + } as ActionExecutionContext); +}; diff --git a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.test.tsx b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.test.tsx index 33e2435644305..127d0b90d71c9 100644 --- a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.test.tsx +++ b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.test.tsx @@ -10,8 +10,11 @@ import React from 'react'; import type { DataView } from '@kbn/data-views-plugin/public'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { renderHook } from '@testing-library/react-hooks'; +import type { UiActionsStart } from '@kbn/ui-actions-plugin/public'; import { discoverServiceMock as mockDiscoverServices } from '../__mocks__/services'; import { useConfirmPersistencePrompt } from './use_confirm_persistence_prompt'; +import { getDiscoverStateMock } from '../__mocks__/discover_state.mock'; +import { setUiActions } from '../kibana_services'; jest.mock('./show_confirm_panel', () => { return { @@ -28,11 +31,16 @@ const mockDataView = { toSpec: () => ({}), } as DataView; +setUiActions({ + getTrigger: jest.fn(() => {}), + getAction: jest.fn(() => ({ execute: jest.fn() })), +} as unknown as UiActionsStart); + describe('useConfirmPersistencePrompt', () => { it('should save data view correctly', async () => { mockDiscoverServices.dataViews.createAndSave = jest.fn().mockResolvedValue(mockDataView); const hook = renderHook( - (d: DataView) => useConfirmPersistencePrompt(() => Promise.resolve(mockDataView)), + (d: DataView) => useConfirmPersistencePrompt(getDiscoverStateMock({})), { initialProps: mockDataView, wrapper: ({ children }) => ( @@ -52,7 +60,7 @@ describe('useConfirmPersistencePrompt', () => { .fn() .mockRejectedValue(new Error('failed to save')); const hook = renderHook( - (d: DataView) => useConfirmPersistencePrompt(() => Promise.resolve(mockDataView)), + (d: DataView) => useConfirmPersistencePrompt(getDiscoverStateMock({})), { initialProps: mockDataView, wrapper: ({ children }) => ( diff --git a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts index 3884bfd085a48..5332ffac6e0be 100644 --- a/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts +++ b/src/plugins/discover/public/hooks/use_confirm_persistence_prompt.ts @@ -7,31 +7,39 @@ */ import { useCallback } from 'react'; +import uuid from 'uuid/v4'; import { i18n } from '@kbn/i18n'; import type { DataView } from '@kbn/data-views-plugin/public'; import { SavedSearch } from '@kbn/saved-search-plugin/public'; import { useDiscoverServices } from './use_discover_services'; import { showConfirmPanel } from './show_confirm_panel'; import { persistSavedSearch } from '../application/main/utils/persist_saved_search'; +import { DiscoverStateContainer } from '../application/main/services/discover_state'; +import { updateFiltersReferences } from '../application/main/utils/update_filter_references'; -export const useConfirmPersistencePrompt = ( - updateAdHocDataViewId: (dataView: DataView) => Promise -) => { +export const useConfirmPersistencePrompt = (stateContainer: DiscoverStateContainer) => { const services = useDiscoverServices(); - const persistDataView: (dataView: DataView) => Promise = useCallback( - async (dataView) => { + const persistDataView: (adHocDataView: DataView) => Promise = useCallback( + async (adHocDataView) => { try { - const updatedDataView = await updateAdHocDataViewId(dataView); + const persistedDataView = await services.dataViews.createAndSave({ + ...adHocDataView.toSpec(), + id: uuid(), + }); + services.dataViews.clearInstanceCache(adHocDataView.id); + + updateFiltersReferences(adHocDataView, persistedDataView); - const response = await services.dataViews.createAndSave(updatedDataView.toSpec()); + stateContainer.actions.removeAdHocDataViewById(adHocDataView.id!); + await stateContainer.replaceUrlAppState({ index: persistedDataView.id }); const message = i18n.translate('discover.dataViewPersist.message', { defaultMessage: "Saved '{dataViewName}'", - values: { dataViewName: response.getName() }, + values: { dataViewName: persistedDataView.getName() }, }); services.toastNotifications.addSuccess(message); - return response; + return persistedDataView; } catch (error) { services.toastNotifications.addDanger({ title: i18n.translate('discover.dataViewPersistError.title', { @@ -42,7 +50,7 @@ export const useConfirmPersistencePrompt = ( throw new Error(error); } }, - [services.dataViews, services.toastNotifications, updateAdHocDataViewId] + [services.dataViews, services.toastNotifications, stateContainer] ); const openConfirmSavePrompt: (dataView: DataView) => Promise = useCallback( @@ -61,7 +69,7 @@ export const useConfirmPersistencePrompt = ( ); const onUpdateSuccess = useCallback( - (id: string, savedSearch: SavedSearch) => { + (savedSearch: SavedSearch) => { services.toastNotifications.addSuccess({ title: i18n.translate('discover.notifications.updateSavedSearchTitle', { defaultMessage: `Search '{savedSearchTitle}' updated with saved data view`, @@ -94,7 +102,7 @@ export const useConfirmPersistencePrompt = ( ({ savedSearch, dataView, state }) => { return persistSavedSearch(savedSearch, { dataView, - onSuccess: (id) => onUpdateSuccess(id, savedSearch), + onSuccess: () => onUpdateSuccess(savedSearch), onError: (error) => onUpdateError(error, savedSearch), state, saveOptions: {}, From badaf79e51415200702edf8158138633142249d2 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Thu, 3 Nov 2022 13:39:25 +0300 Subject: [PATCH 08/36] [Discover] remove log --- .../application/main/components/top_nav/discover_topnav.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 9ba3bbb7619be..67199ec191782 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -61,8 +61,6 @@ export const DiscoverTopNav = ({ const adHocDataViewList = useInternalStateSelector((state) => state.dataViewsAdHoc); const dataView = useInternalStateSelector((state) => state.dataView!); - // eslint-disable-next-line no-console - console.log('adHocDataViewList', adHocDataViewList); const showDatePicker = useMemo( () => dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP, [dataView] From a01df77a62dc138b4f59b1ec2ab48facb8ad13d1 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 4 Nov 2022 09:04:19 +0100 Subject: [PATCH 09/36] Rename dataViewsAdHoc to dataViewAdHocList --- .../components/top_nav/discover_topnav.tsx | 2 +- .../main/hooks/use_discover_state.ts | 3 +- .../discover_internal_state_container.ts | 10 +++--- .../main/services/discover_state.test.ts | 33 +++++++++++++++++++ .../main/services/discover_state.ts | 14 ++++++-- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 67199ec191782..eb33af42cf413 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -58,7 +58,7 @@ export const DiscoverTopNav = ({ updateAdHocDataViewId, }: DiscoverTopNavProps) => { const history = useHistory(); - const adHocDataViewList = useInternalStateSelector((state) => state.dataViewsAdHoc); + const adHocDataViewList = useInternalStateSelector((state) => state.dataViewAdHocList); const dataView = useInternalStateSelector((state) => state.dataView!); const showDatePicker = useMemo( 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 37935167d9d8b..2a92ec36e255d 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 @@ -61,9 +61,8 @@ export function useDiscoverState({ savedSearch, services, }); - container.actions.setDataView(dataView); + container.actions.setDataView(savedSearch.searchSource.getField('index')!); return container; - // eslint-disable-next-line react-hooks/exhaustive-deps }, [history, savedSearch, services]); const { setUrlTracking } = useUrlTracking(savedSearch, dataView); diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index 6ac294cbccabd..535d826fb2c14 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -15,7 +15,7 @@ import { DataView } from '@kbn/data-views-plugin/common'; export interface InternalState { dataView: DataView | undefined; - dataViewsAdHoc: DataView[]; + dataViewAdHocList: DataView[]; } interface InternalStateTransitions { @@ -39,7 +39,7 @@ export function getInternalStateContainer() { return createStateContainer( { dataView: undefined, - dataViewsAdHoc: [], + dataViewAdHocList: [], }, { setDataView: (prevState: InternalState) => (nextDataView: DataView) => ({ @@ -48,16 +48,16 @@ export function getInternalStateContainer() { }), appendAdHocDataView: (prevState: InternalState) => (dataViewAdHoc: DataView) => ({ ...prevState, - dataViewsAdHoc: prevState.dataViewsAdHoc.concat(dataViewAdHoc), + dataViewAdHocList: prevState.dataViewAdHocList.concat(dataViewAdHoc), }), removeAdHocDataViewById: (prevState: InternalState) => (id: string) => ({ ...prevState, - dataViewsAdHoc: prevState.dataViewsAdHoc.filter((dataView) => dataView.id !== id), + dataViewAdHocList: prevState.dataViewAdHocList.filter((dataView) => dataView.id !== id), }), replaceAdHocDataViewWithId: (prevState: InternalState) => (prevId: string, newDataView: DataView) => ({ ...prevState, - dataViewsAdHoc: prevState.dataViewsAdHoc.map((dataView) => + dataViewAdHocList: prevState.dataViewAdHocList.map((dataView) => dataView.id === prevId ? newDataView : dataView ), }), diff --git a/src/plugins/discover/public/application/main/services/discover_state.test.ts b/src/plugins/discover/public/application/main/services/discover_state.test.ts index c29da2897bcee..f4eed5c33863e 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.test.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.test.ts @@ -16,6 +16,8 @@ import { dataPluginMock } from '@kbn/data-plugin/public/mocks'; import type { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public'; import { savedSearchMock, savedSearchMockWithTimeField } from '../../../__mocks__/saved_search'; import { discoverServiceMock } from '../../../__mocks__/services'; +import { dataViewMock } from '../../../__mocks__/data_view'; +import { dataViewComplexMock } from '../../../__mocks__/data_view_complex'; let history: History; let state: DiscoverStateContainer; @@ -216,4 +218,35 @@ describe('createSearchSessionRestorationDataProvider', () => { }); }); }); + + describe('actions', () => { + beforeEach(async () => { + history = createBrowserHistory(); + state = getDiscoverStateContainer({ + services: discoverServiceMock, + history, + savedSearch: savedSearchMock, + }); + }); + + test('setDataView', async () => { + state.actions.setDataView(dataViewMock); + expect(state.internalState.getState().dataView).toBe(dataViewMock); + }); + + test('appendAdHocDataView', async () => { + state.actions.appendAdHocDataView(dataViewMock); + expect(state.internalState.getState().dataViewAdHocList).toEqual([dataViewMock]); + }); + test('removeAdHocDataViewById', async () => { + state.actions.appendAdHocDataView(dataViewMock); + state.actions.removeAdHocDataViewById(dataViewMock.id!); + expect(state.internalState.getState().dataViewAdHocList).toEqual([]); + }); + test('replaceAdHocDataViewWithId', async () => { + state.actions.appendAdHocDataView(dataViewMock); + state.actions.replaceAdHocDataViewWithId(dataViewMock.id!, dataViewComplexMock); + expect(state.internalState.getState().dataViewAdHocList).toEqual([dataViewComplexMock]); + }); + }); }); diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 558f18c0cc1e7..51ac8d1408150 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -52,7 +52,7 @@ interface DiscoverStateContainerParams { /** * Browser history */ - history: History; + history?: History; /** * The current savedSearch */ @@ -129,10 +129,20 @@ export interface DiscoverStateContainer { */ setDataView: (dataView: DataView) => void; /** - * Manage AdHoc data views + * Append a given ad-hoc data view to the list of ad-hoc data view */ appendAdHocDataView: (dataView: DataView) => void; + /** + * Remove the ad-hoc data view of the given id from the list of ad-hoc data view + * @param id + */ removeAdHocDataViewById: (id: string) => void; + /** + * Replace the data view of the given id with the given data view + * Used when the spec of a data view changed to prevent duplicates + * @param id + * @param dataView + */ replaceAdHocDataViewWithId: (id: string, dataView: DataView) => void; }; } From ad88bff3ed43dd2606d25019befd3a2f7d518195 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 4 Nov 2022 13:02:27 +0100 Subject: [PATCH 10/36] Fix context_app.tsx --- .../discover/public/application/context/context_app.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/context/context_app.tsx b/src/plugins/discover/public/application/context/context_app.tsx index 7fc2d60fe1040..efd8992e48ca8 100644 --- a/src/plugins/discover/public/application/context/context_app.tsx +++ b/src/plugins/discover/public/application/context/context_app.tsx @@ -66,9 +66,10 @@ export const ContextApp = ({ dataView, anchorId, referrer }: ContextAppProps) => config: uiSettings, dataView, dataViews, - state: appState, useNewFieldsApi, setAppState: stateContainer.setAppState, + columns: appState.columns, + sort: appState.sort, }); useEffect(() => { From fc45043fa7a28c1298c834e402a939d3c20c99dc Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Thu, 17 Nov 2022 16:56:40 +0300 Subject: [PATCH 11/36] [Discover] fix linting --- src/plugins/unified_search/public/dataview_picker/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/unified_search/public/dataview_picker/index.tsx b/src/plugins/unified_search/public/dataview_picker/index.tsx index 1321ae8b04cd2..46010d9b6d41f 100644 --- a/src/plugins/unified_search/public/dataview_picker/index.tsx +++ b/src/plugins/unified_search/public/dataview_picker/index.tsx @@ -8,7 +8,7 @@ import React from 'react'; import type { EuiButtonProps, EuiSelectableProps } from '@elastic/eui'; -import type { DataView, DataViewListItem } from '@kbn/data-views-plugin/public'; +import type { DataView } from '@kbn/data-views-plugin/public'; import type { AggregateQuery, Query } from '@kbn/es-query'; import { ChangeDataView } from './change_dataview'; From 9914d6fe1027ee0765db26d3a5d26a20de7a611c Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Mon, 12 Dec 2022 17:14:31 +0300 Subject: [PATCH 12/36] [Discover] remove redundant comment --- .../application/main/hooks/use_discover_state.ts | 13 ------------- 1 file changed, 13 deletions(-) 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 95a9eaf135cac..f7996ff4e04a1 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 @@ -137,19 +137,6 @@ export function useDiscoverState({ trackUiMetric, }); - // const [savedDataViewList, setSavedDataViewList] = useState(initialDataViewList); - - // /** - // * Updates data views selector state - // */ - // const updateDataViewList = useCallback( - // async (newAdHocDataViews: DataView[]) => { - // setSavedDataViewList(await data.dataViews.getIdsWithTitle()); - // onAddAdHocDataViews(newAdHocDataViews); - // }, - // [data.dataViews, onAddAdHocDataViews] - // ); - /** * Data fetching logic */ From 80bb56c59a624da4289489481c2e9a86752d5c27 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 12 Dec 2022 22:12:50 +0100 Subject: [PATCH 13/36] Fix functional - with the recent change the expected number of changes 0 is correct --- test/functional/apps/discover/group2/_search_on_page_load.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/apps/discover/group2/_search_on_page_load.ts b/test/functional/apps/discover/group2/_search_on_page_load.ts index cb2b21e3849db..2f3ad427335cd 100644 --- a/test/functional/apps/discover/group2/_search_on_page_load.ts +++ b/test/functional/apps/discover/group2/_search_on_page_load.ts @@ -35,6 +35,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const waitForFetches = (fetchesNumber: number) => async () => { const nrOfFetches = await PageObjects.discover.getNrOfFetches(); + log.debug('actual number of fetches', nrOfFetches); return nrOfFetches === fetchesNumber; }; @@ -144,7 +145,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await testSubjects.click('discoverNewButton'); await PageObjects.header.waitUntilLoadingHasFinished(); - await retry.waitFor('number of fetches to be 1', waitForFetches(1)); + await retry.waitFor('number of fetches to be 0', waitForFetches(0)); expect(await PageObjects.discover.doesSidebarShowFields()).to.be(false); }); }); From 9ddf6be901042649ff183bac8508bace08551a0e Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Tue, 13 Dec 2022 12:18:15 +0300 Subject: [PATCH 14/36] [Discover] return savedDataViews list --- .../components/layout/discover_layout.tsx | 4 ++++ .../main/components/layout/types.ts | 4 +++- .../top_nav/discover_topnav.test.tsx | 2 ++ .../components/top_nav/discover_topnav.tsx | 24 +++++++------------ .../application/main/discover_main_app.tsx | 4 ++++ .../main/hooks/use_discover_state.ts | 21 +++++++++++++--- .../dataview_picker/change_dataview.tsx | 4 ++-- .../public/dataview_picker/index.tsx | 10 ++++---- 8 files changed, 46 insertions(+), 27 deletions(-) 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 d90591c7879ba..8a3ced1b69f6c 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 @@ -71,6 +71,8 @@ export function DiscoverLayout({ persistDataView, updateAdHocDataViewId, searchSessionManager, + savedDataViewList, + updateDataViewList, }: DiscoverLayoutProps) { const { trackUiMetric, @@ -311,6 +313,8 @@ export function DiscoverLayout({ onFieldEdited={onFieldEdited} persistDataView={persistDataView} updateAdHocDataViewId={updateAdHocDataViewId} + savedDataViewList={savedDataViewList} + updateDataViewList={updateDataViewList} /> Promise; updateAdHocDataViewId: (dataView: DataView) => Promise; searchSessionManager: DiscoverSearchSessionManager; + savedDataViewList: DataViewListItem[]; + updateDataViewList: (newAdHocDataViews: DataView[]) => void; } diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx index 3cf331b4d52b1..0d53dc4adc346 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx @@ -48,6 +48,8 @@ function getProps(savePermissions = true): DiscoverTopNavProps { isPlainRecord: false, persistDataView: jest.fn(), updateAdHocDataViewId: jest.fn(), + savedDataViewList: [], + updateDataViewList: jest.fn(), }; } diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 3e3d8c1796875..21e6bb5e58115 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -5,10 +5,10 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef } from 'react'; import { useHistory } from 'react-router-dom'; import type { Query, TimeRange, AggregateQuery } from '@kbn/es-query'; -import { DataViewType, type DataView } from '@kbn/data-views-plugin/public'; +import { type DataViewListItem, DataViewType, type DataView } from '@kbn/data-views-plugin/public'; import type { DataViewPickerProps } from '@kbn/unified-search-plugin/public'; import { useInternalStateSelector } from '../../services/discover_internal_state_container'; import { ENABLE_SQL } from '../../../../../common'; @@ -38,6 +38,8 @@ export type DiscoverTopNavProps = Pick< onFieldEdited: () => Promise; persistDataView: (dataView: DataView) => Promise; updateAdHocDataViewId: (dataView: DataView) => Promise; + savedDataViewList: DataViewListItem[]; + updateDataViewList: (newAdHocDataViews: DataView[]) => void; }; export const DiscoverTopNav = ({ @@ -56,11 +58,12 @@ export const DiscoverTopNav = ({ onFieldEdited, persistDataView, updateAdHocDataViewId, + savedDataViewList, + updateDataViewList, }: DiscoverTopNavProps) => { const history = useHistory(); const adHocDataViewList = useInternalStateSelector((state) => state.dataViewAdHocList); const dataView = useInternalStateSelector((state) => state.dataView!); - const [refreshListId, setRefreshListId] = useState(0); const showDatePicker = useMemo( () => dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP, @@ -140,17 +143,6 @@ export const DiscoverTopNav = ({ }); }, [dataViewEditor, onChangeDataView, stateContainer.actions]); - /** - * Updates data views selector state - */ - const updateDataViewList = useCallback( - (newAdHocDataViews: DataView[]) => { - stateContainer.actions.setAdHocDataViews(newAdHocDataViews); - setRefreshListId((prev) => prev + 1); - }, - [stateContainer.actions] - ); - const onCreateDefaultAdHocDataView = useCallback( async (pattern: string) => { const newDataView = await dataViews.create({ @@ -221,7 +213,7 @@ export const DiscoverTopNav = ({ if (isSQLModeEnabled) { supportedTextBasedLanguages.push('SQL'); } - const dataViewPickerProps = { + const dataViewPickerProps: DataViewPickerProps = { trigger: { label: dataView?.getName() || '', 'data-test-subj': 'discover-dataView-switch-link', @@ -234,7 +226,7 @@ export const DiscoverTopNav = ({ onChangeDataView, textBasedLanguages: supportedTextBasedLanguages as DataViewPickerProps['textBasedLanguages'], adHocDataViews: adHocDataViewList, - refreshListId, + savedDataViews: savedDataViewList, }; const onTextBasedSavedAndExit = useCallback( 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 6d7712612da73..f0e3101125c31 100644 --- a/src/plugins/discover/public/application/main/discover_main_app.tsx +++ b/src/plugins/discover/public/application/main/discover_main_app.tsx @@ -60,6 +60,8 @@ export function DiscoverMainApp(props: DiscoverMainProps) { searchSource, stateContainer, searchSessionManager, + savedDataViewList, + updateDataViewList, } = useDiscoverState({ services, history: usedHistory, @@ -116,6 +118,8 @@ export function DiscoverMainApp(props: DiscoverMainProps) { persistDataView={persistDataView} updateAdHocDataViewId={updateAdHocDataViewId} searchSessionManager={searchSessionManager} + savedDataViewList={savedDataViewList} + updateDataViewList={updateDataViewList} /> ); 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 f7996ff4e04a1..d047d75c23629 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 @@ -8,7 +8,7 @@ import { useMemo, useEffect, useState, useCallback } from 'react'; import { isEqual } from 'lodash'; import { History } from 'history'; -import { type DataViewListItem, DataViewType } from '@kbn/data-views-plugin/public'; +import { type DataViewListItem, type DataView, DataViewType } from '@kbn/data-views-plugin/public'; import { SavedSearch, getSavedSearch } from '@kbn/saved-search-plugin/public'; import type { SortOrder } from '@kbn/saved-search-plugin/public'; import { useTextBasedQueryLanguage } from './use_text_based_query_language'; @@ -36,7 +36,7 @@ export function useDiscoverState({ history, savedSearch, setExpandedDoc, - dataViewList, + dataViewList: initialDataViewList, }: { services: DiscoverServices; savedSearch: SavedSearch; @@ -137,6 +137,19 @@ export function useDiscoverState({ trackUiMetric, }); + const [savedDataViewList, setSavedDataViewList] = useState(initialDataViewList); + + /** + * Updates data views selector state + */ + const updateDataViewList = useCallback( + async (newAdHocDataViews: DataView[]) => { + setSavedDataViewList(await data.dataViews.getIdsWithTitle()); + stateContainer.actions.setAdHocDataViews(newAdHocDataViews); + }, + [data.dataViews, stateContainer.actions] + ); + /** * Data fetching logic */ @@ -156,7 +169,7 @@ export function useDiscoverState({ documents$: data$.documents$, dataViews, stateContainer, - dataViewList, + dataViewList: initialDataViewList, savedSearch, }); @@ -317,5 +330,7 @@ export function useDiscoverState({ persistDataView, updateAdHocDataViewId, searchSessionManager, + savedDataViewList, + updateDataViewList, }; } diff --git a/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx b/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx index d0d1af1adae89..96980e8db184d 100644 --- a/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx +++ b/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx @@ -62,7 +62,7 @@ export function ChangeDataView({ isMissingCurrent, currentDataViewId, adHocDataViews, - refreshListId, + savedDataViews, onChangeDataView, onAddField, onDataViewCreated, @@ -114,7 +114,7 @@ export function ChangeDataView({ setDataViewsList(dataViewsRefs); }; fetchDataViews(); - }, [data, currentDataViewId, adHocDataViews, refreshListId]); + }, [data, currentDataViewId, adHocDataViews, savedDataViews]); useEffect(() => { if (trigger.label) { diff --git a/src/plugins/unified_search/public/dataview_picker/index.tsx b/src/plugins/unified_search/public/dataview_picker/index.tsx index 46010d9b6d41f..9cb50d5e85a79 100644 --- a/src/plugins/unified_search/public/dataview_picker/index.tsx +++ b/src/plugins/unified_search/public/dataview_picker/index.tsx @@ -8,7 +8,7 @@ import React from 'react'; import type { EuiButtonProps, EuiSelectableProps } from '@elastic/eui'; -import type { DataView } from '@kbn/data-views-plugin/public'; +import type { DataView, DataViewListItem } from '@kbn/data-views-plugin/public'; import type { AggregateQuery, Query } from '@kbn/es-query'; import { ChangeDataView } from './change_dataview'; @@ -55,9 +55,9 @@ export interface DataViewPickerProps { */ adHocDataViews?: DataView[]; /** - * Refreshes the list of dataviews. + * Saved data views */ - refreshListId?: number; + savedDataViews?: DataViewListItem[]; /** * EuiSelectable properties. */ @@ -106,7 +106,7 @@ export const DataViewPicker = ({ isMissingCurrent, currentDataViewId, adHocDataViews, - refreshListId, + savedDataViews, onChangeDataView, onEditDataView, onAddField, @@ -131,7 +131,7 @@ export const DataViewPicker = ({ onCreateDefaultAdHocDataView={onCreateDefaultAdHocDataView} trigger={trigger} adHocDataViews={adHocDataViews} - refreshListId={refreshListId} + savedDataViews={savedDataViews} selectableProps={selectableProps} textBasedLanguages={textBasedLanguages} onSaveTextLanguageQuery={onSaveTextLanguageQuery} From 2fb845b068f24009453f09517edf1c548750a68e Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 14 Dec 2022 09:01:29 +0100 Subject: [PATCH 15/36] migrate dataViewList to internal state container --- .../components/layout/discover_layout.tsx | 7 ++++--- .../main/components/layout/types.ts | 3 +-- .../top_nav/discover_topnav.test.tsx | 2 +- .../components/top_nav/discover_topnav.tsx | 19 ++++++------------- .../application/main/discover_main_app.tsx | 10 ++++++++-- .../main/hooks/use_discover_state.ts | 7 ++----- .../discover_internal_state_container.ts | 9 ++++++++- .../main/services/discover_state.ts | 10 ++++++++++ 8 files changed, 40 insertions(+), 27 deletions(-) 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 8a3ced1b69f6c..6d7068d933e7e 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 @@ -71,7 +71,6 @@ export function DiscoverLayout({ persistDataView, updateAdHocDataViewId, searchSessionManager, - savedDataViewList, updateDataViewList, }: DiscoverLayoutProps) { const { @@ -183,9 +182,11 @@ export function DiscoverLayout({ const contentCentered = resultState === 'uninitialized' || resultState === 'none'; const onDataViewCreated = useCallback( - (nextDataView: DataView) => { + async (nextDataView: DataView) => { if (!nextDataView.isPersisted()) { stateContainer.actions.appendAdHocDataViews(nextDataView); + } else { + await stateContainer.actions.loadDataViewList(); } if (nextDataView.id) { onChangeDataView(nextDataView.id); @@ -308,12 +309,12 @@ export function DiscoverLayout({ updateQuery={onUpdateQuery} resetSavedSearch={resetSavedSearch} onChangeDataView={onChangeDataView} + onDataViewCreated={onDataViewCreated} isPlainRecord={isPlainRecord} textBasedLanguageModeErrors={textBasedLanguageModeErrors} onFieldEdited={onFieldEdited} persistDataView={persistDataView} updateAdHocDataViewId={updateAdHocDataViewId} - savedDataViewList={savedDataViewList} updateDataViewList={updateDataViewList} /> 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 a97dcef00a9f8..748fd1265ac90 100644 --- a/src/plugins/discover/public/application/main/components/layout/types.ts +++ b/src/plugins/discover/public/application/main/components/layout/types.ts @@ -7,7 +7,7 @@ */ import type { Query, TimeRange, AggregateQuery } from '@kbn/es-query'; -import type { DataView, DataViewListItem } from '@kbn/data-views-plugin/public'; +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 { RequestAdapter } from '@kbn/inspector-plugin/public'; @@ -35,6 +35,5 @@ export interface DiscoverLayoutProps { persistDataView: (dataView: DataView) => Promise; updateAdHocDataViewId: (dataView: DataView) => Promise; searchSessionManager: DiscoverSearchSessionManager; - savedDataViewList: DataViewListItem[]; updateDataViewList: (newAdHocDataViews: DataView[]) => void; } diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx index 0d53dc4adc346..85c7e381588ea 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.test.tsx @@ -48,8 +48,8 @@ function getProps(savePermissions = true): DiscoverTopNavProps { isPlainRecord: false, persistDataView: jest.fn(), updateAdHocDataViewId: jest.fn(), - savedDataViewList: [], updateDataViewList: jest.fn(), + onDataViewCreated: jest.fn(), }; } diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 21e6bb5e58115..7f16de2631de6 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -8,7 +8,7 @@ import React, { useCallback, useEffect, useMemo, useRef } from 'react'; import { useHistory } from 'react-router-dom'; import type { Query, TimeRange, AggregateQuery } from '@kbn/es-query'; -import { type DataViewListItem, DataViewType, type DataView } from '@kbn/data-views-plugin/public'; +import { DataViewType, type DataView } from '@kbn/data-views-plugin/public'; import type { DataViewPickerProps } from '@kbn/unified-search-plugin/public'; import { useInternalStateSelector } from '../../services/discover_internal_state_container'; import { ENABLE_SQL } from '../../../../../common'; @@ -33,12 +33,12 @@ export type DiscoverTopNavProps = Pick< stateContainer: DiscoverStateContainer; resetSavedSearch: () => void; onChangeDataView: (dataView: string) => void; + onDataViewCreated: (dataView: DataView) => void; isPlainRecord: boolean; textBasedLanguageModeErrors?: Error; onFieldEdited: () => Promise; persistDataView: (dataView: DataView) => Promise; updateAdHocDataViewId: (dataView: DataView) => Promise; - savedDataViewList: DataViewListItem[]; updateDataViewList: (newAdHocDataViews: DataView[]) => void; }; @@ -53,18 +53,18 @@ export const DiscoverTopNav = ({ savedSearch, resetSavedSearch, onChangeDataView, + onDataViewCreated, isPlainRecord, textBasedLanguageModeErrors, onFieldEdited, persistDataView, updateAdHocDataViewId, - savedDataViewList, updateDataViewList, }: DiscoverTopNavProps) => { const history = useHistory(); const adHocDataViewList = useInternalStateSelector((state) => state.dataViewAdHocList); const dataView = useInternalStateSelector((state) => state.dataView!); - + const savedDataViewList = useInternalStateSelector((state) => state.dataViewList); const showDatePicker = useMemo( () => dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP, [dataView] @@ -131,17 +131,10 @@ export const DiscoverTopNav = ({ const createNewDataView = useCallback(() => { closeDataViewEditor.current = dataViewEditor.openEditor({ - onSave: async (dataViewToSave) => { - if (!dataViewToSave.isPersisted()) { - stateContainer.actions.appendAdHocDataViews(dataViewToSave); - } - if (dataViewToSave.id) { - onChangeDataView(dataViewToSave.id); - } - }, + onSave: onDataViewCreated, allowAdHocDataView: true, }); - }, [dataViewEditor, onChangeDataView, stateContainer.actions]); + }, [dataViewEditor, onDataViewCreated]); const onCreateDefaultAdHocDataView = useCallback( async (pattern: string) => { 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 f0e3101125c31..972ec6469d552 100644 --- a/src/plugins/discover/public/application/main/discover_main_app.tsx +++ b/src/plugins/discover/public/application/main/discover_main_app.tsx @@ -60,7 +60,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) { searchSource, stateContainer, searchSessionManager, - savedDataViewList, updateDataViewList, } = useDiscoverState({ services, @@ -94,6 +93,14 @@ export function DiscoverMainApp(props: DiscoverMainProps) { addHelpMenuToAppChrome(chrome, docLinks); }, [stateContainer, chrome, docLinks]); + /** + * Set initial data view list + * Can be removed once the state container work was completed + */ + useEffect(() => { + stateContainer.internalState.transitions.setDataViewList(dataViewList); + }, [stateContainer, dataViewList]); + const resetCurrentSavedSearch = useCallback(() => { resetSavedSearch(savedSearch.id); }, [resetSavedSearch, savedSearch]); @@ -118,7 +125,6 @@ export function DiscoverMainApp(props: DiscoverMainProps) { persistDataView={persistDataView} updateAdHocDataViewId={updateAdHocDataViewId} searchSessionManager={searchSessionManager} - savedDataViewList={savedDataViewList} updateDataViewList={updateDataViewList} /> 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 d047d75c23629..bec5392169ef6 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 @@ -137,17 +137,15 @@ export function useDiscoverState({ trackUiMetric, }); - const [savedDataViewList, setSavedDataViewList] = useState(initialDataViewList); - /** * Updates data views selector state */ const updateDataViewList = useCallback( async (newAdHocDataViews: DataView[]) => { - setSavedDataViewList(await data.dataViews.getIdsWithTitle()); + await stateContainer.actions.loadDataViewList(); stateContainer.actions.setAdHocDataViews(newAdHocDataViews); }, - [data.dataViews, stateContainer.actions] + [stateContainer.actions] ); /** @@ -330,7 +328,6 @@ export function useDiscoverState({ persistDataView, updateAdHocDataViewId, searchSessionManager, - savedDataViewList, updateDataViewList, }; } diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index 5b8d933750127..95bfafc739589 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -11,15 +11,17 @@ import { createStateContainerReactHelpers, ReduxLikeStateContainer, } from '@kbn/kibana-utils-plugin/common'; -import { DataView } from '@kbn/data-views-plugin/common'; +import { DataView, DataViewListItem } from '@kbn/data-views-plugin/common'; export interface InternalState { dataView: DataView | undefined; + dataViewList: DataViewListItem[]; dataViewAdHocList: DataView[]; } interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; + setDataViewList: (state: InternalState) => (dataView: DataViewListItem[]) => InternalState; setAdHocDataViews: (state: InternalState) => (dataViews: DataView[]) => InternalState; appendAdHocDataViews: ( state: InternalState @@ -43,12 +45,17 @@ export function getInternalStateContainer() { { dataView: undefined, dataViewAdHocList: [], + dataViewList: [], }, { setDataView: (prevState: InternalState) => (nextDataView: DataView) => ({ ...prevState, dataView: nextDataView, }), + setDataViewList: (prevState: InternalState) => (nextDataViewList: DataViewListItem[]) => ({ + ...prevState, + dataViewList: nextDataViewList, + }), setAdHocDataViews: (prevState: InternalState) => (newAdHocDataViewList: DataView[]) => ({ ...prevState, dataViewAdHocList: newAdHocDataViewList, diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index e7b7b8dcb9e42..1a0048a2860b4 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -128,6 +128,10 @@ export interface DiscoverStateContainer { * Set the currently selected data view */ setDataView: (dataView: DataView) => void; + /** + * Load current list of data views, add them to internal state + */ + loadDataViewList: () => Promise; /** * Set new adhoc data view list */ @@ -241,6 +245,11 @@ export function getDiscoverStateContainer({ const removeAdHocDataViewById = (id: string) => internalStateContainer.transitions.removeAdHocDataViewById(id); + const loadDataViewList = async () => { + const dataViewList = await services.dataViews.getIdsWithTitle(true); + internalStateContainer.transitions.setDataViewList(dataViewList); + }; + return { kbnUrlStateStorage: stateStorage, appState: appStateContainerModified, @@ -322,6 +331,7 @@ export function getDiscoverStateContainer({ }, actions: { setDataView, + loadDataViewList, setAdHocDataViews, appendAdHocDataViews, replaceAdHocDataViewWithId, From 16f92b3c8d69a2042ae988010ae2c274bddac14d Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 14 Dec 2022 13:36:01 +0100 Subject: [PATCH 16/36] undo change_dataview.tsx --- .../unified_search/public/dataview_picker/change_dataview.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx b/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx index 96980e8db184d..e218f27dadc55 100644 --- a/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx +++ b/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx @@ -98,7 +98,9 @@ export function ChangeDataView({ useEffect(() => { const fetchDataViews = async () => { - const dataViewsRefs: DataViewListItemEnhanced[] = await data.dataViews.getIdsWithTitle(); + const dataViewsRefs: DataViewListItemEnhanced[] = savedDataViews + ? savedDataViews + : await data.dataViews.getIdsWithTitle(); if (adHocDataViews?.length) { adHocDataViews.forEach((adHocDataView) => { if (adHocDataView.id) { From f49e221b8ddb71efa917b6d512724829c627c1a7 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 14 Dec 2022 17:28:47 +0100 Subject: [PATCH 17/36] remove unnecessary refetch function usage --- .../application/main/components/layout/discover_layout.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 6d7068d933e7e..56ef50615b256 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 @@ -191,9 +191,8 @@ export function DiscoverLayout({ if (nextDataView.id) { onChangeDataView(nextDataView.id); } - savedSearchRefetch$.next('reset'); }, - [onChangeDataView, savedSearchRefetch$, stateContainer] + [onChangeDataView, stateContainer] ); const savedSearchTitle = useRef(null); From 9b8e176c3c42241ae2bb9b39c1f3d4f26a22b3a3 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 15 Dec 2022 08:13:04 +0100 Subject: [PATCH 18/36] address review comments --- .../main/components/layout/discover_main_content.tsx | 3 +-- .../public/application/main/components/layout/types.ts | 4 ++-- .../public/application/main/hooks/use_discover_state.ts | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx b/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx index 6a8adb106fffa..b085e61f348da 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_main_content.tsx @@ -52,8 +52,7 @@ export const DiscoverMainContent = ({ stateContainer, savedSearch, }: DiscoverMainContentProps) => { - const services = useDiscoverServices(); - const { trackUiMetric } = services; + const { trackUiMetric } = useDiscoverServices(); const setDiscoverViewMode = useCallback( (mode: VIEW_MODE) => { 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 748fd1265ac90..f2a8ebe9269e8 100644 --- a/src/plugins/discover/public/application/main/components/layout/types.ts +++ b/src/plugins/discover/public/application/main/components/layout/types.ts @@ -10,14 +10,14 @@ 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 { RequestAdapter } from '@kbn/inspector-plugin/public'; import { DataTableRecord } from '../../../../types'; import { DiscoverStateContainer } from '../../services/discover_state'; import { DataRefetch$, SavedSearchData } from '../../hooks/use_saved_search'; import type { DiscoverSearchSessionManager } from '../../services/discover_search_session'; +import type { InspectorAdapters } from '../../hooks/use_inspector'; export interface DiscoverLayoutProps { - inspectorAdapters: { requests: RequestAdapter }; + inspectorAdapters: InspectorAdapters; navigateTo: (url: string) => void; onChangeDataView: (id: string) => void; onUpdateQuery: ( 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 bec5392169ef6..d2e5d7d32142f 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 @@ -36,7 +36,7 @@ export function useDiscoverState({ history, savedSearch, setExpandedDoc, - dataViewList: initialDataViewList, + dataViewList, }: { services: DiscoverServices; savedSearch: SavedSearch; @@ -167,7 +167,7 @@ export function useDiscoverState({ documents$: data$.documents$, dataViews, stateContainer, - dataViewList: initialDataViewList, + dataViewList, savedSearch, }); From 4bbb236c9074b484f860f490aa77148b10eb3a09 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 15 Dec 2022 22:44:30 +0100 Subject: [PATCH 19/36] Fix change data view issue --- .../components/layout/discover_documents.tsx | 26 +++++-- .../main/hooks/use_discover_state.ts | 70 ++++++++++--------- .../main/services/discover_state.ts | 3 +- 3 files changed, 59 insertions(+), 40 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx index 5879b5e7b9160..04a1c36164379 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx @@ -78,9 +78,19 @@ function DiscoverDocumentsComponent({ onFieldEdited?: () => void; }) { const { capabilities, dataViews, uiSettings } = useDiscoverServices(); - const [query, sort, rowHeight, rowsPerPage, grid, columns] = useAppStateSelector((state) => { - return [state.query, state.sort, state.rowHeight, state.rowsPerPage, state.grid, state.columns]; - }); + const [query, sort, rowHeight, rowsPerPage, grid, columns, index] = useAppStateSelector( + (state) => { + return [ + state.query, + state.sort, + state.rowHeight, + state.rowsPerPage, + state.grid, + state.columns, + state.index, + ]; + } + ); const useNewFieldsApi = useMemo(() => !uiSettings.get(SEARCH_FIELDS_FROM_SOURCE), [uiSettings]); const hideAnnouncements = useMemo(() => uiSettings.get(HIDE_ANNOUNCEMENTS), [uiSettings]); @@ -89,6 +99,11 @@ function DiscoverDocumentsComponent({ const documentState = useDataState(documents$); const isLoading = documentState.fetchStatus === FetchStatus.LOADING; + // this is needed to prevent data view pushing onSort because there has been a state change with a new data view + // but the data view in internal state was not set. due to the change of the timefield, this can lead to a change + // of state, EuiDataGrid pushing an onSort event, because the next timefield is already used a sorting param + // but it's not an available column + const dataViewChanged = index !== dataView.id; const isPlainRecord = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]); const rows = useMemo(() => documentState.result || [], [documentState.result]); @@ -144,8 +159,9 @@ function DiscoverDocumentsComponent({ ); if ( - (!documentState.result || documentState.result.length === 0) && - documentState.fetchStatus === FetchStatus.LOADING + dataViewChanged || + ((!documentState.result || documentState.result.length === 0) && + documentState.fetchStatus === FetchStatus.LOADING) ) { return (
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 d2e5d7d32142f..43f574c25b42f 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 @@ -88,40 +88,6 @@ export function useDiscoverState({ return shouldSearchOnPageLoad ? FetchStatus.LOADING : FetchStatus.UNINITIALIZED; }, [uiSettings, savedSearch.id, searchSessionManager, timefilter]); - /** - * Function triggered when user changes data view in the sidebar - */ - const onChangeDataView = useCallback( - async (id: string) => { - const nextDataView = await dataViews.get(id); - if (nextDataView && dataView) { - const nextAppState = getDataViewAppState( - dataView, - nextDataView, - state.columns || [], - (state.sort || []) as SortOrder[], - uiSettings.get(MODIFY_COLUMNS_ON_SWITCH), - uiSettings.get(SORT_DEFAULT_ORDER_SETTING), - state.query - ); - setUrlTracking(nextDataView); - stateContainer.setAppState(nextAppState); - } - setExpandedDoc(undefined); - }, - [ - setUrlTracking, - uiSettings, - dataView, - dataViews, - setExpandedDoc, - state.columns, - state.query, - state.sort, - stateContainer, - ] - ); - /** * Adhoc data views functionality */ @@ -254,6 +220,42 @@ export function useDiscoverState({ stateContainer, ]); + /** + * Function triggered when user changes data view in the sidebar + */ + const onChangeDataView = useCallback( + async (id: string) => { + const nextDataView = await dataViews.get(id); + if (nextDataView && dataView) { + reset(); + const nextAppState = getDataViewAppState( + dataView, + nextDataView, + state.columns || [], + (state.sort || []) as SortOrder[], + uiSettings.get(MODIFY_COLUMNS_ON_SWITCH), + uiSettings.get(SORT_DEFAULT_ORDER_SETTING), + state.query + ); + setUrlTracking(nextDataView); + stateContainer.setAppState(nextAppState); + } + setExpandedDoc(undefined); + }, + [ + setUrlTracking, + uiSettings, + dataView, + dataViews, + setExpandedDoc, + state.columns, + state.query, + state.sort, + stateContainer, + reset, + ] + ); + /** * function to revert any changes to a given saved search */ diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index 1a0048a2860b4..d6faf9c97ecff 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -234,8 +234,9 @@ export function getDiscoverStateContainer({ } }; - const setDataView = (dataView: DataView) => + const setDataView = (dataView: DataView) => { internalStateContainer.transitions.setDataView(dataView); + }; const setAdHocDataViews = (dataViews: DataView[]) => internalStateContainer.transitions.setAdHocDataViews(dataViews); const appendAdHocDataViews = (dataViews: DataView | DataView[]) => From 27377bbc676e833e8b46b777013d453374cdb077 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 16 Dec 2022 07:48:32 +0100 Subject: [PATCH 20/36] Fix unit tests --- .../main/components/layout/discover_documents.test.tsx | 1 + .../application/main/components/layout/discover_documents.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx index fac5e025888d8..c6af7846a4d23 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.test.tsx @@ -38,6 +38,7 @@ function mountComponent(fetchStatus: FetchStatus, hits: EsHitRecord[]) { result: hits.map((hit) => buildDataTableRecord(hit, dataViewMock)), }) as DataDocuments$; const stateContainer = getDiscoverStateMock({}); + stateContainer.setAppState({ index: dataViewMock.id }); const props = { expandedDoc: undefined, diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx index 04a1c36164379..3bcc0dc19e5a9 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx @@ -103,7 +103,7 @@ function DiscoverDocumentsComponent({ // but the data view in internal state was not set. due to the change of the timefield, this can lead to a change // of state, EuiDataGrid pushing an onSort event, because the next timefield is already used a sorting param // but it's not an available column - const dataViewChanged = index !== dataView.id; + const dataViewChanged = index && dataView.id && index !== dataView.id; const isPlainRecord = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]); const rows = useMemo(() => documentState.result || [], [documentState.result]); From 383ac1dc35efbd2143fc00c376589f373e917cfc Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 16 Dec 2022 13:20:47 +0100 Subject: [PATCH 21/36] Fix functional tests --- .../public/application/main/hooks/use_discover_state.ts | 3 --- 1 file changed, 3 deletions(-) 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 43f574c25b42f..6e74438ff3cc0 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 @@ -192,7 +192,6 @@ export function useDiscoverState({ return; } savedSearch.searchSource.setField('index', nextDataView); - reset(); stateContainer.actions.setDataView(nextDataView); } @@ -227,7 +226,6 @@ export function useDiscoverState({ async (id: string) => { const nextDataView = await dataViews.get(id); if (nextDataView && dataView) { - reset(); const nextAppState = getDataViewAppState( dataView, nextDataView, @@ -252,7 +250,6 @@ export function useDiscoverState({ state.query, state.sort, stateContainer, - reset, ] ); From 5cab7580091999aa61c68695e99ceea870988763 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 19 Dec 2022 15:03:18 +0100 Subject: [PATCH 22/36] Fix functionals --- .../public/application/main/hooks/use_discover_state.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 6e74438ff3cc0..a038f23be2dc4 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 @@ -62,7 +62,11 @@ export function useDiscoverState({ savedSearch, services, }); - container.actions.setDataView(savedSearch.searchSource.getField('index')!); + const nextDataView = savedSearch.searchSource.getField('index')!; + container.actions.setDataView(nextDataView); + if (!nextDataView.isPersisted()) { + container.actions.appendAdHocDataViews(nextDataView); + } return container; }, [history, savedSearch, services]); From 43bb325d75db7dfa2995fd6ac35e6090399a1ea0 Mon Sep 17 00:00:00 2001 From: Dzmitry Tamashevich Date: Mon, 19 Dec 2022 17:33:14 +0300 Subject: [PATCH 23/36] [Discover] fix two adhoc data views diplayed issue on click use without saving --- .../dataview_picker/change_dataview.tsx | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx b/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx index e218f27dadc55..6184c9d08ff18 100644 --- a/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx +++ b/src/plugins/unified_search/public/dataview_picker/change_dataview.tsx @@ -26,6 +26,7 @@ import { EuiToolTip, } from '@elastic/eui'; import { useKibana } from '@kbn/kibana-react-plugin/public'; +import type { DataView } from '@kbn/data-views-plugin/public'; import type { IUnifiedSearchPluginServices } from '../types'; import type { DataViewPickerPropsExtended } from '.'; import type { DataViewListItemEnhanced } from './dataview_list'; @@ -58,6 +59,15 @@ export const TextBasedLanguagesList = (props: TextBasedLanguagesListProps) => ( ); +const mapAdHocDataView = (adHocDataView: DataView) => { + return { + title: adHocDataView.title, + name: adHocDataView.name, + id: adHocDataView.id!, + isAdhoc: true, + }; +}; + export function ChangeDataView({ isMissingCurrent, currentDataViewId, @@ -98,22 +108,13 @@ export function ChangeDataView({ useEffect(() => { const fetchDataViews = async () => { - const dataViewsRefs: DataViewListItemEnhanced[] = savedDataViews + const savedDataViewRefs: DataViewListItemEnhanced[] = savedDataViews ? savedDataViews : await data.dataViews.getIdsWithTitle(); - if (adHocDataViews?.length) { - adHocDataViews.forEach((adHocDataView) => { - if (adHocDataView.id) { - dataViewsRefs.push({ - title: adHocDataView.title, - name: adHocDataView.name, - id: adHocDataView.id, - isAdhoc: true, - }); - } - }); - } - setDataViewsList(dataViewsRefs); + const adHocDataViewRefs: DataViewListItemEnhanced[] = + adHocDataViews?.map(mapAdHocDataView) || []; + + setDataViewsList(savedDataViewRefs.concat(adHocDataViewRefs)); }; fetchDataViews(); }, [data, currentDataViewId, adHocDataViews, savedDataViews]); From c20939489e67454adc4e960b6fbd1dbb5aeacd7e Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 19 Dec 2022 17:11:21 +0100 Subject: [PATCH 24/36] Prevent duplicate ad-hocs in the data view select --- .../discover_internal_state_container.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index 95bfafc739589..1a42ff30efcb8 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -61,10 +61,21 @@ export function getInternalStateContainer() { dataViewAdHocList: newAdHocDataViewList, }), appendAdHocDataViews: - (prevState: InternalState) => (dataViewsAdHoc: DataView | DataView[]) => ({ - ...prevState, - dataViewAdHocList: prevState.dataViewAdHocList.concat(dataViewsAdHoc), - }), + (prevState: InternalState) => (dataViewsAdHoc: DataView | DataView[]) => { + // check for already existing data views + const concatList = ( + Array.isArray(dataViewsAdHoc) ? dataViewsAdHoc : [dataViewsAdHoc] + ).filter((dataView) => { + return !prevState.dataViewAdHocList.find((el: DataView) => el.id === dataView.id); + }); + if (!concatList.length) { + return prevState; + } + return { + ...prevState, + dataViewAdHocList: prevState.dataViewAdHocList.concat(dataViewsAdHoc), + }; + }, removeAdHocDataViewById: (prevState: InternalState) => (id: string) => ({ ...prevState, dataViewAdHocList: prevState.dataViewAdHocList.filter((dataView) => dataView.id !== id), From 4c929b82a7fe89292968a9633673ac405f2998c5 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Mon, 19 Dec 2022 18:53:39 +0100 Subject: [PATCH 25/36] Fix loading state change --- .../discover/public/application/main/hooks/use_discover_state.ts | 1 + 1 file changed, 1 insertion(+) 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 a038f23be2dc4..f530c6ca29fd2 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 @@ -196,6 +196,7 @@ export function useDiscoverState({ return; } savedSearch.searchSource.setField('index', nextDataView); + reset(); stateContainer.actions.setDataView(nextDataView); } From 7027786ba8e72be1d5beee7ecfd08601d036c8e2 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 20 Dec 2022 17:00:14 +0100 Subject: [PATCH 26/36] Update src/plugins/discover/public/hooks/use_data_grid_columns.ts Co-authored-by: Julia Rechkunova --- src/plugins/discover/public/hooks/use_data_grid_columns.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/discover/public/hooks/use_data_grid_columns.ts b/src/plugins/discover/public/hooks/use_data_grid_columns.ts index a9540b275fb66..d81974aac26fa 100644 --- a/src/plugins/discover/public/hooks/use_data_grid_columns.ts +++ b/src/plugins/discover/public/hooks/use_data_grid_columns.ts @@ -53,7 +53,7 @@ export const useColumns = ({ dataViews, setAppState, useNewFieldsApi, - columns, + columns: usedColumns, sort, }), [capabilities, columns, config, dataView, dataViews, setAppState, sort, useNewFieldsApi] From 857afed7d076b2e0dc761d65621d0ad1f50d95d3 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 20 Dec 2022 17:00:31 +0100 Subject: [PATCH 27/36] Update src/plugins/discover/public/hooks/use_data_grid_columns.ts Co-authored-by: Julia Rechkunova --- src/plugins/discover/public/hooks/use_data_grid_columns.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/discover/public/hooks/use_data_grid_columns.ts b/src/plugins/discover/public/hooks/use_data_grid_columns.ts index d81974aac26fa..c693117916d71 100644 --- a/src/plugins/discover/public/hooks/use_data_grid_columns.ts +++ b/src/plugins/discover/public/hooks/use_data_grid_columns.ts @@ -56,7 +56,7 @@ export const useColumns = ({ columns: usedColumns, sort, }), - [capabilities, columns, config, dataView, dataViews, setAppState, sort, useNewFieldsApi] + [capabilities, config, dataView, dataViews, setAppState, sort, useNewFieldsApi, usedColumns] ); return { From 74f5058f99e58fdb12d9d1eb3f829ce3f812c4be Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 20 Dec 2022 17:17:07 +0100 Subject: [PATCH 28/36] Fix storybook state name --- .../main/components/layout/__stories__/get_layout_props.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/__stories__/get_layout_props.ts b/src/plugins/discover/public/application/main/components/layout/__stories__/get_layout_props.ts index 1001a6604e304..f5b64ebf85adc 100644 --- a/src/plugins/discover/public/application/main/components/layout/__stories__/get_layout_props.ts +++ b/src/plugins/discover/public/application/main/components/layout/__stories__/get_layout_props.ts @@ -91,10 +91,9 @@ const getCommonProps = (dataView: DataView) => { savedSearch: savedSearchMock, savedSearchRefetch$: new Subject(), searchSource: searchSourceMock, - stateContainer: { setAppState: action('Set app state'), - appStateContainer: { + appState: { getState: () => ({ interval: 'auto', }), From 5091f46ec826a50b6a2a6d56534fffd95d5a2472 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Tue, 20 Dec 2022 19:01:03 +0100 Subject: [PATCH 29/36] Improve saved/adhoc data views naming --- .../layout/discover_layout.test.tsx | 2 -- .../components/top_nav/discover_topnav.tsx | 12 +++++----- .../application/main/discover_main_app.tsx | 2 +- .../discover_internal_state_container.ts | 24 +++++++++---------- .../main/services/discover_state.test.ts | 6 ++--- .../main/services/discover_state.ts | 2 +- 6 files changed, 23 insertions(+), 25 deletions(-) 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 3275de027176f..6a582a7312929 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 @@ -120,9 +120,7 @@ function mountComponent( setExpandedDoc: jest.fn(), persistDataView: jest.fn(), updateAdHocDataViewId: jest.fn(), - adHocDataViewList: [], searchSessionManager: createSearchSessionMock(session).searchSessionManager, - savedDataViewList: [], updateDataViewList: jest.fn(), }; diff --git a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx index 7f16de2631de6..241fa39e86312 100644 --- a/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/plugins/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -62,9 +62,9 @@ export const DiscoverTopNav = ({ updateDataViewList, }: DiscoverTopNavProps) => { const history = useHistory(); - const adHocDataViewList = useInternalStateSelector((state) => state.dataViewAdHocList); + const adHocDataViews = useInternalStateSelector((state) => state.adHocDataViews); const dataView = useInternalStateSelector((state) => state.dataView!); - const savedDataViewList = useInternalStateSelector((state) => state.dataViewList); + const savedDataViews = useInternalStateSelector((state) => state.savedDataViews); const showDatePicker = useMemo( () => dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP, [dataView] @@ -163,7 +163,7 @@ export const DiscoverTopNav = ({ searchSource, onOpenSavedSearch, isPlainRecord, - adHocDataViews: adHocDataViewList, + adHocDataViews, updateDataViewList, persistDataView, updateAdHocDataViewId, @@ -178,7 +178,7 @@ export const DiscoverTopNav = ({ searchSource, onOpenSavedSearch, isPlainRecord, - adHocDataViewList, + adHocDataViews, persistDataView, updateAdHocDataViewId, updateDataViewList, @@ -218,8 +218,8 @@ export const DiscoverTopNav = ({ onCreateDefaultAdHocDataView, onChangeDataView, textBasedLanguages: supportedTextBasedLanguages as DataViewPickerProps['textBasedLanguages'], - adHocDataViews: adHocDataViewList, - savedDataViews: savedDataViewList, + adHocDataViews, + savedDataViews, }; const onTextBasedSavedAndExit = useCallback( 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 972ec6469d552..395f0f0fa7f0f 100644 --- a/src/plugins/discover/public/application/main/discover_main_app.tsx +++ b/src/plugins/discover/public/application/main/discover_main_app.tsx @@ -98,7 +98,7 @@ export function DiscoverMainApp(props: DiscoverMainProps) { * Can be removed once the state container work was completed */ useEffect(() => { - stateContainer.internalState.transitions.setDataViewList(dataViewList); + stateContainer.internalState.transitions.setSavedDataViews(dataViewList); }, [stateContainer, dataViewList]); const resetCurrentSavedSearch = useCallback(() => { diff --git a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts index 1a42ff30efcb8..44d2c5df2ae1c 100644 --- a/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/services/discover_internal_state_container.ts @@ -15,13 +15,13 @@ import { DataView, DataViewListItem } from '@kbn/data-views-plugin/common'; export interface InternalState { dataView: DataView | undefined; - dataViewList: DataViewListItem[]; - dataViewAdHocList: DataView[]; + savedDataViews: DataViewListItem[]; + adHocDataViews: DataView[]; } interface InternalStateTransitions { setDataView: (state: InternalState) => (dataView: DataView) => InternalState; - setDataViewList: (state: InternalState) => (dataView: DataViewListItem[]) => InternalState; + setSavedDataViews: (state: InternalState) => (dataView: DataViewListItem[]) => InternalState; setAdHocDataViews: (state: InternalState) => (dataViews: DataView[]) => InternalState; appendAdHocDataViews: ( state: InternalState @@ -44,21 +44,21 @@ export function getInternalStateContainer() { return createStateContainer( { dataView: undefined, - dataViewAdHocList: [], - dataViewList: [], + adHocDataViews: [], + savedDataViews: [], }, { setDataView: (prevState: InternalState) => (nextDataView: DataView) => ({ ...prevState, dataView: nextDataView, }), - setDataViewList: (prevState: InternalState) => (nextDataViewList: DataViewListItem[]) => ({ + setSavedDataViews: (prevState: InternalState) => (nextDataViewList: DataViewListItem[]) => ({ ...prevState, - dataViewList: nextDataViewList, + savedDataViews: nextDataViewList, }), setAdHocDataViews: (prevState: InternalState) => (newAdHocDataViewList: DataView[]) => ({ ...prevState, - dataViewAdHocList: newAdHocDataViewList, + adHocDataViews: newAdHocDataViewList, }), appendAdHocDataViews: (prevState: InternalState) => (dataViewsAdHoc: DataView | DataView[]) => { @@ -66,24 +66,24 @@ export function getInternalStateContainer() { const concatList = ( Array.isArray(dataViewsAdHoc) ? dataViewsAdHoc : [dataViewsAdHoc] ).filter((dataView) => { - return !prevState.dataViewAdHocList.find((el: DataView) => el.id === dataView.id); + return !prevState.adHocDataViews.find((el: DataView) => el.id === dataView.id); }); if (!concatList.length) { return prevState; } return { ...prevState, - dataViewAdHocList: prevState.dataViewAdHocList.concat(dataViewsAdHoc), + adHocDataViews: prevState.adHocDataViews.concat(dataViewsAdHoc), }; }, removeAdHocDataViewById: (prevState: InternalState) => (id: string) => ({ ...prevState, - dataViewAdHocList: prevState.dataViewAdHocList.filter((dataView) => dataView.id !== id), + adHocDataViews: prevState.adHocDataViews.filter((dataView) => dataView.id !== id), }), replaceAdHocDataViewWithId: (prevState: InternalState) => (prevId: string, newDataView: DataView) => ({ ...prevState, - dataViewAdHocList: prevState.dataViewAdHocList.map((dataView) => + adHocDataViews: prevState.adHocDataViews.map((dataView) => dataView.id === prevId ? newDataView : dataView ), }), diff --git a/src/plugins/discover/public/application/main/services/discover_state.test.ts b/src/plugins/discover/public/application/main/services/discover_state.test.ts index 255b4c2874297..8136ced9f4ff3 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.test.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.test.ts @@ -236,17 +236,17 @@ describe('createSearchSessionRestorationDataProvider', () => { test('appendAdHocDataViews', async () => { state.actions.appendAdHocDataViews(dataViewMock); - expect(state.internalState.getState().dataViewAdHocList).toEqual([dataViewMock]); + expect(state.internalState.getState().adHocDataViews).toEqual([dataViewMock]); }); test('removeAdHocDataViewById', async () => { state.actions.appendAdHocDataViews(dataViewMock); state.actions.removeAdHocDataViewById(dataViewMock.id!); - expect(state.internalState.getState().dataViewAdHocList).toEqual([]); + expect(state.internalState.getState().adHocDataViews).toEqual([]); }); test('replaceAdHocDataViewWithId', async () => { state.actions.appendAdHocDataViews(dataViewMock); state.actions.replaceAdHocDataViewWithId(dataViewMock.id!, dataViewComplexMock); - expect(state.internalState.getState().dataViewAdHocList).toEqual([dataViewComplexMock]); + expect(state.internalState.getState().adHocDataViews).toEqual([dataViewComplexMock]); }); }); }); diff --git a/src/plugins/discover/public/application/main/services/discover_state.ts b/src/plugins/discover/public/application/main/services/discover_state.ts index d6faf9c97ecff..b4a1bcbf0f1fa 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.ts @@ -248,7 +248,7 @@ export function getDiscoverStateContainer({ const loadDataViewList = async () => { const dataViewList = await services.dataViews.getIdsWithTitle(true); - internalStateContainer.transitions.setDataViewList(dataViewList); + internalStateContainer.transitions.setSavedDataViews(dataViewList); }; return { From 06c6bb6b25fc8b08a13a48f3152a06f20ba32aa2 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 21 Dec 2022 15:59:00 +0100 Subject: [PATCH 30/36] Fix type errors --- .../main/hooks/use_adhoc_data_views.test.tsx | 25 ------------------- .../main/hooks/use_saved_search.test.tsx | 4 --- .../hooks/use_text_based_query_language.ts | 2 +- 3 files changed, 1 insertion(+), 30 deletions(-) diff --git a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx index f3aab2c802182..770373f74825f 100644 --- a/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.test.tsx @@ -135,29 +135,4 @@ describe('useAdHocDataViews', () => { expect(mockDiscoverServices.dataViews.clearInstanceCache).toHaveBeenCalledWith(mockDataView.id); expect(updatedDataView!.id).toEqual('updated-mock-id'); }); - - it('should update the adHocList correctly for text based mode', async () => { - const hook = renderHook((d: DataView) => - useAdHocDataViews({ - dataView: mockDataView, - savedSearch: savedSearchMock, - stateContainer: { - appStateContainer: { getState: jest.fn().mockReturnValue({}) }, - replaceUrlAppState: jest.fn(), - kbnUrlStateStorage: { - kbnUrlControls: { flush: jest.fn() }, - }, - } as unknown as GetStateReturn, - setUrlTracking: jest.fn(), - dataViews: mockDiscoverServices.dataViews, - filterManager: mockDiscoverServices.filterManager, - toastNotifications: mockDiscoverServices.toastNotifications, - isTextBasedMode: true, - }) - ); - - const adHocList = await hook.result.current.adHocDataViewList; - expect(adHocList.length).toBe(1); - expect(adHocList[0].id).toEqual('mock-id'); - }); }); diff --git a/src/plugins/discover/public/application/main/hooks/use_saved_search.test.tsx b/src/plugins/discover/public/application/main/hooks/use_saved_search.test.tsx index 682ed7dbe0188..c598df55b1d1d 100644 --- a/src/plugins/discover/public/application/main/hooks/use_saved_search.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_saved_search.test.tsx @@ -14,8 +14,6 @@ import { RecordRawType, useSavedSearch } from './use_saved_search'; import { getDiscoverStateContainer } from '../services/discover_state'; import { useDiscoverState } from './use_discover_state'; import { FetchStatus } from '../../types'; -import { dataViewMock } from '../../../__mocks__/data_view'; -import { DataViewListItem } from '@kbn/data-views-plugin/common'; import { setUrlTracker } from '../../../kibana_services'; import { urlTrackerMock } from '../../../__mocks__/url_tracker.mock'; import React from 'react'; @@ -67,7 +65,6 @@ describe('test useSavedSearch', () => { history, savedSearch: savedSearchMock, setExpandedDoc: jest.fn(), - dataViewList: [dataViewMock as DataViewListItem], }); }, { @@ -118,7 +115,6 @@ describe('test useSavedSearch', () => { history, savedSearch: savedSearchMock, setExpandedDoc: jest.fn(), - dataViewList: [dataViewMock as DataViewListItem], }); }, { diff --git a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts index 8c8ad3747d102..220a3f1702faa 100644 --- a/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts +++ b/src/plugins/discover/public/application/main/hooks/use_text_based_query_language.ts @@ -92,7 +92,7 @@ export function useTextBasedQueryLanguage({ dataViewObj = await dataViews.create({ title: indexPatternFromQuery, }); - stateContainer.internalState.transitions.appendAdHocDataViews(dataViewObj); + stateContainer.internalState.transitions.setAdHocDataViews([dataViewObj]); if (dataViewObj.fields.getByName('@timestamp')?.type === 'date') { dataViewObj.timeFieldName = '@timestamp'; From 475033b6166ef454ccba445d949259c6d209e17e Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Wed, 21 Dec 2022 18:32:59 +0100 Subject: [PATCH 31/36] Fix jest test --- .../main/hooks/use_test_based_query_language.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts index 8c41b6acd35b5..db94bd6638568 100644 --- a/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts +++ b/src/plugins/discover/public/application/main/hooks/use_test_based_query_language.test.ts @@ -29,6 +29,7 @@ function getHookProps( ) { const stateContainer = getDiscoverStateMock({ isTimeBased: true }); stateContainer.replaceUrlAppState = replaceUrlAppState; + stateContainer.setAppState({ columns: [] }); stateContainer.internalState.transitions.setSavedDataViews([dataViewMock as DataViewListItem]); const msgLoading = { @@ -69,7 +70,7 @@ describe('useTextBasedQueryLanguage', () => { renderHook(() => useTextBasedQueryLanguage(props)); await waitFor(() => expect(replaceUrlAppState).toHaveBeenCalledTimes(1)); - expect(replaceUrlAppState).toHaveBeenCalledWith({ columns: [], index: 'the-data-view-id' }); + expect(replaceUrlAppState).toHaveBeenCalledWith({ index: 'the-data-view-id' }); replaceUrlAppState.mockReset(); @@ -156,7 +157,6 @@ describe('useTextBasedQueryLanguage', () => { await waitFor(() => { expect(replaceUrlAppState).toHaveBeenCalledWith({ - columns: [], index: 'the-data-view-id', }); }); From 177e1b3101547f880d5331c01f92a77636eaa2fe Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 29 Dec 2022 08:33:17 +0100 Subject: [PATCH 32/36] apply review suggestion --- .../discover/public/application/main/discover_main_route.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/plugins/discover/public/application/main/discover_main_route.tsx b/src/plugins/discover/public/application/main/discover_main_route.tsx index 52e0397c29f62..a8cc10bd4ae02 100644 --- a/src/plugins/discover/public/application/main/discover_main_route.tsx +++ b/src/plugins/discover/public/application/main/discover_main_route.tsx @@ -63,9 +63,6 @@ export function DiscoverMainRoute(props: Props) { const [hasUserDataView, setHasUserDataView] = useState(false); const [showNoDataPage, setShowNoDataPage] = useState(false); const { id } = useParams(); - useEffect(() => { - setLoading(true); - }, [id]); /** * Get location state of scoped history only on initial load @@ -145,6 +142,7 @@ export function DiscoverMainRoute(props: Props) { const loadSavedSearch = useCallback(async () => { try { + setLoading(true); const currentSavedSearch = await getSavedSearch(id, { search: services.data.search, savedObjectsClient: core.savedObjects.client, From ce9cc2ab55500e30896f7181cdd0d49d3f14960a Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 29 Dec 2022 10:57:51 +0100 Subject: [PATCH 33/36] Improve documentation --- .../components/layout/discover_documents.tsx | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx index 1d597c5923955..c99c61be44597 100644 --- a/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx +++ b/src/plugins/discover/public/application/main/components/layout/discover_documents.tsx @@ -99,12 +99,16 @@ function DiscoverDocumentsComponent({ const sampleSize = useMemo(() => uiSettings.get(SAMPLE_SIZE_SETTING), [uiSettings]); const documentState = useDataState(documents$); - const isLoading = documentState.fetchStatus === FetchStatus.LOADING; - // this is needed to prevent data view pushing onSort because there has been a state change with a new data view - // but the data view in internal state was not set. due to the change of the timefield, this can lead to a change - // of state, EuiDataGrid pushing an onSort event, because the next timefield is already used a sorting param - // but it's not an available column - const dataViewChanged = index && dataView.id && index !== dataView.id; + const isDataLoading = documentState.fetchStatus === FetchStatus.LOADING; + // This is needed to prevent EuiDataGrid pushing onSort because the data view has been switched. + // 1. When switching the data view, the sorting in the URL is reset to the default sorting of the selected data view. + // 2. The new sort param is already available in this component and propagated to the EuiDataGrid. + // 3. currentColumns are still referring to the old state + // 4. since the new sort by field isn't available in currentColumns EuiDataGrid is emitting a 'onSort', which is unsorting the grid + // 5. this is propagated to Discover's URL and causes an unwanted change of state to an unsorted state + // This solution switches to the loading state in this component when the URL index doesn't match the dataView.id + const isDataViewLoading = index && dataView.id && index !== dataView.id; + const isEmptyDataResult = !documentState.result || documentState.result.length === 0; const isPlainRecord = useMemo(() => getRawRecordType(query) === RecordRawType.PLAIN, [query]); const rows = useMemo(() => documentState.result || [], [documentState.result]); @@ -159,11 +163,7 @@ function DiscoverDocumentsComponent({ [isPlainRecord, uiSettings, dataView.timeFieldName] ); - if ( - dataViewChanged || - ((!documentState.result || documentState.result.length === 0) && - documentState.fetchStatus === FetchStatus.LOADING) - ) { + if (isDataViewLoading || (isEmptyDataResult && isDataLoading)) { return (
@@ -190,7 +190,7 @@ function DiscoverDocumentsComponent({ dataView={dataView} rows={rows} sort={sort || []} - isLoading={isLoading} + isLoading={isDataLoading} searchDescription={savedSearch.description} sharedItemTitle={savedSearch.title} onAddColumn={onAddColumn} @@ -216,7 +216,7 @@ function DiscoverDocumentsComponent({ columns={currentColumns} expandedDoc={expandedDoc} dataView={dataView} - isLoading={isLoading} + isLoading={isDataLoading} rows={rows} sort={(sort as SortOrder[]) || []} sampleSize={sampleSize} From 1ab6046a8b6975b3d02bba516724ddcd880d7f0f Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 29 Dec 2022 21:41:08 +0100 Subject: [PATCH 34/36] Add functional test --- .../group2/_indexpattern_without_timefield.ts | 16 +++++++++++++++ test/functional/config.base.js | 2 +- .../index_pattern_without_timefield/data.json | 19 ++++++++++++++++++ .../mappings.json | 20 +++++++++++++++++++ .../index_pattern_without_timefield.json | 18 ++++++++++++++++- 5 files changed, 73 insertions(+), 2 deletions(-) diff --git a/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts b/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts index 8fe192618f2ff..7b351633530de 100644 --- a/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts +++ b/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts @@ -129,5 +129,21 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.header.waitUntilLoadingHasFinished(); await PageObjects.discover.assertHitCount('1'); }); + + it('should allow switching from data views with different timefield and adjust sorting', async () => { + await PageObjects.common.navigateToApp('discover'); + await PageObjects.discover.selectIndexPattern('with-timefield'); + await PageObjects.header.waitUntilLoadingHasFinished(); + let url = await browser.getCurrentUrl(); + expect(url).to.contain(`@timestamp`); + + await PageObjects.discover.selectIndexPattern('with-different-timefield'); + await PageObjects.header.waitUntilLoadingHasFinished(); + url = await browser.getCurrentUrl(); + expect(url).to.contain(`with-different-timefield`); + browser.goBack(); + await PageObjects.header.waitUntilLoadingHasFinished(); + expect(url).to.contain(`@timestamp`); + }); }); } diff --git a/test/functional/config.base.js b/test/functional/config.base.js index aadaae3c8d81b..5e4d89d6c195b 100644 --- a/test/functional/config.base.js +++ b/test/functional/config.base.js @@ -391,7 +391,7 @@ export default async function ({ readConfigFile }) { cluster: [], indices: [ { - names: ['without-timefield', 'with-timefield'], + names: ['without-timefield', 'with-timefield', 'with-different-timefield'], privileges: ['read', 'view_index_metadata'], field_security: { grant: ['*'], except: [] }, }, diff --git a/test/functional/fixtures/es_archiver/index_pattern_without_timefield/data.json b/test/functional/fixtures/es_archiver/index_pattern_without_timefield/data.json index aea1bf770c18f..3bd71cfb198dd 100644 --- a/test/functional/fixtures/es_archiver/index_pattern_without_timefield/data.json +++ b/test/functional/fixtures/es_archiver/index_pattern_without_timefield/data.json @@ -31,3 +31,22 @@ } } +{ + "type": "doc", + "value": { + "id": "with-different-timefield-1", + "index": "with-different-timefield", + "source": { + "@message" : "ole", + "with-different-timefield": "2019-10-22T23:50:13.253Z", + "referer": "http://twitter.com/error/takuya-onishi", + "request": "/uploads/dafydd-williams.jpg", + "response": "200", + "type": "apache", + "url": "https://media-for-the-masses.theacademyofperformingartsandscience.org/uploads/dafydd-williams.jpg" + } + } +} + + + diff --git a/test/functional/fixtures/es_archiver/index_pattern_without_timefield/mappings.json b/test/functional/fixtures/es_archiver/index_pattern_without_timefield/mappings.json index dd41e01592a7b..0d1fe46408f36 100644 --- a/test/functional/fixtures/es_archiver/index_pattern_without_timefield/mappings.json +++ b/test/functional/fixtures/es_archiver/index_pattern_without_timefield/mappings.json @@ -30,3 +30,23 @@ } } } + +{ + "type": "index", + "value": { + "index": "with-different-timefield", + "mappings": { + "properties": { + "different-timefield": { + "type": "date" + } + } + }, + "settings": { + "index": { + "number_of_replicas": "0", + "number_of_shards": "1" + } + } + } +} diff --git a/test/functional/fixtures/kbn_archiver/index_pattern_without_timefield.json b/test/functional/fixtures/kbn_archiver/index_pattern_without_timefield.json index d5906dc8a2e99..d0c7b2f55b975 100644 --- a/test/functional/fixtures/kbn_archiver/index_pattern_without_timefield.json +++ b/test/functional/fixtures/kbn_archiver/index_pattern_without_timefield.json @@ -14,6 +14,22 @@ "version": "WzEzLDJd" } +{ + "attributes": { + "fields": "[]", + "timeFieldName": "with-different-timefield", + "title": "with-different-timefield" + }, + "coreMigrationVersion": "7.17.1", + "id": "with-different-timefield", + "migrationVersion": { + "index-pattern": "7.11.0" + }, + "references": [], + "type": "index-pattern", + "version": "WzEzLDJd" +} + { "attributes": { "fields": "[]", @@ -27,4 +43,4 @@ "references": [], "type": "index-pattern", "version": "WzEyLDJd" -} \ No newline at end of file +} From d0bc3d4852855bbd85bfecb7b023672a1ab8938c Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 29 Dec 2022 23:43:59 +0100 Subject: [PATCH 35/36] Add missing await to functional test --- .../apps/discover/group2/_indexpattern_without_timefield.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts b/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts index 7b351633530de..04a8a2f430b5d 100644 --- a/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts +++ b/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts @@ -141,7 +141,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.header.waitUntilLoadingHasFinished(); url = await browser.getCurrentUrl(); expect(url).to.contain(`with-different-timefield`); - browser.goBack(); + await browser.goBack(); await PageObjects.header.waitUntilLoadingHasFinished(); expect(url).to.contain(`@timestamp`); }); From ca6926a2088df5a863d7a6067907a0ab657245f8 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 30 Dec 2022 08:03:37 +0100 Subject: [PATCH 36/36] Fix functional test --- .../apps/discover/group2/_indexpattern_without_timefield.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts b/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts index 04a8a2f430b5d..2531a368861c3 100644 --- a/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts +++ b/test/functional/apps/discover/group2/_indexpattern_without_timefield.ts @@ -130,7 +130,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.discover.assertHitCount('1'); }); - it('should allow switching from data views with different timefield and adjust sorting', async () => { + it('should allow switching from data views with different timefields and sort correctly', async () => { await PageObjects.common.navigateToApp('discover'); await PageObjects.discover.selectIndexPattern('with-timefield'); await PageObjects.header.waitUntilLoadingHasFinished(); @@ -143,6 +143,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(url).to.contain(`with-different-timefield`); await browser.goBack(); await PageObjects.header.waitUntilLoadingHasFinished(); + url = await browser.getCurrentUrl(); expect(url).to.contain(`@timestamp`); }); });