diff --git a/fixtures/validation-struct-err.csv b/fixtures/validation-struct-err.csv new file mode 100644 index 00000000..456cbd01 --- /dev/null +++ b/fixtures/validation-struct-err.csv @@ -0,0 +1,5 @@ +Employee number,Title,First name,Surname,Employment start date,Salary,Contribution percentage,Payment date +1,Mr,Jon,Arbuckle,2025-01-01,,,2025-04-01 +,,,,,,,, +,,,,,,,, +2,Mr,Odie,Arbuckle,2025-02-01,123,12%,2025-04-01 diff --git a/fixtures/validation-test.csv b/fixtures/validation-test.csv new file mode 100644 index 00000000..fa7cd6f7 --- /dev/null +++ b/fixtures/validation-test.csv @@ -0,0 +1,3 @@ +Employee number,Title,First name,Surname,Employment start date,Salary,Contribution percentage,Payment date +1,Mr,Jon,Arbuckle,2025-01-01,,,2025-04-01 +2,Mr,Odie,Arbuckle,2025-02-01,123,12%,2025-04-01 diff --git a/lib/importer/nunjucks/importer/macros/field_mapper.njk b/lib/importer/nunjucks/importer/macros/field_mapper.njk index b03848b3..c90b807c 100644 --- a/lib/importer/nunjucks/importer/macros/field_mapper.njk +++ b/lib/importer/nunjucks/importer/macros/field_mapper.njk @@ -17,14 +17,33 @@ {% macro importerFieldMapper(data, caption='', columnTitle='Column', examplesTitle='Example values', fieldsTitle='Fields') %} {% set fields = data['importer.session']['fields'] %} {% set headings = importerGetHeaders(data) %} - {% set error = headings.error %} + {% set headingError = headings.error %} + {% set error = importerError(data) %} - {% if error %} + {% if headingError %}

- Error: {{ error.text }} + Error: {{ headingError.text }}

{% endif %} + {% if error %} +
+
+

+ {{ error.text }} +

+
+
    + {% for e in error.extra %} +
  • + {{ e }} +
    • + {% endfor %} +
