diff --git a/.husky/pre-commit b/.husky/pre-commit index 251a23e7..6f5cf54d 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,4 +1,5 @@ #!/usr/bin/env sh . "$(dirname -- "$0")/_/husky.sh" -npm run lint \ No newline at end of file +npm run lint:fix +npm run lint diff --git a/src/controllers/uploadController.js b/src/controllers/uploadController.js index fc00d304..9af2c1a5 100644 --- a/src/controllers/uploadController.js +++ b/src/controllers/uploadController.js @@ -18,30 +18,40 @@ class UploadController extends Controller { } async post (req, res, next) { + if (req.file !== undefined) { + try { + const jsonResult = await this.validateFile({ + filePath: req.file.path, + fileName: req.file.originalname, + dataset: req.sessionModel.get('dataset'), + dataSubject: req.sessionModel.get('data-subject'), + organization: 'mockOrg' + }) + req.body.validationResult = jsonResult + } catch (error) { + console.log(error) + } + } + super.post(req, res, next) + } + + async validateFile ({ filePath, fileName, dataset, dataSubject, organization }) { const formData = new FormData() - formData.append('dataset', req.sessionModel.get('dataset')) - formData.append('collection', req.sessionModel.get('data-subject')) - formData.append('organization', 'mockOrg') + formData.append('dataset', dataset) + formData.append('collection', dataSubject) + formData.append('organization', organization) - const filePath = req.file.path const file = new Blob([await readFile(filePath)], { type: lookup(filePath) }) - formData.append('upload_file', file, req.file.originalname) + formData.append('upload_file', file, fileName) - try { - // post the data to the api - const result = await fetch(apiRoute, { - method: 'POST', - body: formData - }) + // post the data to the api + const result = await fetch(apiRoute, { + method: 'POST', + body: formData + }) - const json = await result.json() - - req.sessionModel.set('validationResult', json) - super.post(req, res, next) - } catch (e) { - res.send(e) - } + return await result.json() } } diff --git a/src/routes/form-wizard/steps.js b/src/routes/form-wizard/steps.js index 6a066ded..87a89d0f 100644 --- a/src/routes/form-wizard/steps.js +++ b/src/routes/form-wizard/steps.js @@ -16,6 +16,7 @@ module.exports = { }, '/upload': { controller: require('../../controllers/uploadController'), + fields: ['validationResult'], next: 'errors' }, '/errors': { diff --git a/src/views/data-subject.html b/src/views/data-subject.html index e762c8b8..4a26f76d 100644 --- a/src/views/data-subject.html +++ b/src/views/data-subject.html @@ -3,9 +3,28 @@ {% from 'govuk/components/back-link/macro.njk' import govukBackLink %} {% from 'govuk/components/button/macro.njk' import govukButton %} {% from 'govuk/components/radios/macro.njk' import govukRadios %} +{% from 'govuk/components/error-message/macro.njk' import govukErrorMessage %} +{% from 'govuk/components/error-summary/macro.njk' import govukErrorSummary %} + + {% set pageName = 'Data subject' %} +{% set errorMessage = 'Please select a data subject' %} + +{% if 'data-subject' in errors %} + {% set dataSubjectError = true %} +{% endif %} + +{% block pageTitle %} + {% if dataSubjectError %} + Error: {{super()}} + {% else %} + {{super()}} + {% endif %} +{% endblock %} + + {% block beforeContent %} {{ govukBackLink({ text: "Back", @@ -16,6 +35,17 @@ {% block content %}
+ {% if dataSubjectError %} + {{ govukErrorSummary({ + titleText: "There is a problem", + errorList: [ + { + text: errorMessage, + href: "#data-subject" + } + ] + }) }} + {% endif %}
{{ govukRadios({ @@ -46,7 +76,10 @@ text: "Tree preservation order" } ], - value: data.check.dataSubject + value: data.check.dataSubject, + errorMessage: { + text: errorMessage + } if dataSubjectError else undefined }) }} {{ govukButton({ diff --git a/src/views/dataset.html b/src/views/dataset.html index f4559f87..3f3c844d 100644 --- a/src/views/dataset.html +++ b/src/views/dataset.html @@ -2,9 +2,26 @@ {% from 'govuk/components/back-link/macro.njk' import govukBackLink %} {% from 'govuk/components/button/macro.njk' import govukButton %} {% from 'govuk/components/radios/macro.njk' import govukRadios %} +{% from 'govuk/components/error-message/macro.njk' import govukErrorMessage %} +{% from 'govuk/components/error-summary/macro.njk' import govukErrorSummary %} {% set pageName = 'Dataset' %} +{% set errorMessage = 'Please select a dataset' %} + + +{% if 'dataset' in errors %} + {% set datasetError = true %} +{% endif %} + +{% block pageTitle %} + {% if datasetError %} + Error: {{super()}} + {% else %} + {{super()}} + {% endif %} +{% endblock %} + {% block beforeContent %} {{ govukBackLink({ text: "Back", @@ -13,28 +30,42 @@ {% endblock %} {% block content %} -
-
- - - {{ govukRadios({ - idPrefix: "dataset", - name: "dataset", - fieldset: { - legend: { - text: pageName, - isPageHeading: true, - classes: "govuk-fieldset__legend--l" - } - }, - items: options.datasetItems, - value: data.check.dataset - }) }} - - {{ govukButton({ - text: "Continue" - }) }} - -
+
+
+ {% if datasetError %} + {{ govukErrorSummary({ + titleText: "There is a problem", + errorList: [ + { + text: errorMessage, + href: "#dataset" + } + ] + }) }} + {% endif %} +
+ + {{ govukRadios({ + idPrefix: "dataset", + name: "dataset", + fieldset: { + legend: { + text: pageName, + isPageHeading: true, + classes: "govuk-fieldset__legend--l" + } + }, + items: options.datasetItems, + value: data.check.dataset, + errorMessage: { + text: errorMessage + } if datasetError else undefined + }) }} + + {{ govukButton({ + text: "Continue" + }) }} +
+
{% endblock %} diff --git a/src/views/pages/file-not-found.html b/src/views/pages/file-not-found.html new file mode 100644 index 00000000..e69de29b diff --git a/src/views/upload.html b/src/views/upload.html index d3465eec..8e4e3031 100644 --- a/src/views/upload.html +++ b/src/views/upload.html @@ -3,9 +3,27 @@ {% from "govuk/components/back-link/macro.njk" import govukBackLink %} {% from "govuk/components/file-upload/macro.njk" import govukFileUpload %} {% from "govuk/components/button/macro.njk" import govukButton %} +{% from 'govuk/components/error-message/macro.njk' import govukErrorMessage %} +{% from 'govuk/components/error-summary/macro.njk' import govukErrorSummary %} + {% set pageName = 'Upload data' %} +{% set errorMessage = 'Please select a file' %} + + +{% if 'validationResult' in errors %} + {% set datasetError = true %} +{% endif %} + +{% block pageTitle %} + {% if datasetError %} + Error: {{super()}} + {% else %} + {{super()}} + {% endif %} +{% endblock %} + {% block beforeContent %} {{ govukBackLink({ text: "Back", @@ -16,6 +34,17 @@ {% block content %}
+ {% if datasetError %} + {{ govukErrorSummary({ + titleText: "There is a problem", + errorList: [ + { + text: errorMessage, + href: "#datafile" + } + ] + }) }} + {% endif %}
{{ govukFileUpload({ @@ -28,7 +57,10 @@ }, hint: { text: "You can upload a CSV, GeoJSON, GML or Geopackage file" - } + }, + errorMessage: { + text: errorMessage + } if datasetError else undefined }) }} {{ govukButton({ text: "Continue" diff --git a/test/acceptance/validation_errors.test.js b/test/acceptance/validation_errors.test.js new file mode 100644 index 00000000..c283ac5a --- /dev/null +++ b/test/acceptance/validation_errors.test.js @@ -0,0 +1,82 @@ +import { test, expect } from '@playwright/test' + +test('when the user clicks continue on the data subject page without entering a data subject, the page correctly indicates there\'s an error', async ({ page }) => { + await page.goto('/') + await page.getByRole('button', { name: 'Start now' }).click() + await page.getByRole('button', { name: 'Continue' }).click() + + await page.waitForSelector('input#data-subject.govuk-radios__input') + + const errorLink = await page.getByRole('link', { name: 'Please select a data subject' }) + const fieldError = await page.getByText('Error: Please select a data subject') + const errorSummary = await page.getByText('There is a problem') + + expect(await errorSummary.isVisible(), 'Page should show the error summary').toBeTruthy() + expect(await errorLink.isVisible(), 'Page should the error message that is a link to the problem field').toBeTruthy() + expect(await fieldError.isVisible(), 'Page should show the error message next to the problem field').toBeTruthy() + await errorLink.click() + const problemFieldIsFocused = await page.$eval('input#data-subject.govuk-radios__input', (el) => el === document.activeElement) + expect(problemFieldIsFocused, 'The focus should be on the problem field').toBeTruthy() + + expect(await page.title(), 'Page title should indicate there\'s an error').toMatch(/Error: .*/) +}) + +test('when the user clicks continue on the dataset page without entering a dataset, the page correctly indicates there\'s an error', async ({ page }) => { + await page.goto('/') + // start page + await page.getByRole('button', { name: 'Start now' }).click() + + // data subject page + await page.getByLabel('Conservation area').check() + await page.getByRole('button', { name: 'Continue' }).click() + + // dataset page + await page.getByRole('button', { name: 'Continue' }).click() + + await page.waitForSelector('input#dataset.govuk-radios__input') + + const errorLink = await page.getByRole('link', { name: 'Please select a dataset' }) + const fieldError = await page.getByText('Error: Please select a dataset') + const errorSummary = await page.getByText('There is a problem') + + expect(await errorSummary.isVisible(), 'Page should show the error summary').toBeTruthy() + expect(await errorLink.isVisible(), 'Page should the error message that is a link to the problem field').toBeTruthy() + expect(await fieldError.isVisible(), 'Page should show the error message next to the problem field').toBeTruthy() + await errorLink.click() + const problemFieldIsFocused = await page.$eval('input#dataset.govuk-radios__input', (el) => el === document.activeElement) + expect(problemFieldIsFocused, 'The focus should be on the problem field').toBeTruthy() + + expect(await page.title(), 'Page title should indicate there\'s an error').toMatch(/Error: .*/) +}) + +test('when the user clicks continue on the file upload page without selecting a file, the page correctly indicates there\'s an error', async ({ page }) => { + await page.goto('/') + // start page + await page.getByRole('button', { name: 'Start now' }).click() + + // data subject page + await page.getByLabel('Conservation area').check() + await page.getByRole('button', { name: 'Continue' }).click() + + // dataset page + await page.getByLabel('Conservation area dataset').check() + await page.getByRole('button', { name: 'Continue' }).click() + + // file upload page + await page.getByRole('button', { name: 'Continue' }).click() + await page.waitForSelector('input#datafile.govuk-file-upload') + + const errorLink = await page.getByRole('link', { name: 'Please select a file' }) + const fieldError = await page.getByText('Error: Please select a file') + const errorSummary = await page.getByText('There is a problem') + + expect(await errorSummary.isVisible(), 'Page should show the error summary').toBeTruthy() + expect(await errorLink.isVisible(), 'Page should the error message that is a link to the problem field').toBeTruthy() + expect(await fieldError.isVisible(), 'Page should show the error message next to the problem field').toBeTruthy() + await errorLink.click() + + const problemFieldIsFocused = await page.$eval('input#datafile.govuk-file-upload', (el) => el === document.activeElement) + expect(problemFieldIsFocused, 'The focus should be on the problem field').toBeTruthy() + + expect(await page.title(), 'Page title should indicate there\'s an error').toMatch(/Error: .*/) +}) diff --git a/test/unit/uploadController.test.js b/test/unit/uploadController.test.js index 9222a783..9aef7d6d 100644 --- a/test/unit/uploadController.test.js +++ b/test/unit/uploadController.test.js @@ -24,7 +24,8 @@ describe('UploadController', () => { sessionModel: { get: () => 'test', set: vi.fn() - } + }, + body: {} } const res = { send: vi.fn(), @@ -34,6 +35,6 @@ describe('UploadController', () => { await uploadController.post(req, res, next) - expect(req.sessionModel.set).toHaveBeenCalledWith('validationResult', mockApiValue) + expect(req.body.validationResult).toEqual(mockApiValue) }) })