Skip to content

Commit

Permalink
feat(backend): add cancellable onBeforeSearchChange & revert on error
Browse files Browse the repository at this point in the history
- this enhancement idea came from this [Discussion](#337)
- in previous code, if an error happens on the backend server while querying, the search input values would still be changed and we had no clue of the previous value, this PR bring this functionality that if an error occurs it will rollback to previous filter input values.
- this PR also adds the option to prevent bubbling of `onBeforeSearchChange` (a new event) to optionally cancel the search query (the search input value would still be applied because you can't cancel that part but you can call `filterService.resetToPreviousSearchFilters()` to rollback the last search)
  • Loading branch information
ghiscoding committed Sep 3, 2021
1 parent a96c3f2 commit b26a53d
Show file tree
Hide file tree
Showing 10 changed files with 211 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ <h3 class="title is-3">
</div>
</h3>

<h6 class="title is-6 italic">
NOTE: The last column (filter & sort) will always throw an error and its only purpose is to demo what would happen
when you encounter a backend server error (the UI should rollback to previous state before you did the action).
</h6>

<div class="row">
<button class="button is-small" data-test="clear-filters-sorting"
onclick.delegate="clearAllFiltersAndSorts()" title="Clear all Filters & Sorts">
Expand Down
22 changes: 15 additions & 7 deletions examples/webpack-demo-vanilla-bundle/src/examples/example09.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,16 @@ export class Example09 {
// this._bindingEventService.bind(gridContainerElm, 'onafterexporttoexcel', () => console.log('onAfterExportToExcel'));
this.sgb = new Slicker.GridBundle(gridContainerElm, this.columnDefinitions, { ...ExampleGridOptions, ...this.gridOptions }, []);

// you can optionally cancel the sort for whatever reason
// you can optionally cancel the Sort, Filter
// this._bindingEventService.bind(gridContainerElm, 'onbeforesort', (e) => {
// e.preventDefault();
// return false;
// });
// this._bindingEventService.bind(gridContainerElm, 'onbeforesearchchange', (e) => {
// e.preventDefault();
// this.sgb.filterService.resetToPreviousSearchFilters(); // optionally reset filter input value
// return false;
// });
}

dispose() {
Expand Down Expand Up @@ -75,7 +80,7 @@ export class Example09 {
collection: [{ value: '', label: '' }, { value: 'male', label: 'male' }, { value: 'female', label: 'female' }]
}
},
{ id: 'company', name: 'Company', field: 'company', sortable: true },
{ id: 'company', name: 'Company', field: 'company', filterable: true, sortable: true },
];

this.gridOptions = {
Expand Down Expand Up @@ -116,7 +121,8 @@ export class Example09 {
enableCount: this.isCountEnabled, // add the count in the OData query, which will return a property named "odata.count" (v2) or "@odata.count" (v4)
version: this.odataVersion // defaults to 2, the query string is slightly different between OData 2 and 4
},
onError: () => {
onError: (error: Error) => {
this.errorStatus = error.message;
this.errorStatusClass = 'visible notification is-light is-danger is-small is-narrow';
this.displaySpinner(false, true);
},
Expand Down Expand Up @@ -175,7 +181,6 @@ export class Example09 {
* in your case the getCustomer() should be a WebAPI function returning a Promise
*/
getCustomerDataApiMock(query): Promise<any> {
this.errorStatus = '';
this.errorStatusClass = 'hidden';

// the mock is returning a Promise, just like a WebAPI typically does
Expand Down Expand Up @@ -226,14 +231,17 @@ export class Example09 {
const fieldName = filterMatch[1].trim();
columnFilters[fieldName] = { type: 'ends', term: filterMatch[2].trim() };
}

// simular a backend error when trying to sort on the "Company" field
if (filterBy.includes('company')) {
throw new Error('Cannot filter by the field "Company"');
}
}
}

// simular a backend error when trying to sort on the "Company" field
if (orderBy.includes('company')) {
const errorMsg = 'Cannot sort by the field "Company"';
this.errorStatus = errorMsg;
throw new Error(errorMsg);
throw new Error('Cannot sort by the field "Company"');
}

const sort = orderBy.includes('asc')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ <h3 class="title is-3">
</div>
</h3>

<h6 class="title is-6 italic">
NOTE: The last column (filter & sort) will always throw an error and its only purpose is to demo what would happen
when you encounter a backend server error (the UI should rollback to previous state before you did the action).
</h6>

<div class="row">
<button class="button is-small" data-test="clear-filters-sorting"
onclick.delegate="clearAllFiltersAndSorts()" title="Clear all Filters & Sorts">
Expand Down
14 changes: 9 additions & 5 deletions examples/webpack-demo-vanilla-bundle/src/examples/example15.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class Example15 {
}
}
},
{ id: 'company', name: 'Company', field: 'company', sortable: true },
{ id: 'company', name: 'Company', field: 'company', filterable: true, sortable: true },
];

