Add spec for delta builder

frontend/karma.conf.js

@@ -16,7 +16,6 @@ module.exports = function (config) {
     plugins: [
       require('karma-jasmine'),
       require('karma-chrome-launcher'),
-      require('karma-jasmine-html-reporter'),
       require('karma-coverage-istanbul-reporter'),
       require('@angular-devkit/build-angular/plugins/karma')
     ],
@@ -30,7 +29,7 @@ module.exports = function (config) {
     angularCli: {
       environment: 'dev'
     },
-    reporters: ['progress', 'kjhtml'],
+    reporters: ['progress'],
     port: 9876,
     colors: true,
     failOnEmptyTestSuite: true,

frontend/src/app/modules/common/drag-and-drop/reorder-delta-builder.spec.ts

@@ -0,0 +1,244 @@
+// -- copyright
+// OpenProject is a project management system.
+// Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License version 3.
+//
+// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+// Copyright (C) 2006-2013 Jean-Philippe Lang
+// Copyright (C) 2010-2013 the ChiliProject Team
+//
+// This program is free software; you can redistribute it and/or
+// modify it under the terms of the GNU General Public License
+// as published by the Free Software Foundation; either version 2
+// of the License, or (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//
+// See doc/COPYRIGHT.rdoc for more details.
+// ++
+
+import {QueryOrder} from "core-app/modules/hal/dm-services/query-order-dm.service";
+import {
+  DEFAULT_ORDER,
+  MAX_ORDER,
+  ReorderDeltaBuilder
+} from "core-app/modules/common/drag-and-drop/reorder-delta-builder";
+
+describe('ReorderDeltaBuilder', () => {
+  const work_packages:string[] = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10'];
+
+  function buildDelta(
+    wpId:string,
+    positions:QueryOrder,
+    wps:string[] = work_packages,
+    fromIndex:number|null = null) {
+    // As work_packages is already the list with moved element, simply compute the index
+    let index:number = work_packages.indexOf(wpId);
+
+    if (index === -1) {
+      throw "Invalid wpId given for work_packages, must be contained.";
+    }
+    return new ReorderDeltaBuilder(wps, positions, wpId, index, fromIndex).buildDelta();
+  }
+
+  it('Empty, inserting at beginning sets the delta for wpId 1 to the default value', () => {
+    let delta = buildDelta('1', {});
+    expect(Object.keys(delta)).toEqual(['1']);
+    expect(delta['1']).toEqual(DEFAULT_ORDER);
+  });
+
+  it('Empty, inserting at end sets the delta for all predecessors', () => {
+    let delta = buildDelta('10', {});
+    expect(Object.keys(delta).length).toEqual(work_packages.length);
+    expect(delta).toEqual({
+      '1': 0,
+      '2': 16384,
+      '3': 32768,
+      '4': 49152,
+      '5': 65536,
+      '6': 81920,
+      '7': 98304,
+      '8': 114688,
+      '9': 131072,
+      '10': 139264 // 131072 + ORDER_DISTANCE/2
+    });
+  });
+
+  it('Empty, inserting at end middle the delta for all predecessors', () => {
+    let delta = buildDelta('5', {});
+    expect(Object.keys(delta).length).toEqual(5);
+    expect(delta).toEqual({
+      '1': 0,
+      '2': 16384,
+      '3': 32768,
+      '4': 49152,
+      '5': 57344 // 49152 + 8192
+    });
+  });
+
+  it('has no problems inserting in the beginning old sort oder (1..n)', () => {
+    let positions = {
+      '5': 0,
+      '2': 1,
+      '3': 2,
+      '4': 3,
+      '1': 4,
+      '6': 5,
+      '7': 6,
+      '8': 7,
+      '9': 8,
+      '10': 9,
+    };
+
+    let delta = buildDelta('1', positions);
+
+    expect(Object.keys(delta).length).toEqual(1);
+    // Expected to set to 1(position of 2) - 8192(half default distance)
+    expect(delta['1']).toEqual(-8191);
+  });
+
+  it('has no problems inserting in the middle of old sort oder (1..n)', () => {
+    let positions = {
+      '1': 0,
+      '2': 1,
+      '3': 2,
+      '4': 4,
+      '6': 5,
+      '7': 6,
+      '8': 7,
+      '9': 8,
+      '10': 9,
+    };
+
+    let delta = buildDelta('5', positions);
+
+    // 0 does not have to be updated
+    expect(Object.keys(delta).length).toEqual(9);
+    expect(delta).toEqual({
+      '2': 1,
+      '3': 2,
+      '4': 3,
+      '5': 4,
+      '6': 5,
+      '7': 6,
+      '8': 7,
+      '9': 8,
+      '10': 9
+    });
+  });
+
+  it('will reorder old sort if there is not enough data', () => {
+    let positions = {
+      '1': 0,
+      '3': 1,
+      '4': 2,
+    };
+
+    let delta = buildDelta('2', positions);
+
+    expect(Object.keys(delta).length).toEqual(9);
+    expect(delta).toEqual({
+      '2': 1,
+      '3': 2,
+      '4': 3,
+      '5': 4,
+      '6': 5,
+      '7': 6,
+      '8': 7,
+      '9': 8,
+      '10': 9,
+    });
+  });
+
+  it('will shift min position when successor is max', () => {
+    let positions = {
+      '1': DEFAULT_ORDER,
+      '2': MAX_ORDER - 1,
+      '4': MAX_ORDER,
+    };
+
+    let delta = buildDelta('3', positions, ['1', '2', '3', '4']);
+    expect(Object.keys(delta).length).toEqual(3);
+    expect(delta).toEqual({
+      // 1 remains at DEFAULT_ORDER
+      // rest is evenly spaced until MAX
+      '2': 715827882,
+      // 715827882 * 2
+      '3': 1431655764,
+      // 715827882 * 3
+      // gets reassigned due to integer floor()
+      '4': 2147483646
+    });
+  });
+
+  it('will shift first position back when predecessor is max', () => {
+    let positions = {
+      '1': MAX_ORDER - 1,
+      '2': MAX_ORDER,
+    };
+
+    let delta = buildDelta('3', positions, ['1', '2', '3']);
+    expect(Object.keys(delta).length).toEqual(3);
+    expect(delta).toEqual({
+      '1': MAX_ORDER - 4,
+      '2': MAX_ORDER - 2,
+      '3': MAX_ORDER
+    });
+  });
+
+  it('with successor position, sets the delta for wpId 1 to the default value', () => {
+    let positions = { '2': DEFAULT_ORDER };
+    let delta = buildDelta('1', positions);
+
+    expect(Object.keys(delta)).toEqual(['1']);
+    // expect position to be ORDER_DISTANCE/2 before successor position
+    expect(delta['1']).toEqual(-8192);
+  });
+
+  it('fills in all predecessors when inserting at index > 0', () => {
+    let positions = {};
+    let delta = buildDelta('2', positions);
+
+    expect(Object.keys(delta)).toEqual(['1', '2']);
+    // expect position to be ORDER_DISTANCE/2 before successor position
+    expect(delta['1']).toEqual(0);
+    expect(delta['2']).toEqual(8192);
+  });
+
+  it('just sets the default when order contains wpId only', () => {
+    let positions = {};
+    let delta = buildDelta('1', positions, ['1']);
+
+    expect(Object.keys(delta)).toEqual(['1']);
+    expect(delta['1']).toEqual(0);
+  });
+
+  it('just shifts two values when index <- 1 -> fromIndex', () => {
+    let positions = { '1': 8192, '2': 0, '3': 16384};
+    // From index 1 to 0
+    let delta = buildDelta('1', positions, ['1', '2', '3'], 1);
+
+    expect(Object.keys(delta)).toEqual(['1', '2']);
+    expect(delta['1']).toEqual(0);
+    expect(delta['2']).toEqual(8192);
+  });
+
+  // Move to where successor has undefined position
+  it('adds to the predecessor if successor has no position', () => {
+    let positions = { '1': 0 };
+    let delta = buildDelta('2', positions, ['1', '2', '3']);
+
+    expect(Object.keys(delta)).toEqual(['2']);
+    expect(delta['2']).toEqual(8192);
+  });
+
+});

frontend/src/app/modules/common/drag-and-drop/reorder-delta-builder.ts

@@ -1,5 +1,6 @@
 import {QueryOrder} from "core-app/modules/hal/dm-services/query-order-dm.service";
 import {debugLog, timeOutput} from "core-app/helpers/debug_output";
+import {debug} from "util";
 
 // min allowed position
 export const MIN_ORDER = -2147483647;
@@ -38,7 +39,7 @@ export class ReorderDeltaBuilder {
   }
 
   public buildDelta():QueryOrder {
-    timeOutput("Building delta", () => this.buildInsertPosition());
+    timeOutput(`Building delta for ${this.wpId}@${this.index}`, () => this.buildInsertPosition());
 
     debugLog("Order DELTA was built as %O", this.delta);
 
@@ -66,16 +67,28 @@ export class ReorderDeltaBuilder {
     // Ensure previous positions exist so we can insert wpId @ index
     const predecessorPosition = this.buildUpPredecessorPosition();
 
+    // Ensure we reorder when predecessor is at max already
+    if (predecessorPosition >= MAX_ORDER) {
+      debugLog(`Predecessor position is at max order, need to reorder`);
+      return this.reorderedInsert();
+    }
+
     // Get the actual successor position, it might vary wildly from the optimal position
     const successorPosition = this.positionFor(this.index + 1);
 
     if (successorPosition === undefined) {
       // Successor does not have a position yet (is NULL), any position will work
-      // so let's use the optimal one.
+      // so let's use the optimal one which is halfway to a potential successor
       this.delta[this.wpId] = predecessorPosition + (ORDER_DISTANCE / 2);
       return;
     }
 
+    // Ensure we reorder when successor is at max already
+    if (successorPosition >= MAX_ORDER) {
+      debugLog(`Successor position is at max order, need to reorder`);
+      return this.reorderedInsert();
+    }
+
     // successor exists and has a position
     // We will want to insert at the half way from predecessorPosition ... successorPosition
     const distance = Math.floor((successorPosition - predecessorPosition) / 2);
@@ -177,19 +190,24 @@ export class ReorderDeltaBuilder {
 
     // Get the current distance between orders
     // Both must be set by now due to +buildUpPredecessorPosition+ having run.
-    let min = this.minPosition!;
-    let max = this.maxPosition!;
+    let [minWpId, min] = this.minPosition!;
+    let [maxWpId, max] = this.maxPosition!;
+
 
     // We can keep min and max orders if distance/(items to distribute) >= 1
     let space = Math.floor((max - min) / (itemsToDistribute - 1));
 
+    debugLog(`Min=${min}  Max=${max}, items=${itemsToDistribute}, space=${space}`)
+
     // If no space is left, first try to add to the max item
     // Or subtract from the min item
     if (space < 1) {
       if ((max + itemsToDistribute) <= MAX_ORDER) {
         max += itemsToDistribute;
+        this.delta[maxWpId] = max;
       } else if ((min - itemsToDistribute) >= MIN_ORDER) {
         min -= itemsToDistribute;
+        this.delta[minWpId] = min;
       } else {
         // This should not happen in a 4-byte integer with our frontend
         throw "Elements cannot be moved further and no space is left. Too many elements";
@@ -199,16 +217,11 @@ export class ReorderDeltaBuilder {
       space = Math.floor((max - min) / (itemsToDistribute - 1));
     }
 
+    debugLog(`Min=${min}  Max=${max}, items=${itemsToDistribute}, space=${space}`)
+
     // Assign positions for all values in between min/max
     for (let i = 1; i < itemsToDistribute; i++) {
       const wpId = this.order[i];
-
-      // If we reached a point where the position is undefined
-      // and larger than our point of insertion, we can keep them this way
-      if (i > this.index && this.livePosition(wpId) === undefined) {
-        return;
-      }
-
       this.delta[wpId] = min + (i * space);
     }
   }
@@ -216,22 +229,23 @@ export class ReorderDeltaBuilder {
   /**
    * Returns the minimal position assigned currently
    */
-  private get minPosition():number|undefined {
+  private get minPosition():[string, number] {
     const wpId = this.order[0]!;
-    return this.livePosition(wpId);
+    return [wpId, this.livePosition(wpId)!];
   }
 
   /**
    * Returns the maximum position assigned currently.
    * Note that a list can be unpositioned at the beginning, so this may return undefined
    */
-  private get maxPosition():number|undefined {
+  private get maxPosition():[string, number]|undefined {
     for (let i = this.order.length - 1; i >= 0; i--) {
-      let position = this.livePosition(this.order[i]);
+      let wpId = this.order[i];
+      let position = this.livePosition(wpId);
 
       // Return the first set position.
       if (position !== undefined) {
-        return position;
+        return [wpId, position];
       }
     }