Skip to content

Commit

Permalink
changing the way aggconfig field filter works (#22756)
Browse files Browse the repository at this point in the history
  • Loading branch information
ppisljar authored Sep 11, 2018
1 parent 0ab1369 commit 6246c58
Show file tree
Hide file tree
Showing 34 changed files with 102 additions and 130 deletions.
2 changes: 1 addition & 1 deletion src/ui/public/agg_types/__tests__/agg_params.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
16 changes: 6 additions & 10 deletions src/ui/public/agg_types/__tests__/param_types/_field.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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();
});
Expand Down
3 changes: 1 addition & 2 deletions src/ui/public/agg_types/agg_params.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
});
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/buckets/date_histogram.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export const dateHistogramBucketAgg = new BucketAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'date',
default: function (agg) {
return agg.getIndexPattern().timeFieldName;
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/buckets/date_range.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const dateRangeBucketAgg = new BucketAggType({
},
params: [{
name: 'field',
type: 'field',
filterFieldTypes: 'date',
default: function (agg) {
return agg.getIndexPattern().timeFieldName;
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/buckets/geo_hash.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const geoHashBucketAgg = new BucketAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'geo_point'
},
{
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/buckets/histogram.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export const histogramBucketAgg = new BucketAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number'
},
{
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/buckets/ip_range.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const ipRangeBucketAgg = new BucketAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'ip'
}, {
name: 'ipRangeType',
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/buckets/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export const rangeBucketAgg = new BucketAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: ['number']
},
{
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/buckets/significant_terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const significantTermsBucketAgg = new BucketAggType({
params: [
{
name: 'field',
type: 'field',
scriptable: false,
filterFieldTypes: 'string'
},
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/buckets/terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export const termsBucketAgg = new BucketAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: ['number', 'boolean', 'date', 'ip', 'string']
},
{
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/avg.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const avgMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number'
}
]
Expand Down
3 changes: 2 additions & 1 deletion src/ui/public/agg_types/metrics/cardinality.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ export const cardinalityMetricAgg = new MetricAggType({
},
params: [
{
name: 'field'
name: 'field',
type: 'field'
}
]
});
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/geo_bounds.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const geoBoundsMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'geo_point'
}
]
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/geo_centroid.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const geoCentroidMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'geo_point'
}
],
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/max.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const maxMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number,date'
}
]
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/median.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const medianMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number'
},
{
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/min.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const minMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number,date'
}
]
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/percentile_ranks.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const percentileRanksMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number'
},
{
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/percentiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const percentilesMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number'
},
{
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/std_deviation.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const stdDeviationMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number'
}
],
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/agg_types/metrics/sum.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const sumMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
filterFieldTypes: 'number'
}
],
Expand Down
16 changes: 16 additions & 0 deletions src/ui/public/agg_types/metrics/top_hit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -40,6 +55,7 @@ export const topHitMetricAgg = new MetricAggType({
params: [
{
name: 'field',
type: 'field',
onlyAggregatable: false,
filterFieldTypes: '*',
write(agg, output) {
Expand Down
46 changes: 29 additions & 17 deletions src/ui/public/agg_types/param_types/field.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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.
*
Expand Down
17 changes: 9 additions & 8 deletions src/ui/public/agg_types/param_types/filter/field_filters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);
});
});
Loading

0 comments on commit 6246c58

Please sign in to comment.