[control group] apply selections on reset (#189830)

Fixes #189580 PR awaits until all control filters are ready and then applies selections during reset. --------- Co-authored-by: Elastic Machine <[email protected]>
elastic · Aug 8, 2024 · b87e967 · b87e967
1 parent 88144cc
commit b87e967
Show file tree

Hide file tree

Showing 24 changed files with 440 additions and 231 deletions.
diff --git a/examples/controls_example/public/app/react_control_example/react_control_example.tsx b/examples/controls_example/public/app/react_control_example/react_control_example.tsx
@@ -8,7 +8,7 @@
 
 import React, { useEffect, useMemo, useState } from 'react';
 import { BehaviorSubject, combineLatest, Subject } from 'rxjs';
-
+import useMountedState from 'react-use/lib/useMountedState';
 import {
   EuiBadge,
   EuiButton,
@@ -76,6 +76,7 @@ export const ReactControlExample = ({
   core: CoreStart;
   dataViews: DataViewsPublicPluginStart;
 }) => {
+  const isMounted = useMountedState();
   const dataLoading$ = useMemo(() => {
     return new BehaviorSubject<boolean | undefined>(false);
   }, []);
@@ -112,6 +113,7 @@ export const ReactControlExample = ({
   const [controlGroupApi, setControlGroupApi] = useState<ControlGroupApi | undefined>(undefined);
   const [isControlGroupInitialized, setIsControlGroupInitialized] = useState(false);
   const [dataViewNotFound, setDataViewNotFound] = useState(false);
+  const [isResetting, setIsResetting] = useState(false);
 
   const dashboardApi = useMemo(() => {
     const query$ = new BehaviorSubject<Query | AggregateQuery | undefined>(undefined);
@@ -361,9 +363,15 @@ export const ReactControlExample = ({
             </EuiFlexItem>
             <EuiFlexItem grow={false}>
               <EuiButtonEmpty
-                isDisabled={!controlGroupApi}
-                onClick={() => {
-                  controlGroupApi?.resetUnsavedChanges();
+                isDisabled={!controlGroupApi || isResetting}
+                isLoading={isResetting}
+                onClick={async () => {
+                  if (!controlGroupApi) {
+                    return;
+                  }
+                  setIsResetting(true);
+                  await controlGroupApi.asyncResetUnsavedChanges();
+                  if (isMounted()) setIsResetting(false);
                 }}
               >
                 Reset

diff --git a/...controls_example/public/react_controls/control_group/control_group_unsaved_changes_api.ts b/...controls_example/public/react_controls/control_group/control_group_unsaved_changes_api.ts
@@ -20,6 +20,7 @@ import {
 import { combineLatest, map } from 'rxjs';
 import { ControlsInOrder, getControlsInOrder } from './init_controls_manager';
 import { ControlGroupRuntimeState, ControlPanelsState } from './types';
+import { apiPublishesAsyncFilters } from '../data_controls/publishes_async_filters';
 
 export type ControlGroupComparatorState = Pick<
   ControlGroupRuntimeState,
@@ -33,6 +34,7 @@ export type ControlGroupComparatorState = Pick<
 };
 
 export function initializeControlGroupUnsavedChanges(
+  applySelections: () => void,
   children$: PresentationContainer['children$'],
   comparators: StateComparators<ControlGroupComparatorState>,
   snapshotControlsRuntimeState: () => ControlPanelsState,
@@ -68,12 +70,25 @@ export function initializeControlGroupUnsavedChanges(
           return Object.keys(unsavedChanges).length ? unsavedChanges : undefined;
         })
       ),
-      resetUnsavedChanges: () => {
+      asyncResetUnsavedChanges: async () => {
         controlGroupUnsavedChanges.api.resetUnsavedChanges();
+
+        const filtersReadyPromises: Array<Promise<void>> = [];
         Object.values(children$.value).forEach((controlApi) => {
           if (apiPublishesUnsavedChanges(controlApi)) controlApi.resetUnsavedChanges();
+          if (apiPublishesAsyncFilters(controlApi)) {
+            filtersReadyPromises.push(controlApi.untilFiltersReady());
+          }
         });
+
+        await Promise.all(filtersReadyPromises);
+
+        if (!comparators.autoApplySelections[0].value) {
+          applySelections();
+        }
       },
-    } as PublishesUnsavedChanges<ControlGroupRuntimeState>,
+    } as Pick<PublishesUnsavedChanges, 'unsavedChanges'> & {
+      asyncResetUnsavedChanges: () => Promise<void>;
+    },
   };
 }
diff --git a/examples/controls_example/public/react_controls/control_group/get_control_group_factory.tsx b/examples/controls_example/public/react_controls/control_group/get_control_group_factory.tsx
@@ -96,6 +96,7 @@ export const getControlGroupEmbeddableFactory = (services: {
       const dataLoading$ = new BehaviorSubject<boolean | undefined>(false);
 
       const unsavedChanges = initializeControlGroupUnsavedChanges(
+        selectionsManager.applySelections,
         controlsManager.api.children$,
         {
           ...controlsManager.comparators,

diff --git a/examples/controls_example/public/react_controls/control_group/types.ts b/examples/controls_example/public/react_controls/control_group/types.ts
@@ -55,10 +55,11 @@ export type ControlGroupApi = PresentationContainer &
   HasSerializedChildState<ControlPanelState> &
   HasEditCapabilities &
   PublishesDataLoading &
-  PublishesUnsavedChanges &
+  Pick<PublishesUnsavedChanges, 'unsavedChanges'> &
   PublishesControlGroupDisplaySettings &
   PublishesTimeslice &
   Partial<HasParentApi<PublishesUnifiedSearch> & HasSaveNotification> & {
+    asyncResetUnsavedChanges: () => Promise<void>;
     autoApplySelections$: PublishingSubject<boolean>;
     controlFetch$: (controlUuid: string) => Observable<ControlFetchContext>;
     getLastSavedControlState: (controlUuid: string) => object;

diff --git a/examples/controls_example/public/react_controls/data_controls/initialize_data_control.ts b/examples/controls_example/public/react_controls/data_controls/initialize_data_control.ts
@@ -7,7 +7,7 @@
  */
 
 import { isEqual } from 'lodash';
-import { BehaviorSubject, combineLatest, first, switchMap } from 'rxjs';
+import { BehaviorSubject, combineLatest, debounceTime, first, skip, switchMap, tap } from 'rxjs';
 
 import { CoreStart } from '@kbn/core-lifecycle-browser';
 import {
@@ -45,9 +45,12 @@ export const initializeDataControl = <EditorState extends object = {}>(
   api: ControlApiInitialization<DataControlApi>;
   cleanup: () => void;
   comparators: StateComparators<DefaultDataControlState>;
+  setters: {
+    onSelectionChange: () => void;
+    setOutputFilter: (filter: Filter | undefined) => void;
+  };
   stateManager: ControlStateManager<DefaultDataControlState>;
   serialize: () => SerializedPanelState<DefaultControlState>;
-  untilFiltersInitialized: () => Promise<void>;
 } => {
   const defaultControl = initializeDefaultControlApi(state);
 
@@ -57,6 +60,7 @@ export const initializeDataControl = <EditorState extends object = {}>(
   const fieldName = new BehaviorSubject<string>(state.fieldName);
   const dataViews = new BehaviorSubject<DataView[] | undefined>(undefined);
   const filters$ = new BehaviorSubject<Filter[] | undefined>(undefined);
+  const filtersReady$ = new BehaviorSubject<boolean>(false);
   const field$ = new BehaviorSubject<DataViewField | undefined>(undefined);
   const fieldFormatter = new BehaviorSubject<DataControlFieldFormatter>((toFormat: any) =>
     String(toFormat)
@@ -69,14 +73,14 @@ export const initializeDataControl = <EditorState extends object = {}>(
     title: panelTitle,
   };
 
-  function clearBlockingError() {
-    if (defaultControl.api.blockingError.value) {
-      defaultControl.api.setBlockingError(undefined);
-    }
-  }
-
   const dataViewIdSubscription = dataViewId
     .pipe(
+      tap(() => {
+        filtersReady$.next(false);
+        if (defaultControl.api.blockingError.value) {
+          defaultControl.api.setBlockingError(undefined);
+        }
+      }),
       switchMap(async (currentDataViewId) => {
         let dataView: DataView | undefined;
         try {
@@ -90,14 +94,17 @@ export const initializeDataControl = <EditorState extends object = {}>(
     .subscribe(({ dataView, error }) => {
       if (error) {
         defaultControl.api.setBlockingError(error);
-      } else {
-        clearBlockingError();
       }
       dataViews.next(dataView ? [dataView] : undefined);
     });
 
-  const fieldNameSubscription = combineLatest([dataViews, fieldName]).subscribe(
-    ([nextDataViews, nextFieldName]) => {
+  const fieldNameSubscription = combineLatest([dataViews, fieldName])
+    .pipe(
+      tap(() => {
+        filtersReady$.next(false);
+      })
+    )
+    .subscribe(([nextDataViews, nextFieldName]) => {
       const dataView = nextDataViews
         ? nextDataViews.find(({ id }) => dataViewId.value === id)
         : undefined;
@@ -115,8 +122,8 @@ export const initializeDataControl = <EditorState extends object = {}>(
             })
           )
         );
-      } else {
-        clearBlockingError();
+      } else if (defaultControl.api.blockingError.value) {
+        defaultControl.api.setBlockingError(undefined);
       }
 
       field$.next(field);
@@ -125,8 +132,7 @@ export const initializeDataControl = <EditorState extends object = {}>(
       if (spec) {
         fieldFormatter.next(dataView.getFormatterForField(spec).getConverterFor('text'));
       }
-    }
-  );
+    });
 
   const onEdit = async () => {
     // get the initial state from the state manager
@@ -172,6 +178,13 @@ export const initializeDataControl = <EditorState extends object = {}>(
     });
   };
 
+  const filtersReadySubscription = filters$.pipe(skip(1), debounceTime(0)).subscribe(() => {
+    // Set filtersReady$.next(true); in filters$ subscription instead of setOutputFilter
+    // to avoid signaling filters ready until after filters have been emitted
+    // to avoid timing issues
+    filtersReady$.next(true);
+  });
+
   const api: ControlApiInitialization<DataControlApi> = {
     ...defaultControl.api,
     panelTitle,
@@ -181,24 +194,41 @@ export const initializeDataControl = <EditorState extends object = {}>(
     fieldFormatter,
     onEdit,
     filters$,
-    setOutputFilter: (newFilter: Filter | undefined) => {
-      filters$.next(newFilter ? [newFilter] : undefined);
-    },
     isEditingEnabled: () => true,
+    untilFiltersReady: async () => {
+      return new Promise((resolve) => {
+        combineLatest([defaultControl.api.blockingError, filtersReady$])
+          .pipe(
+            first(([blockingError, filtersReady]) => filtersReady || blockingError !== undefined)
+          )
+          .subscribe(() => {
+            resolve();
+          });
+      });
+    },
   };
 
   return {
     api,
     cleanup: () => {
       dataViewIdSubscription.unsubscribe();
       fieldNameSubscription.unsubscribe();
+      filtersReadySubscription.unsubscribe();
     },
     comparators: {
       ...defaultControl.comparators,
       title: [panelTitle, (value: string | undefined) => panelTitle.next(value)],
       dataViewId: [dataViewId, (value: string) => dataViewId.next(value)],
       fieldName: [fieldName, (value: string) => fieldName.next(value)],
     },
+    setters: {
+      onSelectionChange: () => {
+        filtersReady$.next(false);
+      },
+      setOutputFilter: (newFilter: Filter | undefined) => {
+        filters$.next(newFilter ? [newFilter] : undefined);
+      },
+    },
     stateManager,
     serialize: () => {
       return {
@@ -217,19 +247,5 @@ export const initializeDataControl = <EditorState extends object = {}>(
         ],
       };
     },
-    untilFiltersInitialized: async () => {
-      return new Promise((resolve) => {
-        combineLatest([defaultControl.api.blockingError, filters$])
-          .pipe(
-            first(
-              ([blockingError, filters]) =>
-                blockingError !== undefined || (filters?.length ?? 0) > 0
-            )
-          )
-          .subscribe(() => {
-            resolve();
-          });
-      });
-    },
   };
 };
diff --git a/examples/controls_example/public/react_controls/data_controls/mocks/api_mocks.tsx b/examples/controls_example/public/react_controls/data_controls/mocks/api_mocks.tsx
@@ -11,12 +11,16 @@ import { BehaviorSubject } from 'rxjs';
 import { OptionsListSuggestions } from '@kbn/controls-plugin/common/options_list/types';
 import { DataViewField } from '@kbn/data-views-plugin/common';
 
+import { PublishingSubject } from '@kbn/presentation-publishing';
 import { OptionsListSelection } from '../../../../common/options_list/options_list_selections';
 import { OptionsListSearchTechnique } from '../../../../common/options_list/suggestions_searching';
 import { OptionsListSortingType } from '../../../../common/options_list/suggestions_sorting';
 import { OptionsListDisplaySettings } from '../options_list_control/types';
 
 export const getOptionsListMocks = () => {
+  const selectedOptions$ = new BehaviorSubject<OptionsListSelection[] | undefined>(undefined);
+  const exclude$ = new BehaviorSubject<boolean | undefined>(undefined);
+  const existsSelected$ = new BehaviorSubject<boolean | undefined>(undefined);
   return {
     api: {
       uuid: 'testControl',
@@ -30,17 +34,23 @@ export const getOptionsListMocks = () => {
       },
       fieldFormatter: new BehaviorSubject((value: string | number) => String(value)),
       makeSelection: jest.fn(),
+      setExclude: (next: boolean | undefined) => exclude$.next(next),
     },
     stateManager: {
       searchString: new BehaviorSubject<string>(''),
       searchStringValid: new BehaviorSubject<boolean>(true),
       fieldName: new BehaviorSubject<string>('field'),
-      exclude: new BehaviorSubject<boolean | undefined>(undefined),
-      existsSelected: new BehaviorSubject<boolean | undefined>(undefined),
+      exclude: exclude$ as PublishingSubject<boolean | undefined>,
+      existsSelected: existsSelected$ as PublishingSubject<boolean | undefined>,
       sort: new BehaviorSubject<OptionsListSortingType | undefined>(undefined),
-      selectedOptions: new BehaviorSubject<OptionsListSelection[] | undefined>(undefined),
+      selectedOptions: selectedOptions$ as PublishingSubject<OptionsListSelection[] | undefined>,
       searchTechnique: new BehaviorSubject<OptionsListSearchTechnique | undefined>(undefined),
     },
     displaySettings: {} as OptionsListDisplaySettings,
+    // setSelectedOptions and setExistsSelected are not exposed via API because
+    // they are not used by components
+    // they are needed in tests however so expose them as top level keys
+    setSelectedOptions: (next: OptionsListSelection[] | undefined) => selectedOptions$.next(next),
+    setExistsSelected: (next: boolean | undefined) => existsSelected$.next(next),
   };
 };
diff --git a/...eact_controls/data_controls/options_list_control/components/options_list_control.test.tsx b/...eact_controls/data_controls/options_list_control/components/options_list_control.test.tsx
@@ -10,10 +10,9 @@ import React from 'react';
 
 import { DataViewField } from '@kbn/data-views-plugin/common';
 import { render } from '@testing-library/react';
-import { ControlStateManager } from '../../../types';
 import { getOptionsListMocks } from '../../mocks/api_mocks';
-import { OptionsListControlContext } from '../options_list_context_provider';
-import { OptionsListComponentApi, OptionsListComponentState } from '../types';
+import { ContextStateManager, OptionsListControlContext } from '../options_list_context_provider';
+import { OptionsListComponentApi } from '../types';
 import { OptionsListControl } from './options_list_control';
 
 describe('Options list control', () => {
@@ -31,7 +30,7 @@ describe('Options list control', () => {
         value={{
           api: api as unknown as OptionsListComponentApi,
           displaySettings,
-          stateManager: stateManager as unknown as ControlStateManager<OptionsListComponentState>,
+          stateManager: stateManager as unknown as ContextStateManager,
         }}
       >
         <OptionsListControl controlPanelClassName="controlPanel" />
@@ -42,8 +41,8 @@ describe('Options list control', () => {
   test('if exclude = false and existsSelected = true, then the option should read "Exists"', async () => {
     const mocks = getOptionsListMocks();
     mocks.api.uuid = 'testExists';
-    mocks.stateManager.exclude.next(false);
-    mocks.stateManager.existsSelected.next(true);
+    mocks.api.setExclude(false);
+    mocks.setExistsSelected(true);
     const control = mountComponent(mocks);
     const existsOption = control.getByTestId('optionsList-control-testExists');
     expect(existsOption).toHaveTextContent('Exists');
@@ -52,8 +51,8 @@ describe('Options list control', () => {
   test('if exclude = true and existsSelected = true, then the option should read "Does not exist"', async () => {
     const mocks = getOptionsListMocks();
     mocks.api.uuid = 'testDoesNotExist';
-    mocks.stateManager.exclude.next(true);
-    mocks.stateManager.existsSelected.next(true);
+    mocks.api.setExclude(true);
+    mocks.setExistsSelected(true);
     const control = mountComponent(mocks);
     const existsOption = control.getByTestId('optionsList-control-testDoesNotExist');
     expect(existsOption).toHaveTextContent('DOES NOT Exist');
@@ -68,7 +67,7 @@ describe('Options list control', () => {
         { value: 'bark', docCount: 10 },
         { value: 'meow', docCount: 12 },
       ]);
-      mocks.stateManager.selectedOptions.next(['woof', 'bark']);
+      mocks.setSelectedOptions(['woof', 'bark']);
       mocks.api.field$.next({
         name: 'Test keyword field',
         type: 'keyword',
@@ -87,7 +86,7 @@ describe('Options list control', () => {
       { value: 2, docCount: 10 },
       { value: 3, docCount: 12 },
     ]);
-    mocks.stateManager.selectedOptions.next([1, 2]);
+    mocks.setSelectedOptions([1, 2]);
     mocks.api.field$.next({
       name: 'Test keyword field',
       type: 'number',
@@ -105,7 +104,7 @@ describe('Options list control', () => {
       { value: 'bark', docCount: 10 },
       { value: 'meow', docCount: 12 },
     ]);
-    mocks.stateManager.selectedOptions.next(['woof', 'bark']);
+    mocks.setSelectedOptions(['woof', 'bark']);
     mocks.api.invalidSelections$.next(new Set(['woof']));
     mocks.api.field$.next({
       name: 'Test keyword field',