diff --git a/src/plugins/discover/public/__mocks__/services.ts b/src/plugins/discover/public/__mocks__/services.ts index b00b5a95b3958..94a3249bdf271 100644 --- a/src/plugins/discover/public/__mocks__/services.ts +++ b/src/plugins/discover/public/__mocks__/services.ts @@ -14,6 +14,7 @@ import { uiActionsPluginMock } from '@kbn/ui-actions-plugin/public/mocks'; import { expressionsPluginMock } from '@kbn/expressions-plugin/public/mocks'; import { savedSearchPluginMock } from '@kbn/saved-search-plugin/public/mocks'; import { + analyticsServiceMock, chromeServiceMock, coreMock, docLinksServiceMock, @@ -149,6 +150,7 @@ export function createDiscoverServicesMock(): DiscoverServices { corePluginMock.chrome.getActiveSolutionNavId$.mockReturnValue(new BehaviorSubject(null)); return { + analytics: analyticsServiceMock.createAnalyticsServiceStart(), application: corePluginMock.application, core: corePluginMock, charts: chartPluginMock.createSetupContract(), diff --git a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx index 287b5a60386f4..c8f829d442444 100644 --- a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx +++ b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.test.tsx @@ -97,7 +97,6 @@ describe('useDiscoverHistogram', () => { stateContainer.appState.update({ interval: 'auto', hideChart: false, - breakdownField: 'extension', }); const appState = stateContainer.appState; const wrappedStateContainer = Object.create(appState); @@ -166,7 +165,6 @@ describe('useDiscoverHistogram', () => { expect(Object.keys(params?.initialState ?? {})).toEqual([ 'chartHidden', 'timeInterval', - 'breakdownField', 'totalHitsStatus', 'totalHitsResult', ]); @@ -204,7 +202,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: '1m', chartHidden: true, - breakdownField: 'test', totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -217,7 +214,6 @@ describe('useDiscoverHistogram', () => { expect(stateContainer.appState.update).toHaveBeenCalledWith({ interval: state.timeInterval, hideChart: state.chartHidden, - breakdownField: state.breakdownField, }); }); @@ -228,7 +224,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: containerState.interval, chartHidden: containerState.hideChart, - breakdownField: containerState.breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -254,18 +249,14 @@ describe('useDiscoverHistogram', () => { api.setTimeInterval = jest.fn((timeInterval) => { params = { ...params, timeInterval }; }); - api.setBreakdownField = jest.fn((breakdownField) => { - params = { ...params, breakdownField }; - }); act(() => { hook.result.current.ref(api); }); - stateContainer.appState.update({ hideChart: true, interval: '1m', breakdownField: 'test' }); + stateContainer.appState.update({ hideChart: true, interval: '1m' }); expect(api.setTotalHits).not.toHaveBeenCalled(); expect(api.setChartHidden).toHaveBeenCalled(); expect(api.setTimeInterval).toHaveBeenCalled(); - expect(api.setBreakdownField).toHaveBeenCalled(); - expect(Object.keys(params ?? {})).toEqual(['breakdownField', 'timeInterval', 'chartHidden']); + expect(Object.keys(params ?? {})).toEqual(['timeInterval', 'chartHidden']); }); it('should exclude totalHitsStatus and totalHitsResult from Unified Histogram state updates', async () => { @@ -275,7 +266,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: containerState.interval, chartHidden: containerState.hideChart, - breakdownField: containerState.breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -310,7 +300,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: containerState.interval, chartHidden: containerState.hideChart, - breakdownField: containerState.breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -355,7 +344,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: containerState.interval, chartHidden: containerState.hideChart, - breakdownField: containerState.breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -381,22 +369,13 @@ describe('useDiscoverHistogram', () => { }); it('should set isChartLoading to true for fetch start', async () => { - const fetch$ = new Subject<{ - options: { - reset: boolean; - fetchMore: boolean; - }; - searchSessionId: string; - }>(); + const fetch$ = new Subject(); const stateContainer = getStateContainer(); stateContainer.appState.update({ query: { esql: 'from *' } }); - stateContainer.dataState.fetch$ = fetch$; + stateContainer.dataState.fetchChart$ = fetch$; const { hook } = await renderUseDiscoverHistogram({ stateContainer }); act(() => { - fetch$.next({ - options: { reset: false, fetchMore: false }, - searchSessionId: '1234', - }); + fetch$.next(); }); expect(hook.result.current.isChartLoading).toBe(true); }); @@ -404,15 +383,9 @@ describe('useDiscoverHistogram', () => { describe('refetching', () => { it('should call refetch when savedSearchFetch$ is triggered', async () => { - const savedSearchFetch$ = new Subject<{ - options: { - reset: boolean; - fetchMore: boolean; - }; - searchSessionId: string; - }>(); + const savedSearchFetch$ = new Subject(); const stateContainer = getStateContainer(); - stateContainer.dataState.fetch$ = savedSearchFetch$; + stateContainer.dataState.fetchChart$ = savedSearchFetch$; const { hook } = await renderUseDiscoverHistogram({ stateContainer }); const api = createMockUnifiedHistogramApi(); act(() => { @@ -420,24 +393,15 @@ describe('useDiscoverHistogram', () => { }); expect(api.refetch).toHaveBeenCalled(); act(() => { - savedSearchFetch$.next({ - options: { reset: false, fetchMore: false }, - searchSessionId: '1234', - }); + savedSearchFetch$.next(); }); expect(api.refetch).toHaveBeenCalledTimes(2); }); it('should skip the next refetch when hideChart changes from true to false', async () => { - const savedSearchFetch$ = new Subject<{ - options: { - reset: boolean; - fetchMore: boolean; - }; - searchSessionId: string; - }>(); + const savedSearchFetch$ = new Subject(); const stateContainer = getStateContainer(); - stateContainer.dataState.fetch$ = savedSearchFetch$; + stateContainer.dataState.fetchChart$ = savedSearchFetch$; const { hook, initialProps } = await renderUseDiscoverHistogram({ stateContainer }); const api = createMockUnifiedHistogramApi(); act(() => { @@ -451,45 +415,9 @@ describe('useDiscoverHistogram', () => { hook.rerender({ ...initialProps, hideChart: false }); }); act(() => { - savedSearchFetch$.next({ - options: { reset: false, fetchMore: false }, - searchSessionId: '1234', - }); - }); - expect(api.refetch).toHaveBeenCalledTimes(1); - }); - - it('should skip the next refetch when fetching more', async () => { - const savedSearchFetch$ = new Subject<{ - options: { - reset: boolean; - fetchMore: boolean; - }; - searchSessionId: string; - }>(); - const stateContainer = getStateContainer(); - stateContainer.dataState.fetch$ = savedSearchFetch$; - const { hook } = await renderUseDiscoverHistogram({ stateContainer }); - const api = createMockUnifiedHistogramApi(); - act(() => { - hook.result.current.ref(api); - }); - expect(api.refetch).toHaveBeenCalledTimes(1); - act(() => { - savedSearchFetch$.next({ - options: { reset: false, fetchMore: true }, - searchSessionId: '1234', - }); + savedSearchFetch$.next(); }); expect(api.refetch).toHaveBeenCalledTimes(1); - - act(() => { - savedSearchFetch$.next({ - options: { reset: false, fetchMore: false }, - searchSessionId: '1234', - }); - }); - expect(api.refetch).toHaveBeenCalledTimes(2); }); }); 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 66b5be4d02ab7..3f2acf0ce933b 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 @@ -11,6 +11,8 @@ import { useQuerySubscriber } from '@kbn/unified-field-list/src/hooks/use_query_ import { canImportVisContext, UnifiedHistogramApi, + UnifiedHistogramContainerProps, + UnifiedHistogramCreationOptions, UnifiedHistogramExternalVisContextStatus, UnifiedHistogramFetchStatus, UnifiedHistogramState, @@ -41,7 +43,10 @@ import { checkHitCount, sendErrorTo } from '../../hooks/use_saved_search_message import type { DiscoverStateContainer } from '../../state_management/discover_state'; import { addLog } from '../../../../utils/add_log'; import { useInternalStateSelector } from '../../state_management/discover_internal_state_container'; -import type { DiscoverAppState } from '../../state_management/discover_app_state_container'; +import { + useAppStateSelector, + type DiscoverAppState, +} from '../../state_management/discover_app_state_container'; import { DataDocumentsMsg } from '../../state_management/discover_data_state_container'; import { useSavedSearch } from '../../state_management/discover_state_provider'; import { useIsEsqlMode } from '../../hooks/use_is_esql_mode'; @@ -59,7 +64,13 @@ export const useDiscoverHistogram = ({ stateContainer, inspectorAdapters, hideChart, -}: UseDiscoverHistogramProps) => { +}: UseDiscoverHistogramProps): Omit< + UnifiedHistogramContainerProps, + 'container' | 'getCreationOptions' +> & { + ref: (api: UnifiedHistogramApi | null) => void; + getCreationOptions: () => UnifiedHistogramCreationOptions; +} => { const services = useDiscoverServices(); const { main$, documents$, totalHits$ } = stateContainer.dataState.data$; const savedSearchState = useSavedSearch(); @@ -73,11 +84,7 @@ export const useDiscoverHistogram = ({ const [isSuggestionLoading, setIsSuggestionLoading] = useState(false); const getCreationOptions = useCallback(() => { - const { - hideChart: chartHidden, - interval: timeInterval, - breakdownField, - } = stateContainer.appState.getState(); + const { hideChart: chartHidden, interval: timeInterval } = stateContainer.appState.getState(); return { localStorageKeyPrefix: 'discover', @@ -85,7 +92,6 @@ export const useDiscoverHistogram = ({ initialState: { chartHidden, timeInterval, - breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, }, @@ -104,7 +110,6 @@ export const useDiscoverHistogram = ({ const oldState = { hideChart: appState.hideChart, interval: appState.interval, - breakdownField: appState.breakdownField, }; const newState = { ...oldState, ...stateChanges }; @@ -130,10 +135,6 @@ export const useDiscoverHistogram = ({ useEffect(() => { const subscription = createAppStateObservable(stateContainer.appState.state$).subscribe( (changes) => { - if ('breakdownField' in changes) { - unifiedHistogram?.setBreakdownField(changes.breakdownField); - } - if ('timeInterval' in changes && changes.timeInterval) { unifiedHistogram?.setTimeInterval(changes.timeInterval); } @@ -252,7 +253,7 @@ export const useDiscoverHistogram = ({ return; } - const fetchStart = stateContainer.dataState.fetch$.subscribe(() => { + const fetchStart = stateContainer.dataState.fetchChart$.subscribe(() => { if (!skipRefetch.current) { setIsSuggestionLoading(true); } @@ -265,7 +266,7 @@ export const useDiscoverHistogram = ({ fetchStart.unsubscribe(); fetchComplete.unsubscribe(); }; - }, [isEsqlMode, stateContainer.dataState.fetch$, esqlFetchComplete$]); + }, [isEsqlMode, stateContainer.dataState.fetchChart$, esqlFetchComplete$]); /** * Data fetching @@ -289,7 +290,7 @@ export const useDiscoverHistogram = ({ return; } - let fetch$: Observable; + let fetchChart$: Observable; // When in ES|QL mode, we refetch under two conditions: // 1. When the current Lens suggestion changes. This syncs the visualization @@ -299,18 +300,15 @@ export const useDiscoverHistogram = ({ // which are required to get the latest Lens suggestion, which would trigger // a refetch anyway and result in multiple unnecessary fetches. if (isEsqlMode) { - fetch$ = merge( + fetchChart$ = merge( createCurrentSuggestionObservable(unifiedHistogram.state$).pipe(map(() => 'lens')), esqlFetchComplete$.pipe(map(() => 'discover')) ).pipe(debounceTime(50)); } else { - fetch$ = stateContainer.dataState.fetch$.pipe( - filter(({ options }) => !options.fetchMore), // don't update histogram for "Load more" in the grid - map(() => 'discover') - ); + fetchChart$ = stateContainer.dataState.fetchChart$.pipe(map(() => 'discover')); } - const subscription = fetch$.subscribe((source) => { + const subscription = fetchChart$.subscribe((source) => { if (!skipRefetch.current) { if (source === 'discover') addLog('Unified Histogram - Discover refetch'); if (source === 'lens') addLog('Unified Histogram - Lens suggestion refetch'); @@ -328,7 +326,7 @@ export const useDiscoverHistogram = ({ return () => { subscription.unsubscribe(); }; - }, [isEsqlMode, stateContainer.dataState.fetch$, esqlFetchComplete$, unifiedHistogram]); + }, [isEsqlMode, stateContainer.dataState.fetchChart$, esqlFetchComplete$, unifiedHistogram]); const dataView = useInternalStateSelector((state) => state.dataView!); @@ -381,6 +379,19 @@ export const useDiscoverHistogram = ({ [stateContainer] ); + const breakdownField = useAppStateSelector((state) => state.breakdownField); + + const onBreakdownFieldChange = useCallback< + NonNullable + >( + (nextBreakdownField) => { + if (nextBreakdownField !== breakdownField) { + stateContainer.appState.update({ breakdownField: nextBreakdownField }); + } + }, + [breakdownField, stateContainer.appState] + ); + return { ref, getCreationOptions, @@ -402,6 +413,8 @@ export const useDiscoverHistogram = ({ ? savedSearchState?.visContext : undefined, onVisContextChanged: isEsqlMode ? onVisContextChanged : undefined, + breakdownField, + onBreakdownFieldChange, }; }; @@ -433,10 +446,6 @@ const createUnifiedHistogramStateObservable = (state$?: Observable Object.keys(changes).length > 0) @@ -454,10 +463,6 @@ const createAppStateObservable = (state$: Observable) => { return changes; } - if (prev?.breakdownField !== curr.breakdownField) { - changes.breakdownField = curr.breakdownField; - } - if (prev?.interval !== curr.interval) { changes.timeInterval = curr.interval; } diff --git a/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts b/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts index 169a644e69499..c7b80181867e0 100644 --- a/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts +++ b/src/plugins/discover/public/application/main/data_fetching/fetch_all.test.ts @@ -78,6 +78,7 @@ describe('test fetchAll', () => { resetDefaultProfileState: { columns: false, rowHeight: false, + breakdownField: false, }, }), searchSessionId: '123', @@ -270,6 +271,7 @@ describe('test fetchAll', () => { resetDefaultProfileState: { columns: false, rowHeight: false, + breakdownField: false, }, }), }; @@ -392,6 +394,7 @@ describe('test fetchAll', () => { resetDefaultProfileState: { columns: false, rowHeight: false, + breakdownField: false, }, }), }; diff --git a/src/plugins/discover/public/application/main/data_fetching/fetch_all.ts b/src/plugins/discover/public/application/main/data_fetching/fetch_all.ts index 3b54e6f8ce083..6a493b94d2fe4 100644 --- a/src/plugins/discover/public/application/main/data_fetching/fetch_all.ts +++ b/src/plugins/discover/public/application/main/data_fetching/fetch_all.ts @@ -9,7 +9,15 @@ import { Adapters } from '@kbn/inspector-plugin/common'; import type { SavedSearch, SortOrder } from '@kbn/saved-search-plugin/public'; -import { BehaviorSubject, combineLatest, filter, firstValueFrom, switchMap } from 'rxjs'; +import { + BehaviorSubject, + combineLatest, + distinctUntilChanged, + filter, + firstValueFrom, + race, + switchMap, +} from 'rxjs'; import { reportPerformanceMetricEvent } from '@kbn/ebt-tools'; import { isEqual } from 'lodash'; import { isOfAggregateQueryType } from '@kbn/es-query'; @@ -27,7 +35,11 @@ import { } from '../hooks/use_saved_search_messages'; import { fetchDocuments } from './fetch_documents'; import { FetchStatus } from '../../types'; -import { DataMsg, SavedSearchData } from '../state_management/discover_data_state_container'; +import { + DataMain$, + DataMsg, + SavedSearchData, +} from '../state_management/discover_data_state_container'; import { DiscoverServices } from '../../../build_services'; import { fetchEsql } from './fetch_esql'; import { InternalState } from '../state_management/discover_internal_state_container'; @@ -173,12 +185,17 @@ export function fetchAll( // but their errors will be shown in-place (e.g. of the chart). .catch(sendErrorTo(dataSubjects.documents$, dataSubjects.main$)); - // Return a promise that will resolve once all the requests have finished or failed + // Return a promise that will resolve once all the requests have finished or failed, or no results are found return firstValueFrom( - combineLatest([ - isComplete(dataSubjects.documents$).pipe(switchMap(async () => onFetchRecordsComplete?.())), - isComplete(dataSubjects.totalHits$), - ]) + race( + combineLatest([ + isComplete(dataSubjects.documents$).pipe( + switchMap(async () => onFetchRecordsComplete?.()) + ), + isComplete(dataSubjects.totalHits$), + ]), + noResultsFound(dataSubjects.main$) + ) ).then(() => { // Send a complete message to main$ once all queries are done and if main$ // is not already in an ERROR state, e.g. because the document query has failed. @@ -250,6 +267,18 @@ export async function fetchMoreDocuments( const isComplete = (subject: BehaviorSubject) => { return subject.pipe( - filter(({ fetchStatus }) => [FetchStatus.COMPLETE, FetchStatus.ERROR].includes(fetchStatus)) + filter(({ fetchStatus }) => [FetchStatus.COMPLETE, FetchStatus.ERROR].includes(fetchStatus)), + distinctUntilChanged((a, b) => a.fetchStatus === b.fetchStatus) + ); +}; + +const noResultsFound = (subject: DataMain$) => { + return subject.pipe( + filter( + ({ fetchStatus, foundDocuments }) => fetchStatus === FetchStatus.COMPLETE && !foundDocuments + ), + distinctUntilChanged( + (a, b) => a.fetchStatus === b.fetchStatus && a.foundDocuments === b.foundDocuments + ) ); }; diff --git a/src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx b/src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx index 486c477c8833e..410406b7eb2d7 100644 --- a/src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx +++ b/src/plugins/discover/public/application/main/hooks/use_esql_mode.test.tsx @@ -29,7 +29,8 @@ import { buildDataTableRecord, EsHitRecord } from '@kbn/discover-utils'; function getHookProps( query: AggregateQuery | Query | undefined, dataViewsService?: DataViewsContract, - appState?: Partial + appState?: Partial, + defaultFetchStatus: FetchStatus = FetchStatus.PARTIAL ) { const replaceUrlState = jest.fn(); const stateContainer = getDiscoverStateMock({ isTimeBased: true }); @@ -38,7 +39,7 @@ function getHookProps( stateContainer.internalState.transitions.setSavedDataViews([dataViewMock as DataViewListItem]); const msgLoading = { - fetchStatus: FetchStatus.PARTIAL, + fetchStatus: defaultFetchStatus, query, }; stateContainer.dataState.data$.documents$.next(msgLoading); @@ -83,15 +84,16 @@ const getHookContext = (stateContainer: DiscoverStateContainer) => { }; const renderHookWithContext = ( useDataViewsService: boolean = false, - appState?: DiscoverAppState + appState?: DiscoverAppState, + defaultFetchStatus?: FetchStatus ) => { - const props = getHookProps(query, useDataViewsService ? getDataViewsService() : undefined); + const props = getHookProps( + query, + useDataViewsService ? getDataViewsService() : undefined, + appState, + defaultFetchStatus + ); props.stateContainer.actions.setDataView(dataViewMock); - if (appState) { - props.stateContainer.appState.getState = jest.fn(() => { - return appState; - }); - } renderHook(() => useEsqlMode(props), { wrapper: getHookContext(props.stateContainer), @@ -493,46 +495,74 @@ describe('useEsqlMode', () => { }); it('should call setResetDefaultProfileState correctly when index pattern changes', async () => { - const { stateContainer } = renderHookWithContext(false); + const { stateContainer } = renderHookWithContext( + false, + { query: { esql: 'from pattern' } }, + FetchStatus.LOADING + ); const documents$ = stateContainer.dataState.data$.documents$; expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }); documents$.next({ fetchStatus: FetchStatus.PARTIAL, + query: { esql: 'from pattern' }, + }); + stateContainer.appState.update({ query: { esql: 'from pattern1' } }); + documents$.next({ + fetchStatus: FetchStatus.LOADING, query: { esql: 'from pattern1' }, }); await waitFor(() => expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: true, rowHeight: true, + breakdownField: true, }) ); + documents$.next({ + fetchStatus: FetchStatus.PARTIAL, + query: { esql: 'from pattern1' }, + }); stateContainer.internalState.transitions.setResetDefaultProfileState({ columns: false, rowHeight: false, + breakdownField: false, }); + stateContainer.appState.update({ query: { esql: 'from pattern1' } }); documents$.next({ - fetchStatus: FetchStatus.PARTIAL, + fetchStatus: FetchStatus.LOADING, query: { esql: 'from pattern1' }, }); await waitFor(() => expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }) ); documents$.next({ fetchStatus: FetchStatus.PARTIAL, + query: { esql: 'from pattern1' }, + }); + stateContainer.appState.update({ query: { esql: 'from pattern2' } }); + documents$.next({ + fetchStatus: FetchStatus.LOADING, query: { esql: 'from pattern2' }, }); await waitFor(() => expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: true, rowHeight: true, + breakdownField: true, }) ); + documents$.next({ + fetchStatus: FetchStatus.PARTIAL, + query: { esql: 'from pattern2' }, + }); }); it('should call setResetDefaultProfileState correctly when columns change', async () => { @@ -543,21 +573,7 @@ describe('useEsqlMode', () => { expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, - }); - documents$.next({ - fetchStatus: FetchStatus.PARTIAL, - query: { esql: 'from pattern' }, - result: result1, - }); - await waitFor(() => - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ - columns: true, - rowHeight: true, - }) - ); - stateContainer.internalState.transitions.setResetDefaultProfileState({ - columns: false, - rowHeight: false, + breakdownField: false, }); documents$.next({ fetchStatus: FetchStatus.PARTIAL, @@ -568,6 +584,7 @@ describe('useEsqlMode', () => { expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }) ); documents$.next({ @@ -579,6 +596,7 @@ describe('useEsqlMode', () => { expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: true, rowHeight: false, + breakdownField: false, }) ); }); diff --git a/src/plugins/discover/public/application/main/hooks/use_esql_mode.ts b/src/plugins/discover/public/application/main/hooks/use_esql_mode.ts index c2dcd467c104b..f84903e8b59ac 100644 --- a/src/plugins/discover/public/application/main/hooks/use_esql_mode.ts +++ b/src/plugins/discover/public/application/main/hooks/use_esql_mode.ts @@ -74,6 +74,36 @@ export function useEsqlMode({ return; } + // We need to reset the default profile state on index pattern changes + // when loading starts to ensure the correct pre fetch state is available + // before data fetching is triggered + if (next.fetchStatus === FetchStatus.LOADING) { + // We have to grab the current query from appState + // here since nextQuery has not been updated yet + const appStateQuery = stateContainer.appState.getState().query; + + if (isOfAggregateQueryType(appStateQuery)) { + if (prev.current.initialFetch) { + prev.current.query = appStateQuery.esql; + } + + const indexPatternChanged = + getIndexPatternFromESQLQuery(appStateQuery.esql) !== + getIndexPatternFromESQLQuery(prev.current.query); + + // Reset all default profile state when index pattern changes + if (indexPatternChanged) { + stateContainer.internalState.transitions.setResetDefaultProfileState({ + columns: true, + rowHeight: true, + breakdownField: true, + }); + } + } + + return; + } + if (next.fetchStatus !== FetchStatus.PARTIAL) { return; } @@ -110,15 +140,13 @@ export function useEsqlMode({ const { viewMode } = stateContainer.appState.getState(); const changeViewMode = viewMode !== getValidViewMode({ viewMode, isEsqlMode: true }); - if (indexPatternChanged) { - stateContainer.internalState.transitions.setResetDefaultProfileState({ - columns: true, - rowHeight: true, - }); - } else if (allColumnsChanged) { + // If the index pattern hasn't changed, but the available columns have changed + // due to transformational commands, reset the associated default profile state + if (!indexPatternChanged && allColumnsChanged) { stateContainer.internalState.transitions.setResetDefaultProfileState({ columns: true, rowHeight: false, + breakdownField: false, }); } diff --git a/src/plugins/discover/public/application/main/state_management/discover_app_state_container.test.ts b/src/plugins/discover/public/application/main/state_management/discover_app_state_container.test.ts index c190b09566be0..361688f10c393 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_app_state_container.test.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_app_state_container.test.ts @@ -272,11 +272,13 @@ describe('Test discover app state container', () => { expect(internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }); state.initAndSync(); expect(internalState.get().resetDefaultProfileState).toEqual({ columns: true, rowHeight: true, + breakdownField: true, }); }); @@ -287,11 +289,13 @@ describe('Test discover app state container', () => { expect(internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }); state.initAndSync(); expect(internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: true, + breakdownField: true, }); }); @@ -302,11 +306,13 @@ describe('Test discover app state container', () => { expect(internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }); state.initAndSync(); expect(internalState.get().resetDefaultProfileState).toEqual({ columns: true, rowHeight: false, + breakdownField: true, }); }); @@ -323,11 +329,13 @@ describe('Test discover app state container', () => { expect(internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }); state.initAndSync(); expect(internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }); }); }); diff --git a/src/plugins/discover/public/application/main/state_management/discover_app_state_container.ts b/src/plugins/discover/public/application/main/state_management/discover_app_state_container.ts index bf49d699126ab..2e2fd0c6be084 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_app_state_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_app_state_container.ts @@ -263,11 +263,12 @@ export const getDiscoverAppStateContainer = ({ addLog('[appState] initialize state and sync with URL', currentSavedSearch); if (!currentSavedSearch.id) { - const { columns, rowHeight } = getCurrentUrlState(stateStorage, services); + const { breakdownField, columns, rowHeight } = getCurrentUrlState(stateStorage, services); internalStateContainer.transitions.setResetDefaultProfileState({ columns: columns === undefined, rowHeight: rowHeight === undefined, + breakdownField: breakdownField === undefined, }); } diff --git a/src/plugins/discover/public/application/main/state_management/discover_data_state_container.test.ts b/src/plugins/discover/public/application/main/state_management/discover_data_state_container.test.ts index 1bbb16ab3c9dd..40b688c0d2152 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_data_state_container.test.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_data_state_container.test.ts @@ -178,6 +178,7 @@ describe('test getDataStateContainer', () => { stateContainer.internalState.transitions.setResetDefaultProfileState({ columns: true, rowHeight: true, + breakdownField: true, }); dataState.data$.totalHits$.next({ @@ -192,6 +193,7 @@ describe('test getDataStateContainer', () => { expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }); expect(stateContainer.appState.get().columns).toEqual(['message', 'extension']); expect(stateContainer.appState.get().rowHeight).toEqual(3); @@ -209,6 +211,7 @@ describe('test getDataStateContainer', () => { stateContainer.internalState.transitions.setResetDefaultProfileState({ columns: false, rowHeight: false, + breakdownField: false, }); dataState.data$.totalHits$.next({ fetchStatus: FetchStatus.COMPLETE, @@ -221,6 +224,7 @@ describe('test getDataStateContainer', () => { expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: false, rowHeight: false, + breakdownField: false, }); expect(stateContainer.appState.get().columns).toEqual(['default_column']); expect(stateContainer.appState.get().rowHeight).toBeUndefined(); diff --git a/src/plugins/discover/public/application/main/state_management/discover_data_state_container.ts b/src/plugins/discover/public/application/main/state_management/discover_data_state_container.ts index 4d01c2b72d327..5bc27fdb45a60 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_data_state_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_data_state_container.ts @@ -44,13 +44,6 @@ export interface SavedSearchData { export type DataMain$ = BehaviorSubject; export type DataDocuments$ = BehaviorSubject; export type DataTotalHits$ = BehaviorSubject; -export type DataFetch$ = Observable<{ - options: { - reset: boolean; - fetchMore: boolean; - }; - searchSessionId: string; -}>; export type DataRefetch$ = Subject; @@ -94,10 +87,6 @@ export interface DiscoverDataStateContainer { * Fetch more data from ES */ fetchMore: () => void; - /** - * Observable emitting when a next fetch is triggered - */ - fetch$: DataFetch$; /** * Container of data observables (orchestration, data table, total hits, available fields) */ @@ -106,6 +95,14 @@ export interface DiscoverDataStateContainer { * Observable triggering fetching data from ES */ refetch$: DataRefetch$; + /** + * Emits when the chart should be fetched + */ + fetchChart$: Observable; + /** + * Used to disable the next fetch that would otherwise be triggered by a URL state change + */ + disableNextFetchOnStateChange$: BehaviorSubject; /** * Start subscribing to other observables that trigger data fetches */ @@ -159,6 +156,8 @@ export function getDataStateContainer({ const { data, uiSettings, toastNotifications, profilesManager } = services; const { timefilter } = data.query.timefilter; const inspectorAdapters = { requests: new RequestAdapter() }; + const fetchChart$ = new Subject(); + const disableNextFetchOnStateChange$ = new BehaviorSubject(false); /** * The observable to trigger data fetching in UI @@ -266,6 +265,22 @@ export function getDataStateContainer({ query: appStateContainer.getState().query, }); + const { resetDefaultProfileState, dataView } = internalStateContainer.getState(); + const defaultProfileState = dataView + ? getDefaultProfileState({ profilesManager, resetDefaultProfileState, dataView }) + : undefined; + const preFetchStateUpdate = defaultProfileState?.getPreFetchState(); + + if (preFetchStateUpdate) { + disableNextFetchOnStateChange$.next(true); + await appStateContainer.replaceUrlState(preFetchStateUpdate); + disableNextFetchOnStateChange$.next(false); + } + + // Trigger chart fetching after the pre fetch state has been updated + // to ensure state values that would affect data fetching are set + fetchChart$.next(); + abortController = new AbortController(); const prevAutoRefreshDone = autoRefreshDone; const fetchAllStartTime = window.performance.now(); @@ -278,36 +293,24 @@ export function getDataStateContainer({ ...commonFetchDeps, }, async () => { - const { resetDefaultProfileState, dataView } = internalStateContainer.getState(); const { esqlQueryColumns } = dataSubjects.documents$.getValue(); const defaultColumns = uiSettings.get(DEFAULT_COLUMNS_SETTING, []); - const clearResetProfileState = () => { - internalStateContainer.transitions.setResetDefaultProfileState({ - columns: false, - rowHeight: false, - }); - }; - - if (!dataView) { - clearResetProfileState(); - return; - } - - const stateUpdate = getDefaultProfileState({ - profilesManager, - resetDefaultProfileState, + const postFetchStateUpdate = defaultProfileState?.getPostFetchState({ defaultColumns, - dataView, esqlQueryColumns, }); - if (!stateUpdate) { - clearResetProfileState(); - return; + if (postFetchStateUpdate) { + await appStateContainer.replaceUrlState(postFetchStateUpdate); } - await appStateContainer.replaceUrlState(stateUpdate); - clearResetProfileState(); + // Clear the default profile state flags after the data fetching + // is done so refetches don't reset the state again + internalStateContainer.transitions.setResetDefaultProfileState({ + columns: false, + rowHeight: false, + breakdownField: false, + }); } ); @@ -377,9 +380,10 @@ export function getDataStateContainer({ return { fetch: fetchQuery, fetchMore, - fetch$, data$: dataSubjects, refetch$, + fetchChart$, + disableNextFetchOnStateChange$, subscribe, reset, inspectorAdapters, diff --git a/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts b/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts index d5f7deecf980a..67b6551e985f4 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_internal_state_container.ts @@ -26,7 +26,7 @@ export interface InternalState { customFilters: Filter[]; overriddenVisContextAfterInvalidation: UnifiedHistogramVisContext | {} | undefined; // it will be used during saved search saving isESQLToDataViewTransitionModalVisible?: boolean; - resetDefaultProfileState: { columns: boolean; rowHeight: boolean }; + resetDefaultProfileState: { columns: boolean; rowHeight: boolean; breakdownField: boolean }; } export interface InternalStateTransitions { @@ -77,7 +77,7 @@ export function getInternalStateContainer() { expandedDoc: undefined, customFilters: [], overriddenVisContextAfterInvalidation: undefined, - resetDefaultProfileState: { columns: false, rowHeight: false }, + resetDefaultProfileState: { columns: false, rowHeight: false, breakdownField: false }, }, { setDataView: (prevState: InternalState) => (nextDataView: DataView) => ({ diff --git a/src/plugins/discover/public/application/main/state_management/discover_state.test.ts b/src/plugins/discover/public/application/main/state_management/discover_state.test.ts index a64e36bc39097..538ea0687b88e 100644 --- a/src/plugins/discover/public/application/main/state_management/discover_state.test.ts +++ b/src/plugins/discover/public/application/main/state_management/discover_state.test.ts @@ -99,10 +99,12 @@ describe('Test discover state', () => { state.appState.update({}, true); stopSync = startSync(state.appState); }); + afterEach(() => { stopSync(); stopSync = () => {}; }); + test('setting app state and syncing to URL', async () => { state.appState.update({ dataSource: createDataViewDataSource({ dataViewId: 'modified' }), @@ -124,6 +126,7 @@ describe('Test discover state', () => { } `); }); + test('URL navigation to url without _a, state should not change', async () => { history.push('/#?_a=(dataSource:(dataViewId:modified,type:dataView))'); history.push('/'); @@ -241,6 +244,7 @@ describe('Test discover initial state sort handling', () => { expect(state.appState.getState().sort).toEqual([['timestamp', 'desc']]); unsubscribe(); }); + test('Empty URL should use saved search sort for state', async () => { const nextSavedSearch = { ...savedSearchMock, ...{ sort: [['bytes', 'desc']] as SortOrder[] } }; const { state } = await getState('/', { savedSearch: nextSavedSearch }); @@ -436,10 +440,12 @@ describe('Test discover state actions', () => { expect(dataState.data$.totalHits$.value.result).toBe(0); expect(dataState.data$.documents$.value.result).toEqual([]); }); + test('loadDataViewList', async () => { const { state } = await getState(''); expect(state.internalState.getState().savedDataViews.length).toBe(3); }); + test('loadSavedSearch with no id given an empty URL', async () => { const { state, getCurrentUrl } = await getState(''); await state.actions.loadDataViewList(); @@ -486,6 +492,7 @@ describe('Test discover state actions', () => { expect(state.savedSearchState.getHasChanged$().getValue()).toBe(false); unsubscribe(); }); + test('loadNewSavedSearch with URL changing interval state', async () => { const { state, getCurrentUrl } = await getState( '/#?_a=(interval:month,columns:!(bytes))&_g=()', @@ -501,6 +508,7 @@ describe('Test discover state actions', () => { expect(state.savedSearchState.getHasChanged$().getValue()).toBe(true); unsubscribe(); }); + test('loadSavedSearch with no id, given URL changes state', async () => { const { state, getCurrentUrl } = await getState( '/#?_a=(interval:month,columns:!(bytes))&_g=()', @@ -516,6 +524,7 @@ describe('Test discover state actions', () => { expect(state.savedSearchState.getHasChanged$().getValue()).toBe(true); unsubscribe(); }); + test('loadSavedSearch given an empty URL, no state changes', async () => { const { state, getCurrentUrl } = await getState('/', { savedSearch: savedSearchMock }); const newSavedSearch = await state.actions.loadSavedSearch({ @@ -530,6 +539,7 @@ describe('Test discover state actions', () => { expect(state.savedSearchState.getHasChanged$().getValue()).toBe(false); unsubscribe(); }); + test('loadSavedSearch given a URL with different interval and columns modifying the state', async () => { const url = '/#?_a=(interval:month,columns:!(message))&_g=()'; const { state, getCurrentUrl } = await getState(url, { @@ -683,6 +693,7 @@ describe('Test discover state actions', () => { ); expect(state.savedSearchState.getHasChanged$().getValue()).toBe(true); }); + test('loadSavedSearch generating a new saved search, updated by ad-hoc data view', async () => { const { state } = await getState('/'); const dataViewSpecMock = { @@ -810,6 +821,7 @@ describe('Test discover state actions', () => { expect(getCurrentUrl()).toContain(dataViewComplexMock.id); unsubscribe(); }); + test('onDataViewCreated - persisted data view', async () => { const { state } = await getState('/', { savedSearch: savedSearchMock }); await state.actions.loadSavedSearch({ savedSearchId: savedSearchMock.id }); @@ -826,6 +838,7 @@ describe('Test discover state actions', () => { ); unsubscribe(); }); + test('onDataViewCreated - ad-hoc data view', async () => { const { state } = await getState('/', { savedSearch: savedSearchMock }); await state.actions.loadSavedSearch({ savedSearchId: savedSearchMock.id }); @@ -842,6 +855,7 @@ describe('Test discover state actions', () => { ); unsubscribe(); }); + test('onDataViewEdited - persisted data view', async () => { const { state } = await getState('/', { savedSearch: savedSearchMock }); await state.actions.loadSavedSearch({ savedSearchId: savedSearchMock.id }); @@ -857,6 +871,7 @@ describe('Test discover state actions', () => { }); unsubscribe(); }); + test('onDataViewEdited - ad-hoc data view', async () => { const { state } = await getState('/', { savedSearch: savedSearchMock }); const unsubscribe = state.actions.initializeAndSync(); @@ -903,6 +918,7 @@ describe('Test discover state actions', () => { expect(state.internalState.getState().adHocDataViews[0].id).toBe('ad-hoc-id'); unsubscribe(); }); + test('undoSavedSearchChanges - when changing data views', async () => { const { state, getCurrentUrl } = await getState('/', { savedSearch: savedSearchMock }); // Load a given persisted saved search diff --git a/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts b/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts index 841b0bb513e4c..f809dd2fe3ff4 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/build_state_subscribe.ts @@ -158,6 +158,17 @@ export const buildStateSubscribe = queryChanged: logEntry(queryChanged, prevQuery, nextQuery), }; + if (dataState.disableNextFetchOnStateChange$.getValue()) { + addLog( + '[buildStateSubscribe] next fetch skipped on state change', + JSON.stringify(logData, null, 2) + ); + + dataState.disableNextFetchOnStateChange$.next(false); + + return; + } + addLog( '[buildStateSubscribe] state changes triggers data fetching', JSON.stringify(logData, null, 2) diff --git a/src/plugins/discover/public/application/main/state_management/utils/change_data_view.test.ts b/src/plugins/discover/public/application/main/state_management/utils/change_data_view.test.ts index ff25a77930ebb..184ece84c8414 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/change_data_view.test.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/change_data_view.test.ts @@ -81,6 +81,7 @@ describe('changeDataView', () => { expect(params.internalState.transitions.setResetDefaultProfileState).toHaveBeenCalledWith({ columns: true, rowHeight: true, + breakdownField: true, }); }); }); diff --git a/src/plugins/discover/public/application/main/state_management/utils/change_data_view.ts b/src/plugins/discover/public/application/main/state_management/utils/change_data_view.ts index 54885c5de9bfd..d2815800fa9c6 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/change_data_view.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/change_data_view.ts @@ -43,7 +43,11 @@ export async function changeDataView( let nextDataView: DataView | null = null; internalState.transitions.setIsDataViewLoading(true); - internalState.transitions.setResetDefaultProfileState({ columns: true, rowHeight: true }); + internalState.transitions.setResetDefaultProfileState({ + columns: true, + rowHeight: true, + breakdownField: true, + }); try { nextDataView = typeof id === 'string' ? await dataViews.get(id, false) : id; diff --git a/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts b/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts index 883cb558aab84..6ba709bdd04ec 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts @@ -22,82 +22,119 @@ const { profilesManagerMock } = createContextAwarenessMocks(); profilesManagerMock.resolveDataSourceProfile({}); describe('getDefaultProfileState', () => { - it('should return expected columns', () => { - let appState = getDefaultProfileState({ - profilesManager: profilesManagerMock, - resetDefaultProfileState: { - columns: true, - rowHeight: false, - }, - defaultColumns: ['messsage', 'bytes'], - dataView: dataViewWithTimefieldMock, - esqlQueryColumns: undefined, + describe('getPreFetchState', () => { + it('should return expected breakdownField', () => { + let appState = getDefaultProfileState({ + profilesManager: profilesManagerMock, + resetDefaultProfileState: { + columns: false, + rowHeight: false, + breakdownField: true, + }, + dataView: dataViewWithTimefieldMock, + }).getPreFetchState(); + expect(appState).toEqual({ + breakdownField: 'extension', + }); + appState = getDefaultProfileState({ + profilesManager: profilesManagerMock, + resetDefaultProfileState: { + columns: false, + rowHeight: false, + breakdownField: true, + }, + dataView: emptyDataView, + }).getPreFetchState(); + expect(appState).toEqual(undefined); }); - expect(appState).toEqual({ - columns: ['message', 'extension', 'bytes'], - grid: { - columns: { - extension: { - width: 200, - }, - message: { - width: 100, + }); + + describe('getPostFetchState', () => { + it('should return expected columns', () => { + let appState = getDefaultProfileState({ + profilesManager: profilesManagerMock, + resetDefaultProfileState: { + columns: true, + rowHeight: false, + breakdownField: false, + }, + dataView: dataViewWithTimefieldMock, + }).getPostFetchState({ + defaultColumns: ['messsage', 'bytes'], + esqlQueryColumns: undefined, + }); + expect(appState).toEqual({ + columns: ['message', 'extension', 'bytes'], + grid: { + columns: { + extension: { + width: 200, + }, + message: { + width: 100, + }, }, }, - }, - }); - appState = getDefaultProfileState({ - profilesManager: profilesManagerMock, - resetDefaultProfileState: { - columns: true, - rowHeight: false, - }, - defaultColumns: ['messsage', 'bytes'], - dataView: emptyDataView, - esqlQueryColumns: [ - { id: '1', name: 'foo', meta: { type: 'string' } }, - { id: '2', name: 'bar', meta: { type: 'string' } }, - ], - }); - expect(appState).toEqual({ - columns: ['foo', 'bar'], - grid: { - columns: { - foo: { - width: 300, + }); + appState = getDefaultProfileState({ + profilesManager: profilesManagerMock, + resetDefaultProfileState: { + columns: true, + rowHeight: false, + breakdownField: false, + }, + dataView: emptyDataView, + }).getPostFetchState({ + defaultColumns: ['messsage', 'bytes'], + esqlQueryColumns: [ + { id: '1', name: 'foo', meta: { type: 'string' } }, + { id: '2', name: 'bar', meta: { type: 'string' } }, + ], + }); + expect(appState).toEqual({ + columns: ['foo', 'bar'], + grid: { + columns: { + foo: { + width: 300, + }, }, }, - }, + }); }); - }); - it('should return expected rowHeight', () => { - const appState = getDefaultProfileState({ - profilesManager: profilesManagerMock, - resetDefaultProfileState: { - columns: false, - rowHeight: true, - }, - defaultColumns: [], - dataView: dataViewWithTimefieldMock, - esqlQueryColumns: undefined, - }); - expect(appState).toEqual({ - rowHeight: 3, + it('should return expected rowHeight', () => { + const appState = getDefaultProfileState({ + profilesManager: profilesManagerMock, + resetDefaultProfileState: { + columns: false, + rowHeight: true, + breakdownField: false, + }, + dataView: dataViewWithTimefieldMock, + }).getPostFetchState({ + defaultColumns: [], + esqlQueryColumns: undefined, + }); + expect(appState).toEqual({ + rowHeight: 3, + }); }); - }); - it('should return undefined', () => { - const appState = getDefaultProfileState({ - profilesManager: profilesManagerMock, - resetDefaultProfileState: { - columns: false, - rowHeight: false, - }, - defaultColumns: [], - dataView: dataViewWithTimefieldMock, - esqlQueryColumns: undefined, + it('should return undefined', () => { + const appState = getDefaultProfileState({ + profilesManager: profilesManagerMock, + resetDefaultProfileState: { + columns: false, + rowHeight: false, + breakdownField: false, + }, + dataView: dataViewWithTimefieldMock, + }).getPostFetchState({ + defaultColumns: [], + esqlQueryColumns: undefined, + }); + expect(appState).toEqual(undefined); }); - expect(appState).toEqual(undefined); }); }); diff --git a/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.ts b/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.ts index 621ee2edf3240..37da88122ba19 100644 --- a/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.ts +++ b/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.ts @@ -22,48 +22,79 @@ import type { DataDocumentsMsg } from '../discover_data_state_container'; export const getDefaultProfileState = ({ profilesManager, resetDefaultProfileState, - defaultColumns, dataView, - esqlQueryColumns, }: { profilesManager: ProfilesManager; resetDefaultProfileState: InternalState['resetDefaultProfileState']; - defaultColumns: string[]; dataView: DataView; - esqlQueryColumns: DataDocumentsMsg['esqlQueryColumns']; }) => { - const stateUpdate: DiscoverAppState = {}; const defaultState = getDefaultState(profilesManager, dataView); - if (resetDefaultProfileState.columns) { - const mappedDefaultColumns = defaultColumns.map((name) => ({ name })); - const isValidColumn = getIsValidColumn(dataView, esqlQueryColumns); - const validColumns = uniqBy( - defaultState.columns?.concat(mappedDefaultColumns).filter(isValidColumn), - 'name' - ); + return { + /** + * Returns state that should be updated before data fetching occurs, + * for example state used as part of the data fetching process + * @returns The state to reset to before fetching data + */ + getPreFetchState: () => { + const stateUpdate: DiscoverAppState = {}; - if (validColumns?.length) { - const hasAutoWidthColumn = validColumns.some(({ width }) => !width); - const columns = validColumns.reduce( - (acc, { name, width }, index) => { - // Ensure there's at least one auto width column so the columns fill the grid - const skipColumnWidth = !hasAutoWidthColumn && index === validColumns.length - 1; - return width && !skipColumnWidth ? { ...acc, [name]: { width } } : acc; - }, - undefined - ); + if ( + resetDefaultProfileState.breakdownField && + defaultState.breakdownField !== undefined && + dataView.fields.getByName(defaultState.breakdownField) + ) { + stateUpdate.breakdownField = defaultState.breakdownField; + } - stateUpdate.grid = columns ? { columns } : undefined; - stateUpdate.columns = validColumns.map(({ name }) => name); - } - } + return Object.keys(stateUpdate).length ? stateUpdate : undefined; + }, - if (resetDefaultProfileState.rowHeight && defaultState.rowHeight !== undefined) { - stateUpdate.rowHeight = defaultState.rowHeight; - } + /** + * Returns state that should be updated after data fetching occurs, + * for example state used to modify the UI after receiving data + * @returns The state to reset to after fetching data + */ + getPostFetchState: ({ + defaultColumns, + esqlQueryColumns, + }: { + defaultColumns: string[]; + esqlQueryColumns: DataDocumentsMsg['esqlQueryColumns']; + }) => { + const stateUpdate: DiscoverAppState = {}; - return Object.keys(stateUpdate).length ? stateUpdate : undefined; + if (resetDefaultProfileState.columns) { + const mappedDefaultColumns = defaultColumns.map((name) => ({ name })); + const isValidColumn = getIsValidColumn(dataView, esqlQueryColumns); + const validColumns = uniqBy( + defaultState.columns?.concat(mappedDefaultColumns).filter(isValidColumn), + 'name' + ); + + if (validColumns?.length) { + const hasAutoWidthColumn = validColumns.some(({ width }) => !width); + const columns = validColumns.reduce( + (acc, { name, width }, index) => { + // Ensure there's at least one auto width column so the columns fill the grid + const skipColumnWidth = !hasAutoWidthColumn && index === validColumns.length - 1; + return width && !skipColumnWidth ? { ...acc, [name]: { width } } : acc; + }, + undefined + ); + + stateUpdate.grid = columns ? { columns } : undefined; + stateUpdate.columns = validColumns.map(({ name }) => name); + } + } + + if (resetDefaultProfileState.rowHeight && defaultState.rowHeight !== undefined) { + stateUpdate.rowHeight = defaultState.rowHeight; + } + + return Object.keys(stateUpdate).length ? stateUpdate : undefined; + }, + }; }; const getDefaultState = (profilesManager: ProfilesManager, dataView: DataView) => { diff --git a/src/plugins/discover/public/context_awareness/__mocks__/index.tsx b/src/plugins/discover/public/context_awareness/__mocks__/index.tsx index 153d401cc980a..8fb4a0bd769aa 100644 --- a/src/plugins/discover/public/context_awareness/__mocks__/index.tsx +++ b/src/plugins/discover/public/context_awareness/__mocks__/index.tsx @@ -84,6 +84,7 @@ export const createContextAwarenessMocks = ({ }, ], rowHeight: 3, + breakdownField: 'extension', })), getAdditionalCellActions: jest.fn((prev) => () => [ ...prev(), diff --git a/src/plugins/discover/public/context_awareness/profile_providers/example/example_data_source_profile/profile.tsx b/src/plugins/discover/public/context_awareness/profile_providers/example/example_data_source_profile/profile.tsx index 46ecce387e877..07c836b127843 100644 --- a/src/plugins/discover/public/context_awareness/profile_providers/example/example_data_source_profile/profile.tsx +++ b/src/plugins/discover/public/context_awareness/profile_providers/example/example_data_source_profile/profile.tsx @@ -220,6 +220,7 @@ export const createExampleDataSourceProfileProvider = (): DataSourceProfileProvi ]; }, getDefaultAppState: () => () => ({ + breakdownField: 'log.level', columns: [ { name: '@timestamp', diff --git a/src/plugins/discover/public/context_awareness/profile_providers/observability/logs_data_source_profile/accessors/get_default_app_state.ts b/src/plugins/discover/public/context_awareness/profile_providers/observability/logs_data_source_profile/accessors/get_default_app_state.ts index d9c08af91f5d8..11d5aab2763cd 100644 --- a/src/plugins/discover/public/context_awareness/profile_providers/observability/logs_data_source_profile/accessors/get_default_app_state.ts +++ b/src/plugins/discover/public/context_awareness/profile_providers/observability/logs_data_source_profile/accessors/get_default_app_state.ts @@ -7,6 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ +import { LOG_LEVEL_FIELD } from '@kbn/discover-utils'; import type { DataSourceProfileProvider } from '../../../../profiles'; import { DefaultAppStateColumn } from '../../../../types'; @@ -14,10 +15,12 @@ export const createGetDefaultAppState = ({ defaultColumns, }: { defaultColumns?: DefaultAppStateColumn[]; -}): DataSourceProfileProvider['profile']['getDefaultAppState'] => { +} = {}): DataSourceProfileProvider['profile']['getDefaultAppState'] => { return (prev) => (params) => { const appState = { ...prev(params) }; + appState.breakdownField = LOG_LEVEL_FIELD; + if (defaultColumns) { appState.columns = []; diff --git a/src/plugins/discover/public/context_awareness/profile_providers/observability/logs_data_source_profile/profile.ts b/src/plugins/discover/public/context_awareness/profile_providers/observability/logs_data_source_profile/profile.ts index bebd340acfb38..7601680d6155c 100644 --- a/src/plugins/discover/public/context_awareness/profile_providers/observability/logs_data_source_profile/profile.ts +++ b/src/plugins/discover/public/context_awareness/profile_providers/observability/logs_data_source_profile/profile.ts @@ -13,6 +13,7 @@ import { getCellRenderers, getRowIndicatorProvider, getRowAdditionalLeadingControls, + createGetDefaultAppState, } from './accessors'; import { extractIndexPatternFrom } from '../../extract_index_pattern_from'; import { OBSERVABILITY_ROOT_PROFILE_ID } from '../consts'; @@ -22,6 +23,7 @@ export const createLogsDataSourceProfileProvider = ( ): DataSourceProfileProvider => ({ profileId: 'observability-logs-data-source-profile', profile: { + getDefaultAppState: createGetDefaultAppState(), getCellRenderers, getRowIndicatorProvider, getRowAdditionalLeadingControls, diff --git a/src/plugins/discover/public/context_awareness/types.ts b/src/plugins/discover/public/context_awareness/types.ts index 4b75e6473aafd..51034e97155b6 100644 --- a/src/plugins/discover/public/context_awareness/types.ts +++ b/src/plugins/discover/public/context_awareness/types.ts @@ -123,6 +123,10 @@ export interface DefaultAppStateExtension { * * 1-20: number of lines to display */ rowHeight?: number; + /** + * The field to apply for the histogram breakdown + */ + breakdownField?: string; } /** diff --git a/src/plugins/unified_histogram/public/container/container.tsx b/src/plugins/unified_histogram/public/container/container.tsx index 15367ae51d9b5..ce55d0773344e 100644 --- a/src/plugins/unified_histogram/public/container/container.tsx +++ b/src/plugins/unified_histogram/public/container/container.tsx @@ -28,6 +28,7 @@ import { useStateProps } from './hooks/use_state_props'; import { useStateSelector } from './utils/use_state_selector'; import { topPanelHeightSelector } from './utils/state_selectors'; import { exportVisContext } from '../utils/external_vis_context'; +import { getBreakdownField } from './utils/local_storage_utils'; type LayoutProps = Pick< UnifiedHistogramLayoutProps, @@ -50,6 +51,8 @@ export type UnifiedHistogramContainerProps = { searchSessionId?: UnifiedHistogramRequestContext['searchSessionId']; requestAdapter?: UnifiedHistogramRequestContext['adapter']; isChartLoading?: boolean; + breakdownField?: string; + onBreakdownFieldChange?: (breakdownField: string | undefined) => void; onVisContextChanged?: ( nextVisContext: UnifiedHistogramVisContext | undefined, externalVisContextStatus: UnifiedHistogramExternalVisContextStatus @@ -86,19 +89,15 @@ export type UnifiedHistogramApi = { refetch: () => void; } & Pick< UnifiedHistogramStateService, - | 'state$' - | 'setChartHidden' - | 'setTopPanelHeight' - | 'setBreakdownField' - | 'setTimeInterval' - | 'setTotalHits' + 'state$' | 'setChartHidden' | 'setTopPanelHeight' | 'setTimeInterval' | 'setTotalHits' >; export const UnifiedHistogramContainer = forwardRef< UnifiedHistogramApi, UnifiedHistogramContainerProps ->(({ onVisContextChanged, ...containerProps }, ref) => { +>(({ onBreakdownFieldChange, onVisContextChanged, ...containerProps }, ref) => { const [layoutProps, setLayoutProps] = useState(); + const [localStorageKeyPrefix, setLocalStorageKeyPrefix] = useState(); const [stateService, setStateService] = useState(); const [lensSuggestionsApi, setLensSuggestionsApi] = useState(); const [input$] = useState(() => new Subject()); @@ -114,6 +113,7 @@ export const UnifiedHistogramContainer = forwardRef< const apiHelper = await services.lens.stateHelperApi(); setLayoutProps(pick(options, 'disableAutoFetching', 'disableTriggers', 'disabledActions')); + setLocalStorageKeyPrefix(options?.localStorageKeyPrefix); setStateService(createStateService({ services, ...options })); setLensSuggestionsApi(() => apiHelper.suggestions); }); @@ -133,21 +133,34 @@ export const UnifiedHistogramContainer = forwardRef< 'state$', 'setChartHidden', 'setTopPanelHeight', - 'setBreakdownField', 'setTimeInterval', 'setTotalHits' ), }); }, [input$, stateService]); - const { dataView, query, searchSessionId, requestAdapter, isChartLoading } = containerProps; + + const { services, dataView, query, columns, searchSessionId, requestAdapter, isChartLoading } = + containerProps; const topPanelHeight = useStateSelector(stateService?.state$, topPanelHeightSelector); + const initialBreakdownField = useMemo( + () => + localStorageKeyPrefix + ? getBreakdownField(services.storage, localStorageKeyPrefix) + : undefined, + [localStorageKeyPrefix, services.storage] + ); const stateProps = useStateProps({ + services, + localStorageKeyPrefix, stateService, dataView, query, searchSessionId, requestAdapter, - columns: containerProps.columns, + columns, + breakdownField: initialBreakdownField, + ...pick(containerProps, 'breakdownField'), + onBreakdownFieldChange, }); const handleVisContextChange: UnifiedHistogramLayoutProps['onVisContextChanged'] | undefined = diff --git a/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts b/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts index 7a2d36da4b1fe..f57392998d1f5 100644 --- a/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts +++ b/src/plugins/unified_histogram/public/container/hooks/use_state_props.test.ts @@ -26,7 +26,6 @@ import { useStateProps } from './use_state_props'; describe('useStateProps', () => { const initialState: UnifiedHistogramState = { - breakdownField: 'bytes', chartHidden: false, lensRequestAdapter: new RequestAdapter(), lensAdapters: lensAdaptersMock, @@ -44,7 +43,6 @@ describe('useStateProps', () => { }); jest.spyOn(stateService, 'setChartHidden'); jest.spyOn(stateService, 'setTopPanelHeight'); - jest.spyOn(stateService, 'setBreakdownField'); jest.spyOn(stateService, 'setTimeInterval'); jest.spyOn(stateService, 'setLensRequestAdapter'); jest.spyOn(stateService, 'setTotalHits'); @@ -56,25 +54,22 @@ describe('useStateProps', () => { const stateService = getStateService({ initialState }); const { result } = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewWithTimefieldMock, query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` Object { "breakdown": Object { - "field": Object { - "aggregatable": true, - "displayName": "bytes", - "filterable": true, - "name": "bytes", - "scripted": false, - "type": "number", - }, + "field": undefined, }, "chart": Object { "hidden": false, @@ -147,12 +142,16 @@ describe('useStateProps', () => { const stateService = getStateService({ initialState }); const { result } = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewWithTimefieldMock, query: { esql: 'FROM index' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` @@ -240,12 +239,16 @@ describe('useStateProps', () => { }); const { result } = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewWithTimefieldMock, query: { esql: 'FROM index | keep field1' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); expect(result.current.chart).toStrictEqual({ hidden: false, timeInterval: 'auto' }); @@ -271,17 +274,20 @@ describe('useStateProps', () => { initialState: { ...initialState, currentSuggestionContext: undefined, - breakdownField, }, }); const { result } = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewWithTimefieldMock, query: { esql: 'FROM index' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: esqlColumns, + breakdownField, + onBreakdownFieldChange: undefined, }) ); @@ -314,25 +320,30 @@ describe('useStateProps', () => { }); const { result } = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewWithTimefieldMock, query: { esql: 'FROM index' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: esqlColumns, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); const { onBreakdownFieldChange } = result.current; act(() => { onBreakdownFieldChange({ name: breakdownField } as DataViewField); }); - expect(stateService.setBreakdownField).toHaveBeenLastCalledWith(breakdownField); }); it('should return the correct props when a rollup data view is used', () => { const stateService = getStateService({ initialState }); const { result } = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: { ...dataViewWithTimefieldMock, @@ -342,6 +353,8 @@ describe('useStateProps', () => { requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` @@ -415,12 +428,16 @@ describe('useStateProps', () => { const stateService = getStateService({ initialState }); const { result } = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewMock, query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` @@ -494,12 +511,16 @@ describe('useStateProps', () => { const stateService = getStateService({ initialState }); const { result } = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewWithTimefieldMock, query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); @@ -553,8 +574,6 @@ describe('useStateProps', () => { act(() => { onBreakdownFieldChange({ name: 'field' } as DataViewField); }); - expect(stateService.setBreakdownField).toHaveBeenLastCalledWith('field'); - act(() => { onSuggestionContextChange({ suggestion: { title: 'Stacked Bar' }, @@ -569,12 +588,16 @@ describe('useStateProps', () => { const stateService = getStateService({ initialState }); const hook = renderHook(() => useStateProps({ + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewWithTimefieldMock, query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); (stateService.setLensRequestAdapter as jest.Mock).mockClear(); @@ -589,12 +612,16 @@ describe('useStateProps', () => { it('should clear lensRequestAdapter when chart is undefined', () => { const stateService = getStateService({ initialState }); const initialProps = { + services: unifiedHistogramServicesMock, + localStorageKeyPrefix: undefined, stateService, dataView: dataViewWithTimefieldMock, query: { language: 'kuery', query: '' }, requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }; const hook = renderHook((props: Parameters[0]) => useStateProps(props), { initialProps, diff --git a/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts b/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts index 660e47f33cf0c..46244d69d1f89 100644 --- a/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts +++ b/src/plugins/unified_histogram/public/container/hooks/use_state_props.ts @@ -17,11 +17,11 @@ import { useCallback, useEffect, useMemo } from 'react'; import { UnifiedHistogramChartLoadEvent, UnifiedHistogramFetchStatus, + UnifiedHistogramServices, UnifiedHistogramSuggestionContext, } from '../../types'; import type { UnifiedHistogramStateService } from '../services/state_service'; import { - breakdownFieldSelector, chartHiddenSelector, timeIntervalSelector, totalHitsResultSelector, @@ -30,23 +30,31 @@ import { lensDataLoadingSelector$, } from '../utils/state_selectors'; import { useStateSelector } from '../utils/use_state_selector'; +import { setBreakdownField } from '../utils/local_storage_utils'; export const useStateProps = ({ + services, + localStorageKeyPrefix, stateService, dataView, query, searchSessionId, requestAdapter, columns, + breakdownField, + onBreakdownFieldChange: originalOnBreakdownFieldChange, }: { + services: UnifiedHistogramServices; + localStorageKeyPrefix: string | undefined; stateService: UnifiedHistogramStateService | undefined; dataView: DataView; query: Query | AggregateQuery | undefined; searchSessionId: string | undefined; requestAdapter: RequestAdapter | undefined; columns: DatatableColumn[] | undefined; + breakdownField: string | undefined; + onBreakdownFieldChange: ((breakdownField: string | undefined) => void) | undefined; }) => { - const breakdownField = useStateSelector(stateService?.state$, breakdownFieldSelector); const chartHidden = useStateSelector(stateService?.state$, chartHiddenSelector); const timeInterval = useStateSelector(stateService?.state$, timeIntervalSelector); const totalHitsResult = useStateSelector(stateService?.state$, totalHitsResultSelector); @@ -166,9 +174,9 @@ export const useStateProps = ({ const onBreakdownFieldChange = useCallback( (newBreakdownField: DataViewField | undefined) => { - stateService?.setBreakdownField(newBreakdownField?.name); + originalOnBreakdownFieldChange?.(newBreakdownField?.name); }, - [stateService] + [originalOnBreakdownFieldChange] ); const onSuggestionContextChange = useCallback( @@ -182,6 +190,13 @@ export const useStateProps = ({ * Effects */ + // Sync the breakdown field with local storage + useEffect(() => { + if (localStorageKeyPrefix) { + setBreakdownField(services.storage, localStorageKeyPrefix, breakdownField); + } + }, [breakdownField, localStorageKeyPrefix, services.storage]); + // Clear the Lens request adapter when the chart is hidden useEffect(() => { if (chartHidden || !chart) { diff --git a/src/plugins/unified_histogram/public/container/services/state_service.test.ts b/src/plugins/unified_histogram/public/container/services/state_service.test.ts index 66f0549e9571f..5c3024eef7ddb 100644 --- a/src/plugins/unified_histogram/public/container/services/state_service.test.ts +++ b/src/plugins/unified_histogram/public/container/services/state_service.test.ts @@ -14,10 +14,8 @@ import { lensAdaptersMock } from '../../__mocks__/lens_adapters'; import { getChartHidden, getTopPanelHeight, - getBreakdownField, setChartHidden, setTopPanelHeight, - setBreakdownField, } from '../utils/local_storage_utils'; import { createStateService, UnifiedHistogramState } from './state_service'; @@ -27,10 +25,8 @@ jest.mock('../utils/local_storage_utils', () => { ...originalModule, getChartHidden: jest.fn(originalModule.getChartHidden), getTopPanelHeight: jest.fn(originalModule.getTopPanelHeight), - getBreakdownField: jest.fn(originalModule.getBreakdownField), setChartHidden: jest.fn(originalModule.setChartHidden), setTopPanelHeight: jest.fn(originalModule.setTopPanelHeight), - setBreakdownField: jest.fn(originalModule.setBreakdownField), }; }); @@ -38,14 +34,11 @@ describe('UnifiedHistogramStateService', () => { beforeEach(() => { (getChartHidden as jest.Mock).mockClear(); (getTopPanelHeight as jest.Mock).mockClear(); - (getBreakdownField as jest.Mock).mockClear(); (setChartHidden as jest.Mock).mockClear(); (setTopPanelHeight as jest.Mock).mockClear(); - (setBreakdownField as jest.Mock).mockClear(); }); const initialState: UnifiedHistogramState = { - breakdownField: 'bytes', chartHidden: false, lensRequestAdapter: new RequestAdapter(), lensAdapters: lensAdaptersMock, @@ -61,7 +54,6 @@ describe('UnifiedHistogramStateService', () => { let state: UnifiedHistogramState | undefined; stateService.state$.subscribe((s) => (state = s)); expect(state).toEqual({ - breakdownField: undefined, chartHidden: false, lensRequestAdapter: undefined, timeInterval: 'auto', @@ -97,10 +89,6 @@ describe('UnifiedHistogramStateService', () => { unifiedHistogramServicesMock.storage, localStorageKeyPrefix ); - expect(getBreakdownField as jest.Mock).toHaveBeenCalledWith( - unifiedHistogramServicesMock.storage, - localStorageKeyPrefix - ); }); it('should not get values from storage if localStorageKeyPrefix is not provided', () => { @@ -110,7 +98,6 @@ describe('UnifiedHistogramStateService', () => { }); expect(getChartHidden as jest.Mock).not.toHaveBeenCalled(); expect(getTopPanelHeight as jest.Mock).not.toHaveBeenCalled(); - expect(getBreakdownField as jest.Mock).not.toHaveBeenCalled(); }); it('should update state', () => { @@ -128,9 +115,6 @@ describe('UnifiedHistogramStateService', () => { stateService.setTopPanelHeight(200); newState = { ...newState, topPanelHeight: 200 }; expect(state).toEqual(newState); - stateService.setBreakdownField('test'); - newState = { ...newState, breakdownField: 'test' }; - expect(state).toEqual(newState); stateService.setTimeInterval('test'); newState = { ...newState, timeInterval: 'test' }; expect(state).toEqual(newState); @@ -166,12 +150,10 @@ describe('UnifiedHistogramStateService', () => { expect(state).toEqual(initialState); stateService.setChartHidden(true); stateService.setTopPanelHeight(200); - stateService.setBreakdownField('test'); expect(state).toEqual({ ...initialState, chartHidden: true, topPanelHeight: 200, - breakdownField: 'test', }); expect(setChartHidden as jest.Mock).toHaveBeenCalledWith( unifiedHistogramServicesMock.storage, @@ -183,11 +165,6 @@ describe('UnifiedHistogramStateService', () => { localStorageKeyPrefix, 200 ); - expect(setBreakdownField as jest.Mock).toHaveBeenCalledWith( - unifiedHistogramServicesMock.storage, - localStorageKeyPrefix, - 'test' - ); }); it('should not save state to storage if localStorageKeyPrefix is not provided', () => { @@ -200,15 +177,12 @@ describe('UnifiedHistogramStateService', () => { expect(state).toEqual(initialState); stateService.setChartHidden(true); stateService.setTopPanelHeight(200); - stateService.setBreakdownField('test'); expect(state).toEqual({ ...initialState, chartHidden: true, topPanelHeight: 200, - breakdownField: 'test', }); expect(setChartHidden as jest.Mock).not.toHaveBeenCalled(); expect(setTopPanelHeight as jest.Mock).not.toHaveBeenCalled(); - expect(setBreakdownField as jest.Mock).not.toHaveBeenCalled(); }); }); diff --git a/src/plugins/unified_histogram/public/container/services/state_service.ts b/src/plugins/unified_histogram/public/container/services/state_service.ts index c3cf82bf94578..cdca02396e3a9 100644 --- a/src/plugins/unified_histogram/public/container/services/state_service.ts +++ b/src/plugins/unified_histogram/public/container/services/state_service.ts @@ -13,10 +13,8 @@ import { PublishingSubject } from '@kbn/presentation-publishing'; import { UnifiedHistogramFetchStatus } from '../..'; import type { UnifiedHistogramServices, UnifiedHistogramChartLoadEvent } from '../../types'; import { - getBreakdownField, getChartHidden, getTopPanelHeight, - setBreakdownField, setChartHidden, setTopPanelHeight, } from '../utils/local_storage_utils'; @@ -26,10 +24,6 @@ import type { UnifiedHistogramSuggestionContext } from '../../types'; * The current state of the container */ export interface UnifiedHistogramState { - /** - * The current field used for the breakdown - */ - breakdownField: string | undefined; /** * The current Lens suggestion */ @@ -108,10 +102,6 @@ export interface UnifiedHistogramStateService { * Sets the current top panel height */ setTopPanelHeight: (topPanelHeight: number | undefined) => void; - /** - * Sets the current breakdown field - */ - setBreakdownField: (breakdownField: string | undefined) => void; /** * Sets the current time interval */ @@ -141,16 +131,13 @@ export const createStateService = ( let initialChartHidden = false; let initialTopPanelHeight: number | undefined; - let initialBreakdownField: string | undefined; if (localStorageKeyPrefix) { initialChartHidden = getChartHidden(services.storage, localStorageKeyPrefix) ?? false; initialTopPanelHeight = getTopPanelHeight(services.storage, localStorageKeyPrefix); - initialBreakdownField = getBreakdownField(services.storage, localStorageKeyPrefix); } const state$ = new BehaviorSubject({ - breakdownField: initialBreakdownField, chartHidden: initialChartHidden, currentSuggestionContext: undefined, lensRequestAdapter: undefined, @@ -187,14 +174,6 @@ export const createStateService = ( updateState({ topPanelHeight }); }, - setBreakdownField: (breakdownField: string | undefined) => { - if (localStorageKeyPrefix) { - setBreakdownField(services.storage, localStorageKeyPrefix, breakdownField); - } - - updateState({ breakdownField }); - }, - setCurrentSuggestionContext: ( suggestionContext: UnifiedHistogramSuggestionContext | undefined ) => { diff --git a/src/plugins/unified_histogram/public/container/utils/state_selectors.ts b/src/plugins/unified_histogram/public/container/utils/state_selectors.ts index 9274c4fabd301..c20ec4193c53c 100644 --- a/src/plugins/unified_histogram/public/container/utils/state_selectors.ts +++ b/src/plugins/unified_histogram/public/container/utils/state_selectors.ts @@ -9,7 +9,6 @@ import type { UnifiedHistogramState } from '../services/state_service'; -export const breakdownFieldSelector = (state: UnifiedHistogramState) => state.breakdownField; export const chartHiddenSelector = (state: UnifiedHistogramState) => state.chartHidden; export const timeIntervalSelector = (state: UnifiedHistogramState) => state.timeInterval; export const topPanelHeightSelector = (state: UnifiedHistogramState) => state.topPanelHeight; diff --git a/src/plugins/unified_histogram/public/mocks.ts b/src/plugins/unified_histogram/public/mocks.ts index 4772739e28361..11ebd50239257 100644 --- a/src/plugins/unified_histogram/public/mocks.ts +++ b/src/plugins/unified_histogram/public/mocks.ts @@ -15,7 +15,6 @@ export const createMockUnifiedHistogramApi = () => { state$: new Observable(), setChartHidden: jest.fn(), setTopPanelHeight: jest.fn(), - setBreakdownField: jest.fn(), setTimeInterval: jest.fn(), setTotalHits: jest.fn(), refetch: jest.fn(), diff --git a/test/functional/apps/discover/context_awareness/extensions/_get_default_app_state.ts b/test/functional/apps/discover/context_awareness/extensions/_get_default_app_state.ts index 7e7a12840f76d..5fd8e97c8f059 100644 --- a/test/functional/apps/discover/context_awareness/extensions/_get_default_app_state.ts +++ b/test/functional/apps/discover/context_awareness/extensions/_get_default_app_state.ts @@ -40,8 +40,15 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); } + function expectBreakdown(breakdown: string) { + return retry.try(async () => { + const breakdownFieldValue = await discover.getBreakdownFieldValue(); + expect(breakdownFieldValue).to.be(breakdown); + }); + } + describe('ES|QL mode', () => { - it('should render default columns and row height', async () => { + it('should render default state', async () => { const state = kbnRison.encode({ dataSource: { type: 'esql' }, query: { @@ -57,9 +64,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); const rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); - it('should render default columns and row height when switching index patterns', async () => { + it('should render state when switching index patterns', async () => { const state = kbnRison.encode({ dataSource: { type: 'esql' }, query: { @@ -83,9 +91,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); - it('should reset default columns and row height when clicking "New"', async () => { + it('should reset default state when clicking "New"', async () => { const state = kbnRison.encode({ dataSource: { type: 'esql' }, query: { @@ -111,6 +120,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); const rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); it('should merge and dedup configured default columns with default profile columns', async () => { @@ -131,7 +141,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); describe('data view mode', () => { - it('should render default columns and row height', async () => { + it('should render default state', async () => { await common.navigateToActualUrl('discover', undefined, { ensureCurrentUrl: false, }); @@ -144,9 +154,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); const rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); - it('should render default columns and row height when switching data views', async () => { + it('should render default state when switching data views', async () => { await common.navigateToActualUrl('discover', undefined, { ensureCurrentUrl: false, }); @@ -166,9 +177,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); - it('should reset default columns and row height when clicking "New"', async () => { + it('should reset default state when clicking "New"', async () => { await common.navigateToActualUrl('discover', undefined, { ensureCurrentUrl: false, }); @@ -190,6 +202,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); const rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); it('should merge and dedup configured default columns with default profile columns', async () => { diff --git a/test/functional/page_objects/discover_page.ts b/test/functional/page_objects/discover_page.ts index 8feccfd955c7f..13289ae9b8aa5 100644 --- a/test/functional/page_objects/discover_page.ts +++ b/test/functional/page_objects/discover_page.ts @@ -224,6 +224,12 @@ export class DiscoverPageObject extends FtrService { ); } + public async getBreakdownFieldValue() { + const breakdownButton = await this.testSubjects.find('unifiedHistogramBreakdownSelectorButton'); + + return breakdownButton.getVisibleText(); + } + public async chooseBreakdownField(field: string, value?: string) { await this.retry.try(async () => { await this.testSubjects.click('unifiedHistogramBreakdownSelectorButton'); diff --git a/x-pack/test_serverless/functional/test_suites/common/discover/context_awareness/extensions/_get_default_app_state.ts b/x-pack/test_serverless/functional/test_suites/common/discover/context_awareness/extensions/_get_default_app_state.ts index 5867eb528bf8a..6bb8eaee1df95 100644 --- a/x-pack/test_serverless/functional/test_suites/common/discover/context_awareness/extensions/_get_default_app_state.ts +++ b/x-pack/test_serverless/functional/test_suites/common/discover/context_awareness/extensions/_get_default_app_state.ts @@ -43,8 +43,15 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); } + function expectBreakdown(breakdown: string) { + return retry.try(async () => { + const breakdownFieldValue = await PageObjects.discover.getBreakdownFieldValue(); + expect(breakdownFieldValue).to.be(breakdown); + }); + } + describe('ES|QL mode', () => { - it('should render default columns and row height', async () => { + it('should render default state', async () => { const state = kbnRison.encode({ dataSource: { type: 'esql' }, query: { @@ -60,9 +67,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); const rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); - it('should render default columns and row height when switching index patterns', async () => { + it('should render default state when switching index patterns', async () => { const state = kbnRison.encode({ dataSource: { type: 'esql' }, query: { @@ -86,9 +94,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); - it('should reset default columns and row height when clicking "New"', async () => { + it('should reset default state when clicking "New"', async () => { const state = kbnRison.encode({ dataSource: { type: 'esql' }, query: { @@ -114,6 +123,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); const rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); it('should merge and dedup configured default columns with default profile columns', async () => { @@ -134,7 +144,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { }); describe('data view mode', () => { - it('should render default columns and row height', async () => { + it('should render default state', async () => { await PageObjects.common.navigateToActualUrl('discover', undefined, { ensureCurrentUrl: false, }); @@ -147,9 +157,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); const rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); - it('should render default columns and row height when switching data views', async () => { + it('should render default state when switching data views', async () => { await PageObjects.common.navigateToActualUrl('discover', undefined, { ensureCurrentUrl: false, }); @@ -169,9 +180,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); - it('should reset default columns and row height when clicking "New"', async () => { + it('should reset default state when clicking "New"', async () => { await PageObjects.common.navigateToActualUrl('discover', undefined, { ensureCurrentUrl: false, }); @@ -194,6 +206,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(rowHeightValue).to.be('Custom'); const rowHeightNumber = await dataGrid.getCustomRowHeightNumber(); expect(rowHeightNumber).to.be(5); + await expectBreakdown('Breakdown by log.level'); }); it('should merge and dedup configured default columns with default profile columns', async () => {