Small Discover controls fixes

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<UseUnifiedHistogramProps['onBreakdownFieldChange']>

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<ControlGroupRendererApi | undefined>();
 
   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);
 

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
+            );
           }
         });
       });
@@ -179,22 +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 getHasReset$ observable
+      // Mock the savedSearchState with getInitial$ observable
       const stateContainer = getStateContainer();
-      const mockGetHasReset = jest.fn().mockReturnValue(
-        new Observable((subscriber) => {
+      const mockGetInitial$ = new BehaviorSubject(null) as unknown as BehaviorSubject<SavedSearch>;
+      jest.spyOn(mockGetInitial$, 'pipe').mockReturnValue(
+        new Observable(() => {
           return () => mockUnsubscribeReset();
         })
       );
-
-      jest
-        .spyOn(stateContainer.savedSearchState, 'getHasReset$')
-        .mockImplementation(mockGetHasReset);
+      jest.spyOn(stateContainer.savedSearchState, 'getInitial$').mockReturnValue(mockGetInitial$);
 
       const { hook } = await renderUseESQLVariables({
         isEsqlMode: true,
@@ -210,19 +210,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();
@@ -238,8 +235,11 @@ describe('useESQLVariables', () => {
         controlGroupApi: mockControlGroupApiWithUpdate as unknown as ControlGroupRendererApi,
       });
 
+      expect(mockUpdateInput).not.toHaveBeenCalled();
+
+      // Simulate getInitial$ emitting a new saved search
       act(() => {
-        hasReset$.next(true);
+        mockGetInitial$.next(mockInitialSavedSearch as SavedSearch);
       });
 
       await waitFor(() => {
@@ -248,9 +248,6 @@ describe('useESQLVariables', () => {
           initialChildControlState: mockControlState,
         });
       });
-
-      // Assert that the hasReset$ observable was reset to false
-      expect(hasReset$.getValue()).toBe(false);
     });
   });
 
@@ -332,7 +329,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 +369,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',

src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_esql_variables.ts

@@ -8,68 +8,21 @@
  */
 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,
   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<ESQLControlState> | 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<ESQLControlState> => {
-  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
@@ -104,8 +57,8 @@ 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$());
 
   const savedSearchState = useSavedSearch();
 
@@ -117,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) => {
@@ -144,7 +93,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 +104,14 @@ export const useESQLVariables = ({
       savedSearchResetSubsciption.unsubscribe();
     };
   }, [
-    initialSavedSearch?.controlGroupJson,
     controlGroupApi,
-    setControlGroupState,
     currentEsqlVariables,
     dispatch,
     isEsqlMode,
-    stateContainer,
+    setControlGroupState,
+    setEsqlVariables,
+    stateContainer.dataState,
+    stateContainer.savedSearchState,
   ]);
 
   const onSaveControl = useCallback(

src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_all.ts

@@ -80,15 +80,13 @@ export function fetchAll(
     abortController,
     getCurrentTab,
     onFetchRecordsComplete,
-    internalState,
   } = params;
   const { data, expressions } = services;
 
   try {
     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

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();
-    });
-  });
 });