Skip to content

Commit

Permalink
Fix bug in propFilter logic when it's not passed any filters. (#20569) (
Browse files Browse the repository at this point in the history
#20606)

* Fix bug in propFilter logic when it's not passed any filters. Reduce lodash dependency.
* Default filters to empty array and add tests.
  • Loading branch information
cjcenizal authored Jul 10, 2018
1 parent b5e80c3 commit 510915d
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 11 deletions.
16 changes: 16 additions & 0 deletions src/ui/public/filters/__tests__/prop_filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ describe('prop filter', function () {
return objects;
}

it('returns list when no filters are provided', function () {
const objects = getObjects('table', 'table', 'pie');
expect(nameFilter(objects)).to.eql(objects);
});

it('returns list when empty list of filters is provided', function () {
const objects = getObjects('table', 'table', 'pie');
expect(nameFilter(objects, [])).to.eql(objects);
});

it('should keep only the tables', function () {
const objects = getObjects('table', 'table', 'pie');
expect(nameFilter(objects, 'table')).to.eql(getObjects('table', 'table'));
Expand Down Expand Up @@ -74,4 +84,10 @@ describe('prop filter', function () {
const line = (value) => value === 'line';
expect(nameFilter(objects, line)).to.eql(getObjects('line'));
});

it('gracefully handles a filter function with zero arity', function () {
const objects = getObjects('table', 'line', 'pie');
const rejectEverything = () => false;
expect(nameFilter(objects, rejectEverything)).to.eql([]);
});
});
33 changes: 22 additions & 11 deletions src/ui/public/filters/_prop_filter.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/

import _ from 'lodash';
import { isFunction } from 'lodash';

/**
* Filters out a list by a given filter. This is currently used to implement:
Expand All @@ -37,15 +37,22 @@ export function propFilter(prop) {
* - Can be also an array, a single value as a string, or a comma-separated list of items
* @return {array} - the filtered list
*/
return function (list, filters) {
if (!filters) return filters;

if (_.isFunction(filters)) {
return function (list, filters = []) {
if (isFunction(filters)) {
return list.filter((item) => filters(item[prop]));
}

if (!Array.isArray(filters)) filters = filters.split(',');
if (_.contains(filters, '*')) return list;
if (!Array.isArray(filters)) {
filters = filters.split(',');
}

if (filters.length === 0) {
return list;
}

if (filters.includes('*')) {
return list;
}

const options = filters.reduce(function (options, filter) {
let type = 'include';
Expand All @@ -64,11 +71,15 @@ export function propFilter(prop) {
return list.filter(function (item) {
const value = item[prop];

const excluded = options.exclude && _.contains(options.exclude, value);
if (excluded) return false;
const excluded = options.exclude && options.exclude.includes(value);
if (excluded) {
return false;
}

const included = !options.include || _.contains(options.include, value);
if (included) return true;
const included = !options.include || options.include.includes(value);
if (included) {
return true;
}

return false;
});
Expand Down

0 comments on commit 510915d

Please sign in to comment.