+
+
+
+ {% endif %} {% if caption %} @@ -39,14 +58,17 @@ {% for h in headings.data %} - + {% set hIndex = loop.index0 %} + {% set currentValue = importerErrorMappingData(error, hIndex) %} + + diff --git a/lib/importer/src/attribute-types.js b/lib/importer/src/attribute-types.js index 5cb801e8..ed793dbd 100644 --- a/lib/importer/src/attribute-types.js +++ b/lib/importer/src/attribute-types.js @@ -5,14 +5,13 @@ // An attribute type is a function from an input value to a result. class AttributeMappingResult { - constructor(value, warnings, errors) { + constructor(value, errors) { this.value = value; - this.warnings = warnings || []; - this.errors = errors; + this.errors = errors || []; } get valid() { - return !this.errors; + return this.errors.length == 0; } get empty() { @@ -22,20 +21,20 @@ class AttributeMappingResult { // These helpers define the three kinds of results: -function emptyMapping(warnings) { - return new AttributeMappingResult(undefined, warnings); +function emptyMapping(errors) { + return new AttributeMappingResult(undefined, errors); } -function successfulMapping(outputValue, warnings) { - return new AttributeMappingResult(outputValue, warnings); +function successfulMapping(outputValue, errors) { + return new AttributeMappingResult(outputValue, errors); } -function failedMapping(warnings, errors) { - return new AttributeMappingResult(undefined, warnings, errors); +function failedMapping(errors) { + return new AttributeMappingResult(undefined, errors); } // Create an optional version of an existing type, that allows empty strings or -// undefined values and maps then to "undefined", and converts any validation errors to warnings +// undefined values and maps then to "undefined" exports.optionalType = (baseType) => { return (inputValue) => { if (inputValue !== undefined && inputValue != "") { @@ -44,7 +43,7 @@ exports.optionalType = (baseType) => { return result; } else { // Convert any errors into warnings, as this is an optional field - return emptyMapping(result.warnings.concat(result.errors)); + return emptyMapping(result.errors); } } else { @@ -60,7 +59,7 @@ exports.requiredType = (baseType) => { return baseType(inputValue); } else { - return failedMapping([], ["A value must be provided"]); + return failedMapping(["A value must be provided"]); } }; } @@ -74,19 +73,28 @@ exports.basicStringType = (inputValue) => { exports.basicNumberType = (inputValue) => { const result = parseFloat(inputValue); if (isNaN(result)) { - return failedMapping([], ["This is not a valid number"]); + return failedMapping(["This is not a valid number"]); } else { return successfulMapping(result); } }; -exports.typeFromName = (name) => { - if (!name) return null +exports.mapperForField = (field) => { + if (!field) return null - switch (name.toLowerCase()) { + let m = exports.basicStringType; + + switch (field.type.toLowerCase()) { case "number": - return exports.basicNumberType + m = exports.basicNumberType + break default: - return exports.basicStringType + m = exports.basicStringType + } + + if (field.required) { + return exports.requiredType(m) } + + return exports.optionalType(m) } diff --git a/lib/importer/src/attribute-types.test.js b/lib/importer/src/attribute-types.test.js index 8d265b99..c077c9a6 100644 --- a/lib/importer/src/attribute-types.test.js +++ b/lib/importer/src/attribute-types.test.js @@ -6,23 +6,21 @@ test('required basic string', () => { let r = t(undefined); expect(r.valid).toBeFalsy(); - expect(r.warnings).toMatchObject([]); expect(r.errors).toMatchObject(["A value must be provided"]); r = t(""); expect(r.valid).toBeFalsy(); - expect(r.warnings).toMatchObject([]); expect(r.errors).toMatchObject(["A value must be provided"]); r = t("foo"); expect(r.valid).toBeTruthy(); expect(r.value).toEqual("foo"); - expect(r.warnings).toMatchObject([]); + expect(r.errors).toMatchObject([]); r = t(123); expect(r.valid).toBeTruthy(); expect(r.value).toEqual("123"); - expect(r.warnings).toMatchObject([]); + expect(r.errors).toMatchObject([]); }); test('optional basic string', () => { @@ -31,23 +29,23 @@ test('optional basic string', () => { let r = t(undefined); expect(r.valid).toBeTruthy(); - expect(r.warnings).toMatchObject([]); + expect(r.errors).toMatchObject([]); expect(r.value).toEqual(undefined); r = t(""); expect(r.valid).toBeTruthy(); - expect(r.warnings).toMatchObject([]); + expect(r.errors).toMatchObject([]); expect(r.value).toEqual(undefined); r = t("foo"); expect(r.valid).toBeTruthy(); expect(r.value).toEqual("foo"); - expect(r.warnings).toMatchObject([]); + expect(r.errors).toMatchObject([]); r = t(123); expect(r.valid).toBeTruthy(); expect(r.value).toEqual("123"); - expect(r.warnings).toMatchObject([]); + expect(r.errors).toMatchObject([]); }); test('required basic number', () => { @@ -55,13 +53,12 @@ test('required basic number', () => { let r = t("foo"); expect(r.valid).toBeFalsy(); - expect(r.warnings).toMatchObject([]); expect(r.errors).toMatchObject(["This is not a valid number"]); r = t(123); expect(r.valid).toBeTruthy(); expect(r.value).toEqual(123); - expect(r.warnings).toMatchObject([]); + expect(r.errors).toMatchObject([]); }); test('optional basic number', () => { @@ -69,12 +66,12 @@ test('optional basic number', () => { const t = attributeTypes.optionalType(attributeTypes.basicNumberType); let r = t("foo"); - expect(r.valid).toBeTruthy(); + expect(r.valid).toBeFalsy(); expect(r.value).toBeUndefined(); - expect(r.warnings).toMatchObject(["This is not a valid number"]); + expect(r.errors).toMatchObject(["This is not a valid number"]); r = t(123); expect(r.valid).toBeTruthy(); expect(r.value).toEqual(123); - expect(r.warnings).toMatchObject([]); + expect(r.errors).toMatchObject([]); }); diff --git a/lib/importer/src/backend.js b/lib/importer/src/backend.js index a85aa401..851b6b24 100644 --- a/lib/importer/src/backend.js +++ b/lib/importer/src/backend.js @@ -481,7 +481,7 @@ function mapCellValue(cell) { return cell.v; } -exports.SessionPerformMappingJob = (sid, range, mapping) => { +exports.SessionPerformMappingJob = (sid, range, mapping, includeErrorRow = false) => { assert(sessionStore.get(sid)); assert(sessionStore.get(sid).wb.Sheets[range.sheet]); assert(range.end.row >= range.start.row); @@ -499,13 +499,26 @@ exports.SessionPerformMappingJob = (sid, range, mapping) => { const attrMap = Object.entries(mapping.attributeMappings); const attrTypes = mapping.attributeTypes || {}; + + // TODO: Replace this with a map/sum once range is a real type. + let expectedColumnCount = 0; + for (let rowIdx = range.start.row; rowIdx <= range.end.row; rowIdx++) { + let row = getMergedRow(data, merges, rowIdx); + if (row && row.length > expectedColumnCount) expectedColumnCount = row.length; + } + for (let rowIdx = range.start.row; rowIdx <= range.end.row; rowIdx++) { let row = getMergedRow(data, merges, rowIdx); let record = {}; let foundSomeValues = false; let rowWarnings = []; let rowErrors = []; + if (row) { + if (row.length < expectedColumnCount) { + rowWarnings.push("Row has fewer columns than expected, wanted " + expectedColumnCount + " but found " + + row.length) + } + attrMap.forEach((element) => { const [attr, m] = element; @@ -518,6 +531,10 @@ exports.SessionPerformMappingJob = (sid, range, mapping) => { if (cell && cell.v) { record[attr] = mapCellValue(cell); foundSomeValues = true; + } else { + // Ensure we add every cell so that validation can fail those without + // values (where they are required). + record[attr] = undefined } } }); @@ -533,10 +550,6 @@ exports.SessionPerformMappingJob = (sid, range, mapping) => { const result = attrType(inputVal); - result.warnings.forEach((text) => { - rowWarnings.push([attr, text]); - }); - if (result.valid) { // Succeeded, but maybe an empty result if (result.value !== undefined) { @@ -545,7 +558,12 @@ exports.SessionPerformMappingJob = (sid, range, mapping) => { } else { // Failed result.errors.forEach((text) => { - rowErrors.push([attr, text]); + if (includeErrorRow) { + rowErrors.push({ row: rowIdx, field: attr, message: text, data: row.map((x) => mapCellValue(x)) }); + } else { + rowErrors.push({ row: rowIdx, field: attr, message: text }); + } + }); } }); @@ -553,11 +571,14 @@ exports.SessionPerformMappingJob = (sid, range, mapping) => { if (rowErrors.length == 0) { records.push(mappedRecord); } + } else { + rowWarnings.push("Row is empty") } if (rowWarnings.length > 0) { warnings.set(rowIdx, rowWarnings); } + if (rowErrors.length > 0) { errors.set(rowIdx, rowErrors); } @@ -596,7 +617,6 @@ exports.JobGetWarnings = (jid) => { exports.JobGetErrors = (jid) => { assert(jobStore.has(jid)); let job = jobStore.get(jid); - return job.errors; } diff --git a/lib/importer/src/backend.test.js b/lib/importer/src/backend.test.js index b03364c1..72b27d7e 100644 --- a/lib/importer/src/backend.test.js +++ b/lib/importer/src/backend.test.js @@ -3,9 +3,9 @@ const attributeTypes = require('./attribute-types'); const testFiles = new Map([ ["test", [ - ["test.xlsx", "Cool Data"], - ["test.csv", "Sheet1"], - ["test.ods", "Cool Data"] + ["test.xlsx", "Cool Data"], + ["test.csv", "Sheet1"], + ["test.ods", "Cool Data"] ]], ["merged-cells", [ ["merged-cells.xlsx", "Cool Data"], @@ -18,9 +18,9 @@ const testFiles = new Map([ ["tribbles.ods", "My lovely tribbles"], ]], ["test-validation", - [["test-validation.xlsx", "Cool Data"]], - [["test-validation.csv", "Sheet1"]], - [["test-validation.ods", "Cool Data"]], + [["test-validation.xlsx", "Cool Data"]], + [["test-validation.csv", "Sheet1"]], + [["test-validation.ods", "Cool Data"]], ] ]); @@ -34,12 +34,12 @@ describe("Backend tests", () => { // ODS number formats will result in a test fail. For the // purpose of these tests, we will disable that functionality. beforeEach(() => { - jest.spyOn(console, 'error').mockImplementation(() => {}); + jest.spyOn(console, 'error').mockImplementation(() => { }); }); test('happy path', () => { const fixtures = testFiles.get("test").map(prefix_fixture) - for(let [filename, sheetname] of fixtures) { + for (let [filename, sheetname] of fixtures) { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); @@ -55,22 +55,23 @@ describe("Backend tests", () => { const dataRange = { sheet: sheetname, - start: {row: 3, column: 0}, - end: {row: 5, column: 2}}; + start: { row: 3, column: 0 }, + end: { row: 5, column: 2 } + }; const samples = backend.SessionGetInputSampleRows(sid, dataRange, - 1, 1, 1); + 1, 1, 1); expect(samples).toMatchObject([ - [ [ {value: 'Boris'}, {value: '13'}, {value: 'High'} ] ], - [ [ {value: 'Nelly'}, {value: '14'}, {value: 'High'} ] ], - [ [ {value: 'Sid'}, {value: '10'}, {value: 'Medium'} ] ] + [[{ value: 'Boris' }, { value: '13' }, { value: 'High' }]], + [[{ value: 'Nelly' }, { value: '14' }, { value: 'High' }]], + [[{ value: 'Sid' }, { value: '10' }, { value: 'Medium' }]] ]); const values = backend.SessionGetInputValues(sid, dataRange, 2); expect(values).toMatchObject([ - { inputValues: [ 'Boris', 'Nelly' ], hasMore: true }, - { inputValues: [ '13', '14' ], hasMore: true }, - { inputValues: [ 'High', 'Medium' ], hasMore: false } + { inputValues: ['Boris', 'Nelly'], hasMore: true }, + { inputValues: ['13', '14'], hasMore: true }, + { inputValues: ['High', 'Medium'], hasMore: false } ]); const mapping = { @@ -96,7 +97,7 @@ describe("Backend tests", () => { expect(jobSummary).toMatchObject({ recordCount: 3, errorCount: 0, - warningCount: 0 + warningCount: 2 }); const warnings = backend.JobGetWarnings(jid); @@ -107,9 +108,9 @@ describe("Backend tests", () => { const sampleRecords = backend.JobGetSampleRecords(jid, 1, 1, 1); expect(sampleRecords).toMatchObject([ - [ { Name: 'Boris', Age: 13, Egginess: 'High' } ], - [ { Name: 'Nelly', Age: 14, Egginess: 'High' } ], - [ { Name: 'Sid', Age: 10, Egginess: 'Medium' } ] + [{ Name: 'Boris', Age: 13, Egginess: 'High' }], + [{ Name: 'Nelly', Age: 14, Egginess: 'High' }], + [{ Name: 'Sid', Age: 10, Egginess: 'Medium' }] ]); const allRecords = backend.JobGetRecords(jid, 0, 3); @@ -132,82 +133,83 @@ describe("Backend tests", () => { const dataRange = { sheet: 'Cool Data', - start: {row: 3, column: 2}, - end: {row: 5, column: 5}}; + start: { row: 3, column: 2 }, + end: { row: 5, column: 5 } + }; const samples = backend.SessionGetInputSampleRows(sid, dataRange, - 1, 1, 1); + 1, 1, 1); expect(samples).toMatchObject([ - [ [ {value: 'Boris'}, undefined, {value: '13'}, - // This cell is merged with the next row - {merge: {column: 0, columns: 1, row: 0, rows: 2}, value:"High"}] ], - [ [ {value: 'Nelly'}, // These two cells are merged - {merge: {column: 0, columns: 2, row: 0, rows: 1}, value: '14'}, - {merge: {column: 1, columns: 2, row: 0, rows: 1}, value: '14'}, - // This cell is merged with the previous row - {merge: {column: 0, columns: 1, row: 1, rows: 2}, value:"High"} ] ], - [ [ {value: 'Sid'}, undefined, {value: '10'}, {value: 'Medium'} ] ] + [[{ value: 'Boris' }, undefined, { value: '13' }, + // This cell is merged with the next row + { merge: { column: 0, columns: 1, row: 0, rows: 2 }, value: "High" }]], + [[{ value: 'Nelly' }, // These two cells are merged + { merge: { column: 0, columns: 2, row: 0, rows: 1 }, value: '14' }, + { merge: { column: 1, columns: 2, row: 0, rows: 1 }, value: '14' }, + // This cell is merged with the previous row + { merge: { column: 0, columns: 1, row: 1, rows: 2 }, value: "High" }]], + [[{ value: 'Sid' }, undefined, { value: '10' }, { value: 'Medium' }]] ]); const allRange = { sheet: 'Cool Data', - start: {row: 0, column: 0}, - end: {row: 7, column: 5} + start: { row: 0, column: 0 }, + end: { row: 7, column: 5 } }; const wholeSheet = backend.SessionGetInputSampleRows(sid, - allRange, - 8, 0, 0); + allRange, + 8, 0, 0); // console.log(JSON.stringify(wholeSheet[0], null, 3)); expect(wholeSheet[0]).toMatchObject([ - [{merge: {column: 0, columns: 8, row: 0, rows: 1}, value:"TEST SPREADSHEET"}, - {merge: {column: 1, columns: 8, row: 0, rows: 1}, value:"TEST SPREADSHEET"}, - {merge: {column: 2, columns: 8, row: 0, rows: 1}, value:"TEST SPREADSHEET"}, - {merge: {column: 3, columns: 8, row: 0, rows: 1}, value:"TEST SPREADSHEET"}, - {merge: {column: 4, columns: 8, row: 0, rows: 1}, value:"TEST SPREADSHEET"}, - {merge: {column: 5, columns: 8, row: 0, rows: 1}, value:"TEST SPREADSHEET"} + [{ merge: { column: 0, columns: 8, row: 0, rows: 1 }, value: "TEST SPREADSHEET" }, + { merge: { column: 1, columns: 8, row: 0, rows: 1 }, value: "TEST SPREADSHEET" }, + { merge: { column: 2, columns: 8, row: 0, rows: 1 }, value: "TEST SPREADSHEET" }, + { merge: { column: 3, columns: 8, row: 0, rows: 1 }, value: "TEST SPREADSHEET" }, + { merge: { column: 4, columns: 8, row: 0, rows: 1 }, value: "TEST SPREADSHEET" }, + { merge: { column: 5, columns: 8, row: 0, rows: 1 }, value: "TEST SPREADSHEET" } ], [undefined, undefined, undefined, undefined, undefined, undefined], - [{merge: {column: 0, columns: 2, row: 0, rows:1}, value:"Heading row:"}, - {merge: {column: 1, columns: 2, row: 0, rows:1}, value:"Heading row:"}, - {value: "Name"}, - undefined, - {value: "Age"}, - {value: "Egginess"}], - [{merge: {column: 0, columns: 2, row: 0, rows:2}, value:"Here are some random notes left by the user"}, - {merge: {column: 1, columns: 2, row: 0, rows:2}, value:"Here are some random notes left by the user"}, - {value: "Boris"}, - undefined, - {value: "13"}, - {merge: {column: 0, columns: 1, row: 0, rows: 2}, value:"High"} + [{ merge: { column: 0, columns: 2, row: 0, rows: 1 }, value: "Heading row:" }, + { merge: { column: 1, columns: 2, row: 0, rows: 1 }, value: "Heading row:" }, + { value: "Name" }, + undefined, + { value: "Age" }, + { value: "Egginess" }], + [{ merge: { column: 0, columns: 2, row: 0, rows: 2 }, value: "Here are some random notes left by the user" }, + { merge: { column: 1, columns: 2, row: 0, rows: 2 }, value: "Here are some random notes left by the user" }, + { value: "Boris" }, + undefined, + { value: "13" }, + { merge: { column: 0, columns: 1, row: 0, rows: 2 }, value: "High" } ], - [{merge: {column: 0, columns: 2, row: 1, rows:2}, value:"Here are some random notes left by the user"}, - {merge: {column: 1, columns: 2, row: 1, rows:2}, value:"Here are some random notes left by the user"}, - {value: "Nelly"}, - {merge: {column: 0, columns: 2, row: 0, rows: 1}, value: '14'}, - {merge: {column: 1, columns: 2, row: 0, rows: 1}, value: '14'}, - {merge: {column: 0, columns: 1, row: 1, rows: 2}, value:"High"} + [{ merge: { column: 0, columns: 2, row: 1, rows: 2 }, value: "Here are some random notes left by the user" }, + { merge: { column: 1, columns: 2, row: 1, rows: 2 }, value: "Here are some random notes left by the user" }, + { value: "Nelly" }, + { merge: { column: 0, columns: 2, row: 0, rows: 1 }, value: '14' }, + { merge: { column: 1, columns: 2, row: 0, rows: 1 }, value: '14' }, + { merge: { column: 0, columns: 1, row: 1, rows: 2 }, value: "High" } ], - [{merge: {column: 0, columns: 1, row: 0, rows:2}, value:"And another note"}, - undefined, - {value: "Sid"}, - undefined, - {value: "10"}, - {value: "Medium"}], - [{merge: {column: 0, columns: 1, row: 1, rows:2}, value:"And another note"}, - undefined, - {value: "AVERAGE"}, - undefined, - {value: "11.5"}, - undefined], + [{ merge: { column: 0, columns: 1, row: 0, rows: 2 }, value: "And another note" }, + undefined, + { value: "Sid" }, + undefined, + { value: "10" }, + { value: "Medium" }], + [{ merge: { column: 0, columns: 1, row: 1, rows: 2 }, value: "And another note" }, + undefined, + { value: "AVERAGE" }, + undefined, + { value: "11.5" }, + undefined], [undefined, undefined, undefined, undefined, undefined, undefined] ]); const values = backend.SessionGetInputValues(sid, dataRange, 2); expect(values).toMatchObject([ - { inputValues: [ 'Boris', 'Nelly' ], hasMore: true }, - { inputValues: [ '14', undefined ], hasMore: false }, // The value 14 is merged across both these columns - { inputValues: [ '13', '14' ], hasMore: true }, - { inputValues: [ 'High', 'Medium' ], hasMore: false } // High egginess is merged between rows + { inputValues: ['Boris', 'Nelly'], hasMore: true }, + { inputValues: ['14', undefined], hasMore: false }, // The value 14 is merged across both these columns + { inputValues: ['13', '14'], hasMore: true }, + { inputValues: ['High', 'Medium'], hasMore: false } // High egginess is merged between rows ]); // Do a mapping @@ -244,105 +246,109 @@ describe("Backend tests", () => { const sampleRecords = backend.JobGetSampleRecords(jid, 1, 1, 1); expect(sampleRecords).toMatchObject([ - [ { Name: 'Boris', Age: 13, Egginess: 'High' } ], - [ { Name: 'Nelly', Age: 14, Egginess: 'High' } ], // The 14 is merged across from a cell in column 1 - [ { Name: 'Sid', Age: 10, Egginess: 'Medium' } ] + [{ Name: 'Boris', Age: 13, Egginess: 'High' }], + [{ Name: 'Nelly', Age: 14, Egginess: 'High' }], // The 14 is merged across from a cell in column 1 + [{ Name: 'Sid', Age: 10, Egginess: 'Medium' }] ]); }); test('pad narrow samples', () => { const fixtures = testFiles.get("test").map(prefix_fixture) - for(let [filename, sheetname] of fixtures) { + for (let [filename, sheetname] of fixtures) { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); const dataRange = { sheet: sheetname, - start: {row: 3, column: 0}, - end: {row: 5, column: 3}}; + start: { row: 3, column: 0 }, + end: { row: 5, column: 3 } + }; const samples = backend.SessionGetInputSampleRows(sid, dataRange, - 1, 1, 1); + 1, 1, 1); expect(samples).toMatchObject([ - [ [ {value: 'Boris'}, {value: '13'}, {value: 'High'}, undefined ] ], - [ [ {value: 'Nelly'}, {value: '14'}, {value: 'High'}, undefined ] ], - [ [ {value: 'Sid'}, {value: '10'}, {value: 'Medium'}, undefined ] ] + [[{ value: 'Boris' }, { value: '13' }, { value: 'High' }, undefined]], + [[{ value: 'Nelly' }, { value: '14' }, { value: 'High' }, undefined]], + [[{ value: 'Sid' }, { value: '10' }, { value: 'Medium' }, undefined]] ]); } }); test('suggest data range', () => { const fixtures = testFiles.get("test").map(prefix_fixture) - for( let [filename, sheetname] of fixtures) { + for (let [filename, sheetname] of fixtures) { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); const headerRange = { sheet: sheetname, - start: {row: 2, column: 0}, - end: {row: 2, column: 2}}; + start: { row: 2, column: 0 }, + end: { row: 2, column: 2 } + }; const footerRange = { sheet: sheetname, - start: {row: 6, column: 0}, - end: {row: 5, column: 1}}; + start: { row: 6, column: 0 }, + end: { row: 5, column: 1 } + }; const defaultDataRange = backend.SessionSuggestDataRange(sid, null, null); expect(defaultDataRange).toMatchObject({ sheet: sheetname, - start: {row: 1, column: 0}, - end: {row: 6, column: 6} + start: { row: 1, column: 0 }, + end: { row: 6, column: 6 } }); const headerBasedDataRange = backend.SessionSuggestDataRange(sid, headerRange, null); expect(headerBasedDataRange).toMatchObject({ sheet: sheetname, - start: {row: 3, column: 0}, - end: {row: 6, column: 2} + start: { row: 3, column: 0 }, + end: { row: 6, column: 2 } }); const headerAndFooterBasedDataRange = backend.SessionSuggestDataRange(sid, headerRange, footerRange); expect(headerAndFooterBasedDataRange).toMatchObject({ sheet: sheetname, - start: {row: 3, column: 0}, - end: {row: 6, column: 2} + start: { row: 3, column: 0 }, + end: { row: 6, column: 2 } }); } }); test('sample clamping', () => { const fixtures = testFiles.get("test").map(prefix_fixture) - for(let [filename, sheetname] of fixtures) { + for (let [filename, sheetname] of fixtures) { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); const dataRange = { sheet: sheetname, - start: {row: 3, column: 0}, - end: {row: 5, column: 2}}; + start: { row: 3, column: 0 }, + end: { row: 5, column: 2 } + }; // There are three rows available in the range. Let's see how the sampling // functions allocate them if we ask for too many sample rows. // 10:10:10 -> 1:0:2 because we reduce middleCount first, then split the difference, rounding towards endCount const samples1 = backend.SessionGetInputSampleRows(sid, dataRange, - 10, 10, 10); + 10, 10, 10); expect(samples1).toMatchObject([ - [ [ {value: 'Boris'}, {value: '13'}, {value: 'High'} ] ], - [ ], - [ [ {value: 'Nelly'}, {value: '14'}, {value: 'High'} ], - [ {value: 'Sid'}, {value: '10'}, {value: 'Medium'} ] ] + [[{ value: 'Boris' }, { value: '13' }, { value: 'High' }]], + [], + [[{ value: 'Nelly' }, { value: '14' }, { value: 'High' }], + [{ value: 'Sid' }, { value: '10' }, { value: 'Medium' }]] ]); // Let's see if only middleCount is clamped if startCount and endCount are reasonable const samples2 = backend.SessionGetInputSampleRows(sid, dataRange, - 1, 10, 1); + 1, 10, 1); expect(samples2).toMatchObject([ - [ [ {value: 'Boris'}, {value: '13'}, {value: 'High'} ] ], - [ [ {value: 'Nelly'}, {value: '14'}, {value: 'High'} ] ], - [ [ {value: 'Sid'}, {value: '10'}, {value: 'Medium'} ] ] + [[{ value: 'Boris' }, { value: '13' }, { value: 'High' }]], + [[{ value: 'Nelly' }, { value: '14' }, { value: 'High' }]], + [[{ value: 'Sid' }, { value: '10' }, { value: 'Medium' }]] ]); // Ok, now check the same works for job result sampling @@ -366,32 +372,33 @@ describe("Backend tests", () => { const sampleRecords1 = backend.JobGetSampleRecords(jid, 10, 10, 10); expect(sampleRecords1).toMatchObject([ - [ { Name: 'Boris', Age: 13, Egginess: 'High' } ], - [ ], - [ { Name: 'Nelly', Age: 14, Egginess: 'High' }, - { Name: 'Sid', Age: 10, Egginess: 'Medium' } ] + [{ Name: 'Boris', Age: 13, Egginess: 'High' }], + [], + [{ Name: 'Nelly', Age: 14, Egginess: 'High' }, + { Name: 'Sid', Age: 10, Egginess: 'Medium' }] ]); // Let's see if only middleCount is clamped if startCount and endCount are reasonable const sampleRecords2 = backend.JobGetSampleRecords(jid, 1, 10, 1); expect(sampleRecords2).toMatchObject([ - [ { Name: 'Boris', Age: 13, Egginess: 'High' } ], - [ { Name: 'Nelly', Age: 14, Egginess: 'High' } ], - [ { Name: 'Sid', Age: 10, Egginess: 'Medium' } ] + [{ Name: 'Boris', Age: 13, Egginess: 'High' }], + [{ Name: 'Nelly', Age: 14, Egginess: 'High' }], + [{ Name: 'Sid', Age: 10, Egginess: 'Medium' }] ]); } }); test('sampling algorithm', () => { const fixtures = testFiles.get("test").map(prefix_fixture) - for(let [filename, sheetname] of fixtures) { + for (let [filename, sheetname] of fixtures) { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); const dataRange = { sheet: sheetname, - start: {row: 3, column: 0}, - end: {row: 5, column: 2}}; + start: { row: 3, column: 0 }, + end: { row: 5, column: 2 } + }; // We can be fairly confident startCount and endCount work right, but // middleCount has an *algorithm* that tries to pick a random sample of rows @@ -401,47 +408,47 @@ describe("Backend tests", () => { // First: Pick all 3 const samples1 = backend.SessionGetInputSampleRows(sid, dataRange, - 0, 10, 0); + 0, 10, 0); // No rows in the start and end samples, 3 rows in the middle sample expect(samples1[0]).toHaveLength(0); expect(samples1[1]).toHaveLength(3); expect(samples1[2]).toHaveLength(0); expect(samples1[1]).toMatchObject([ - [ {value: 'Boris'}, {value: '13'}, {value: 'High'} ], - [ {value: 'Nelly'}, {value: '14'}, {value: 'High'} ], - [ {value: 'Sid'}, {value: '10'}, {value: 'Medium'} ]]); + [{ value: 'Boris' }, { value: '13' }, { value: 'High' }], + [{ value: 'Nelly' }, { value: '14' }, { value: 'High' }], + [{ value: 'Sid' }, { value: '10' }, { value: 'Medium' }]]); // Now pick 1 const samples2 = backend.SessionGetInputSampleRows(sid, dataRange, - 0, 1, 0); + 0, 1, 0); // No rows in the start and end samples, 1 row in the middle sample expect(samples2[0]).toHaveLength(0); expect(samples2[1]).toHaveLength(1); expect(samples2[2]).toHaveLength(0); expect([ - [ {value: 'Boris'}, {value: '13'}, {value: 'High'} ], - [ {value: 'Nelly'}, {value: '14'}, {value: 'High'} ], - [ {value: 'Sid'}, {value: '10'}, {value: 'Medium'} ]]).toContainEqual(samples2[1][0]); + [{ value: 'Boris' }, { value: '13' }, { value: 'High' }], + [{ value: 'Nelly' }, { value: '14' }, { value: 'High' }], + [{ value: 'Sid' }, { value: '10' }, { value: 'Medium' }]]).toContainEqual(samples2[1][0]); // FIXME: Do 100 samples of 1 record, ensuring we get a roughly equal distribution of results // Now pick 2 const samples3 = backend.SessionGetInputSampleRows(sid, dataRange, - 0, 2, 0); + 0, 2, 0); // No rows in the start and end samples, 2 rows in the middle sample expect(samples3[0]).toHaveLength(0); expect(samples3[1]).toHaveLength(2); expect(samples3[2]).toHaveLength(0); expect([ // Sid can't be first, if there's two rows and they're in order - [ {value: 'Boris'}, {value: '13'}, {value: 'High'} ], - [ {value: 'Nelly'}, {value: '14'}, {value: 'High'} ]]).toContainEqual(samples3[1][0]); + [{ value: 'Boris' }, { value: '13' }, { value: 'High' }], + [{ value: 'Nelly' }, { value: '14' }, { value: 'High' }]]).toContainEqual(samples3[1][0]); expect([ // Boris can't be second, if there's two rows and they're in order - [ {value: 'Nelly'}, {value: '14'}, {value: 'High'} ], - [ {value: 'Sid'}, {value: '10'}, {value: 'Medium'} ]]).toContainEqual(samples3[1][1]); + [{ value: 'Nelly' }, { value: '14' }, { value: 'High' }], + [{ value: 'Sid' }, { value: '10' }, { value: 'Medium' }]]).toContainEqual(samples3[1][1]); // Test they're not equal. expect(samples3[1][0]).not.toMatchObject(samples3[1][1]); } @@ -449,13 +456,13 @@ describe("Backend tests", () => { test('trailing stub rows in dimensions', () => { const fixtures = testFiles.get("tribbles").map(prefix_fixture) - for(let [filename, sheetname] of fixtures) { + for (let [filename, sheetname] of fixtures) { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); const dimensions = backend - .SessionGetInputDimensions(sid) - .sheetDimensions.get(sheetname); + .SessionGetInputDimensions(sid) + .sheetDimensions.get(sheetname); // There are physically 23 rows, because some of them have styles even // if they have no content. We only expect to process 9 rows. @@ -468,14 +475,15 @@ describe("Backend tests", () => { test('validation', () => { const fixtures = testFiles.get("test-validation").map(prefix_fixture) - for( const [filename, sheetname] of fixtures) { + for (const [filename, sheetname] of fixtures) { const sid = backend.CreateSession(); backend.SessionSetFile(sid, filename); const dataRange = { sheet: sheetname, - start: {row: 3, column: 0}, - end: {row: 8, column: 3}}; + start: { row: 3, column: 0 }, + end: { row: 8, column: 3 } + }; const mapping = { attributeMappings: { @@ -494,29 +502,31 @@ test('validation', () => { // Do a mapping - const jid = backend.SessionPerformMappingJob(sid, dataRange, mapping); + const jid = backend.SessionPerformMappingJob(sid, dataRange, mapping, false); // Look at the job results const jobSummary = backend.JobGetSummary(jid); expect(jobSummary).toMatchObject({ - recordCount: 3, - errorCount: 2, + recordCount: 2, + errorCount: 3, warningCount: 1 }); const warnings = backend.JobGetWarnings(jid); // Weight is optional so errors here become warnings - expect(warnings).toMatchObject(new Map([[8, [['Weight', 'This is not a valid number']]]])); + expect(warnings).toMatchObject(new Map([[4, ['Row is empty']]])); const errors = backend.JobGetErrors(jid); - expect(errors).toMatchObject(new Map([[6, [['Age', 'This is not a valid number']]], - [7, [['Age', 'This is not a valid number']]]])); + expect(errors).toMatchObject(new Map([ + [6, [{ "row": 6, "field": 'Age', "message": 'This is not a valid number' }]], + [7, [{ "row": 7, "field": 'Name', "message": 'A value must be provided' }, { "row": 7, "field": 'Age', "message": 'This is not a valid number' }]], + [8, [{ "row": 8, "field": 'Weight', "message": 'This is not a valid number' }]] + ])); const allRecords = backend.JobGetRecords(jid, 0, 3); expect(allRecords).toMatchObject([ { Name: 'Boris', Age: 13, Egginess: 'High', Weight: 5.5 }, - { Name: 'Nelly', Age: 10}, - { Name: 'Dave', Age: 15} + { Name: 'Nelly', Age: 10 } ]); backend.JobDelete(sid, jid); diff --git a/lib/importer/src/config.js b/lib/importer/src/config.js index 59aafa40..916298c6 100644 --- a/lib/importer/src/config.js +++ b/lib/importer/src/config.js @@ -10,7 +10,6 @@ // we isolate the fields specific to the plugin here, and store the location of // the config.json so that we can write write to it at runtime. // -const attrTypes = require("./attribute-types") const fs = require("fs"); const fse = require("fs-extra"); const path = require("node:path"); @@ -24,16 +23,6 @@ exports.MappingField = class { this.required = f.required this.type = f.type } - - mapperForField = () => { - const fieldType = attrTypes.typeFromName(this.type) - if (this.required) { - return attrTypes.requiredType(fieldType) - } - - return attrTypes.optionalType(fieldType) - } - } exports.PluginConfig = class { @@ -47,6 +36,8 @@ exports.PluginConfig = class { if (this.fields.find(Boolean)) { if (typeof this.fields[0] === 'string' || this.fields[0] instanceof String) { this.fields = this.fields.map((x) => new exports.MappingField({ name: x, type: "text", required: false })) + } else { + this.fields = this.fields.map((x) => new exports.MappingField({ name: x.name, type: x.type, required: x.required })) } } diff --git a/lib/importer/src/filters.js b/lib/importer/src/filters.js index 3ff5bcb8..27d6602e 100644 --- a/lib/importer/src/filters.js +++ b/lib/importer/src/filters.js @@ -9,5 +9,10 @@ const currency = (content) => { return "£" + num.toLocaleString() } +// Converts the provided text to a slug style format that +// can be used as a HTML id (for instance) +const slugify = (content) => { + return content.toLowerCase().replace(/[^\w\s']|_/g, " ").replace(/\s+/g, "-"); +} -module.exports = { currency } +module.exports = { currency, slugify } diff --git a/lib/importer/src/functions.js b/lib/importer/src/functions.js index a0406676..d8995e07 100644 --- a/lib/importer/src/functions.js +++ b/lib/importer/src/functions.js @@ -3,6 +3,8 @@ const sheets_lib = require("./sheets.js"); const IMPORTER_SESSION_KEY = "importer.session"; const IMPORTER_ERROR_KEY = "importer.error"; +const IMPORTER_ERROR_DATA_KEY = "importer.error.postdata"; +const IMPORTER_ERROR_EXTRA_KEY = "importer.error.extra"; //-------------------------------------------------------------------- // Allows the templates to get a list of viable sheet names, and for @@ -27,11 +29,26 @@ const importSheetPreview = (data) => { //-------------------------------------------------------------------- const importerError = (data) => { if (data[IMPORTER_ERROR_KEY]) { - return { text: data[IMPORTER_ERROR_KEY] }; + return { text: data[IMPORTER_ERROR_KEY], extra: data[IMPORTER_ERROR_EXTRA_KEY], data: data[IMPORTER_ERROR_DATA_KEY] }; } return false; } + +//-------------------------------------------------------------------- +// When submitting a mapping and getting an error response, this +// function will allow the mapping component to select the previously +// submitted values for each field. +//-------------------------------------------------------------------- +const importerErrorMappingData = (error, key) => { + if (!error || !error.data) { + return "" + } + + const m = new Map(Object.entries(error.data).map(([k, v]) => [parseInt(k), v])) + return m.get(key) || "" +} + //-------------------------------------------------------------------- // Allows a template to obtain `count` rows from the start of the data // range. @@ -140,6 +157,8 @@ const importerMappedData = (data) => { extraRecordCount: mapResults.extraRecordCount, errorCount: mapResults.errorCount, warningCount: mapResults.warningCount, + warnings: mapResults.warnings, + errors: mapResults.errors }; } @@ -203,6 +222,7 @@ const parseNumberFromString = (s) => { module.exports = { importerError, + importerErrorMappingData, importSheetPreview, importerGetRows, importerGetHeaders, diff --git a/lib/importer/src/index.js b/lib/importer/src/index.js index f30c246b..13a533ad 100644 --- a/lib/importer/src/index.js +++ b/lib/importer/src/index.js @@ -9,6 +9,8 @@ const tpl_filters = require("./filters.js") const IMPORTER_SESSION_KEY = "importer.session"; const IMPORTER_ERROR_KEY = "importer.error"; +const IMPORTER_ERROR_DATA_KEY = "importer.error.postdata"; +const IMPORTER_ERROR_EXTRA_KEY = "importer.error.extra"; //-------------------------------------------------------------------- // The endpoints used to receive POST requests from importer macros, @@ -75,6 +77,7 @@ exports.Initialise = (config, router, prototypeKit) => { const cleanRequest = (request) => { delete request.session.data['reference_number']; delete request.session.data[IMPORTER_ERROR_KEY]; + delete request.session.data[IMPORTER_ERROR_EXTRA_KEY]; }; @@ -287,9 +290,38 @@ exports.Initialise = (config, router, prototypeKit) => { return; } + request.session.data['reference_number'] = session.id.match(/\d+/g).join("").substring(0, 8); + session.mapping = request.body; - request.session.data['reference_number'] = session.id.match(/\d+/g).join("").substring(0, 8); + const values = new Array(); + + for (const value of Object.values(session.mapping)) { + if (value != '') { + values.push(value) + } + } + + // For each key in the config that is required, we need to ensure that the field name is present + // in the values array. If not then we will + const required_fields = plugin_config.fields + .filter((f) => f.required) + .filter((f) => !values.includes(f.name)) + .map((f) => f.name) + + // If required_fields has any values left, then they are required fields not present in the + // mapping. In which case we should return an error for the user. + if (required_fields.length > 0) { + request.session.data[IMPORTER_ERROR_KEY] = "The following fields are required" + request.session.data[IMPORTER_ERROR_EXTRA_KEY] = required_fields + request.session.data[IMPORTER_ERROR_DATA_KEY] = session.mapping + response.redirect(request.get('Referrer')); + return; + } else { + delete request.session.data[IMPORTER_ERROR_KEY] + delete request.session.data[IMPORTER_ERROR_EXTRA_KEY] + delete request.session.data[IMPORTER_ERROR_DATA_KEY] + } // Ensure the session is persisted. Currently in session, eventually another way request.session.data[IMPORTER_SESSION_KEY] = session; diff --git a/lib/importer/src/sheets.js b/lib/importer/src/sheets.js index 41f8bd15..5df6e9a2 100644 --- a/lib/importer/src/sheets.js +++ b/lib/importer/src/sheets.js @@ -1,4 +1,5 @@ const backend = require("./backend"); +const attributeTypes = require('./attribute-types'); const DEFAULT_PREVIEW_LIMIT = 20 @@ -236,14 +237,22 @@ exports.MapData = (session, previewLimit = DEFAULT_PREVIEW_LIMIT) => { // Convert session.mapping (a map from column index -> attribute name) into a mapping for the backend let rewrittenMapping = new Map(); + const attrTypes = {} + for (const [columnIndex, attributeName] of Object.entries(session.mapping)) { if (attributeName !== null && attributeName !== undefined && attributeName != '') { rewrittenMapping[attributeName] = parseInt(columnIndex); } + + const f = session.fields.find((x) => x.name == attributeName) + if (f) { + attrTypes[f.name] = attributeTypes.mapperForField(f) + } } const mapping = { - attributeMappings: rewrittenMapping + attributeMappings: rewrittenMapping, + attributeTypes: attrTypes }; // Apply the mapping diff --git a/lib/importer/src/validation.test.js b/lib/importer/src/validation.test.js new file mode 100644 index 00000000..2405d947 --- /dev/null +++ b/lib/importer/src/validation.test.js @@ -0,0 +1,326 @@ +/** + * Validation tests + */ + +const session = require('./session'); +const backend = require('./backend'); +const attributeTypes = require('./attribute-types'); +const mock_files = require('mock-fs'); + + +describe("Validation tests", () => { + + test('Validation happy path', () => { + const sess = createTestSession([{ name: 'field', type: 'text', required: false }], "validation-test.csv") + expect(sess).not.toBe(null); + + const config = { + fields: [ + fieldHelper("Title", "text", false), + fieldHelper("First name", "text", false), + fieldHelper("Surname", "text", false), + fieldHelper("Employee number", "text", false), + fieldHelper("Employment start date", "text", false), + fieldHelper("Salary", "text", false), + fieldHelper("Contribution percentage", "text", false), + fieldHelper("Payment date", "text", false), + ] + } + + withCleanup(() => { + // Mock the config file and tell this test that it is the current directory + mock_files({ + './empty/app/config.json': JSON.stringify(config) + }) + process.env.KIT_PROJECT_DIR = './empty/app/config.json' + + const dataRange = { + sheet: "Sheet1", + start: { row: 0, column: 0 }, + end: { row: 2, column: 10 } + }; + + const mapping = createTestMapping( + { + 0: "Title", + 1: "First name", + 2: "Surname", + 3: "Employee number", + 4: "Employment start date", + 5: "Salary", + 6: "Contribution percentage", + 7: "Payment date" + + }, config.fields + ) + + const jid = backend.SessionPerformMappingJob(sess.id, dataRange, mapping); + + const jobSummary = backend.JobGetSummary(jid); + expect(jobSummary).toMatchObject({ + recordCount: 3, + errorCount: 0, + warningCount: 0 + }); + + const warnings = backend.JobGetWarnings(jid); + expect(warnings).toMatchObject({}); + + const errors = backend.JobGetErrors(jid); + expect(errors).toMatchObject({}); + + + }, () => { + mock_files.restore(); + }); + }); + + test('Missing required values', () => { + const sess = createTestSession([{ name: 'field', type: 'text', required: false }], "validation-test.csv") + expect(sess).not.toBe(null); + + const config = { + fields: [ + fieldHelper("Title", "text", false), + fieldHelper("First name", "text", false), + fieldHelper("Surname", "text", false), + fieldHelper("Employee number", "text", false), + fieldHelper("Employment start date", "text", false), + fieldHelper("Salary", "text", true), + fieldHelper("Contribution percentage", "text", true), + fieldHelper("Payment date", "text", false), + ] + } + + withCleanup(() => { + // Mock the config file and tell this test that it is the current directory + mock_files({ + './empty/app/config.json': JSON.stringify(config) + }) + process.env.KIT_PROJECT_DIR = './empty/app/config.json' + + const dataRange = { + sheet: "Sheet1", + start: { row: 0, column: 0 }, + end: { row: 2, column: 10 } + }; + + const mapping = createTestMapping( + { + 0: "Employee number", + 1: "Title", + 2: "First name", + 3: "Surname", + 4: "Employment start date", + 5: "Salary", + 6: "Contribution percentage", + 7: "Payment date" + + }, config.fields + ) + + const jid = backend.SessionPerformMappingJob(sess.id, dataRange, mapping); + + const jobSummary = backend.JobGetSummary(jid); + expect(jobSummary).toMatchObject({ + recordCount: 2, + errorCount: 1, + warningCount: 0 + }); + + const errors = backend.JobGetErrors(jid); + expect(errors).not.toBe(null) + + const row1Errors = errors.get(1) + + expect(row1Errors[0].field).toBe("Salary") + expect(row1Errors[0].message).toBe("A value must be provided") + + expect(row1Errors[1].field).toBe("Contribution percentage") + expect(row1Errors[1].message).toBe("A value must be provided") + }, () => { + mock_files.restore(); + }); + }); + + test('Incorrect field type', () => { + const sess = createTestSession([{ name: 'field', type: 'text', required: false }], "validation-test.csv") + expect(sess).not.toBe(null); + + const config = { + fields: [ + fieldHelper("Title", "number", false), + fieldHelper("First name", "text", false), + fieldHelper("Surname", "text", false), + fieldHelper("Employee number", "text", false), + fieldHelper("Employment start date", "text", false), + fieldHelper("Salary", "text", false), + fieldHelper("Contribution percentage", "text", false), + fieldHelper("Payment date", "text", false), + ] + } + + withCleanup(() => { + // Mock the config file and tell this test that it is the current directory + mock_files({ + './empty/app/config.json': JSON.stringify(config) + }) + process.env.KIT_PROJECT_DIR = './empty/app/config.json' + + const dataRange = { + sheet: "Sheet1", + start: { row: 0, column: 0 }, + end: { row: 2, column: 10 } + }; + + const mapping = createTestMapping( + { + 0: "Employee number", + 1: "Title", + 2: "First name", + 3: "Surname", + 4: "Employment start date", + 5: "Salary", + 6: "Contribution percentage", + 7: "Payment date" + + }, config.fields + ) + + const jid = backend.SessionPerformMappingJob(sess.id, dataRange, mapping); + + const jobSummary = backend.JobGetSummary(jid); + expect(jobSummary).toMatchObject({ + recordCount: 0, + errorCount: 3, + warningCount: 0 + }); + + const errors = backend.JobGetErrors(jid); + expect(errors).not.toBe(null) + + const row1Errors = errors.get(1) + + expect(row1Errors[0].field).toBe("Title") + expect(row1Errors[0].message).toBe("This is not a valid number") + }, () => { + mock_files.restore(); + }); + }); + + test('Structural warnings', () => { + const sess = createTestSession([{ name: 'field', type: 'text', required: false }], "validation-struct-err.csv") + expect(sess).not.toBe(null); + + const config = { + fields: [ + fieldHelper("Title", "text", false), + fieldHelper("First name", "text", false), + fieldHelper("Surname", "text", false), + fieldHelper("Employee number", "text", false), + fieldHelper("Employment start date", "text", false), + fieldHelper("Salary", "text", false), + fieldHelper("Contribution percentage", "text", false), + fieldHelper("Payment date", "text", false), + ] + } + + withCleanup(() => { + // Mock the config file and tell this test that it is the current directory + mock_files({ + './empty/app/config.json': JSON.stringify(config) + }) + process.env.KIT_PROJECT_DIR = './empty/app/config.json' + + const dataRange = { + sheet: "Sheet1", + start: { row: 0, column: 0 }, + end: { row: 2, column: 10 } + }; + + const mapping = createTestMapping( + { + 0: "Employee number", + 1: "Title", + 2: "First name", + 3: "Surname", + 4: "Employment start date", + 5: "Salary", + 6: "Contribution percentage", + 7: "Payment date" + + }, config.fields + ) + + const jid = backend.SessionPerformMappingJob(sess.id, dataRange, mapping); + + const jobSummary = backend.JobGetSummary(jid); + expect(jobSummary).toMatchObject({ + recordCount: 2, + errorCount: 0, + warningCount: 1 + }); + + const warn = backend.JobGetWarnings(jid); + expect(warn).not.toBe(null) + + expect(warn.get(2)).not.toBe(null) + expect(warn.get(2)[0]).toBe("Row is empty") + }, () => { + mock_files.restore(); + }); + }); +}); + + + +function withCleanup(fn, cleanup) { + try { + fn(); + } finally { + cleanup(); + } +} + +const createTestSession = (fields, fixture) => { + const response = session.CreateSession( + { fields: fields, uploadPath: "../../fixtures" }, + { + file: { + mimetype: "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + filename: fixture + } + } + ); + + backend.SessionSetFile(response.session.id, "../../fixtures/" + fixture); + return response.session; +} + +const createTestMapping = (mapping, fields) => { + // Convert session.mapping (a map from column index -> attribute name) into a mapping for the backend + let rewrittenMapping = new Map(); + const attrTypes = {} + + for (const [columnIndex, attributeName] of Object.entries(mapping)) { + if (attributeName !== null && attributeName !== undefined && attributeName != '') { + rewrittenMapping[attributeName] = parseInt(columnIndex); + } + + const f = fields.find((x) => x.name == attributeName) + if (f) { + attrTypes[f.name] = attributeTypes.mapperForField(f) + } + } + + return { + attributeMappings: rewrittenMapping, + attributeTypes: attrTypes + }; +} + +const fieldHelper = (name, typ = "text", required = true) => { + return { + name: name, type: typ, required: required + } +} diff --git a/lib/importer/templates/review.html b/lib/importer/templates/review.html index e02fc3cf..3cee7d0b 100644 --- a/lib/importer/templates/review.html +++ b/lib/importer/templates/review.html @@ -1,15 +1,77 @@ {% extends "layouts/main.html" %} {% block pageTitle %} {{ serviceName }} – GOV.UK Prototype Kit {% endblock %} -{% block beforeContent %} -{{ govukBackLink({ text: "Back", href: "javascript:window.history.back()" }) }} -{% endblock %} {% block content %} +{% set results = importerMappedData(data) %} +

