switched algorithm to use data-field-name attribute on dragged td items

Author: ddooley

docs/README_user_manual.md

@@ -243,6 +243,16 @@ Select the profile in the dropdown, then click **Delete profile**. The profile i
 
 Click **Reset** to discard any drag-drop changes made in the current session and restore the table to its auto-detected state (as it appeared when the file was first loaded).
 
+### Limitation — JSON files must have consistent field names across all records
+
+When loading a JSON data file, DataHarmonizer builds the column header by scanning every record (row) in each table and collecting the union of all field names encountered. This works correctly when every record in a given class/table uses the same set of field names.
+
+However, if some records contain a typo or variant spelling of a field name — for example because records were merged in from another source — the union will contain **both** the correct name and the mistyped one. The correctly named field will match the schema and its data will load normally. The mistyped name will appear in the Field Mapper report as an unmatched data column (visible when the **Concise view** checkbox is off).
+
+This situation cannot be resolved through the Field Mapper: two distinct data columns cannot both be mapped to the same schema field, and even if they could, the per-record ambiguity (which name applies to which record) has no clean resolution at load time.
+
+**Remedy:** fix the JSON file directly so that every record of a given class uses identical field names before loading it into DataHarmonizer.
+
 ---
 
 ## Clearing Data

lib/FieldMapper.js

@@ -26,6 +26,22 @@
  * Incomming data file might not have section headers, so don't make
  * code dependent on them.
  *
+ * Assumption — JSON field name consistency across records:
+ * When building the data header from a JSON file, all records (rows) of a
+ * given class/table are scanned and their field names are unioned together.
+ * This works correctly when all records use the same field names. However,
+ * if some records have a typo in a field name (e.g. "sample_collector_sample_idz"
+ * instead of "sample_collector_sample_id"), perhaps introduced by merging records
+ * from another data file, the union of field names will contain BOTH the correct
+ * and the mistyped name. The correct name will match the schema and be loaded;
+ * the mistyped name will appear in the FieldMapper report as an unmatched data
+ * field (visible when the "concise" checkbox is off). There is no practical UI
+ * mechanism to resolve this: two distinct data fields cannot be merged into one
+ * schema slot, and even if a user could specify such a mapping the per-record
+ * ambiguity (correct name in some records, typo in others) has no clean
+ * resolution at load time. The recommended remedy is to fix the source JSON
+ * file so that all records of a class share identical field names.
+ *
  * localStorage "DataHarmonizer" stores all objects via a YAML string.
  *    - DataHarmonizer.schema = {} object holding schema name keys. Current
  *        loaded schema name key will be added if working on field-mappings. 
@@ -42,7 +58,8 @@ import $ from 'jquery';
 import {readBrowserDHSettings, saveBrowserDHSettings, clearDH} from './Toolbar';
 import YAML from 'yaml';
 import { updateSheetRange } from '../lib/utils/files';
-import { dataObjectToArray } from '../lib/utils/fields';
+// dataObjectToArray no longer used (JSON rows pre-converted to arrays in openJSONDataFile)
+// import { dataObjectToArray } from '../lib/utils/fields';
 import { utils as XlsxUtils } from 'xlsx/xlsx.js';
 
 // A call like fm = new FieldMapper().bind(this), provides caller environment
