Elasticsearch: Persist custom value for size option in Terms Bucket Agg (#36194)

* Add custom hook to handle creatable select options

* Refactor Terms Settings Editor

* Make props of Bucket Aggregation settings editors consistent

* Rename hook to something more descriptive

* Move render test helper

* Add tests

* small refactor

* Remove useless test

public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/DateHistogramSettingsEditor.test.tsx

@@ -1,92 +0,0 @@
-import { getDefaultTimeRange } from '@grafana/data';
-import { render, screen } from '@testing-library/react';
-import userEvent from '@testing-library/user-event';
-import { ElasticDatasource } from 'app/plugins/datasource/elasticsearch/datasource';
-import { ElasticsearchQuery } from 'app/plugins/datasource/elasticsearch/types';
-import React, { ComponentProps, ReactNode } from 'react';
-import { ElasticsearchProvider } from '../../ElasticsearchQueryContext';
-import { DateHistogram } from '../aggregations';
-import { DateHistogramSettingsEditor } from './DateHistogramSettingsEditor';
-
-const renderWithESProvider = (
-  ui: ReactNode,
-  {
-    providerProps: {
-      datasource = {} as ElasticDatasource,
-      query = { refId: 'A' },
-      onChange = () => void 0,
-      onRunQuery = () => void 0,
-      range = getDefaultTimeRange(),
-    } = {},
-    ...renderOptions
-  }: { providerProps?: Partial<Omit<ComponentProps<typeof ElasticsearchProvider>, 'children'>> } & Parameters<
-    typeof render
-  >[1]
-) => {
-  return render(
-    <ElasticsearchProvider
-      query={query}
-      onChange={onChange}
-      datasource={datasource}
-      onRunQuery={onRunQuery}
-      range={range}
-    >
-      {ui}
-    </ElasticsearchProvider>,
-    renderOptions
-  );
-};
-
-describe('DateHistogram Settings Editor', () => {
-  describe('Custom options for interval', () => {
-    it('Allows users to create and select case sensitive custom options', () => {
-      const bucketAgg: DateHistogram = {
-        id: '1',
-        type: 'date_histogram',
-        settings: {
-          interval: 'auto',
-        },
-      };
-
-      const query: ElasticsearchQuery = {
-        refId: 'A',
-        bucketAggs: [bucketAgg],
-        metrics: [{ id: '2', type: 'count' }],
-        query: '',
-      };
-
-      const onChange = jest.fn();
-
-      renderWithESProvider(<DateHistogramSettingsEditor bucketAgg={bucketAgg} />, {
-        providerProps: { query, onChange },
-      });
-
-      const intervalInput = screen.getByLabelText('Interval') as HTMLInputElement;
-
-      expect(intervalInput).toBeInTheDocument();
-      expect(screen.getByText('auto')).toBeInTheDocument();
-
-      // we open the menu
-      userEvent.click(intervalInput);
-
-      // default options don't have 1M but 1m
-      expect(screen.queryByText('1M')).not.toBeInTheDocument();
-      expect(screen.getByText('1m')).toBeInTheDocument();
-
-      // we type in the input 1M, which should prompt an option creation
-      userEvent.type(intervalInput, '1M');
-      const creatableOption = screen.getByLabelText('Select option');
-      expect(creatableOption).toHaveTextContent('Create: 1M');
-
-      // we click on the creatable option to trigger its creation
-      userEvent.click(creatableOption);
-
-      expect(onChange).toHaveBeenCalled();
-
-      // we open the menu again
-      userEvent.click(intervalInput);
-      // the created option should be available
-      expect(screen.getByText('1M')).toBeInTheDocument();
-    });
-  });
-});

public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/DateHistogramSettingsEditor.tsx

@@ -1,4 +1,4 @@
-import React, { ComponentProps, useState } from 'react';
+import React, { ComponentProps } from 'react';
 import { InlineField, Input, Select } from '@grafana/ui';
 import { DateHistogram } from '../aggregations';
 import { bucketAggregationConfig } from '../utils';
@@ -7,8 +7,9 @@ import { SelectableValue } from '@grafana/data';
 import { changeBucketAggregationSetting } from '../state/actions';
 import { inlineFieldProps } from '.';
 import { uniqueId } from 'lodash';
+import { useCreatableSelectPersistedBehaviour } from '../../../hooks/useCreatableSelectPersistedBehaviour';
 
-type IntervalOption = SelectableValue<string>;
+type IntervalOption = Required<Pick<SelectableValue<string>, 'label' | 'value'>>;
 
 const defaultIntervalOptions: IntervalOption[] = [
   { label: 'auto', value: 'auto' },
@@ -42,43 +43,23 @@ interface Props {
   bucketAgg: DateHistogram;
 }
 
-const getInitialState = (initialValue?: string): IntervalOption[] => {
-  return defaultIntervalOptions.concat(
-    defaultIntervalOptions.some(hasValue(initialValue))
-      ? []
-      : {
-          value: initialValue,
-          label: initialValue,
-        }
-  );
-};
-
 export const DateHistogramSettingsEditor = ({ bucketAgg }: Props) => {
   const dispatch = useDispatch();
 
-  const [intervalOptions, setIntervalOptions] = useState<IntervalOption[]>(
-    getInitialState(bucketAgg.settings?.interval)
-  );
-
-  const addIntervalOption = (value: string) => setIntervalOptions([...intervalOptions, { value, label: value }]);
-
   const handleIntervalChange = (v: string) => dispatch(changeBucketAggregationSetting(bucketAgg, 'interval', v));
 
   return (
     <>
       <InlineField label="Interval" {...inlineFieldProps}>
-        <Select<string>
+        <Select
           inputId={uniqueId('es-date_histogram-interval')}
-          onChange={(e) => handleIntervalChange(e.value!)}
-          options={intervalOptions}
-          value={bucketAgg.settings?.interval || bucketAggregationConfig[bucketAgg.type].defaultSettings?.interval}
-          allowCustomValue
           isValidNewOption={isValidNewOption}
           filterOption={optionStartsWithValue}
-          onCreateOption={(value) => {
-            addIntervalOption(value);
-            handleIntervalChange(value);
-          }}
+          {...useCreatableSelectPersistedBehaviour({
+            options: defaultIntervalOptions,
+            value: bucketAgg.settings?.interval || bucketAggregationConfig.date_histogram.defaultSettings?.interval,
+            onChange: handleIntervalChange,
+          })}
         />
       </InlineField>
 
@@ -86,7 +67,7 @@ export const DateHistogramSettingsEditor = ({ bucketAgg }: Props) => {
         <Input
           onBlur={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'min_doc_count', e.target.value!))}
           defaultValue={
-            bucketAgg.settings?.min_doc_count || bucketAggregationConfig[bucketAgg.type].defaultSettings?.min_doc_count
+            bucketAgg.settings?.min_doc_count || bucketAggregationConfig.date_histogram.defaultSettings?.min_doc_count
           }
         />
       </InlineField>
@@ -95,7 +76,7 @@ export const DateHistogramSettingsEditor = ({ bucketAgg }: Props) => {
         <Input
           onBlur={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'trimEdges', e.target.value!))}
           defaultValue={
-            bucketAgg.settings?.trimEdges || bucketAggregationConfig[bucketAgg.type].defaultSettings?.trimEdges
+            bucketAgg.settings?.trimEdges || bucketAggregationConfig.date_histogram.defaultSettings?.trimEdges
           }
         />
       </InlineField>
@@ -107,7 +88,7 @@ export const DateHistogramSettingsEditor = ({ bucketAgg }: Props) => {
       >
         <Input
           onBlur={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'offset', e.target.value!))}
-          defaultValue={bucketAgg.settings?.offset || bucketAggregationConfig[bucketAgg.type].defaultSettings?.offset}
+          defaultValue={bucketAgg.settings?.offset || bucketAggregationConfig.date_histogram.defaultSettings?.offset}
         />
       </InlineField>
     </>

public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/FiltersSettingsEditor/index.tsx

@@ -10,25 +10,25 @@ import { addFilter, changeFilter, removeFilter } from './state/actions';
 import { reducer as filtersReducer } from './state/reducer';
 
 interface Props {
-  value: Filters;
+  bucketAgg: Filters;
 }
 
-export const FiltersSettingsEditor = ({ value }: Props) => {
+export const FiltersSettingsEditor = ({ bucketAgg }: Props) => {
   const upperStateDispatch = useDispatch<BucketAggregationAction<Filters>>();
 
   const dispatch = useStatelessReducer(
-    (newState) => upperStateDispatch(changeBucketAggregationSetting(value, 'filters', newState)),
-    value.settings?.filters,
+    (newState) => upperStateDispatch(changeBucketAggregationSetting(bucketAgg, 'filters', newState)),
+    bucketAgg.settings?.filters,
     filtersReducer
   );
 
   // The model might not have filters (or an empty array of filters) in it because of the way it was built in previous versions of the datasource.
   // If this is the case we add a default one.
   useEffect(() => {
-    if (!value.settings?.filters?.length) {
+    if (!bucketAgg.settings?.filters?.length) {
       dispatch(addFilter());
     }
-  }, [dispatch, value.settings?.filters?.length]);
+  }, [dispatch, bucketAgg.settings?.filters?.length]);
 
   return (
     <>
@@ -38,7 +38,7 @@ export const FiltersSettingsEditor = ({ value }: Props) => {
           flex-direction: column;
         `}
       >
-        {value.settings?.filters!.map((filter, index) => (
+        {bucketAgg.settings?.filters!.map((filter, index) => (
           <div
             key={index}
             className={css`
@@ -69,7 +69,7 @@ export const FiltersSettingsEditor = ({ value }: Props) => {
             </InlineField>
             <AddRemove
               index={index}
-              elements={value.settings?.filters || []}
+              elements={bucketAgg.settings?.filters || []}
               onAdd={() => dispatch(addFilter())}
               onRemove={() => dispatch(removeFilter(index))}
             />

public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/TermsSettingsEditor.tsx

@@ -0,0 +1,69 @@
+import React from 'react';
+import { InlineField, Select, Input } from '@grafana/ui';
+import { Terms } from '../aggregations';
+import { useDispatch } from '../../../../hooks/useStatelessReducer';
+import { inlineFieldProps } from '.';
+import { bucketAggregationConfig, createOrderByOptionsFromMetrics, orderOptions, sizeOptions } from '../utils';
+import { useCreatableSelectPersistedBehaviour } from '../../../hooks/useCreatableSelectPersistedBehaviour';
+import { changeBucketAggregationSetting } from '../state/actions';
+import { useQuery } from '../../ElasticsearchQueryContext';
+
+interface Props {
+  bucketAgg: Terms;
+}
+
+export const TermsSettingsEditor = ({ bucketAgg }: Props) => {
+  const { metrics } = useQuery();
+  const orderBy = createOrderByOptionsFromMetrics(metrics);
+
+  const dispatch = useDispatch();
+
+  return (
+    <>
+      <InlineField label="Order" {...inlineFieldProps}>
+        <Select
+          onChange={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'order', e.value!))}
+          options={orderOptions}
+          value={bucketAgg.settings?.order || bucketAggregationConfig.terms.defaultSettings?.order}
+        />
+      </InlineField>
+
+      <InlineField label="Size" {...inlineFieldProps}>
+        <Select
+          // TODO: isValidNewOption should only allow numbers & template variables
+          {...useCreatableSelectPersistedBehaviour({
+            options: sizeOptions,
+            value: bucketAgg.settings?.size || bucketAggregationConfig.terms.defaultSettings?.size,
+            onChange(value) {
+              dispatch(changeBucketAggregationSetting(bucketAgg, 'size', value));
+            },
+          })}
+        />
+      </InlineField>
+
+      <InlineField label="Min Doc Count" {...inlineFieldProps}>
+        <Input
+          onBlur={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'min_doc_count', e.target.value!))}
+          defaultValue={
+            bucketAgg.settings?.min_doc_count || bucketAggregationConfig.terms.defaultSettings?.min_doc_count
+          }
+        />
+      </InlineField>
+
+      <InlineField label="Order By" {...inlineFieldProps}>
+        <Select
+          onChange={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'orderBy', e.value!))}
+          options={orderBy}
+          value={bucketAgg.settings?.orderBy || bucketAggregationConfig.terms.defaultSettings?.orderBy}
+        />
+      </InlineField>
+
+      <InlineField label="Missing" {...inlineFieldProps}>
+        <Input
+          onBlur={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'missing', e.target.value!))}
+          defaultValue={bucketAgg.settings?.missing || bucketAggregationConfig.terms.defaultSettings?.missing}
+        />
+      </InlineField>
+    </>
+  );
+};

