Merge pull request #371 from ghiscoding/bugfix/row-move-with-override…

…-check

fix(plugin): row move shouldn't go further when onBefore returns false

examples/webpack-demo-vanilla-bundle/src/examples/example07.ts

@@ -297,10 +297,10 @@ export class Example7 {
     return collection.sort((item1, item2) => item1.value - item2.value);
   }
 
-  onBeforeMoveRow(e: Event, data: any) {
-    for (let i = 0; i < data.rows.length; i++) {
+  onBeforeMoveRow(e: Event, data: { rows: number[]; insertBefore: number; }) {
+    for (const rowIdx of data.rows) {
       // no point in moving before or after itself
-      if (data.rows[i] === data.insertBefore || data.rows[i] === data.insertBefore - 1) {
+      if (rowIdx === data.insertBefore || rowIdx === data.insertBefore - 1) {
         e.stopPropagation();
         return false;
       }

packages/common/src/extensions/__tests__/rowMoveManagerExtension.spec.ts

@@ -1,5 +1,4 @@
 import { RowMoveManagerExtension } from '../rowMoveManagerExtension';
-import { ExtensionUtility } from '../extensionUtility';
 import { SharedService } from '../../services/shared.service';
 import { TranslateServiceStub } from '../../../../../test/translateServiceStub';
 import { Column, GridOption, RowMoveManager, SlickGrid, SlickNamespace, SlickRowMoveManager } from '../../interfaces/index';
@@ -33,7 +32,6 @@ describe('rowMoveManagerExtension', () => {
   jest.mock('slickgrid/plugins/slick.rowselectionmodel', () => mockSelectionModel);
   Slick.RowSelectionModel = mockSelectionModel;
 
-  let extensionUtility: ExtensionUtility;
   let sharedService: SharedService;
   let translateService: TranslateServiceStub;
   let extension: RowMoveManagerExtension;
@@ -52,7 +50,6 @@ describe('rowMoveManagerExtension', () => {
   beforeEach(() => {
     sharedService = new SharedService();
     translateService = new TranslateServiceStub();
-    extensionUtility = new ExtensionUtility(sharedService, translateService);
     extension = new RowMoveManagerExtension(sharedService);
   });
 
@@ -234,6 +231,28 @@ describe('rowMoveManagerExtension', () => {
         expect.anything()
       );
       expect(onBeforeSpy).toHaveBeenCalledWith(expect.anything(), { insertBefore: 3, rows: [1], grid: gridStub });
+      expect(onBeforeSpy).toHaveReturnedWith(undefined);
+      expect(onMoveSpy).not.toHaveBeenCalled();
+    });
+
+    it('should call internal event handler subscribe and expect the "onBeforeMoveRows" option to be called AND have returned with a boolean when original callbacks returns a boolean', () => {
+      const handlerSpy = jest.spyOn(extension.eventHandler, 'subscribe');
+      gridOptionsMock.rowMoveManager.onBeforeMoveRows = () => false;
+      jest.spyOn(SharedService.prototype, 'gridOptions', 'get').mockReturnValue(gridOptionsMock);
+      const onBeforeSpy = jest.spyOn(SharedService.prototype.gridOptions.rowMoveManager as RowMoveManager, 'onBeforeMoveRows');
+      const onMoveSpy = jest.spyOn(SharedService.prototype.gridOptions.rowMoveManager as RowMoveManager, 'onMoveRows');
+
+      const instance = extension.create(columnsMock, gridOptionsMock) as SlickRowMoveManager;
+      extension.register();
+      instance.onBeforeMoveRows.notify({ insertBefore: 3, rows: [1], grid: gridStub }, new Slick.EventData(), gridStub);
+
+      expect(handlerSpy).toHaveBeenCalledTimes(2);
+      expect(handlerSpy).toHaveBeenCalledWith(
+        { notify: expect.anything(), subscribe: expect.anything(), unsubscribe: expect.anything(), },
+        expect.anything()
+      );
+      expect(onBeforeSpy).toHaveBeenCalledWith(expect.anything(), { insertBefore: 3, rows: [1], grid: gridStub });
+      expect(onBeforeSpy).toHaveReturnedWith(false);
       expect(onMoveSpy).not.toHaveBeenCalled();
     });
 

packages/common/src/extensions/rowMoveManagerExtension.ts

@@ -112,7 +112,7 @@ export class RowMoveManagerExtension implements Extension {
         if (onBeforeMoveRowsHandler) {
           (this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onBeforeMoveRowsHandler>>).subscribe(onBeforeMoveRowsHandler, (e, args) => {
             if (this.sharedService.gridOptions.rowMoveManager && typeof this.sharedService.gridOptions.rowMoveManager.onBeforeMoveRows === 'function') {
-              this.sharedService.gridOptions.rowMoveManager.onBeforeMoveRows(e, args);
+              return this.sharedService.gridOptions.rowMoveManager.onBeforeMoveRows(e, args);
             }
           });
         }

packages/common/src/interfaces/rowMoveManager.interface.ts

@@ -13,7 +13,7 @@ export interface RowMoveManager extends RowMoveManagerOption {
   onExtensionRegistered?: (plugin: SlickRowMoveManager) => void;
 
   /** SlickGrid Event fired before the row is moved. */
-  onBeforeMoveRows?: (e: SlickEventData, args: { grid: SlickGrid; rows: number[]; insertBefore: number; }) => void;
+  onBeforeMoveRows?: (e: SlickEventData, args: { grid: SlickGrid; rows: number[]; insertBefore: number; }) => boolean | void;
 
   /** SlickGrid Event fired while the row is moved. */
   onMoveRows?: (e: SlickEventData, args: { grid: SlickGrid; rows: number[]; insertBefore: number; }) => void;