@@ -184,7 +201,7 @@ export class FieldMapper {
         // as possible
         dh.slot_names.forEach((slot_name, index) => {
           let value = null;
-          if (index in slot_to_data_col) {
+          if (index in slot_to_data_col && slot_to_data_col[index] >= 0) {
             value = row[slot_to_data_col[index]];
           }
           new_row.push(value);
@@ -196,24 +213,14 @@ export class FieldMapper {
           // Overwrite any (empty) fields with user-defined column mapping.
           Object.entries(map_obj?.map || {}).forEach(([ptr, mapping]) => {
             const col_from = data_field_to_col[mapping.from];
-            const col_to = dh.slot_title_to_column[mapping.to];
+            const col_to = dh.slot_name_to_column[mapping.to];
             new_row[col_to] = row[col_from];
           });
         } 
 
-        // if JSON, then examine some slot's datatype fields and overwrite with
-        // new converted values.  
-        // Determine if this is still needed.
-        if (this.file.ext === 'json') {
-          new_table[row_ptr] = dataObjectToArray(new_row, dh, {
-            serializedDateFormat: this.dateExportBehavior,
-            dateFormat: this.dateFormat,// Probably NULL!
-            datetimeFormat: this.datetimeFormat, // Probably NULL!
-            timeFormat: this.timeFormat,// Probably NULL!
-          });
-        }
-        else
-          new_table.push(new_row);
+        // JSON rows are now pre-converted to value arrays in openJSONDataFile,
+        // so push new_row the same way as tabular data.
+        new_table.push(new_row);
 
       })
 
@@ -244,7 +251,7 @@ export class FieldMapper {
    * @param {Integer} header_row which is 1 more than actual row (natural number).
    * @param {String} file_name name of data file user selected for loading.
    */
-  appendFieldMappingModal(dh) {
+  appendFieldMappingModal(dh, append_html = true) {
 
     const data_fields = this.data[dh.template.name].header;
 
@@ -258,8 +265,8 @@ export class FieldMapper {
 
     // FUTURE: PROTECT AGAINST DUPLICATE FIELD NAMES IN DATA FILE.
     // Preliminary scan for all matches via ordered slot_names array
-    let slot_matches = new Array(dh.slot_names.length).fill(false);
-    let data_matches = new Array(data_fields.length).fill(false);
+    let slot_matches = new Array(dh.slot_names.length).fill(-1);
+    let data_matches = new Array(data_fields.length).fill(-1);
     //let found_by_title = false;
     dh.slot_names.forEach((slot_name, index) => {
       if (slot_name in data_field_to_col) { // JSON data matches on slot.name
@@ -284,7 +291,10 @@ export class FieldMapper {
     this.data[dh.template.name].slot_to_data_col_matches = slot_matches;
     //this.data[dh.template.name].data_matches = data_matches;
 
-    // Display template/tab/class (i.e. this call to extend content is 
+    // When called for a perfectly-matched file, skip HTML generation.
+    if (!append_html) return;
+
+    // Display template/tab/class (i.e. this call to extend content is
     // dedicated to that content.
     let html = `
       <tbody class="field-mapping-template">
@@ -307,18 +317,38 @@ export class FieldMapper {
           </tr>
       `;
 
-      let old_match_data_row = null;
+      // last_shown_data_row tracks the last data file column index displayed.
+      // Starting at -1 so the first pre-match check begins at data column 0.
+      let last_shown_data_row = -1;
+
       Object.entries(section.children).forEach(([section_slot_row, slot]) => {
 
-        if (slot_matches[slot_row] !== false && slot_matches[slot_row] >= 0) {
+        if (slot_matches[slot_row] >= 0) {
           const data_row = slot_matches[slot_row];
-          old_match_data_row = data_row;
-          // HERE slots are always displayed by their title (for multilingual sanity?)
-          const ordering = (slot_row != data_row) ? `<span class="reordered">${data_row}</span>` : data_row
+
+          // Show any unmatched data fields that appear BEFORE this slot's
+          // match position. This ensures data columns skipped before the
+          // first match (or between out-of-order matches) are not lost.
+          for (let dr = last_shown_data_row + 1; dr < data_row; dr++) {
+            if (data_matches[dr] < 0 && !done_data_row[dr]) {
+              done_data_row[dr] = true;
+              html += `
+              <tr class="field-mismatch">
+                <td>&nbsp;</td>
+                <td class="draggable-mapping-item field-mismatch" data-field-name="${data_fields[dr]}">${dr}) ${data_fields[dr]}</td>
+              </tr>`;
+              last_shown_data_row = dr;
+            }
+          }
+
+          // Show the matched slot row. data-slot-name and data-field-name
+          // attributes are used by getProfileMapping() instead of text parsing.
+          last_shown_data_row = data_row;
+          const ordering = (slot_row != data_row) ? `<span class="reordered">${data_row}</span>` : data_row;
           html += `
           <tr class="field-match">
-            <td>${slot_row}) ${slot.title}</td>
-            <td class="field-match">${ordering}) ${data_fields[data_row]}</td>
+            <td data-slot-name="${slot.name}">${slot_row}) ${slot.title}</td>
+            <td class="field-match" data-field-name="${data_fields[data_row]}">${ordering}) ${data_fields[data_row]}</td>
           </tr>`;
 
         }
@@ -327,33 +357,24 @@ export class FieldMapper {
           // Do slot side's mismatched item.
           html += `
           <tr class="field-mismatch">
-            <td class="field-mismatch">${slot_row}) ${slot.title}</td>
-            <td class="draggable-mapping-item field-mismatch"></td>
+            <td class="field-mismatch" data-slot-name="${slot.name}">${slot_row}) ${slot.title}</td>
+            <td class="draggable-mapping-item field-mismatch" data-field-name=""></td>
           </tr>`;
-        }
 
-        // If we don't have a match, then we have to ensure all the 
-        // mismatched field headers for both tables are provided until
-        // the next match. (If the data table had columns whose order is 
-        // completely different, then semantically the template section
-        // slots will have unrelated fields near them, c'est la vie.)
-
-        // Possibility that next data row(s) are not a match so squeeze them
-        // in here.
-        // Add each data_matches' entry between last match and next one.
-        let data_row = (old_match_data_row || -1) + 1;
-        while (data_row < data_matches.length 
-          && (data_matches[data_row] === false
-          && ! done_data_row[data_row])
-        ){
-            done_data_row[data_row] = true; // Best way to handle this case? 
-            const ordering = slot_row != data_row ? 'reordered' : ''
-          html += `
-          <tr class="field-mismatch">
-            <td>&nbsp;</td>
-            <td class="draggable-mapping-item field-mismatch">${data_row}) ${data_fields[data_row]}</td>
-          </tr>`;
-          data_row += 1;
+          // Show unmatched data fields after last shown, up to the next match.
+          let dr = last_shown_data_row + 1;
+          while (dr < data_matches.length
+            && (data_matches[dr] < 0 && !done_data_row[dr])
+          ) {
+            done_data_row[dr] = true;
+            html += `
+            <tr class="field-mismatch">
+              <td>&nbsp;</td>
+              <td class="draggable-mapping-item field-mismatch" data-field-name="${data_fields[dr]}">${dr}) ${data_fields[dr]}</td>
+            </tr>`;
+            last_shown_data_row = dr;
+            dr++;
+          }
         }
 
         slot_row += 1;
@@ -423,43 +444,36 @@ export class FieldMapper {
    */
   getProfileMapping() {
 
-    function get_label (nmstr) {
-      const i = nmstr.indexOf(' ')+1; 
-      return (i ? nmstr.slice(i) : '')
-    };
-    // In each tbody[data-table] section that has a template_name, look for
-    // any tr which has a "field-mismatch" attribute. That tr first td will 
-    // have slotname of schema/template.
-    // Each 
+    // Read slot name and data field name from data attributes rather than
+    // parsing the visible "N) fieldname" text, which was fragile.
     const schema = this.context.getSchemaRef();
     let mapping = {
       schema_version: schema.version,
-      tables: {} 
+      tables: {}
     };
 
     $('table#field-mapping-table tbody[data-table]').each((t_index, tbody) => {
 
       // A given schema class often has multiple tbody, each for a data-table section
       const table_name = $(tbody).attr('data-table');
 
-      mapping.tables[table_name]??= {}; // init empty value if it doesn't exist.
-      $(tbody).find('td:first-child.field-mismatch').each((index, slot_field) => {
-        // Retrieve labels, get past column id) prefix;
-        const data_label = get_label($(slot_field).next('td').text());
-        if (data_label.length > 0) {// target has a mapping in it.
+      mapping.tables[table_name] ??= {};
+      // Only look at unmatched slot rows (left cell has data-slot-name).
+      $(tbody).find('td:first-child.field-mismatch[data-slot-name]').each((index, slot_field) => {
+        const data_field_name = $(slot_field).next('td').attr('data-field-name');
+        if (data_field_name && data_field_name.length > 0) {
           let row_map = {
-            'to': get_label($(slot_field).text()),
-            'from': data_label
+            'to': $(slot_field).attr('data-slot-name'),
+            'from': data_field_name
           };
           const table_section = $(tbody).attr('data-section') || '';
           if (table_section)
             row_map['section'] = table_section;
 
-          // Adds table if it isn't present.
-          mapping.tables[table_name].map??= []; // establishes array if not there.
+          mapping.tables[table_name].map ??= [];
           mapping.tables[table_name].map.push(row_map);
         }
-      })
+      });
       // The .map attribute exists only if 1+ mappings.
 
     });
@@ -544,41 +558,50 @@ export class FieldMapper {
       //activeClass: "ui-state-active",
       hoverClass: "ui-state-hover",
       drop: function(event, ui) {
+        // Swap visible text labels.
         const source_text = ui.draggable[0].innerText;
         ui.draggable[0].innerText = event.target.innerText;
         event.target.innerText = source_text;
+        // Swap data-field-name attributes so getProfileMapping() reads
+        // the correct field names without text parsing.
+        const src_field = $(ui.draggable[0]).attr('data-field-name');
+        const tgt_field = $(event.target).attr('data-field-name');
+        $(ui.draggable[0]).attr('data-field-name', tgt_field ?? '');
+        $(event.target).attr('data-field-name', src_field ?? '');
         $(this).css("background-color", "lightskyblue");
-        ui.draggable.css("background-color", "lightblue"); 
+        ui.draggable.css("background-color", "lightblue");
       }
     });
   }
 
-  // User has dragged one field down to the row of another within a table.
-  // Switch the labels of the selected fields.
-  // WARNING: THIS TEXT MATCHING ALGORITHM IS VERY SENSITIVE TO spaces etc. 
-  // in field label HTML display
+  // Apply a saved mapping profile to the current field-mapping display.
+  // Uses data-slot-name and data-field-name attributes for reliable matching.
   applyFieldMapping(profile_name) {
     const [dh_settings, profile] = this.getProfile(profile_name);
 
     Object.entries(profile.tables || {}).forEach(([table_name, table_obj]) => {
 
-      const mismatched_rows = $(`table#field-mapping-table tbody[data-table="${table_name}"] > tr.field-mismatch`);
       // Doing table by table; otherwise identical field names in different
       // tables lead to garbled rule implementation.
-
       Object.entries(table_obj?.map || {}).forEach(([index, mapping]) => {
-        // Find mapping.from text. mapping.from td will initially have empty 
-        // data field td. First fetch the 2nd data file field value
-        const schema_slot_td = $(mismatched_rows).find('td:first-child')
-          .filter(function() {return $(this).text().endsWith(') ' + mapping.to)}); 
-        const schema_data_td = $(mismatched_rows).find('td:eq(1)')
-          .filter(function() {return $(this).text().endsWith(') ' + mapping.from)});
-
-        // Now do the switch of values as mapping dictates:
+        // Find the unmatched slot TD by data-slot-name attribute.
+        const schema_slot_td = $(
+          `table#field-mapping-table tbody[data-table="${table_name}"] > tr.field-mismatch > td:first-child[data-slot-name="${mapping.to}"]`
+        );
+        // Find the data field TD by data-field-name attribute.
+        const schema_data_td = $(
+          `table#field-mapping-table tbody[data-table="${table_name}"] > tr.field-mismatch > td.draggable-mapping-item[data-field-name="${mapping.from}"]`
+        );
+
+        // Move data-field-name from the source data TD to the slot's right TD.
+        const src_field = $(schema_data_td).attr('data-field-name');
+        $(schema_data_td).attr('data-field-name', '');
+        $(schema_slot_td).next('td').attr('data-field-name', src_field ?? '');
+
+        // Move the visible text label as well.
         const source_data_td_text = $(schema_data_td).text();
-        $(schema_data_td).text(''); // Clear out old data td side.
+        $(schema_data_td).text('');
         $(schema_slot_td).next('td').text(source_data_td_text);
-
       });
     });
   }