Skip to content

Commit

Permalink
fix(gridService): crud methods should support custom dataset id (#453)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ghiscoding
ghiscoding committed May 12, 2020
1 parent 0030763 commit 2c91f35
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 11 deletions.
118 changes: 116 additions & 2 deletions src/app/modules/angular-slickgrid/services/__tests__/grid.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ describe('Grid Service', () => {
let service: GridService;
let sharedService = new SharedService();
let translate: TranslateService;
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true } as GridOption);
const mockGridOptions = { enableAutoResize: true } as GridOption;
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);

beforeEach(() => {
TestBed.configureTestingModule({
Expand All @@ -79,7 +80,6 @@ describe('Grid Service', () => {
imports: [TranslateModule.forRoot()]
});
translate = TestBed.get(TranslateService);
sharedService = TestBed.get(SharedService);
service = TestBed.get(GridService);
service.init(gridStub, dataviewStub);
});
Expand Down Expand Up @@ -125,6 +125,32 @@ describe('Grid Service', () => {
expect(() => service.upsertItem(null)).toThrowError('Calling Upsert of an item requires the item to include an "id" property');
});

it('should throw an error when 1st argument for the item object is missing the Id defined by the "datasetIdPropertyName" property', () => {
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, datasetIdPropertyName: 'customId' } as GridOption);
expect(() => service.upsertItem(null)).toThrowError('Calling Upsert of an item requires the item to include an "customId" property');

// reset mock
jest.spyOn(gridStub, 'getOptions').mockReturnValue({});
});

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', () => {
mockGridOptions.datasetIdPropertyName = 'customId';
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
const mockItem = { customId: 0, user: { firstName: 'John', lastName: 'Doe' } };
const dataviewSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined);
const serviceSpy = jest.spyOn(service, 'addItem');
const rxSpy = jest.spyOn(service.onItemUpserted, 'next');

const upsertRow = service.upsertItem(mockItem);

expect(upsertRow).toEqual({ added: 0, updated: undefined });
expect(serviceSpy).toHaveBeenCalledTimes(1);
expect(dataviewSpy).toHaveBeenCalledWith(0);
expect(serviceSpy).toHaveBeenCalledWith(mockItem, { highlightRow: true, position: 'top', resortGrid: false, selectRow: false, triggerEvent: true });
expect(rxSpy).toHaveBeenCalledWith(mockItem);
jest.spyOn(gridStub, 'getOptions').mockReturnValue({});
});

it('should expect the service to call the "addItem" when calling "upsertItem" with the item not being found in the grid', () => {
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
const dataviewSpy = jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined);
Expand Down Expand Up @@ -303,6 +329,10 @@ describe('Grid Service', () => {
});

describe('updateItem methods', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('should throw an error when 1st argument for the item object is missing', () => {
expect(() => service.updateItem(null)).toThrowError('Calling Update of an item requires the item to include an "id" property');
});
Expand Down Expand Up @@ -412,6 +442,25 @@ describe('Grid Service', () => {
expect(rxSpy).toHaveBeenCalledWith(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', () => {
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);
const getRowIndexSpy = jest.spyOn(dataviewStub, 'getIdxById').mockReturnValue(mockItem.customId);
const updateSpy = jest.spyOn(service, 'updateItemById');
const rxSpy = jest.spyOn(service.onItemUpdated, 'next');

service.updateItem(mockItem);

expect(updateSpy).toHaveBeenCalledTimes(1);
expect(getRowIdSpy).toHaveBeenCalledWith(0);
expect(getRowIndexSpy).toHaveBeenCalledWith(0);
expect(updateSpy).toHaveBeenCalledWith(mockItem.customId, mockItem, { highlightRow: true, selectRow: false, scrollRowIntoView: false, triggerEvent: true });
expect(rxSpy).toHaveBeenCalledWith(mockItem);
delete mockGridOptions.datasetIdPropertyName;
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
});

it('should throw an error when calling "updateItemById" without a valid "id"', () => {
const mockItem = { id: 0, user: { firstName: 'John', lastName: 'Doe' } };
expect(() => service.updateItemById(undefined, mockItem)).toThrowError('Cannot update a row without a valid "id"');
Expand All @@ -422,6 +471,14 @@ describe('Grid Service', () => {
jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(undefined);
expect(() => service.updateItemById(5, mockItem)).toThrowError('The item to update in the grid was not found with id: 5');
});

it('should throw an error when 1st argument for the item object is missing the Id defined by the "datasetIdPropertyName" property', () => {
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, datasetIdPropertyName: 'customId' } as GridOption);
expect(() => service.updateItem(null)).toThrowError('Calling Update of an item requires the item to include an "customId" property');

// reset mock
jest.spyOn(gridStub, 'getOptions').mockReturnValue({});
});
});

