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

482 dataset details page provision level view table #609

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
f1c091b
Merge commit '00281c82ae7b3a77cb201725db7e03678523cffb' into 482-data…
GeorgeGoodall Oct 31, 2024
1633dbb
add initial routing
GeorgeGoodall Oct 31, 2024
87c2fc6
add example params to table
GeorgeGoodall Oct 31, 2024
0b02013
have table show
GeorgeGoodall Oct 31, 2024
4d70354
have table span whole page
GeorgeGoodall Oct 31, 2024
d176f03
add navigation to the top
GeorgeGoodall Oct 31, 2024
fae5f0b
Merge remote-tracking branch 'origin/main' into 482-dataset-details-p…
GeorgeGoodall Oct 31, 2024
3c2bb3e
get the task count for the navigation and also update param name
GeorgeGoodall Oct 31, 2024
336a89f
update nav item name and move
GeorgeGoodall Oct 31, 2024
4d62339
have table output real data
GeorgeGoodall Nov 1, 2024
bc0daba
add pagination
GeorgeGoodall Nov 4, 2024
218f05d
fix tests
GeorgeGoodall Nov 4, 2024
92985d7
Merge commit 'e927f0d2c0b4ab5c1814ff6b30507b4d456a4bfe' into 482-data…
GeorgeGoodall Nov 4, 2024
491af4e
moved middleware to common and added tests
GeorgeGoodall Nov 4, 2024
8680e16
move middleware to common
GeorgeGoodall Nov 4, 2024
89c2db1
remove duplicate middleware
GeorgeGoodall Nov 4, 2024
4f4219e
add tests for extract json fields from entities
GeorgeGoodall Nov 4, 2024
a8b8b0d
add tests for replace underscore in entities
GeorgeGoodall Nov 4, 2024
27a3c1f
move set default params into common, added tests for dataview
GeorgeGoodall Nov 4, 2024
fa6c9d7
have req.parsedParams always overwrite req.params even if values are …
GeorgeGoodall Nov 4, 2024
380c335
remove scripts block
GeorgeGoodall Nov 4, 2024
cf34d76
fix test
GeorgeGoodall Nov 4, 2024
46fb950
update template schema
GeorgeGoodall Nov 4, 2024
cd4876e
update test name
GeorgeGoodall Nov 4, 2024
3e965dd
improve tests
GeorgeGoodall Nov 4, 2024
9647ed5
remove duplicate tests
GeorgeGoodall Nov 4, 2024
8be9b1d
ensure issues is defined
GeorgeGoodall Nov 4, 2024
d084deb
Update src/middleware/dataview.middleware.js
GeorgeGoodall-GovUk Nov 4, 2024
70766a3
Update src/middleware/dataview.middleware.js
GeorgeGoodall-GovUk Nov 4, 2024
2c8945e
Merge commit '70766a37b284f4df03df6ce5f2bb48f0614688c8' into 482-data…
GeorgeGoodall Nov 4, 2024
ab0cdbb
Update src/middleware/common.middleware.js
GeorgeGoodall-GovUk Nov 4, 2024
fea553e
Merge commit 'ab0cdbb9b4fe4c4e90e661a40f7356340eb661b3' into 482-data…
GeorgeGoodall Nov 4, 2024
282e811
Update src/middleware/dataview.middleware.js
GeorgeGoodall-GovUk Nov 4, 2024
306c991
avoid potential overwrite of entity keys
GeorgeGoodall Nov 4, 2024
2a19718
linting
GeorgeGoodall Nov 4, 2024
7007128
Merge branch '482-dataset-details-page-provision-level-view-table' of…
GeorgeGoodall Nov 4, 2024
cbc4bfd
Update src/middleware/common.middleware.js
GeorgeGoodall-GovUk Nov 4, 2024
f12a941
add check to pagination method
GeorgeGoodall Nov 4, 2024
f159cf8
added more tests
GeorgeGoodall Nov 4, 2024
e3a939c
Merge branch '482-dataset-details-page-provision-level-view-table' of…
GeorgeGoodall Nov 4, 2024
42bd900
add check on extractJsonFieldsFromEntities
GeorgeGoodall Nov 4, 2024
4133172
improved tests around the get max page middleware
GeorgeGoodall Nov 4, 2024
5444224
Revert "Update src/middleware/dataview.middleware.js"
GeorgeGoodall Nov 4, 2024
35ecef7
Revert "Update src/middleware/dataview.middleware.js"
GeorgeGoodall Nov 4, 2024
2124549
Revert "Update src/middleware/dataview.middleware.js"
GeorgeGoodall Nov 4, 2024
7002824
have view data after tasklist in navigation
GeorgeGoodall Nov 5, 2024
bb5fe11
update table schema
GeorgeGoodall Nov 5, 2024
cf548f0
have numbers and links format differently
GeorgeGoodall Nov 5, 2024
2c7c985
update title
GeorgeGoodall Nov 5, 2024
d5b7cd2
update tab text
GeorgeGoodall Nov 5, 2024
bdd1ff2
update other tab name
GeorgeGoodall Nov 5, 2024
2352459
wrap table in grid column full
GeorgeGoodall Nov 5, 2024
d58f091
Revert "have view data after tasklist in navigation"
GeorgeGoodall Nov 5, 2024
4859171
fix bugs around page number
GeorgeGoodall Nov 5, 2024
e3bfe59
remove how to fix block
GeorgeGoodall Nov 5, 2024
7aeee9c
have line output indicating data range
GeorgeGoodall Nov 5, 2024
0161b78
fix tests
GeorgeGoodall Nov 6, 2024
f93321e
add some tests for set offset
GeorgeGoodall Nov 6, 2024
0705d51
use safe Number.isNaN
GeorgeGoodall Nov 6, 2024
1880b92
make sure to use math.ceil for getting pageNumber
GeorgeGoodall Nov 6, 2024
3693703
have the column headers as the unique field names
GeorgeGoodall Nov 6, 2024
5724382
improve link detection to not consider substrings
GeorgeGoodall Nov 6, 2024
0e419e1
Update src/middleware/dataview.middleware.js
GeorgeGoodall-GovUk Nov 6, 2024
87a2064
Update src/middleware/dataview.middleware.js
GeorgeGoodall-GovUk Nov 6, 2024
4d8f856
Merge branch 'main' into 482-dataset-details-page-provision-level-vie…
GeorgeGoodall-GovUk Nov 6, 2024
3987f00
linting
GeorgeGoodall Nov 6, 2024
f08845e
fix tests
GeorgeGoodall Nov 7, 2024
2c7ec20
Merge branch 'main' into 482-dataset-details-page-provision-level-vie…
GeorgeGoodall-GovUk Nov 7, 2024
281dad8
Merge commit 'a89489aa3fc65e5746306a64113fe2456d0c9edb' into 482-data…
GeorgeGoodall Nov 8, 2024
2835a03
Merge commit '2e993a763e3c9c80db42be0955ed682c34417f89' into 482-data…
GeorgeGoodall Nov 11, 2024
a2207e5
make column titles in the data view the same as the raw specification…
GeorgeGoodall Nov 11, 2024
dee5f70
make sure check column names are still prettified
GeorgeGoodall Nov 11, 2024
6df2a22
fix tests
GeorgeGoodall Nov 11, 2024
165fcf2
fix tests
GeorgeGoodall Nov 11, 2024
fc6013d
Merge remote-tracking branch 'origin/main' into 482-dataset-details-p…
GeorgeGoodall Nov 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/controllers/OrganisationsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ import getIssueDetailsMiddleware from '../middleware/issueDetails.middleware.js'
import getOrganisationsMiddleware from '../middleware/organisations.middleware.js'
import getGetStartedMiddleware from '../middleware/getStarted.middleware.js'
import getOverviewMiddleware from '../middleware/overview.middleware.js'
import getDatasetDataviewMiddleware from '../middleware/dataview.middleware.js'

