[controls] fix Dashboard getting stuck at loading in Kibana when Cont…

…rols is used and mapping changed from integer to keyword (#163529) Closes #162474 ### Changes * RangeSliderEmbeddable - call setInitializationFinished when runRangeSliderQuery throws. This fixes the issue * Investigated if OptionsListEmbeddable is vulnerable to the same issue. It's not because it uses its own REST API that has a service wrapper `OptionsListService`. `OptionsListService` handles REST API errors. * Add unit test verifying OptionsListService.runOptionsListRequest does not throw when there are REST API errors and always returns a response. * Add unit tests ensuring setInitializationFinished is called for both RangeSliderEmbeddable and OptionsListEmbeddable in all cases * Other clean up * Fix uses of `dataViewsService.get`. `dataViewsService.get` throws when data view is not found. It does not return undefined. PR updates OptionsListEmbeddable, RangeSliderEmbeddable, and mocked data service * Fix uses of `dataView.getFieldByName`. `dataView.getFieldByName` returns undefined when field is not found and never throws. PR updates OptionsListEmbeddable and RangeSliderEmbeddable * Remove `resp` wrapper around mocked `fetch` results. ### Test instructions 1) In console run ``` PUT test1 PUT test1/_mapping { "properties": { "value": { "type": "integer" } } } PUT test1/_doc/1 { "value" : 1 } PUT test1/_doc/2 { "value" : 10 } ``` 2) create data view `test*` 3) create dashboard with range slider control on test*.value. 4) select a range in the range slider 5) save dashboard 6) run the following in console ``` PUT test2 PUT test2/_mapping { "properties": { "value": { "type": "keyword" } } } PUT test2/_doc/1 { "value" : "foo" } DELETE test1 ``` 7) Open dashboard saved above. Verify dashboard opens and control displays an error message about being unable to run aggregation on keyword field. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Devon Thomson <[email protected]>
elastic · Aug 11, 2023 · 0a74fa0 · 0a74fa0
1 parent 2a67e0f
commit 0a74fa0
Show file tree

Hide file tree

Showing 12 changed files with 471 additions and 104 deletions.
diff --git a/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.test.tsx b/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.test.tsx
@@ -0,0 +1,108 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { ControlGroupInput } from '../../../common';
+import { lazyLoadReduxToolsPackage } from '@kbn/presentation-util-plugin/public';
+import { storybookFlightsDataView } from '@kbn/presentation-util-plugin/public/mocks';
+import { OPTIONS_LIST_CONTROL } from '../../../common';
+import { ControlGroupContainer } from '../../control_group/embeddable/control_group_container';
+import { pluginServices } from '../../services';
+import { injectStorybookDataView } from '../../services/data_views/data_views.story';
+import { OptionsListEmbeddableFactory } from './options_list_embeddable_factory';
+import { OptionsListEmbeddable } from './options_list_embeddable';
+
+pluginServices.getServices().controls.getControlFactory = jest
+  .fn()
+  .mockImplementation((type: string) => {
+    if (type === OPTIONS_LIST_CONTROL) return new OptionsListEmbeddableFactory();
+  });
+
+describe('initialize', () => {
+  describe('without selected options', () => {
+    test('should notify control group when initialization is finished', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      // data view not required for test case
+      // setInitializationFinished is called before fetching options when value is not provided
+      injectStorybookDataView(undefined);
+
+      const control = await container.addOptionsListControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'OriginCityName',
+      });
+
+      expect(container.getInput().panels[control.getInput().id].type).toBe(OPTIONS_LIST_CONTROL);
+      expect(container.getOutput().embeddableLoaded[control.getInput().id]).toBe(true);
+    });
+  });
+
+  describe('with selected options', () => {
+    test('should set error message when data view can not be found', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(undefined);
+
+      const control = (await container.addOptionsListControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'OriginCityName',
+        selectedOptions: ['Seoul', 'Tokyo'],
+      })) as OptionsListEmbeddable;
+
+      // await redux dispatch
+      await new Promise((resolve) => process.nextTick(resolve));
+
+      const reduxState = control.getState();
+      expect(reduxState.output.loading).toBe(false);
+      expect(reduxState.componentState.error).toBe(
+        'mock DataViews service currentDataView is undefined, call injectStorybookDataView to set'
+      );
+    });
+
+    test('should set error message when field can not be found', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(storybookFlightsDataView);
+
+      const control = (await container.addOptionsListControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'myField',
+        selectedOptions: ['Seoul', 'Tokyo'],
+      })) as OptionsListEmbeddable;
+
+      // await redux dispatch
+      await new Promise((resolve) => process.nextTick(resolve));
+
+      const reduxState = control.getState();
+      expect(reduxState.output.loading).toBe(false);
+      expect(reduxState.componentState.error).toBe('Could not locate field: myField');
+    });
+
+    test('should notify control group when initialization is finished', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(storybookFlightsDataView);
+
+      const control = await container.addOptionsListControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'OriginCityName',
+        selectedOptions: ['Seoul', 'Tokyo'],
+      });
+
+      expect(container.getInput().panels[control.getInput().id].type).toBe(OPTIONS_LIST_CONTROL);
+      expect(container.getOutput().embeddableLoaded[control.getInput().id]).toBe(true);
+    });
+  });
+});
diff --git a/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx b/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx
@@ -245,13 +245,6 @@ export class OptionsListEmbeddable
     if (!this.dataView || this.dataView.id !== dataViewId) {
       try {
         this.dataView = await this.dataViewsService.get(dataViewId);
-        if (!this.dataView)
-          throw new Error(
-            i18n.translate('controls.optionsList.errors.dataViewNotFound', {
-              defaultMessage: 'Could not locate data view: {dataViewId}',
-              values: { dataViewId },
-            })
-          );
       } catch (e) {
         this.dispatch.setErrorMessage(e.message);
       }
@@ -260,25 +253,21 @@ export class OptionsListEmbeddable
     }
 
     if (this.dataView && (!this.field || this.field.name !== fieldName)) {
-      try {
-        const originalField = this.dataView.getFieldByName(fieldName);
-        if (!originalField) {
-          throw new Error(
-            i18n.translate('controls.optionsList.errors.fieldNotFound', {
-              defaultMessage: 'Could not locate field: {fieldName}',
-              values: { fieldName },
-            })
-          );
-        }
-
-        this.field = originalField.toSpec();
-      } catch (e) {
-        this.dispatch.setErrorMessage(e.message);
+      const field = this.dataView.getFieldByName(fieldName);
+      if (field) {
+        this.field = field.toSpec();
+        this.dispatch.setField(this.field);
+      } else {
+        this.dispatch.setErrorMessage(
+          i18n.translate('controls.optionsList.errors.fieldNotFound', {
+            defaultMessage: 'Could not locate field: {fieldName}',
+            values: { fieldName },
+          })
+        );
       }
-      this.dispatch.setField(this.field);
     }
 
-    return { dataView: this.dataView, field: this.field! };
+    return { dataView: this.dataView, field: this.field };
   };
 
   private runOptionsListQuery = async (size: number = MIN_OPTIONS_LIST_REQUEST_SIZE) => {

diff --git a/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.test.tsx b/src/plugins/controls/public/range_slider/embeddable/range_slider_embeddable.test.tsx
@@ -0,0 +1,211 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { of } from 'rxjs';
+import { ControlGroupInput } from '../../../common';
+import { lazyLoadReduxToolsPackage } from '@kbn/presentation-util-plugin/public';
+import { storybookFlightsDataView } from '@kbn/presentation-util-plugin/public/mocks';
+import { RANGE_SLIDER_CONTROL } from '../../../common';
+import { ControlGroupContainer } from '../../control_group/embeddable/control_group_container';
+import { pluginServices } from '../../services';
+import { injectStorybookDataView } from '../../services/data_views/data_views.story';
+import { RangeSliderEmbeddableFactory } from './range_slider_embeddable_factory';
+import { RangeSliderEmbeddable } from './range_slider_embeddable';
+
+let totalResults = 20;
+beforeEach(() => {
+  totalResults = 20;
+
+  pluginServices.getServices().controls.getControlFactory = jest
+    .fn()
+    .mockImplementation((type: string) => {
+      if (type === RANGE_SLIDER_CONTROL) return new RangeSliderEmbeddableFactory();
+    });
+
+  pluginServices.getServices().data.searchSource.create = jest.fn().mockImplementation(() => {
+    let isAggsRequest = false;
+    return {
+      setField: (key: string) => {
+        if (key === 'aggs') {
+          isAggsRequest = true;
+        }
+      },
+      fetch$: () => {
+        return isAggsRequest
+          ? of({
+              rawResponse: { aggregations: { minAgg: { value: 0 }, maxAgg: { value: 1000 } } },
+            })
+          : of({
+              rawResponse: { hits: { total: { value: totalResults } } },
+            });
+      },
+    };
+  });
+});
+
+describe('initialize', () => {
+  describe('without selected range', () => {
+    test('should notify control group when initialization is finished', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      // data view not required for test case
+      // setInitializationFinished is called before fetching slider range when value is not provided
+      injectStorybookDataView(undefined);
+
+      const control = await container.addRangeSliderControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'AvgTicketPrice',
+      });
+
+      expect(container.getInput().panels[control.getInput().id].type).toBe(RANGE_SLIDER_CONTROL);
+      expect(container.getOutput().embeddableLoaded[control.getInput().id]).toBe(true);
+    });
+  });
+
+  describe('with selected range', () => {
+    test('should set error message when data view can not be found', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(undefined);
+
+      const control = (await container.addRangeSliderControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'AvgTicketPrice',
+        value: ['150', '300'],
+      })) as RangeSliderEmbeddable;
+
+      // await redux dispatch
+      await new Promise((resolve) => process.nextTick(resolve));
+
+      const reduxState = control.getState();
+      expect(reduxState.output.loading).toBe(false);
+      expect(reduxState.componentState.error).toBe(
+        'mock DataViews service currentDataView is undefined, call injectStorybookDataView to set'
+      );
+    });
+
+    test('should set error message when field can not be found', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(storybookFlightsDataView);
+
+      const control = (await container.addRangeSliderControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'myField',
+        value: ['150', '300'],
+      })) as RangeSliderEmbeddable;
+
+      // await redux dispatch
+      await new Promise((resolve) => process.nextTick(resolve));
+
+      const reduxState = control.getState();
+      expect(reduxState.output.loading).toBe(false);
+      expect(reduxState.componentState.error).toBe('Could not locate field: myField');
+    });
+
+    test('should set invalid state when filter returns zero results', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(storybookFlightsDataView);
+      totalResults = 0;
+
+      const control = (await container.addRangeSliderControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'AvgTicketPrice',
+        value: ['150', '300'],
+      })) as RangeSliderEmbeddable;
+
+      // await redux dispatch
+      await new Promise((resolve) => process.nextTick(resolve));
+
+      const reduxState = control.getState();
+      expect(reduxState.output.filters?.length).toBe(0);
+      expect(reduxState.componentState.isInvalid).toBe(true);
+    });
+
+    test('should set range and filter', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(storybookFlightsDataView);
+
+      const control = (await container.addRangeSliderControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'AvgTicketPrice',
+        value: ['150', '300'],
+      })) as RangeSliderEmbeddable;
+
+      // await redux dispatch
+      await new Promise((resolve) => process.nextTick(resolve));
+
+      const reduxState = control.getState();
+      expect(reduxState.output.filters?.length).toBe(1);
+      expect(reduxState.output.filters?.[0].query).toEqual({
+        range: {
+          AvgTicketPrice: {
+            gte: 150,
+            lte: 300,
+          },
+        },
+      });
+      expect(reduxState.componentState.isInvalid).toBe(false);
+      expect(reduxState.componentState.min).toBe(0);
+      expect(reduxState.componentState.max).toBe(1000);
+    });
+
+    test('should notify control group when initialization is finished', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(storybookFlightsDataView);
+
+      const control = await container.addRangeSliderControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'AvgTicketPrice',
+        value: ['150', '300'],
+      });
+
+      expect(container.getInput().panels[control.getInput().id].type).toBe(RANGE_SLIDER_CONTROL);
+      expect(container.getOutput().embeddableLoaded[control.getInput().id]).toBe(true);
+    });
+
+    test('should notify control group when initialization throws', async () => {
+      const reduxEmbeddablePackage = await lazyLoadReduxToolsPackage();
+      const controlGroupInput = { chainingSystem: 'NONE', panels: {} } as ControlGroupInput;
+      const container = new ControlGroupContainer(reduxEmbeddablePackage, controlGroupInput);
+
+      injectStorybookDataView(storybookFlightsDataView);
+
+      pluginServices.getServices().data.searchSource.create = jest.fn().mockImplementation(() => ({
+        setField: () => {},
+        fetch$: () => {
+          throw new Error('Simulated _search request error');
+        },
+      }));
+
+      const control = await container.addRangeSliderControl({
+        dataViewId: 'demoDataFlights',
+        fieldName: 'AvgTicketPrice',
+        value: ['150', '300'],
+      });
+
+      expect(container.getInput().panels[control.getInput().id].type).toBe(RANGE_SLIDER_CONTROL);
+      expect(container.getOutput().embeddableLoaded[control.getInput().id]).toBe(true);
+    });
+  });
+});