From fe987ea132b5eef166fe2748f86120941f133513 Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Wed, 20 Nov 2024 22:00:51 -0400 Subject: [PATCH] Comments and cleanup --- .../public/application/main/hooks/use_esql_mode.ts | 10 ++++++++++ .../state_management/discover_data_state_container.ts | 4 ++++ .../state_management/utils/build_state_subscribe.ts | 2 +- .../utils/get_default_profile_state.ts | 11 +++++++++++ .../unified_histogram/public/container/container.tsx | 5 ++--- 5 files changed, 28 insertions(+), 4 deletions(-) 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 b4fc22c7671a8..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,7 +74,12 @@ 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)) { @@ -86,6 +91,7 @@ export function useEsqlMode({ getIndexPatternFromESQLQuery(appStateQuery.esql) !== getIndexPatternFromESQLQuery(prev.current.query); + // Reset all default profile state when index pattern changes if (indexPatternChanged) { stateContainer.internalState.transitions.setResetDefaultProfileState({ columns: true, @@ -94,6 +100,8 @@ export function useEsqlMode({ }); } } + + return; } if (next.fetchStatus !== FetchStatus.PARTIAL) { @@ -132,6 +140,8 @@ export function useEsqlMode({ const { viewMode } = stateContainer.appState.getState(); const changeViewMode = viewMode !== getValidViewMode({ viewMode, isEsqlMode: true }); + // 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, 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 d5d9ab01c0560..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 @@ -277,6 +277,8 @@ export function getDataStateContainer({ 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(); @@ -302,6 +304,8 @@ export function getDataStateContainer({ await appStateContainer.replaceUrlState(postFetchStateUpdate); } + // 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, 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 a22f0af350e00..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 @@ -160,7 +160,7 @@ export const buildStateSubscribe = if (dataState.disableNextFetchOnStateChange$.getValue()) { addLog( - '[buildStateSubscribe] fetch disabled on state changes', + '[buildStateSubscribe] next fetch skipped on state change', JSON.stringify(logData, null, 2) ); 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 62e1d45323183..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 @@ -31,6 +31,11 @@ export const getDefaultProfileState = ({ const defaultState = getDefaultState(profilesManager, dataView); 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 = {}; @@ -44,6 +49,12 @@ export const getDefaultProfileState = ({ return Object.keys(stateUpdate).length ? stateUpdate : undefined; }, + + /** + * 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, diff --git a/src/plugins/unified_histogram/public/container/container.tsx b/src/plugins/unified_histogram/public/container/container.tsx index ce55d0773344e..f2056d6bf07e4 100644 --- a/src/plugins/unified_histogram/public/container/container.tsx +++ b/src/plugins/unified_histogram/public/container/container.tsx @@ -95,7 +95,7 @@ export type UnifiedHistogramApi = { export const UnifiedHistogramContainer = forwardRef< UnifiedHistogramApi, UnifiedHistogramContainerProps ->(({ onBreakdownFieldChange, onVisContextChanged, ...containerProps }, ref) => { +>(({ breakdownField, onBreakdownFieldChange, onVisContextChanged, ...containerProps }, ref) => { const [layoutProps, setLayoutProps] = useState(); const [localStorageKeyPrefix, setLocalStorageKeyPrefix] = useState(); const [stateService, setStateService] = useState(); @@ -158,8 +158,7 @@ export const UnifiedHistogramContainer = forwardRef< searchSessionId, requestAdapter, columns, - breakdownField: initialBreakdownField, - ...pick(containerProps, 'breakdownField'), + breakdownField: breakdownField ?? initialBreakdownField, onBreakdownFieldChange, });