From 465d56faea68df7ea0ae8ef489dda0649c35f2c6 Mon Sep 17 00:00:00 2001 From: Luke Elmers Date: Mon, 17 Dec 2018 08:39:49 -0700 Subject: [PATCH] Fix broken filtering on "other" bucket in inspector. (#26794) --- .../inspector_views/public/data/data_table.js | 2 + .../inspector/build_tabular_inspector_data.js | 42 +++++++----- src/ui/public/vis/vis_filters.js | 29 ++++++-- test/functional/apps/visualize/_inspector.js | 68 ++++++++++++++++++- .../functional/page_objects/visualize_page.js | 22 ++++++ 5 files changed, 140 insertions(+), 23 deletions(-) diff --git a/src/legacy/core_plugins/inspector_views/public/data/data_table.js b/src/legacy/core_plugins/inspector_views/public/data/data_table.js index 2280171a0f01..6d72cc5a1e81 100644 --- a/src/legacy/core_plugins/inspector_views/public/data/data_table.js +++ b/src/legacy/core_plugins/inspector_views/public/data/data_table.js @@ -58,6 +58,7 @@ class DataTableFormat extends Component { iconType="plusInCircle" color="text" aria-label="Filter for value" + data-test-subj="filterForInspectorCellValue" className="insDataTableFormat__filter" onClick={() => col.filter(value)} /> @@ -73,6 +74,7 @@ class DataTableFormat extends Component { iconType="minusInCircle" color="text" aria-label="Filter out value" + data-test-subj="filterOutInspectorCellValue" className="insDataTableFormat__filter" onClick={() => col.filterOut(value)} /> diff --git a/src/ui/public/inspector/build_tabular_inspector_data.js b/src/ui/public/inspector/build_tabular_inspector_data.js index 83d9acd72f82..9164551ce636 100644 --- a/src/ui/public/inspector/build_tabular_inspector_data.js +++ b/src/ui/public/inspector/build_tabular_inspector_data.js @@ -17,6 +17,8 @@ * under the License. */ +import { set } from 'lodash'; +import { createFilter } from '../vis/vis_filters'; import { FormattedData } from './adapters/data'; /** @@ -24,34 +26,42 @@ import { FormattedData } from './adapters/data'; * inspector. It will only be called when the data view in the inspector is opened. */ export async function buildTabularInspectorData(table, queryFilter) { - const columns = table.columns.map((col, index) => { + const rows = table.rows.map(row => { + return table.columns.reduce((prev, cur, colIndex) => { + const value = row[cur.id]; + const fieldFormatter = cur.aggConfig.fieldFormatter('text'); + prev[`col-${colIndex}-${cur.aggConfig.id}`] = new FormattedData(value, fieldFormatter(value)); + return prev; + }, {}); + }); + + const columns = table.columns.map((col, colIndex) => { const field = col.aggConfig.getField(); const isCellContentFilterable = col.aggConfig.isFilterable() && (!field || field.filterable); return ({ name: col.name, - field: `col${index}`, - filter: isCellContentFilterable && ((value) => { - const filter = col.aggConfig.createFilter(value.raw); + field: `col-${colIndex}-${col.aggConfig.id}`, + filter: isCellContentFilterable && (value => { + const rowIndex = rows.findIndex(row => row[`col-${colIndex}-${col.aggConfig.id}`].raw === value.raw); + const filter = createFilter(table, colIndex, rowIndex, value.raw); queryFilter.addFilters(filter); }), - filterOut: isCellContentFilterable && ((value) => { - const filter = col.aggConfig.createFilter(value.raw); - filter.meta = filter.meta || {}; - filter.meta.negate = true; + filterOut: isCellContentFilterable && (value => { + const rowIndex = rows.findIndex(row => row[`col-${colIndex}-${col.aggConfig.id}`].raw === value.raw); + const filter = createFilter(table, colIndex, rowIndex, value.raw); + const notOther = value.raw !== '__other__'; + const notMissing = value.raw !== '__missing__'; + if (Array.isArray(filter)) { + filter.forEach(f => set(f, 'meta.negate', (notOther && notMissing))); + } else { + set(filter, 'meta.negate', (notOther && notMissing)); + } queryFilter.addFilters(filter); }), }); }); - const rows = table.rows.map(row => { - return table.columns.reduce((prev, cur, index) => { - const value = row[cur.id]; - const fieldFormatter = cur.aggConfig.fieldFormatter('text'); - prev[`col${index}`] = new FormattedData(value, fieldFormatter(value)); - return prev; - }, {}); - }); return { columns, rows }; } diff --git a/src/ui/public/vis/vis_filters.js b/src/ui/public/vis/vis_filters.js index 86ef72f51cbd..3085788c49c1 100644 --- a/src/ui/public/vis/vis_filters.js +++ b/src/ui/public/vis/vis_filters.js @@ -22,7 +22,16 @@ import { FilterBarPushFiltersProvider } from '../filter_bar/push_filters'; import { FilterBarQueryFilterProvider } from '../filter_bar/query_filter'; import { onBrushEvent } from '../utils/brush_event'; -const getTerms = (table, columnIndex, rowIndex) => { +/** + * For terms aggregations on `__other__` buckets, this assembles a list of applicable filter + * terms based on a specific cell in the tabified data. + * + * @param {object} table - tabified table data + * @param {number} columnIndex - current column index + * @param {number} rowIndex - current row index + * @return {array} - array of terms to filter against + */ +const getOtherBucketFilterTerms = (table, columnIndex, rowIndex) => { if (rowIndex === -1) { return []; } @@ -42,15 +51,25 @@ const getTerms = (table, columnIndex, rowIndex) => { }))]; }; -const createFilter = (data, columnIndex, rowIndex, cellValue) => { - const { aggConfig, id: columnId } = data.columns[columnIndex]; +/** + * Assembles the filters needed to apply filtering against a specific cell value, while accounting + * for cases like if the value is a terms agg in an `__other__` or `__missing__` bucket. + * + * @param {object} table - tabified table data + * @param {number} columnIndex - current column index + * @param {number} rowIndex - current row index + * @param {string} cellValue - value of the current cell + * @return {array|string} - filter or list of filters to provide to queryFilter.addFilters() + */ +const createFilter = (table, columnIndex, rowIndex, cellValue) => { + const { aggConfig, id: columnId } = table.columns[columnIndex]; let filter = []; - const value = rowIndex > -1 ? data.rows[rowIndex][columnId] : cellValue; + const value = rowIndex > -1 ? table.rows[rowIndex][columnId] : cellValue; if (value === null || value === undefined) { return; } if (aggConfig.type.name === 'terms' && aggConfig.params.otherBucket) { - const terms = getTerms(data, columnIndex, rowIndex); + const terms = getOtherBucketFilterTerms(table, columnIndex, rowIndex); filter = aggConfig.createFilter(value, { terms }); } else { filter = aggConfig.createFilter(value); diff --git a/test/functional/apps/visualize/_inspector.js b/test/functional/apps/visualize/_inspector.js index 7ddf7d817471..ca9281a8246e 100644 --- a/test/functional/apps/visualize/_inspector.js +++ b/test/functional/apps/visualize/_inspector.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']); describe('visualize app', function describeIndexTests() { @@ -39,9 +40,7 @@ export default function ({ getService, getPageObjects }) { }); describe('inspector table', function indexPatternCreation() { - it('should update table header when columns change', async function () { - await PageObjects.visualize.openInspector(); let headers = await PageObjects.visualize.getInspectorTableHeaders(); expect(headers).to.eql(['Count']); @@ -57,6 +56,71 @@ export default function ({ getService, getPageObjects }) { headers = await PageObjects.visualize.getInspectorTableHeaders(); expect(headers).to.eql(['Count', 'Average machine.ram']); }); + + describe('filtering on inspector table values', function () { + before(async function () { + log.debug('Add X-Axis terms agg on machine.os.raw'); + await PageObjects.visualize.clickBucket('X-Axis'); + await PageObjects.visualize.selectAggregation('Terms'); + await PageObjects.visualize.selectField('machine.os.raw'); + await PageObjects.visualize.setSize(2); + await PageObjects.visualize.toggleOtherBucket(); + await PageObjects.visualize.clickGo(); + }); + + beforeEach(async function () { + await PageObjects.visualize.openInspector(); + }); + + afterEach(async function () { + await PageObjects.visualize.closeInspector(); + await filterBar.removeFilter('machine.os.raw'); + }); + + it('should allow filtering for values', async function () { + let data = await PageObjects.visualize.getInspectorTableData(); + expect(data).to.eql([ + ['win 8', '2,904', '13,031,579,645.108'], + ['win xp', '2,858', '13,073,190,186.423'], + ['Other', '6,920', '13,123,599,766.011'], + ]); + + await PageObjects.visualize.filterForInspectorTableCell(1, 1); + data = await PageObjects.visualize.getInspectorTableData(); + expect(data).to.eql([ + ['win 8', '2,904', '13,031,579,645.108'], + ]); + }); + + it('should allow filtering out values', async function () { + await PageObjects.visualize.filterOutInspectorTableCell(1, 1); + const data = await PageObjects.visualize.getInspectorTableData(); + expect(data).to.eql([ + ['win xp', '2,858', '13,073,190,186.423'], + ['win 7', '2,814', '13,186,695,551.251'], + ['Other', '4,106', '13,080,420,659.354'], + ]); + }); + + it('should allow filtering for other values', async function () { + await PageObjects.visualize.filterForInspectorTableCell(1, 3); + const data = await PageObjects.visualize.getInspectorTableData(); + expect(data).to.eql([ + ['win 7', '2,814', '13,186,695,551.251'], + ['ios', '2,784', '13,009,497,206.823'], + ['Other', '1,322', '13,228,964,670.613'], + ]); + }); + + it('should allow filtering out other values', async function () { + await PageObjects.visualize.filterOutInspectorTableCell(1, 3); + const data = await PageObjects.visualize.getInspectorTableData(); + expect(data).to.eql([ + ['win 8', '2,904', '13,031,579,645.108'], + ['win xp', '2,858', '13,073,190,186.423'], + ]); + }); + }); }); }); } diff --git a/test/functional/page_objects/visualize_page.js b/test/functional/page_objects/visualize_page.js index 1d21a96e6f99..f777e1d7922d 100644 --- a/test/functional/page_objects/visualize_page.js +++ b/test/functional/page_objects/visualize_page.js @@ -1138,6 +1138,28 @@ export function VisualizePageProvider({ getService, getPageObjects }) { }); } + async filterForInspectorTableCell(column, row) { + await retry.try(async () => { + const table = await testSubjects.find('inspectorTable'); + const cell = await table.findByCssSelector(`tbody tr:nth-child(${row}) td:nth-child(${column})`); + await browser.moveMouseTo(cell); + const filterBtn = await testSubjects.findDescendant('filterForInspectorCellValue', cell); + await filterBtn.click(); + }); + await renderable.waitForRender(); + } + + async filterOutInspectorTableCell(column, row) { + await retry.try(async () => { + const table = await testSubjects.find('inspectorTable'); + const cell = await table.findByCssSelector(`tbody tr:nth-child(${row}) td:nth-child(${column})`); + await browser.moveMouseTo(cell); + const filterBtn = await testSubjects.findDescendant('filterOutInspectorCellValue', cell); + await filterBtn.click(); + }); + await renderable.waitForRender(); + } + async toggleLegend(show = true) { await retry.try(async () => { const isVisible = find.byCssSelector('vislib-legend');