describe('addItem methods', () => {
Expand Down Expand Up @@ -603,6 +660,37 @@ describe('Grid Service', () => {
expect(selectSpy).toHaveBeenCalledWith(0);
expect(rxSpy).toHaveBeenCalledWith(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', () => {
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...mockGridOptions, datasetIdPropertyName: 'customId' });
const mockItem = { customId: 0, user: { firstName: 'John', lastName: 'Doe' } };

// datasetIdPropertyName: 'customId'
const addSpy = jest.spyOn(dataviewStub, 'insertItem');
const selectSpy = jest.spyOn(gridStub, 'setSelectedRows');
const scrollSpy = jest.spyOn(gridStub, 'scrollRowIntoView');
const rxSpy = jest.spyOn(service.onItemAdded, 'next');

service.addItem(mockItem);

expect(addSpy).toHaveBeenCalledTimes(1);
expect(addSpy).toHaveBeenCalledWith(0, mockItem);
expect(selectSpy).not.toHaveBeenCalled();
expect(scrollSpy).toHaveBeenCalledWith(0);
expect(rxSpy).toHaveBeenCalledWith(mockItem);
delete mockGridOptions.datasetIdPropertyName;
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
});

it('should throw an error when 1st argument for the item object is missing the Id defined by the "datasetIdPropertyName" property', () => {
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, datasetIdPropertyName: 'customId' } as GridOption);
expect(() => service.addItem(null)).toThrowError('Adding an item requires the item to include an "customId" property');
expect(() => service.addItem({ user: 'John' })).toThrowError('Adding an item requires the item to include an "customId" property');

// reset mock
delete mockGridOptions.datasetIdPropertyName;
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
});
});

describe('deleteItem methods', () => {
Expand Down Expand Up @@ -747,6 +835,32 @@ describe('Grid Service', () => {
const output = service.deleteItemByIds(5, { triggerEvent: true });
expect(output).toEqual([]);
});

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', () => {
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...mockGridOptions, datasetIdPropertyName: 'customId' });
const mockItem = { customId: 4, user: { firstName: 'John', lastName: 'Doe' } };
const deleteSpy = jest.spyOn(dataviewStub, 'deleteItem');
const rxSpy = jest.spyOn(service.onItemDeleted, 'next');

const output = service.deleteItemById(mockItem.customId);

expect(output).toEqual(4);
expect(deleteSpy).toHaveBeenCalledTimes(1);
expect(deleteSpy).toHaveBeenCalledWith(mockItem.customId);
expect(rxSpy).toHaveBeenLastCalledWith(mockItem.customId);
delete mockGridOptions.datasetIdPropertyName;
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
});

it('should throw an error when 1st argument for the item object is missing the Id defined by the "datasetIdPropertyName" property', () => {
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, datasetIdPropertyName: 'customId' } as GridOption);
expect(() => service.deleteItem(null)).toThrowError('Deleting an item requires the item to include an "customId" property');
expect(() => service.deleteItem({ user: 'John' })).toThrowError('Deleting an item requires the item to include an "customId" property');

// reset mock
delete mockGridOptions.datasetIdPropertyName;
jest.spyOn(gridStub, 'getOptions').mockReturnValue(mockGridOptions);
});
});

describe('clearAllFiltersAndSorts method', () => {
Expand Down
20 changes: 12 additions & 8 deletions src/app/modules/angular-slickgrid/services/grid.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,8 +299,9 @@ export class GridService {
if (!this._grid || !this._gridOptions || !this._dataView) {
throw new Error('We could not find SlickGrid Grid, DataView objects');
}
if (!item || !item.hasOwnProperty('id')) {
throw new Error(`Adding an item requires the item to include an "id" property`);
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
if (!item || !item.hasOwnProperty(idPropName)) {
throw new Error(`Adding an item requires the item to include an "${idPropName}" property`);
}

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

if (!item || !item.hasOwnProperty('id')) {
throw new Error(`Deleting an item requires the item to include an "id" property`);
if (!item || !item.hasOwnProperty(idPropName)) {
throw new Error(`Deleting an item requires the item to include an "${idPropName}" property`);
}
return this.deleteItemById(item.id, options);
}
Expand Down Expand Up @@ -539,10 +541,11 @@ export class GridService {
*/
updateItem(item: any, options?: GridServiceUpdateOption): number {
options = { ...GridServiceUpdateOptionDefaults, ...options };
const itemId = (!item || !item.hasOwnProperty('id')) ? undefined : item.id;
const idPropName = this._gridOptions.datasetIdPropertyName || 'id';
const itemId = (!item || !item.hasOwnProperty(idPropName)) ? undefined : item[idPropName];

if (itemId === undefined) {
throw new Error(`Calling Update of an item requires the item to include an "id" property`);
throw new Error(`Calling Update of an item requires the item to include an "${idPropName}" property`);
}

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

if (itemId === undefined) {
throw new Error(`Calling Upsert of an item requires the item to include an "id" property`);
throw new Error(`Calling Upsert of an item requires the item to include an "${idPropName}" property`);
}

return this.upsertItemById(itemId, item, options);
Expand Down
2 changes: 1 addition & 1 deletion src/app/modules/angular-slickgrid/services/utilities.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export function charArraysEqual(a: any[], b: any[], orderMatters: boolean = fals
return false;
}

if (!orderMatters) {
if (!orderMatters && a.sort && b.sort) {
a.sort();
b.sort();
}
Expand Down

0 comments on commit 2c91f35

Please sign in to comment.