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 e14b5afc30d04..00c3c7fc20fe3 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); @@ -168,7 +167,6 @@ describe('useDiscoverHistogram', () => { expect(Object.keys(params?.initialState ?? {})).toEqual([ 'chartHidden', 'timeInterval', - 'breakdownField', 'totalHitsStatus', 'totalHitsResult', ]); @@ -206,7 +204,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: '1m', chartHidden: true, - breakdownField: 'test', totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -219,7 +216,6 @@ describe('useDiscoverHistogram', () => { expect(stateContainer.appState.update).toHaveBeenCalledWith({ interval: state.timeInterval, hideChart: state.chartHidden, - breakdownField: state.breakdownField, }); }); @@ -230,7 +226,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: containerState.interval, chartHidden: containerState.hideChart, - breakdownField: containerState.breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -256,18 +251,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 () => { @@ -277,7 +268,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: containerState.interval, chartHidden: containerState.hideChart, - breakdownField: containerState.breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -312,7 +302,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: containerState.interval, chartHidden: containerState.hideChart, - breakdownField: containerState.breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -357,7 +346,6 @@ describe('useDiscoverHistogram', () => { const state = { timeInterval: containerState.interval, chartHidden: containerState.hideChart, - breakdownField: containerState.breakdownField, totalHitsStatus: UnifiedHistogramFetchStatus.loading, totalHitsResult: undefined, } as unknown as UnifiedHistogramState; @@ -383,22 +371,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); }); @@ -406,15 +385,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(() => { @@ -422,24 +395,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(() => { @@ -453,45 +417,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.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 2b5929eb3a45e..f8109d324bfe5 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 @@ -30,7 +30,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 }); @@ -39,7 +40,7 @@ function getHookProps( stateContainer.internalState.transitions.setSavedDataViews([dataViewMock as DataViewListItem]); const msgLoading = { - fetchStatus: FetchStatus.PARTIAL, + fetchStatus: defaultFetchStatus, query, }; stateContainer.dataState.data$.documents$.next(msgLoading); @@ -82,15 +83,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), @@ -492,7 +494,11 @@ 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, @@ -501,6 +507,11 @@ describe('useEsqlMode', () => { }); 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(() => @@ -510,13 +521,18 @@ describe('useEsqlMode', () => { 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(() => @@ -528,6 +544,11 @@ describe('useEsqlMode', () => { ); 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(() => @@ -537,6 +558,10 @@ describe('useEsqlMode', () => { breakdownField: true, }) ); + documents$.next({ + fetchStatus: FetchStatus.PARTIAL, + query: { esql: 'from pattern2' }, + }); }); it('should call setResetDefaultProfileState correctly when columns change', async () => { @@ -554,23 +579,6 @@ describe('useEsqlMode', () => { query: { esql: 'from pattern' }, result: result1, }); - await waitFor(() => - expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ - columns: true, - rowHeight: true, - breakdownField: true, - }) - ); - stateContainer.internalState.transitions.setResetDefaultProfileState({ - columns: false, - rowHeight: false, - breakdownField: false, - }); - documents$.next({ - fetchStatus: FetchStatus.PARTIAL, - query: { esql: 'from pattern' }, - result: result1, - }); await waitFor(() => expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: false, @@ -587,7 +595,7 @@ describe('useEsqlMode', () => { expect(stateContainer.internalState.get().resetDefaultProfileState).toEqual({ columns: true, rowHeight: false, - breakdownField: true, + 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 b32849868fdab..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,17 +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, - breakdownField: 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: true, + breakdownField: false, }); } 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 ccd4874540243..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,37 +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, - breakdownField: 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, + }); } ); @@ -378,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_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/get_default_profile_state.test.ts b/src/plugins/discover/public/application/main/state_management/utils/get_default_profile_state.test.ts index 06b8bdf6b00f3..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,103 +22,119 @@ const { profilesManagerMock } = createContextAwarenessMocks(); profilesManagerMock.resolveDataSourceProfile({}); describe('getDefaultProfileState', () => { - it('should return expected columns', () => { - let appState = getDefaultProfileState({ - profilesManager: profilesManagerMock, - resetDefaultProfileState: { - columns: true, - rowHeight: false, - breakdownField: false, - }, - defaultColumns: ['messsage', 'bytes'], - dataView: dataViewWithTimefieldMock, - esqlQueryColumns: undefined, - }); - expect(appState).toEqual({ - columns: ['message', 'extension', 'bytes'], - grid: { - columns: { - extension: { - width: 200, - }, - message: { - width: 100, - }, + describe('getPreFetchState', () => { + it('should return expected breakdownField', () => { + let appState = getDefaultProfileState({ + profilesManager: profilesManagerMock, + resetDefaultProfileState: { + columns: false, + rowHeight: false, + breakdownField: true, }, - }, - }); - appState = getDefaultProfileState({ - profilesManager: profilesManagerMock, - resetDefaultProfileState: { - columns: true, - rowHeight: false, - breakdownField: 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, - }, + 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); }); }); - it('should return expected rowHeight', () => { - const appState = getDefaultProfileState({ - profilesManager: profilesManagerMock, - resetDefaultProfileState: { - columns: false, - rowHeight: true, - breakdownField: false, - }, - defaultColumns: [], - dataView: dataViewWithTimefieldMock, - esqlQueryColumns: undefined, - }); - expect(appState).toEqual({ - rowHeight: 3, + 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, + 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 breakdownField', () => { - const appState = getDefaultProfileState({ - profilesManager: profilesManagerMock, - resetDefaultProfileState: { - columns: false, - rowHeight: false, - breakdownField: true, - }, - defaultColumns: [], - dataView: dataViewWithTimefieldMock, - esqlQueryColumns: undefined, - }); - expect(appState).toEqual({ - breakdownField: 'breakdown.field', + 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, - breakdownField: 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 8020458673fab..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,52 +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 = {}; - if (resetDefaultProfileState.breakdownField && defaultState.breakdownField !== undefined) { - stateUpdate.breakdownField = defaultState.breakdownField; - } + 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 Object.keys(stateUpdate).length ? stateUpdate : undefined; + 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 d8a85cf4e3f1c..8fb4a0bd769aa 100644 --- a/src/plugins/discover/public/context_awareness/__mocks__/index.tsx +++ b/src/plugins/discover/public/context_awareness/__mocks__/index.tsx @@ -84,7 +84,7 @@ export const createContextAwarenessMocks = ({ }, ], rowHeight: 3, - breakdownField: 'breakdown.field', + breakdownField: 'extension', })), getAdditionalCellActions: jest.fn((prev) => () => [ ...prev(), 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 44a36be34d1ab..db3f0081d1b70 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 @@ -27,7 +27,6 @@ import { useStateProps } from './use_state_props'; describe('useStateProps', () => { const initialState: UnifiedHistogramState = { - breakdownField: 'bytes', chartHidden: false, lensRequestAdapter: new RequestAdapter(), lensAdapters: lensAdaptersMock, @@ -45,7 +44,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'); @@ -57,25 +55,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, @@ -148,12 +143,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(` @@ -241,12 +240,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' }); @@ -272,17 +275,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, }) ); @@ -315,25 +321,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, @@ -343,6 +354,8 @@ describe('useStateProps', () => { requestAdapter: new RequestAdapter(), searchSessionId: '123', columns: undefined, + breakdownField: undefined, + onBreakdownFieldChange: undefined, }) ); expect(result.current).toMatchInlineSnapshot(` @@ -416,12 +429,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(` @@ -495,12 +512,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, }) ); const { @@ -539,8 +560,6 @@ describe('useStateProps', () => { act(() => { onBreakdownFieldChange({ name: 'field' } as DataViewField); }); - expect(stateService.setBreakdownField).toHaveBeenLastCalledWith('field'); - act(() => { onSuggestionContextChange({ suggestion: { title: 'Stacked Bar' }, @@ -555,12 +574,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(); @@ -575,12 +598,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 fcc19fcd78a00..c5dcfda7b6568 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 { lensEmbeddableOutputSelector$, } 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); @@ -56,6 +64,7 @@ export const useStateProps = ({ stateService?.state$, lensEmbeddableOutputSelector$ ); + /** * Contexts */ @@ -169,9 +178,9 @@ export const useStateProps = ({ const onBreakdownFieldChange = useCallback( (newBreakdownField: DataViewField | undefined) => { - stateService?.setBreakdownField(newBreakdownField?.name); + originalOnBreakdownFieldChange?.(newBreakdownField?.name); }, - [stateService] + [originalOnBreakdownFieldChange] ); const onSuggestionContextChange = useCallback( @@ -185,6 +194,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 dcce90037ec99..37e26533a4c68 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 551773cfe1892..52f134d40de81 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 { BehaviorSubject, Observable } from 'rxjs'; 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 */ @@ -143,16 +133,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, @@ -189,14 +176,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 6eacbaaef9500..651bfa3b5f2d3 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(),