From 310e9033a48cf95b2d6499214c1a403b5b1db22c Mon Sep 17 00:00:00 2001 From: George Goodall Date: Mon, 14 Oct 2024 13:39:34 +0100 Subject: [PATCH] have issue details show rows based on spec, and error summary based on entities --- src/middleware/common.middleware.js | 12 +- src/middleware/issueDetails.middleware.js | 171 ++++++---------------- src/middleware/issueTable.middleware.js | 18 +-- src/services/performanceDbApi.js | 2 +- 4 files changed, 58 insertions(+), 145 deletions(-) diff --git a/src/middleware/common.middleware.js b/src/middleware/common.middleware.js index 800b3a1b..bf682e0b 100644 --- a/src/middleware/common.middleware.js +++ b/src/middleware/common.middleware.js @@ -175,7 +175,7 @@ export async function reformatIssuesToBeByEntryNumber (req, res, next) { export function formatErrorSummaryParams (req, res, next) { const { lpa, dataset: datasetId, issue_type: issueType, issue_field: issueField } = req.params - const { issuesByEntryNumber, entityCount: entityCountRow, issuesWithReferences } = req + const { entityCount: entityCountRow, issuesWithReferences, entities } = req const { entity_count: entityCount } = entityCountRow ?? { entity_count: 0 } @@ -184,12 +184,12 @@ export function formatErrorSummaryParams (req, res, next) { let errorHeading let issueItems - if (Object.keys(issuesByEntryNumber).length < entityCount) { - errorHeading = performanceDbApi.getTaskMessage({ issue_type: issueType, num_issues: issuesWithReferences.length, entityCount, field: issueField }, true) - issueItems = Object.keys(issuesByEntryNumber).map((entryNumber, i) => { + if (entities.length < entityCount) { + errorHeading = performanceDbApi.getTaskMessage({ issue_type: issueType, num_issues: entities.length, entityCount, field: issueField }, true) + issueItems = entities.map((entity, index) => { return { - html: performanceDbApi.getTaskMessage({ issue_type: issueType, num_issues: 1, field: issueField }) + ` in record ${entryNumber}`, - href: `${BaseSubpath}${entryNumber}` + html: performanceDbApi.getTaskMessage({ issue_type: issueType, num_issues: 1, field: issueField }) + ` in entity ${entity?.reference?.value || entity?.reference}`, + href: `${BaseSubpath}${index + 1}` } }) } else { diff --git a/src/middleware/issueDetails.middleware.js b/src/middleware/issueDetails.middleware.js index 0ac54fd9..034533d8 100644 --- a/src/middleware/issueDetails.middleware.js +++ b/src/middleware/issueDetails.middleware.js @@ -1,6 +1,26 @@ -import performanceDbApi from '../services/performanceDbApi.js' -import { fetchDatasetInfo, fetchEntitiesFromOrganisationAndEntryNumbers, fetchEntityCount, fetchIssueEntitiesCount, fetchIssues, fetchLatestResource, fetchOrgInfo, formatErrorSummaryParams, getEntryNumbersWithIssues, isResourceIdNotInParams, logPageError, reformatIssuesToBeByEntryNumber, takeResourceIdFromParams, validateQueryParams } from './common.middleware.js' -import { fetchIf, parallel, renderTemplate } from './middleware.builders.js' +import { + addIssuesToEntities, + extractJsonFieldFromEntities, + fetchActiveResourcesForOrganisationAndDataset, + fetchDatasetInfo, + fetchEntitiesFromIssuesWithReferences, + fetchEntityCount, + fetchIssueEntitiesCount, + fetchIssuesWithReferencesFromResourcesDatasetIssuetypefield, + fetchLatestResource, + fetchOrgInfo, + fetchSpecification, + formatErrorSummaryParams, + hasEntities, + isResourceIdNotInParams, + logPageError, + nestEntityFields, + pullOutDatasetSpecification, + replaceUnderscoreWithHyphenForEntities, + takeResourceIdFromParams, + validateQueryParams +} from './common.middleware.js' +import { fetchIf, renderTemplate } from './middleware.builders.js' import * as v from 'valibot' import { pagination } from '../utils/pagination.js' @@ -17,30 +37,6 @@ const validateIssueDetailsQueryParams = validateQueryParams.bind({ schema: IssueDetailsQueryParams }) -/** - * - * Middleware. Updates `req` with `entryData` - * - * Requires `dataset` and `entryNumber` - * - * @param {*} req - * @param {*} res - * @param {*} next - * - */ -async function fetchEntry (req, res, next) { - const { dataset: datasetId } = req.params - const { entryNumber } = req - - req.entryData = await performanceDbApi.getEntry( - req.resource.resource, - entryNumber, - datasetId - ) - - next() -} - /** * * @param {string} errorMessage @@ -71,73 +67,6 @@ const getIssueField = (text, html, classes) => { } } -/** - * - * @param {*} issueType - * @param {*} issuesByEntryNumber - * @param {*} row - * @returns {{key: {text: string}, value: { html: string}, classes: string}} - */ -const processEntryRow = (issueType, issuesByEntryNumber = {}, row) => { - const { entry_number: entryNumber } = row - console.assert(entryNumber, 'precessEntryRow(): entry_number not in row') - - let hasError = false - let issueIndex - if (issuesByEntryNumber[entryNumber]) { - issueIndex = issuesByEntryNumber[entryNumber].findIndex( - (issue) => issue.field === row.field - ) - hasError = issueIndex >= 0 - } - - let valueHtml = '' - let classes = '' - if (hasError) { - const message = - issuesByEntryNumber[entryNumber][issueIndex].message || issueType - valueHtml += issueErrorMessageHtml(message, null) - classes += 'dl-summary-card-list__row--error' - } - valueHtml += row.value - - return getIssueField(row.field, valueHtml, classes) -} - -/** - * Middleware. Extracts the entry number from the page number in the request. - * - * @param {object} req - The request object - * @param {object} res - The response object - * @param {function} next - The next middleware function - * - * @throws {Error} If the page number cannot be parsed as an integer - * @throws {Error} If the entry number is not found (404) - */ -export function getEntryNumberFromPageNumber (req, res, next) { - const { issuesByEntryNumber } = req - const { pageNumber } = req.params - - const pageNumberAsInt = parseInt(pageNumber) - if (isNaN(pageNumberAsInt)) { - const error = new Error('page number could not be parsed as an integer') - error.status = 400 - return next(error) - } - - const issuesByEntryNumberIndex = pageNumberAsInt - 1 - const pageNumberToEntryNumberMap = Object.keys(issuesByEntryNumber) - - if (issuesByEntryNumberIndex < 0 || issuesByEntryNumberIndex >= pageNumberToEntryNumberMap.length) { - const error = new Error('not found') - error.status = 404 - return next(error) - } - - req.entryNumber = pageNumberToEntryNumberMap[issuesByEntryNumberIndex] - next() -} - /** * Middleware. Prepares template parameters for the issue details page. * @@ -151,47 +80,42 @@ export function getEntryNumberFromPageNumber (req, res, next) { * from the request, and organizes it into a template parameters object that can be used to render the page. */ export function prepareIssueDetailsTemplateParams (req, res, next) { - const { entryData, issueEntitiesCount, issuesByEntryNumber, errorSummary, entryNumber } = req + const { entities, issueEntitiesCount, errorSummary, specification } = req const { lpa, dataset: datasetId, issue_type: issueType, issue_field: issueField, pageNumber: pageNumberString } = req.params const pageNumber = parseInt(pageNumberString) const BaseSubpath = `/organisations/${lpa}/${datasetId}/${issueType}/${issueField}/entry/` - const fields = entryData.map((row) => processEntryRow(issueType, issuesByEntryNumber, row)) - const entityIssues = issuesByEntryNumber[entryNumber] || [] - for (const issue of entityIssues) { - if (!fields.find((field) => field.key.text === issue.field)) { - const errorMessage = issue.message || issueType - // TODO: pull the html out of here and into the template - const valueHtml = issueErrorMessageHtml(errorMessage, issue.value) - const classes = 'dl-summary-card-list__row--error' + const entity = entities[pageNumber - 1] - fields.push(getIssueField(issue.field, valueHtml, classes)) + const fields = specification.fields.map(({ field }) => { + let valueHtml = '' + let classes = '' + if (entity[field].issue) { + valueHtml += issueErrorMessageHtml(entity[field].issue.message, null) + classes += 'dl-summary-card-list__row--error' } - } + valueHtml += entity[field].value || '' + return getIssueField(field, valueHtml, classes) + }) - const geometries = entryData - .filter((row) => row.field === 'geometry') - .map((row) => row.value) const entry = { - title: `entry: ${entryNumber}`, + title: `entry: ${entity.reference.value}`, fields, - geometries + geometries: [entity.geometry.value] } const paginationObj = { items: [] } - const entryNumbers = Object.keys(issuesByEntryNumber) - if (pageNumber > 1) { paginationObj.previous = { href: `${BaseSubpath}${pageNumber - 1}` } } - if (pageNumber < entryNumbers.length) { + if (pageNumber < entities.length) { paginationObj.next = { href: `${BaseSubpath}${pageNumber + 1}` } @@ -244,16 +168,17 @@ export default [ fetchOrgInfo, fetchDatasetInfo, fetchIf(isResourceIdNotInParams, fetchLatestResource, takeResourceIdFromParams), - fetchIssues, - getEntryNumbersWithIssues, - fetchEntitiesFromOrganisationAndEntryNumbers, - reformatIssuesToBeByEntryNumber, - getEntryNumberFromPageNumber, - parallel([ - fetchEntry, - fetchEntityCount, - fetchIssueEntitiesCount - ]), + fetchSpecification, + pullOutDatasetSpecification, + fetchActiveResourcesForOrganisationAndDataset, + fetchIssuesWithReferencesFromResourcesDatasetIssuetypefield, + fetchEntitiesFromIssuesWithReferences, + fetchIf(hasEntities, extractJsonFieldFromEntities), + fetchIf(hasEntities, replaceUnderscoreWithHyphenForEntities), + fetchIf(hasEntities, nestEntityFields), + fetchIf(hasEntities, addIssuesToEntities), + fetchEntityCount, + fetchIssueEntitiesCount, formatErrorSummaryParams, prepareIssueDetailsTemplateParams, getIssueDetails, diff --git a/src/middleware/issueTable.middleware.js b/src/middleware/issueTable.middleware.js index b69ec516..937c5ac3 100644 --- a/src/middleware/issueTable.middleware.js +++ b/src/middleware/issueTable.middleware.js @@ -15,7 +15,6 @@ import { nestEntityFields, paginateEntitiesAndPullOutCount, pullOutDatasetSpecification, - reformatIssuesToBeByEntryNumber, replaceUnderscoreWithHyphenForEntities, takeResourceIdFromParams, validateQueryParams, @@ -23,7 +22,7 @@ import { fetchIssuesWithReferencesFromResourcesDatasetIssuetypefield, fetchEntitiesFromIssuesWithReferences } from './common.middleware.js' -import { fetchIf, parallel, renderTemplate } from './middleware.builders.js' +import { fetchIf, renderTemplate } from './middleware.builders.js' import * as v from 'valibot' const paginationPageLength = 20 @@ -160,21 +159,11 @@ export const getIssueTable = renderTemplate({ handlerName: 'getIssueTable' }) -// const getEntitiesWithIssuesMiddlewareChain = [ -// fetchResourcesFromOrganisationAndDataset, -// fetchIssuesFromResourcesDatasetIssuetypefield, -// // have a list of issues with resource, and entry number -// getReferencesOfIssueEntities, -// getEntitiesFromRefernces -// ] - export default [ validateIssueTableQueryParams, setDefaultQueryParams, - parallel([ - fetchOrgInfo, - fetchDatasetInfo - ]), + fetchOrgInfo, + fetchDatasetInfo, fetchIf(isResourceIdNotInParams, fetchLatestResource, takeResourceIdFromParams), fetchSpecification, pullOutDatasetSpecification, @@ -188,7 +177,6 @@ export default [ fetchIf(hasEntities, nestEntityFields), fetchIf(hasEntities, addIssuesToEntities), fetchEntityCount, - reformatIssuesToBeByEntryNumber, formatErrorSummaryParams, createPaginationTemplatePrams, prepareIssueTableTemplateParams, diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index b68edba9..1359df0e 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -409,7 +409,7 @@ export default { issuesWithReferenceFromResourcesDatasetIssueTypeFieldQuery ({ resources, dataset, issueType, issueField }) { return /* sql */ ` - SELECT DISTINCT i.message, i.value, i.field, i.issue_type, i.entry_number, f.value as reference + SELECT DISTINCT i.message, i.value, i.field, i.issue_type, i.entry_number, i.resource, f.value as reference FROM issue i LEFT JOIN fact_resource fr ON i.entry_number = fr.entry_number AND i.resource = fr.resource LEFT JOIN fact f ON fr.fact = f.fact