[Lens] Make Lens intervals default value adapt to histogram:maxBars A…

…dvanced Setting changes (elastic#89305) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
nreese · Feb 1, 2021 · c66124e · c66124e
1 parent 3255f90
commit c66124e
Show file tree

Hide file tree

Showing 11 changed files with 75 additions and 60 deletions.
diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx
@@ -167,7 +167,7 @@ export function getIndexPatternDatasource({
       });
     },
 
-    toExpression,
+    toExpression: (state, layerId) => toExpression(state, layerId, uiSettings),
 
     renderDataPanel(
       domElement: Element,

diff --git a/...lugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.test.tsx b/...lugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.test.tsx
@@ -83,9 +83,11 @@ const indexPattern2: IndexPattern = {
   ]),
 };
 
+const uiSettingsMock = {} as IUiSettingsClient;
+
 const defaultOptions = {
   storage: {} as IStorageWrapper,
-  uiSettings: {} as IUiSettingsClient,
+  uiSettings: uiSettingsMock,
   savedObjectsClient: {} as SavedObjectsClientContract,
   dateRange: {
     fromDate: 'now-1y',
@@ -200,7 +202,8 @@ describe('date_histogram', () => {
         layer.columns.col1 as DateHistogramIndexPatternColumn,
         'col1',
         indexPattern1,
-        layer
+        layer,
+        uiSettingsMock
       );
       expect(esAggsFn).toEqual(
         expect.objectContaining({
@@ -252,7 +255,8 @@ describe('date_histogram', () => {
             },
           ]),
         },
-        layer
+        layer,
+        uiSettingsMock
       );
       expect(esAggsFn).toEqual(
         expect.objectContaining({

diff --git a/...ugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx b/...ugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.test.tsx
@@ -16,9 +16,11 @@ import type { IndexPatternLayer } from '../../../types';
 import { createMockedIndexPattern } from '../../../mocks';
 import { FilterPopover } from './filter_popover';
 
+const uiSettingsMock = {} as IUiSettingsClient;
+
 const defaultProps = {
   storage: {} as IStorageWrapper,
-  uiSettings: {} as IUiSettingsClient,
+  uiSettings: uiSettingsMock,
   savedObjectsClient: {} as SavedObjectsClientContract,
   dateRange: { fromDate: 'now-1d', toDate: 'now' },
   data: dataPluginMock.createStartContract(),
@@ -84,7 +86,8 @@ describe('filters', () => {
         layer.columns.col1 as FiltersIndexPatternColumn,
         'col1',
         createMockedIndexPattern(),
-        layer
+        layer,
+        uiSettingsMock
       );
       expect(esAggsFn).toEqual(
         expect.objectContaining({

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/index.ts
@@ -239,7 +239,8 @@ interface FieldlessOperationDefinition<C extends BaseIndexPatternColumn> {
     column: C,
     columnId: string,
     indexPattern: IndexPattern,
-    layer: IndexPatternLayer
+    layer: IndexPatternLayer,
+    uiSettings: IUiSettingsClient
   ) => ExpressionAstFunction;
 }
 
@@ -283,7 +284,8 @@ interface FieldBasedOperationDefinition<C extends BaseIndexPatternColumn> {
     column: C,
     columnId: string,
     indexPattern: IndexPattern,
-    layer: IndexPatternLayer
+    layer: IndexPatternLayer,
+    uiSettings: IUiSettingsClient
   ) => ExpressionAstFunction;
   /**
    * Optional function to return the suffix used for ES bucket paths and esaggs column id.

diff --git a/...ck/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx b/...ck/plugins/lens/public/indexpattern_datasource/operations/definitions/last_value.test.tsx
@@ -15,9 +15,11 @@ import { LastValueIndexPatternColumn } from './last_value';
 import { lastValueOperation } from './index';
 import type { IndexPattern, IndexPatternLayer } from '../../types';
 
+const uiSettingsMock = {} as IUiSettingsClient;
+
 const defaultProps = {
   storage: {} as IStorageWrapper,
-  uiSettings: {} as IUiSettingsClient,
+  uiSettings: uiSettingsMock,
   savedObjectsClient: {} as SavedObjectsClientContract,
   dateRange: { fromDate: 'now-1d', toDate: 'now' },
   data: dataPluginMock.createStartContract(),
@@ -70,7 +72,8 @@ describe('last_value', () => {
         { ...lastValueColumn, params: { ...lastValueColumn.params } },
         'col1',
         {} as IndexPattern,
-        layer
+        layer,
+        uiSettingsMock
       );
       expect(esAggsFn).toEqual(
         expect.objectContaining({

diff --git a/...ck/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx b/...ck/plugins/lens/public/indexpattern_datasource/operations/definitions/percentile.test.tsx
@@ -17,9 +17,11 @@ import { EuiFieldNumber } from '@elastic/eui';
 import { act } from 'react-dom/test-utils';
 import { EuiFormRow } from '@elastic/eui';
 
+const uiSettingsMock = {} as IUiSettingsClient;
+
 const defaultProps = {
   storage: {} as IStorageWrapper,
-  uiSettings: {} as IUiSettingsClient,
+  uiSettings: uiSettingsMock,
   savedObjectsClient: {} as SavedObjectsClientContract,
   dateRange: { fromDate: 'now-1d', toDate: 'now' },
   data: dataPluginMock.createStartContract(),
@@ -72,7 +74,8 @@ describe('percentile', () => {
         percentileColumn,
         'col1',
         {} as IndexPattern,
-        layer
+        layer,
+        uiSettingsMock
       );
       expect(esAggsFn).toEqual(
         expect.objectContaining({

diff --git a/...lugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx b/...lugins/lens/public/indexpattern_datasource/operations/definitions/ranges/range_editor.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React, { useEffect, useState } from 'react';
+import React, { useState } from 'react';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n/react';
 import {
@@ -190,15 +190,6 @@ export const RangeEditor = ({
 }) => {
   const [isAdvancedEditor, toggleAdvancedEditor] = useState(params.type === MODES.Range);
 
-  // if the maxBars in the params is set to auto refresh it with the default value only on bootstrap
-  useEffect(() => {
-    if (!isAdvancedEditor) {
-      if (params.maxBars !== maxBars) {
-        setParam('maxBars', maxBars);
-      }
-    }
-  }, [maxBars, params.maxBars, setParam, isAdvancedEditor]);
-
   if (isAdvancedEditor) {
     return (
       <AdvancedRangeEditor

diff --git a/...plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx b/...plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.test.tsx
@@ -51,13 +51,15 @@ dataPluginMockValue.fieldFormats.deserialize = jest.fn().mockImplementation(({ p
 type ReactMouseEvent = React.MouseEvent<HTMLAnchorElement, MouseEvent> &
   React.MouseEvent<HTMLButtonElement, MouseEvent>;
 
+// need this for MAX_HISTOGRAM value
+const uiSettingsMock = ({
+  get: jest.fn().mockReturnValue(100),
+} as unknown) as IUiSettingsClient;
+
 const sourceField = 'MyField';
 const defaultOptions = {
   storage: {} as IStorageWrapper,
-  // need this for MAX_HISTOGRAM value
-  uiSettings: ({
-    get: () => 100,
-  } as unknown) as IUiSettingsClient,
+  uiSettings: uiSettingsMock,
   savedObjectsClient: {} as SavedObjectsClientContract,
   dateRange: {
     fromDate: 'now-1y',
@@ -143,7 +145,8 @@ describe('ranges', () => {
         layer.columns.col1 as RangeIndexPatternColumn,
         'col1',
         {} as IndexPattern,
-        layer
+        layer,
+        uiSettingsMock
       );
       expect(esAggsFn).toMatchInlineSnapshot(`
         Object {
@@ -166,6 +169,9 @@ describe('ranges', () => {
             "interval": Array [
               "auto",
             ],
+            "maxBars": Array [
+              49.5,
+            ],
             "min_doc_count": Array [
               false,
             ],
@@ -186,7 +192,8 @@ describe('ranges', () => {
         layer.columns.col1 as RangeIndexPatternColumn,
         'col1',
         {} as IndexPattern,
-        layer
+        layer,
+        uiSettingsMock
       );
 
       expect(esAggsFn).toEqual(
@@ -206,7 +213,8 @@ describe('ranges', () => {
         layer.columns.col1 as RangeIndexPatternColumn,
         'col1',
         {} as IndexPattern,
-        layer
+        layer,
+        uiSettingsMock
       );
 
       expect(esAggsFn).toEqual(
@@ -226,7 +234,8 @@ describe('ranges', () => {
         layer.columns.col1 as RangeIndexPatternColumn,
         'col1',
         {} as IndexPattern,
-        layer
+        layer,
+        uiSettingsMock
       );
 
       expect((esAggsFn as { arguments: unknown }).arguments).toEqual(
@@ -275,7 +284,7 @@ describe('ranges', () => {
 
       it('should start update the state with the default maxBars value', () => {
         const updateLayerSpy = jest.fn();
-        mount(
+        const instance = mount(
           <InlineOptions
             {...defaultOptions}
             layer={layer}
@@ -285,19 +294,7 @@ describe('ranges', () => {
           />
         );
 
-        expect(updateLayerSpy).toHaveBeenCalledWith({
-          ...layer,
-          columns: {
-            ...layer.columns,
-            col1: {
-              ...layer.columns.col1,
-              params: {
-                ...layer.columns.col1.params,
-                maxBars: GRANULARITY_DEFAULT_VALUE,
-              },
-            },
-          },
-        });
+        expect(instance.find(EuiRange).prop('value')).toEqual(String(GRANULARITY_DEFAULT_VALUE));
       });
 
       it('should update state when changing Max bars number', () => {
@@ -313,8 +310,6 @@ describe('ranges', () => {
           />
         );
 
-        // There's a useEffect in the component that updates the value on bootstrap
-        // because there's a debouncer, wait a bit before calling onChange
         act(() => {
           jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
 
@@ -358,8 +353,6 @@ describe('ranges', () => {
           />
         );
 
-        // There's a useEffect in the component that updates the value on bootstrap
-        // because there's a debouncer, wait a bit before calling onChange
         act(() => {
           jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
           // minus button
@@ -368,6 +361,7 @@ describe('ranges', () => {
             .find('button')
             .prop('onClick')!({} as ReactMouseEvent);
           jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
+          instance.update();
         });
 
         expect(updateLayerSpy).toHaveBeenCalledWith({
@@ -391,6 +385,7 @@ describe('ranges', () => {
             .find('button')
             .prop('onClick')!({} as ReactMouseEvent);
           jest.advanceTimersByTime(TYPING_DEBOUNCE_TIME * 4);
+          instance.update();
         });
 
         expect(updateLayerSpy).toHaveBeenCalledWith({
@@ -788,7 +783,7 @@ describe('ranges', () => {
           instance.find(EuiLink).first().prop('onClick')!({} as ReactMouseEvent);
         });
 
-        expect(updateLayerSpy.mock.calls[1][0].columns.col1.params.format).toEqual({
+        expect(updateLayerSpy.mock.calls[0][0].columns.col1.params.format).toEqual({
           id: 'custom',
           params: { decimals: 3 },
         });

diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx
@@ -132,7 +132,7 @@ export const rangeOperation: OperationDefinition<RangeIndexPatternColumn, 'field
       sourceField: field.name,
     };
   },
-  toEsAggsFn: (column, columnId) => {
+  toEsAggsFn: (column, columnId, indexPattern, layer, uiSettings) => {
     const { sourceField, params } = column;
     if (params.type === MODES.Range) {
       return buildExpressionFunction<AggFunctionsMapping['aggRange']>('aggRange', {
@@ -158,13 +158,15 @@ export const rangeOperation: OperationDefinition<RangeIndexPatternColumn, 'field
         ),
       }).toAst();
     }
+    const maxBarsDefaultValue =
+      (uiSettings.get(UI_SETTINGS.HISTOGRAM_MAX_BARS) - MIN_HISTOGRAM_BARS) / 2;
+
     return buildExpressionFunction<AggFunctionsMapping['aggHistogram']>('aggHistogram', {
       id: columnId,
       enabled: true,
       schema: 'segment',
       field: sourceField,
-      // fallback to 0 in case of empty string
-      maxBars: params.maxBars === AUTO_BARS ? undefined : params.maxBars,
+      maxBars: params.maxBars === AUTO_BARS ? maxBarsDefaultValue : params.maxBars,
       interval: 'auto',
       has_extended_bounds: false,
       min_doc_count: false,

diff --git a/...k/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx b/...k/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx
@@ -17,9 +17,11 @@ import type { TermsIndexPatternColumn } from '.';
 import { termsOperation } from '../index';
 import { IndexPattern, IndexPatternLayer } from '../../../types';
 
+const uiSettingsMock = {} as IUiSettingsClient;
+
 const defaultProps = {
   storage: {} as IStorageWrapper,
-  uiSettings: {} as IUiSettingsClient,
+  uiSettings: uiSettingsMock,
   savedObjectsClient: {} as SavedObjectsClientContract,
   dateRange: { fromDate: 'now-1d', toDate: 'now' },
   data: dataPluginMock.createStartContract(),
@@ -66,7 +68,8 @@ describe('terms', () => {
         { ...termsColumn, params: { ...termsColumn.params, otherBucket: true } },
         'col1',
         {} as IndexPattern,
-        layer
+        layer,
+        uiSettingsMock
       );
       expect(esAggsFn).toEqual(
         expect.objectContaining({
@@ -89,7 +92,8 @@ describe('terms', () => {
         },
         'col1',
         {} as IndexPattern,
-        layer
+        layer,
+        uiSettingsMock
       );
       expect(esAggsFn).toEqual(
         expect.objectContaining({
@@ -129,7 +133,8 @@ describe('terms', () => {
               },
             },
           },
-        }
+        },
+        uiSettingsMock
       );
       expect(esAggsFn).toEqual(
         expect.objectContaining({