changing the way aggconfig field filter works (elastic#22756) (elasti…

…c#22920)
ppisljar · Sep 12, 2018 · 89af63c · 89af63c
1 parent 3c9fcd5
commit 89af63c
Show file tree

Hide file tree

Showing 34 changed files with 102 additions and 130 deletions.
diff --git a/src/ui/public/agg_types/__tests__/agg_params.js b/src/ui/public/agg_types/__tests__/agg_params.js
@@ -43,7 +43,7 @@ describe('AggParams class', function () {
   describe('AggParam creation', function () {
     it('Uses the FieldParamType class for params with the name "field"', function () {
       const params = [
-        { name: 'field' }
+        { name: 'field', type: 'field' }
       ];
       const aggParams = new AggParams(params);
 

diff --git a/src/ui/public/agg_types/__tests__/param_types/_field.js b/src/ui/public/agg_types/__tests__/param_types/_field.js
@@ -37,22 +37,20 @@ describe('Field', function () {
   describe('constructor', function () {
     it('it is an instance of BaseParamType', function () {
       const aggParam = new FieldParamType({
-        name: 'field'
+        name: 'field', type: 'field'
       });
 
       expect(aggParam).to.be.a(BaseParamType);
     });
   });
 
-  describe('getFieldOptions', function () {
+  describe('getAvailableFields', function () {
     it('should return only aggregatable fields by default', function () {
       const aggParam = new FieldParamType({
-        name: 'field'
+        name: 'field', type: 'field'
       });
 
-      const fields = aggParam.getFieldOptions({
-        getIndexPattern: () => indexPattern
-      });
+      const fields = aggParam.getAvailableFields(indexPattern.fields);
       expect(fields).to.not.have.length(0);
       for (const field of fields) {
         expect(field.aggregatable).to.be(true);
@@ -61,14 +59,12 @@ describe('Field', function () {
 
     it('should return all fields if onlyAggregatable is false', function () {
       const aggParam = new FieldParamType({
-        name: 'field'
+        name: 'field', type: 'field'
       });
 
       aggParam.onlyAggregatable = false;
 
-      const fields = aggParam.getFieldOptions({
-        getIndexPattern: () => indexPattern
-      });
+      const fields = aggParam.getAvailableFields(indexPattern.fields);
       const nonAggregatableFields = reject(fields, 'aggregatable');
       expect(nonAggregatableFields).to.not.be.empty();
     });

diff --git a/src/ui/public/agg_types/agg_params.js b/src/ui/public/agg_types/agg_params.js
@@ -54,8 +54,7 @@ function AggParams(params) {
   AggParams.Super.call(this, {
     index: ['name'],
     initialSet: params.map(function (config) {
-      const type = config.name === 'field' ? config.name : config.type;
-      const Class = paramTypeMap[type] || paramTypeMap._default;
+      const Class = paramTypeMap[config.type] || paramTypeMap._default;
       return new Class(config);
     })
   });

diff --git a/src/ui/public/agg_types/buckets/date_histogram.js b/src/ui/public/agg_types/buckets/date_histogram.js
@@ -86,6 +86,7 @@ export const dateHistogramBucketAgg = new BucketAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'date',
       default: function (agg) {
         return agg.getIndexPattern().timeFieldName;

diff --git a/src/ui/public/agg_types/buckets/date_range.js b/src/ui/public/agg_types/buckets/date_range.js
@@ -41,6 +41,7 @@ export const dateRangeBucketAgg = new BucketAggType({
   },
   params: [{
     name: 'field',
+    type: 'field',
     filterFieldTypes: 'date',
     default: function (agg) {
       return agg.getIndexPattern().timeFieldName;

diff --git a/src/ui/public/agg_types/buckets/geo_hash.js b/src/ui/public/agg_types/buckets/geo_hash.js
@@ -72,6 +72,7 @@ export const geoHashBucketAgg = new BucketAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'geo_point'
     },
     {

diff --git a/src/ui/public/agg_types/buckets/histogram.js b/src/ui/public/agg_types/buckets/histogram.js
@@ -57,6 +57,7 @@ export const histogramBucketAgg = new BucketAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number'
     },
     {

diff --git a/src/ui/public/agg_types/buckets/ip_range.js b/src/ui/public/agg_types/buckets/ip_range.js
@@ -40,6 +40,7 @@ export const ipRangeBucketAgg = new BucketAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'ip'
     }, {
       name: 'ipRangeType',

diff --git a/src/ui/public/agg_types/buckets/range.js b/src/ui/public/agg_types/buckets/range.js
@@ -68,6 +68,7 @@ export const rangeBucketAgg = new BucketAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: ['number']
     },
     {

diff --git a/src/ui/public/agg_types/buckets/significant_terms.js b/src/ui/public/agg_types/buckets/significant_terms.js
@@ -31,6 +31,7 @@ export const significantTermsBucketAgg = new BucketAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       scriptable: false,
       filterFieldTypes: 'string'
     },

diff --git a/src/ui/public/agg_types/buckets/terms.js b/src/ui/public/agg_types/buckets/terms.js
@@ -127,6 +127,7 @@ export const termsBucketAgg = new BucketAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: ['number', 'boolean', 'date', 'ip',  'string']
     },
     {

diff --git a/src/ui/public/agg_types/metrics/avg.js b/src/ui/public/agg_types/metrics/avg.js
@@ -28,6 +28,7 @@ export const avgMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number'
     }
   ]

diff --git a/src/ui/public/agg_types/metrics/cardinality.js b/src/ui/public/agg_types/metrics/cardinality.js
@@ -31,7 +31,8 @@ export const cardinalityMetricAgg = new MetricAggType({
   },
   params: [
     {
-      name: 'field'
+      name: 'field',
+      type: 'field'
     }
   ]
 });
diff --git a/src/ui/public/agg_types/metrics/geo_bounds.js b/src/ui/public/agg_types/metrics/geo_bounds.js
@@ -28,6 +28,7 @@ export const geoBoundsMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'geo_point'
     }
   ]

diff --git a/src/ui/public/agg_types/metrics/geo_centroid.js b/src/ui/public/agg_types/metrics/geo_centroid.js
@@ -28,6 +28,7 @@ export const geoCentroidMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'geo_point'
     }
   ],

diff --git a/src/ui/public/agg_types/metrics/max.js b/src/ui/public/agg_types/metrics/max.js
@@ -28,6 +28,7 @@ export const maxMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number,date'
     }
   ]

diff --git a/src/ui/public/agg_types/metrics/median.js b/src/ui/public/agg_types/metrics/median.js
@@ -30,6 +30,7 @@ export const medianMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number'
     },
     {

diff --git a/src/ui/public/agg_types/metrics/min.js b/src/ui/public/agg_types/metrics/min.js
@@ -28,6 +28,7 @@ export const minMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number,date'
     }
   ]

diff --git a/src/ui/public/agg_types/metrics/percentile_ranks.js b/src/ui/public/agg_types/metrics/percentile_ranks.js
@@ -46,6 +46,7 @@ export const percentileRanksMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number'
     },
     {

diff --git a/src/ui/public/agg_types/metrics/percentiles.js b/src/ui/public/agg_types/metrics/percentiles.js
@@ -40,6 +40,7 @@ export const percentilesMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number'
     },
     {

diff --git a/src/ui/public/agg_types/metrics/std_deviation.js b/src/ui/public/agg_types/metrics/std_deviation.js
@@ -56,6 +56,7 @@ export const stdDeviationMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number'
     }
   ],

diff --git a/src/ui/public/agg_types/metrics/sum.js b/src/ui/public/agg_types/metrics/sum.js
@@ -28,6 +28,7 @@ export const sumMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       filterFieldTypes: 'number'
     }
   ],

diff --git a/src/ui/public/agg_types/metrics/top_hit.js b/src/ui/public/agg_types/metrics/top_hit.js
@@ -21,11 +21,26 @@ import _ from 'lodash';
 import { MetricAggType } from './metric_agg_type';
 import topSortEditor from '../controls/top_sort.html';
 import aggregateAndSizeEditor from '../controls/top_aggregate_and_size.html';
+import { aggTypeFieldFilters } from '../param_types/filter';
 
 const isNumber = function (type) {
   return type === 'number';
 };
 
+aggTypeFieldFilters.addFilter(
+  (
+    field,
+    fieldParamType,
+    aggConfig,
+    vis
+  ) => {
+    if (aggConfig.type.name !== 'top_hit' || vis.type.name === 'table' || vis.type.name === 'metric') {
+      return true;
+    }
+    return field.type === 'number';
+
+  });
+
 export const topHitMetricAgg = new MetricAggType({
   name: 'top_hits',
   title: 'Top Hit',
@@ -40,6 +55,7 @@ export const topHitMetricAgg = new MetricAggType({
   params: [
     {
       name: 'field',
+      type: 'field',
       onlyAggregatable: false,
       filterFieldTypes: '*',
       write(agg, output) {

diff --git a/src/ui/public/agg_types/param_types/field.js b/src/ui/public/agg_types/param_types/field.js
@@ -25,7 +25,9 @@ import '../../filters/field_type';
 import { IndexedArray } from '../../indexed_array';
 import { toastNotifications } from '../../notify';
 import { createLegacyClass } from '../../utils/legacy_class';
-import { aggTypeFieldFilters } from './filter';
+import { propFilter } from '../../filters/_prop_filter';
+
+const filterByType = propFilter('type');
 
 export function FieldParamType(config) {
   FieldParamType.Super.call(this, config);
@@ -49,21 +51,6 @@ FieldParamType.prototype.serialize = function (field) {
   return field.name;
 };
 
-/**
- * Get the options for this field from the indexPattern
- */
-FieldParamType.prototype.getFieldOptions = function (aggConfig) {
-  const indexPattern = aggConfig.getIndexPattern();
-  const fields = aggTypeFieldFilters
-    .filter(indexPattern.fields.raw, this, indexPattern, aggConfig);
-
-  return new IndexedArray({
-    index: ['name'],
-    group: ['type'],
-    initialSet: sortBy(fields, ['type', 'name']),
-  });
-};
-
 /**
  * Called to read values from a database record into the
  * aggConfig object
@@ -78,14 +65,39 @@ FieldParamType.prototype.deserialize = function (fieldName, aggConfig) {
     throw new SavedObjectNotFound('index-pattern-field', fieldName);
   }
 
-  const validField = this.getFieldOptions(aggConfig).byName[fieldName];
+  const validField = this.getAvailableFields(aggConfig.getIndexPattern().fields).byName[fieldName];
   if (!validField) {
     toastNotifications.addDanger(`Saved "field" parameter is now invalid. Please select a new field.`);
   }
 
   return validField;
 };
 
+/**
+ * filter the fields to the available ones
+ */
+FieldParamType.prototype.getAvailableFields = function (fields) {
+  const filteredFields = fields.filter(field => {
+    const { onlyAggregatable, scriptable, filterFieldTypes } = this;
+
+    if ((onlyAggregatable && !field.aggregatable) || (!scriptable && field.scripted)) {
+      return false;
+    }
+
+    if (!filterFieldTypes) {
+      return true;
+    }
+
+    return filterByType([field], filterFieldTypes).length !== 0;
+  });
+
+  return new IndexedArray({
+    index: ['name'],
+    group: ['type'],
+    initialSet: sortBy(filteredFields, ['type', 'name']),
+  });
+};
+
 /**
  * Write the aggregation parameter.
  *

diff --git a/src/ui/public/agg_types/param_types/filter/field_filters.test.ts b/src/ui/public/agg_types/param_types/filter/field_filters.test.ts
@@ -17,45 +17,46 @@
  * under the License.
  */
 
+import { Vis } from 'ui/vis';
 import { AggTypeFieldFilters } from './field_filters';
 
 describe('AggTypeFieldFilters', () => {
   let registry: AggTypeFieldFilters;
   const fieldParamType = {};
-  const indexPattern = {};
   const aggConfig = {};
+  const vis = {} as Vis;
 
   beforeEach(() => {
     registry = new AggTypeFieldFilters();
   });
 
   it('should filter nothing without registered filters', async () => {
     const fields = [{ name: 'foo' }, { name: 'bar' }];
-    const filtered = registry.filter(fields, fieldParamType, indexPattern, aggConfig);
+    const filtered = registry.filter(fields, fieldParamType, aggConfig, vis);
     expect(filtered).toEqual(fields);
   });
 
   it('should pass all fields to the registered filter', async () => {
     const fields = [{ name: 'foo' }, { name: 'bar' }];
     const filter = jest.fn();
     registry.addFilter(filter);
-    registry.filter(fields, fieldParamType, indexPattern, aggConfig);
-    expect(filter).toHaveBeenCalledWith(fields[0], fieldParamType, indexPattern, aggConfig);
-    expect(filter).toHaveBeenCalledWith(fields[1], fieldParamType, indexPattern, aggConfig);
+    registry.filter(fields, fieldParamType, aggConfig, vis);
+    expect(filter).toHaveBeenCalledWith(fields[0], fieldParamType, aggConfig, vis);
+    expect(filter).toHaveBeenCalledWith(fields[1], fieldParamType, aggConfig, vis);
   });
 
   it('should allow registered filters to filter out fields', async () => {
     const fields = [{ name: 'foo' }, { name: 'bar' }];
-    let filtered = registry.filter(fields, fieldParamType, indexPattern, aggConfig);
+    let filtered = registry.filter(fields, fieldParamType, aggConfig, vis);
     expect(filtered).toEqual(fields);
 
     registry.addFilter(() => true);
     registry.addFilter(field => field.name !== 'foo');
-    filtered = registry.filter(fields, fieldParamType, indexPattern, aggConfig);
+    filtered = registry.filter(fields, fieldParamType, aggConfig, vis);
     expect(filtered).toEqual([fields[1]]);
 
     registry.addFilter(field => field.name !== 'bar');
-    filtered = registry.filter(fields, fieldParamType, indexPattern, aggConfig);
+    filtered = registry.filter(fields, fieldParamType, aggConfig, vis);
     expect(filtered).toEqual([]);
   });
 });