From 607f2dad2291fc9414c4cbe06ac85f70a78bed34 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 7 Nov 2024 17:39:44 +0000 Subject: [PATCH 01/16] check: error result page - design and content changes second part of #571 --- src/controllers/resultsController.js | 7 ++++++- src/models/requestData.js | 4 ++++ src/views/check/results/errors.html | 31 +++++++++++++++++----------- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/controllers/resultsController.js b/src/controllers/resultsController.js index 0ba30029..cf1a21c7 100644 --- a/src/controllers/resultsController.js +++ b/src/controllers/resultsController.js @@ -64,7 +64,12 @@ 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) 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/results/errors.html b/src/views/check/results/errors.html index 161653e2..29feb259 100644 --- a/src/views/check/results/errors.html +++ b/src/views/check/results/errors.html @@ -4,6 +4,7 @@ {% 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 'govuk/components/error-summary/macro.njk' import govukErrorSummary %} {% from "../../components/table.html" import table %} {% from '../../components/dataset-banner.html' import datasetBanner %} @@ -35,23 +36,17 @@

{{pageName}}

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

- Check your errors -

- {{ table(options.tableParams) }} {% endif %} @@ -71,8 +66,20 @@

+ + {% 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. +
  2. Check your updated data to make sure it meets the specifications.
    3. +
+ {{ govukButton({ - text: "Upload a new version", + text: "Check your data", href: "/check/upload-method" }) }} From 2cc6a49fef90c0f5e8995897b2a6f42d0fc98875 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 7 Nov 2024 17:40:10 +0000 Subject: [PATCH 02/16] check: error result page - update tests --- test/unit/errorsPage.test.js | 11 +++++++++-- test/unit/resultsController.test.js | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/test/unit/errorsPage.test.js b/test/unit/errorsPage.test.js index 0b7a94a3..6644b8f9 100644 --- a/test/unit/errorsPage.test.js +++ b/test/unit/errorsPage.test.js @@ -37,6 +37,13 @@ describe('errors page', () => { limit: 50 } + const summaryItem = (errorMessage) => { + return { + text: errorMessage, + href: '' + } + } + const params = { options: { tableParams: { @@ -45,7 +52,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) @@ -64,7 +71,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 1ca37f37..18d04934 100644 --- a/test/unit/resultsController.test.js +++ b/test/unit/resultsController.test.js @@ -136,7 +136,7 @@ describe('ResultsController', () => { }, datasetName: req.sessionModel.get('dataset'), - errorSummary: ['error summary'], + errorSummary: [{ text: 'error summary', href: '' }], mappings: { fields: 'geometries' }, geometries: ['geometries'], pagination: 'pagination', From e949297f8e29ed6bb8851320535e2a5edf47d6c8 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Fri, 8 Nov 2024 10:39:59 +0000 Subject: [PATCH 03/16] check: update integration tests --- test/integration/test_recieving_results.playwright.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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') From 66d062e3a143816f1e3fdcaf3c43be9ff5819f7c Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Fri, 8 Nov 2024 11:20:00 +0000 Subject: [PATCH 04/16] check: update acceptance tests --- test/PageObjectModels/resultsPage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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()) }) From c08a0f0065c22a729af919fbe88e8295e1e6ed25 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 12 Nov 2024 09:50:26 +0000 Subject: [PATCH 05/16] update guidance link --- src/views/check/results/errors.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/views/check/results/errors.html b/src/views/check/results/errors.html index 29feb259..be74b1c4 100644 --- a/src/views/check/results/errors.html +++ b/src/views/check/results/errors.html @@ -74,7 +74,7 @@

How to improve the data

{% endif %}
    -
  1. Fix the errors indicated. You should use the data specifications to help you do this.
    2. +
  2. Fix the errors indicated. You should use the data specifications to help you do this.
  3. Check your updated data to make sure it meets the specifications.
From 210669ad14694b2514b108693e13714205fece11 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Tue, 12 Nov 2024 10:52:10 +0000 Subject: [PATCH 06/16] Revert "update guidance link" This reverts commit c08a0f0065c22a729af919fbe88e8295e1e6ed25. --- src/views/check/results/errors.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/views/check/results/errors.html b/src/views/check/results/errors.html index be74b1c4..29feb259 100644 --- a/src/views/check/results/errors.html +++ b/src/views/check/results/errors.html @@ -74,7 +74,7 @@

How to improve the data

{% endif %}
    -
  1. Fix the errors indicated. You should use the data specifications to help you do this.
    2. +
  2. Fix the errors indicated. You should use the data specifications to help you do this.
  3. Check your updated data to make sure it meets the specifications.
From ffc1f42405fe644517e57a6ed17d414e4090f6ea Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 14 Nov 2024 10:34:55 +0000 Subject: [PATCH 07/16] check tool: add CSS class to date columns in errors table --- src/views/components/table.html | 5 +++++ 1 file changed, 5 insertions(+) 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 %} From de3edc3c705947d33ecca6c82800afca80a95702 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 21 Nov 2024 14:10:03 +0000 Subject: [PATCH 08/16] add back links on status and results pages --- src/controllers/resultsController.js | 1 + src/controllers/statusController.js | 20 ++++++++++++++++++++ src/views/check/statusPage/status.html | 10 ++++++++++ src/views/error.html | 11 ++++++++++- 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/controllers/resultsController.js b/src/controllers/resultsController.js index 729fbf30..aee6192e 100644 --- a/src/controllers/resultsController.js +++ b/src/controllers/resultsController.js @@ -75,6 +75,7 @@ class ResultsController extends PageController { 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/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/error.html b/src/views/error.html index 87f60603..bef9f851 100644 --- a/src/views/error.html +++ b/src/views/error.html @@ -1,10 +1,19 @@ - +{% from 'govuk/components/back-link/macro.njk' import govukBackLink %} {% from "govuk/components/button/macro.njk" import govukButton %} {% set pageName = "Error" %} {% extends "layouts/main.html" %} +{% block beforeContent %} + {% if options.lastPage %} + {{ govukBackLink({ + text: options.backLinkText | default("Back"), + href: options.lastPage + }) }} + {% endif %} +{% endblock %} + {% block content %}

Error

From 00337334f12415108859f0f592449473836a4c33 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 21 Nov 2024 14:15:19 +0000 Subject: [PATCH 09/16] update result controller test --- test/unit/resultsController.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/resultsController.test.js b/test/unit/resultsController.test.js index bbfc91f7..18249e0d 100644 --- a/test/unit/resultsController.test.js +++ b/test/unit/resultsController.test.js @@ -142,7 +142,8 @@ describe('ResultsController', () => { pagination: 'pagination', requestParams: 'params', template: 'results/no-errors', - id: req.params.id + id: req.params.id, + lastPage: `/check/status/${req.params.id}` }) }) From 580e70377fdfce782b57d0b96216bc93a8772b94 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 21 Nov 2024 14:26:14 +0000 Subject: [PATCH 10/16] use proper name of the dataset on confirmation page --- src/views/check/confirmation.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/views/check/confirmation.html b/src/views/check/confirmation.html index c72c3db0..951031f4 100644 --- a/src/views/check/confirmation.html +++ b/src/views/check/confirmation.html @@ -31,7 +31,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 %} From 44ff47472938aaafabafd08f3d2499b20d68bced Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 21 Nov 2024 16:16:15 +0000 Subject: [PATCH 11/16] remove back button - wrong error page --- src/views/error.html | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/views/error.html b/src/views/error.html index bef9f851..aca677e5 100644 --- a/src/views/error.html +++ b/src/views/error.html @@ -1,19 +1,8 @@ -{% from 'govuk/components/back-link/macro.njk' import govukBackLink %} {% from "govuk/components/button/macro.njk" import govukButton %} {% set pageName = "Error" %} {% extends "layouts/main.html" %} - -{% block beforeContent %} - {% if options.lastPage %} - {{ govukBackLink({ - text: options.backLinkText | default("Back"), - href: options.lastPage - }) }} - {% endif %} -{% endblock %} - {% block content %}

Error

From 4670e90cd69bddd8eeb1ec5fab6060fa4929a255 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 21 Nov 2024 16:29:22 +0000 Subject: [PATCH 12/16] back clink on confirmation page --- src/views/check/confirmation.html | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/views/check/confirmation.html b/src/views/check/confirmation.html index 951031f4..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 From 5e65c9af6f33f5549582b5b05e3d0c4191c46085 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 21 Nov 2024 16:56:28 +0000 Subject: [PATCH 13/16] check: move the map above the table --- src/views/check/results/errors.html | 19 +++++++++++++++++++ src/views/check/results/no-errors.html | 11 ++++------- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/views/check/results/errors.html b/src/views/check/results/errors.html index 29feb259..4429f18a 100644 --- a/src/views/check/results/errors.html +++ b/src/views/check/results/errors.html @@ -45,7 +45,12 @@

+
+ {% if options.geometries %} +
+ {% endif %} + {% if options.tableParams.rows|length > 0 %} {{ table(options.tableParams) }} {% endif %} @@ -86,3 +91,17 @@

How to improve the data

{% 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..37b5fa68 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 %} - -

From b4c1d5116fa1196ca47a0b4d35947aa6e1b67480 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 21 Nov 2024 17:03:54 +0000 Subject: [PATCH 14/16] check: add aria-label to the element containing the map --- src/views/check/results/errors.html | 2 +- src/views/check/results/no-errors.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/views/check/results/errors.html b/src/views/check/results/errors.html index 4429f18a..1edc23c2 100644 --- a/src/views/check/results/errors.html +++ b/src/views/check/results/errors.html @@ -48,7 +48,7 @@

{% if options.geometries %} -
+
{% endif %} {% if options.tableParams.rows|length > 0 %} diff --git a/src/views/check/results/no-errors.html b/src/views/check/results/no-errors.html index 37b5fa68..cb4fc127 100644 --- a/src/views/check/results/no-errors.html +++ b/src/views/check/results/no-errors.html @@ -55,7 +55,7 @@

{% if options.geometries %} -
+
{% endif %} {{ table(options.tableParams) }} From 64efa6851cbc58c1b793c1c588829b2d86d35663 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Mon, 2 Dec 2024 13:20:10 +0000 Subject: [PATCH 15/16] check: fix capitalisation --- src/content/statusPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 = { From d1f8aa5b1875826733ebb422c71d016c07569088 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Wed, 4 Dec 2024 13:33:14 +0000 Subject: [PATCH 16/16] ensure readable dataset name is displayed in dataset banner --- src/views/components/dataset-banner.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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