Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(service): should be able to update dataview item not shown in grid #730

Merged
merged 2 commits into from
Aug 2, 2022
Merged

fix(service): should be able to update dataview item not shown in grid #730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d46108
fix(service): should be able to update dataview item not shown in grid
ghiscoding Aug 2, 2022
d314778
tests: add missing grid service unit tests
ghiscoding Aug 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 30 additions & 9 deletions packages/common/src/services/__tests__/grid.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'jest-extended';
import { BasePubSubService } from '@slickgrid-universal/event-pub-sub';

import { FilterService, GridService, GridStateService, PaginationService, SharedService, SortService, TreeDataService } from '../index';
Expand Down Expand Up @@ -373,6 +374,7 @@ describe('Grid Service', () => {
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
const getRowIdSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(mockItem.id);
const getRowIndexSpy = jest.spyOn(dataviewStub, 'getIdxById').mockReturnValue(mockItem.id);
const serviceHighlightSpy = jest.spyOn(service, 'highlightRow');
const updateSpy = jest.spyOn(service, 'updateItemById');
const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish');

Expand All @@ -381,10 +383,30 @@ describe('Grid Service', () => {
expect(updateSpy).toHaveBeenCalledTimes(1);
expect(getRowIdSpy).toHaveBeenCalledWith(0);
expect(getRowIndexSpy).toHaveBeenCalledWith(0);
expect(serviceHighlightSpy).toHaveBeenCalled();
expect(updateSpy).toHaveBeenCalledWith(mockItem.id, mockItem, { highlightRow: true, selectRow: false, scrollRowIntoView: false, skipError: false, triggerEvent: true });
expect(pubSubSpy).toHaveBeenLastCalledWith(`onItemUpdated`, mockItem);
});

it('should be able to update an item that exist in the dataview even when it is not showing in the grid (filtered from the grid) but will not highlight/selectRow since it is not in showing in the grid', () => {
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
const getRowIdSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined);
const getRowIndexSpy = jest.spyOn(dataviewStub, 'getIdxById').mockReturnValue(mockItem.id);
const serviceHighlightSpy = jest.spyOn(service, 'highlightRow');
const updateRowSpy = jest.spyOn(gridStub, 'updateRow');
const selectSpy = jest.spyOn(service, 'setSelectedRows');
const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish');

service.updateItemById(0, mockItem, { highlightRow: true, selectRow: true });

expect(getRowIdSpy).toHaveBeenCalledWith(0);
expect(getRowIndexSpy).toHaveBeenCalledWith(0);
expect(serviceHighlightSpy).not.toHaveBeenCalled();
expect(updateRowSpy).not.toHaveBeenCalled();
expect(selectSpy).not.toHaveBeenCalled();
expect(pubSubSpy).toHaveBeenLastCalledWith(`onItemUpdated`, mockItem);
});

it('should expect the service to call the "updateItemById" when calling "updateItem" and setting the "selecRow" flag and the grid option "enableRowSelection" is set', () => {
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
const getRowIdSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(mockItem.id);
Expand Down Expand Up @@ -469,7 +491,7 @@ describe('Grid Service', () => {
expect(pubSubSpy).toHaveBeenLastCalledWith(`onItemUpdated`, mockItem);
});

it('should expect the service to call the DataView "insertItem" when calling "addItem" with an item that has an Id defined by the "datasetIdPropertyName" property', () => {
it('should expect the service to call the DataView "updateItem" when calling "addItem" with an item that has an Id defined by the "datasetIdPropertyName" property', () => {
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...mockGridOptions, datasetIdPropertyName: 'customId' });
const mockItem = { customId: 0, user: { firstName: 'John', lastName: 'Doe' } };
const getRowIdSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(mockItem.customId);
Expand All @@ -494,15 +516,14 @@ describe('Grid Service', () => {
expect(() => service.updateItemById(undefined as any, mockItem)).toThrowError('Cannot update a row without a valid "id"');
});

it('should NOT throw an error when "skipError" is enabled even when calling "updateItemById" without a valid "id"', () => {
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
expect(() => service.updateItemById(undefined as any, mockItem, { skipError: true })).not.toThrowError('Cannot update a row without a valid "id"');
it('should throw an error when calling "updateItemById" with an invalid/undefined item', () => {
jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined as any);
expect(() => service.updateItemById(5, undefined)).toThrowError('The item to update in the grid was not found with id: 5');
});

it('should throw an error when calling "updateItemById" and not finding the item in the grid', () => {
it('should NOT throw an error when "skipError" is enabled even when calling "updateItemById" without a valid "id"', () => {
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined as any);
expect(() => service.updateItemById(5, mockItem)).toThrowError('The item to update in the grid was not found with id: 5');
expect(() => service.updateItemById(undefined as any, mockItem, { skipError: true })).not.toThrowError('Cannot update a row without a valid "id"');
});

it('should NOT throw an error when "skipError" is enabled even when calling "updateItemById" and not finding the item in the grid', () => {
Expand Down Expand Up @@ -1237,7 +1258,7 @@ describe('Grid Service', () => {
const clearPinningSpy = jest.spyOn(service, 'clearPinning');
jest.spyOn(SharedService.prototype, 'slickGrid', 'get').mockReturnValue(gridStub);

service.setPinning(mockPinning);
service.setPinning(mockPinning as any);

expect(clearPinningSpy).toHaveBeenCalled();
});
Expand Down Expand Up @@ -1745,7 +1766,7 @@ describe('Grid Service', () => {
service.highlightRow(0, 10, 15);
jest.runAllTimers(); // fast-forward timer

expect(updateSpy).toHaveBeenCalledWith(0, mockItem)
expect(updateSpy).toHaveBeenCalledWith(0, mockItem);
expect(renderSpy).toHaveBeenCalledTimes(3);
});
});
Expand Down
26 changes: 14 additions & 12 deletions packages/common/src/services/grid.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ export class GridService {
* @param options: provide the possibility to do certain actions after or during the upsert (highlightRow, selectRow, triggerEvent)
* @return grid row index
*/
updateItem<T = any>(item: T, options?: GridServiceUpdateOption): number {
updateItem<T = any>(item: T, options?: GridServiceUpdateOption): number | undefined {
options = { ...GridServiceUpdateOptionDefaults, ...options };
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
const itemId = (!item || !(idPropName in item)) ? undefined : (item as any)[idPropName];
Expand All @@ -665,7 +665,7 @@ export class GridService {
* @param options: provide the possibility to do certain actions after or during the update (highlightRow, selectRow, triggerEvent)
* @return grid row indexes
*/
updateItems<T = any>(items: T | T[], options?: GridServiceUpdateOption): number[] {
updateItems<T = any>(items: T | T[], options?: GridServiceUpdateOption): Array<number | undefined> {
options = { ...GridServiceUpdateOptionDefaults, ...options };
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';

Expand Down Expand Up @@ -731,40 +731,42 @@ export class GridService {
* @param options: provide the possibility to do certain actions after or during the upsert (highlightRow, selectRow, triggerEvent)
* @return grid row number
*/
updateItemById<T = any>(itemId: number | string, item: T, options?: GridServiceUpdateOption): number {
updateItemById<T = any>(itemId: number | string, item: T, options?: GridServiceUpdateOption): number | undefined {
options = { ...GridServiceUpdateOptionDefaults, ...options };
if (!options?.skipError && itemId === undefined) {
throw new Error(`Cannot update a row without a valid "id"`);
}
const rowNumber = this._dataView.getRowById(itemId) as number;

// when using pagination the item to update might not be on current page, so we bypass this condition
if (!options?.skipError && ((!item || rowNumber === undefined) && !this._gridOptions.enablePagination)) {
if (!options?.skipError && (!item && !this._gridOptions.enablePagination)) {
throw new Error(`The item to update in the grid was not found with id: ${itemId}`);
}

if (this._dataView.getIdxById(itemId) !== undefined) {
// Update the item itself inside the dataView
this._dataView.updateItem<T>(itemId, item);
this._grid.updateRow(rowNumber);
if (rowNumber !== undefined) {
this._grid.updateRow(rowNumber);
}

if (this._gridOptions?.enableTreeData) {
// if we add/remove item(s) from the dataset, we need to also refresh our tree data filters
this.invalidateHierarchicalDataset();
}

// do we want to scroll to the row so that it shows in the Viewport (UI)
if (options.scrollRowIntoView) {
if (options.scrollRowIntoView && rowNumber !== undefined) {
this._grid.scrollRowIntoView(rowNumber);
}

// highlight the row we just updated, if defined
if (options.highlightRow) {
if (options.highlightRow && rowNumber !== undefined) {
this.highlightRow(rowNumber);
}

// select the row in the grid
if (options.selectRow && this._gridOptions && (this._gridOptions.enableCheckboxSelector || this._gridOptions.enableRowSelection)) {
if (rowNumber !== undefined && options.selectRow && this._gridOptions && (this._gridOptions.enableCheckboxSelector || this._gridOptions.enableRowSelection)) {
this.setSelectedRow(rowNumber);
}

Expand All @@ -781,7 +783,7 @@ export class GridService {
* @param item object which must contain a unique "id" property and any other suitable properties
* @param options: provide the possibility to do certain actions after or during the upsert (highlightRow, resortGrid, selectRow, triggerEvent)
*/
upsertItem<T = any>(item: T, options?: GridServiceInsertOption): { added: number | undefined, updated: number | undefined } {
upsertItem<T = any>(item: T, options?: GridServiceInsertOption): { added: number | undefined; updated: number | undefined; } {
options = { ...GridServiceInsertOptionDefaults, ...options };
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
const itemId = (!item || !(idPropName in item)) ? undefined : (item as any)[idPropName];
Expand All @@ -799,7 +801,7 @@ export class GridService {
* @param options: provide the possibility to do certain actions after or during the upsert (highlightRow, resortGrid, selectRow, triggerEvent)
* @return row numbers in the grid
*/
upsertItems<T = any>(items: T | T[], options?: GridServiceInsertOption): { added: number | undefined, updated: number | undefined }[] {
upsertItems<T = any>(items: T | T[], options?: GridServiceInsertOption): { added: number | undefined; updated: number | undefined; }[] {
options = { ...GridServiceInsertOptionDefaults, ...options };
// when it's not an array, we can call directly the single item upsert
if (!Array.isArray(items)) {
Expand All @@ -809,7 +811,7 @@ export class GridService {
// begin bulk transaction
this._dataView.beginUpdate(true);

const upsertedRows: { added: number | undefined, updated: number | undefined }[] = [];
const upsertedRows: { added: number | undefined, updated: number | undefined; }[] = [];
items.forEach((item: T) => {
upsertedRows.push(this.upsertItem<T>(item, { ...options, highlightRow: false, resortGrid: false, selectRow: false, triggerEvent: false }));
});
Expand Down Expand Up @@ -852,7 +854,7 @@ export class GridService {
* @param options: provide the possibility to do certain actions after or during the upsert (highlightRow, resortGrid, selectRow, triggerEvent)
* @return grid row number in the grid
*/
upsertItemById<T = any>(itemId: number | string, item: T, options?: GridServiceInsertOption): { added: number | undefined, updated: number | undefined } {
upsertItemById<T = any>(itemId: number | string, item: T, options?: GridServiceInsertOption): { added: number | undefined; updated: number | undefined; } {
let isItemAdded = false;
options = { ...GridServiceInsertOptionDefaults, ...options };
if (!options?.skipError && (itemId === undefined && !this.hasRowSelectionEnabled())) {
Expand Down