From baba296a5e01a33eb3297428ba7e13250e4f4e0d Mon Sep 17 00:00:00 2001 From: ppisljar Date: Thu, 18 Oct 2018 17:36:33 +0200 Subject: [PATCH 1/6] fixing other bucket filters --- .../buckets/_terms_other_bucket_helper.js | 6 ++-- src/ui/public/filter_bar/push_filters.js | 29 +++++++++++++++++++ src/ui/public/vis/response_handlers/legacy.js | 16 ++++++++-- src/ui/public/vis/vis.js | 22 ++++++++++---- 4 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 src/ui/public/filter_bar/push_filters.js diff --git a/src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js b/src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js index 2840da808b752..86927148ea4bf 100644 --- a/src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js +++ b/src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js @@ -112,6 +112,8 @@ const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => { const filterAgg = aggConfigs.createAggConfig({ type: 'filters', id: 'other', + }, { + addToAggConfigs: false, }); // nest all the child aggregations of aggWithOtherBucket @@ -129,8 +131,8 @@ const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => { if (aggIndex < index) { _.each(agg.buckets, (bucket, bucketObjKey) => { const bucketKey = currentAgg.getKey(bucket, Number.isInteger(bucketObjKey) ? null : bucketObjKey); - const filter = _.cloneDeep(bucket.filter) || currentAgg.createFilter(bucketKey); - const newFilters = [...filters, filter]; + const filter = _.cloneDeep(bucket.filters) || currentAgg.createFilter(bucketKey); + const newFilters = [...filters, filter].flat(); walkBucketTree(newAggIndex, bucket, newAgg.id, newFilters, `${key}-${bucketKey.toString()}`); }); return; diff --git a/src/ui/public/filter_bar/push_filters.js b/src/ui/public/filter_bar/push_filters.js new file mode 100644 index 0000000000000..cce206c922b7b --- /dev/null +++ b/src/ui/public/filter_bar/push_filters.js @@ -0,0 +1,29 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import _ from 'lodash'; + +export function FilterBarPushFiltersProvider() { + return function ($state) { + if (!_.isObject($state)) throw new Error('pushFilters requires a state object'); + return function (filters) { + $state.$newFilters = filters; + }; + }; +} diff --git a/src/ui/public/vis/response_handlers/legacy.js b/src/ui/public/vis/response_handlers/legacy.js index 7f5c130dc254a..e9f06aceaa4b4 100644 --- a/src/ui/public/vis/response_handlers/legacy.js +++ b/src/ui/public/vis/response_handlers/legacy.js @@ -43,7 +43,7 @@ const LegacyResponseHandlerProvider = function () { const splitMap = {}; let splitIndex = 0; - table.rows.forEach(row => { + table.rows.forEach((row, rowIndex) => { const splitValue = row[splitColumn.id]; const splitColumnIndex = table.columns.findIndex(column => column === splitColumn); @@ -74,6 +74,11 @@ const LegacyResponseHandlerProvider = function () { const newRow = _.map(converted.tables[tableIndex].tables[0].columns, column => { const value = row[column.id]; const aggConfigResult = new AggConfigResult(column.aggConfig, previousSplitAgg, value, value); + aggConfigResult.rawData = { + table: table, + columnIndex: table.columns.findIndex(c => c.id === column.id), + rowIndex: rowIndex, + }; if (column.aggConfig.type.type === 'buckets') { previousSplitAgg = aggConfigResult; } @@ -86,11 +91,16 @@ const LegacyResponseHandlerProvider = function () { converted.tables.push({ columns: table.columns.map(column => ({ title: column.name, ...column })), - rows: table.rows.map(row => { + rows: table.rows.map((row, rowIndex) => { let previousSplitAgg; - return table.columns.map(column => { + return table.columns.map((column, columnIndex) => { const value = row[column.id]; const aggConfigResult = new AggConfigResult(column.aggConfig, previousSplitAgg, value, value); + aggConfigResult.rawData = { + table: table, + column: columnIndex, + row: rowIndex, + }; if (column.aggConfig.type.type === 'buckets') { previousSplitAgg = aggConfigResult; } diff --git a/src/ui/public/vis/vis.js b/src/ui/public/vis/vis.js index 85d59bf4ccd57..9a34f36d5ffad 100644 --- a/src/ui/public/vis/vis.js +++ b/src/ui/public/vis/vis.js @@ -34,7 +34,7 @@ import { AggConfigs } from './agg_configs'; import { PersistedState } from '../persisted_state'; import { onBrushEvent } from '../utils/brush_event'; import { FilterBarQueryFilterProvider } from '../filter_bar/query_filter'; -import { FilterBarClickHandlerProvider } from '../filter_bar/filter_bar_click_handler'; +import { FilterBarPushFiltersProvider } from '../filter_bar/push_filters'; import { updateVisualizationConfig } from './vis_update'; import { SearchSourceProvider } from '../courier/search_source'; import { SavedObjectsClientProvider } from '../saved_objects'; @@ -54,7 +54,7 @@ const getTerms = (table, columnIndex, rowIndex) => { return row[column.id] === table.rows[rowIndex][column.id] || i >= columnIndex; }); }); - const terms = rows.map(row => row[columnIndex]); + const terms = rows.map(row => row[table.columns[columnIndex].id]); return [...new Set(terms.filter(term => { const notOther = term !== '__other__'; @@ -66,9 +66,9 @@ const getTerms = (table, columnIndex, rowIndex) => { export function VisProvider(Private, indexPatterns, getAppState) { const visTypes = Private(VisTypesRegistryProvider); const queryFilter = Private(FilterBarQueryFilterProvider); - const filterBarClickHandler = Private(FilterBarClickHandlerProvider); const SearchSource = Private(SearchSourceProvider); const savedObjectsClient = Private(SavedObjectsClientProvider); + const filterBarPushFilters = Private(FilterBarPushFiltersProvider); class Vis extends EventEmitter { constructor(indexPattern, visState) { @@ -99,10 +99,18 @@ export function VisProvider(Private, indexPatterns, getAppState) { // the filter method will be removed in the near feature // you should rather use addFilter method below filter: (event) => { + let data = event.datum.aggConfigResult; + const filters = []; + while (data.$parent) { + const { key, rawData } = data.$parent; + const { table, column, row } = rawData; + filters.push(this.API.events.createFilter(table, column, row, key)); + data = data.$parent; + } const appState = getAppState(); - filterBarClickHandler(appState)(event); + filterBarPushFilters(appState)(filters.flat()); }, - addFilter: (data, columnIndex, rowIndex, cellValue) => { + createFilter: (data, columnIndex, rowIndex, cellValue) => { const { aggConfig, id: columnId } = data.columns[columnIndex]; let filter = []; const value = rowIndex > -1 ? data.rows[rowIndex][columnId] : cellValue; @@ -115,6 +123,10 @@ export function VisProvider(Private, indexPatterns, getAppState) { } else { filter = aggConfig.createFilter(value); } + return filter; + }, + addFilter: (data, columnIndex, rowIndex, cellValue) => { + const filter = this.API.events.createFilter(data, columnIndex, rowIndex, cellValue); queryFilter.addFilters(filter); }, brush: (event) => { onBrushEvent(event, getAppState()); From dbb4485f6a7730c673777fb40033a819ba01b1d1 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Fri, 19 Oct 2018 07:17:05 +0200 Subject: [PATCH 2/6] adding selenium tests --- src/ui/public/vis/vis.js | 2 +- test/functional/apps/visualize/_pie_chart.js | 79 +++++++++++++------ .../functional/page_objects/visualize_page.js | 11 ++- 3 files changed, 67 insertions(+), 25 deletions(-) diff --git a/src/ui/public/vis/vis.js b/src/ui/public/vis/vis.js index 9a34f36d5ffad..5c2f97f02a237 100644 --- a/src/ui/public/vis/vis.js +++ b/src/ui/public/vis/vis.js @@ -114,7 +114,7 @@ export function VisProvider(Private, indexPatterns, getAppState) { const { aggConfig, id: columnId } = data.columns[columnIndex]; let filter = []; const value = rowIndex > -1 ? data.rows[rowIndex][columnId] : cellValue; - if (!value) { + if (value == null) { return; } if (aggConfig.type.name === 'terms' && aggConfig.params.otherBucket) { diff --git a/test/functional/apps/visualize/_pie_chart.js b/test/functional/apps/visualize/_pie_chart.js index a91ee5e5ee440..85e63532307c4 100644 --- a/test/functional/apps/visualize/_pie_chart.js +++ b/test/functional/apps/visualize/_pie_chart.js @@ -89,29 +89,64 @@ export default function ({ getService, getPageObjects }) { expect(data).to.eql(expectedTableData); }); - it('should show other and missing bucket', async function () { - const expectedTableData = [ 'win 8', 'win xp', 'win 7', 'ios', 'Missing', 'Other' ]; + describe('other bucket', () => { + it('should show other and missing bucket', async function () { + const expectedTableData = [ 'win 8', 'win xp', 'win 7', 'ios', 'Missing', 'Other' ]; + + await PageObjects.visualize.navigateToNewVisualization(); + log.debug('clickPieChart'); + await PageObjects.visualize.clickPieChart(); + await PageObjects.visualize.clickNewSearch(); + log.debug('Set absolute time range from \"' + fromTime + '\" to \"' + toTime + '\"'); + await PageObjects.header.setAbsoluteRange(fromTime, toTime); + log.debug('select bucket Split Slices'); + await PageObjects.visualize.clickBucket('Split Slices'); + log.debug('Click aggregation Terms'); + await PageObjects.visualize.selectAggregation('Terms'); + log.debug('Click field machine.os.raw'); + await PageObjects.visualize.selectField('machine.os.raw'); + await PageObjects.visualize.toggleOtherBucket(); + await PageObjects.visualize.toggleMissingBucket(); + log.debug('clickGo'); + await PageObjects.visualize.clickGo(); + await PageObjects.common.sleep(1003); + const pieData = await PageObjects.visualize.getPieChartLabels(); + log.debug('pieData.length = ' + pieData.length); + expect(pieData).to.eql(expectedTableData); + }); - await PageObjects.visualize.navigateToNewVisualization(); - log.debug('clickPieChart'); - await PageObjects.visualize.clickPieChart(); - await PageObjects.visualize.clickNewSearch(); - log.debug('Set absolute time range from \"' + fromTime + '\" to \"' + toTime + '\"'); - await PageObjects.header.setAbsoluteRange(fromTime, toTime); - log.debug('select bucket Split Slices'); - await PageObjects.visualize.clickBucket('Split Slices'); - log.debug('Click aggregation Histogram'); - await PageObjects.visualize.selectAggregation('Terms'); - log.debug('Click field memory'); - await PageObjects.visualize.selectField('machine.os.raw'); - await PageObjects.visualize.toggleOtherBucket(); - await PageObjects.visualize.toggleMissingBucket(); - log.debug('clickGo'); - await PageObjects.visualize.clickGo(); - await PageObjects.common.sleep(1003); - const pieData = await PageObjects.visualize.getPieChartLabels(); - log.debug('pieData.length = ' + pieData.length); - expect(pieData).to.eql(expectedTableData); + it.skip('should apply correct filter on other bucket', async () => { + const expectedTableData = [ 'win 8', 'win xp', 'win 7', 'ios', 'Missing', 'Other' ]; + + const pieSlice = await PageObjects.visualize.filterPieSlice('Other'); + await pieSlice.click(); + await PageObjects.header.waitUntilLoadingHasFinished(); + const pieData = await PageObjects.visualize.getPieChartLabels(); + log.debug('pieData.length = ' + pieData.length); + expect(pieData).to.eql(expectedTableData); + }); + + it('should show two levels of other buckets', async () => { + const expectedTableData = [ 'win 8', 'CN', 'IN', 'US', 'ID', 'BR', 'Other', 'win xp', + 'CN', 'IN', 'US', 'ID', 'BR', 'Other', 'win 7', 'CN', 'IN', 'US', 'ID', 'BR', 'Other', + 'ios', 'IN', 'CN', 'US', 'ID', 'BR', 'Other', 'Missing', 'CN', 'IN', 'US', 'BR', 'PK', + 'Other', 'Other', 'IN', 'CN', 'US', 'ID', 'BR', 'Other' ]; + + await PageObjects.visualize.toggleOpenEditor(2, 'false'); + await PageObjects.visualize.clickAddBucket(); + await PageObjects.visualize.clickBucket('Split Slices'); + await PageObjects.visualize.selectAggregation('Terms'); + log.debug('Click field geo.src'); + await PageObjects.visualize.selectField('geo.src'); + await PageObjects.visualize.toggleOtherBucket(); + await PageObjects.visualize.toggleMissingBucket(); + log.debug('clickGo'); + await PageObjects.visualize.clickGo(); + await PageObjects.header.waitUntilLoadingHasFinished(); + const pieData = await PageObjects.visualize.getPieChartLabels(); + log.debug('pieData.length = ' + pieData.length); + expect(pieData).to.eql(expectedTableData); + }); }); describe('disabled aggs', () => { diff --git a/test/functional/page_objects/visualize_page.js b/test/functional/page_objects/visualize_page.js index 5526bb89d06c0..7d733d1dfb730 100644 --- a/test/functional/page_objects/visualize_page.js +++ b/test/functional/page_objects/visualize_page.js @@ -586,11 +586,11 @@ export function VisualizePageProvider({ getService, getPageObjects }) { } async toggleOtherBucket() { - return await find.clickByCssSelector('input[name="showOther"]'); + return await find.clickByCssSelector('vis-editor-agg-params:not(.ng-hide) input[name="showOther"]'); } async toggleMissingBucket() { - return await find.clickByCssSelector('input[name="showMissing"]'); + return await find.clickByCssSelector('vis-editor-agg-params:not(.ng-hide) input[name="showMissing"]'); } async isApplyEnabled() { @@ -1114,6 +1114,13 @@ export function VisualizePageProvider({ getService, getPageObjects }) { return await bucketType.click(); } + async filterPieSlice(name) { + const slice = await this.getPieSlice(name); + const event = document.createEvent('SVGEvents'); + event.initEvent('click', true, true); + slice.dispatchEvent(event); + } + async getPieSlice(name) { return await testSubjects.find(`pieSlice-${name.split(' ').join('-')}`); } From eb84bff48c78b0ff0cfe88412f6a47adc11cf886 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Sun, 21 Oct 2018 13:06:16 +0200 Subject: [PATCH 3/6] using lodash flatten --- src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js b/src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js index 86927148ea4bf..7e2d4070189d4 100644 --- a/src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js +++ b/src/ui/public/agg_types/buckets/_terms_other_bucket_helper.js @@ -132,7 +132,7 @@ const buildOtherBucketAgg = (aggConfigs, aggWithOtherBucket, response) => { _.each(agg.buckets, (bucket, bucketObjKey) => { const bucketKey = currentAgg.getKey(bucket, Number.isInteger(bucketObjKey) ? null : bucketObjKey); const filter = _.cloneDeep(bucket.filters) || currentAgg.createFilter(bucketKey); - const newFilters = [...filters, filter].flat(); + const newFilters = _.flatten([...filters, filter]); walkBucketTree(newAggIndex, bucket, newAgg.id, newFilters, `${key}-${bucketKey.toString()}`); }); return; From f9ea47c7f42a8adf093548da9582afa30a1a6b64 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Mon, 22 Oct 2018 15:33:52 +0200 Subject: [PATCH 4/6] using lodash flatten --- src/ui/public/vis/vis.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/vis/vis.js b/src/ui/public/vis/vis.js index 5c2f97f02a237..e3f3df6cb7c6d 100644 --- a/src/ui/public/vis/vis.js +++ b/src/ui/public/vis/vis.js @@ -108,7 +108,7 @@ export function VisProvider(Private, indexPatterns, getAppState) { data = data.$parent; } const appState = getAppState(); - filterBarPushFilters(appState)(filters.flat()); + filterBarPushFilters(appState)(_.flatten(filters)); }, createFilter: (data, columnIndex, rowIndex, cellValue) => { const { aggConfig, id: columnId } = data.columns[columnIndex]; From 4aad715b4a5381d37fdeadb56e271484d498950d Mon Sep 17 00:00:00 2001 From: ppisljar Date: Mon, 22 Oct 2018 15:43:22 +0200 Subject: [PATCH 5/6] fixing based on tims review --- src/ui/public/vis/vis.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/vis/vis.js b/src/ui/public/vis/vis.js index e3f3df6cb7c6d..2c8b11d99ef97 100644 --- a/src/ui/public/vis/vis.js +++ b/src/ui/public/vis/vis.js @@ -114,7 +114,7 @@ export function VisProvider(Private, indexPatterns, getAppState) { const { aggConfig, id: columnId } = data.columns[columnIndex]; let filter = []; const value = rowIndex > -1 ? data.rows[rowIndex][columnId] : cellValue; - if (value == null) { + if (value === null || value === undefined) { return; } if (aggConfig.type.name === 'terms' && aggConfig.params.otherBucket) { From 6f3197291d20b8fa957998f8aa5306139c333f5b Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 23 Oct 2018 10:43:34 +0200 Subject: [PATCH 6/6] Fix functional tests --- test/functional/apps/visualize/_pie_chart.js | 21 ++++++++++--------- .../functional/page_objects/visualize_page.js | 6 +++--- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/test/functional/apps/visualize/_pie_chart.js b/test/functional/apps/visualize/_pie_chart.js index 85e63532307c4..e79dc9e8c690d 100644 --- a/test/functional/apps/visualize/_pie_chart.js +++ b/test/functional/apps/visualize/_pie_chart.js @@ -21,6 +21,7 @@ import expect from 'expect.js'; export default function ({ getService, getPageObjects }) { const log = getService('log'); + const filterBar = getService('filterBar'); const PageObjects = getPageObjects(['common', 'visualize', 'header', 'settings']); const fromTime = '2015-09-19 06:31:44.000'; const toTime = '2015-09-23 18:31:44.000'; @@ -91,13 +92,13 @@ export default function ({ getService, getPageObjects }) { describe('other bucket', () => { it('should show other and missing bucket', async function () { - const expectedTableData = [ 'win 8', 'win xp', 'win 7', 'ios', 'Missing', 'Other' ]; + const expectedTableData = [ 'win 8', 'win xp', 'win 7', 'ios', 'Missing', 'Other' ]; await PageObjects.visualize.navigateToNewVisualization(); log.debug('clickPieChart'); await PageObjects.visualize.clickPieChart(); await PageObjects.visualize.clickNewSearch(); - log.debug('Set absolute time range from \"' + fromTime + '\" to \"' + toTime + '\"'); + log.debug(`Set absolute time range from "${fromTime}" to "${toTime}"`); await PageObjects.header.setAbsoluteRange(fromTime, toTime); log.debug('select bucket Split Slices'); await PageObjects.visualize.clickBucket('Split Slices'); @@ -111,23 +112,23 @@ export default function ({ getService, getPageObjects }) { await PageObjects.visualize.clickGo(); await PageObjects.common.sleep(1003); const pieData = await PageObjects.visualize.getPieChartLabels(); - log.debug('pieData.length = ' + pieData.length); + log.debug(`pieData.length = ${pieData.length}`); expect(pieData).to.eql(expectedTableData); }); - it.skip('should apply correct filter on other bucket', async () => { - const expectedTableData = [ 'win 8', 'win xp', 'win 7', 'ios', 'Missing', 'Other' ]; + it('should apply correct filter on other bucket', async () => { + const expectedTableData = [ 'Missing', 'osx' ]; - const pieSlice = await PageObjects.visualize.filterPieSlice('Other'); - await pieSlice.click(); + await PageObjects.visualize.filterPieSlice('Other'); await PageObjects.header.waitUntilLoadingHasFinished(); const pieData = await PageObjects.visualize.getPieChartLabels(); - log.debug('pieData.length = ' + pieData.length); + log.debug(`pieData.length = ${pieData.length}`); expect(pieData).to.eql(expectedTableData); + await filterBar.removeFilter('machine.os.raw'); }); it('should show two levels of other buckets', async () => { - const expectedTableData = [ 'win 8', 'CN', 'IN', 'US', 'ID', 'BR', 'Other', 'win xp', + const expectedTableData = [ 'win 8', 'CN', 'IN', 'US', 'ID', 'BR', 'Other', 'win xp', 'CN', 'IN', 'US', 'ID', 'BR', 'Other', 'win 7', 'CN', 'IN', 'US', 'ID', 'BR', 'Other', 'ios', 'IN', 'CN', 'US', 'ID', 'BR', 'Other', 'Missing', 'CN', 'IN', 'US', 'BR', 'PK', 'Other', 'Other', 'IN', 'CN', 'US', 'ID', 'BR', 'Other' ]; @@ -144,7 +145,7 @@ export default function ({ getService, getPageObjects }) { await PageObjects.visualize.clickGo(); await PageObjects.header.waitUntilLoadingHasFinished(); const pieData = await PageObjects.visualize.getPieChartLabels(); - log.debug('pieData.length = ' + pieData.length); + log.debug(`pieData.length = ${pieData.length}`); expect(pieData).to.eql(expectedTableData); }); }); diff --git a/test/functional/page_objects/visualize_page.js b/test/functional/page_objects/visualize_page.js index 7d733d1dfb730..be5c930bb3e19 100644 --- a/test/functional/page_objects/visualize_page.js +++ b/test/functional/page_objects/visualize_page.js @@ -1116,9 +1116,9 @@ export function VisualizePageProvider({ getService, getPageObjects }) { async filterPieSlice(name) { const slice = await this.getPieSlice(name); - const event = document.createEvent('SVGEvents'); - event.initEvent('click', true, true); - slice.dispatchEvent(event); + // Since slice is an SVG element we can't simply use .click() for it + await remote.moveMouseTo(slice); + await remote.clickMouseButton(); } async getPieSlice(name) {