Skip to content

Commit

Permalink
have issue details show rows based on spec, and error summary based o…
Browse files Browse the repository at this point in the history
…n entities
  • Loading branch information
GeorgeGoodall committed Oct 14, 2024
1 parent b1c8240 commit 310e903
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 145 deletions.
12 changes: 6 additions & 6 deletions src/middleware/common.middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand All @@ -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) {

Check failure on line 187 in src/middleware/common.middleware.js

View workflow job for this annotation

GitHub Actions / run-tests / test

test/unit/middleware/common.middleware.test.js > formatErrorSummaryParams > formats the error summary params

TypeError: Cannot read properties of undefined (reading 'length') ❯ Module.formatErrorSummaryParams src/middleware/common.middleware.js:187:16 ❯ test/unit/middleware/common.middleware.test.js:87:5

Check failure on line 187 in src/middleware/common.middleware.js

View workflow job for this annotation

GitHub Actions / test

test/unit/middleware/common.middleware.test.js > formatErrorSummaryParams > formats the error summary params

TypeError: Cannot read properties of undefined (reading 'length') ❯ Module.formatErrorSummaryParams src/middleware/common.middleware.js:187:16 ❯ test/unit/middleware/common.middleware.test.js:87:5
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 {
Expand Down
171 changes: 48 additions & 123 deletions src/middleware/issueDetails.middleware.js
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -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
Expand Down Expand Up @@ -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.
*
Expand All @@ -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]

Check failure on line 89 in src/middleware/issueDetails.middleware.js

View workflow job for this annotation

GitHub Actions / run-tests / test

test/unit/middleware/issueDetails.middleware.test.js > issueDetails.middleware.js > prepareIssueDetailsTemplateParams > should correctly set the template params

TypeError: Cannot read properties of undefined (reading '0') ❯ Module.prepareIssueDetailsTemplateParams src/middleware/issueDetails.middleware.js:89:26 ❯ test/unit/middleware/issueDetails.middleware.test.js:79:7

Check failure on line 89 in src/middleware/issueDetails.middleware.js

View workflow job for this annotation

GitHub Actions / run-tests / test

test/unit/middleware/issueDetails.middleware.test.js > issueDetails.middleware.js > prepareIssueDetailsTemplateParams > should correctly set the template params with the correct geometry params

TypeError: Cannot read properties of undefined (reading '0') ❯ Module.prepareIssueDetailsTemplateParams src/middleware/issueDetails.middleware.js:89:26 ❯ test/unit/middleware/issueDetails.middleware.test.js:177:7

Check failure on line 89 in src/middleware/issueDetails.middleware.js

View workflow job for this annotation

GitHub Actions / test

test/unit/middleware/issueDetails.middleware.test.js > issueDetails.middleware.js > prepareIssueDetailsTemplateParams > should correctly set the template params

TypeError: Cannot read properties of undefined (reading '0') ❯ Module.prepareIssueDetailsTemplateParams src/middleware/issueDetails.middleware.js:89:26 ❯ test/unit/middleware/issueDetails.middleware.test.js:79:7

Check failure on line 89 in src/middleware/issueDetails.middleware.js

View workflow job for this annotation

GitHub Actions / test

test/unit/middleware/issueDetails.middleware.test.js > issueDetails.middleware.js > prepareIssueDetailsTemplateParams > should correctly set the template params with the correct geometry params

TypeError: Cannot read properties of undefined (reading '0') ❯ Module.prepareIssueDetailsTemplateParams src/middleware/issueDetails.middleware.js:89:26 ❯ test/unit/middleware/issueDetails.middleware.test.js:177:7

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}`
}
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 3 additions & 15 deletions src/middleware/issueTable.middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@ import {
nestEntityFields,
paginateEntitiesAndPullOutCount,
pullOutDatasetSpecification,
reformatIssuesToBeByEntryNumber,
replaceUnderscoreWithHyphenForEntities,
takeResourceIdFromParams,
validateQueryParams,
fetchActiveResourcesForOrganisationAndDataset,
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
Expand Down Expand Up @@ -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,
Expand All @@ -188,7 +177,6 @@ export default [
fetchIf(hasEntities, nestEntityFields),
fetchIf(hasEntities, addIssuesToEntities),
fetchEntityCount,
reformatIssuesToBeByEntryNumber,
formatErrorSummaryParams,
createPaginationTemplatePrams,
prepareIssueTableTemplateParams,
Expand Down
2 changes: 1 addition & 1 deletion src/services/performanceDbApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 310e903

Please sign in to comment.