Skip to content

Commit 2c91f35

Browse files
authored
fix(gridService): crud methods should support custom dataset id (#453)
1 parent 0030763 commit 2c91f35

File tree

3 files changed

+129
-11
lines changed

3 files changed

+129
-11
lines changed

src/app/modules/angular-slickgrid/services/__tests__/grid.service.spec.ts

+116-2
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ describe('Grid Service', () => {
6464
let service: GridService;
6565
let sharedService = new SharedService();
6666
let translate: TranslateService;
67-
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true } as GridOption);
67+
const mockGridOptions = { enableAutoResize: true } as GridOption;
68+
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
6869

6970
beforeEach(() => {
7071
TestBed.configureTestingModule({
@@ -79,7 +80,6 @@ describe('Grid Service', () => {
7980
imports: [TranslateModule.forRoot()]
8081
});
8182
translate = TestBed.get(TranslateService);
82-
sharedService = TestBed.get(SharedService);
8383
service = TestBed.get(GridService);
8484
service.init(gridStub, dataviewStub);
8585
});
@@ -125,6 +125,32 @@ describe('Grid Service', () => {
125125
expect(() => service.upsertItem(null)).toThrowError('Calling Upsert of an item requires the item to include an "id" property');
126126
});
127127

128+
it('should throw an error when 1st argument for the item object is missing the Id defined by the "datasetIdPropertyName" property', () => {
129+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, datasetIdPropertyName: 'customId' } as GridOption);
130+
expect(() => service.upsertItem(null)).toThrowError('Calling Upsert of an item requires the item to include an "customId" property');
131+
132+
// reset mock
133+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({});
134+
});
135+
136+
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', () => {
137+
mockGridOptions.datasetIdPropertyName = 'customId';
138+
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
139+
const mockItem = { customId: 0, user: { firstName: 'John', lastName: 'Doe' } };
140+
const dataviewSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined);
141+
const serviceSpy = jest.spyOn(service, 'addItem');
142+
const rxSpy = jest.spyOn(service.onItemUpserted, 'next');
143+
144+
const upsertRow = service.upsertItem(mockItem);
145+
146+
expect(upsertRow).toEqual({ added: 0, updated: undefined });
147+
expect(serviceSpy).toHaveBeenCalledTimes(1);
148+
expect(dataviewSpy).toHaveBeenCalledWith(0);
149+
expect(serviceSpy).toHaveBeenCalledWith(mockItem, { highlightRow: true, position: 'top', resortGrid: false, selectRow: false, triggerEvent: true });
150+
expect(rxSpy).toHaveBeenCalledWith(mockItem);
151+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({});
152+
});
153+
128154
it('should expect the service to call the "addItem" when calling "upsertItem" with the item not being found in the grid', () => {
129155
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
130156
const dataviewSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined);
@@ -303,6 +329,10 @@ describe('Grid Service', () => {
303329
});
304330

305331
describe('updateItem methods', () => {
332+
beforeEach(() => {
333+
jest.clearAllMocks();
334+
});
335+
306336
it('should throw an error when 1st argument for the item object is missing', () => {
307337
expect(() => service.updateItem(null)).toThrowError('Calling Update of an item requires the item to include an "id" property');
308338
});
@@ -412,6 +442,25 @@ describe('Grid Service', () => {
412442
expect(rxSpy).toHaveBeenCalledWith(mockItem);
413443
});
414444

445+
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', () => {
446+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...mockGridOptions, datasetIdPropertyName: 'customId' });
447+
const mockItem = { customId: 0, user: { firstName: 'John', lastName: 'Doe' } };
448+
const getRowIdSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(mockItem.customId);
449+
const getRowIndexSpy = jest.spyOn(dataviewStub, 'getIdxById').mockReturnValue(mockItem.customId);
450+
const updateSpy = jest.spyOn(service, 'updateItemById');
451+
const rxSpy = jest.spyOn(service.onItemUpdated, 'next');
452+
453+
service.updateItem(mockItem);
454+
455+
expect(updateSpy).toHaveBeenCalledTimes(1);
456+
expect(getRowIdSpy).toHaveBeenCalledWith(0);
457+
expect(getRowIndexSpy).toHaveBeenCalledWith(0);
458+
expect(updateSpy).toHaveBeenCalledWith(mockItem.customId, mockItem, { highlightRow: true, selectRow: false, scrollRowIntoView: false, triggerEvent: true });
459+
expect(rxSpy).toHaveBeenCalledWith(mockItem);
460+
delete mockGridOptions.datasetIdPropertyName;
461+
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
462+
});
463+
415464
it('should throw an error when calling "updateItemById" without a valid "id"', () => {
416465
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
417466
expect(() => service.updateItemById(undefined, mockItem)).toThrowError('Cannot update a row without a valid "id"');
@@ -422,6 +471,14 @@ describe('Grid Service', () => {
422471
jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined);
423472
expect(() => service.updateItemById(5, mockItem)).toThrowError('The item to update in the grid was not found with id: 5');
424473
});
474+
475+
it('should throw an error when 1st argument for the item object is missing the Id defined by the "datasetIdPropertyName" property', () => {
476+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, datasetIdPropertyName: 'customId' } as GridOption);
477+
expect(() => service.updateItem(null)).toThrowError('Calling Update of an item requires the item to include an "customId" property');
478+
479+
// reset mock
480+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({});
481+
});
425482
});
426483

