Skip to content

Commit

Permalink
feat(filters): add option to filter empty values for select filter (#310
Browse files Browse the repository at this point in the history
)

* feat(filters): add option to filter empty values for select filter
  • Loading branch information
ghiscoding committed Apr 14, 2021
1 parent 2f60c79 commit c58a92a
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 24 deletions.
Expand Up @@ -72,5 +72,6 @@ describe('SelectFilter', () => {
expect(spyGetHeaderRow).toHaveBeenCalled();
expect(filterCount).toBe(1);
expect(filter.isMultipleSelect).toBe(true);
expect(filter.columnDef.filter.emptySearchTermReturnAllValues).toBeFalse();
});
});
Expand Up @@ -74,6 +74,7 @@ describe('SelectFilter', () => {
expect(spyGetHeaderRow).toHaveBeenCalled();
expect(filterCount).toBe(1);
expect(filter.isMultipleSelect).toBe(false);
expect(filter.columnDef.filter.emptySearchTermReturnAllValues).toBeUndefined();
});

it('should create the select filter with empty search term when passed an empty string as a filter argument and not expect "filled" css class either', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/filters/multipleSelectFilter.ts
Expand Up @@ -7,7 +7,7 @@ export class MultipleSelectFilter extends SelectFilter {
/**
* Initialize the Filter
*/
constructor(protected readonly translaterService: TranslaterService, protected readonly collectionService: CollectionService, protected readonly rxjs: RxJsFacade) {
constructor(protected readonly translaterService: TranslaterService, protected readonly collectionService: CollectionService, protected readonly rxjs?: RxJsFacade) {
super(translaterService, collectionService, rxjs, true);
}
}
7 changes: 7 additions & 0 deletions packages/common/src/filters/selectFilter.ts
Expand Up @@ -143,6 +143,13 @@ export class SelectFilter implements Filter {
}
this.defaultOptions.placeholder = placeholder || '';

// when we're using a multiple-select filter and we have an empty select option,
// we probably want this value to be a valid filter option that will ONLY return value that are empty (not everything like its default behavior)
// user can still override it by defining it
if (this._isMultipleSelect && this.columnDef?.filter) {
this.columnDef.filter.emptySearchTermReturnAllValues = this.columnDef.filter?.emptySearchTermReturnAllValues ?? false;
}

// always render the Select (dropdown) DOM element,
// if that is the case, the Select will simply be without any options but we still have to render it (else SlickGrid would throw an error)
const newCollection = this.columnFilter.collection || [];
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/filters/singleSelectFilter.ts
Expand Up @@ -7,7 +7,7 @@ export class SingleSelectFilter extends SelectFilter {
/**
* Initialize the Filter
*/
constructor(protected readonly translaterService: TranslaterService, protected readonly collectionService: CollectionService, protected readonly rxjs: RxJsFacade) {
constructor(protected readonly translaterService: TranslaterService, protected readonly collectionService: CollectionService, protected readonly rxjs?: RxJsFacade) {
super(translaterService, collectionService, rxjs, false);
}
}
11 changes: 11 additions & 0 deletions packages/common/src/interfaces/columnFilter.interface.ts
Expand Up @@ -115,6 +115,17 @@ export interface ColumnFilter {
*/
queryField?: string;

/**
* Defaults to true, should an empty search term have the effect of returning all results?
* Typically that would be True except for a dropdown Select Filter,
* we might really want to filter an empty string and/or `undefined` and for these special cases we can set this flag to `false`.
*
* NOTE: for a dropdown Select Filter, we will assume that on a multipleSelect Filter it should default to `false`
* however for a singleSelect Filter (and any other type of Filters) it should default to `true`.
* In any case, the user can overrides it this flag.
*/
emptySearchTermReturnAllValues?: boolean;

/** What is the Field Type that can be used by the Filter (as precedence over the "type" set the column definition) */
type?: typeof FieldType[keyof typeof FieldType];

Expand Down
61 changes: 54 additions & 7 deletions packages/common/src/services/__tests__/filter.service.spec.ts
Expand Up @@ -301,6 +301,7 @@ describe('FilterService', () => {
searchTerms: ['John'],
grid: gridStub
};
sharedService.allColumns = [mockColumn];

service.init(gridStub);
service.bindLocalOnFilter(gridStub);
Expand All @@ -320,6 +321,7 @@ describe('FilterService', () => {
gridOptionMock.backendServiceApi = undefined;
mockColumn = { id: 'firstName', field: 'firstName', filterable: true } as Column;
mockArgs = { grid: gridStub, column: mockColumn, node: document.getElementById(DOM_ELEMENT_ID) };
sharedService.allColumns = [mockColumn];
});

it('should execute the search callback normally when a input change event is triggered and searchTerms are defined', () => {
Expand Down Expand Up @@ -350,6 +352,7 @@ describe('FilterService', () => {
it('should execute the callback normally when a input change event is triggered and the searchTerm comes from this event.target', () => {
const expectationColumnFilter = { columnDef: mockColumn, columnId: 'firstName', operator: 'EQ', searchTerms: ['John'], parsedSearchTerms: ['John'], type: FieldType.string };
const spySearchChange = jest.spyOn(service.onSearchChange as any, 'notify');
sharedService.allColumns = [mockColumn];

service.init(gridStub);
service.bindLocalOnFilter(gridStub);
Expand Down Expand Up @@ -382,29 +385,46 @@ describe('FilterService', () => {
expect(service.getColumnFilters()).toEqual({});
});

it('should delete the column filters entry (from column filter object) when searchTerms is empty array and even when triggered event is undefined', () => {
it('should NOT delete the column filters entry (from column filter object) even when searchTerms is empty when user set `emptySearchTermReturnAllValues` to False', () => {
const expectationColumnFilter = { columnDef: mockColumn, columnId: 'firstName', operator: 'EQ', searchTerms: [''], parsedSearchTerms: [''], type: FieldType.string };
const spySearchChange = jest.spyOn(service.onSearchChange as any, 'notify');
sharedService.allColumns = [mockColumn];

service.init(gridStub);
service.bindLocalOnFilter(gridStub);
mockArgs.column.filter = { emptySearchTermReturnAllValues: false };
gridStub.onHeaderRowCellRendered.notify(mockArgs as any, new Slick.EventData(), gridStub);
service.getFiltersMetadata()[0].callback(undefined, { columnDef: mockColumn, operator: 'EQ', searchTerms: [], shouldTriggerQuery: true });
service.getFiltersMetadata()[0].callback(new CustomEvent('input'), { columnDef: mockColumn, operator: 'EQ', searchTerms: [''], shouldTriggerQuery: true });

expect(service.getColumnFilters()).toEqual({});
expect(service.getColumnFilters()).toContainEntry(['firstName', expectationColumnFilter]);
expect(spySearchChange).toHaveBeenCalledWith({
clearFilterTriggered: undefined,
shouldTriggerQuery: true,
columnId: 'firstName',
columnDef: mockColumn,
columnFilters: { firstName: expectationColumnFilter },
operator: 'EQ',
searchTerms: [''],
parsedSearchTerms: [''],
grid: gridStub
}, expect.anything());
expect(service.getCurrentLocalFilters()).toEqual([{ columnId: 'firstName', operator: 'EQ', searchTerms: [''] }]);
});

it('should delete the column filters entry (from column filter object) when searchTerms & operator are undefined or not provided', () => {
it('should delete the column filters entry (from column filter object) when searchTerms is empty array and even when triggered event is undefined', () => {
service.init(gridStub);
service.bindLocalOnFilter(gridStub);
gridStub.onHeaderRowCellRendered.notify(mockArgs as any, new Slick.EventData(), gridStub);
service.getFiltersMetadata()[0].callback(undefined, { columnDef: mockColumn, shouldTriggerQuery: true });
service.getFiltersMetadata()[0].callback(undefined, { columnDef: mockColumn, operator: 'EQ', searchTerms: [], shouldTriggerQuery: true });

expect(service.getColumnFilters()).toEqual({});
});

it('should have an column filters object when callback is called with an undefined column definition', () => {
it('should delete the column filters entry (from column filter object) when searchTerms & operator are undefined or not provided', () => {
service.init(gridStub);
service.bindLocalOnFilter(gridStub);
gridStub.onHeaderRowCellRendered.notify(mockArgs as any, new Slick.EventData(), gridStub);
service.getFiltersMetadata()[0].callback(undefined, { columnDef: undefined, operator: 'EQ', searchTerms: ['John'], shouldTriggerQuery: true });
service.getFiltersMetadata()[0].callback(undefined, { columnDef: mockColumn, shouldTriggerQuery: true });

expect(service.getColumnFilters()).toEqual({});
});
Expand Down Expand Up @@ -560,6 +580,7 @@ describe('FilterService', () => {
mockColumn2 = { id: 'lastName', field: 'lastName', filterable: true, filter: { model: Filters.inputText } } as Column;
const mockArgs1 = { grid: gridStub, column: mockColumn1, node: document.getElementById(DOM_ELEMENT_ID) };
const mockArgs2 = { grid: gridStub, column: mockColumn2, node: document.getElementById(DOM_ELEMENT_ID) };
sharedService.allColumns = [mockColumn1, mockColumn2];

service.init(gridStub);
service.bindLocalOnFilter(gridStub);
Expand Down Expand Up @@ -1063,6 +1084,7 @@ describe('FilterService', () => {
{ columnId: 'firstName', searchTerms: ['Jane'], operator: 'StartsWith' },
{ columnId: 'isActive', searchTerms: [false] }
];
sharedService.allColumns = [mockColumn1, mockColumn2];
});

it('should throw an error when there are no filters defined in the column definitions', (done) => {
Expand Down Expand Up @@ -1210,6 +1232,30 @@ describe('FilterService', () => {
});
});

it('should call "updateSingleFilter" method with an empty search term and still expect event "emitFilterChanged" to be trigged local when setting `emptySearchTermReturnAllValues` to False', () => {
const expectation = {
firstName: { columnId: 'firstName', columnDef: mockColumn1, searchTerms: [''], operator: 'StartsWith', type: FieldType.string },
};
const emitSpy = jest.spyOn(service, 'emitFilterChanged');
const setFilterArgsSpy = jest.spyOn(dataViewStub, 'setFilterArgs');
const refreshSpy = jest.spyOn(dataViewStub, 'refresh');
service.init(gridStub);
service.bindLocalOnFilter(gridStub);
mockArgs1.column.filter = { emptySearchTermReturnAllValues: false };
mockArgs2.column.filter = { emptySearchTermReturnAllValues: false };
gridStub.onHeaderRowCellRendered.notify(mockArgs1 as any, new Slick.EventData(), gridStub);
gridStub.onHeaderRowCellRendered.notify(mockArgs2 as any, new Slick.EventData(), gridStub);
service.updateSingleFilter({ columnId: 'firstName', searchTerms: [''], operator: 'StartsWith' });

expect(setFilterArgsSpy).toHaveBeenCalledWith({ columnFilters: expectation, grid: gridStub });
expect(refreshSpy).toHaveBeenCalled();
expect(emitSpy).toHaveBeenCalledWith('local');
expect(service.getColumnFilters()).toEqual({
firstName: { columnId: 'firstName', columnDef: mockColumn1, searchTerms: [''], operator: 'StartsWith', type: FieldType.string },
});
expect(service.getCurrentLocalFilters()).toEqual([{ columnId: 'firstName', operator: 'StartsWith', searchTerms: [''] }]);
});

it('should call "updateSingleFilter" method and expect event "emitFilterChanged" to be trigged local when using "bindBackendOnFilter" and also expect filters to be set in dataview', () => {
const expectation = {
firstName: { columnId: 'firstName', columnDef: mockColumn1, searchTerms: ['Jane'], operator: 'StartsWith', type: FieldType.string },
Expand Down Expand Up @@ -1474,6 +1520,7 @@ describe('FilterService', () => {
mockArgs2 = { grid: gridStub, column: mockColumn2, node: document.getElementById(DOM_ELEMENT_ID) };
jest.spyOn(dataViewStub, 'getItems').mockReturnValue(dataset);
jest.spyOn(gridStub, 'getColumns').mockReturnValue([mockColumn1, mockColumn2]);
sharedService.allColumns = [mockColumn1, mockColumn2];
});

it('should return True when item is found and its parent is not collapsed', () => {
Expand Down
35 changes: 20 additions & 15 deletions packages/common/src/services/filter.service.ts
Expand Up @@ -595,14 +595,16 @@ export class FilterService {
for (const colId of Object.keys(this._columnFilters)) {
const columnFilter = this._columnFilters[colId];
const filter = { columnId: colId || '' } as CurrentFilter;
const columnDef = this.sharedService.allColumns.find(col => col.id === filter.columnId);
const emptySearchTermReturnAllValues = columnDef?.filter?.emptySearchTermReturnAllValues ?? true;

if (columnFilter?.searchTerms) {
filter.searchTerms = columnFilter.searchTerms;
}
if (columnFilter.operator) {
filter.operator = columnFilter.operator;
}
if (Array.isArray(filter.searchTerms) && filter.searchTerms.length > 0 && filter.searchTerms[0] !== '') {
if (Array.isArray(filter.searchTerms) && filter.searchTerms.length > 0 && (!emptySearchTermReturnAllValues || filter.searchTerms[0] !== '')) {
currentFilters.push(filter);
}
}
Expand All @@ -616,10 +618,10 @@ export class FilterService {
* @param caller
*/
emitFilterChanged(caller: EmitterType) {
if (caller === EmitterType.remote && this._gridOptions && this._gridOptions.backendServiceApi) {
if (caller === EmitterType.remote && this._gridOptions?.backendServiceApi) {
let currentFilters: CurrentFilter[] = [];
const backendService = this._gridOptions.backendServiceApi.service;
if (backendService && backendService.getCurrentFilters) {
if (backendService?.getCurrentFilters) {
currentFilters = backendService.getCurrentFilters() as CurrentFilter[];
}
this.pubSubService.publish('onFilterChanged', currentFilters);
Expand Down Expand Up @@ -650,7 +652,7 @@ export class FilterService {
// query backend, except when it's called by a ClearFilters then we won't
if (args?.shouldTriggerQuery) {
const query = await backendApi.service.processOnFilterChanged(event, args);
const totalItems = this._gridOptions && this._gridOptions.pagination && this._gridOptions.pagination.totalItems || 0;
const totalItems = this._gridOptions?.pagination?.totalItems ?? 0;
this.backendUtilities?.executeBackendCallback(backendApi, query, args, startTime, totalItems, this.emitFilterChanged.bind(this), this.httpCancelRequests$);
}
}
Expand All @@ -666,7 +668,7 @@ export class FilterService {
if (Array.isArray(filters)) {
this._columnDefinitions.forEach((columnDef: Column) => {
// clear any columnDef searchTerms before applying Presets
if (columnDef.filter && columnDef.filter.searchTerms) {
if (columnDef.filter?.searchTerms) {
delete columnDef.filter.searchTerms;
}

Expand Down Expand Up @@ -792,14 +794,14 @@ export class FilterService {
}
});

const backendApi = this._gridOptions && this._gridOptions.backendServiceApi;
const backendApi = this._gridOptions?.backendServiceApi;

// refresh the DataView and trigger an event after all filters were updated and rendered
this._dataView.refresh();

if (backendApi) {
const backendApiService = backendApi && backendApi.service;
if (backendApiService && backendApiService.updateFilters) {
const backendApiService = backendApi?.service;
if (backendApiService?.updateFilters) {
backendApiService.updateFilters(filters, true);
if (triggerBackendQuery) {
this.backendUtilities?.refreshBackendDataset(this._gridOptions);
Expand Down Expand Up @@ -827,7 +829,9 @@ export class FilterService {
const columnDef = this.sharedService.allColumns.find(col => col.id === filter.columnId);
if (columnDef && filter.columnId) {
this._columnFilters = {};
if (Array.isArray(filter.searchTerms) && (filter.searchTerms.length > 1 || (filter.searchTerms.length === 1 && filter.searchTerms[0] !== ''))) {
const emptySearchTermReturnAllValues = columnDef.filter?.emptySearchTermReturnAllValues ?? true;

if (Array.isArray(filter.searchTerms) && (filter.searchTerms.length > 1 || (filter.searchTerms.length === 1 && (!emptySearchTermReturnAllValues || filter.searchTerms[0] !== '')))) {
// pass a columnFilter object as an object which it's property name must be a column field name (e.g.: 'duration': {...} )
this._columnFilters[filter.columnId] = {
columnId: filter.columnId,
Expand All @@ -838,11 +842,11 @@ export class FilterService {
};
}

const backendApi = this._gridOptions && this._gridOptions.backendServiceApi;
const backendApi = this._gridOptions?.backendServiceApi;

if (backendApi) {
const backendApiService = backendApi && backendApi.service;
if (backendApiService && backendApiService.updateFilters) {
const backendApiService = backendApi?.service;
if (backendApiService?.updateFilters) {
backendApiService.updateFilters(this._columnFilters, true);
if (triggerBackendQuery) {
this.backendUtilities?.refreshBackendDataset(this._gridOptions);
Expand Down Expand Up @@ -872,7 +876,7 @@ export class FilterService {
const columnDef = args.column;
const columnId = columnDef?.id ?? '';

if (columnDef && columnId !== 'selector' && columnDef.filterable) {
if (columnId !== 'selector' && columnDef?.filterable) {
let searchTerms: SearchTerm[] | undefined;
let operator: OperatorString | OperatorType | undefined;
const newFilter: Filter | undefined = this.filterFactory.createFilter(columnDef.filter);
Expand Down Expand Up @@ -922,7 +926,7 @@ export class FilterService {
*/
protected callbackSearchEvent(event: Event | undefined, args: FilterCallbackArg) {
if (args) {
const searchTerm = ((event && event.target) ? (event.target as HTMLInputElement).value : undefined);
const searchTerm = ((event?.target) ? (event.target as HTMLInputElement).value : undefined);
const searchTerms = (args.searchTerms && Array.isArray(args.searchTerms)) ? args.searchTerms : (searchTerm ? [searchTerm] : undefined);
const columnDef = args.columnDef || null;
const columnId = columnDef?.id ?? '';
Expand All @@ -931,10 +935,11 @@ export class FilterService {
const hasSearchTerms = searchTerms && Array.isArray(searchTerms);
const termsCount = hasSearchTerms && searchTerms && searchTerms.length;
const oldColumnFilters = { ...this._columnFilters };
const emptySearchTermReturnAllValues = columnDef.filter?.emptySearchTermReturnAllValues ?? true;
let parsedSearchTerms: SearchTerm | SearchTerm[] | undefined;

if (columnDef && columnId) {
if (!hasSearchTerms || termsCount === 0 || (termsCount === 1 && Array.isArray(searchTerms) && searchTerms[0] === '')) {
if (!hasSearchTerms || termsCount === 0 || (termsCount === 1 && Array.isArray(searchTerms) && emptySearchTermReturnAllValues && searchTerms[0] === '')) {
// delete the property from the columnFilters when it becomes empty
// without doing this, it would leave an incorrect state of the previous column filters when filtering on another column
delete this._columnFilters[columnId];
Expand Down

0 comments on commit c58a92a

Please sign in to comment.