Merge pull request #49 from digital-land/apiErrorSummary

now render error summary from api
digital-land · Jan 23, 2024 · 46ea040 · 46ea040
2 parents 54763ff + a67d8f7
commit 46ea040
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 69 deletions.
diff --git a/src/controllers/errorsController.js b/src/controllers/errorsController.js
@@ -8,13 +8,12 @@ class ErrorsController extends PageController {
   get (req, res, next) {
     const validationResult = req.sessionModel.get('validationResult')
 
-    const { aggregatedIssues, issueCounts, missingColumns } = this.getAggregatedErrors(validationResult)
+    const { aggregatedIssues } = this.getAggregatedErrors(validationResult)
 
     const rows = Object.values(aggregatedIssues)
 
     req.form.options.rows = rows
-    req.form.options.issueCounts = issueCounts
-    req.form.options.missingColumns = missingColumns
+    req.form.options.errorSummary = validationResult['error-summary']
     req.form.options.columnNames = rows.length > 0 ? Object.keys(rows[0]) : []
 
     const dataSetValue = req.sessionModel.get('dataset')
@@ -34,7 +33,6 @@ class ErrorsController extends PageController {
 
   getAggregatedErrors (apiResponseData) {
     const aggregatedIssues = {}
-    const issueCounts = {}
 
     apiResponseData['issue-log'].forEach(issue => {
       if (issue.severity === severityLevels.error) {
@@ -61,29 +59,11 @@ class ErrorsController extends PageController {
             },
             value: rowValues[issue.field]
           }
-          const key = issue.field + '_' + issue['issue-type']
-          if (issueCounts[key]) {
-            issueCounts[key].count += 1
-          } else {
-            issueCounts[key] = {
-              count: 1,
-              type: issue['issue-type'],
-              description: issue.description,
-              field: issue.field
-            }
-          }
         }
       }
     })
 
-    const missingColumns = []
-    apiResponseData['column-field-log'].forEach(columnField => {
-      if (columnField.missing) {
-        missingColumns.push(columnField.field)
-      }
-    })
-
-    return { aggregatedIssues, issueCounts, missingColumns }
+    return { aggregatedIssues }
   }
 
   lookupMappedColumnNameFromOriginal (originalColumnName, columnFieldLogs) {

diff --git a/src/views/errors.html b/src/views/errors.html
@@ -17,18 +17,10 @@ <h1 class="govuk-heading-l">
       </h1>
 
       <ul class="govuk-list govuk-list--bullet">
-        {% for key, issue in options.issueCounts %}
-
-          {% if issue.count > 1 %}
-            {% set issueString = 'issues' %}
-          {% else %}
-            {% set issueString = 'issue' %}
-          {% endif %}
-
-          <li>{{issue.count}} {{issue.type}} {{issueString}}, relating to column {{issue.field}}</li>
-        {% endfor %}
-        {% for column in options.missingColumns %}
-          <li>Missing required column {{column}}</li>
+        {% for summaryMessage in options.errorSummary %}
+          <li>
+            {{summaryMessage}}
+          </li>
         {% endfor %}
       </ul>
   </div>

diff --git a/test/acceptance/missing_required_column.test.js b/test/acceptance/missing_required_column.test.js
@@ -23,5 +23,5 @@ test('when the user uploads a file with a missing required column, the page corr
 
   expect(await page.title()).toBe('Your data has errors - Publish planning and housing data for England')
 
-  expect(await page.textContent('.govuk-list')).toContain('Missing required column reference')
+  expect(await page.textContent('.govuk-list')).toContain('Reference column missing')
 })
diff --git a/test/testData/API_RUN_PIPELINE_RESPONSE.json b/test/testData/API_RUN_PIPELINE_RESPONSE.json
@@ -26,5 +26,11 @@
     {"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}
+  ],
+  "error-summary": [
+    "1 documentation URL must be a real URL",
+    "19 geometries must be in Well-Known Text (WKT) format",
+    "3 start dates must be a real date",
+    "Reference column missing"
   ]
 }
diff --git a/test/unit/errorsController.test.js b/test/unit/errorsController.test.js
@@ -81,17 +81,12 @@ describe('ErrorsController', () => {
             }
           }
         ],
-        missingColumns: [
-          'reference'
-        ],
-        issueCounts: {
-          'Start date_invalid-value': {
-            count: 1,
-            description: 'invalid-value',
-            type: 'invalid-value',
-            field: 'Start date'
-          }
-        }
+        errorSummary: [
+          '1 documentation URL must be a real URL',
+          '19 geometries must be in Well-Known Text (WKT) format',
+          '3 start dates must be a real date',
+          'Reference column missing'
+        ]
       }
     }
 
@@ -139,23 +134,10 @@ describe('ErrorsController', () => {
           }
         }
       }
-      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)
+      const { aggregatedIssues } = errorsController.getAggregatedErrors(mockApiValue)
 
       expect(aggregatedIssues).toEqual(expectedAggregatedIssues)
-      expect(issueCounts).toEqual(expectedIssueCounts)
-      expect(missingColumns).toEqual(expectedMissingColumns)
     })
   })
 })
diff --git a/test/unit/errorsPage.test.js b/test/unit/errorsPage.test.js
@@ -75,19 +75,17 @@ describe('errors page', () => {
             }
           }
         ],
-        issueCounts: {
-          'geography_fake error': {
-            count: 1,
-            type: 'fake error',
-            field: 'Geometry'
-          }
-        },
+        errorSummary: [
+          '1 documentation URL must be a real URL',
+          '19 geometries must be in Well-Known Text (WKT) format'
+        ],
         dataset: 'Dataset test'
       }
     }
     const html = nunjucks.render('errors.html', params).replace(/(\r\n|\n|\r)/gm, '').replace(/\t/gm, '').replace(/\s+/g, ' ')
 
-    expect(html).toContain('<li>1 fake error issue, relating to column Geometry</li>')
+    expect(html).toContain('<li> 1 documentation URL must be a real URL </li>')
+    expect(html).toContain('<li> 19 geometries must be in Well-Known Text (WKT) format </li>')
     expect(html).toContain('<span class="govuk-caption-l"> Dataset test </span>')
     expect(html).toContain('<td class="govuk-table__cell app-wrap"> <div class="govuk-inset-text app-inset-text---error"> <p class="app-inset-text__value">POLYGON ((-0.125888391245 51.54316508186, -0.125891457623 51.543177267548, -0.125903428774 51.54322160042))</p> <p class="app-inset-text__error">fake error</p></div> </td>')
   })