Skip to content

Commit

Permalink
fix(service): should be able to update dataview item not shown in grid (
Browse files Browse the repository at this point in the history 
#730)

* fix(service): should be able to update dataview item not shown in grid
- even if an item is not showing in the grid, we should still be able to update it as long as it exists and is found in the DataView
  • Loading branch information
@ghiscoding
ghiscoding committed Aug 2, 2022
1 parent 053806e commit dc88c87
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 21 deletions.
39 changes: 30 additions & 9 deletions packages/common/src/services/__tests__/grid.service.spec.ts
View file
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
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

0 comments on commit dc88c87

Please sign in to comment.