Customisable error messages (#299)

Error messages are currently generated by the backend validation functions, but are not able to be customised by our users.

This PR now returns error objects back to the client (prototype kit templates) so that they can choose what the error messages should contain if the language used by default is not suitable. These errors look like:

{ 
    variant: '' // The error generated,
    field: '' // The field that generated the error 
    type: '' // The expected type of the field 
    value: '' // The value found in the field where the error occurred
}

These properties for each error can be used in the templates, but the value field is not useful here as the errors are grouped by 'variant' and 'field' so that they appear together.

Currently it requires users to update both the error messages and the warning messages until we have more clarity on which 'errors' are in each category. It is possible to use the error 'variant' (type of error), the 'type' code (the expected type of that field) and the 'field' name in error messages, but the 'value' field is the value where the error was found, and this is not useful when errors are grouped together.

In future, we may with to move the error message customisation into the dudk itself so that it can be used by non-prototype kit users, and to re-instate backend support for customisable error messages, you can revert the commits 966afd6 and ee6f49d

Author: rossjones

lib/importer/nunjucks/importer/macros/table_view.njk

@@ -29,6 +29,8 @@ Renders a table view using the provided parameters to determine what is being sh
             {% for rowObj in params.rows %}
             <tr class="govuk-table__row {% if importerRowHasHeader(rowObj) %}
             validation-error{%endif%}">
+
+
                 {% if params.showRowNumbers %}
                   <th scope="row" class="rowIndex">{{ rowObj.index }}</th>
                 {% endif %}

lib/importer/src/dudk/backend.js

@@ -4,6 +4,7 @@ const assert = require('node:assert').strict;
 const attributeTypes = require('./types/attribute-types');
 const guesser = require('./types/guesser');
 const store = require("./session_store")
+const errorTypes = require('./util/errors');
 
 // An implementation of the interface described in https://struct.register-dynamics.co.uk/trac/wiki/DataImporter/API
 
@@ -625,6 +626,7 @@ exports.SessionDelete = (sid) => {
   dimensionsCache.delete(sid);
 }
 
+
 class JobResult {
   constructor(records, errors, warnings) {
     this.records = records;
@@ -697,7 +699,7 @@ exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false
       }
 
       if (row.length < expectedColumnCount) {
-        warnings.push({row: rowIdx, field: "", message: "Row has fewer columns than expected, wanted " + expectedColumnCount + " but found " + row.length})
+        warnings.push({row: rowIdx, field: "", type: errorTypes.ValidationError.ShortRow({ expected: expectedColumnCount, actual: row.length})});
       }
 
       attrMap.forEach((element) => {
@@ -736,7 +738,7 @@ exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false
         const result = attrType(inputVal, {name: attr});
 
         result.warnings.forEach((text) => {
-          warnings.push({ row: rowIdx, field: attr, message: text });
+          warnings.push({ row: rowIdx, field: attr, type: text });
         });
 
         if (result.valid) {
@@ -749,9 +751,9 @@ exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false
           result.errors.forEach((text) => {
             rowErrorCount += 1;
             if (includeErrorRow) {
-              errors.push({ row: rowIdx, field: attr, message: text, data: row });
+              errors.push({ row: rowIdx, field: attr, type: text, data: row });
             } else {
-              errors.push({ row: rowIdx, field: attr, message: text });
+              errors.push({ row: rowIdx, field: attr, type: text });
             }
 
           });
@@ -762,7 +764,7 @@ exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false
         records.push(mappedRecord);
       }
     } else {
-      warnings.push({ row: rowIdx, message: "Row is empty" })
+      warnings.push({ row: rowIdx, type: errorTypes.ValidationError.EmptyRow({}) })
     }
   }
 

lib/importer/src/dudk/backend.test.js

@@ -571,15 +571,15 @@ test('validation', () => {
     });
 
     const warnings = backend.JobGetWarnings(sid, jid); // Weight is optional so errors here become warnings
-    expect(warnings).toStrictEqual([
-      { "row": 4, "message": 'Row is empty' },
-      { "row": 8, "field": "Weight", "message": "This is not a valid number" }]);
+    expect(warnings).toMatchObject([
+      { "row": 4, "type": {variant: "EmptyRow"} },
+      { "row": 8, "field": "Weight", "type": {variant: "IncorrectType"} }]);
 
     const errors = backend.JobGetErrors(sid, jid);
-    expect(errors).toStrictEqual([
-      { "row": 6, "field": 'Age', "message": "This is not a valid number" },
-      { "row": 7, "field": 'Name', "message": "'Name' must be supplied" },
-      { "row": 7, "field": 'Age', "message": "This is not a valid number" }
+    expect(errors).toMatchObject([
+      { "row": 6, "field": 'Age', "type": { variant: "IncorrectType"} },
+      { "row": 7, "field": 'Name', "type": { variant: "FieldRequired"} },
+      { "row": 7, "field": 'Age', "type": { variant: "IncorrectType"} }
     ]);
 
     const allRecords = backend.JobGetRecords(sid, jid, 0, 3);

lib/importer/src/dudk/sheets.js

@@ -318,14 +318,14 @@ exports.MapData = (sid, sheet, mapping, fields, previewLimit = DEFAULT_PREVIEW_L
 
 // Remaps the errors/warnings structure from
 // [
-//  { row: [{index:1, row:[]}], field: "A", message: "An error occurred" },
-//  { row: [{index:1, row:[]}], field: "B", message: "An error occurred" },
-//  { row: [{index:2, row:[]}], field: "B", message: "An error occurred" }
+//  { row: [{index:1, row:[]}], field: "A", type: {variant: ..., value: ..., type: ...} },
+//  { row: [{index:1, row:[]}], field: "B", type: {variant: ..., value: ..., type: ...} },
+//  { row: [{index:2, row:[]}], field: "B", type: {variant: ..., value: ..., type: ...} }
 // ]
 // to
 // [
 //  {
-//    message: "An error occurred",
+//    type: {variant: ..., value: ..., type: ...},
 //    meta: {first: 1, count: 2}
 //    fields: [ "A", "B"] },
 //    rows: [[{index:1, row:[]}], [{index:2, row:[]}]] },
@@ -346,14 +346,17 @@ const remapErrorStructure = (errors, row_data, mappingIndices, headerRangeOffset
       return row.some(cell => cell && cell.error);
     }
 
-
     for (const error of errors) {
-        if (!messageMap.has(error.message)) {
-            messageMap.set(error.message, new Map());
+        const key = `${error.type.variant}-${error.field}`;
+
+        if (!messageMap.has(key)) {
+            messageMap.set(key, new Map());
         }
 
-        const errorObj = messageMap.get(error.message);
-        errorObj.message = error.message;
+        const errorObj = messageMap.get(key);
+        errorObj.type = error.type;
+        errorObj.type.field = error.field;
+
         if( !errorObj.fields) {
             errorObj.fields = new Set();
         }
@@ -370,11 +373,13 @@ const remapErrorStructure = (errors, row_data, mappingIndices, headerRangeOffset
         }
 
         if (row_data && Array.isArray(row_data) && realErrorIndex < row_data.length )  {
-
             let previousRow = structuredClone(row_data[realErrorIndex - 1]);
             let currentRow = structuredClone(row_data[realErrorIndex]);
             let nextRow = structuredClone(row_data[realErrorIndex + 1]);
 
+            // currentRow is the error and so should have the error flag
+            currentRow.error = true;
+
             if (previousRow) {
               if (!rowHasError(errorObj.rows.get(previousRow.index)?.row)) {
                 errorObj.rows.set(previousRow.index,  previousRow);
@@ -400,12 +405,13 @@ const remapErrorStructure = (errors, row_data, mappingIndices, headerRangeOffset
     }
 
     // Convert the nested maps/sets to the desired array structure
-    return Array.from(messageMap.entries().map(([message, errorObj]) => {
+    return Array.from(messageMap.values().map((errorObj) => {
         // Some rows may not have any errors because they are for surrounding context,
         // so we need to filter them out for the meta info
-        const errorRows = Array.from(errorObj.rows.values()).filter(rowObj => rowObj.row.some(element=>element && element.error) );
+        const errorRows = Array.from(errorObj.rows.values()).filter(rowObj => rowObj.error);
+
         return {
-            message: message,
+            type: errorObj.type,
             meta: {
                 first: errorRows[0]?.index || 0,
                 count: errorRows?.length || 0

lib/importer/src/dudk/sheets.test.js

@@ -25,10 +25,10 @@ test('trailing columns', () => {
 describe('remapErrorStructure', () => {
     test('groups errors by message, collects rows', () => {
         const errors = [
-            { row: 1, field: "A", message: "Error 1" },
-            { row: 1, field: "B", message: "Error 2" },
-            { row: 2, field: "B", message: "Error 2" },
-            { row: 3, field: "A", message: "Error 1" },
+            { row: 1, field: "A", type: {variant: "IncorrectType"} },
+            { row: 1, field: "B", type: {variant: "IncorrectType"} },
+            { row: 2, field: "B", type: {variant: "IncorrectType"} },
+            { row: 3, field: "A", type: {variant: "IncorrectType"} },
         ];
         // Adjusted expected result to match grouping by error, then by field
         const row_data = [
@@ -38,30 +38,32 @@ describe('remapErrorStructure', () => {
         const result = sheets_lib.RemapErrorStructure(errors, row_data, RewrittenMapping);
         expect(result).toEqual([
             {
-                message: "Error 1",
+                type: {variant: "IncorrectType", field: "A"},
                 fields: [ "A"],
-                meta: { first: 1, count: 2 },
+                // Only 1 error (in the two rows) because A appears twice but in
+                // rows 1 and 3 and here we have rows 1 and 2
+                meta: { first: 1, count: 1 },
                 rows: [
-                    {index:1, row:[{value:"0", error: true},{value:"1", error: true}]},
+                    {index:1, error: true, row:[{value:"0", error: true},{value:"1", error: true}]},
                     {index:2, row:[{value:"2", error: true}, {value:"3", error: true}]}
                 ]
             },
             {
-                message: "Error 2",
+                type: {variant: "IncorrectType", field: "B"},
                 fields: ["B"],
                 meta: { first: 1, count: 2 },
                 rows: [
-                    {index:1, row: [{value:"0", error: true},{value:"1", error: true}]},
-                    {index:2, row: [{value:"2", error: true}, {value:"3", error: true}]}
+                    {index:1, error: true, row: [{value:"0", error: true},{value:"1", error: true}]},
+                    {index:2, error: true, row: [{value:"2", error: true}, {value:"3", error: true}]}
                 ]
             }
         ]);
     });
 
     test('with row_data, includes adjacent rows', () => {
         const errors = [
-            { row: 1, field: "A", message: "Error 1" },
-            { row: 2, field: "A", message: "Error 1" },
+            { row: 1, field: "A", type: "Error 1" },
+            { row: 2, field: "A", type: "Error 1" },
         ];
         const row_data = [
             {index: 1, row: [{value:"0", error: true},{value:"1"}]},
@@ -70,21 +72,21 @@ describe('remapErrorStructure', () => {
         const result = sheets_lib.RemapErrorStructure(errors, row_data, RewrittenMapping);
         expect(result).toEqual([
             {
-                message: "Error 1",
+                type: "Error 1",
                 fields: ["A"],
                 meta: {first: 1, count: 2},
                 rows: [
-                    {index: 1, row: [{value:"0", error: true},{value:"1"}]},
-                    {index: 2, row: [{value:"2", error: true}, {value:"3"}]}
+                    {index: 1, error: true, row: [{value:"0", error: true},{value:"1"}]},
+                    {index: 2, error: true, row: [{value:"2", error: true}, {value:"3"}]}
                 ]
             }
         ]);
     });
 
     test('does not add out-of-bounds adjacent rows', () => {
         const errors = [
-            { row: 1, field: "A", message: "Error 1" },
-            { row: 3, field: "A", message: "Error 1" },
+            { row: 1, field: "A", type: "Error 1" },
+            { row: 3, field: "A", type: "Error 1" },
         ];
         const row_data = [
             {index: 1, row: [{value:"0", error: true},{value:"1"}]},
@@ -96,11 +98,11 @@ describe('remapErrorStructure', () => {
         // For row 3: 2, 3
         expect(result).toEqual([
             {
-                message: "Error 1",
+                type: "Error 1",
                 fields: ["A"],
                 meta: { first: 1, count: 1 },
                 rows: [
-                    {index: 1, row: [{value:"0", error: true},{value:"1"}]},
+                    {index: 1, error: true, row: [{value:"0", error: true},{value:"1"}]},
                     {index: 2, row: [{value:"2"},{value:"3"}]}
                 ]
             }
@@ -115,23 +117,23 @@ describe('remapErrorStructure', () => {
 
     test('ignores errors with missing row in row_data', () => {
         const errors = [
-            { row: 5, field: "A", message: "Error 1" }
+            { row: 5, field: "A", type: "Error 1" }
         ];
         const row_data = [
             {index: 1, row: [{value:"0"},{value:"1"}]},
             {index: 2, row: [{value:"2"}, {value:"3"}]}
         ];
         const result = sheets_lib.RemapErrorStructure(errors, row_data, RewrittenMapping);
         expect(result).toEqual([
-            { message: "Error 1", meta: { count: 0, first: 0}, fields: ["A"], rows: [] }
+            { type: "Error 1", meta: { count: 0, first: 0}, fields: ["A"], rows: [] }
         ]);
     });
 
     test('remapping errors - missing fields', () => {
         const errors =  [
-            { row: 2, field: 'Salary', message: "'Salary' must be supplied" },
-            { row: 3, field: 'Salary', message: "'Salary' must be supplied" },
-            { row: 5, field: 'Salary', message: "'Salary' must be supplied" }
+            { row: 2, field: 'Salary', type: { variant: "FieldRequired" } },
+            { row: 3, field: 'Salary', type: { variant: "FieldRequired" } },
+            { row: 5, field: 'Salary', type: { variant: "FieldRequired" } }
         ] ;
         const row_data =
         [
@@ -157,16 +159,16 @@ describe('remapErrorStructure', () => {
         const result = sheets_lib.RemapErrorStructure(errors, row_data, mappingIndices, headerRangeOffset);
         expect(result).toEqual([
             {
-                message: "'Salary' must be supplied",
+                type: {variant: "FieldRequired", field: "Salary"},
                 fields: ['Salary'],
                 meta: { first: 3, count: 3 },
                 rows: [
                     // errors in index 3, 5 and 6
                     { index: 2, row: [{value: "2"}, {value: "Mr"}, {value: "Odie"}, {value: "Arbuckle"}, {value: "2025-02-01"}, {value: "123" }, {value: "12%"}, {value: "2025-04-01"}]},
-                    { index: 3, row: [{ value: "1" }, { value: "Mr" }, { value: "Jon" }, { value: "Arbuckle" }, { value: "2025-01-01" }, { value: "   ", error: true}, undefined, { value: "2025-04-01" }] },
-                    { index: 4, row: [{ value: "1" }, { value: "Mr" }, { value: "Jon" }, { value: "Arbuckle" }, { value: "2025-01-01" }, { value: "   ", error: true}, undefined, { value: "2025-04-01" }] },
+                    { index: 3, error: true, row: [{ value: "1" }, { value: "Mr" }, { value: "Jon" }, { value: "Arbuckle" }, { value: "2025-01-01" }, { value: "   ", error: true}, undefined, { value: "2025-04-01" }] },
+                    { index: 4, error: true, row: [{ value: "1" }, { value: "Mr" }, { value: "Jon" }, { value: "Arbuckle" }, { value: "2025-01-01" }, { value: "   ", error: true}, undefined, { value: "2025-04-01" }] },
                     { index: 5, row: [{value: "2"}, {value: "Mr"}, {value: "Odie"}, {value: "Arbuckle"}, {value: "2025-02-01"}, {value: "123"}, {value: "12%"}, {value: "2025-04-01"}]},
-                    { index: 6, row: [{ value: "1" }, { value: "Mr" }, { value: "Jon" }, { value: "Arbuckle" }, { value: "2025-01-01" }, { value: "   ", error: true}, undefined, { value: "2025-04-01" }] },
+                    { index: 6, error: true, row: [{ value: "1" }, { value: "Mr" }, { value: "Jon" }, { value: "Arbuckle" }, { value: "2025-01-01" }, { value: "   ", error: true}, undefined, { value: "2025-04-01" }] },
                     { index: 7, row: [{value: "2"}, {value: "Mr"}, {value: "Odie"}, {value: "Arbuckle"}, {value: "2025-02-01"}, {value: "123"}, {value: "12%"}, {value: "2025-04-01"}]}
                 ]
             }
@@ -176,12 +178,12 @@ describe('remapErrorStructure', () => {
 
     test('remapping errors - multiple errors', () => {
         const errors =  [
-            { row: 3, field: 'Salary', message: "'Salary' must be supplied" },
-            { row: 4, field: 'Salary', message: 'This is not a valid number' },
-            { row: 5, field: 'Salary', message: "'Salary' must be supplied" },
-            { row: 6, field: 'Salary', message: 'This is not a valid number' },
-            { row: 7, field: 'Salary', message: "'Salary' must be supplied" },
-            { row: 8, field: 'Salary', message: "'Salary' must be supplied" }
+            { row: 3, field: 'Salary', type: {variant: "FieldRequired"} },
+            { row: 4, field: 'Salary', type: {variant: "IncorrectType"} },
+            { row: 5, field: 'Salary', type: {variant: "FieldRequired"} },
+            { row: 6, field: 'Salary', type: {variant: "IncorrectType"} },
+            { row: 7, field: 'Salary', type: {variant: "FieldRequired"} },
+            { row: 8, field: 'Salary', type: {variant: "FieldRequired"} }
         ] ;
         const row_data = [
             { index: 4, row: [
@@ -207,9 +209,9 @@ describe('remapErrorStructure', () => {
         const headerRangeOffset = 3 ;
 
         const result = sheets_lib.RemapErrorStructure(errors, row_data, mappingIndices, headerRangeOffset);
-        expect(result).toEqual([
+        expect(result).toMatchObject([
             {
-                message: "'Salary' must be supplied",
+                type: { variant: "FieldRequired" },
                 meta: { first: 4, count: 4 },
                 fields: [ 'Salary' ],
                 rows:[
@@ -222,7 +224,7 @@ describe('remapErrorStructure', () => {
                 ]
             },
             {
-                message: 'This is not a valid number',
+                type: { variant: "IncorrectType" },
                 meta: { first: 5, count: 2 },
                 fields: [ 'Salary' ],
                 rows: [

lib/importer/src/dudk/types/attribute-types.js

@@ -5,6 +5,7 @@
 // An attribute type is a function from an input value to a result.
 
 const ati = require('./attribute-type-internals');
+const errors = require('../util/errors')
 
 // Create an optional version of an existing type, that allows empty strings or
 // undefined values and maps then to the empty mapping, and converts any validation errors to warnings
@@ -32,7 +33,7 @@ exports.requiredType = (baseType, field) => {
       return baseType(inputValue);
     }
     else {
-      return ati.failedMapping([], [`'${field.name}' must be supplied`]);
+      return ati.failedMapping([], [errors.ValidationError.FieldRequired({value: inputValue?.v, type: field.type})]);
     }
   };
 }
@@ -46,7 +47,7 @@ exports.basicStringType = (inputCell) => {
 exports.basicNumberType = (inputCell) => {
   const result = parseFloat(inputCell.v);
   if (isNaN(result)) {
-    return ati.failedMapping([], ["This is not a valid number"]);
+    return ati.failedMapping([], [errors.ValidationError.IncorrectType({ value: inputCell.v, type: "number"})]);
   } else {
     return ati.successfulMapping(result);
   }