Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken endpoints UX changes #738

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
4 changes: 3 additions & 1 deletion src/controllers/OrganisationsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -17,7 +18,8 @@ const organisationsController = {
issueTableMiddleware,
getStartedMiddleware,
overviewMiddleware,
datasetDataviewMiddleware
datasetDataviewMiddleware,
datasetEndpointIssueMiddleware
}

export default organisationsController
82 changes: 79 additions & 3 deletions src/middleware/common.middleware.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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} entities with no json field`,
{ type: types.App, endpoint: req.originalUrl })
}

rosado marked this conversation as resolved.
Show resolved Hide resolved
next()
}

Expand Down Expand Up @@ -629,3 +636,72 @@ export const prepareIssueDetailsTemplateParams = (req, res, next) => {

next()
}

export const validateOrgAndDatasetQueryParams = validateQueryParams({
schema: v.object({
lpa: v.string(),
dataset: 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'
})
86 changes: 86 additions & 0 deletions src/middleware/datasetEndpointIssue.middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import * as v from 'valibot'
import {
fetchDatasetInfo,
getDatasetTaskListError,
validateOrgAndDatasetQueryParams, validateQueryParams
} from './common.middleware.js'
import { fetchOne } from './middleware.builders.js'
import '../types/datasette.js'

const fetchOrgInfoWithStatGeo = fetchOne({
query: ({ params }) => {
return /* sql */ `SELECT name, organisation, statistical_geography FROM organisation WHERE organisation = '${params.lpa}'`
},
result: 'orgInfo'
})
rosado marked this conversation as resolved.
Show resolved Hide resolved

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'
})
rosado marked this conversation as resolved.
Show resolved Hide resolved

/**
*
* @param { { orgInfo: OrgInfo, dataset: DatasetInfo, source: Source, now: Date }} req
* @param res
* @param next
*/
export const prepareDatasetEndpointIssueTemplateParams = (req, res, next) => {
const { orgInfo: organisation, dataset, source, now } = req

/** @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((now || new Date()).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
]
100 changes: 29 additions & 71 deletions src/middleware/datasetOverview.middleware.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -249,8 +209,6 @@ export default [
fetchResourceStatus,
fetchIf(isResourceIdInParams, fetchLatestResource, takeResourceIdFromParams),
fetchIf(isResourceAccessible, fetchLpaDatasetIssues),
onlyIf(isResourceNotAccessible, prepareDatasetTaskListErrorTemplateParams),
onlyIf(isResourceNotAccessible, getDatasetTaskListError),
fetchSpecification,
pullOutDatasetSpecification,
fetchSources,
Expand Down
Loading
Loading