427484
describe('addItem methods', () => {
@@ -603,6 +660,37 @@ describe('Grid Service', () => {
603660
expect(selectSpy).toHaveBeenCalledWith(0);
604661
expect(rxSpy).toHaveBeenCalledWith(mockItem);
605662
});
663+
664+
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', () => {
665+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...mockGridOptions, datasetIdPropertyName: 'customId' });
666+
const mockItem = { customId: 0, user: { firstName: 'John', lastName: 'Doe' } };
667+
668+
// datasetIdPropertyName: 'customId'
669+
const addSpy = jest.spyOn(dataviewStub, 'insertItem');
670+
const selectSpy = jest.spyOn(gridStub, 'setSelectedRows');
671+
const scrollSpy = jest.spyOn(gridStub, 'scrollRowIntoView');
672+
const rxSpy = jest.spyOn(service.onItemAdded, 'next');
673+
674+
service.addItem(mockItem);
675+
676+
expect(addSpy).toHaveBeenCalledTimes(1);
677+
expect(addSpy).toHaveBeenCalledWith(0, mockItem);
678+
expect(selectSpy).not.toHaveBeenCalled();
679+
expect(scrollSpy).toHaveBeenCalledWith(0);
680+
expect(rxSpy).toHaveBeenCalledWith(mockItem);
681+
delete mockGridOptions.datasetIdPropertyName;
682+
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
683+
});
684+
685+
it('should throw an error when 1st argument for the item object is missing the Id defined by the "datasetIdPropertyName" property', () => {
686+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, datasetIdPropertyName: 'customId' } as GridOption);
687+
expect(() => service.addItem(null)).toThrowError('Adding an item requires the item to include an "customId" property');
688+
expect(() => service.addItem({ user: 'John' })).toThrowError('Adding an item requires the item to include an "customId" property');
689+
690+
// reset mock
691+
delete mockGridOptions.datasetIdPropertyName;
692+
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
693+
});
606694
});
607695