this.gridOptions = {
Expand Down Expand Up @@ -129,7 +129,8 @@ export class Example15 {
enableCount: this.isCountEnabled, // add the count in the OData query, which will return a property named "odata.count" (v2) or "@odata.count" (v4)
version: this.odataVersion // defaults to 2, the query string is slightly different between OData 2 and 4
},
onError: () => {
onError: (error: Error) => {
this.errorStatus = error.message;
this.errorStatusClass = 'visible notification is-light is-danger is-small is-narrow';
this.displaySpinner(false, true);
},
Expand Down Expand Up @@ -265,14 +266,17 @@ export class Example15 {
const fieldName = filterMatch[1].trim();
columnFilters[fieldName] = { type: 'ends', term: filterMatch[2].trim() };
}

// simular a backend error when trying to sort on the "Company" field
if (filterBy.includes('company')) {
throw new Error('Cannot filter by the field "Company"');
}
}
}

// simular a backend error when trying to sort on the "Company" field
if (orderBy.includes('company')) {
const errorMsg = 'Cannot sort by the field "Company"';
this.errorStatus = errorMsg;
throw new Error(errorMsg);
throw new Error('Cannot sort by the field "Company"');
}

const sort = orderBy.includes('asc')
Expand Down
11 changes: 8 additions & 3 deletions packages/common/src/services/__tests__/filter.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -565,14 +565,19 @@ describe('FilterService', () => {
gridOptionMock.backendServiceApi!.onError = () => jest.fn();
const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish');
const spyOnError = jest.spyOn(gridOptionMock.backendServiceApi as BackendServiceApi, 'onError');
const updateFilterSpy = jest.spyOn(service, 'updateFilters');
jest.spyOn(gridOptionMock.backendServiceApi as BackendServiceApi, 'process');

// get previous filters before calling the query that will fail
const previousFilters = service.getPreviousFilters();

service.clearFilters();
expect(pubSubSpy).toHaveBeenCalledWith(`onBeforeFilterClear`, true, 0);

setTimeout(() => {
expect(pubSubSpy).toHaveBeenCalledWith(`onFilterCleared`, true);
expect(spyOnError).toHaveBeenCalledWith(errorExpected);
expect(updateFilterSpy).toHaveBeenCalledWith(previousFilters, false, false, false);
done();
});
});
Expand Down Expand Up @@ -970,7 +975,7 @@ describe('FilterService', () => {
});