const organisationsController = {
getOrganisationsMiddleware,
getDatasetTaskListMiddleware,
getDatasetOverviewMiddleware,
getIssueDetailsMiddleware,
getGetStartedMiddleware,
getOverviewMiddleware
getOverviewMiddleware,
getDatasetDataviewMiddleware
}

export default organisationsController
3 changes: 2 additions & 1 deletion src/controllers/resultsController.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import PageController from './pageController.js'
import { getRequestData } from '../services/asyncRequestApi.js'
import prettifyColumnName from '../filters/prettifyColumnName.js'

const failedFileRequestTemplate = 'results/failedFileRequest'
const failedUrlRequestTemplate = 'results/failedUrlRequest'
Expand Down Expand Up @@ -59,7 +60,7 @@ class ResultsController extends PageController {
})

req.form.options.tableParams = {
columns: responseDetails.getColumns(),
columns: responseDetails.getColumns().map(column => prettifyColumnName(column)),
rows,
fields: responseDetails.getFields()
}
Expand Down
201 changes: 201 additions & 0 deletions src/middleware/common.middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { types } from '../utils/logging.js'
import performanceDbApi from '../services/performanceDbApi.js'
import { fetchOne, FetchOptions, FetchOneFallbackPolicy, fetchMany, renderTemplate } from './middleware.builders.js'
import * as v from 'valibot'
import { pagination } from '../utils/pagination.js'