608696
describe('deleteItem methods', () => {
@@ -747,6 +835,32 @@ describe('Grid Service', () => {
747835
const output = service.deleteItemByIds(5, { triggerEvent: true });
748836
expect(output).toEqual([]);
749837
});
838+
839+
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', () => {
840+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...mockGridOptions, datasetIdPropertyName: 'customId' });
841+
const mockItem = { customId: 4, user: { firstName: 'John', lastName: 'Doe' } };
842+
const deleteSpy = jest.spyOn(dataviewStub, 'deleteItem');
843+
const rxSpy = jest.spyOn(service.onItemDeleted, 'next');
844+
845+
const output = service.deleteItemById(mockItem.customId);
846+
847+
expect(output).toEqual(4);
848+
expect(deleteSpy).toHaveBeenCalledTimes(1);
849+
expect(deleteSpy).toHaveBeenCalledWith(mockItem.customId);
850+
expect(rxSpy).toHaveBeenLastCalledWith(mockItem.customId);
851+
delete mockGridOptions.datasetIdPropertyName;
852+
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
853+
});
854+
855+
it('should throw an error when 1st argument for the item object is missing the Id defined by the "datasetIdPropertyName" property', () => {
856+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, datasetIdPropertyName: 'customId' } as GridOption);
857+
expect(() => service.deleteItem(null)).toThrowError('Deleting an item requires the item to include an "customId" property');
858+
expect(() => service.deleteItem({ user: 'John' })).toThrowError('Deleting an item requires the item to include an "customId" property');
859+
860+
// reset mock
861+
delete mockGridOptions.datasetIdPropertyName;
862+
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
863+
});
750864
});
751865

752866
describe('clearAllFiltersAndSorts method', () => {

src/app/modules/angular-slickgrid/services/grid.service.ts

+12-8
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,9 @@ export class GridService {
299299
if (!this._grid || !this._gridOptions || !this._dataView) {
300300
throw new Error('We could not find SlickGrid Grid, DataView objects');
301301
}
302-
if (!item || !item.hasOwnProperty('id')) {
303-
throw new Error(`Adding an item requires the item to include an "id" property`);
302+
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
303+
if (!item || !item.hasOwnProperty(idPropName)) {
304+
throw new Error(`Adding an item requires the item to include an "${idPropName}" property`);
304305
}
305306

306307
// insert position top/bottom, defaults to top
@@ -423,9 +424,10 @@ export class GridService {
423424
*/
424425
deleteItem(item: any, options?: GridServiceDeleteOption): number | string {
425426
options = { ...GridServiceDeleteOptionDefaults, ...options };
427+
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
426428

427-
if (!item || !item.hasOwnProperty('id')) {
428-
throw new Error(`Deleting an item requires the item to include an "id" property`);
429+
if (!item || !item.hasOwnProperty(idPropName)) {
430+
throw new Error(`Deleting an item requires the item to include an "${idPropName}" property`);
429431
}
430432
return this.deleteItemById(item.id, options);
431433
}
@@ -539,10 +541,11 @@ export class GridService {
539541
*/
540542
updateItem(item: any, options?: GridServiceUpdateOption): number {
541543
options = { ...GridServiceUpdateOptionDefaults, ...options };
542-
const itemId = (!item || !item.hasOwnProperty('id')) ? undefined : item.id;
544+
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
545+
const itemId = (!item || !item.hasOwnProperty(idPropName)) ? undefined : item[idPropName];
543546

544547
if (itemId === undefined) {
545-
throw new Error(`Calling Update of an item requires the item to include an "id" property`);
548+
throw new Error(`Calling Update of an item requires the item to include an "${idPropName}" property`);
546549
}
547550

548551
return this.updateItemById(itemId, item, options);
@@ -639,10 +642,11 @@ export class GridService {
639642
*/
640643
upsertItem(item: any, options?: GridServiceInsertOption): { added: number, updated: number } {
641644
options = { ...GridServiceInsertOptionDefaults, ...options };
642-
const itemId = (!item || !item.hasOwnProperty('id')) ? undefined : item.id;
645+
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
646+
const itemId = (!item || !item.hasOwnProperty(idPropName)) ? undefined : item[idPropName];
643647

644648
if (itemId === undefined) {
645-
throw new Error(`Calling Upsert of an item requires the item to include an "id" property`);
649+
throw new Error(`Calling Upsert of an item requires the item to include an "${idPropName}" property`);
646650
}
647651

648652
return this.upsertItemById(itemId, item, options);

src/app/modules/angular-slickgrid/services/utilities.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ export function charArraysEqual(a: any[], b: any[], orderMatters: boolean = fals
115115
return false;
116116
}
117117

118-
if (!orderMatters) {
118+
if (!orderMatters && a.sort && b.sort) {
119119
a.sort();
120120
b.sort();
121121
}

0 commit comments

Comments
 (0)