Update UI to reflect new distinction between safe/maybe/unsafe formats and types

Author: alaric-rd

lib/importer/nunjucks/importer/macros/format_picker.njk

@@ -79,16 +79,22 @@
       <td class="govuk-table__cell govuk-table__cell--numeric">
          {% if possibleFormats %}
          <select class="govuk-select" style="float: right;" name="format-{{h.index}}">
-                {% for format in possibleFormats %}
+            <optgroup label="Matches every row">
+                {% for format in possibleFormats[0] %}
                     <option value="{{format.name}}"
-                            {% if format.name == currentValue %}selected{% endif %}>
-                      {% if format.likely %}
-                         {{format.displayName}} (*)
-                      {% else %}
-                         {{format.displayName}}
-                      {% endif %}
+                      {% if format.name == currentValue %}selected{% endif %}>
+                      {{format.displayName}}
                     </option>
                 {% endfor %}
+            </optgroup>
+            <optgroup label="Matches some rows">
+                {% for format in possibleFormats[1] %}
+                    <option value="{{format.name}}"
+                      {% if format.name == currentValue %}selected{% endif %}>
+                      {{format.displayName}}
+                    </option>
+                {% endfor %}
+            </optgroup>
          </select>
          {% endif %}
       </td>

lib/importer/src/functions.js

@@ -82,10 +82,13 @@ const importerMappedColumnDetails = (data) => {
 //--------------------------------------------------------------------
 // As long as column->field mappings have been set up, this function will return
 // an array indexed on column, with each element being list of possible formats
-// for that column. The array element for a column will be false if the column is mapped to a
-// field whose type does not require formats, otherwise it will be a list of
-// objects with 'name' (internal code), 'displayName' (human-facing name) and
-// 'description' (longer description) fields.
+// for that column. The array element for a column will be false if the column
+// is mapped to a field whose type does not require formats, otherwise it will
+// be TWO lists of objects with 'name' (internal code), 'displayName'
+// (human-facing name) and 'description' (longer description) fields. The first
+// is "safe" formats that will work for all the values in the column, the second
+// are "maybe" formats that will work for some of them. Those that work for none
+// of them are not present at all.
 // --------------------------------------------------------------------
 const importerPossibleColumnFormats = (data) => {
     // FIXME: This should probably move into the backend, as a SessionSuggestFormats function
@@ -105,11 +108,20 @@ const importerPossibleColumnFormats = (data) => {
         const columnDef = session.fields.find((field) => field.name == columnField);
         const columnTypeName = columnDef.type;
         const columnType = supportedTypes.get(columnTypeName);
-        let likelyFormats = new Set(); // Names of formats this column is likely to have, according to the guesser
-        if (columnGuesses.types && columnGuesses.types.has(columnTypeName)) {
-            const guessedFormats = columnGuesses.types.get(columnTypeName);
+
+        let safeFormats = new Set(); // Names of formats this column is likely to have, according to the guesser
+        if (columnGuesses.safeTypes && columnGuesses.safeTypes.has(columnTypeName)) {
+            const guessedFormats = columnGuesses.safeTypes.get(columnTypeName);
+            if(guessedFormats && columnType.formats) {
+                safeFormats = new Set(Array.from(columnType.formats.keys()).filter((fmtName) => guessedFormats.includes(fmtName)));
+            }
+        }
+
+        let maybeFormats = new Set(); // Names of formats this column merely might have, according to the guesser
+        if (columnGuesses.maybeTypes && columnGuesses.maybeTypes.has(columnTypeName)) {
+            const guessedFormats = columnGuesses.maybeTypes.get(columnTypeName);
             if(guessedFormats && columnType.formats) {
-                likelyFormats = new Set(Array.from(columnType.formats.keys()).filter((fmtName) => guessedFormats.includes(fmtName)));
+                maybeFormats = new Set(Array.from(columnType.formats.keys()).filter((fmtName) => guessedFormats.includes(fmtName)));
             }
         }
 
@@ -119,10 +131,13 @@ const importerPossibleColumnFormats = (data) => {
                     name: fmtEntry[0],
                     displayName: fmtEntry[1].displayName,
                     description: fmtEntry[1].description,
-                    likely: likelyFormats.has(fmtEntry[0])
+                    safe: safeFormats.has(fmtEntry[0]),
+                    maybe: maybeFormats.has(fmtEntry[0])
                 };
             });
-            return options;
+            const formats = [options.filter((fmtEntry) => fmtEntry.safe),
+                             options.filter((fmtEntry) => fmtEntry.maybe)];
+            return formats;
         } else {
             return false;
         }
@@ -158,19 +173,19 @@ const importerGetTrailingRows = (data, count) => {
 //     "All {n} rows of {sheet}"
 //--------------------------------------------------------------------
 const importerGetTableCaption =
-    (data, prefix, rowsAskedFor, sheetName = null) => {
-        const session_data = data[IMPORTER_SESSION_KEY];
-        const session = new session_lib.Session(session_data)
+      (data, prefix, rowsAskedFor, sheetName = null) => {
+          const session_data = data[IMPORTER_SESSION_KEY];
+          const session = new session_lib.Session(session_data)
 
-        const sheet = (sheetName ??= session.sheet);
-        const rowCount = sheets_lib.GetTotalRows(session.backendSid, sheet);
+          const sheet = (sheetName ??= session.sheet);
+          const rowCount = sheets_lib.GetTotalRows(session.backendSid, sheet);
 
-        if (rowCount > rowsAskedFor) {
-            return `${prefix} ${rowsAskedFor} rows of '${sheet}'`;
-        }
+          if (rowCount > rowsAskedFor) {
+              return `${prefix} ${rowsAskedFor} rows of '${sheet}'`;
+          }
 
-        return `All rows of '${sheet}'`;
-    }
+          return `All rows of '${sheet}'`;
+      }
 
 //--------------------------------------------------------------------
 // In the absence of a step to choose headers, we will instead provide
@@ -208,8 +223,8 @@ const importerGetHeaders = (data) => {
                 session.backendSid,
                 session.sheet,
                 index,
-          /* cellWidth */ 20,
-          /* count */ 5,
+                /* cellWidth */ 20,
+                /* count */ 5,
             ).inputValues;
 
             let exampleString = examples.join(", ").trim();
@@ -306,8 +321,8 @@ const data_avg = (data, column) => {
     }
 
     const numbers = mapResults.resultRecords
-        .filter((x) => x !== undefined && x[idx] !== undefined)
-        .map((x) => parseNumberFromString(x[idx]))
+          .filter((x) => x !== undefined && x[idx] !== undefined)
+          .map((x) => parseNumberFromString(x[idx]))
 
 
     const avg = numbers.reduce((acc, i) => acc + i, 0) / numbers.length
@@ -354,7 +369,7 @@ module.exports = {
 }
 
 /*
-Local Variables:
-js-indent-level: 4
-End:
+  Local Variables:
+  js-indent-level: 4
+  End:
 */

lib/importer/src/index.js

@@ -442,7 +442,7 @@ exports.Initialise = (config, router, prototypeKit) => {
         if ("other" in request.query) {
           const columnsNeedingFormats = backend.SessionGetColumnsNeedingFormats(session.backendSid);
           if(columnsNeedingFormats.length == 0) {
-            const otherPage = decodeURIComponent(request.query.other)
+            const otherPage = decodeURIComponent(request.query.other);
             if (otherPage) { request.query.next = request.query.other }
           }
         }