Merge pull request #640 from digital-land/rosado/571-content-and-styl…

…ing-2

Update content & design elements of the check tool's "you've got errors" page.

src/content/statusPage.js

@@ -1,6 +1,6 @@
 export const headingTexts = {
   checking: 'Checking your data',
-  checked: 'Data Checked'
+  checked: 'Data checked'
 }
 
 export const messageTexts = {

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()
       }

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)

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 })

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 %}
-    <p><a href="{{ options.deepLink.referrer }}">Return to {{ options.deepLink.dataset }} overview</a></p>
+    <p><a href="{{ options.deepLink.referrer }}">Return to {{ options.deepLink.dataset | datasetSlugToReadableName }} overview</a></p>
   {% else %}
     <p><a href="/">Return to Home</a></p>
   {% endif %}

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 @@ <h1 class="govuk-heading-l">
         {{pageName}}
       </h1>
 
-      <ul class="govuk-list govuk-list--bullet">
-        {% for summaryMessage in options.errorSummary %}
-          <li>
-            {{summaryMessage}}
-          </li>
-        {% endfor %}
-      </ul>
+      {{ govukErrorSummary({
+        titleText: "There’s a problem",
+        errorList: options.errorSummary
+      }) }}
+
   </div>
 </div>
 
 <div class="govuk-grid-row">
+
   <div class="govuk-grid-column-full">
-    {% if options.tableParams.rows|length > 0 %}
-      <h2 class="govuk-heading-m">
-        Check your errors
-      </h2>
+    {% if options.geometries %}
+      <div id="map" class="app-map" aria-label="Map showing contour of the submitted data on a map"></div>
+    {% endif %}
 
+    {% if options.tableParams.rows|length > 0 %}
       {{ table(options.tableParams) }}
     {% endif %}
 
@@ -72,11 +71,37 @@ <h2 class="govuk-heading-m">
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
+
+    {% if options.deepLink.orgName %}
+      <h3 class="govuk-heading-m">How to improve {{ options.deepLink.orgName }}'s data</h3>
+    {% else %}
+      <h3 class="govuk-heading-m">How to improve the data</h3>
+    {% endif %}
+
+    <ol class="govuk-list govuk-list--number">
+      <li>Fix the errors indicated. You should use the <a href="/guidance/specifications/">data specifications</a> to help you do this.</li>
+      <li>Check your updated data to make sure it meets the specifications.</li>
+    </ol>
+
     {{ govukButton({
-      text: "Upload a new version",
+      text: "Check your data",
       href: "/check/upload-method"
     }) }}
 
   </div>
 </div>
 {% endblock %}
+
+{% block scripts %}
+  {{ super() }}
+  {% if options.geometries %}
+    <link href='https://unpkg.com/maplibre-gl@latest/dist/maplibre-gl.css' rel='stylesheet' />
+    <script>
+      window.serverContext = {
+        containerId: 'map',
+        geometries: {{ options.geometries | dump | safe }},
+      }
+    </script>
+    <script src="/public/js/map.bundle.js"></script>
+  {% endif %}
+{% endblock %}

src/views/check/results/no-errors.html

@@ -54,6 +54,10 @@ <h1 class="govuk-heading-l">
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-full">
 
+      {% if options.geometries %}
+        <div id="map" class="app-map" aria-label="Map showing contour of the submitted data on a map"></div>
+      {% endif %}
+
       {{ table(options.tableParams) }}
 
       {% if options.pagination.items | length > 1 %}
@@ -69,13 +73,6 @@ <h1 class="govuk-heading-l">
       {% endif %}
 
 
-
-      {% if options.geometries %}
-        <p>Here’s your data on a map:</p>
-        <div id="map" class="app-map"></div>
-      {% endif %}
-
-
     </div>
   </div>
 

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 %}
 <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds" aria-live="assertive">

src/views/components/dataset-banner.html

@@ -1,3 +1,3 @@
 {% macro datasetBanner(datasetName) %}
-<span class="govuk-caption-xl">{{ datasetName }}</span>
+<span class="govuk-caption-xl">{{ datasetName | datasetSlugToReadableName }}</span>
 {% endmacro %}

src/views/components/table.html

@@ -37,6 +37,11 @@
 {% macro table(params) %}
     <div class="app-scrollable-container dl-scrollable">
         <table class="govuk-table dl-table">
+            <colgroup>
+                {% for field in params.fields %}
+                    <col class="{% if (r/.*-date/g).test(field) %}govuk-table__cell--numeric{% endif %}" />
+                {% endfor %}
+            </colgroup>
             <thead class="govuk-table__head dl-table__head">
                 <tr class="govuk-table__row">
                     {% for column in params.columns %}

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 %}
     <h1 class="govuk-heading-xl">Error</h1>
 

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())
     })
 

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')

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

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}`
       })
     })