fix(modifyRows): modifyRows uses the new entity when using enableRowHashing

When using enableRowHashing (which is set right now to true as default), modifyRows match the old and the new row but not using the new entity at all.

Credit to @idangozlan for the fix. This PR merely adds unit tests to his [work](#4818).

Author: mportuga

src/js/core/factories/Grid.js

@@ -1066,7 +1066,7 @@ angular.module('ui.grid')
    * @methodOf ui.grid.class:Grid
    * @description returns the GridRow that contains the rowEntity
    * @param {object} rowEntity the gridOptions.data array element instance
-   * @param {array} rows [optional] the rows to look in - if not provided then
+   * @param {array} lookInRows [optional] the rows to look in - if not provided then
    * looks in grid.rows
    */
   Grid.prototype.getRow = function getRow(rowEntity, lookInRows) {
@@ -1131,13 +1131,20 @@ angular.module('ui.grid')
     self.rows.length = 0;
 
     newRawData.forEach( function( newEntity, i ) {
-      var newRow;
+      var newRow, oldRow;
+
       if ( self.options.enableRowHashing ){
         // if hashing is enabled, then this row will be in the hash if we already know about it
-        newRow = oldRowHash.get( newEntity );
+        oldRow = oldRowHash.get( newEntity );
       } else {
         // otherwise, manually search the oldRows to see if we can find this row
-        newRow = self.getRow(newEntity, oldRows);
+        oldRow = self.getRow(newEntity, oldRows);
+      }
+
+      // update newRow to have an entity
+      if ( oldRow ) {
+        newRow = oldRow;
+        newRow.entity = newEntity;
       }
 
       // if we didn't find the row, it must be new, so create it

test/unit/core/factories/Grid.spec.js

@@ -414,86 +414,109 @@ describe('Grid factory', function () {
   });
 
   describe('follow source array', function() {
-    it('should insert it on position 0', function() {
-      var dataRows = [{str:'abc'}];
-      var grid = new Grid({ id: 1 });
+    var dataRows, grid;
 
-      grid.modifyRows(dataRows);
+    beforeEach(function() {
+      dataRows = [{str:'abc'},{str:'cba'},{str:'bac'}];
+      grid = new Grid({ id: 1 });
+      grid.options.enableRowHashing = false;
+
+      spyOn(grid, 'getRow').and.callThrough();
 
+      grid.modifyRows(dataRows);
+    });
 
-      expect(grid.rows.length).toBe(1);
+    it('should update the grid rows', function() {
+      expect(grid.rows.length).toBe(3);
       expect(grid.rows[0].entity.str).toBe('abc');
+      expect(grid.rows[1].entity.str).toBe('cba');
+      expect(grid.rows[2].entity.str).toBe('bac');
+    });
 
+    it('should insert it on position 0', function() {
       dataRows.splice(0,0,{str:'cba'});
       grid.modifyRows(dataRows);
 
-      expect(grid.rows.length).toBe(2);
+      expect(grid.getRow).toHaveBeenCalled();
+      expect(grid.rows.length).toBe(4);
       expect(grid.rows[0].entity.str).toBe('cba');
     });
 
     it('should swap', function() {
-      var dataRows = [{str:'abc'},{str:'cba'}];
-      var grid = new Grid({ id: 1 });
-
-      grid.modifyRows(dataRows);
-
-      expect(grid.rows[0].entity.str).toBe('abc');
-      expect(grid.rows[1].entity.str).toBe('cba');
-
       var tmpRow = dataRows[0];
+
       dataRows[0] = dataRows[1];
       dataRows[1] = tmpRow;
       grid.modifyRows(dataRows);
 
+      expect(grid.getRow).toHaveBeenCalled();
       expect(grid.rows[0].entity.str).toBe('cba');
       expect(grid.rows[1].entity.str).toBe('abc');
     });
 
     it('should delete and insert new in the middle', function() {
-      var dataRows = [{str:'abc'},{str:'cba'},{str:'bac'}];
-      var grid = new Grid({ id: 1 });
-
-      grid.modifyRows(dataRows);
-
-      expect(grid.rows.length).toBe(3);
-      expect(grid.rows[0].entity.str).toBe('abc');
-      expect(grid.rows[1].entity.str).toBe('cba');
-      expect(grid.rows[2].entity.str).toBe('bac');
-
       dataRows[1] = {str:'xyz'};
       grid.modifyRows(dataRows);
 
+      expect(grid.getRow).toHaveBeenCalled();
       expect(grid.rows.length).toBe(3);
       expect(grid.rows[0].entity.str).toBe('abc');
       expect(grid.rows[1].entity.str).toBe('xyz');
       expect(grid.rows[2].entity.str).toBe('bac');
     });
+  });
+
+  describe('when row hashing is enabled', function() {
+    var dataRows, grid;
+
+    beforeEach(function() {
+      dataRows = [{str:'abc'},{str:'cba'},{str:'bac'}];
+      grid = new Grid({ id: 1 });
+      grid.options.enableRowHashing = true;
+
+      spyOn(grid, 'getRow').and.callThrough();
 
-    /*
-     * No longer trying to keep order of sort - we run rowsProcessors
-     * immediately after anyway, which will resort.
-     *
-    it('should keep the order of the sort', function() {
-      var dataRows = [{str:'abc'},{str:'cba'},{str:'bac'}];
-      var grid = new Grid({ id: 1 });
-      grid.options.columnDefs = [{name:'1',type:'string'}];
-      grid.buildColumns();
       grid.modifyRows(dataRows);
+    });
 
+    it('should update the grid rows', function() {
       expect(grid.rows.length).toBe(3);
       expect(grid.rows[0].entity.str).toBe('abc');
       expect(grid.rows[1].entity.str).toBe('cba');
       expect(grid.rows[2].entity.str).toBe('bac');
+    });
 
-      grid.sortColumn(grid.columns[0]);
-
-      dataRows.splice(0,0,{str:'xyz'});
+    it('should insert it on position 0', function() {
+      dataRows.splice(0,0,{str:'cba'});
       grid.modifyRows(dataRows);
+
+      expect(grid.getRow).not.toHaveBeenCalled();
       expect(grid.rows.length).toBe(4);
+      expect(grid.rows[0].entity.str).toBe('cba');
+    });
+
+    it('should swap', function() {
+      var tmpRow = dataRows[0];
+
+      dataRows[0] = dataRows[1];
+      dataRows[1] = tmpRow;
+      grid.modifyRows(dataRows);
+
+      expect(grid.getRow).not.toHaveBeenCalled();
+      expect(grid.rows[0].entity.str).toBe('cba');
+      expect(grid.rows[1].entity.str).toBe('abc');
+    });
+
+    it('should delete and insert new in the middle', function() {
+      dataRows[1] = {str:'xyz'};
+      grid.modifyRows(dataRows);
+
+      expect(grid.getRow).not.toHaveBeenCalled();
+      expect(grid.rows.length).toBe(3);
       expect(grid.rows[0].entity.str).toBe('abc');
-      expect(grid.rows[3].entity.str).toBe('xyz');
+      expect(grid.rows[1].entity.str).toBe('xyz');
+      expect(grid.rows[2].entity.str).toBe('bac');
     });
-    */
   });
 
   describe('binding', function() {