From 46dc8f6a7e41281168172185c5ce113cddcd5f33 Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Mon, 15 Sep 2025 17:09:00 -0300 Subject: [PATCH 1/6] Remove controlGroupState from tabs storage manager --- .../main/state_management/tabs_storage_manager.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts index 69ff234516648..7937bcd24c951 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts @@ -31,7 +31,6 @@ export type TabStateInLocalStorage = Pick & { internalState: TabState['initialInternalState'] | undefined; appState: DiscoverAppState | undefined; globalState: TabState['globalState'] | undefined; - controlGroupState: ControlPanelsState | undefined; }; type RecentlyClosedTabStateInLocalStorage = TabStateInLocalStorage & @@ -170,7 +169,6 @@ export const createTabsStorageManager = ({ internalState: getInternalStateForTabWithoutRuntimeState(tabState.id), appState: getAppStateForTabWithoutRuntimeState(tabState.id), globalState: tabState.globalState, - controlGroupState: tabState.controlGroupState, }; }; @@ -202,16 +200,12 @@ export const createTabsStorageManager = ({ const globalState = getDefinedStateOnly( tabStateInStorage.globalState || defaultTabState.globalState ); - const controlGroupState = getDefinedStateOnly( - tabStateInStorage.controlGroupState || defaultTabState.controlGroupState - ); return { ...defaultTabState, ...pick(tabStateInStorage, 'id', 'label'), initialInternalState: internalState, initialAppState: appState, globalState: globalState || {}, - controlGroupState, }; }; From b80caa782515ca9354c80417742a7463b266d864 Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Mon, 15 Sep 2025 17:18:50 -0300 Subject: [PATCH 2/6] Cleanup unused types --- .../application/main/state_management/tabs_storage_manager.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts index 7937bcd24c951..b69a8fa0b1af3 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts @@ -15,8 +15,6 @@ import { } from '@kbn/kibana-utils-plugin/public'; import type { TabItem } from '@kbn/unified-tabs'; import type { Storage } from '@kbn/kibana-utils-plugin/public'; -import type { ControlPanelsState } from '@kbn/controls-plugin/public'; -import type { ESQLControlState } from '@kbn/esql-types'; import type { DiscoverSession } from '@kbn/saved-search-plugin/common'; import { TABS_STATE_URL_KEY } from '../../../../common/constants'; import type { TabState, RecentlyClosedTabState } from './redux/types'; From 6a425835db10ef26741d92de774fd0ff96d9e9ed Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Mon, 15 Sep 2025 17:20:20 -0300 Subject: [PATCH 3/6] Move esqlVariables to tab state to prevent refresh on tab switch --- .../main/components/chart/use_discover_histogram.ts | 3 +-- .../main/components/top_nav/discover_topnav.tsx | 2 +- .../main/components/top_nav/use_esql_variables.ts | 11 +++++++---- .../application/main/data_fetching/fetch_all.ts | 6 ++---- .../main/state_management/redux/constants.ts | 1 + .../main/state_management/redux/internal_state.ts | 13 ++++++++----- .../main/state_management/redux/types.ts | 8 ++++---- 7 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/platform/plugins/shared/discover/public/application/main/components/chart/use_discover_histogram.ts b/src/platform/plugins/shared/discover/public/application/main/components/chart/use_discover_histogram.ts index 4a8f2f3f98bd2..235928ffec91f 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/chart/use_discover_histogram.ts +++ b/src/platform/plugins/shared/discover/public/application/main/components/chart/use_discover_histogram.ts @@ -60,7 +60,6 @@ import { useCurrentTabSelector, useInternalStateDispatch, type InitialUnifiedHistogramLayoutProps, - useInternalStateSelector, } from '../../state_management/redux'; const EMPTY_ESQL_COLUMNS: DatatableColumn[] = []; @@ -372,7 +371,7 @@ export const useDiscoverHistogram = ( const chartHidden = useAppStateSelector((state) => state.hideChart); const timeInterval = useAppStateSelector((state) => state.interval); const breakdownField = useAppStateSelector((state) => state.breakdownField); - const esqlVariables = useInternalStateSelector((state) => state.esqlVariables); + const esqlVariables = useCurrentTabSelector((tab) => tab.esqlVariables); const onBreakdownFieldChange = useCallback< NonNullable diff --git a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/discover_topnav.tsx b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/discover_topnav.tsx index b2f28dd754db1..5a5c4bd88ce4c 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/discover_topnav.tsx +++ b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/discover_topnav.tsx @@ -63,7 +63,7 @@ export const DiscoverTopNav = ({ const [controlGroupApi, setControlGroupApi] = useState(); const query = useAppStateSelector((state) => state.query); - const esqlVariables = useInternalStateSelector((state) => state.esqlVariables); + const esqlVariables = useCurrentTabSelector((tab) => tab.esqlVariables); const timeRange = useCurrentTabSelector((tab) => tab.dataRequestParams.timeRangeAbsolute); diff --git a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts index 013afc3f88e19..5cd97390fa3e8 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts +++ b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts @@ -104,6 +104,7 @@ export const useESQLVariables = ({ } => { const dispatch = useInternalStateDispatch(); const setControlGroupState = useCurrentTabAction(internalStateActions.setControlGroupState); + const setEsqlVariables = useCurrentTabAction(internalStateActions.setEsqlVariables); const currentControlGroupState = useCurrentTabSelector((tab) => tab.controlGroupState); const initialSavedSearch = useObservable(stateContainer.savedSearchState.getInitial$()); @@ -144,7 +145,7 @@ export const useESQLVariables = ({ const newVariables = extractEsqlVariables(currentTabControlState); if (!isEqual(newVariables, currentEsqlVariables)) { // Update the ESQL variables in the internal state - dispatch(internalStateActions.setEsqlVariables(newVariables)); + dispatch(setEsqlVariables({ esqlVariables: newVariables })); stateContainer.dataState.fetch(); } } @@ -155,13 +156,15 @@ export const useESQLVariables = ({ savedSearchResetSubsciption.unsubscribe(); }; }, [ - initialSavedSearch?.controlGroupJson, controlGroupApi, - setControlGroupState, currentEsqlVariables, dispatch, + initialSavedSearch?.controlGroupJson, isEsqlMode, - stateContainer, + setControlGroupState, + setEsqlVariables, + stateContainer.dataState, + stateContainer.savedSearchState, ]); const onSaveControl = useCallback( diff --git a/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_all.ts b/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_all.ts index 46982548eaca0..61c8bcb9522b6 100644 --- a/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_all.ts +++ b/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_all.ts @@ -80,7 +80,6 @@ export function fetchAll( abortController, getCurrentTab, onFetchRecordsComplete, - internalState, } = params; const { data, expressions } = services; @@ -88,7 +87,6 @@ export function fetchAll( const searchSource = savedSearch.searchSource.createChild(); const dataView = searchSource.getField('index')!; const { query, sort } = appStateContainer.getState(); - const { esqlVariables } = internalState.getState(); const prevQuery = dataSubjects.documents$.getValue().query; const isEsqlQuery = isOfAggregateQueryType(query); const currentTab = getCurrentTab(); @@ -125,7 +123,7 @@ export function fetchAll( expressions, scopedProfilesManager, timeRange: currentTab.dataRequestParams.timeRangeAbsolute, - esqlVariables, + esqlVariables: currentTab.esqlVariables, searchSessionId: params.searchSessionId, }) : fetchDocuments(searchSource, params); @@ -193,7 +191,7 @@ export function fetchAll( const isFirstQuery = !prevQuery; const queryChanged = !isEqual(query, prevQuery); - const hasEsqlVariables = Boolean(esqlVariables?.length); + const hasEsqlVariables = Boolean(currentTab.esqlVariables?.length); return isFirstQuery || queryChanged || hasEsqlVariables ? FetchStatus.PARTIAL diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/constants.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/constants.ts index 4d1b56d3bfaf4..6b445d4f67e83 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/constants.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/constants.ts @@ -20,6 +20,7 @@ export const DEFAULT_TAB_STATE: Omit = { }, overriddenVisContextAfterInvalidation: undefined, controlGroupState: undefined, + esqlVariables: undefined, resetDefaultProfileState: { resetId: '', columns: false, diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/internal_state.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/internal_state.ts index 631916fc10e59..b64df6ee06ec6 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/internal_state.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/internal_state.ts @@ -56,7 +56,6 @@ const initialState: DiscoverInternalState = { savedDataViews: [], expandedDoc: undefined, isESQLToDataViewTransitionModalVisible: false, - esqlVariables: undefined, tabsBarVisibility: TabsBarVisibility.default, tabs: { areInitializing: false, @@ -184,14 +183,18 @@ export const internalStateSlice = createSlice({ tab.controlGroupState = action.payload.controlGroupState; }), + setEsqlVariables: ( + state, + action: TabAction<{ esqlVariables: ESQLControlVariable[] | undefined }> + ) => + withTab(state, action, (tab) => { + tab.esqlVariables = action.payload.esqlVariables; + }), + setIsESQLToDataViewTransitionModalVisible: (state, action: PayloadAction) => { state.isESQLToDataViewTransitionModalVisible = action.payload; }, - setEsqlVariables: (state, action: PayloadAction) => { - state.esqlVariables = action.payload; - }, - setResetDefaultProfileState: { prepare: ( payload: TabActionPayload<{ diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/types.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/types.ts index e1ddecc2140cf..0342c2bb3feaa 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/types.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/types.ts @@ -47,6 +47,10 @@ export interface TabState extends TabItem { // The following properties are used to manage the tab's state after it has been initialized. globalState: TabStateGlobalState; controlGroupState: ControlPanelsState | undefined; + /** + * ESQL query variables + */ + esqlVariables: ESQLControlVariable[] | undefined; isDataViewLoading: boolean; dataRequestParams: InternalStateDataRequestParams; overriddenVisContextAfterInvalidation: UnifiedHistogramVisContext | {} | undefined; // it will be used during saved search saving @@ -85,10 +89,6 @@ export interface DiscoverInternalState { expandedDoc: DataTableRecord | undefined; initialDocViewerTabId?: string; isESQLToDataViewTransitionModalVisible: boolean; - /** - * ESQL query variables - */ - esqlVariables?: ESQLControlVariable[]; tabsBarVisibility: TabsBarVisibility; tabs: { areInitializing: boolean; From 8e5a4f28ef41d34aa6a1b6890b497f9d7edb87c2 Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Mon, 15 Sep 2025 18:29:38 -0300 Subject: [PATCH 4/6] Init esqlVariables from local storage tabs --- .../top_nav/use_esql_variables.test.tsx | 14 +++-- .../components/top_nav/use_esql_variables.ts | 51 +----------------- .../main/state_management/redux/index.ts | 8 ++- .../redux/tab_mapping_utils.test.ts | 2 + .../main/state_management/redux/utils.ts | 52 +++++++++++++++++++ .../state_management/tabs_storage_manager.ts | 10 +++- 6 files changed, 83 insertions(+), 54 deletions(-) diff --git a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx index ad72871f21c65..f8332b955ca57 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx +++ b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx @@ -157,7 +157,9 @@ describe('useESQLVariables', () => { ); } if (action.type === 'internalState/setEsqlVariables') { - expect(action.payload).toEqual(mockNewVariables); + expect((action.payload as { esqlVariables: unknown }).esqlVariables).toEqual( + mockNewVariables + ); } }); }); @@ -332,7 +334,10 @@ describe('useESQLVariables', () => { (call) => (call[0] as { type: string }).type === 'internalState/setEsqlVariables' ); expect(setEsqlVariablesCall).toBeTruthy(); - expect((setEsqlVariablesCall![0] as unknown as { payload: unknown }).payload).toEqual([ + expect( + (setEsqlVariablesCall![0] as unknown as { payload: { esqlVariables: unknown } }).payload + .esqlVariables + ).toEqual([ { key: 'numericVar', type: 'values', @@ -369,7 +374,10 @@ describe('useESQLVariables', () => { (call) => (call[0] as { type: string }).type === 'internalState/setEsqlVariables' ); expect(setEsqlVariablesCall).toBeTruthy(); - expect((setEsqlVariablesCall![0] as unknown as { payload: unknown }).payload).toEqual([ + expect( + (setEsqlVariablesCall![0] as unknown as { payload: { esqlVariables: unknown } }).payload + .esqlVariables + ).toEqual([ { key: 'textVar', type: 'values', diff --git a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts index 5cd97390fa3e8..67ce8f84b4b0b 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts +++ b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts @@ -14,62 +14,15 @@ import { ESQL_CONTROL } from '@kbn/controls-constants'; import type { ESQLControlState, ESQLControlVariable } from '@kbn/esql-types'; import type { DiscoverStateContainer } from '../../state_management/discover_state'; import { + extractEsqlVariables, internalStateActions, + parseControlGroupJson, useCurrentTabAction, useCurrentTabSelector, useInternalStateDispatch, } from '../../state_management/redux'; import { useSavedSearch } from '../../state_management/discover_state_provider'; -/** - * @param panels - The control panels state, which may be null. - * @description Extracts ESQL variables from the control panels state. - * Each ESQL control panel is expected to have a `variableName`, `variableType`, and `selectedOptions`. - * Returns an array of `ESQLControlVariable` objects. - * If `panels` is null or empty, it returns an empty array. - * @returns An array of ESQLControlVariable objects. - */ -const extractEsqlVariables = ( - panels: ControlPanelsState | null -): ESQLControlVariable[] => { - if (!panels || Object.keys(panels).length === 0) { - return []; - } - const variables = Object.values(panels).reduce((acc: ESQLControlVariable[], panel) => { - if (panel.type === ESQL_CONTROL) { - acc.push({ - key: panel.variableName, - type: panel.variableType, - // If the selected option is not a number, keep it as a string - value: isNaN(Number(panel.selectedOptions?.[0])) - ? panel.selectedOptions?.[0] - : Number(panel.selectedOptions?.[0]), - }); - } - return acc; - }, []); - - return variables; -}; - -/** - * Parses a JSON string into a ControlPanelsState object. - * If the JSON is invalid or null, it returns an empty object. - * - * @param jsonString - The JSON string to parse. - * @returns A ControlPanelsState object or an empty object if parsing fails. - */ - -const parseControlGroupJson = ( - jsonString?: string | null -): ControlPanelsState => { - try { - return jsonString ? JSON.parse(jsonString) : {}; - } catch (e) { - return {}; - } -}; - /** * Custom hook to manage ESQL variables in the control group for Discover. * It synchronizes ESQL control panel state with the application's internal Redux state diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/index.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/index.ts index 48ea85c750bd2..135ed1a2f698e 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/index.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/index.ts @@ -94,7 +94,13 @@ export { useAdHocDataViews, } from './runtime_state'; -export { type TabActionInjector, createTabActionInjector, createTabItem } from './utils'; +export { + type TabActionInjector, + createTabActionInjector, + createTabItem, + parseControlGroupJson, + extractEsqlVariables, +} from './utils'; export { fromSavedObjectTabToTabState, diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/tab_mapping_utils.test.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/tab_mapping_utils.test.ts index 3cb564564e64f..b2b98a2413e53 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/tab_mapping_utils.test.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/tab_mapping_utils.test.ts @@ -63,6 +63,7 @@ describe('tab mapping utils', () => { "timeRangeRelative": undefined, }, "duplicatedFromId": "0", + "esqlVariables": undefined, "globalState": Object { "refreshInterval": Object { "pause": true, @@ -131,6 +132,7 @@ describe('tab mapping utils', () => { "timeRangeRelative": undefined, }, "duplicatedFromId": "0", + "esqlVariables": undefined, "globalState": Object { "refreshInterval": Object { "pause": false, diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/utils.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/utils.ts index bafa61b94093c..862250ba2ba66 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/redux/utils.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/redux/utils.ts @@ -12,6 +12,9 @@ import { i18n } from '@kbn/i18n'; import { getNextTabNumber, type TabItem } from '@kbn/unified-tabs'; import { createAsyncThunk, miniSerializeError } from '@reduxjs/toolkit'; import { SavedObjectNotFound } from '@kbn/kibana-utils-plugin/common'; +import type { ControlPanelsState } from '@kbn/controls-plugin/public'; +import type { ESQLControlState, ESQLControlVariable } from '@kbn/esql-types'; +import { ESQL_CONTROL } from '@kbn/controls-constants'; import type { DiscoverInternalState, TabState } from './types'; import type { InternalStateDispatch, @@ -67,3 +70,52 @@ export const createTabItem = (allTabs: TabState[]): TabItem => { return { id, label }; }; + +/** + * Parses a JSON string into a ControlPanelsState object. + * If the JSON is invalid or null, it returns an empty object. + * + * @param jsonString - The JSON string to parse. + * @returns A ControlPanelsState object or an empty object if parsing fails. + */ + +export const parseControlGroupJson = ( + jsonString?: string | null +): ControlPanelsState => { + try { + return jsonString ? JSON.parse(jsonString) : {}; + } catch (e) { + return {}; + } +}; + +/** + * @param panels - The control panels state, which may be null. + * @description Extracts ESQL variables from the control panels state. + * Each ESQL control panel is expected to have a `variableName`, `variableType`, and `selectedOptions`. + * Returns an array of `ESQLControlVariable` objects. + * If `panels` is null or empty, it returns an empty array. + * @returns An array of ESQLControlVariable objects. + */ +export const extractEsqlVariables = ( + panels: ControlPanelsState | null +): ESQLControlVariable[] => { + if (!panels || Object.keys(panels).length === 0) { + return []; + } + const variables = Object.values(panels).reduce((acc: ESQLControlVariable[], panel) => { + if (panel.type === ESQL_CONTROL) { + acc.push({ + key: panel.variableName, + type: panel.variableType, + // If the selected option is not a number, keep it as a string + value: isNaN(Number(panel.selectedOptions?.[0])) + ? panel.selectedOptions?.[0] + : Number(panel.selectedOptions?.[0]), + }); + } + return acc; + }, []); + + return variables; +}; diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts index b69a8fa0b1af3..0aea2321e54c5 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/tabs_storage_manager.ts @@ -18,7 +18,7 @@ import type { Storage } from '@kbn/kibana-utils-plugin/public'; import type { DiscoverSession } from '@kbn/saved-search-plugin/common'; import { TABS_STATE_URL_KEY } from '../../../../common/constants'; import type { TabState, RecentlyClosedTabState } from './redux/types'; -import { createTabItem } from './redux/utils'; +import { createTabItem, extractEsqlVariables, parseControlGroupJson } from './redux/utils'; import type { DiscoverAppState } from './discover_app_state_container'; import { fromSavedObjectTabToTabState } from './redux'; @@ -198,12 +198,20 @@ export const createTabsStorageManager = ({ const globalState = getDefinedStateOnly( tabStateInStorage.globalState || defaultTabState.globalState ); + const controlGroupState = internalState?.controlGroupJson + ? parseControlGroupJson(internalState.controlGroupJson) + : undefined; + const esqlVariables = controlGroupState + ? extractEsqlVariables(controlGroupState) + : defaultTabState.esqlVariables; + return { ...defaultTabState, ...pick(tabStateInStorage, 'id', 'label'), initialInternalState: internalState, initialAppState: appState, globalState: globalState || {}, + esqlVariables, }; }; From a3832f5e74fd952ddc3df858400d3418eb11d760 Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Mon, 15 Sep 2025 18:55:41 -0300 Subject: [PATCH 5/6] Remove getHasReset$ --- .../top_nav/use_esql_variables.test.tsx | 29 +++----- .../components/top_nav/use_esql_variables.ts | 18 ++--- .../discover_saved_search_container.test.ts | 70 ------------------- .../discover_saved_search_container.ts | 8 --- .../main/state_management/discover_state.ts | 1 - 5 files changed, 16 insertions(+), 110 deletions(-) diff --git a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx index f8332b955ca57..a2c5027c12a56 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx +++ b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx @@ -186,17 +186,11 @@ describe('useESQLVariables', () => { }) ); - // Mock the savedSearchState with getHasReset$ observable + // Mock the savedSearchState with getInitial$ observable const stateContainer = getStateContainer(); - const mockGetHasReset = jest.fn().mockReturnValue( - new Observable((subscriber) => { - return () => mockUnsubscribeReset(); - }) - ); - jest - .spyOn(stateContainer.savedSearchState, 'getHasReset$') - .mockImplementation(mockGetHasReset); + .spyOn(stateContainer.savedSearchState.getInitial$(), 'unsubscribe') + .mockImplementation(mockUnsubscribeReset()); const { hook } = await renderUseESQLVariables({ isEsqlMode: true, @@ -212,19 +206,16 @@ describe('useESQLVariables', () => { expect(mockUnsubscribeReset).toHaveBeenCalledTimes(1); }); - it('should reset control panels from saved search state when getHasReset$ emits true', async () => { + it('should reset control panels from saved search state when getInitial$ emits', async () => { const mockInitialSavedSearch = { controlGroupJson: JSON.stringify(mockControlState), // other saved search properties }; + const mockGetInitial$ = new BehaviorSubject(mockInitialSavedSearch as SavedSearch); const stateContainer = getStateContainer(); - const hasReset$ = new BehaviorSubject(false); - jest - .spyOn(stateContainer.savedSearchState, 'getInitial$') - .mockReturnValue(new BehaviorSubject(mockInitialSavedSearch as SavedSearch)); - jest.spyOn(stateContainer.savedSearchState, 'getHasReset$').mockReturnValue(hasReset$); + jest.spyOn(stateContainer.savedSearchState, 'getInitial$').mockReturnValue(mockGetInitial$); // Create a mock control group API with a mock updateInput method const mockUpdateInput = jest.fn(); @@ -240,8 +231,11 @@ describe('useESQLVariables', () => { controlGroupApi: mockControlGroupApiWithUpdate as unknown as ControlGroupRendererApi, }); + expect(mockUpdateInput).not.toHaveBeenCalled(); + + // Simulate the getInitial$ emitting a new saved search act(() => { - hasReset$.next(true); + mockGetInitial$.next(mockInitialSavedSearch as SavedSearch); }); await waitFor(() => { @@ -250,9 +244,6 @@ describe('useESQLVariables', () => { initialChildControlState: mockControlState, }); }); - - // Assert that the hasReset$ observable was reset to false - expect(hasReset$.getValue()).toBe(false); }); }); diff --git a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts index 67ce8f84b4b0b..eb17c8199402f 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts +++ b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts @@ -8,10 +8,10 @@ */ import { isEqual } from 'lodash'; import { useCallback, useEffect } from 'react'; -import useObservable from 'react-use/lib/useObservable'; import type { ControlPanelsState, ControlGroupRendererApi } from '@kbn/controls-plugin/public'; import { ESQL_CONTROL } from '@kbn/controls-constants'; import type { ESQLControlState, ESQLControlVariable } from '@kbn/esql-types'; +import { skip } from 'rxjs'; import type { DiscoverStateContainer } from '../../state_management/discover_state'; import { extractEsqlVariables, @@ -59,7 +59,6 @@ export const useESQLVariables = ({ const setControlGroupState = useCurrentTabAction(internalStateActions.setControlGroupState); const setEsqlVariables = useCurrentTabAction(internalStateActions.setEsqlVariables); const currentControlGroupState = useCurrentTabSelector((tab) => tab.controlGroupState); - const initialSavedSearch = useObservable(stateContainer.savedSearchState.getInitial$()); const savedSearchState = useSavedSearch(); @@ -71,15 +70,11 @@ export const useESQLVariables = ({ // Handling the reset unsaved changes badge const savedSearchResetSubsciption = stateContainer.savedSearchState - .getHasReset$() - .subscribe((hasReset) => { - if (hasReset) { - const savedControlGroupState = parseControlGroupJson( - initialSavedSearch?.controlGroupJson - ); - controlGroupApi.updateInput({ initialChildControlState: savedControlGroupState }); - stateContainer.savedSearchState.getHasReset$().next(false); - } + .getInitial$() + .pipe(skip(1)) // Skip the initial emission since it's a BehaviorSubject + .subscribe((initialSavedSearch) => { + const savedControlGroupState = parseControlGroupJson(initialSavedSearch?.controlGroupJson); + controlGroupApi.updateInput({ initialChildControlState: savedControlGroupState }); }); const inputSubscription = controlGroupApi.getInput$().subscribe((input) => { @@ -112,7 +107,6 @@ export const useESQLVariables = ({ controlGroupApi, currentEsqlVariables, dispatch, - initialSavedSearch?.controlGroupJson, isEsqlMode, setControlGroupState, setEsqlVariables, diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/discover_saved_search_container.test.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/discover_saved_search_container.test.ts index da401345d5d5d..d920585b026a4 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/discover_saved_search_container.test.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/discover_saved_search_container.test.ts @@ -288,74 +288,4 @@ describe('DiscoverSavedSearchContainer', () => { unsubscribe(); }); }); - - describe('getHasReset$', () => { - it('should initially return false', () => { - const container = getSavedSearchContainer({ - services, - internalState, - getCurrentTab, - }); - expect(container.getHasReset$().getValue()).toBe(false); - }); - - it('should return a BehaviorSubject that can be subscribed to', () => { - const container = getSavedSearchContainer({ - services, - internalState, - getCurrentTab, - }); - const hasReset$ = container.getHasReset$(); - - expect(typeof hasReset$.subscribe).toBe('function'); - expect(typeof hasReset$.getValue).toBe('function'); - expect(typeof hasReset$.next).toBe('function'); - }); - - it('should allow updating the reset state', () => { - const container = getSavedSearchContainer({ - services, - internalState, - getCurrentTab, - }); - const hasReset$ = container.getHasReset$(); - - // Initially false - expect(hasReset$.getValue()).toBe(false); - - // Update to true - hasReset$.next(true); - expect(hasReset$.getValue()).toBe(true); - - // Update back to false - hasReset$.next(false); - expect(hasReset$.getValue()).toBe(false); - }); - - it('should notify subscribers when reset state changes', () => { - const container = getSavedSearchContainer({ - services, - internalState, - getCurrentTab, - }); - const hasReset$ = container.getHasReset$(); - const mockSubscriber = jest.fn(); - - const subscription = hasReset$.subscribe(mockSubscriber); - - // Should be called initially with false - expect(mockSubscriber).toHaveBeenCalledWith(false); - - // Should be called when value changes to true - hasReset$.next(true); - expect(mockSubscriber).toHaveBeenCalledWith(true); - - // Should be called when value changes back to false - hasReset$.next(false); - expect(mockSubscriber).toHaveBeenCalledWith(false); - - expect(mockSubscriber).toHaveBeenCalledTimes(3); - subscription.unsubscribe(); - }); - }); }); diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/discover_saved_search_container.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/discover_saved_search_container.ts index 72d5ddfef2b7e..053a38242b615 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/discover_saved_search_container.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/discover_saved_search_container.ts @@ -83,11 +83,6 @@ export interface DiscoverSavedSearchContainer { * Can be used to track if the saved search has been modified and displayed in the UI */ getHasChanged$: () => BehaviorSubject; - /** - * Get an BehaviorSubject containing the state if the saved search has been reset to its initial state - * Can be used to track if the saved search has been reset - */ - getHasReset$: () => BehaviorSubject; /** * Get the current state of the saved search */ @@ -138,7 +133,6 @@ export function getSavedSearchContainer({ const savedSearchInitial$ = new BehaviorSubject(initialSavedSearch); const savedSearchCurrent$ = new BehaviorSubject(copySavedSearch(initialSavedSearch)); const hasChanged$ = new BehaviorSubject(false); - const hasReset$ = new BehaviorSubject(false); const set = (savedSearch: SavedSearch) => { addLog('[savedSearch] set', savedSearch); hasChanged$.next(false); @@ -150,7 +144,6 @@ export function getSavedSearchContainer({ const getInitial$ = () => savedSearchInitial$; const getCurrent$ = () => savedSearchCurrent$; const getHasChanged$ = () => hasChanged$; - const getHasReset$ = () => hasReset$; const getTitle = () => savedSearchCurrent$.getValue().title; const getId = () => savedSearchCurrent$.getValue().id; @@ -263,7 +256,6 @@ export function getSavedSearchContainer({ initUrlTracking, getCurrent$, getHasChanged$, - getHasReset$, getId, getInitial$, getState, diff --git a/src/platform/plugins/shared/discover/public/application/main/state_management/discover_state.ts b/src/platform/plugins/shared/discover/public/application/main/state_management/discover_state.ts index 9f3e6e18b457a..12f3aa32b67be 100644 --- a/src/platform/plugins/shared/discover/public/application/main/state_management/discover_state.ts +++ b/src/platform/plugins/shared/discover/public/application/main/state_management/discover_state.ts @@ -575,7 +575,6 @@ export function getDiscoverStateContainer({ savedSearchContainer.set(nextSavedSearch); await appStateContainer.replaceUrlState(newAppState); - savedSearchContainer.getHasReset$().next(true); return nextSavedSearch; }; From 2798c79c2e76dc1f16170850ef30b30581b005b6 Mon Sep 17 00:00:00 2001 From: Davis McPhee Date: Mon, 15 Sep 2025 20:17:13 -0300 Subject: [PATCH 6/6] Small test fix --- .../components/top_nav/use_esql_variables.test.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx index a2c5027c12a56..4042ced776019 100644 --- a/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx +++ b/src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.test.tsx @@ -181,16 +181,20 @@ describe('useESQLVariables', () => { // Mock the getInput$ observable jest.spyOn(mockControlGroupAPI.inputSubject, 'asObservable').mockReturnValue( - new Observable((subscriber) => { + new Observable(() => { return () => mockUnsubscribeInput(); }) ); // Mock the savedSearchState with getInitial$ observable const stateContainer = getStateContainer(); - jest - .spyOn(stateContainer.savedSearchState.getInitial$(), 'unsubscribe') - .mockImplementation(mockUnsubscribeReset()); + const mockGetInitial$ = new BehaviorSubject(null) as unknown as BehaviorSubject; + jest.spyOn(mockGetInitial$, 'pipe').mockReturnValue( + new Observable(() => { + return () => mockUnsubscribeReset(); + }) + ); + jest.spyOn(stateContainer.savedSearchState, 'getInitial$').mockReturnValue(mockGetInitial$); const { hook } = await renderUseESQLVariables({ isEsqlMode: true, @@ -233,7 +237,7 @@ describe('useESQLVariables', () => { expect(mockUpdateInput).not.toHaveBeenCalled(); - // Simulate the getInitial$ emitting a new saved search + // Simulate getInitial$ emitting a new saved search act(() => { mockGetInitial$.next(mockInitialSavedSearch as SavedSearch); });