Skip to content

Commit

Permalink
feat(backend): add cancellable Pagination change & revert on error
Browse files Browse the repository at this point in the history
- in previous code, if an error happens on the backend server while querying, the Pagination would still be changed and we had no clue of the previous page number (or page size change), this PR bring this functionality that if an error occurs it will rollback to previous Pagination
- when using a Backend Service, you can prevent the Pagination via `onBeforePaginationChange` while on a local (in-memory) it would be via the DataView `onBeforePagingInfoChanged` event
  • Loading branch information
ghiscoding committed Sep 4, 2021
1 parent 913ef72 commit 7a8d903
Show file tree
Hide file tree
Showing 10 changed files with 387 additions and 49 deletions.
15 changes: 11 additions & 4 deletions examples/webpack-demo-vanilla-bundle/src/examples/example09.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ <h3 class="title is-3">
</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).
<span class="has-text-danger">NOTE:</span> For demo purposes, 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).
Also changing Page Size to 50,000 will also throw which again is for demo purposes.
</h6>

<div class="row">
Expand All @@ -26,6 +28,11 @@ <h6 class="title is-6 italic">
<button class="button is-small" data-test="set-dynamic-sorting" onclick.delegate="setSortingDynamically()">
Set Sorting Dynamically
</button>
<button class="button is-small is-danger is-outlined" style="margin-left: 75px" data-test="throw-page-error-btn"
onclick.delegate="throwPageChangeError()">
<span>Throw Error Going to Last Page... </span>
<i class="mdi mdi-page-last"></i>
</button>
</div>

<br />
Expand All @@ -35,10 +42,10 @@ <h6 class="title is-6 italic">
<span>
<label>Programmatically go to first/last page:</label>
<button class="button is-small" data-test="goto-first-page" onclick.delegate="goToFirstPage()">
<i class="fa fa-caret-left fa-lg"></i>&lt;
<i class="mdi mdi-page-first"></i>
</button>
<button class="button is-small" data-test="goto-last-page" onclick.delegate="goToLastPage()">
<i class="fa fa-caret-right fa-lg"></i>&gt;
<i class="mdi mdi-page-last"></i>
</button>
</span>

