From ba12a6a42afbcad3b03af705384e41b2f0a8c026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 2 Nov 2020 07:26:58 +0100 Subject: [PATCH] TableFilters: Fixes filtering with field overrides (#28690) * TableFilters: Fixes filtering with field overrides * Refactor: changes after PR comments --- .../src/components/Table/utils.test.ts | 158 +++++++++++++++--- .../grafana-ui/src/components/Table/utils.ts | 56 ++++--- 2 files changed, 171 insertions(+), 43 deletions(-) diff --git a/packages/grafana-ui/src/components/Table/utils.test.ts b/packages/grafana-ui/src/components/Table/utils.test.ts index 8885f32e412..0653a8498c5 100644 --- a/packages/grafana-ui/src/components/Table/utils.test.ts +++ b/packages/grafana-ui/src/components/Table/utils.test.ts @@ -5,6 +5,7 @@ import { getColumns, getFilteredOptions, getTextAlign, + rowToFieldValue, sortOptions, valuesToOptions, } from './utils'; @@ -71,28 +72,88 @@ describe('Table utils', () => { }); describe('filterByValue', () => { - it.each` - rows | id | filterValues | expected - ${[]} | ${'0'} | ${[{ value: 'a' }]} | ${[]} - ${[{ values: { 0: 'a' } }]} | ${'0'} | ${null} | ${[{ values: { 0: 'a' } }]} - ${[{ values: { 0: 'a' } }]} | ${'0'} | ${undefined} | ${[{ values: { 0: 'a' } }]} - ${[{ values: { 0: 'a' } }]} | ${'1'} | ${[{ value: 'b' }]} | ${[]} - ${[{ values: { 0: 'a' } }]} | ${'0'} | ${[{ value: 'a' }]} | ${[{ values: { 0: 'a' } }]} - ${[{ values: { 0: 'a' } }, { values: { 1: 'a' } }]} | ${'0'} | ${[{ value: 'a' }]} | ${[{ values: { 0: 'a' } }]} - ${[{ values: { 0: 'a' } }, { values: { 0: 'b' } }, { values: { 0: 'c' } }]} | ${'0'} | ${[{ value: 'a' }, { value: 'b' }]} | ${[{ values: { 0: 'a' } }, { values: { 0: 'b' } }]} - `( - "when called with rows: '$rows.toString()', id: '$id' and filterValues: '$filterValues' then result should be '$expected'", - ({ rows, id, filterValues, expected }) => { - expect(filterByValue(rows, id, filterValues)).toEqual(expected); - } - ); + describe('happy path', () => { + const field: any = { values: new ArrayVector(['a', 'aa', 'ab', 'b', 'ba', 'bb', 'c']) }; + const rows: any = [ + { index: 0, values: { 0: 'a' } }, + { index: 1, values: { 0: 'aa' } }, + { index: 2, values: { 0: 'ab' } }, + { index: 3, values: { 0: 'b' } }, + { index: 4, values: { 0: 'ba' } }, + { index: 5, values: { 0: 'bb' } }, + { index: 6, values: { 0: 'c' } }, + ]; + const filterValues = [{ value: 'a' }, { value: 'b' }, { value: 'c' }]; + + const result = filterByValue(field)(rows, '0', filterValues); + + expect(result).toEqual([ + { index: 0, values: { 0: 'a' } }, + { index: 3, values: { 0: 'b' } }, + { index: 6, values: { 0: 'c' } }, + ]); + }); + + describe('fast exit cases', () => { + describe('no rows', () => { + it('should return empty array', () => { + const field: any = { values: new ArrayVector(['a']) }; + const rows: any = []; + const filterValues = [{ value: 'a' }]; + + const result = filterByValue(field)(rows, '', filterValues); + + expect(result).toEqual([]); + }); + }); + + describe('no filterValues', () => { + it('should return rows', () => { + const field: any = { values: new ArrayVector(['a']) }; + const rows: any = [{}]; + const filterValues = undefined; + + const result = filterByValue(field)(rows, '', filterValues); + + expect(result).toEqual([{}]); + }); + }); + + describe('no field', () => { + it('should return rows', () => { + const field = undefined; + const rows: any = [{}]; + const filterValues = [{ value: 'a' }]; + + const result = filterByValue(field)(rows, '', filterValues); + + expect(result).toEqual([{}]); + }); + }); + + describe('missing id in values', () => { + it('should return rows', () => { + const field: any = { values: new ArrayVector(['a', 'b', 'c']) }; + const rows: any = [ + { index: 0, values: { 0: 'a' } }, + { index: 1, values: { 0: 'b' } }, + { index: 2, values: { 0: 'c' } }, + ]; + const filterValues = [{ value: 'a' }, { value: 'b' }, { value: 'c' }]; + + const result = filterByValue(field)(rows, '1', filterValues); + + expect(result).toEqual([]); + }); + }); + }); }); describe('calculateUniqueFieldValues', () => { describe('when called without field', () => { it('then it should return an empty object', () => { const field = undefined; - const rows = [{ id: 0 }]; + const rows = [{ index: 0 }]; const result = calculateUniqueFieldValues(rows, field); @@ -142,15 +203,15 @@ describe('Table utils', () => { text: `${value}.0`, })), }; - const rows: any[] = [{ id: 0 }, { id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }]; + const rows: any[] = [{ index: 0 }, { index: 1 }, { index: 2 }, { index: 3 }, { index: 4 }]; const result = calculateUniqueFieldValues(rows, field); expect(field.display).toHaveBeenCalledTimes(5); expect(result).toEqual({ - '1.0': 1, - '2.0': 2, - '3.0': 3, + '1.0': '1.0', + '2.0': '2.0', + '3.0': '3.0', }); }); }); @@ -163,7 +224,7 @@ describe('Table utils', () => { name: 'value', type: FieldType.number, }; - const rows: any[] = [{ id: 0 }, { id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }]; + const rows: any[] = [{ index: 0 }, { index: 1 }, { index: 2 }, { index: 3 }, { index: 4 }]; const result = calculateUniqueFieldValues(rows, field); @@ -182,7 +243,7 @@ describe('Table utils', () => { name: 'value', type: FieldType.number, }; - const rows: any[] = [{ id: 0 }, { id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }]; + const rows: any[] = [{ index: 0 }, { index: 1 }, { index: 2 }, { index: 3 }, { index: 4 }]; const result = calculateUniqueFieldValues(rows, field); @@ -196,6 +257,59 @@ describe('Table utils', () => { }); }); + describe('rowToFieldValue', () => { + describe('happy paths', () => { + describe('field without field display', () => { + const field: any = { values: new ArrayVector(['a', 'b', 'c']) }; + const row = { index: 1 }; + + const result = rowToFieldValue(row, field); + + expect(result).toEqual('b'); + }); + + describe('field with display processor', () => { + const field: Field = { + config: {}, + values: new ArrayVector([1, 2, 2, 1, 3, 5, 6]), + name: 'value', + type: FieldType.number, + display: jest.fn((value: any) => ({ + numeric: 1, + percent: 0.01, + color: '', + title: `${value}.0`, + text: `${value}.0`, + })), + }; + const row = { index: 4 }; + + const result = rowToFieldValue(row, field); + + expect(result).toEqual('3.0'); + }); + }); + + describe('quick exist paths', () => { + describe('field is missing', () => { + const field = undefined; + const row = { index: 0 }; + + const result = rowToFieldValue(row, field); + + expect(result).toEqual(''); + }); + describe('row is missing', () => { + const field: any = { values: new ArrayVector(['a', 'b', 'c']) }; + const row = undefined; + + const result = rowToFieldValue(row, field); + + expect(result).toEqual(''); + }); + }); + }); + describe('valuesToOptions', () => { describe('when called with a record object', () => { it('then it should return sorted options from that object', () => { diff --git a/packages/grafana-ui/src/components/Table/utils.ts b/packages/grafana-ui/src/components/Table/utils.ts index bf9686fcefa..63da460c9fa 100644 --- a/packages/grafana-ui/src/components/Table/utils.ts +++ b/packages/grafana-ui/src/components/Table/utils.ts @@ -78,7 +78,7 @@ export function getColumns(data: DataFrame, availableWidth: number, columnMinWid sortType: selectSortType(field.type), width: fieldTableOptions.width, minWidth: 50, - filter: memoizeOne(filterByValue), + filter: memoizeOne(filterByValue(field)), justifyContent: getTextAlign(field), }); } @@ -116,23 +116,28 @@ function getCellComponent(displayMode: TableCellDisplayMode, field: Field) { return DefaultCell; } -export function filterByValue(rows: Row[], id: string, filterValues?: SelectableValue[]) { - if (rows.length === 0) { - return rows; - } - - if (!filterValues) { - return rows; - } - - return rows.filter(row => { - if (!row.values.hasOwnProperty(id)) { - return false; +export function filterByValue(field?: Field) { + return function(rows: Row[], id: string, filterValues?: SelectableValue[]) { + if (rows.length === 0) { + return rows; } - const value = row.values[id]; - return filterValues.find(filter => filter.value === value) !== undefined; - }); + if (!filterValues) { + return rows; + } + + if (!field) { + return rows; + } + + return rows.filter(row => { + if (!row.values.hasOwnProperty(id)) { + return false; + } + const value = rowToFieldValue(row, field); + return filterValues.find(filter => filter.value === value) !== undefined; + }); + }; } export function calculateUniqueFieldValues(rows: any[], field?: Field) { @@ -143,16 +148,25 @@ export function calculateUniqueFieldValues(rows: any[], field?: Field) { const set: Record = {}; for (let index = 0; index < rows.length; index++) { - const fieldIndex = parseInt(rows[index].id, 10); - const fieldValue = field.values.get(fieldIndex); - const displayValue = field.display ? field.display(fieldValue) : fieldValue; - const value = field.display ? formattedValueToString(displayValue) : displayValue; - set[value || '(Blanks)'] = fieldValue; + const value = rowToFieldValue(rows[index], field); + set[value || '(Blanks)'] = value; } return set; } +export function rowToFieldValue(row: any, field?: Field): string { + if (!field || !row) { + return ''; + } + + const fieldValue = field.values.get(row.index); + const displayValue = field.display ? field.display(fieldValue) : fieldValue; + const value = field.display ? formattedValueToString(displayValue) : displayValue; + + return value; +} + export function valuesToOptions(unique: Record): SelectableValue[] { return Object.keys(unique) .reduce((all, key) => all.concat({ value: unique[key], label: key }), [] as SelectableValue[])