/**
* Middleware. Set `req.handlerName` to a string that will identify
Expand Down Expand Up @@ -112,3 +113,203 @@ export const getDatasetTaskListError = renderTemplate({
template: 'organisations/http-error.html',
handlerName: 'getDatasetTaskListError'
})

export const getIsPageNumberInRange = (maxPagesKey) => {
/**
* Middleware. Short-circuits with 404 error if pageNumber is not in range.
* Updates req with `pageNumber`
*
* @param req
* @param res
* @param next
*/
return (req, res, next) => {
const { pageNumber } = req.parsedParams
if (!Number.isInteger(pageNumber)) {
return next(new Error('Page number not a number'))
}
if (pageNumber < 1 || req[maxPagesKey] < pageNumber) {
const error = new Error('Page not found')
error.status = 404
return next(error)
}
next()
}
}

/**
* Creates pagination template parameters for the request.
*
* @param {Object} req - The request object.
* @param {Object} res - The response object.
* @param {Function} next - The next middleware function in the chain.
*
* @description
* This middleware function extracts pagination-related parameters from the request,
* calculates the total number of pages, and creates a pagination object that can be used
* to render pagination links in the template.
*
* @returns {void}
*/
export const createPaginationTemplateParams = (req, res, next) => {
const { resultsCount, urlSubPath, paginationPageLength } = req
if (typeof resultsCount !== 'number' || typeof paginationPageLength !== 'number' || !urlSubPath) {
logger.error('Missing or invalid pagination parameters', { resultsCount, urlSubPath, paginationPageLength })
return next(new Error('Invalid pagination parameters'))
}
let { pageNumber } = req.params
pageNumber = parseInt(pageNumber)

if (Number.isNaN(pageNumber) || pageNumber <= 0) {
throw new Error('Invalid page number')
}

if (resultsCount <= 0) {
return next()
}

const totalPages = Math.ceil(resultsCount / paginationPageLength)

const paginationObj = {}
if (pageNumber > 1) {
paginationObj.previous = {
href: `${urlSubPath}${pageNumber - 1}`
}
}

if (pageNumber < totalPages) {
paginationObj.next = {
href: `${urlSubPath}${pageNumber + 1}`
}
}

paginationObj.items = pagination(totalPages, pageNumber).map(item => {
if (item === '...') {
return {
type: 'ellipsis',
ellipsis: true,
href: '#'
}
} else {
return {
type: 'number',
number: item,
href: `${urlSubPath}${item}`,
current: pageNumber === parseInt(item)
}
}
})

req.pagination = paginationObj

next()
}