Review your data

+

+ During import there were {{results.errorCount}} errors and {{results.warningCount}} warnings. +

+ + {% if results.errorCount != 0 %} +
{{ h.name }} {{ h.examples }}
+ + + + + + + + + {% for idx, error in results.errors %} + + + + + {% endfor %} + +
Errors
RowError
{{idx}} {{error}}
+ {% endif %} + + {% if results.warningCount != 0 %} + + + + + + + + + + {% for idx, warning in results.warnings %} + + + + + {% endfor %} + +
Warnings
RowWarning
{{idx}} {{warning}}
+ {% endif %} + +

+ The uploaded data contains the following data: + +

+

+ +

+ If the details above are correct you can continue to submit your return by clicking the 'Submit' button below. +

+ +

+ If there were problems with the uploaded data, you should correct the issues and re-upload the data +

+ +
{{ govukButton({ text: "Submit" }) }} diff --git a/prototypes/basic/app/config.json b/prototypes/basic/app/config.json index 01737a60..9592ced6 100644 --- a/prototypes/basic/app/config.json +++ b/prototypes/basic/app/config.json @@ -1,14 +1,46 @@ { "serviceName": "Upload monthly pension return", "fields": [ - "Title", - "First name", - "Surname", - "Employee number", - "Employment start date", - "Salary", - "Contribution percentage", - "Payment date" + { + "name": "Title", + "type": "text", + "required": false + }, + { + "name": "First name", + "type": "text", + "required": false + }, + { + "name": "Surname", + "type": "text", + "required": false + }, + { + "name": "Employee number", + "type": "text", + "required": false + }, + { + "name": "Employment start date", + "type": "text", + "required": false + }, + { + "name": "Salary", + "type": "text", + "required": false + }, + { + "name": "Contribution percentage", + "type": "text", + "required": false + }, + { + "name": "Payment date", + "type": "text", + "required": false + } ], "uploadPath": "" } diff --git a/prototypes/basic/app/views/review.html b/prototypes/basic/app/views/review.html index f9305f0e..2bf1d8de 100644 --- a/prototypes/basic/app/views/review.html +++ b/prototypes/basic/app/views/review.html @@ -52,6 +52,40 @@

Review your data

During import there were {{results.errorCount}} errors and {{results.warningCount}} warnings.

+ {% if results.errorCount != 0 %} + + + + + + + + + + + {% for idx, errors in results.errors %} + {% for e in errors %} + + + + + + {% endfor %} + {% endfor %} + +
Errors
RowFieldError
{{idx}} {{e.field}} {{e.message}}
+ {% endif %} + + {% if results.warningCount != 0 %} +

+

+

+ {% endif %} +

The uploaded data contains the following data: