Skip to content

Commit

Permalink
fixes filtering on other bucket outside of vislib (#19860)
Browse files Browse the repository at this point in the history
  • Loading branch information
ppisljar authored Jun 20, 2018
1 parent 89e7266 commit 53e83b5
Show file tree
Hide file tree
Showing 13 changed files with 165 additions and 42 deletions.
17 changes: 10 additions & 7 deletions src/core_plugins/metric_vis/public/metric_vis_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,25 +98,25 @@ export class MetricVisComponent extends Component {
const labels = this._getLabels();
const metrics = [];

tableGroups.tables.forEach((table) => {
tableGroups.tables.forEach((table, tableIndex) => {
let bucketAgg;
let rowHeaderIndex;

table.columns.forEach((column, i) => {
table.columns.forEach((column, columnIndex) => {
const aggConfig = column.aggConfig;

if (aggConfig && aggConfig.schema.group === 'buckets') {
bucketAgg = aggConfig;
// Store the current index, so we later know in which position in the
// row array, the bucket agg key will be, so we can create filters on it.
rowHeaderIndex = i;
rowHeaderIndex = columnIndex;
return;
}

table.rows.forEach(row => {
table.rows.forEach((row, rowIndex) => {

let title = column.title;
let value = row[i];
let value = row[columnIndex];
const color = this._getColor(value, labels, colors);

if (isPercentageMode) {
Expand All @@ -143,6 +143,9 @@ export class MetricVisComponent extends Component {
bgColor: shouldColor && config.style.bgColor ? color : null,
lightText: shouldColor && config.style.bgColor && this._needsLightText(color),
filterKey: rowHeaderIndex !== undefined ? row[rowHeaderIndex] : null,
tableIndex: tableIndex,
rowIndex: rowIndex,
columnIndex: rowHeaderIndex,
bucketAgg: bucketAgg,
});
});
Expand All @@ -156,8 +159,8 @@ export class MetricVisComponent extends Component {
if (!metric.filterKey || !metric.bucketAgg) {
return;
}
const filter = metric.bucketAgg.createFilter(metric.filterKey);
this.props.vis.API.queryFilter.addFilters(filter);
const table = this.props.visData.tables[metric.tableIndex];
this.props.vis.API.events.addFilter(table, metric.columnIndex, metric.rowIndex);
};

_renderMetric = (metric, index) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,8 @@ export function RegionMapsVisualizationProvider(Private, Notifier, config) {
return;
}

const agg = this._vis.aggs.bySchemaName.segment[0];
const filter = agg.createFilter(event);
this._vis.API.queryFilter.addFilters(filter);
const rowIndex = this._chartData.tables[0].rows.findIndex(row => row[0] === event);
this._vis.API.events.addFilter(this._chartData.tables[0], 0, rowIndex);
});
this._choroplethLayer.on('styleChanged', (event) => {
const shouldShowWarning = this._vis.params.isDisplayWarning && config.get('visualization:regionmap:showWarnings');
Expand Down
2 changes: 1 addition & 1 deletion src/core_plugins/tagcloud/public/tag_cloud.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class TagCloud extends EventEmitter {
const self = this;
enteringTags.on({
click: function (event) {
self.emit('select', event.rawText);
self.emit('select', event);
},
mouseover: function () {
d3.select(this).style('cursor', 'pointer');
Expand Down
13 changes: 9 additions & 4 deletions src/core_plugins/tagcloud/public/tag_cloud_visualization.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,9 @@ export class TagCloudVisualization {
if (!this._bucketAgg) {
return;
}
const filter = this._bucketAgg.createFilter(event);
this._vis.API.queryFilter.addFilters(filter);
this._vis.API.events.addFilter(
event.meta.data, 0, event.meta.rowIndex
);
});
this._renderComplete$ = Rx.fromEvent(this._tagCloud, 'renderComplete');

Expand Down Expand Up @@ -120,12 +121,16 @@ export class TagCloudVisualization {
this._bucketAgg = null;
}

const tags = data.rows.map(row => {
const tags = data.rows.map((row, rowIndex) => {
const [tag, count] = row;
return {
displayText: this._bucketAgg ? this._bucketAgg.fieldFormatter()(tag) : tag,
rawText: tag,
value: count
value: count,
meta: {
data: data,
rowIndex: rowIndex,
}
};
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,7 @@ describe('Terms Agg Other bucket helper', () => {
vis.aggs[0],
otherAggConfig);

expect(mergedResponse.aggregations['1'].buckets[3].key).to.equal('Other');
expect(mergedResponse.aggregations['1'].buckets[3].filters.length).to.equal(2);
expect(mergedResponse.aggregations['1'].buckets[3].key).to.equal('__other__');
});

it('correctly merges other bucket with nested terms agg', () => {
Expand All @@ -255,8 +254,7 @@ describe('Terms Agg Other bucket helper', () => {
const mergedResponse = mergeOtherBucketAggResponse(vis.aggs, nestedTermResponse,
nestedOtherResponse, vis.aggs[1], otherAggConfig);

expect(mergedResponse.aggregations['1'].buckets[1]['2'].buckets[3].key).to.equal('Other');
expect(mergedResponse.aggregations['1'].buckets[1]['2'].buckets[3].filters.length).to.equal(2);
expect(mergedResponse.aggregations['1'].buckets[1]['2'].buckets[3].key).to.equal('__other__');
});

});
Expand All @@ -265,17 +263,8 @@ describe('Terms Agg Other bucket helper', () => {
it('correctly updates missing bucket key', () => {
init(visConfigNestedTerm);
const updatedResponse = updateMissingBucket(singleTermResponse, vis.aggs, vis.aggs[0]);
expect(updatedResponse.aggregations['1'].buckets.find(bucket => bucket.key === 'Missing')).to.not.be('undefined');
expect(updatedResponse.aggregations['1'].buckets.find(bucket => bucket.key === '__missing__')).to.not.be('undefined');
});

it('correctly sets the bucket filter', () => {
const updatedResponse = updateMissingBucket(singleTermResponse, vis.aggs, vis.aggs[0]);
const missingBucket = updatedResponse.aggregations['1'].buckets.find(bucket => bucket.key === 'Missing');
expect(missingBucket.filters).to.not.be('undefined');
expect(missingBucket.filters[0]).to.eql({
meta: { index: 'logstash-*', negate: true },
exists: { field: 'geo.src' }
});
});
});
});
31 changes: 31 additions & 0 deletions src/ui/public/agg_types/__tests__/buckets/create_filter/terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,36 @@ describe('AggConfig Filters', function () {
expect(filterTrue.query.match).to.have.property('ssl');
expect(filterTrue.query.match.ssl).to.have.property('query', true);
});

it('should generate correct __missing__ filter', () => {
const vis = new Vis(indexPattern, {
type: 'histogram',
aggs: [ { type: 'terms', schema: 'segment', params: { field: '_type' } } ]
});
const aggConfig = vis.aggs.byTypeName.terms[0];
const filter = createFilterTerms(aggConfig, '__missing__');
expect(filter).to.have.property('exists');
expect(filter.exists).to.have.property('field', '_type');
expect(filter).to.have.property('meta');
expect(filter.meta).to.have.property('index', indexPattern.id);
expect(filter.meta).to.have.property('negate', true);
});

it('should generate correct __other__ filter', () => {
const vis = new Vis(indexPattern, {
type: 'histogram',
aggs: [ { type: 'terms', schema: 'segment', params: { field: '_type' } } ]
});
const aggConfig = vis.aggs.byTypeName.terms[0];
const filter = createFilterTerms(aggConfig, '__other__', { terms: ['apache'] })[0];
expect(filter).to.have.property('query');
expect(filter.query).to.have.property('bool');
expect(filter.query.bool).to.have.property('should');
expect(filter.query.bool.should[0]).to.have.property('match_phrase');
expect(filter.query.bool.should[0].match_phrase).to.have.property('_type', 'apache');
expect(filter).to.have.property('meta');
expect(filter.meta).to.have.property('index', indexPattern.id);
expect(filter.meta).to.have.property('negate', true);
});
});
});
30 changes: 30 additions & 0 deletions src/ui/public/agg_types/__tests__/buckets/terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,36 @@ describe('Terms Agg', function () {
expect($rootScope.agg.params.orderBy).to.be('_key');
});

describe('custom field formatter', () => {
beforeEach(() => {
init({
responseValueAggs: [
{
id: 'agg1',
type: {
name: 'count'
}
}
],
aggParams: {
otherBucketLabel: 'Other',
missingBucketLabel: 'Missing'
}
});
$rootScope.$digest();
});

it ('converts __other__ key', () => {
const formatter = $rootScope.agg.type.getFormat($rootScope.agg).getConverterFor('text');
expect(formatter('__other__')).to.be('Other');
});

it ('converts __missing__ key', () => {
const formatter = $rootScope.agg.type.getFormat($rootScope.agg).getConverterFor('text');
expect(formatter('__missing__')).to.be('Missing');
});
});

it('adds "custom metric" option');
it('lists all metric agg responses');
it('lists individual values of a multi-value metric');
Expand Down
17 changes: 9 additions & 8 deletions src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@

import _ from 'lodash';
import { AggConfig } from '../../vis/agg_config';
import { buildPhrasesFilter } from '../../filter_manager/lib/phrases';
import { buildExistsFilter } from '../../filter_manager/lib/exists';
import { buildPhrasesFilter } from '../../filter_manager/lib/phrases';
import { buildQueryFromFilters } from '../../courier/data_source/build_query/from_filters';

/**
Expand Down Expand Up @@ -102,6 +102,7 @@ const getOtherAggTerms = (requestAgg, key, otherAgg) => {
);
};


const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => {
const bucketAggs = aggConfigs.filter(agg => agg.type.type === 'buckets');
const index = bucketAggs.findIndex(agg => agg.id === aggWithOtherBucket.id);
Expand Down Expand Up @@ -175,12 +176,11 @@ const mergeOtherBucketAggResponse = (aggsConfig, response, otherResponse, otherA
const phraseFilter = buildPhrasesFilter(otherAgg.params.field, requestFilterTerms, otherAgg.params.field.indexPattern);
phraseFilter.meta.negate = true;
bucket.filters = [ phraseFilter ];
bucket.key = otherAgg.params.otherBucketLabel;
bucket.key = '__other__';

if (aggResultBuckets.some(bucket => bucket.key === '__missing__')) {
bucket.filters.push(buildExistsFilter(otherAgg.params.field, otherAgg.params.field.indexPattern));
}

aggResultBuckets.push(bucket);
});
return updatedResponse;
Expand All @@ -190,12 +190,13 @@ const updateMissingBucket = (response, aggConfigs, agg) => {
const updatedResponse = _.cloneDeep(response);
const aggResultBuckets = getAggConfigResultMissingBuckets(updatedResponse.aggregations, agg.id);
aggResultBuckets.forEach(bucket => {
bucket.key = agg.params.missingBucketLabel;
const existsFilter = buildExistsFilter(agg.params.field, agg.params.field.indexPattern);
existsFilter.meta.negate = true;
bucket.filters = [ existsFilter ];
bucket.key = '__missing__';
});
return updatedResponse;
};

export { buildOtherBucketAgg, mergeOtherBucketAggResponse, updateMissingBucket };
export {
buildOtherBucketAgg,
mergeOtherBucketAggResponse,
updateMissingBucket,
};
27 changes: 25 additions & 2 deletions src/ui/public/agg_types/buckets/create_filter/terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,30 @@
*/

import { buildPhraseFilter } from '../../../filter_manager/lib/phrase';
import { buildPhrasesFilter } from '../../../filter_manager/lib/phrases';
import { buildExistsFilter } from '../../../filter_manager/lib/exists';

export function createFilterTerms(aggConfig, key) {
return buildPhraseFilter(aggConfig.params.field, key, aggConfig.vis.indexPattern);
export function createFilterTerms(aggConfig, key, params) {
const field = aggConfig.params.field;
const indexPattern = field.indexPattern;

if (key === '__other__') {
const terms = params.terms;

const phraseFilter = buildPhrasesFilter(field, terms, indexPattern);
phraseFilter.meta.negate = true;

const filters = [phraseFilter];

if (terms.some(term => term === '__missing__')) {
filters.push(buildExistsFilter(field, indexPattern));
}

return filters;
} else if (key === '__missing__') {
const existsFilter = buildExistsFilter(field, indexPattern);
existsFilter.meta.negate = true;
return existsFilter;
}
return buildPhraseFilter(field, key, indexPattern);
}
16 changes: 16 additions & 0 deletions src/ui/public/agg_types/buckets/terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@ export const termsBucketAgg = new BucketAggType({
const params = agg.params;
return agg.getFieldDisplayName() + ': ' + params.order.display;
},
getFormat: function (bucket) {
return {
getConverterFor: (type) => {
return (val) => {
if (val === '__other__') {
return bucket.params.otherBucketLabel;
}
if (val === '__missing__') {
return bucket.params.missingBucketLabel;
}
const converter = bucket.params.field.format.getConverterFor(type);
return converter(val);
};
}
};
},
createFilter: createFilterTerms,
postFlightRequest: async (resp, aggConfigs, aggConfig, nestedSearchSource) => {
if (aggConfig.params.otherBucket) {
Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/vis/__tests__/_agg_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ describe('AggConfig', function () {
type: 'table',
aggs: [
{
type: 'terms',
type: 'histogram',
schema: 'bucket',
params: { field: 'bytes' }
}
Expand Down
4 changes: 2 additions & 2 deletions src/ui/public/vis/agg_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ AggConfig.prototype.isFilterable = function () {
return _.isFunction(this.type.createFilter);
};

AggConfig.prototype.createFilter = function (key) {
AggConfig.prototype.createFilter = function (key, params = {}) {
if (!this.isFilterable()) {
throw new TypeError('The "' + this.type.title + '" aggregation does not support filtering.');
}
Expand All @@ -197,7 +197,7 @@ AggConfig.prototype.createFilter = function (key) {
throw new TypeError(message);
}

return this.type.createFilter(this, key);
return this.type.createFilter(this, key, params);
};

/**
Expand Down
26 changes: 26 additions & 0 deletions src/ui/public/vis/vis.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ import { queryManagerFactory } from '../query_manager';
import { SearchSourceProvider } from '../courier/data_source/search_source';
import { SavedObjectsClientProvider } from '../saved_objects';

const getTerms = (table, columnIndex, rowIndex) => {
// get only rows where cell value matches current row for all the fields before columnIndex
const rows = table.rows.filter(row => row.every((cell, i) => cell === table.rows[rowIndex][i] || i >= columnIndex));
const terms = rows.map(row => row[columnIndex]);

return [...new Set(terms.filter(term => {
const notOther = term !== '__other__';
const notMissing = term !== '__missing__';
return notOther && notMissing;
}))];
};

export function VisProvider(Private, Promise, indexPatterns, timefilter, getAppState) {
const visTypes = Private(VisTypesRegistryProvider);
const brushEvent = Private(UtilsBrushEventProvider);
Expand Down Expand Up @@ -75,9 +87,23 @@ export function VisProvider(Private, Promise, indexPatterns, timefilter, getAppS
queryFilter: queryFilter,
queryManager: queryManagerFactory(getAppState),
events: {
// the filter method will be removed in the near feature
// you should rather use addFilter method below
filter: (event) => {
const appState = getAppState();
filterBarClickHandler(appState)(event);
},
addFilter: (data, columnIndex, rowIndex) => {
const agg = data.columns[columnIndex].aggConfig;
let filter = [];
const value = data.rows[rowIndex][columnIndex];
if (agg.type.name === 'terms' && agg.params.otherBucket) {
const terms = getTerms(data, columnIndex, rowIndex);
filter = agg.createFilter(value, { terms });
} else {
filter = agg.createFilter(value);
}
queryFilter.addFilters(filter);
}, brush: (event) => {
const appState = getAppState();
brushEvent(appState)(event);
Expand Down

0 comments on commit 53e83b5

Please sign in to comment.