Merge branch 'main' into rosado/588-include-ds-info-severity

index.js

@@ -13,9 +13,11 @@ import { setupSession } from './src/serverSetup/session.js'
 import { setupNunjucks } from './src/serverSetup/nunjucks.js'
 import { setupSentry } from './src/serverSetup/sentry.js'
 import { getDatasetSlugNameMapping } from './src/utils/datasetteQueries/getDatasetSlugNameMapping.js'
+import { initDatasetSlugToReadableNameFilter } from './src/utils/datasetSlugToReadableName.js'
 
 dotenv.config()
 
+await initDatasetSlugToReadableNameFilter()
 const app = express()
 
 setupMiddlewares(app)

src/assets/js/statusPage.js

@@ -1,6 +1,7 @@
 // poll the server for the status of the job
 
 import { finishedProcessingStatuses } from '../../utils/utils.js'
+import { buttonTexts, buttonAriaLabels, headingTexts, messageTexts } from '../../content/statusPage.js'
 
 export default class StatusPage {
   constructor (pollingInterval, maxPollAttempts) {
@@ -15,20 +16,6 @@ export default class StatusPage {
     this.continueButton = document.querySelector('#js-async-continue-button')
   }
 
-  headingTexts = {
-    checkingFile: 'Checking File',
-    fileChecked: 'File Checked'
-  }
-
-  messageTexts = {
-    pleaseWait: 'Please wait'
-  }
-
-  buttonTexts = {
-    continue: 'Continue',
-    retrieveLatestStatus: 'Retrieve Latest Status'
-  }
-
   beginPolling (statusEndpoint) {
     this.pollAttempts = 0
 
@@ -58,24 +45,26 @@ export default class StatusPage {
 
   updatePageToChecking () {
     // update the page
-    this.heading.textContent = this.headingTexts.checkingFile
-    this.processingMessage.textContent = this.messageTexts.pleaseWait
+    this.heading.textContent = headingTexts.checking
+    this.processingMessage.textContent = messageTexts.checking
     this.continueButton.style.display = 'none'
   }
 
   updatePageToComplete () {
     // update the page
-    this.heading.textContent = this.headingTexts.fileChecked
+    this.heading.textContent = headingTexts.checked
     this.processingMessage.style.display = 'none'
-    this.continueButton.textContent = this.buttonTexts.continue
+    this.continueButton.textContent = buttonTexts.checked
+    this.continueButton.ariaLabel = buttonAriaLabels.checked
     this.continueButton.style.display = 'block'
   }
 
   updatePageForPollingTimeout () {
     // update the page
-    this.heading.textContent = this.headingTexts.checkingFile
+    this.heading.textContent = headingTexts.checking
     this.processingMessage.style.display = 'none'
-    this.continueButton.textContent = this.buttonTexts.retrieveLatestStatus
+    this.continueButton.textContent = buttonTexts.checking
+    this.continueButton.ariaLabel = buttonAriaLabels.checking
     this.continueButton.style.display = 'block'
   }
 }

src/assets/scss/index.scss

@@ -170,3 +170,46 @@ code * {
 .padding-top {
   padding-top: 40px;
 }
+
+.dl-scrollable {
+  max-width: 100%;
+  overflow-x: auto;
+  padding: 0px;
+}
+
+
+.dl-table {
+  overflow-y: auto;
+  overflow-x: hidden;
+  max-height: 80vh;
+  width: 100%;
+  margin-bottom: 0px;
+  border-collapse: separate;
+
+  th, td {
+    padding: govuk-spacing(1) govuk-spacing(2);
+  }
+}
+
+
+.dl-table__head {
+  position: sticky;
+  top: 0;
+  z-index: 1;
+
+  box-shadow: 0 2px 2px -1px rgba(0, 0, 0, 0.4);
+  tr {
+    background-color: govuk-colour("light-grey");
+  }
+}
+
+
+.dl-table__header {
+  border: 1px solid $govuk-border-colour;
+  text-wrap: nowrap;
+  background-color: govuk-colour("light-grey");
+}
+
+.dl-table__cell--error {
+  border-left: govuk-spacing(1) solid $govuk-error-colour;
+}

src/content/statusPage.js

@@ -0,0 +1,19 @@
+export const headingTexts = {
+  checking: 'Checking your data',
+  checked: 'Data Checked'
+}
+
+export const messageTexts = {
+  checking: 'Do not close this page',
+  checked: 'You can continue'
+}
+
+export const buttonTexts = {
+  checking: 'Retrieve Latest Status',
+  checked: 'Continue'
+}
+
+export const buttonAriaLabels = {
+  checking: 'Retrieve Latest Status of data check',
+  checked: 'Continue to next step'
+}

src/controllers/pageController.js

@@ -1,6 +1,7 @@
 import hmpoFormWizard from 'hmpo-form-wizard'
 import { logPageView, types } from '../utils/logging.js'
 import logger from '../utils/logger.js'
+import { datasetSlugToReadableName } from '../utils/datasetSlugToReadableName.js'
 const { Controller } = hmpoFormWizard
 
 /**
@@ -38,9 +39,14 @@ class PageController extends Controller {
       const deepLinkInfo = req?.sessionModel?.get(this.checkToolDeepLinkSessionKey)
       if (deepLinkInfo) {
         req.form.options.deepLink = deepLinkInfo
+        req.form.options.datasetName = deepLinkInfo.datasetName
         backLink = wizardBackLink(req.originalUrl, deepLinkInfo)
       }
 
+      if (backLink) {
+        req.form.options.backLinkText = `Back to ${deepLinkInfo.datasetName} overview`
+      }
+
       backLink = backLink ?? this.options.backLink
       if (backLink) {
         req.form.options.lastPage = backLink
@@ -52,6 +58,13 @@ class PageController extends Controller {
       })
     }
 
+    const dataset = req?.sessionModel?.get('dataset')
+    try {
+      req.form.options.datasetName = datasetSlugToReadableName(dataset)
+    } catch (e) {
+      logger.warn(`Failed to get readable dataset name from slug: ${dataset}`)
+    }
+
     super.locals(req, res, next)
   }
 }

src/controllers/statusController.js

@@ -1,12 +1,15 @@
 import PageController from './pageController.js'
 import { getRequestData } from '../services/asyncRequestApi.js'
 import { finishedProcessingStatuses } from '../utils/utils.js'
+import { headingTexts, messageTexts } from '../content/statusPage.js'
 
 class StatusController extends PageController {
   async locals (req, res, next) {
     try {
       req.form.options.data = await getRequestData(req.params.id)
       req.form.options.processingComplete = finishedProcessingStatuses.includes(req.form.options.data.status)
+      req.form.options.headingTexts = headingTexts
+      req.form.options.messageTexts = messageTexts
       req.form.options.pollingEndpoint = `/api/status/${req.form.options.data.id}`
       super.locals(req, res, next)
     } catch (error) {

src/filters/filters.js

@@ -4,9 +4,9 @@ import validationMessageLookup from './validationMessageLookup.js'
 import toErrorList from './toErrorList.js'
 import prettifyColumnName from './prettifyColumnName.js'
 import getFullServiceName from './getFullServiceName.js'
-import { makeDatasetSlugToReadableNameFilter } from './makeDatasetSlugToReadableNameFilter.js'
 import { checkToolDeepLink } from './checkToolDeepLink.js'
 import pluralize from 'pluralize'
+import { datasetSlugToReadableName } from '../utils/datasetSlugToReadableName.js'
 
 /** maps dataset status (as returned by `fetchLpaOverview` middleware to a
  * CSS class used by the govuk-tag component
@@ -27,8 +27,7 @@ export function statusToTagClass (status) {
 
 const { govukMarkdown, govukDateTime } = xGovFilters
 
-const addFilters = (nunjucksEnv, { datasetNameMapping }) => {
-  const datasetSlugToReadableName = makeDatasetSlugToReadableNameFilter(datasetNameMapping)
+const addFilters = (nunjucksEnv) => {
   nunjucksEnv.addFilter('datasetSlugToReadableName', datasetSlugToReadableName)
 
   nunjucksEnv.addFilter('govukMarkdown', govukMarkdown)

src/middleware/common.middleware.js

@@ -39,8 +39,17 @@ export const fetchDatasetInfo = fetchOne({
  * @returns {boolean}
  */
 export const isResourceAccessible = (req) => req.resourceStatus.status === '200'
+export const isResourceIdValid = (req) => req.resourceStatus.resource.trim() !== ''
 export const isResourceNotAccessible = (req) => !isResourceAccessible(req)
 export const isResourceIdInParams = ({ params }) => !('resourceId' in params)
+export const isResourceDataPresent = (req) => 'resource' in req
+
+export const and = (...args) => {
+  return (req) => args.every(arg => arg(req))
+}
+export const or = (...args) => {
+  return (req) => args.some(arg => arg(req))
+}
 
 /**
  * Middleware. Updates req with `resource`.
@@ -72,7 +81,7 @@ export const fetchOrgInfo = fetchOne({
 
 /**
  * Middleware. Validates query params according to schema.
- * Short circuits with 400 error if validation fails
+ * Short circuits with 400 error if validation fails. Potentially updates req with `parsedParams`
  *
  * `this` needs: `{ schema }`
  *
@@ -82,7 +91,7 @@ export const fetchOrgInfo = fetchOne({
  */
 export function validateQueryParamsFn (req, res, next) {
   try {
-    v.parse(this.schema || v.any(), req.params)
+    req.parsedParams = v.parse(this.schema || v.any(), req.params)
     next()
   } catch (error) {
     res.status(400).render('errorPages/400', {})

src/middleware/datasetOverview.middleware.js

@@ -2,6 +2,8 @@ import { fetchDatasetInfo, fetchLatestResource, fetchLpaDatasetIssues, fetchOrgI
 import { fetchOne, fetchIf, fetchMany, renderTemplate, FetchOptions, onlyIf } from './middleware.builders.js'
 import { fetchResourceStatus, prepareDatasetTaskListErrorTemplateParams } from './datasetTaskList.middleware.js'
 import performanceDbApi from '../services/performanceDbApi.js'
+import logger from '../utils/logger.js'
+import { types } from '../utils/logging.js'
 
 const fetchColumnSummary = fetchMany({
   query: ({ params }) => `
@@ -36,11 +38,24 @@ const fetchSpecification = fetchOne({
   result: 'specification'
 })
 
+/**
+ * Middleware. Updates req with `datasetSpecification`
+ *
+ * @param req
+ * @param res
+ * @param next
+ */
 export const pullOutDatasetSpecification = (req, res, next) => {
   const { specification } = req
-  const collectionSpecifications = JSON.parse(specification.json)
-  const datasetSpecification = collectionSpecifications.find((spec) => spec.dataset === req.dataset.dataset)
-  req.specification = datasetSpecification
+  let collectionSpecifications
+  try {
+    collectionSpecifications = JSON.parse(specification.json)
+  } catch (e) {
+    // we can proceed but we probably should notify the user the displayed data may not be complete
+    logger.info('failed to parse specification JSON', { type: types.DataValidation, collection: req.dataset.collection })
+    collectionSpecifications = []
+  }
+  req.datasetSpecification = collectionSpecifications.find((spec) => spec.dataset === req.dataset.dataset)
   next()
 }
 
@@ -105,21 +120,21 @@ const fetchEntityCount = fetchOne({
 })
 
 export const prepareDatasetOverviewTemplateParams = (req, res, next) => {
-  const { orgInfo, specification, columnSummary, entityCount, sources, dataset, issues } = req
+  const { orgInfo, datasetSpecification, columnSummary, entityCount, sources, dataset, issues } = req
 
   const mappingFields = columnSummary[0]?.mapping_field?.split(';') ?? []
   const nonMappingFields = columnSummary[0]?.non_mapping_field?.split(';') ?? []
   const allFields = [...mappingFields, ...nonMappingFields]
 
-  const numberOfFieldsSupplied = specification.fields.map(field => field.field).reduce((acc, current) => {
-    return allFields.includes(current) ? acc + 1 : acc
+  const specFields = datasetSpecification ? datasetSpecification.fields : []
+  const numberOfFieldsSupplied = specFields.reduce((acc, field) => {
+    return allFields.includes(field.field) ? acc + 1 : acc
   }, 0)
-
-  const numberOfFieldsMatched = specification.fields.map(field => field.field).reduce((acc, current) => {
-    return mappingFields.includes(current) ? acc + 1 : acc
+  const numberOfFieldsMatched = specFields.reduce((acc, field) => {
+    return mappingFields.includes(field.field) ? acc + 1 : acc
   }, 0)
 
-  const numberOfExpectedFields = specification.fields.length
+  const numberOfExpectedFields = specFields.length
 
   // I'm pretty sure every endpoint has a separate documentation-url, but this isn't currently represented in the performance db. need to double check this and update if so
   const endpoints = sources.sort((a, b) => {

src/middleware/datasetTaskList.middleware.js

@@ -1,4 +1,15 @@
-import { fetchDatasetInfo, isResourceAccessible, isResourceNotAccessible, fetchLatestResource, fetchEntityCount, logPageError, fetchLpaDatasetIssues, validateQueryParams, getDatasetTaskListError } from './common.middleware.js'
+import {
+  fetchDatasetInfo,
+  isResourceAccessible,
+  isResourceNotAccessible,
+  fetchLatestResource,
+  fetchEntityCount,
+  logPageError,
+  fetchLpaDatasetIssues,
+  validateQueryParams,
+  getDatasetTaskListError,
+  isResourceIdValid, and
+} from './common.middleware.js'
 import { fetchOne, fetchIf, onlyIf, renderTemplate } from './middleware.builders.js'
 import performanceDbApi from '../services/performanceDbApi.js'
 import { statusToTagClass } from '../filters/filters.js'
@@ -46,7 +57,6 @@ export const prepareDatasetTaskListTemplateParams = (req, res, next) => {
   const { issues, entityCount: entityCountRow, params, dataset, orgInfo: organisation } = req
   const { entity_count: entityCount } = entityCountRow ?? { entity_count: 0 }
   const { lpa, dataset: datasetId } = params
-  console.assert(req.resourceStatus.resource === req.resource.resource, 'mismatch between resourceStatus and resource data')
   console.assert(typeof entityCount === 'number', 'entityCount should be a number')
 
   const taskList = issues.map((issue) => {
@@ -113,14 +123,17 @@ const validateParams = validateQueryParams({
   })
 })
 
+/* eslint-disable-next-line no-return-assign */
+const zeroEntityCount = (req) => req.entityCount = { entity_count: 0 }
+
 export default [
   validateParams,
   fetchResourceStatus,
   fetchOrgInfoWithStatGeo,
   fetchDatasetInfo,
   fetchIf(isResourceAccessible, fetchLatestResource),
   fetchIf(isResourceAccessible, fetchLpaDatasetIssues),
-  fetchIf(isResourceAccessible, fetchEntityCount),
+  fetchIf(and(isResourceAccessible, isResourceIdValid), fetchEntityCount, zeroEntityCount),
   onlyIf(isResourceAccessible, prepareDatasetTaskListTemplateParams),
   onlyIf(isResourceAccessible, getDatasetTaskList),
   onlyIf(isResourceNotAccessible, prepareDatasetTaskListErrorTemplateParams),