diff --git a/src/content/statusPage.js b/src/content/statusPage.js index 3288b011..81dffbee 100644 --- a/src/content/statusPage.js +++ b/src/content/statusPage.js @@ -1,6 +1,6 @@ export const headingTexts = { checking: 'Checking your data', - checked: 'Data Checked' + checked: 'Data checked' } export const messageTexts = { diff --git a/src/controllers/resultsController.js b/src/controllers/resultsController.js index bc84c94a..aee6192e 100644 --- a/src/controllers/resultsController.js +++ b/src/controllers/resultsController.js @@ -65,11 +65,17 @@ class ResultsController extends PageController { fields: responseDetails.getFields() } - req.form.options.errorSummary = requestData.getErrorSummary() + req.form.options.errorSummary = requestData.getErrorSummary().map(message => { + return { + text: message, + href: '' + } + }) req.form.options.mappings = responseDetails.getFieldMappings() req.form.options.geometries = responseDetails.getGeometries() req.form.options.pagination = responseDetails.getPagination(req.params.pageNumber) req.form.options.id = req.params.id + req.form.options.lastPage = `/check/status/${req.params.id}` } else { req.form.options.error = requestData.getError() } diff --git a/src/controllers/statusController.js b/src/controllers/statusController.js index afbd8216..9afcae2e 100644 --- a/src/controllers/statusController.js +++ b/src/controllers/statusController.js @@ -3,6 +3,22 @@ import { getRequestData } from '../services/asyncRequestApi.js' import { finishedProcessingStatuses } from '../utils/utils.js' import { headingTexts, messageTexts } from '../content/statusPage.js' +/** + * Attempts to infer how we ended up on this page. + * + * @param req + * @returns {string?} + */ +function getLastPage (req) { + let lastPage + if ('url' in req.form.options.data.params) { + lastPage = '/check/url' + } else if ('original_filename' in req.form.options.data.params) { + lastPage = '/check/upload' + } + return lastPage +} + class StatusController extends PageController { async locals (req, res, next) { try { @@ -11,6 +27,10 @@ class StatusController extends PageController { req.form.options.headingTexts = headingTexts req.form.options.messageTexts = messageTexts req.form.options.pollingEndpoint = `/api/status/${req.form.options.data.id}` + const lastPage = getLastPage(req) + if (lastPage) { + req.form.options.lastPage = lastPage + } super.locals(req, res, next) } catch (error) { next(error, req, res, next) diff --git a/src/models/requestData.js b/src/models/requestData.js index badac124..74458e31 100644 --- a/src/models/requestData.js +++ b/src/models/requestData.js @@ -32,6 +32,10 @@ export default class ResultData { return new ResponseDetails(this.id, response.data, pagination, this.getColumnFieldLog()) } + /** + * + * @returns {string[]} + */ getErrorSummary () { if (!this.response || !this.response.data || !this.response.data['error-summary']) { logger.warn('trying to get error summary when there is none', { requestId: this.id }) diff --git a/src/views/check/confirmation.html b/src/views/check/confirmation.html index c72c3db0..6ff79bd7 100644 --- a/src/views/check/confirmation.html +++ b/src/views/check/confirmation.html @@ -1,4 +1,4 @@ - +{% from 'govuk/components/back-link/macro.njk' import govukBackLink %} {% from 'govuk/components/panel/macro.njk' import govukPanel %} {% from "govuk/components/details/macro.njk" import govukDetails %} @@ -7,6 +7,14 @@ {% set serviceType = 'Check' %} {% set pageName = "You can now publish your data" %} +{% block beforeContent %} + {{ govukBackLink({ + text: "Back", + href: "javascript:window.history.back()" + }) }} +{% endblock %} + + {% set content %} # What to do next @@ -31,7 +39,7 @@ Details about what to do next are also available [in the guidance](/guidance) {% if options.deepLink %} -

Return to {{ options.deepLink.dataset }} overview

+

Return to {{ options.deepLink.dataset | datasetSlugToReadableName }} overview

{% else %}

Return to Home

{% endif %} diff --git a/src/views/check/results/errors.html b/src/views/check/results/errors.html index c83a1bfb..1edc23c2 100644 --- a/src/views/check/results/errors.html +++ b/src/views/check/results/errors.html @@ -4,9 +4,9 @@ {% from 'govuk/components/radios/macro.njk' import govukRadios %} {% from 'govuk/components/inset-text/macro.njk' import govukInsetText %} {% from "govuk/components/pagination/macro.njk" import govukPagination %} - -{% from 'components/dataset-banner.html' import datasetBanner %} -{% from 'components/table.html' import table %} +{% from 'govuk/components/error-summary/macro.njk' import govukErrorSummary %} +{% from "../../components/table.html" import table %} +{% from '../../components/dataset-banner.html' import datasetBanner %} {% set serviceType = 'Check' %} {% set pageName = 'Your data has errors' %} @@ -36,23 +36,22 @@

{{pageName}}

- + {{ govukErrorSummary({ + titleText: "There’s a problem", + errorList: options.errorSummary + }) }} +
+
- {% if options.tableParams.rows|length > 0 %} -

- Check your errors -

+ {% if options.geometries %} +
+ {% endif %} + {% if options.tableParams.rows|length > 0 %} {{ table(options.tableParams) }} {% endif %} @@ -72,11 +71,37 @@

+ + {% if options.deepLink.orgName %} +

How to improve {{ options.deepLink.orgName }}'s data

+ {% else %} +

How to improve the data

+ {% endif %} + +
    +
  1. Fix the errors indicated. You should use the data specifications to help you do this.
  2. +
  3. Check your updated data to make sure it meets the specifications.
  4. +
+ {{ govukButton({ - text: "Upload a new version", + text: "Check your data", href: "/check/upload-method" }) }}
{% endblock %} + +{% block scripts %} + {{ super() }} + {% if options.geometries %} + + + + {% endif %} +{% endblock %} diff --git a/src/views/check/results/no-errors.html b/src/views/check/results/no-errors.html index 2dc44b4b..cb4fc127 100644 --- a/src/views/check/results/no-errors.html +++ b/src/views/check/results/no-errors.html @@ -54,6 +54,10 @@

+ {% if options.geometries %} +
+ {% endif %} + {{ table(options.tableParams) }} {% if options.pagination.items | length > 1 %} @@ -69,13 +73,6 @@

{% endif %} - - {% if options.geometries %} -

Here’s your data on a map:

-
- {% endif %} - -

diff --git a/src/views/check/statusPage/status.html b/src/views/check/statusPage/status.html index 08ab5a4f..f8676ee4 100644 --- a/src/views/check/statusPage/status.html +++ b/src/views/check/statusPage/status.html @@ -1,4 +1,5 @@ {% from "govuk/components/button/macro.njk" import govukButton %} +{% from 'govuk/components/back-link/macro.njk' import govukBackLink %} {% from "./statusContentMacro.html" import statusContent %} {% from '../../components/dataset-banner.html' import datasetBanner %} @@ -17,6 +18,15 @@ {% set buttonClasses = "js-hidden" %} {% endif %} +{% block beforeContent %} + {% if options.lastPage %} + {{ govukBackLink({ + text: options.backLinkText | default("Back"), + href: options.lastPage + }) }} + {% endif %} +{% endblock %} + {% block content %}
diff --git a/src/views/components/dataset-banner.html b/src/views/components/dataset-banner.html index 1ab28274..e99e1e4c 100644 --- a/src/views/components/dataset-banner.html +++ b/src/views/components/dataset-banner.html @@ -1,3 +1,3 @@ {% macro datasetBanner(datasetName) %} -{{ datasetName }} +{{ datasetName | datasetSlugToReadableName }} {% endmacro %} \ No newline at end of file diff --git a/src/views/components/table.html b/src/views/components/table.html index f8808648..b62748a0 100644 --- a/src/views/components/table.html +++ b/src/views/components/table.html @@ -37,6 +37,11 @@ {% macro table(params) %}
+ + {% for field in params.fields %} + + {% endfor %} + {% for column in params.columns %} diff --git a/src/views/error.html b/src/views/error.html index 87f60603..aca677e5 100644 --- a/src/views/error.html +++ b/src/views/error.html @@ -1,10 +1,8 @@ - {% from "govuk/components/button/macro.njk" import govukButton %} {% set pageName = "Error" %} {% extends "layouts/main.html" %} - {% block content %}

Error

diff --git a/test/PageObjectModels/resultsPage.js b/test/PageObjectModels/resultsPage.js index 417b9ac8..2ca0a495 100644 --- a/test/PageObjectModels/resultsPage.js +++ b/test/PageObjectModels/resultsPage.js @@ -39,10 +39,10 @@ export default class ResultsPage extends BasePage { // Check if there's a table expect(await this.page.locator('table').isVisible()) - await this.page.waitForSelector('.govuk-list.govuk-list--bullet') + await this.page.waitForSelector('.govuk-error-summary .govuk-list') // Get the text content of the bullet points const summarytext = await this.page.evaluate(() => { - const bulletPoints = Array.from(document.querySelectorAll('.govuk-list.govuk-list--bullet li')) + const bulletPoints = Array.from(document.querySelectorAll('.govuk-error-summary .govuk-list li')) return bulletPoints.map(li => li.textContent.trim()) }) diff --git a/test/integration/test_recieving_results.playwright.test.js b/test/integration/test_recieving_results.playwright.test.js index 881151d2..d9cc3393 100644 --- a/test/integration/test_recieving_results.playwright.test.js +++ b/test/integration/test_recieving_results.playwright.test.js @@ -40,10 +40,8 @@ test('receiving a result with errors', async ({ page }) => { await resultsPage.navigateToRequest('complete-errors') await resultsPage.expectPageIsErrorsPage() - await page.getByText('1 geometry must be in Well-Known Text (WKT) format 1 documentation URL must be').click() - for (const error of errorResponse.response.data['error-summary']) { - await expect(page.locator('.govuk-list')).toContainText(error) + await expect(page.locator('ul.govuk-list')).toContainText(error) } const tableValues = await getTableContents(page, 'govuk-table') diff --git a/test/unit/errorsPage.test.js b/test/unit/errorsPage.test.js index 7f8f4a6b..84118cf2 100644 --- a/test/unit/errorsPage.test.js +++ b/test/unit/errorsPage.test.js @@ -38,6 +38,13 @@ describe('errors page', () => { limit: 50 } + const summaryItem = (errorMessage) => { + return { + text: errorMessage, + href: '' + } + } + const params = { options: { tableParams: { @@ -46,7 +53,7 @@ describe('errors page', () => { fields: responseDetails.getFields() }, requestParams: requestData.getParams(), - errorSummary: requestData.getErrorSummary(), + errorSummary: requestData.getErrorSummary().map(summaryItem), mappings: responseDetails.getFieldMappings(), verboseRows: responseDetails.getRowsWithVerboseColumns(), pagination: responseDetails.getPagination(0) @@ -65,7 +72,7 @@ describe('errors page', () => { expect(errorItems.length).toEqual(6) params.options.errorSummary.forEach((message, i) => { - expect(errorItems[i].textContent).toContain(message) + expect(errorItems[i].textContent).toContain(message.text) }) // table diff --git a/test/unit/resultsController.test.js b/test/unit/resultsController.test.js index fd0c5976..18249e0d 100644 --- a/test/unit/resultsController.test.js +++ b/test/unit/resultsController.test.js @@ -136,13 +136,14 @@ describe('ResultsController', () => { }, datasetName: req.sessionModel.get('dataset'), - errorSummary: ['error summary'], + errorSummary: [{ text: 'error summary', href: '' }], mappings: { fields: 'geometries' }, geometries: ['geometries'], pagination: 'pagination', requestParams: 'params', template: 'results/no-errors', - id: req.params.id + id: req.params.id, + lastPage: `/check/status/${req.params.id}` }) })