Expand Down
26 changes: 22 additions & 4 deletions examples/webpack-demo-vanilla-bundle/src/examples/example09.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export class Example09 {
errorStatusClass = 'hidden';
status = '';
statusClass = 'is-success';
isPageErrorTest = false;

constructor() {
this._bindingEventService = new BindingEventService();
Expand All @@ -36,7 +37,7 @@ 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, Filter
// you can optionally cancel the Filtering, Sorting or Pagination with code shown below
// this._bindingEventService.bind(gridContainerElm, 'onbeforesort', (e) => {
// e.preventDefault();
// return false;
Expand All @@ -46,6 +47,10 @@ export class Example09 {
// this.sgb.filterService.resetToPreviousSearchFilters(); // optionally reset filter input value
// return false;
// });
// this._bindingEventService.bind(gridContainerElm, 'onbeforepaginationchange', (e) => {
// e.preventDefault();
// return false;
// });
}

dispose() {
Expand Down Expand Up @@ -100,7 +105,7 @@ export class Example09 {
enableRowSelection: true,
enablePagination: true, // you could optionally disable the Pagination
pagination: {
pageSizes: [10, 20, 50, 100, 500],
pageSizes: [10, 20, 50, 100, 500, 50000],
pageSize: defaultPageSize,
},
presets: {
Expand Down Expand Up @@ -192,9 +197,17 @@ export class Example09 {
let countTotalItems = 100;
const columnFilters = {};

if (this.isPageErrorTest) {
this.isPageErrorTest = false;
throw new Error('Server timed out trying to retrieve data for the last page');
}

for (const param of queryParams) {
if (param.includes('$top=')) {
top = +(param.substring('$top='.length));
if (top === 50000) {
throw new Error('Server timed out retrieving 50,000 rows');
}
}
if (param.includes('$skip=')) {
skip = +(param.substring('$skip='.length));
Expand Down Expand Up @@ -234,14 +247,14 @@ export class Example09 {

// 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"');
throw new Error('Server could not filter using the field "Company"');
}
}
}

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

const sort = orderBy.includes('asc')
Expand Down Expand Up @@ -342,6 +355,11 @@ export class Example09 {
]);
}

throwPageChangeError() {
this.isPageErrorTest = true;
this.sgb.paginationService.goToLastPage();
}

// THE FOLLOWING METHODS ARE ONLY FOR DEMO PURPOSES DO NOT USE THIS CODE
// ---

Expand Down
17 changes: 12 additions & 5 deletions examples/webpack-demo-vanilla-bundle/src/examples/example15.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ <h3 class="title is-3">
</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>
<span class="has-text-danger">NOTE:</span> For demo purposes, 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).
Also changing Page Size to 50,000 will also throw which again is for demo purposes.
</h6>s

<div class="row">
<button class="button is-small" data-test="clear-filters-sorting"
Expand All @@ -30,6 +32,11 @@ <h6 class="title is-6 italic">
onclick.delegate="addOtherGender()" disabled.bind="isOtherGenderAdded">
Add Other Gender via RxJS
</button>
<button class="button is-small is-danger is-outlined" style="margin-left: 50px" data-test="throw-page-error-btn"
onclick.delegate="throwPageChangeError()">
<span>Throw Error Going to Last Page... </span>
<i class="mdi mdi-page-last"></i>
</button>
</div>

<br />
Expand All @@ -39,10 +46,10 @@ <h6 class="title is-6 italic">
<span>
<label>Programmatically go to first/last page:</label>
<button class="button is-small" data-test="goto-first-page" onclick.delegate="goToFirstPage()">
<i class="fa fa-caret-left fa-lg"></i>&lt;
<i class="mdi mdi-page-first"></i>
</button>
<button class="button is-small" data-test="goto-last-page" onclick.delegate="goToLastPage()">
<i class="fa fa-caret-right fa-lg"></i>&gt;
<i class="mdi mdi-page-last"></i>
</button>
</span>

Expand Down
20 changes: 17 additions & 3 deletions examples/webpack-demo-vanilla-bundle/src/examples/example15.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export class Example15 {
status = '';
statusClass = 'is-success';
isOtherGenderAdded = false;
isPageErrorTest = false;
genderCollection = [{ value: 'male', label: 'male' }, { value: 'female', label: 'female' }];

constructor() {
Expand Down Expand Up @@ -108,7 +109,7 @@ export class Example15 {
enableRowSelection: true,
enablePagination: true, // you could optionally disable the Pagination
pagination: {
pageSizes: [10, 20, 50, 100, 500],
pageSizes: [10, 20, 50, 100, 500, 50000],
pageSize: defaultPageSize,
},
presets: {
Expand Down Expand Up @@ -227,9 +228,17 @@ export class Example15 {
let countTotalItems = 100;
const columnFilters = {};

if (this.isPageErrorTest) {
this.isPageErrorTest = false;
throw new Error('Server timed out trying to retrieve data for the last page');
}

for (const param of queryParams) {
if (param.includes('$top=')) {
top = +(param.substring('$top='.length));
if (top === 50000) {
throw new Error('Server timed out retrieving 50,000 rows');
}
}
if (param.includes('$skip=')) {
skip = +(param.substring('$skip='.length));
Expand Down Expand Up @@ -269,14 +278,14 @@ export class Example15 {

// 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"');
throw new Error('Server could not filter using the field "Company"');
}
}
}

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

const sort = orderBy.includes('asc')
Expand Down Expand Up @@ -378,6 +387,11 @@ export class Example15 {
]);
}

throwPageChangeError() {
this.isPageErrorTest = true;
this.sgb.paginationService.goToLastPage();
}

// THE FOLLOWING METHODS ARE ONLY FOR DEMO PURPOSES DO NOT USE THIS CODE
// ---

Expand Down
53 changes: 49 additions & 4 deletions packages/common/src/services/__tests__/pagination.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,17 +326,18 @@ describe('PaginationService', () => {
expect(spy).toHaveBeenCalledWith(4, undefined);
});

it('should not expect "processOnPageChanged" method to be called when we are already on same page', () => {
it('should not expect "processOnPageChanged" method to be called when we are already on same page', async () => {
const spy = jest.spyOn(service, 'processOnPageChanged');
mockGridOption.pagination!.pageNumber = 2;

service.init(gridStub, mockGridOption.pagination as Pagination, mockGridOption.backendServiceApi);
service.goToPageNumber(2);
const output = await service.goToPageNumber(2);

expect(service.dataFrom).toBe(26);
expect(service.dataTo).toBe(50);
expect(service.getCurrentPageNumber()).toBe(2);
expect(spy).not.toHaveBeenCalled();
expect(output).toBeFalsy();
});
});

Expand All @@ -348,12 +349,14 @@ describe('PaginationService', () => {
options: {
columnDefinitions: [{ id: 'name', field: 'name' }] as Column[],
datasetName: 'user',
}
},
onError: jest.fn(),
};
});

afterEach(() => {
jest.clearAllMocks();
jest.spyOn(mockPubSub, 'publish').mockReturnValue(true);
});

it('should execute "preProcess" method when defined', () => {
Expand All @@ -366,10 +369,25 @@ describe('PaginationService', () => {
expect(spy).toHaveBeenCalled();
});

it('should execute "process" method and catch error when process Promise rejects', async () => {
it('should NOT execute anything and return a Promise with Pagination before calling the change', async () => {
const pubSubSpy = jest.spyOn(mockPubSub, 'publish').mockReturnValue(false);

const preProcessSpy = jest.fn();
mockGridOption.backendServiceApi!.preProcess = preProcessSpy;

service.init(gridStub, mockGridOption.pagination as Pagination, mockGridOption.backendServiceApi);
const output = await service.processOnPageChanged(1);

expect(output).toBeTruthy();
expect(pubSubSpy).toHaveBeenCalled();
expect(preProcessSpy).not.toHaveBeenCalled();
});

it('should execute "process" method and catch error when process Promise rejects and there is no "onError" defined', async () => {
const mockError = { error: '404' };
const postSpy = jest.fn();
mockGridOption.backendServiceApi!.process = postSpy;
mockGridOption.backendServiceApi!.onError = undefined;
jest.spyOn(mockBackendService, 'processOnPaginationChanged').mockReturnValue('backend query');
jest.spyOn(mockGridOption.backendServiceApi as BackendServiceApi, 'process').mockReturnValue(Promise.reject(mockError));
const backendErrorSpy = jest.spyOn(backendUtilityServiceStub, 'onBackendError');
Expand All @@ -385,6 +403,7 @@ describe('PaginationService', () => {
it('should execute "process" method and catch error when process Observable fails', async () => {
const mockError = 'observable error';
const postSpy = jest.fn();
mockGridOption.backendServiceApi!.onError = undefined;
mockGridOption.backendServiceApi!.process = postSpy;
jest.spyOn(mockBackendService, 'processOnPaginationChanged').mockReturnValue('backend query');
jest.spyOn(mockGridOption.backendServiceApi as BackendServiceApi, 'process').mockReturnValue(throwError(mockError));
Expand Down Expand Up @@ -591,6 +610,32 @@ describe('PaginationService', () => {
});
});

describe('resetToPreviousPagination method', () => {
it('should call "changeItemPerPage" when page size is different', () => {
const changeItemSpy = jest.spyOn(service, 'changeItemPerPage');
const refreshSpy = jest.spyOn(service, 'refreshPagination');

service.init(gridStub, mockGridOption.pagination as Pagination, mockGridOption.backendServiceApi);
service.changeItemPerPage(100, null, false); // change without triggering event to simulate a change
service.resetToPreviousPagination();

expect(changeItemSpy).toHaveBeenCalled();
expect(refreshSpy).toHaveBeenCalled();
});

it('should call "goToPageNumber" when page size is different', () => {
const changeItemSpy = jest.spyOn(service, 'goToPageNumber');
const refreshSpy = jest.spyOn(service, 'refreshPagination');

service.init(gridStub, mockGridOption.pagination as Pagination, mockGridOption.backendServiceApi);
service.goToPageNumber(100, null, false); // change without triggering event to simulate a change
service.resetToPreviousPagination();

expect(changeItemSpy).toHaveBeenCalled();
expect(refreshSpy).toHaveBeenCalled();
});
});

// processOnItemAddedOrRemoved is private but we can spy on recalculateFromToIndexes
describe('processOnItemAddedOrRemoved private method', () => {
afterEach(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/services/backendUtility.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class BackendUtilityService {

/** On a backend service api error, we will run the "onError" if there is 1 provided or just throw back the error when nothing is provided */
onBackendError(e: any, backendApi: BackendServiceApi) {
if (backendApi?.onError) {
if (typeof backendApi?.onError === 'function') {
backendApi.onError(e);
} else {
throw e;
Expand Down
Loading

0 comments on commit 7a8d903

Please sign in to comment.