export const addDatabaseFieldToSpecification = (req, res, next) => {
const { specification, fieldMappings } = req

req.specification.fields = specification.fields.flatMap(fieldObj => {
if (['GeoX', 'GeoY'].includes(fieldObj.field)) { // special case for brownfield land
return { datasetField: 'point', ...fieldObj }
}

const fieldMappingsForField = fieldMappings.filter(mapping => mapping.field === fieldObj.field)

const datasetFields = fieldMappingsForField.map(mapping => mapping.replacement_field).filter(Boolean)

if (datasetFields.length === 0) {
// no dataset fields found, add the field anyway with datasetField set to the same value as fieldObj.field
return { datasetField: fieldObj.field, ...fieldObj }
}

// sometimes a field maps to more than one dataset field, so we need to account for that
const specificationEntriesWithDatasetFields = datasetFields.map(datasetField => ({ datasetField, ...fieldObj }))
return specificationEntriesWithDatasetFields
})

next()
}
Comment on lines +208 to +231
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add input validation and enhance field mapping flexibility

The function lacks validation for required properties and has hardcoded field names. Consider:

  1. Validating input properties
  2. Making the special case handling more configurable
 export const addDatabaseFieldToSpecification = (req, res, next) => {
   const { specification, fieldMappings } = req
+  if (!specification?.fields || !Array.isArray(fieldMappings)) {
+    return next(new Error('Missing required properties'))
+  }
+
+  const SPECIAL_FIELD_MAPPINGS = {
+    GeoX: 'point',
+    GeoY: 'point'
+  }

   req.specification.fields = specification.fields.flatMap(fieldObj => {
-    if (['GeoX', 'GeoY'].includes(fieldObj.field)) {
-      return { datasetField: 'point', ...fieldObj }
+    if (fieldObj.field in SPECIAL_FIELD_MAPPINGS) {
+      return { datasetField: SPECIAL_FIELD_MAPPINGS[fieldObj.field], ...fieldObj }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const addDatabaseFieldToSpecification = (req, res, next) => {
const { specification, fieldMappings } = req
req.specification.fields = specification.fields.flatMap(fieldObj => {
if (['GeoX', 'GeoY'].includes(fieldObj.field)) { // special case for brownfield land
return { datasetField: 'point', ...fieldObj }
}
const fieldMappingsForField = fieldMappings.filter(mapping => mapping.field === fieldObj.field)
const datasetFields = fieldMappingsForField.map(mapping => mapping.replacement_field).filter(Boolean)
if (datasetFields.length === 0) {
// no dataset fields found, add the field anyway with datasetField set to the same value as fieldObj.field
return { datasetField: fieldObj.field, ...fieldObj }
}
// sometimes a field maps to more than one dataset field, so we need to account for that
const specificationEntriesWithDatasetFields = datasetFields.map(datasetField => ({ datasetField, ...fieldObj }))
return specificationEntriesWithDatasetFields
})
next()
}
export const addDatabaseFieldToSpecification = (req, res, next) => {
const { specification, fieldMappings } = req
if (!specification?.fields || !Array.isArray(fieldMappings)) {
return next(new Error('Missing required properties'))
}
const SPECIAL_FIELD_MAPPINGS = {
GeoX: 'point',
GeoY: 'point'
}
req.specification.fields = specification.fields.flatMap(fieldObj => {
if (fieldObj.field in SPECIAL_FIELD_MAPPINGS) {
return { datasetField: SPECIAL_FIELD_MAPPINGS[fieldObj.field], ...fieldObj }
}
const fieldMappingsForField = fieldMappings.filter(mapping => mapping.field === fieldObj.field)
const datasetFields = fieldMappingsForField.map(mapping => mapping.replacement_field).filter(Boolean)
if (datasetFields.length === 0) {
// no dataset fields found, add the field anyway with datasetField set to the same value as fieldObj.field
return { datasetField: fieldObj.field, ...fieldObj }
}
// sometimes a field maps to more than one dataset field, so we need to account for that
const specificationEntriesWithDatasetFields = datasetFields.map(datasetField => ({ datasetField, ...fieldObj }))
return specificationEntriesWithDatasetFields
})
next()
}


export const replaceUnderscoreInSpecification = (req, res, next) => {
req.specification.fields = req.specification.fields.map((spec) => {
if (spec.datasetField) {
spec.datasetField = spec.datasetField.replace(/_/g, '-')
}
return spec
})

next()
}

export const pullOutDatasetSpecification = (req, res, next) => {
const { specification } = req
let collectionSpecifications
try {
collectionSpecifications = JSON.parse(specification.json)
} catch (error) {
logger.error('Invalid JSON in specification.json', { error })
return next(new Error('Invalid specification format'))
}
const datasetSpecification = collectionSpecifications.find((spec) => spec.dataset === req.dataset.dataset)
if (!datasetSpecification) {
logger.error('Dataset specification not found', { dataset: req.dataset.dataset })
return next(new Error('Dataset specification not found'))
}
req.specification = datasetSpecification
next()
}

export const extractJsonFieldFromEntities = (req, res, next) => {
const { entities } = req
if (!Array.isArray(entities)) {
logger.error('Invalid entities array', { entities })
return next(new Error('Invalid entities format'))
}

req.entities = entities.map(entity => {
const jsonField = entity.json
if (!jsonField || jsonField === '') {
logger.info(`common.middleware/extractJsonField: No json field for entity ${entity.toString()}`)
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}`)
}
return entity
})

next()
}

export const replaceUnderscoreInEntities = (req, res, next) => {
if (!req.entities) {
next()
return
}

req.entities = req.entities.map((entity) => {
return Object.keys(entity).reduce((acc, key) => {
const newKey = key.replace(/_/g, '-')
acc[newKey] = entity[key]
return acc
}, {})
})

next()
}

export const setDefaultParams = (req, res, next) => {
if (!req.parsedParams) {
return next()
}

Object.keys(req.parsedParams).forEach((key) => {
req.params[key] = req.parsedParams[key]
})

next()
}
GeorgeGoodall-GovUk marked this conversation as resolved.
Show resolved Hide resolved
31 changes: 4 additions & 27 deletions src/middleware/datasetOverview.middleware.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { fetchDatasetInfo, fetchLatestResource, fetchLpaDatasetIssues, fetchOrgInfo, getDatasetTaskListError, isResourceAccessible, isResourceIdInParams, isResourceNotAccessible, logPageError, takeResourceIdFromParams } from './common.middleware.js'
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 performanceDbApi from '../services/performanceDbApi.js'
import logger from '../utils/logger.js'
import { types } from '../utils/logging.js'

const fetchColumnSummary = fetchMany({
query: ({ params }) => `
Expand Down Expand Up @@ -38,27 +36,6 @@ 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
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()
}

const fetchSources = fetchMany({
query: ({ params }) => `
WITH RankedEndpoints AS (
Expand Down Expand Up @@ -120,13 +97,13 @@ const fetchEntityCount = fetchOne({
})

export const prepareDatasetOverviewTemplateParams = (req, res, next) => {
const { orgInfo, datasetSpecification, columnSummary, entityCount, sources, dataset, issues } = req
const { orgInfo, specification, 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 specFields = datasetSpecification ? datasetSpecification.fields : []
const specFields = specification ? specification.fields : []
const numberOfFieldsSupplied = specFields.reduce((acc, field) => {
return allFields.includes(field.field) ? acc + 1 : acc
}, 0)
Expand Down Expand Up @@ -164,7 +141,7 @@ export const prepareDatasetOverviewTemplateParams = (req, res, next) => {
req.templateParams = {
organisation: orgInfo,
dataset,
issueCount: issues.length ?? 0,
taskCount: issues.length ?? 0,
stats: {
numberOfFieldsSupplied: numberOfFieldsSupplied ?? 0,
numberOfFieldsMatched: numberOfFieldsMatched ?? 0,
Expand Down
Loading
Loading