it('should throw an error when grid argument is an empty object', (done) => {
service.onBackendFilterChange(undefined as any, {}).catch((error) => {
service.onBackendFilterChange(undefined as any, {} as any).catch((error) => {
expect(error.message).toContain(`Something went wrong when trying to bind the "onBackendFilterChange(event, args)" function`);
done();
});
Expand All @@ -979,7 +984,7 @@ describe('FilterService', () => {
it('should throw an error when backendServiceApi is undefined', (done) => {
gridOptionMock.backendServiceApi = undefined;

service.onBackendFilterChange(undefined as any, { grid: gridStub }).catch((error) => {
service.onBackendFilterChange(undefined as any, { grid: gridStub } as any).catch((error) => {
expect(error.message).toContain(`BackendServiceApi requires at least a "process" function and a "service" defined`);
done();
});
Expand All @@ -989,7 +994,7 @@ describe('FilterService', () => {
const spy = jest.spyOn(gridOptionMock.backendServiceApi as BackendServiceApi, 'preProcess');

service.init(gridStub);
service.onBackendFilterChange(undefined as any, { grid: gridStub });
service.onBackendFilterChange(undefined as any, { grid: gridStub } as any);

expect(spy).toHaveBeenCalled();
});
Expand Down
98 changes: 72 additions & 26 deletions packages/common/src/services/filter.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import { Constants } from '../constants';
// using external non-typed js libraries
declare const Slick: SlickNamespace;

interface OnSearchChangeEvent {
interface OnSearchChangeEventArgs {
clearFilterTriggered?: boolean;
shouldTriggerQuery?: boolean;
columnId: string | number;
Expand All @@ -59,7 +59,8 @@ export class FilterService {
protected _columnFilters: ColumnFilters = {};
protected _grid!: SlickGrid;
protected _isTreePresetExecuted = false;
protected _onSearchChange: SlickEvent<OnSearchChangeEvent> | null;
protected _previousFilters: CurrentFilter[] = [];
protected _onSearchChange: SlickEvent<OnSearchChangeEventArgs> | null;
protected _tmpPreFilteredData?: number[];
protected httpCancelRequests$?: Subject<void>; // this will be used to cancel any pending http request

Expand Down Expand Up @@ -133,7 +134,7 @@ export class FilterService {
* Dispose of the filters, since it's a singleton, we don't want to affect other grids with same columns
*/
disposeColumnFilters() {
this.resetColumnFilters();
this.removeAllColumnFiltersProperties();

// also destroy each Filter instances
if (Array.isArray(this._filtersMetadata)) {
Expand All @@ -145,20 +146,6 @@ export class FilterService {
}
}

/**
* When clearing or disposing of all filters, we need to loop through all columnFilters and delete them 1 by 1
* only trying to make columnFilter an empty (without looping) would not trigger a dataset change
*/
resetColumnFilters() {
if (typeof this._columnFilters === 'object') {
for (const columnId in this._columnFilters) {
if (columnId && this._columnFilters[columnId]) {
delete this._columnFilters[columnId];
}
}
}
}

/**
* Bind a backend filter hook to the grid
* @param grid SlickGrid Grid object
Expand Down Expand Up @@ -227,6 +214,9 @@ export class FilterService {
if (!isClearFilterEvent) {
await this.emitFilterChanged(EmitterType.local);
}

// keep a copy of the filters in case we need to rollback
this._previousFilters = this.extractBasicFilterDetails(this._columnFilters);
});
}

Expand Down Expand Up @@ -262,7 +252,7 @@ export class FilterService {
// when using a backend service, we need to manually trigger a filter change but only if the filter was previously filled
if (isBackendApi) {
if (currentColFilter !== undefined) {
this.onBackendFilterChange(event as KeyboardEvent, { grid: this._grid, columnFilters: this._columnFilters });
this.onBackendFilterChange(event as KeyboardEvent, { grid: this._grid, columnFilters: this._columnFilters } as unknown as OnSearchChangeEventArgs);
}
}

Expand All @@ -286,8 +276,8 @@ export class FilterService {
}
});

// also reset the columnFilters object and remove any filters from the object
this.resetColumnFilters();
// also delete the columnFilters object and remove any filters from the object
this.removeAllColumnFiltersProperties();

// also remove any search terms directly on each column definitions
if (Array.isArray(this._columnDefinitions)) {
Expand All @@ -312,8 +302,13 @@ export class FilterService {
const query = queryResponse as string;
const totalItems = this._gridOptions?.pagination?.totalItems ?? 0;
this.backendUtilities?.executeBackendCallback(backendApi, query, callbackArgs, new Date(), totalItems, {
errorCallback: this.resetToPreviousSearchFilters.bind(this),
successCallback: (responseArgs) => this._previousFilters = this.extractBasicFilterDetails(responseArgs.columnFilters),
emitActionChangedCallback: this.emitFilterChanged.bind(this)
});
} else {
// keep a copy of the filters in case we need to rollback
this._previousFilters = this.extractBasicFilterDetails(this._columnFilters);
}

// emit an event when filters are all cleared
Expand Down Expand Up @@ -617,11 +612,15 @@ export class FilterService {
return filteredChildrenAndParents;
}

getColumnFilters() {
getColumnFilters(): ColumnFilters {
return this._columnFilters;
}

getFiltersMetadata() {
getPreviousFilters(): CurrentFilter[] {
return this._previousFilters;
}

getFiltersMetadata(): Filter[] {
return this._filtersMetadata;
}

Expand Down Expand Up @@ -668,7 +667,7 @@ export class FilterService {
}
}

async onBackendFilterChange(event: KeyboardEvent, args: any) {
async onBackendFilterChange(event: KeyboardEvent, args: OnSearchChangeEventArgs) {
const isTriggeringQueryEvent = args?.shouldTriggerQuery;

if (isTriggeringQueryEvent) {
Expand All @@ -695,9 +694,11 @@ export class FilterService {

// query backend, except when it's called by a ClearFilters then we won't
if (isTriggeringQueryEvent) {
const query = await backendApi.service.processOnFilterChanged(event, args);
const query = await backendApi.service.processOnFilterChanged(event, args as FilterChangedArgs);
const totalItems = this._gridOptions?.pagination?.totalItems ?? 0;
this.backendUtilities?.executeBackendCallback(backendApi, query, args, startTime, totalItems, {
errorCallback: this.resetToPreviousSearchFilters.bind(this),
successCallback: (responseArgs) => this._previousFilters = this.extractBasicFilterDetails(responseArgs.columnFilters),
emitActionChangedCallback: this.emitFilterChanged.bind(this),
httpCancelRequestSubject: this.httpCancelRequests$
});
Expand Down Expand Up @@ -732,6 +733,9 @@ export class FilterService {
if (this._gridOptions?.enableTreeData) {
this.refreshTreeDataFilters();
}

// keep reference of the filters
this._previousFilters = this.extractBasicFilterDetails(this._columnFilters);
}
return this._columnDefinitions;
}
Expand Down Expand Up @@ -780,6 +784,14 @@ export class FilterService {
}
}

/**
* Reset (revert) to previous filters, it could be because you prevented `onBeforeSearchChange` OR a Backend Error was thrown.
* It will reapply the previous filter state in the UI.
*/
resetToPreviousSearchFilters() {
this.updateFilters(this._previousFilters, false, false, false);
}

/**
* Toggle the Filter Functionality (show/hide the header row filter bar as well)
* @param {boolean} clearFiltersWhenDisabled - when disabling the filters, do we want to clear the filters before hiding the filters? Defaults to True
Expand Down Expand Up @@ -1052,7 +1064,7 @@ export class FilterService {
const eventKey = (event as KeyboardEvent)?.key;
const eventKeyCode = (event as KeyboardEvent)?.keyCode;
if (this._onSearchChange && (args.forceOnSearchChangeEvent || eventKey === 'Enter' || eventKeyCode === KeyCode.ENTER || !dequal(oldColumnFilters, this._columnFilters))) {
this._onSearchChange.notify({
const eventArgs = {
clearFilterTriggered: args.clearFilterTriggered,
shouldTriggerQuery: args.shouldTriggerQuery,
columnId,
Expand All @@ -1062,7 +1074,10 @@ export class FilterService {
searchTerms,
parsedSearchTerms,
grid: this._grid
}, eventData);
} as OnSearchChangeEventArgs;
if (this.pubSubService.publish('onBeforeSearchChange', eventArgs) !== false) {
this._onSearchChange.notify(eventArgs, eventData);
}
}
}
}
Expand Down Expand Up @@ -1106,6 +1121,37 @@ export class FilterService {
return columnDefinitions;
}

/**
* From a ColumnFilters object, extra only the basic filter details (columnId, operator & searchTerms)
* @param {Object} columnFiltersObject - columnFilters object
* @returns - basic details of a column filter
*/
protected extractBasicFilterDetails(columnFiltersObject: ColumnFilters): CurrentFilter[] {
const filters: CurrentFilter[] = [];

if (columnFiltersObject && typeof columnFiltersObject === 'object') {
for (const columnId of Object.keys(columnFiltersObject)) {
const { operator, searchTerms } = columnFiltersObject[`${columnId}`];
filters.push({ columnId, operator, searchTerms });
}
}
return filters;
}

/**
* When clearing or disposing of all filters, we need to loop through all columnFilters and delete them 1 by 1
* only trying to make columnFilter an empty (without looping) would not trigger a dataset change
*/
protected removeAllColumnFiltersProperties() {
if (typeof this._columnFilters === 'object') {
for (const columnId in this._columnFilters) {
if (columnId && this._columnFilters[columnId]) {
delete this._columnFilters[columnId];
}
}
}
}

/**
* Subscribe to `onBeforeHeaderRowCellDestroy` to destroy Filter(s) to avoid leak and not keep orphan filters
* @param {Object} grid - Slick Grid object
Expand Down
Loading

0 comments on commit b26a53d

Please sign in to comment.