public/app/plugins/datasource/elasticsearch/components/QueryEditor/BucketAggregationsEditor/SettingsEditor/index.tsx

@@ -1,14 +1,14 @@
-import { InlineField, Input, Select } from '@grafana/ui';
+import { InlineField, Input } from '@grafana/ui';
 import React, { ComponentProps } from 'react';
 import { useDispatch } from '../../../../hooks/useStatelessReducer';
 import { SettingsEditorContainer } from '../../SettingsEditorContainer';
 import { changeBucketAggregationSetting } from '../state/actions';
 import { BucketAggregation } from '../aggregations';
-import { bucketAggregationConfig, createOrderByOptionsFromMetrics, orderOptions, sizeOptions } from '../utils';
+import { bucketAggregationConfig } from '../utils';
 import { FiltersSettingsEditor } from './FiltersSettingsEditor';
 import { useDescription } from './useDescription';
-import { useQuery } from '../../ElasticsearchQueryContext';
 import { DateHistogramSettingsEditor } from './DateHistogramSettingsEditor';
+import { TermsSettingsEditor } from './TermsSettingsEditor';
 
 export const inlineFieldProps: Partial<ComponentProps<typeof InlineField>> = {
   labelWidth: 16,
@@ -21,59 +21,13 @@ interface Props {
 export const SettingsEditor = ({ bucketAgg }: Props) => {
   const dispatch = useDispatch();
 
-  const { metrics } = useQuery();
   const settingsDescription = useDescription(bucketAgg);
-  const orderBy = createOrderByOptionsFromMetrics(metrics);
 
   return (
     <SettingsEditorContainer label={settingsDescription}>
-      {bucketAgg.type === 'terms' && (
-        <>
-          <InlineField label="Order" {...inlineFieldProps}>
-            <Select
-              onChange={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'order', e.value!))}
-              options={orderOptions}
-              value={bucketAgg.settings?.order || bucketAggregationConfig[bucketAgg.type].defaultSettings?.order}
-            />
-          </InlineField>
-
-          <InlineField label="Size" {...inlineFieldProps}>
-            <Select
-              onChange={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'size', e.value!))}
-              options={sizeOptions}
-              value={bucketAgg.settings?.size || bucketAggregationConfig[bucketAgg.type].defaultSettings?.size}
-              allowCustomValue
-            />
-          </InlineField>
-
-          <InlineField label="Min Doc Count" {...inlineFieldProps}>
-            <Input
-              onBlur={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'min_doc_count', e.target.value!))}
-              defaultValue={
-                bucketAgg.settings?.min_doc_count ||
-                bucketAggregationConfig[bucketAgg.type].defaultSettings?.min_doc_count
-              }
-            />
-          </InlineField>
-
-          <InlineField label="Order By" {...inlineFieldProps}>
-            <Select
-              onChange={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'orderBy', e.value!))}
-              options={orderBy}
-              value={bucketAgg.settings?.orderBy || bucketAggregationConfig[bucketAgg.type].defaultSettings?.orderBy}
-            />
-          </InlineField>
-
-          <InlineField label="Missing" {...inlineFieldProps}>
-            <Input
-              onBlur={(e) => dispatch(changeBucketAggregationSetting(bucketAgg, 'missing', e.target.value!))}
-              defaultValue={
-                bucketAgg.settings?.missing || bucketAggregationConfig[bucketAgg.type].defaultSettings?.missing
-              }
-            />
-          </InlineField>
-        </>
-      )}
+      {bucketAgg.type === 'terms' && <TermsSettingsEditor bucketAgg={bucketAgg} />}
+      {bucketAgg.type === 'date_histogram' && <DateHistogramSettingsEditor bucketAgg={bucketAgg} />}
+      {bucketAgg.type === 'filters' && <FiltersSettingsEditor bucketAgg={bucketAgg} />}
 
       {bucketAgg.type === 'geohash_grid' && (
         <InlineField label="Precision" {...inlineFieldProps}>
@@ -86,8 +40,6 @@ export const SettingsEditor = ({ bucketAgg }: Props) => {
         </InlineField>
       )}
 
-      {bucketAgg.type === 'date_histogram' && <DateHistogramSettingsEditor bucketAgg={bucketAgg} />}
-
       {bucketAgg.type === 'histogram' && (
         <>
           <InlineField label="Interval" {...inlineFieldProps}>
@@ -110,8 +62,6 @@ export const SettingsEditor = ({ bucketAgg }: Props) => {
           </InlineField>
         </>
       )}
-
-      {bucketAgg.type === 'filters' && <FiltersSettingsEditor value={bucketAgg} />}
     </SettingsEditorContainer>
   );
 };

