broken endpoints ux changes

issue #699

Changes how/where endpoints are displayed.

- The dataset overview page highlights endpoints with 'Error' status
and provides links to the endpoint error page.
- The dataset task list now includes an item for each 'Error'-ing
endpoint
- The task count includes the 'Error'-ing endpoint
- the `latest_200_date` template param no longer includes the time - it
made no sense
- updated the middleware chain for dataset task list to no longer
conditionally include the endpoint error page
- made logging in `extractJsonFieldFromEntities` less noisy

config/index.js

@@ -3,7 +3,7 @@ import { combineConfigs, validateConfig } from './util.js'
 
 /**
  *
- * @returns {{defaultConfig, environment, url, checkService: { userAgent: string }}}
+ * @returns {{defaultConfig, environment, url, checkService: { userAgent: string }, datasetsConfig}}
  */
 const getConfig = () => {
   const environment = process.env.NODE_ENV || process.env.ENVIRONMENT || 'production'

src/controllers/OrganisationsController.js

@@ -7,6 +7,7 @@ import organisationsMiddleware from '../middleware/organisations.middleware.js'
 import getStartedMiddleware from '../middleware/getStarted.middleware.js'
 import overviewMiddleware from '../middleware/overview.middleware.js'
 import datasetDataviewMiddleware from '../middleware/dataview.middleware.js'
+import datasetEndpointIssueMiddleware from '../middleware/datasetEndpointIssue.middleware.js'
 
 const organisationsController = {
   organisationsMiddleware,
@@ -17,7 +18,8 @@ const organisationsController = {
   issueTableMiddleware,
   getStartedMiddleware,
   overviewMiddleware,
-  datasetDataviewMiddleware
+  datasetDataviewMiddleware,
+  datasetEndpointIssueMiddleware
 }
 
 export default organisationsController

src/middleware/common.middleware.js

@@ -1,7 +1,7 @@
 import logger from '../utils/logger.js'
 import { types } from '../utils/logging.js'
 import performanceDbApi from '../services/performanceDbApi.js'
-import { fetchOne, FetchOptions, FetchOneFallbackPolicy, fetchMany, renderTemplate } from './middleware.builders.js'
+import { fetchMany, fetchOne, FetchOneFallbackPolicy, FetchOptions, renderTemplate } from './middleware.builders.js'
 import * as v from 'valibot'
 import { pagination } from '../utils/pagination.js'
 import datasette from '../services/datasette.js'
@@ -307,22 +307,29 @@ export const extractJsonFieldFromEntities = (req, res, next) => {
     return next(new Error('Invalid entities format'))
   }
 
+  let numEntitiesWithNoJson = 0
   req.entities = entities.map(entity => {
     const jsonField = entity.json
     if (!jsonField || jsonField === '') {
-      logger.info(`common.middleware/extractJsonField: No json field for entity ${entity.toString()}`)
+      numEntitiesWithNoJson += 1
       return entity
     }
     entity.json = undefined
     try {
       const parsedJson = JSON.parse(jsonField)
       entity = Object.assign({}, parsedJson, entity)
     } catch (err) {
-      logger.warn(`common.middleware/extractJsonField: Error parsing JSON for entity ${entity.toString()}: ${err.message}`)
+      logger.warn('common.middleware/extractJsonField: Error parsing JSON',
+        { type: types.App, json: jsonField, entity: entity.entity, errorMessage: err.message })
     }
     return entity
   })
 
+  if (numEntitiesWithNoJson > 0) {
+    logger.info(`Got ${numEntitiesWithNoJson.length} entities with no json field`,
+      { type: types.App, endpoint: req.originalUrl })
+  }
+
   next()
 }
 
@@ -629,3 +636,74 @@ export const prepareIssueDetailsTemplateParams = (req, res, next) => {
 
   next()
 }
+
+export const validateOrgAndDatasetQueryParams = validateQueryParams({
+  schema: v.object({
+    lpa: v.string(),
+    dataset: v.string()
+    // resource: v.string()
+  })
+})
+
+export const fetchSources = fetchMany({
+  query: ({ params }) => `
+    WITH RankedEndpoints AS (
+      SELECT
+        rhe.endpoint,
+        rhe.endpoint_url,
+        case 
+            when rhe.status = '' or rhe.status is null then null
+            else cast(rhe.status as int)
+        end as status,
+        rhe.exception,
+        rhe.resource,
+        rhe.latest_log_entry_date,
+        rhe.endpoint_entry_date,
+        rhe.endpoint_end_date,
+        rhe.resource_start_date as resource_start_date,
+        rhe.resource_end_date,
+        s.documentation_url,
+        ROW_NUMBER() OVER (
+          PARTITION BY rhe.endpoint_url
+          ORDER BY
+            rhe.latest_log_entry_date DESC
+        ) AS row_num
+      FROM
+        reporting_historic_endpoints rhe
+        LEFT JOIN source s ON rhe.endpoint = s.endpoint
+      WHERE
+        REPLACE(rhe.organisation, '-eng', '') = '${params.lpa}'
+        AND rhe.pipeline = '${params.dataset}'
+        AND (
+          rhe.resource_end_date >= current_timestamp
+          OR rhe.resource_end_date IS NULL
+          OR rhe.resource_end_date = ''
+        )
+        AND (
+          rhe.endpoint_end_date >= current_timestamp
+          OR rhe.endpoint_end_date IS NULL
+          OR rhe.endpoint_end_date = ''
+        )
+    )
+    SELECT
+      endpoint,
+      endpoint_url,
+      status,
+      exception,
+      resource,
+      latest_log_entry_date,
+      endpoint_entry_date,
+      endpoint_end_date,
+      resource_start_date,
+      resource_end_date,
+      documentation_url
+    FROM
+      RankedEndpoints
+    WHERE
+      row_num = 1
+    ORDER BY
+      latest_log_entry_date DESC;
+  `,
+  result: 'sources'
+})
+

src/middleware/datasetEndpointIssue.middleware.js

@@ -0,0 +1,89 @@
+import * as v from 'valibot'
+import {
+  fetchDatasetInfo,
+  getDatasetTaskListError,
+  validateOrgAndDatasetQueryParams, validateQueryParams
+} from './common.middleware.js'
+import { fetchOne } from './middleware.builders.js'
+
+/** @typedef {import('../types/datasette')} Types */
+
+const fetchOrgInfoWithStatGeo = fetchOne({
+  query: ({ params }) => {
+    return /* sql */ `SELECT name, organisation, statistical_geography FROM organisation WHERE organisation = '${params.lpa}'`
+  },
+  result: 'orgInfo'
+})
+
+const fetchSourceByEndpoint = fetchOne({
+  query: ({ params }) => {
+    return /* sql */ `
+        SELECT
+            rhe.endpoint,
+            rhe.endpoint_url,
+            rhe.status,
+            rhe.exception,
+            rhe.latest_log_entry_date,
+            rle.days_since_200
+        FROM
+            reporting_historic_endpoints rhe
+            LEFT JOIN reporting_latest_endpoints rle 
+                ON rhe.endpoint = rle.endpoint
+        WHERE 
+            rhe.endpoint = '${params.endpoint}'
+        ORDER BY
+            rhe.latest_log_entry_date DESC
+        LIMIT 1`
+  },
+  result: 'source'
+})
+
+/**
+ *
+ * @param { { orgInfo: Types.OrgInfo, dataset: Types.DatasetInfo, source: Types.Source }} req
+ * @param res
+ * @param next
+ */
+export const prepareDatasetEndpointIssueTemplateParams = (req, res, next) => {
+  const { orgInfo: organisation, dataset, source } = req
+
+  const today = new Date()
+
+  /** @type {number|null} */
+  const daysSince200 = source.days_since_200
+  /** @type {String|null} */
+  let last200Datetime = null
+  if (Number.isSafeInteger(daysSince200) && daysSince200 >= 0) {
+    const last200Date = new Date(today.getTime() - daysSince200 * 24 * 60 * 60 * 1000)
+    last200Datetime = last200Date.toISOString().split('T')[0]
+  }
+
+  req.templateParams = {
+    organisation,
+    dataset,
+    errorData: {
+      endpoint_url: source.endpoint_url,
+      http_status: source.status,
+      latest_log_entry_date: source.latest_log_entry_date,
+      latest_200_date: last200Datetime
+    }
+  }
+
+  next()
+}
+
+const validateEndpointQueryParam = validateQueryParams({
+  schema: v.object({
+    endpoint: v.pipe(v.string(), v.minLength(1))
+  })
+})
+
+export default [
+  validateOrgAndDatasetQueryParams,
+  validateEndpointQueryParam,
+  fetchOrgInfoWithStatGeo,
+  fetchDatasetInfo,
+  fetchSourceByEndpoint,
+  prepareDatasetEndpointIssueTemplateParams,
+  getDatasetTaskListError
+]

src/middleware/datasetOverview.middleware.js

@@ -1,6 +1,17 @@
-import { fetchDatasetInfo, fetchLatestResource, fetchLpaDatasetIssues, fetchOrgInfo, getDatasetTaskListError, isResourceAccessible, isResourceIdInParams, isResourceNotAccessible, logPageError, pullOutDatasetSpecification, takeResourceIdFromParams } from './common.middleware.js'
-import { fetchOne, fetchIf, fetchMany, renderTemplate, FetchOptions, onlyIf } from './middleware.builders.js'
-import { fetchResourceStatus, prepareDatasetTaskListErrorTemplateParams } from './datasetTaskList.middleware.js'
+import {
+  fetchDatasetInfo,
+  fetchLatestResource,
+  fetchLpaDatasetIssues,
+  fetchOrgInfo,
+  fetchSources,
+  isResourceAccessible,
+  isResourceIdInParams,
+  logPageError,
+  pullOutDatasetSpecification,
+  takeResourceIdFromParams
+} from './common.middleware.js'
+import { fetchIf, fetchMany, fetchOne, FetchOptions, renderTemplate } from './middleware.builders.js'
+import { fetchResourceStatus } from './datasetTaskList.middleware.js'
 import performanceDbApi from '../services/performanceDbApi.js'
 import { getDeadlineHistory, requiredDatasets } from '../utils/utils.js'
 import logger from '../utils/logger.js'
@@ -39,65 +50,6 @@ const fetchSpecification = fetchOne({
   result: 'specification'
 })
 
-const fetchSources = fetchMany({
-  query: ({ params }) => `
-    WITH RankedEndpoints AS (
-      SELECT
-        rhe.endpoint,
-        rhe.endpoint_url,
-        rhe.status,
-        rhe.exception,
-        rhe.resource,
-        rhe.latest_log_entry_date,
-        rhe.endpoint_entry_date,
-        rhe.endpoint_end_date,
-        rhe.resource_start_date as resource_start_date,
-        rhe.resource_end_date,
-        s.documentation_url,
-        ROW_NUMBER() OVER (
-          PARTITION BY rhe.endpoint_url
-          ORDER BY
-            rhe.latest_log_entry_date DESC
-        ) AS row_num
-      FROM
-        reporting_historic_endpoints rhe
-        LEFT JOIN source s ON rhe.endpoint = s.endpoint
-      WHERE
-        REPLACE(rhe.organisation, '-eng', '') = '${params.lpa}'
-        AND rhe.pipeline = '${params.dataset}'
-        AND (
-          rhe.resource_end_date >= current_timestamp
-          OR rhe.resource_end_date IS NULL
-          OR rhe.resource_end_date = ''
-        )
-        AND (
-          rhe.endpoint_end_date >= current_timestamp
-          OR rhe.endpoint_end_date IS NULL
-          OR rhe.endpoint_end_date = ''
-        )
-    )
-    SELECT
-      endpoint,
-      endpoint_url,
-      status,
-      exception,
-      resource,
-      latest_log_entry_date,
-      endpoint_entry_date,
-      endpoint_end_date,
-      resource_start_date,
-      resource_end_date,
-      documentation_url
-    FROM
-      RankedEndpoints
-    WHERE
-      row_num = 1
-    ORDER BY
-      latest_log_entry_date DESC;
-  `,
-  result: 'sources'
-})
-
 /**
  * Sets notices from a source key in the request object.
  *
@@ -175,6 +127,12 @@ const fetchEntityCount = fetchOne({
   dataset: FetchOptions.fromParams
 })
 
+/**
+ *
+ * @param req {{ orgInfo: OrgInfo, sources: Source[], issues?: Issue[] }} request object
+ * @param res {import('express').Response}
+ * @param next {import('express').NextFunction}
+ */
 export const prepareDatasetOverviewTemplateParams = (req, res, next) => {
   const { orgInfo, datasetSpecification, columnSummary, entityCount, sources, dataset, issues, notice } = req
 
@@ -192,24 +150,26 @@ export const prepareDatasetOverviewTemplateParams = (req, res, next) => {
 
   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
+  let endpointErrorIssues = 0
   const endpoints = sources.sort((a, b) => {
-    if (a.status >= 200 && a.status < 300) return -1
-    if (b.status >= 200 && b.status < 300) return 1
+    if (a.status && a.status >= 200 && a.status < 300) return -1
+    if (b.status && b.status >= 200 && b.status < 300) return 1
     return 0
   }).map((source, index) => {
     let error
 
-    if (parseInt(source.status) < 200 || parseInt(source.status) >= 300) {
+    if (!source.status || source.status < 200 || source.status >= 300) {
       error = {
-        code: parseInt(source.status),
+        code: source.status,
         exception: source.exception
       }
+      endpointErrorIssues += 1
     }
 
     return {
       name: `Data Url ${index}`,
-      endpoint: source.endpoint_url,
+      endpoint: source.endpoint,
+      endpoint_url: source.endpoint_url,
       documentation_url: source.documentation_url,
       lastAccessed: source.latest_log_entry_date,
       lastUpdated: source.resource_start_date, // as in: when was the _resource_ updated, not data under that resource
@@ -220,7 +180,7 @@ export const prepareDatasetOverviewTemplateParams = (req, res, next) => {
   req.templateParams = {
     organisation: orgInfo,
     dataset,
-    taskCount: issues.length ?? 0,
+    taskCount: (issues ?? []).length + endpointErrorIssues,
     stats: {
       numberOfFieldsSupplied: numberOfFieldsSupplied ?? 0,
       numberOfFieldsMatched: numberOfFieldsMatched ?? 0,
@@ -249,8 +209,6 @@ export default [
   fetchResourceStatus,
   fetchIf(isResourceIdInParams, fetchLatestResource, takeResourceIdFromParams),
   fetchIf(isResourceAccessible, fetchLpaDatasetIssues),
-  onlyIf(isResourceNotAccessible, prepareDatasetTaskListErrorTemplateParams),
-  onlyIf(isResourceNotAccessible, getDatasetTaskListError),
   fetchSpecification,
   pullOutDatasetSpecification,
   fetchSources,