Pass only necessary field to the editor_config registry

elastic · Jan 27, 2020 · 5042175 · 5042175
1 parent 2d3b569
commit 5042175
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 21 deletions.
diff --git a/src/legacy/ui/public/vis/editors/config/editor_config_providers.test.ts b/src/legacy/ui/public/vis/editors/config/editor_config_providers.test.ts
@@ -39,6 +39,8 @@ describe('EditorConfigProvider', () => {
       },
     ],
   } as any;
+  const aggTypeName = 'histogram';
+  const fieldName = '@timestamp';
 
   beforeEach(() => {
     registry = new EditorConfigProviderRegistry();
@@ -48,9 +50,9 @@ describe('EditorConfigProvider', () => {
     const provider = jest.fn<any, any>(() => ({}));
     registry.register(provider);
     expect(provider).not.toHaveBeenCalled();
-    const aggConfig = {} as AggConfig;
-    registry.getConfigForAgg(indexPattern, aggConfig);
-    expect(provider).toHaveBeenCalledWith(indexPattern, aggConfig);
+
+    registry.getConfigForAgg(indexPattern, aggTypeName, fieldName);
+    expect(provider).toHaveBeenCalledWith(indexPattern, aggTypeName, fieldName);
   });
 
   it('should call all registered providers with given parameters', () => {
@@ -61,9 +63,9 @@ describe('EditorConfigProvider', () => {
     expect(provider).not.toHaveBeenCalled();
     expect(provider2).not.toHaveBeenCalled();
     const aggConfig = {} as AggConfig;
-    registry.getConfigForAgg(indexPattern, aggConfig);
-    expect(provider).toHaveBeenCalledWith(indexPattern, aggConfig);
-    expect(provider2).toHaveBeenCalledWith(indexPattern, aggConfig);
+    registry.getConfigForAgg(indexPattern, aggTypeName, fieldName);
+    expect(provider).toHaveBeenCalledWith(indexPattern, aggTypeName, fieldName);
+    expect(provider2).toHaveBeenCalledWith(indexPattern, aggTypeName, fieldName);
   });
 
   describe('merging configs', () => {
@@ -72,7 +74,7 @@ describe('EditorConfigProvider', () => {
     }
 
     function getOutputConfig(reg: EditorConfigProviderRegistry) {
-      return reg.getConfigForAgg(indexPattern, {} as AggConfig).singleParam;
+      return reg.getConfigForAgg(indexPattern, aggTypeName, fieldName).singleParam;
     }
 
     it('should have hidden true if at least one config was hidden true', () => {

diff --git a/src/legacy/ui/public/vis/editors/config/editor_config_providers.ts b/src/legacy/ui/public/vis/editors/config/editor_config_providers.ts
@@ -18,14 +18,17 @@
  */
 
 import { TimeIntervalParam } from 'ui/vis/editors/config/types';
-import { AggConfig } from '../..';
 import { IndexPattern } from '../../../../../../plugins/data/public';
 import { leastCommonMultiple } from '../../lib/least_common_multiple';
 import { parseEsInterval } from '../../../../../core_plugins/data/public';
 import { leastCommonInterval } from '../../lib/least_common_interval';
 import { EditorConfig, EditorParamConfig, FixedParam, NumericIntervalParam } from './types';
 
-type EditorConfigProvider = (indexPattern: IndexPattern, aggConfig: AggConfig) => EditorConfig;
+type EditorConfigProvider = (
+  indexPattern: IndexPattern,
+  aggTypeName: string,
+  fieldName: string
+) => EditorConfig;
 
 class EditorConfigProviderRegistry {
   private providers: Set<EditorConfigProvider> = new Set();
@@ -34,8 +37,14 @@ class EditorConfigProviderRegistry {
     this.providers.add(configProvider);
   }
 
-  public getConfigForAgg(indexPattern: IndexPattern, aggConfig: AggConfig): EditorConfig {
-    const configs = Array.from(this.providers).map(provider => provider(indexPattern, aggConfig));
+  public getConfigForAgg(
+    indexPattern: IndexPattern,
+    aggTypeName: string,
+    fieldName: string
+  ): EditorConfig {
+    const configs = Array.from(this.providers).map(provider =>
+      provider(indexPattern, aggTypeName, fieldName)
+    );
     return this.mergeConfigs(configs);
   }
 

diff --git a/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx b/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx
@@ -90,10 +90,12 @@ function DefaultEditorAggParams({
       })
     : '';
 
-  const editorConfig = useMemo(() => editorConfigProviders.getConfigForAgg(indexPattern, agg), [
-    indexPattern,
-    agg,
-  ]);
+  const aggTypeName = agg.type?.name;
+  const fieldName = agg.params?.field?.name;
+  const editorConfig = useMemo(
+    () => editorConfigProviders.getConfigForAgg(indexPattern, aggTypeName, fieldName),
+    [indexPattern, aggTypeName, fieldName]
+  );
   const params = useMemo(() => getAggParamsToRender({ agg, editorConfig, metricAggs, state }), [
     agg,
     editorConfig,

diff --git a/x-pack/legacy/plugins/rollup/public/visualize/editor_config.js b/x-pack/legacy/plugins/rollup/public/visualize/editor_config.js
@@ -9,13 +9,11 @@ import { editorConfigProviders } from 'ui/vis/editors/config/editor_config_provi
 
 export function initEditorConfig() {
   // Limit agg params based on rollup capabilities
-  editorConfigProviders.register((indexPattern, aggConfig) => {
+  editorConfigProviders.register((indexPattern, aggTypeName, fieldName) => {
     if (indexPattern.type !== 'rollup') {
       return {};
     }
 
-    const aggTypeName = aggConfig.type && aggConfig.type.name;
-
     // Exclude certain param options for terms:
     // otherBucket, missingBucket, orderBy, orderAgg
     if (aggTypeName === 'terms') {
@@ -30,11 +28,10 @@ export function initEditorConfig() {
     }
 
     const rollupAggs = indexPattern.typeMeta && indexPattern.typeMeta.aggs;
-    const field = aggConfig.params && aggConfig.params.field && aggConfig.params.field.name;
     const fieldAgg =
-      rollupAggs && field && rollupAggs[aggTypeName] && rollupAggs[aggTypeName][field];
+      rollupAggs && fieldName && rollupAggs[aggTypeName] && rollupAggs[aggTypeName][fieldName];
 
-    if (!rollupAggs || !field || !fieldAgg) {
+    if (!rollupAggs || !fieldName || !fieldAgg) {
       return {};
     }