public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.test.tsx

@@ -0,0 +1,111 @@
+import React from 'react';
+import { render, screen } from '@testing-library/react';
+import { Select, InlineField } from '@grafana/ui';
+import { useCreatableSelectPersistedBehaviour } from './useCreatableSelectPersistedBehaviour';
+import userEvent from '@testing-library/user-event';
+
+describe('useCreatableSelectPersistedBehaviour', () => {
+  it('Should make a Select accept custom values', () => {
+    const MyComp = (_: { force?: boolean }) => (
+      <InlineField label="label">
+        <Select
+          inputId="select"
+          {...useCreatableSelectPersistedBehaviour({
+            options: [{ label: 'Option 1', value: 'Option 1' }],
+            onChange() {},
+          })}
+        />
+      </InlineField>
+    );
+
+    const { rerender } = render(<MyComp />);
+
+    const input = screen.getByLabelText('label') as HTMLInputElement;
+    expect(input).toBeInTheDocument();
+
+    // we open the menu
+    userEvent.click(input);
+
+    expect(screen.getByText('Option 1')).toBeInTheDocument();
+
+    // we type in the input 'Option 2', which should prompt an option creation
+    userEvent.type(input, 'Option 2');
+    const creatableOption = screen.getByLabelText('Select option');
+    expect(creatableOption).toHaveTextContent('Create: Option 2');
+
+    // we click on the creatable option to trigger its creation
+    userEvent.click(creatableOption);
+
+    // Forcing a rerender
+    rerender(<MyComp force={true} />);
+
+    // we open the menu again
+    userEvent.click(input);
+    // the created option should be available
+    expect(screen.getByText('Option 2')).toBeInTheDocument();
+  });
+
+  it('Should handle onChange properly', () => {
+    const onChange = jest.fn();
+    const MyComp = () => (
+      <InlineField label="label">
+        <Select
+          inputId="select"
+          {...useCreatableSelectPersistedBehaviour({
+            options: [{ label: 'Option 1', value: 'Option 1' }],
+            onChange,
+          })}
+        />
+      </InlineField>
+    );
+
+    render(<MyComp />);
+
+    const input = screen.getByLabelText('label') as HTMLInputElement;
+    expect(input).toBeInTheDocument();
+
+    // we open the menu
+    userEvent.click(input);
+
+    const option1 = screen.getByText('Option 1');
+    expect(option1).toBeInTheDocument();
+
+    // Should call onChange when selecting an already existing option
+    userEvent.click(option1);
+    expect(onChange).toHaveBeenCalledWith('Option 1');
+
+    userEvent.click(input);
+
+    // we type in the input 'Option 2', which should prompt an option creation
+    userEvent.type(input, 'Option 2');
+    userEvent.click(screen.getByLabelText('Select option'));
+
+    expect(onChange).toHaveBeenCalledWith('Option 2');
+  });
+
+  it('Should create an option for value if value is not in options', () => {
+    const MyComp = (_: { force?: boolean }) => (
+      <InlineField label="label">
+        <Select
+          inputId="select"
+          {...useCreatableSelectPersistedBehaviour({
+            options: [{ label: 'Option 1', value: 'Option 1' }],
+            value: 'Option 2',
+            onChange() {},
+          })}
+        />
+      </InlineField>
+    );
+
+    render(<MyComp />);
+
+    const input = screen.getByLabelText('label') as HTMLInputElement;
+    expect(input).toBeInTheDocument();
+
+    // we open the menu
+    userEvent.click(input);
+
+    // we expect 2 elemnts having "Option 2": the input itself and the option.
+    expect(screen.getAllByText('Option 2')).toHaveLength(2);
+  });
+});

