Merge pull request #354 from cidgoh/bin-behaviour-fix

Bin behaviour fix

lib/DataHarmonizer.js

@@ -820,6 +820,7 @@ class DataHarmonizer {
         if (!isValidHeaderRow(matrix, specifiedHeaderRow)) {
           $('#specify-headers-err-msg').show();
         } else {
+          // Try to load data again using User specified header row
           const mappedMatrixObj = self.mapMatrixToGrid(
             matrix,
             specifiedHeaderRow - 1
@@ -1683,6 +1684,8 @@ class DataHarmonizer {
    * by a user edit on a field cell. This creates complexity for fields that
    * work together, e.g. either of first two fields of
    * [field][field unit][field bin] could have been focus of change.
+   * Code would be simpler if we could just pass data row reference to
+   * binChangeTest() but instead we have to construct matrix[row] ={cols ...}
    *
    * @param {Array} change array [row, col, ? , value]
    * @param {Object} fields See `data.js`.
@@ -1703,9 +1706,11 @@ class DataHarmonizer {
     // current one.
     if (fields.length > col + 1) {
       // We're prepping a SPARSE ARRAY here for binChangeTest()
-      var matrix = [0];
-      matrix[0] = {}; // Essential for creating sparse array.
-      matrix[0][col] = change[3]; // prime changed value
+      // Its a string because currently
+      let matrix = [];
+      matrix[row] = {}; // Essential for creating sparse array.
+      let matrixRow = matrix[row];
+      matrixRow[col] = change[3]; // prime changed value
 
       const prevName = col > 0 ? fields[col - 1].title : null;
       const nextName = fields.length > col + 1 ? fields[col + 1].title : null;
@@ -1728,15 +1733,17 @@ class DataHarmonizer {
         const nextNextName =
           fields.length > col + 2 ? fields[col + 2].title : null;
         if (nextNextName === field.title + ' bin') {
-          matrix[0][col + 1] = this.hot.getDataAtCell(row, col + 1); //prime unit
-          this.binChangeTest(matrix, row, col, fields, 2, triggered_changes);
+          matrixRow[col + 1] = this.hot.getDataAtCell(row, col + 1); //prime unit
+          matrixRow[col + 2] = this.hot.getDataAtCell(row, col + 2); //prime bin
+          this.binChangeTest(matrix, col, fields, 2, triggered_changes);
           return;
         }
       }
 
       // Match <field>[field bin]
       if (nextName === field.title + ' bin') {
-        this.binChangeTest(matrix, row, col, fields, 1, triggered_changes);
+        matrixRow[col + 1] = this.hot.getDataAtCell(row, col + 1); //prime bin
+        this.binChangeTest(matrix, col, fields, 1, triggered_changes);
         return;
       }
 
@@ -1745,15 +1752,9 @@ class DataHarmonizer {
         // Match [field]<field unit>[field bin]
         if (prevName + ' bin' === nextName) {
           // trigger reevaluation of bin from field
-          matrix[0][col - 1] = this.hot.getDataAtCell(row, col - 1);
-          this.binChangeTest(
-            matrix,
-            row,
-            col - 1,
-            fields,
-            2,
-            triggered_changes
-          );
+          matrixRow[col - 1] = this.hot.getDataAtCell(row, col - 1);
+          matrixRow[col] = this.hot.getDataAtCell(row, col);
+          this.binChangeTest(matrix, col - 1, fields, 2, triggered_changes);
           return;
         }
 
@@ -1769,7 +1770,7 @@ class DataHarmonizer {
           // previously had to block x/y/z with  && dateString.indexOf('/') === -1
           if (dateString) {
             dateString = this.setDateChange(change[3], dateString);
-            matrix[0][col - 1] = dateString;
+            matrixRow[col - 1] = dateString;
             triggered_changes.push([row, col - 1, undefined, dateString]);
           }
           return;
@@ -1787,12 +1788,10 @@ class DataHarmonizer {
    * This is useful when calling `hot.loadData`, as cell changes from said method
    * are not recognized by `afterChange`.
    * @param {Array<Array<String>>} matrix Data meant for grid.
-   * @param {Object} hot Handsontable instance of grid.
-   * @param {Object} data See `data.js`.
    * @return {Array<Array<String>>} Modified matrix.
    */
-  matrixFieldChangeRules(matrix, hot, data) {
-    const fields = this.getFields(data);
+  matrixFieldChangeRules(matrix) {
+    const fields = this.getFields();
     for (let col = 0; col < fields.length; col++) {
       const field = fields[col];
 
@@ -1810,10 +1809,10 @@ class DataHarmonizer {
       if (fields.length > col + 1) {
         const nexttitle = fields[col + 1].title;
 
-        // Rule: for any "x bin" field label, following a "x" field,
+        // Rule: for any <x>[x bin] pattern of field names,
         // find and set appropriate bin selection.
         if (nexttitle === field.title + ' bin') {
-          this.binChangeTest(matrix, 0, col, fields, 1, triggered_changes);
+          this.binChangeTest(matrix, col, fields, 1, triggered_changes);
         }
         // Rule: for any [x], [x unit], [x bin] series of fields
         else if (nexttitle === field.title + ' unit') {
@@ -1829,9 +1828,12 @@ class DataHarmonizer {
                 );
               }
             }
-          } else if (fieldUnitBinTest(fields, col)) {
+          }
+          //
+          else if (fieldUnitBinTest(fields, col)) {
             // 2 specifies bin offset
-            this.binChangeTest(matrix, 0, col, fields, 2, triggered_changes);
+            // Matrix operations have 0 for binOffset
+            this.binChangeTest(matrix, col, fields, 2, triggered_changes);
           }
         }
       }
@@ -1878,17 +1880,23 @@ class DataHarmonizer {
 
   /**
    * Test [field],[field bin] or [field],[field unit],[field bin] combinations
-   * to see if bin update needed.
-   * @param {Array<Array<String>>} matrix Data meant for grid.
-   * @param {Integer} rowOffset
+   * to see if bin update needed.  Outputs any changes required into
+   * triggered_changes array.
+   * @param {Array<Array<String>>} matrix of data row(s) to test.
    * @param {Integer} col column of numeric field.
    * @param {Object} fields See `data.js`.
    * @param {Integer} binOffset column of bin field.
    * @param {Array} triggered_changes array of change which is appended to changes.
    */
-  binChangeTest(matrix, rowOffset, col, fields, binOffset, triggered_changes) {
+  binChangeTest(matrix, col, fields, binOffset, triggered_changes) {
     for (let row in matrix) {
+      // NOTE: row is string!  Adding offsets requires parseInt(row)
+      const hotRowPtr = parseInt(row); // + rowOffset;
+      console.log('row', row, hotRowPtr);
+      const hotRowBinCol = parseInt(col) + binOffset;
+      const hotRowNextCol = parseInt(col) + 1;
       const value = matrix[row][col];
+
       // For IMPORT, this is only run on fields that have a value.
       // Note matrix pass cell by reference so its content can be changed.
       if (value && value.length > 0) {
@@ -1899,7 +1907,7 @@ class DataHarmonizer {
         if (number >= 0) {
           // Here we have the 3 field call, with units sandwitched in the middle
           if (binOffset === 2) {
-            const unit = matrix[row][col + 1];
+            const unit = matrix[row][hotRowNextCol];
             // Host age unit is interpreted by default to be year.
             // If user selects month, value is converted into years for binning.
             // Future solution won't hardcode month / year assumption
@@ -1914,7 +1922,7 @@ class DataHarmonizer {
             //}
           }
           // .flatVocabulary is an array of string bin ranges e.g. "10 - 19"
-          for (const number_range of fields[col + binOffset].flatVocabulary) {
+          for (const number_range of fields[hotRowBinCol].flatVocabulary) {
             // ParseInt just looks at first part of number
             if (number >= parseFloat(number_range)) {
               selection = number_range;
@@ -1926,33 +1934,33 @@ class DataHarmonizer {
           // Integer/date field is a textual value, possibly a metadata 'Missing'
           // etc. If bin field has a value, leave it unchanged; but if it doesn't
           // then populate bin with input field metadata status too.
-          const bin_value = this.hot.getDataAtCell(rowOffset, col + binOffset);
+          const matrixRow = matrix[hotRowPtr];
+          const bin_value =
+            typeof matrixRow !== 'undefined' ? matrixRow[hotRowBinCol] : null;
           selection = bin_value; // Default value is itself.
 
-          const bin_values = fields[col + binOffset].flatVocabulary;
+          const bin_values = fields[hotRowBinCol].flatVocabulary;
           if (!bin_value || (bin_value === '' && value in bin_values)) {
             selection = value;
           }
           // If a unit field exists, then set that to metadata too.
           if (binOffset == 2) {
-            const unit_value = this.hot.getDataAtCell(rowOffset, col + 1);
+            const unit_value =
+              typeof matrix[hotRowPtr] !== 'undefined'
+                ? matrix[hotRowPtr][hotRowNextCol]
+                : null;
             const unit_values = fields[col + 1].flatVocabulary;
             if (!unit_value || (unit_value === '' && value in unit_values)) {
               triggered_changes.push([
-                rowOffset + parseInt(row),
-                col + 1,
+                hotRowPtr,
+                hotRowNextCol,
                 undefined,
                 value,
               ]);
             }
           }
         }
-        triggered_changes.push([
-          rowOffset + parseInt(row),
-          col + binOffset,
-          undefined,
-          selection,
-        ]);
+        triggered_changes.push([hotRowPtr, hotRowBinCol, undefined, selection]);
       }
     }
   }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "data-harmonizer",
-  "version": "1.3.5",
+  "version": "1.3.6",
   "description": "A standardized spreadsheet editor and validator that can be run offline and locally",
   "repository": "git@github.com:cidgoh/DataHarmonizer.git",
   "license": "MIT",