Skip to content

Commit

Permalink
Merge pull request #37 from digital-land/requiredFieldsErrors
Browse files Browse the repository at this point in the history
Required fields errors
  • Loading branch information
GeorgeGoodall authored Dec 21, 2023
2 parents de7a241 + be9b0b2 commit 1236c97
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 70 deletions.
5 changes: 3 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ app.use((err, req, res, next) => {
error: err
})

err.template = err.template || 'errorPages/500'

// handle session expired
if (err.code === 'SESSION_TIMEOUT') {
err.template = 'session-expired'
Expand All @@ -103,8 +105,7 @@ app.use((err, req, res, next) => {

// show error page
err.status = err.status || 500
err.template = err.template || 'error'
res.status(err.status).render('errorPages/500', { err })
res.status(err.status).render(err.template, { err })
})

app.get('/health', (req, res) => {
Expand Down
19 changes: 14 additions & 5 deletions src/controllers/errorsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ class ErrorsController extends PageController {
get (req, res, next) {
const validationResult = req.sessionModel.get('validationResult')

const { aggregatedIssues, issueCounts } = this.getAggregatedErrors(validationResult)
const { aggregatedIssues, issueCounts, missingColumns } = this.getAggregatedErrors(validationResult)

const rows = Object.values(aggregatedIssues)

req.form.options.rows = rows
req.form.options.issueCounts = issueCounts
req.form.options.columnNames = Object.keys(rows[0])
req.form.options.missingColumns = missingColumns
req.form.options.columnNames = rows.length > 0 ? Object.keys(rows[0]) : []

const dataSetValue = req.sessionModel.get('dataset')

Expand Down Expand Up @@ -52,12 +53,13 @@ class ErrorsController extends PageController {
}

if (entryNumber in aggregatedIssues) {
aggregatedIssues[entryNumber][issue.field] = {
const columnName = this.lookupMappedColumnNameFromOriginal(issue.field, apiResponseData['column-field-log'])
aggregatedIssues[entryNumber][columnName] = {
issue: {
type: issue['issue-type'],
description: issue.description
},
value: rowValues[this.lookupOriginalColumnNameFromMapped(issue.field, apiResponseData['column-field-log'])]
value: rowValues[issue.field]
}
const key = issue.field + '_' + issue['issue-type']
if (issueCounts[key]) {
Expand All @@ -74,7 +76,14 @@ class ErrorsController extends PageController {
}
})

return { aggregatedIssues, issueCounts }
const missingColumns = []
apiResponseData['column-field-log'].forEach(columnField => {
if (columnField.missing) {
missingColumns.push(columnField.field)
}
})

return { aggregatedIssues, issueCounts, missingColumns }
}

lookupMappedColumnNameFromOriginal (originalColumnName, columnFieldLogs) {
Expand Down
7 changes: 4 additions & 3 deletions src/controllers/uploadController.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ class UploadController extends PageController {
async post (req, res, next) {
if (req.file !== undefined) {
req.body.datafile = req.file
let jsonResult = {}
try {
const jsonResult = await this.validateFile({
jsonResult = await this.validateFile({
filePath: req.file.path,
fileName: req.file.originalname,
dataset: req.sessionModel.get('dataset'),
Expand All @@ -32,11 +33,11 @@ class UploadController extends PageController {
sessionId: req.sessionID,
ipAddress: req.ip
})
this.errorCount = jsonResult['issue-log'].filter(issue => issue.severity === severityLevels.error).length
req.body.validationResult = jsonResult
} catch (error) {
logger.error({ type: 'Upload', message: 'Error uploading file', error })
}
this.errorCount = jsonResult['issue-log'].filter(issue => issue.severity === severityLevels.error).length + jsonResult['column-field-log'].filter(log => log.missing).length
req.body.validationResult = jsonResult
}
super.post(req, res, next)
}
Expand Down
67 changes: 36 additions & 31 deletions src/views/errors.html
Original file line number Diff line number Diff line change
Expand Up @@ -27,47 +27,52 @@ <h1 class="govuk-heading-l">

<li>{{issue.count}} {{issue.type}} {{issueString}}, relating to column {{issue.field}}</li>
{% endfor %}
{% for column in options.missingColumns %}
<li>Missing required column {{column}}</li>
{% endfor %}
</ul>
</div>
</div>

<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<h2 class="govuk-heading-m">
Check your errors
</h2>
<div class="app-scrollable-container">
<table class="govuk-table">
<thead class="govuk-table__head">
<tr class="govuk-table__row">
{% for column in options.columnNames %}
<th scope="col" class="govuk-table__header">{{column}}</th>
{% endfor %}
</tr>
</thead>
<tbody class="govuk-table__body">
{% for row in options.rows %}
{% if options.rows|length > 0 %}
<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<h2 class="govuk-heading-m">
Check your errors
</h2>
<div class="app-scrollable-container">
<table class="govuk-table">
<thead class="govuk-table__head">
<tr class="govuk-table__row">
{% for columnName, column in row %}
<td class="govuk-table__cell app-wrap">
{% if column.issue %}
{{ govukInsetText({
classes: "app-inset-text---error",
html: '<p class="app-inset-text__value">'+column.value+'</p> <p class="app-inset-text__error">'+column.issue.description +'</p>'
}) }}
{% else %}
{{column.value}}
{% endif %}
</td>
{% for column in options.columnNames %}
<th scope="col" class="govuk-table__header">{{column}}</th>
{% endfor %}
</tr>
{% endfor %}
</thead>
<tbody class="govuk-table__body">
{% for row in options.rows %}
<tr class="govuk-table__row">
{% for columnName, column in row %}
<td class="govuk-table__cell app-wrap">
{% if column.issue %}
{{ govukInsetText({
classes: "app-inset-text---error",
html: '<p class="app-inset-text__value">'+column.value+'</p> <p class="app-inset-text__error">'+column.issue.description +'</p>'
}) }}
{% else %}
{{column.value}}
{% endif %}
</td>
{% endfor %}
</tr>
{% endfor %}

</tbody>
</table>
</tbody>
</table>
</div>
</div>
</div>
</div>
{% endif %}

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
Expand Down
3 changes: 2 additions & 1 deletion test/mock-api/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ app.post(config.api.validationEndpoint, (req, res) => {

if (filename !== 'conservation-area-errors.csv') {
_toSend['issue-log'] = []
_toSend['missing-columns'] = []
}
res.json(_toSend)
})

app.listen(config.api.port, () => {
console.log('listening on port 8082')
console.log('listening on port ' + config.api.port)
})
18 changes: 12 additions & 6 deletions test/testData/API_RUN_PIPELINE_RESPONSE.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"converted-csv": [
{"Reference": "CA6", "Name": "Camden Square", "Geometry": "POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))", "Start date": "40/04/1980", "Legislation": "", "Notes": "", "Point": "POINT (-0.130484959448 51.544845663239)", "End date": "", "Document URL": "https://www.camden.gov.uk/camden-square-conservation-area-appraisal-and-management-strategy"},
{"Reference": "CA20", "Name": "Holly Lodge Estate", "Geometry": "POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))", "Start date": "01/06/1992", "Legislation": "", "Notes": "", "Point": "POINT (-0.150097204178 51.564975754948)", "End date": "", "Document URL": "https://www.camden.gov.uk/holly-lodge-conservation-area"},
{"Reference": "CA9", "Name": "Dartmouth Park", "Geometry": "POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))", "Start date": "01/06/1992", "Legislation": "", "Notes": "", "Point": "POINT (-0.145442349961 51.559999511433)", "End date": "", "Document URL": "https://www.camden.gov.uk/dartmouth-park-conservation-area-appraisal-and-management-strategy"}
{"Name": "Camden Square", "Geometry": "POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))", "Start date": "40/04/1980", "Legislation": "", "Notes": "", "Point": "POINT (-0.130484959448 51.544845663239)", "End date": "", "Document URL": "https://www.camden.gov.uk/camden-square-conservation-area-appraisal-and-management-strategy"},
{"Name": "Holly Lodge Estate", "Geometry": "POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))", "Start date": "01/06/1992", "Legislation": "", "Notes": "", "Point": "POINT (-0.150097204178 51.564975754948)", "End date": "", "Document URL": "https://www.camden.gov.uk/holly-lodge-conservation-area"},
{"Name": "Dartmouth Park", "Geometry": "POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))", "Start date": "01/06/1992", "Legislation": "", "Notes": "", "Point": "POINT (-0.145442349961 51.559999511433)", "End date": "", "Document URL": "https://www.camden.gov.uk/dartmouth-park-conservation-area-appraisal-and-management-strategy"}
],
"issue-log": [
{"dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "line-number": "2", "entry-number": "1", "field": "Start date", "issue-type": "invalid-value", "description": "invalid-value", "value": "40/04/1980", "severity": "error"},
Expand All @@ -17,8 +17,14 @@
]
},
"column-field-log": [
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "WKT", "field": "geometry"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "details", "field": "name"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "ogc_fid", "field": "reference"}
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "Geometry", "field": "geometry"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "Name", "field": "name"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "Point", "field": "point"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "Start date", "field": "start-date"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "End date", "field": "end-date"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "Document URL", "field": "document-URL"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "Legislation", "field": "legislation"},
{"entry-date": "2023-10-03T10:13:45Z", "dataset": "conservation-area", "resource": "0b4284077da580a6daea59ee2227f9c7c55a9a45d57ef470d82418a4391ddf9a", "column": "Notes", "field": "notes"},
{"field": "reference", "missing": true}
]
}
101 changes: 80 additions & 21 deletions test/unit/errorsController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,51 +33,46 @@ describe('ErrorsController', () => {
const expectedFormValues = {
options: {
columnNames: [
'Reference',
'Name',
'Geometry',
'Start date',
'Legislation',
'Notes',
'Point',
'End date',
'Document URL'
'name',
'geometry',
'start-date',
'legislation',
'notes',
'point',
'end-date',
'document-URL'
],
rows: [
{
'Document URL': {
'document-URL': {
issue: false,
value: 'https://www.camden.gov.uk/camden-square-conservation-area-appraisal-and-management-strategy'
},
'End date': {
'end-date': {
issue: false,
value: ''
},
Geometry: {
geometry: {
issue: false,
value: 'POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))'
},
Legislation: {
legislation: {
issue: false,
value: ''
},
Name: {
name: {
issue: false,
value: 'Camden Square'
},
Notes: {
notes: {
issue: false,
value: ''
},
Point: {
point: {
issue: false,
value: 'POINT (-0.130484959448 51.544845663239)'
},
Reference: {
issue: false,
value: 'CA6'
},
'Start date': {
'start-date': {
issue: {
type: 'invalid-value',
description: 'invalid-value'
Expand All @@ -86,6 +81,9 @@ describe('ErrorsController', () => {
}
}
],
missingColumns: [
'reference'
],
issueCounts: {
'Start date_invalid-value': {
count: 1,
Expand All @@ -99,4 +97,65 @@ describe('ErrorsController', () => {

expect(req.form).toEqual(expectedFormValues)
})

describe('getAggregatedErrors', () => {
it('returns the correct values', () => {
const expectedAggregatedIssues = {
1: {
'document-URL': {
issue: false,
value: 'https://www.camden.gov.uk/camden-square-conservation-area-appraisal-and-management-strategy'
},
'end-date': {
issue: false,
value: ''
},
geometry: {
issue: false,
value: 'POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))'
},
legislation: {
issue: false,
value: ''
},
name: {
issue: false,
value: 'Camden Square'
},
notes: {
issue: false,
value: ''
},
point: {
issue: false,
value: 'POINT (-0.130484959448 51.544845663239)'
},
'start-date': {
issue: {
type: 'invalid-value',
description: 'invalid-value'
},
value: '40/04/1980'
}
}
}
const expectedIssueCounts = {
'Start date_invalid-value': {
count: 1,
description: 'invalid-value',
type: 'invalid-value',
field: 'Start date'
}
}
const expectedMissingColumns = [
'reference'
]

const { aggregatedIssues, issueCounts, missingColumns } = errorsController.getAggregatedErrors(mockApiValue)

expect(aggregatedIssues).toEqual(expectedAggregatedIssues)
expect(issueCounts).toEqual(expectedIssueCounts)
expect(missingColumns).toEqual(expectedMissingColumns)
})
})
})
2 changes: 1 addition & 1 deletion test/unit/uploadController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('UploadController', () => {
await uploadController.post(req, res, next)

expect(req.body.validationResult).toEqual(mockApiValue)
expect(uploadController.errorCount).toEqual(mockApiValue['issue-log'].filter(issue => issue.severity === 'error').length)
expect(uploadController.errorCount).toEqual(mockApiValue['issue-log'].filter(issue => issue.severity === 'error').length + mockApiValue['column-field-log'].filter(column => column.missing).length)
})

it('validateFile correctly calls the API', async () => {
Expand Down

0 comments on commit 1236c97

Please sign in to comment.