public/app/plugins/datasource/elasticsearch/components/hooks/useCreatableSelectPersistedBehaviour.ts

@@ -0,0 +1,52 @@
+import { SelectableValue } from '@grafana/data';
+import { Select } from '@grafana/ui';
+import { ComponentProps, useState } from 'react';
+
+const hasValue = <T extends SelectableValue>(searchValue: T['value']) => ({ value }: T) => value === searchValue;
+
+const getInitialState = <T extends SelectableValue>(initialOptions: T[], initialValue?: T['value']): T[] => {
+  if (initialValue === undefined || initialOptions.some(hasValue(initialValue))) {
+    return initialOptions;
+  }
+
+  return initialOptions.concat({
+    value: initialValue,
+    label: initialValue,
+  } as T);
+};
+
+interface Params<T extends SelectableValue> {
+  options: T[];
+  value?: T['value'];
+  onChange: (value: T['value']) => void;
+}
+
+/**
+ * Creates the Props needed by Select to handle custom values and handles custom value creation
+ * and the initial value when it is not present in the option array.
+ */
+export const useCreatableSelectPersistedBehaviour = <T extends SelectableValue>({
+  options: initialOptions,
+  value,
+  onChange,
+}: Params<T>): Pick<
+  ComponentProps<typeof Select>,
+  'onChange' | 'onCreateOption' | 'options' | 'allowCustomValue' | 'value'
+> => {
+  const [options, setOptions] = useState<T[]>(getInitialState(initialOptions, value));
+
+  const addOption = (newValue: T['value']) => setOptions([...options, { value: newValue, label: newValue } as T]);
+
+  return {
+    onCreateOption: (value) => {
+      addOption(value);
+      onChange(value);
+    },
+    onChange: (e) => {
+      onChange(e.value);
+    },
+    allowCustomValue: true,
+    options,
+    value,
+  };
+};

public/app/plugins/datasource/elasticsearch/test-helpers/render.tsx

@@ -0,0 +1,34 @@
+import React, { ComponentProps, ReactNode } from 'react';
+import { render } from '@testing-library/react';
+import { getDefaultTimeRange } from '@grafana/data';
+import { ElasticDatasource } from '../datasource';
+import { ElasticsearchProvider } from '../components/QueryEditor/ElasticsearchQueryContext';
+
+export const renderWithESProvider = (
+  ui: ReactNode,
+  {
+    providerProps: {
+      datasource = {} as ElasticDatasource,
+      query = { refId: 'A' },
+      onChange = () => void 0,
+      onRunQuery = () => void 0,
+      range = getDefaultTimeRange(),
+    } = {},
+    ...renderOptions
+  }: { providerProps?: Partial<Omit<ComponentProps<typeof ElasticsearchProvider>, 'children'>> } & Parameters<
+    typeof render
+  >[1]
+) => {
+  return render(
+    <ElasticsearchProvider
+      query={query}
+      onChange={onChange}
+      datasource={datasource}
+      onRunQuery={onRunQuery}
+      range={range}
+    >
+      {ui}
+    </ElasticsearchProvider>,
+    renderOptions
+  );
+};