From d75ea8741ef34cc2b82030f4aa77d1ce4d2e963e Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 17 Sep 2024 14:21:14 +0100 Subject: [PATCH 01/65] add method to get specifications --- package.json | 2 ++ src/services/DatasetService.js | 61 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/package.json b/package.json index ed0bf9a8..e9f8d381 100644 --- a/package.json +++ b/package.json @@ -76,11 +76,13 @@ "hmpo-form-wizard": "^13.0.0", "hmpo-i18n": "^6.0.1", "js-yaml": "^4.1.0", + "json5": "^2.2.3", "lodash": "^4.17.21", "maplibre-gl": "^4.1.0", "multer": "^1.4.5-lts.1", "notifications-node-client": "^8.2.0", "nunjucks": "^3.2.4", + "papaparse": "^5.4.1", "redis": "^4.6.13", "sass": "^1.69.4", "uuid": "^9.0.1", diff --git a/src/services/DatasetService.js b/src/services/DatasetService.js index 6a1f9323..856c5a03 100644 --- a/src/services/DatasetService.js +++ b/src/services/DatasetService.js @@ -1,6 +1,8 @@ import datasette from './datasette.js' import logger from '../utils/logger.js' import performanceDbApi from './performanceDbApi.js' +import Papa from 'papaparse' +import JSON5 from 'json5' // need to use this as spec is written with single quotes. need to pass this onto data designers team to fix export async function getGeometryEntriesForResourceId (dataset, resourceId) { const sql = ` @@ -33,6 +35,20 @@ export async function getLatestDatasetGeometryEntriesForLpa (dataset, lpa) { } } +/* + Needs to get: + - Number of records + - Number of fields supplied + - Number of fields matched + - Licence + - source documentation url + - for each data url + - the endpoint + - last accessed + - last updated + - any access errors + +*/ async function getDatasetStatsForResourceId (dataset, resourceId) { const sql = ` SELECT 'numberOfRecords' AS metric, COUNT(*) AS value @@ -64,10 +80,24 @@ async function getDatasetStatsForResourceId (dataset, resourceId) { return formattedData } +// async function getColumnSummary (dataset, lpa) { +// const sql = `select * from column_field_summary +// where resource != '' +// and pipeline = '${dataset}' +// AND organisation = '${lpa}' +// limit 1000` + +// const { formattedData } = await datasette.runQuery(sql, 'performance') + +// return formattedData +// } + export async function getDatasetStats (dataset, lpa) { try { const stats = {} const { resource: resourceId } = await performanceDbApi.getLatestResource(lpa, dataset) + // const columnSummary = await getColumnSummary(dataset, lpa) + const metrics = await getDatasetStatsForResourceId(dataset, resourceId) metrics.forEach(({ metric, value }) => { @@ -87,3 +117,34 @@ export async function getDatasetStats (dataset, lpa) { return {} } } + +export async function getSpecifications () { + try { + const response = await fetch('https://raw.githubusercontent.com/digital-land/specification/main/specification/specification.csv') + + const csvData = await response.text() + + const initalParse = Papa.parse(csvData, { header: true }) + + const fullyParsed = initalParse.data.filter(dataset => dataset.datasets !== '').map((dataset) => { + const { json, ...rest } = dataset + const formattedJson = JSON5.parse(json) + return { + ...rest, + json: formattedJson + } + }) + + const specifications = fullyParsed.reduce((accumulator, current) => { + const datasets = current.datasets.split(';') + datasets.forEach((dataset, index) => { + accumulator[dataset] = current.json[index] + }) + return accumulator + }, {}) + + return specifications + } catch (error) { + console.error(error) + } +} From f4138123d08fb1e550d8706093c0e2acfbbd5df7 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 17 Sep 2024 17:34:49 +0100 Subject: [PATCH 02/65] update package lock --- package-lock.json | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index ba9d8aa6..cb91bb1e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29,11 +29,13 @@ "hmpo-form-wizard": "^13.0.0", "hmpo-i18n": "^6.0.1", "js-yaml": "^4.1.0", + "json5": "^2.2.3", "lodash": "^4.17.21", "maplibre-gl": "^4.1.0", "multer": "^1.4.5-lts.1", "notifications-node-client": "^8.2.0", "nunjucks": "^3.2.4", + "papaparse": "^5.4.1", "redis": "^4.6.13", "sass": "^1.69.4", "uuid": "^9.0.1", @@ -8742,7 +8744,8 @@ }, "node_modules/json5": { "version": "2.2.3", - "license": "MIT", + "resolved": "https://registry.npmjs.org/json5/-/json5-2.2.3.tgz", + "integrity": "sha512-XmOWe7eyHYH14cLdVPoyg+GOH3rYX++KpzrylJwSW98t3Nk+U8XOl8FWKOgwtzdb8lXGf6zYwDUzeHMWfxasyg==", "bin": { "json5": "lib/cli.js" }, @@ -9947,6 +9950,11 @@ "version": "1.0.0", "license": "BlueOak-1.0.0" }, + "node_modules/papaparse": { + "version": "5.4.1", + "resolved": "https://registry.npmjs.org/papaparse/-/papaparse-5.4.1.tgz", + "integrity": "sha512-HipMsgJkZu8br23pW15uvo6sib6wne/4woLZPlFf3rpDyMe9ywEXUsuD7+6K9PRkJlVT51j/sCOYDKGGS3ZJrw==" + }, "node_modules/parent-module": { "version": "1.0.1", "dev": true, From d0ffb92169b5fc1c3dc443d06c20162b0b7b450c Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 17 Sep 2024 17:35:55 +0100 Subject: [PATCH 03/65] now get and display endpoints, and correct number of fields and matched --- src/assets/scss/index.scss | 8 ++ src/controllers/OrganisationsController.js | 54 +++++++- src/routes/schemas.js | 14 +- src/services/DatasetService.js | 108 ++++++---------- src/services/performanceDbApi.js | 23 +++- src/views/organisations/dataset-overview.html | 121 ++++++++++++++---- test/unit/services/DatasetService.test.js | 9 +- 7 files changed, 231 insertions(+), 106 deletions(-) diff --git a/src/assets/scss/index.scss b/src/assets/scss/index.scss index 3e1b6a16..07219dca 100644 --- a/src/assets/scss/index.scss +++ b/src/assets/scss/index.scss @@ -107,4 +107,12 @@ $govuk-global-styles: true; code, code * { font-family: monospace; +} + +.padding-top { + padding-top: 20px !important; +} + +.light-bottom-border { + border-bottom: 1px solid #e0e1e2ce !important; } \ No newline at end of file diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index acc931e2..969bd3ca 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -9,7 +9,7 @@ import { templateSchema } from '../routes/schemas.js' import * as v from 'valibot' import { pagination } from '../utils/pagination.js' import config from '../../config/index.js' -import { getDatasetStats, getLatestDatasetGeometryEntriesForLpa } from '../services/DatasetService.js' +import { getFieldStats, getLatestDatasetGeometryEntriesForLpa, getSources } from '../services/DatasetService.js' // get a list of available datasets const availableDatasets = Object.values(dataSubjects) @@ -131,7 +131,47 @@ const fetchDatasetGeometries = async (req, res, next) => { } const fetchDatasetStats = async (req, res, next) => { - req.stats = await getDatasetStats(req.params.dataset, req.params.lpa) + const { dataset, lpa } = req.params + + const { numberOfFieldsSupplied, numberOfFieldsMatched, NumberOfExpectedFields } = await getFieldStats(lpa, dataset) + + const sources = await getSources(lpa, dataset) + + // 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 + const endpoints = sources.sort((a, b) => { + if (a.status >= 200 && a.status < 300) return -1 + if (b.status >= 200 && b.status < 300) return 1 + return 0 + }).map((source, index) => { + let error + + if (parseInt(source.status) <= 200 || parseInt(source.status) > 300) { + error = { + code: parseInt(source.status), + exception: source.exception + } + } + + return { + name: `Data Url ${index}`, + endpoint: source.endpoint_url, + lastAccessed: source.latest_log_entry_date, + lastUpdated: source.endpoint_entry_date, // not sure if this is the lastupdated + error + } + }) + + const numberOfRecords = 10 // ToDo: await performanceDbApi.getEntityCount(lpa, dataset) + + // ToDo: get the documentation url + + req.stats = { + numberOfFieldsSupplied, + numberOfFieldsMatched, + NumberOfExpectedFields, + numberOfRecords, + endpoints + } next() } @@ -139,7 +179,11 @@ const fetchDatasetStats = async (req, res, next) => { const getDatasetOverview = renderTemplate.bind( { templateParams (req) { - const { orgInfo: organisation, dataset, geometries, stats } = req + const { + orgInfo: organisation, + dataset, geometries, + stats + } = req return { organisation, dataset, geometries, stats } }, template: 'organisations/dataset-overview.html', @@ -262,7 +306,7 @@ async function fetchEntityCount (req, res, next) { const resourceId = passedResourceId ?? req.resourceId console.assert(resourceId, 'missng resourceId') - const entityCount = await performanceDbApi.getEntityCount(resourceId, datasetId) + const entityCount = await performanceDbApi.getEntityCountForResource(resourceId, datasetId) req.entityCount = entityCount next() } @@ -659,7 +703,7 @@ const organisationsController = { const issues = await performanceDbApi.getLpaDatasetIssues(resource.resource, datasetId) - const entityCount = await performanceDbApi.getEntityCount(resource.resource, datasetId) + const entityCount = await performanceDbApi.getEntityCountForResource(resource.resource, datasetId) const taskList = issues.map((issue) => { return { diff --git a/src/routes/schemas.js b/src/routes/schemas.js index a2b13e85..7b1e89b9 100644 --- a/src/routes/schemas.js +++ b/src/routes/schemas.js @@ -73,7 +73,19 @@ export const OrgDatasetOverview = v.strictObject({ geometries: v.array(v.string()), stats: v.strictObject({ numberOfRecords: v.integer(), - numberOfFieldsSupplied: v.integer() + numberOfFieldsSupplied: v.integer(), + numberOfFieldsMatched: v.integer(), + NumberOfExpectedFields: v.integer(), + endpoints: v.array(v.strictObject({ + name: v.string(), + endpoint: v.string(), + lastAccessed: v.string(), // ToDo: can we have date type on these? + lastUpdated: v.string(), + error: v.optional(v.strictObject({ + code: v.integer(), + exception: v.string() + })) + })) }) }) diff --git a/src/services/DatasetService.js b/src/services/DatasetService.js index 856c5a03..4a1001e7 100644 --- a/src/services/DatasetService.js +++ b/src/services/DatasetService.js @@ -35,86 +35,41 @@ export async function getLatestDatasetGeometryEntriesForLpa (dataset, lpa) { } } -/* - Needs to get: - - Number of records - - Number of fields supplied - - Number of fields matched - - Licence - - source documentation url - - for each data url - - the endpoint - - last accessed - - last updated - - any access errors - -*/ -async function getDatasetStatsForResourceId (dataset, resourceId) { - const sql = ` - SELECT 'numberOfRecords' AS metric, COUNT(*) AS value - FROM - ( - SELECT - * - FROM - fact_resource fr - WHERE - fr.resource = '${resourceId}' - GROUP BY - entry_number - ) - UNION ALL - SELECT 'numberOfFieldsSupplied' AS metric, COUNT(*) AS value - FROM - ( - SELECT - * - FROM - fact_resource fr - WHERE - fr.resource = '${resourceId}' - )` +async function getColumnSummary (dataset, lpa) { + const sql = `select * from column_field_summary + where resource != '' + and pipeline = '${dataset}' + AND organisation = '${lpa}' + limit 1000` - const { formattedData } = await datasette.runQuery(sql, dataset) + const { formattedData } = await datasette.runQuery(sql, 'performance') return formattedData } -// async function getColumnSummary (dataset, lpa) { -// const sql = `select * from column_field_summary -// where resource != '' -// and pipeline = '${dataset}' -// AND organisation = '${lpa}' -// limit 1000` - -// const { formattedData } = await datasette.runQuery(sql, 'performance') +export async function getFieldStats (lpa, dataset) { + const columnSummary = await getColumnSummary(dataset, lpa) + const specifications = await getSpecifications() + const datasetSpecification = specifications[dataset] -// return formattedData -// } - -export async function getDatasetStats (dataset, lpa) { - try { - const stats = {} - const { resource: resourceId } = await performanceDbApi.getLatestResource(lpa, dataset) - // const columnSummary = await getColumnSummary(dataset, lpa) + const matchingFields = columnSummary[0].matching_field.split(',') + const nonMatchingFields = columnSummary[0].non_matching_field.split(',') + const allFields = [...matchingFields, ...nonMatchingFields] - const metrics = await getDatasetStatsForResourceId(dataset, resourceId) + const numberOfFieldsSupplied = datasetSpecification.fields.map(field => field.field).reduce((acc, current) => { + return allFields.includes(current) ? acc + 1 : acc + }, 0) - metrics.forEach(({ metric, value }) => { - stats[metric] = value - }) + const numberOfFieldsMatched = datasetSpecification.fields.map(field => field.field).reduce((acc, current) => { + return matchingFields.includes(current) ? acc + 1 : acc + }, 0) - return stats - } catch (error) { - logger.warn( - `DatasetService.getDatasetStats(): Error getting dataset stats for ${lpa} in ${dataset}`, - { - errorMessage: error.message, - errorStack: error.stack - } - ) + const NumberOfExpectedFields = datasetSpecification.fields.length - return {} + return { + numberOfFieldsSupplied, + numberOfFieldsMatched, + NumberOfExpectedFields } } @@ -148,3 +103,16 @@ export async function getSpecifications () { console.error(error) } } + +export async function getSources (lpa, dataset) { + const sql = ` + select endpoint, endpoint_url, status, exception, resource, latest_log_entry_date, endpoint_entry_date, endpoint_end_date, resource_start_date, resource_end_date + from reporting_historic_endpoints + where REPLACE(organisation, '-eng', '') = '${lpa}' and pipeline = '${dataset}' + AND (resource_end_date >= current_timestamp OR resource_end_date is null) + ` + + const { formattedData } = await datasette.runQuery(sql, 'performance') + + return formattedData +} diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 0b914052..d830c7c2 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -394,7 +394,7 @@ ORDER BY * @param {string} dataset - The dataset to retrieve the entity count from. * @returns {number} The entity count for the given resource and dataset. */ - async getEntityCount (resource, dataset) { + async getEntityCountForResource (resource, dataset) { const query = /* sql */ ` @@ -405,6 +405,27 @@ ORDER BY const result = await datasette.runQuery(query, dataset) + return result.formattedData[0].entity_count + }, + + /** + * Retrieves the entity count for a given resource and dataset. + * + * @param {string} resource - The resource to retrieve the entity count for. + * @param {string} dataset - The dataset to retrieve the entity count from. + * @returns {number} The entity count for the given resource and dataset. + */ + async getEntityCount (lpa, dataset) { + const query = + /* sql */ + ` + select dataset, entity_count, resource + from dataset_resource + WHERE resource = '${lpa}' + ` + + const result = await datasette.runQuery(query, dataset) + return result.formattedData[0].entity_count } } diff --git a/src/views/organisations/dataset-overview.html b/src/views/organisations/dataset-overview.html index ee9ebf5c..c4d45db1 100644 --- a/src/views/organisations/dataset-overview.html +++ b/src/views/organisations/dataset-overview.html @@ -33,6 +33,93 @@ {% endblock %} +{% set mainRows = [ + { + key: { + text: "Number of records" + }, + value: { + text: stats.numberOfRecords | default(0) + } + }, + { + key: { + text: "Number of fields supplied" + }, + value: { + text: stats.numberOfFieldsSupplied + '/' + stats.NumberOfExpectedFields | default(0) + } + }, + { + key: { + text: "Number of fields matched" + }, + value: { + text: stats.numberOfFieldsMatched + '/' + stats.NumberOfExpectedFields | default(0) + } + }, + { + key: { + text: "License" + }, + value: { + text: "To Do" + } + } +] %} + +{% set endpointRows = []%} + +{% for endpoint in stats.endpoints %} + {% set endpointRow = { + key: { + text: endpoint.name + }, + value: { + text: endpoint.endpoint + }, + classes: 'padding-top' + } %} + {{ endpointRows.push(endpointRow) }} + + + + {% if endpoint.error %} + {% set lastAccessedRow = { + key: { + text: 'Data URL last accessed' + }, + value: { + html: endpoint.lastAccessed + '

There was a '+endpoint.error.code+' error accessing the data URL

' + }, + classes: 'app-inset-text---error' + } %} + {% else %} + {% set lastAccessedRow = { + key: { + text: 'Data URL last accessed' + }, + value: { + text: endpoint.lastAccessed + } + } %} + {% endif %} + + + {{ endpointRows.push(lastAccessedRow) }} + + {% set lastUpdatedRow = { + key: { + text: 'Data URL last updated' + }, + value: { + text: endpoint.lastUpdated + } + } %} + {{ endpointRows.push(lastUpdatedRow) }} + +{% endfor %} + {% block content %}
@@ -57,34 +144,12 @@

Map of dataset

Dataset details

- {{ govukSummaryList({ - rows: [ - { - key: { - text: "Number of records" - }, - value: { - text: stats.numberOfRecords | default(0) - } - }, - { - key: { - text: "Number of fields supplied" - }, - value: { - text: stats.numberOfFieldsSupplied | default(0) - } - }, - { - key: { - text: "License" - }, - value: { - text: "Open Government Licence" - } - } - ] - }) }} + {{ govukSummaryList({ rows: mainRows }) }} + + +

Endpoints

+ + {{ govukSummaryList({ rows: endpointRows }) }}
diff --git a/test/unit/services/DatasetService.test.js b/test/unit/services/DatasetService.test.js index 1bfedd4e..9592ff19 100644 --- a/test/unit/services/DatasetService.test.js +++ b/test/unit/services/DatasetService.test.js @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest' -import { getGeometryEntriesForResourceId, getDatasetStats } from '../../../src/services/DatasetService.js' +import { getGeometryEntriesForResourceId, getDatasetStats, getSpecifications } from '../../../src/services/DatasetService.js' import datasette from '../../../src/services/datasette' vi.mock('../../../src/services/datasette') @@ -48,4 +48,11 @@ describe('DatasetService', () => { expect(result).toEqual({}) }) }) + + describe('getSpecifications', async () => { + it('can get specs', async () => { + const specifications = await getSpecifications() + expect(specifications).toBeDefined() + }) + }) }) From 9ef41de6ad68d15945202a280874bab6c54158b5 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 17 Sep 2024 17:50:28 +0100 Subject: [PATCH 04/65] number of records now correctly output --- src/controllers/OrganisationsController.js | 5 +++-- src/routes/schemas.js | 2 +- src/services/performanceDbApi.js | 8 ++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 969bd3ca..04d90f87 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -104,7 +104,7 @@ const getGetStarted = renderTemplate.bind( const fetchOrgInfo = fetchOne.bind({ query: ({ params }) => { - return `SELECT name, organisation FROM organisation WHERE organisation = '${params.lpa}'` + return `SELECT name, organisation, entity FROM organisation WHERE organisation = '${params.lpa}'` }, result: 'orgInfo' }) @@ -132,6 +132,7 @@ const fetchDatasetGeometries = async (req, res, next) => { const fetchDatasetStats = async (req, res, next) => { const { dataset, lpa } = req.params + const { orgInfo: organisation } = req const { numberOfFieldsSupplied, numberOfFieldsMatched, NumberOfExpectedFields } = await getFieldStats(lpa, dataset) @@ -161,7 +162,7 @@ const fetchDatasetStats = async (req, res, next) => { } }) - const numberOfRecords = 10 // ToDo: await performanceDbApi.getEntityCount(lpa, dataset) + const numberOfRecords = await performanceDbApi.getEntityCount(organisation.entity, dataset) // ToDo: get the documentation url diff --git a/src/routes/schemas.js b/src/routes/schemas.js index 7b1e89b9..73f743a4 100644 --- a/src/routes/schemas.js +++ b/src/routes/schemas.js @@ -38,7 +38,7 @@ const datasetStatusEnum = { 'Not submitted': 'Not submitted' } -const OrgField = v.strictObject({ name: NonEmptyString, organisation: NonEmptyString, statistical_geography: v.optional(NonEmptyString) }) +const OrgField = v.strictObject({ name: NonEmptyString, organisation: NonEmptyString, statistical_geography: v.optional(NonEmptyString), entity: v.optional(v.integer()) }) const DatasetNameField = v.strictObject({ name: NonEmptyString }) export const OrgOverviewPage = v.strictObject({ diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index d830c7c2..c78e9c81 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -415,13 +415,13 @@ ORDER BY * @param {string} dataset - The dataset to retrieve the entity count from. * @returns {number} The entity count for the given resource and dataset. */ - async getEntityCount (lpa, dataset) { + async getEntityCount (orgEntity, dataset) { const query = /* sql */ ` - select dataset, entity_count, resource - from dataset_resource - WHERE resource = '${lpa}' + select count(entity) as entity_count + from entity + WHERE organisation_entity = '${orgEntity}' ` const result = await datasette.runQuery(query, dataset) From f4750f268068c270dae7451eb32a7ead313dc036 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 17 Sep 2024 17:53:05 +0100 Subject: [PATCH 05/65] for now just have static Open Government Licence --- src/views/organisations/dataset-overview.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/views/organisations/dataset-overview.html b/src/views/organisations/dataset-overview.html index c4d45db1..54f66cc5 100644 --- a/src/views/organisations/dataset-overview.html +++ b/src/views/organisations/dataset-overview.html @@ -63,7 +63,7 @@ text: "License" }, value: { - text: "To Do" + text: "Open Government Licence" } } ] %} From 5343714a1a07c8a5464a80efbbbcd68e61ca42c7 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Thu, 19 Sep 2024 09:41:55 +0100 Subject: [PATCH 06/65] entityMerge remote-tracking branch 'origin/main' into 407-dataset-details-summary-table --- src/assets/js/map.js | 237 ++++++++++++++---- src/controllers/OrganisationsController.js | 21 +- src/routes/schemas.js | 6 +- src/views/organisations/dataset-overview.html | 18 +- src/views/organisations/overview.html | 2 +- test/unit/services/DatasetService.test.js | 23 +- .../organisations/dataset-overview.test.js | 13 +- 7 files changed, 224 insertions(+), 96 deletions(-) diff --git a/src/assets/js/map.js b/src/assets/js/map.js index 09a88ce0..e3363cc0 100644 --- a/src/assets/js/map.js +++ b/src/assets/js/map.js @@ -1,22 +1,66 @@ import parse from 'wellknown' import maplibregl from 'maplibre-gl' +const fillColor = '#008' +const lineColor = '#000000' +const boundaryLineColor = '#000000' +const opacity = 0.4 + +/** + * Creates a Map instance. + * @param {MapOptions} opts - The options for creating the map. + * @constructor + */ +/** + * Options for creating a Map instance. + * @typedef {Object} MapOptions + * @property {string} containerId - Required - The ID of the HTML container element for the map. + * @property {string[]} data - Required - An array of URLs or WKT geometries to be added to the map. + * @property {string} [boundaryGeoJsonUrl] - Optional - The URL of the boundary GeoJSON to be added to the map. + * @property {boolean} [interactive] - Optional - Indicates whether the map should be interactive. Default is true. + * @property {boolean} [wktFormat] - Optional - Indicates whether the data is in WKT format. Default is false. + * @property {number[]} [boundingBox] - Optional - The bounding box coordinates [minX, minY, maxX, maxY] to set the initial view of the map. + */ class Map { - constructor (containerId, geometries, interactive = true) { + constructor (opts) { + this.bbox = opts.boundingBox ?? null this.map = new maplibregl.Map({ - container: containerId, + container: opts.containerId, style: 'https://api.maptiler.com/maps/basic-v2/style.json?key=ncAXR9XEn7JgHBLguAUw', zoom: 11, center: [-0.1298779, 51.4959698], - interactive + interactive: opts.interactive ?? true }) this.map.on('load', () => { - this.addDataToMap(geometries) + // Store the first symbol layer id + this.setFirstMapLayerId() + + // Add the boundary GeoJSON to the map + if (opts.boundaryGeoJsonUrl) this.addBoundaryGeoJsonToMap(opts.boundaryGeoJsonUrl) + + // Add layer data to map + if (opts.wktFormat) this.addWktDataToMap(opts.data) + else this.addGeoJsonUrlsToMap(opts.data) + + // Move the map to the bounding box + if (this.bbox) this.setMapViewToBoundingBox() }) } - addDataToMap (geometriesWkt) { + setFirstMapLayerId () { + const layers = this.map.getStyle().layers + + // Find the index of the first symbol layer in the map style + for (let i = 0; i < layers.length; i++) { + if (layers[i].type === 'symbol') { + this.firstMapLayerId = layers[i].id + break + } + } + } + + addWktDataToMap (geometriesWkt) { const geometries = [] geometriesWkt.forEach((geometryWkt, index) => { const name = `geometry-${index}` @@ -31,10 +75,6 @@ class Map { data: geometry }) - const color = '#008' - const lineColor = '#000000' - const opacity = 0.4 - // Add a layer to the map based on the geometry type if (geometry.type === 'Polygon' || geometry.type === 'MultiPolygon') { this.map.addLayer({ @@ -43,10 +83,10 @@ class Map { source: name, layout: {}, paint: { - 'fill-color': color, + 'fill-color': fillColor, 'fill-opacity': opacity } - }) + }, this.firstMapLayerId) this.map.addLayer({ id: name + '-border', @@ -57,7 +97,7 @@ class Map { 'line-color': lineColor, 'line-width': 1 } - }) + }, this.firstMapLayerId) } else if (geometry.type === 'Point' || geometry.type === 'MultiPoint') { this.map.addLayer({ id: name, @@ -65,15 +105,65 @@ class Map { source: name, paint: { 'circle-radius': 10, - 'circle-color': color, + 'circle-color': fillColor, 'circle-opacity': opacity } - }) + }, this.firstMapLayerId) } }) this.bbox = this.calculateBoundingBoxFromGeometries(geometries.map(g => g.coordinates)) - this.setMapViewToBoundingBox() + } + + addGeoJsonUrlsToMap (geoJsonUrls) { + geoJsonUrls.forEach(async (url, index) => { + const name = `geometry-${index}` + this.map.addSource(name, { + type: 'geojson', + data: url + }) + + this.map.addLayer({ + id: name, + type: 'fill', + source: name, + layout: {}, + paint: { + 'fill-color': fillColor, + 'fill-opacity': opacity + } + }, this.firstMapLayerId) + + this.map.addLayer({ + id: `${name}-border`, + type: 'line', + source: name, + layout: {}, + paint: { + 'line-color': lineColor, + 'line-width': 1 + } + }, this.firstMapLayerId) + }) + } + + addBoundaryGeoJsonToMap (geoJsonUrl) { + this.map.addSource('boundary', { + type: 'geojson', + data: geoJsonUrl + }) + + this.map.addLayer({ + id: 'boundary', + type: 'line', + source: 'boundary', + layout: {}, + paint: { + 'line-color': boundaryLineColor, + 'line-width': 2, + 'line-opacity': opacity + } + }, this.firstMapLayerId) } setMapViewToBoundingBox () { @@ -86,51 +176,106 @@ class Map { zoom: 11 }) } +} - calculateBoundingBoxFromGeometries (geometries) { - let minX = Infinity - let minY = Infinity - let maxX = -Infinity - let maxY = -Infinity - - const pullOutCoordinates = (geometry) => { - if (Array.isArray(geometry[0])) { - geometry.forEach(pullOutCoordinates) - } else { - const [x, y] = geometry - - // if x or y isn't a valid number log an error and continue - if (isNaN(x) || isNaN(y)) { - console.error('Invalid coordinates', x, y) - return - } +const calculateBoundingBoxFromGeometries = (geometries) => { + let minX = Infinity + let minY = Infinity + let maxX = -Infinity + let maxY = -Infinity - minX = Math.min(minX, x) - minY = Math.min(minY, y) - maxX = Math.max(maxX, x) - maxY = Math.max(maxY, y) + const pullOutCoordinates = (geometry) => { + if (Array.isArray(geometry[0])) { + geometry.forEach(pullOutCoordinates) + } else { + const [x, y] = geometry + + // if x or y isn't a valid number log an error and continue + if (isNaN(x) || isNaN(y)) { + console.error('Invalid coordinates', x, y) + return } + + minX = Math.min(minX, x) + minY = Math.min(minY, y) + maxX = Math.max(maxX, x) + maxY = Math.max(maxY, y) } + } + + pullOutCoordinates(geometries) - pullOutCoordinates(geometries) + // Return the bounding box + return [[minX, minY], [maxX, maxY]] +} - // Return the bounding box - return [[minX, minY], [maxX, maxY]] +const generatePaginatedGeoJsonLinks = async (geoJsonUrl) => { + const geoJsonLinks = [geoJsonUrl] + const initialResponse = await fetch(geoJsonUrl) + const initialData = await initialResponse.json() + + // return if no pagination is needed + if (!initialData.links || !initialData.links.last) { + return geoJsonLinks + } + + const lastLink = new URL(initialData.links.last) + const limit = parseInt(lastLink.searchParams.get('limit')) + const lastOffset = parseInt(lastLink.searchParams.get('offset')) + + if (!limit || !lastOffset) { + console.error('Invalid pagination links', lastLink) + return geoJsonLinks } + + // create a loop to generate the links + for (let offset = limit; offset < lastOffset; offset += limit) { + const newLink = new URL(geoJsonUrl) + newLink.searchParams.set('offset', offset) + + geoJsonLinks.push(newLink.toString()) + } + + return geoJsonLinks +} + +const generateBoundingBox = async (boundaryGeoJsonUrl) => { + const res = await fetch(boundaryGeoJsonUrl) + const boundaryGeoJson = await res.json() + + return calculateBoundingBoxFromGeometries(boundaryGeoJson.features[0].geometry.coordinates) } -const createMapFromServerContext = () => { - const { containerId, geometries, mapType } = window.serverContext +const createMapFromServerContext = async () => { + const { containerId, geometries, mapType, geoJsonUrl, boundaryGeoJsonUrl } = window.serverContext + const options = { + containerId, + data: geometries, + boundaryGeoJsonUrl, + interactive: mapType !== 'static', + wktFormat: geoJsonUrl === undefined + } + + // if the geoJsonUrl is provided, generate the paginated GeoJSON links + if (geoJsonUrl) { + options.data = await generatePaginatedGeoJsonLinks(geoJsonUrl) + } + + if (options.boundaryGeoJsonUrl) { + options.boundingBox = await generateBoundingBox(boundaryGeoJsonUrl) + } // if any of the required properties are missing, return null - if (!containerId || !geometries) { + if (!options.containerId || !options.data) { console.log('Missing required properties (containerId, geometries) on window.serverContext', window.serverContext) return null } - return new Map(containerId, geometries, mapType !== 'static') + return new Map(options) } -const newMap = createMapFromServerContext() - -window.map = newMap +try { + window.map = await createMapFromServerContext() +} catch (error) { + console.error('Error creating map', error) +} diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 046691b6..3809d370 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -7,7 +7,7 @@ import { statusToTagClass } from '../filters/filters.js' import * as v from 'valibot' import { pagination } from '../utils/pagination.js' import config from '../../config/index.js' -import { getFieldStats, getLatestDatasetGeometryEntriesForLpa, getSources } from '../services/DatasetService.js' +import { getFieldStats, getSources } from '../services/DatasetService.js' import { fetchOne, fetchMany, @@ -35,7 +35,7 @@ const getGetStarted = renderTemplate.bind({ const fetchOrgInfo = fetchOne.bind({ query: ({ params }) => { - return `SELECT name, organisation, entity FROM organisation WHERE organisation = '${params.lpa}'` + return `SELECT name, organisation, entity, statistical_geography FROM organisation WHERE organisation = '${params.lpa}'` }, result: 'orgInfo' }) @@ -54,13 +54,6 @@ const fetchDatasetInfo = fetchOne.bind({ result: 'dataset' }) -const fetchDatasetGeometries = async (req, res, next) => { - const datasetEntries = await getLatestDatasetGeometryEntriesForLpa(req.params.dataset, req.params.lpa) - req.geometries = datasetEntries.map(entry => entry.value) - - next() -} - const fetchDatasetStats = async (req, res, next) => { const { dataset, lpa } = req.params const { orgInfo: organisation } = req @@ -111,12 +104,8 @@ const fetchDatasetStats = async (req, res, next) => { const getDatasetOverview = renderTemplate.bind( { templateParams (req) { - const { - orgInfo: organisation, - dataset, geometries, - stats - } = req - return { organisation, dataset, geometries, stats } + const { orgInfo: organisation, dataset, stats } = req + return { organisation, dataset, stats } }, template: 'organisations/dataset-overview.html', handlerName: 'datasetOverview' @@ -549,7 +538,7 @@ const getGetStartedMiddleware = [ logPageError ] -const getDatasetOverviewMiddleware = [fetchOrgInfo, fetchDatasetName, fetchDatasetGeometries, fetchDatasetStats, getDatasetOverview, logPageError] +const getDatasetOverviewMiddleware = [fetchOrgInfo, fetchDatasetInfo, fetchDatasetStats, getDatasetOverview, logPageError] const getOverviewMiddleware = [ fetchOrgInfo, diff --git a/src/routes/schemas.js b/src/routes/schemas.js index 73f743a4..6be66582 100644 --- a/src/routes/schemas.js +++ b/src/routes/schemas.js @@ -69,8 +69,10 @@ export const OrgGetStarted = v.strictObject({ export const OrgDatasetOverview = v.strictObject({ organisation: OrgField, - dataset: DatasetNameField, - geometries: v.array(v.string()), + dataset: v.strictObject({ + name: NonEmptyString, + dataset: NonEmptyString + }), stats: v.strictObject({ numberOfRecords: v.integer(), numberOfFieldsSupplied: v.integer(), diff --git a/src/views/organisations/dataset-overview.html b/src/views/organisations/dataset-overview.html index 54f66cc5..2f0b9ec0 100644 --- a/src/views/organisations/dataset-overview.html +++ b/src/views/organisations/dataset-overview.html @@ -2,6 +2,15 @@ {% from "govuk/components/summary-list/macro.njk" import govukSummaryList %} {% extends "layouts/main.html" %} +{% set showMap = [ + "article-4-direction-area", + "conservation-area", + "listed-building", + "listed-building-outline", + "tree", + "tree-preservation-zone" +].includes(dataset.dataset) %} + {% set serviceType = 'Submit' %} {% set pageName %} {{organisation.name}} - {{dataset.name}} - Dataset overview @@ -129,7 +138,7 @@

{{ dataset.name }}

-{% if geometries and geometries.length %} +{% if showMap %}
@@ -158,14 +167,15 @@

Endpoints

{% block scripts %} {{ super() }} - {% if geometries and geometries.length %} + {% if showMap %} {% endif %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index 93a5323a..bd200bcd 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -69,7 +69,7 @@

Datasets

-{% if showMap %}
+ {{ datasetNavigation({ + active: "dataset-overview", + dataset: dataset, + organisation: organisation, + issue_count: issueCount + }) }} + {% if showMap %}

Map of dataset

+ {% endif %}
-{% endif %}
diff --git a/src/views/organisations/datasetTaskList.html b/src/views/organisations/datasetTaskList.html index 34b11192..b61dbc1e 100644 --- a/src/views/organisations/datasetTaskList.html +++ b/src/views/organisations/datasetTaskList.html @@ -1,11 +1,10 @@ {% from "govuk/components/breadcrumbs/macro.njk" import govukBreadcrumbs %} +{% from 'govuk/components/task-list/macro.njk' import govukTaskList %} +{% from "../components/dataset-navigation.html" import datasetNavigation %} {% extends "layouts/main.html" %} -{% from 'govuk/components/task-list/macro.njk' import govukTaskList %} - {% set pageName %}{{organisation.name}} - {{dataset.name}} - Task list{% endset %} - {% set serviceType = 'Submit' %} {% block beforeContent %} @@ -40,6 +39,17 @@ {% include "includes/_dataset-page-header.html" %}
+
+
+ {{ datasetNavigation({ + active: "task-list", + dataset: dataset, + organisation: organisation, + issue_count: taskList.length + }) }} +
+
+

From 4f1a87507b963e1484b986b91698c3c3bd827451 Mon Sep 17 00:00:00 2001 From: Dilwoar Hussain Date: Wed, 25 Sep 2024 02:19:32 +0100 Subject: [PATCH 36/65] Make map border red to highlight boundaries Better contrast --- src/assets/js/map.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/assets/js/map.js b/src/assets/js/map.js index e3363cc0..7386452d 100644 --- a/src/assets/js/map.js +++ b/src/assets/js/map.js @@ -3,7 +3,8 @@ import maplibregl from 'maplibre-gl' const fillColor = '#008' const lineColor = '#000000' -const boundaryLineColor = '#000000' +const boundaryLineColor = '#f00' +const boundaryLineOpacity = 1 const opacity = 0.4 /** @@ -161,7 +162,7 @@ class Map { paint: { 'line-color': boundaryLineColor, 'line-width': 2, - 'line-opacity': opacity + 'line-opacity': boundaryLineOpacity } }, this.firstMapLayerId) } From 8fb9e146216d458cfa465b3c17f845063fca65f9 Mon Sep 17 00:00:00 2001 From: Dilwoar Hussain Date: Wed, 25 Sep 2024 02:37:43 +0100 Subject: [PATCH 37/65] Add tests to ensure navigation is showing --- test/unit/datasetTaskListPage.test.js | 9 +++++++++ test/unit/views/organisations/dataset-overview.test.js | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/test/unit/datasetTaskListPage.test.js b/test/unit/datasetTaskListPage.test.js index 99db0894..2fa67da1 100644 --- a/test/unit/datasetTaskListPage.test.js +++ b/test/unit/datasetTaskListPage.test.js @@ -83,6 +83,15 @@ describe('Dataset Task List Page', () => { expect(document.querySelector('h1').textContent).toContain(params.dataset.name) }) + it('Renders the dataset navigation links correctly', () => { + const links = document.querySelectorAll('.app-c-dataset-navigation .govuk-service-navigation__link') + + expect(document.querySelector('.app-c-dataset-navigation')).not.toBeNull() + expect(links.length).toEqual(2) + expect(links[0].textContent).toContain('Dataset overview') + expect(links[1].textContent).toContain('Task list') + }) + const taskListItems = document.querySelectorAll('.govuk-task-list__item') it('Task list items are rendered correctly', () => { expect(taskListItems.length).toEqual(params.taskList.length) diff --git a/test/unit/views/organisations/dataset-overview.test.js b/test/unit/views/organisations/dataset-overview.test.js index 867d88dd..ede7a12a 100644 --- a/test/unit/views/organisations/dataset-overview.test.js +++ b/test/unit/views/organisations/dataset-overview.test.js @@ -35,6 +35,15 @@ describe('Dataset Overview Page', () => { expect(document.querySelector('h1').textContent).toContain('World heritage site buffer zone') }) + it('Renders the dataset navigation links correctly', () => { + const links = document.querySelectorAll('.app-c-dataset-navigation .govuk-service-navigation__link') + + expect(document.querySelector('.app-c-dataset-navigation')).not.toBeNull() + expect(links.length).toEqual(2) + expect(links[0].textContent).toContain('Dataset overview') + expect(links[1].textContent).toContain('Task list') + }) + it('Renders dataset details correctly', () => { expect(document.querySelector('h2.govuk-heading-m').textContent).toContain('Dataset details') const summaryListValues = document.querySelectorAll('dd.govuk-summary-list__value') From 342bc081d20d45feed7988b533050ca8c54d783c Mon Sep 17 00:00:00 2001 From: Dilwoar Hussain Date: Wed, 25 Sep 2024 02:55:41 +0100 Subject: [PATCH 38/65] Add tests for dataset navigation component --- test/unit/datasetTaskListPage.test.js | 6 +- .../components/dataset-navigation.test.js | 93 +++++++++++++++++++ .../organisations/dataset-overview.test.js | 4 +- 3 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 test/unit/views/components/dataset-navigation.test.js diff --git a/test/unit/datasetTaskListPage.test.js b/test/unit/datasetTaskListPage.test.js index 2fa67da1..99a0b3b4 100644 --- a/test/unit/datasetTaskListPage.test.js +++ b/test/unit/datasetTaskListPage.test.js @@ -85,11 +85,13 @@ describe('Dataset Task List Page', () => { it('Renders the dataset navigation links correctly', () => { const links = document.querySelectorAll('.app-c-dataset-navigation .govuk-service-navigation__link') + const activeLink = document.querySelector('.app-c-dataset-navigation .govuk-service-navigation__item.govuk-service-navigation__item--active') + const issueCount = document.querySelector('.app-c-dataset-navigation .govuk-service-navigation__item.govuk-service-navigation__item--active .govuk-tag') expect(document.querySelector('.app-c-dataset-navigation')).not.toBeNull() expect(links.length).toEqual(2) - expect(links[0].textContent).toContain('Dataset overview') - expect(links[1].textContent).toContain('Task list') + expect(activeLink.textContent).toContain('Task list') + expect(issueCount.textContent).toContain('3') }) const taskListItems = document.querySelectorAll('.govuk-task-list__item') diff --git a/test/unit/views/components/dataset-navigation.test.js b/test/unit/views/components/dataset-navigation.test.js new file mode 100644 index 00000000..383d6f16 --- /dev/null +++ b/test/unit/views/components/dataset-navigation.test.js @@ -0,0 +1,93 @@ +import { describe, it, expect } from 'vitest' +import jsdom from 'jsdom' +import { setupNunjucks } from '../../../../src/serverSetup/nunjucks.js' + +describe('Dataset Navigation component', () => { + const params = { + organisation: { + name: 'Mock org', + organisation: 'mock-org' + }, + dataset: { + dataset: 'world-heritage-site-buffer-zone', + name: 'World heritage site buffer zone' + }, + issueCount: 0 + } + + const htmlString = ` + {% from "components/dataset-navigation.html" import datasetNavigation %} + {{ datasetNavigation({ + active: "dataset-overview", + dataset: dataset, + organisation: organisation, + issue_count: issueCount + }) }} + ` + + const nunjucks = setupNunjucks({ datasetNameMapping: new Map() }) + const html = nunjucks.renderString(htmlString, params) + const dom = new jsdom.JSDOM(html) + const document = dom.window.document + + it('Renders the dataset navigation links correctly', () => { + const links = document.querySelectorAll('.app-c-dataset-navigation .govuk-service-navigation__link') + const activeLink = document.querySelector('.app-c-dataset-navigation .govuk-service-navigation__item.govuk-service-navigation__item--active') + const issueCount = document.querySelector('.app-c-dataset-navigation .govuk-service-navigation__item.govuk-service-navigation__item--active .govuk-tag') + + expect(document.querySelector('.app-c-dataset-navigation')).not.toBeNull() + expect(activeLink.textContent).toContain('Dataset overview') + expect(links.length).toEqual(2) + expect(issueCount).toBeNull() + }) + + it('Renders the active dataset navigation links correctly', () => { + const htmlString = ` + {% from "components/dataset-navigation.html" import datasetNavigation %} + {{ datasetNavigation({ + active: "task-list", + dataset: dataset, + organisation: organisation, + issue_count: issueCount + }) }} + ` + + const htmlWithActiveLink = nunjucks.renderString(htmlString, params) + const domWithActiveLink = new jsdom.JSDOM(htmlWithActiveLink) + const documentWithActiveLink = domWithActiveLink.window.document + const links = documentWithActiveLink.querySelectorAll('.app-c-dataset-navigation .govuk-service-navigation__link') + const activeLink = documentWithActiveLink.querySelector('.app-c-dataset-navigation .govuk-service-navigation__item.govuk-service-navigation__item--active') + + expect(document.querySelector('.app-c-dataset-navigation')).not.toBeNull() + expect(activeLink.textContent).toContain('Task list') + expect(links.length).toEqual(2) + }) + + it('Renders the issue count correctly', () => { + const paramsWithIssues = { + ...params, + issueCount: 3 + } + const htmlString = ` + {% from "components/dataset-navigation.html" import datasetNavigation %} + {{ datasetNavigation({ + active: "task-list", + dataset: dataset, + organisation: organisation, + issue_count: issueCount + }) }} + ` + + const htmlWithIssues = nunjucks.renderString(htmlString, paramsWithIssues) + const domWithIssues = new jsdom.JSDOM(htmlWithIssues) + const documentWithIssues = domWithIssues.window.document + const links = documentWithIssues.querySelectorAll('.app-c-dataset-navigation .govuk-service-navigation__link') + const activeLink = documentWithIssues.querySelector('.app-c-dataset-navigation .govuk-service-navigation__item.govuk-service-navigation__item--active') + const issueCount = documentWithIssues.querySelector('.app-c-dataset-navigation .govuk-service-navigation__item.govuk-service-navigation__item--active .govuk-tag') + + expect(document.querySelector('.app-c-dataset-navigation')).not.toBeNull() + expect(activeLink.textContent).toContain('Task list') + expect(issueCount.textContent).toContain('3') + expect(links.length).toEqual(2) + }) +}) diff --git a/test/unit/views/organisations/dataset-overview.test.js b/test/unit/views/organisations/dataset-overview.test.js index ede7a12a..33594824 100644 --- a/test/unit/views/organisations/dataset-overview.test.js +++ b/test/unit/views/organisations/dataset-overview.test.js @@ -37,11 +37,11 @@ describe('Dataset Overview Page', () => { it('Renders the dataset navigation links correctly', () => { const links = document.querySelectorAll('.app-c-dataset-navigation .govuk-service-navigation__link') + const activeLink = document.querySelector('.app-c-dataset-navigation .govuk-service-navigation__item.govuk-service-navigation__item--active') expect(document.querySelector('.app-c-dataset-navigation')).not.toBeNull() expect(links.length).toEqual(2) - expect(links[0].textContent).toContain('Dataset overview') - expect(links[1].textContent).toContain('Task list') + expect(activeLink.textContent).toContain('Dataset overview') }) it('Renders dataset details correctly', () => { From a823ae7627331d75c90bcf89c0cbf4dc45f75154 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 25 Sep 2024 09:57:48 +0100 Subject: [PATCH 39/65] added middleware builders test file --- .../middleware.builders.test.js} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename test/unit/{middleware.test.js => middleware/middleware.builders.test.js} (95%) diff --git a/test/unit/middleware.test.js b/test/unit/middleware/middleware.builders.test.js similarity index 95% rename from test/unit/middleware.test.js rename to test/unit/middleware/middleware.builders.test.js index dc54c84d..456028a0 100644 --- a/test/unit/middleware.test.js +++ b/test/unit/middleware/middleware.builders.test.js @@ -1,14 +1,14 @@ import { describe, it, vi, expect, beforeEach } from 'vitest' -import datasette from '../../src/services/datasette.js' -import * as middleware from '../../src/controllers/middleware.js' +import datasette from '../../../src/services/datasette.js' +import * as middleware from '../../../src/middleware/middleware.builders.js' -vi.mock('../../src/services/datasette.js', () => ({ +vi.mock('../../../src/services/datasette.js', () => ({ default: { runQuery: vi.fn() } })) -describe('Middleware', () => { +describe('middleware.builders', () => { beforeEach(() => { vi.resetAllMocks() }) From 223ec7c297d4f5df33bdfe6f62ddd25eb7fb3953 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 25 Sep 2024 11:24:36 +0100 Subject: [PATCH 40/65] move datasettaskList middleware tests to their own file --- .../datasetTaskList.middleware.test.js | 109 ++++++++++++++++++ test/unit/organisationsController.test.js | 104 ----------------- 2 files changed, 109 insertions(+), 104 deletions(-) create mode 100644 test/unit/middleware/datasetTaskList.middleware.test.js diff --git a/test/unit/middleware/datasetTaskList.middleware.test.js b/test/unit/middleware/datasetTaskList.middleware.test.js new file mode 100644 index 00000000..d176617b --- /dev/null +++ b/test/unit/middleware/datasetTaskList.middleware.test.js @@ -0,0 +1,109 @@ +import { describe, it, vi, expect } from 'vitest' +import performanceDbApi from '../../../src/services/performanceDbApi.js' +import { prepareDatasetTaskListTemplateParams, prepareDatasetTaskListErrorTemplateParams } from '../../../src/middleware/datasetTaskList.middleware.js' + +vi.mock('../../../src/services/performanceDbApi.js') + +describe('datasetTaskList.middleware.js', () => { + describe('prepareDatasetTaskListParams', () => { + it('sets the correct template params on the request object', async () => { + const req = { + params: { lpa: 'example-lpa', dataset: 'example-dataset' }, + resourceStatus: { + resource: 'mock-resource', + endpoint_url: 'http://example.com/resource', + status: '200', + latest_log_entry_date: '', + days_since_200: 0 + }, + orgInfo: { name: 'Example Organisation', organisation: 'ORG' }, + dataset: { name: 'Example Dataset' }, + resource: { resource: 'mock-resource' }, + issues: [ + { + issue: 'Example issue 1', + issue_type: 'Example issue type 1', + field: 'Example issue field 1', + num_issues: 1, + status: 'Error' + }, + { + issue: 'Example issue 2', + issue_type: 'Example issue type 2', + field: 'Example issue field 2', + num_issues: 1, + status: 'Needs fixing' + } + ] + } + const res = { render: vi.fn() } + const next = vi.fn() + + vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce('task message 1').mockReturnValueOnce('task message 2') + + prepareDatasetTaskListTemplateParams(req, res, next) + + const templateParams = { + taskList: [ + { + title: { + text: 'task message 1' + }, + href: '/organisations/example-lpa/example-dataset/Example issue type 1/Example issue field 1', + status: { + tag: { + classes: 'govuk-tag--red', + text: 'Error' + } + } + }, + { + title: { + text: 'task message 2' + }, + href: '/organisations/example-lpa/example-dataset/Example issue type 2/Example issue field 2', + status: { + tag: { + classes: 'govuk-tag--yellow', + text: 'Needs fixing' + } + } + } + ], + organisation: { name: 'Example Organisation', organisation: 'ORG' }, + dataset: { name: 'Example Dataset' } + } + + expect(req.templateParams).toEqual(templateParams) + }) + }) + + describe('prepareDatasetTaskListErrorTemplateParams', () => { + it('sets the correct template params on the request object', async () => { + const resourceStatus = { status: '404', days_since_200: 3, endpoint_url: 'https://example.com', latest_log_entry_date: '2022-01-01T12:00:00.000Z' } + const organisation = { name: 'Example Organisation', organisation: 'ORG' } + const dataset = { name: 'Example Dataset', dataset: 'example-dataset' } + const req = { + params: { lpa: 'example-lpa', dataset: 'example-dataset' }, + resourceStatus, + orgInfo: organisation, + dataset + } + const res = { render: vi.fn() } + const next = vi.fn() + + prepareDatasetTaskListErrorTemplateParams(req, res, next) + + const templateParams = req.templateParams + + const dataTimeRegex = /\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d{1,3})?Z/ + + expect(templateParams.organisation).toEqual(organisation) + expect(templateParams.dataset).toEqual(dataset) + expect(templateParams.errorData.endpoint_url).toEqual('https://example.com') + expect(templateParams.errorData.http_status).toEqual('404') + expect(templateParams.errorData.latest_log_entry_date).toMatch(dataTimeRegex) + expect(templateParams.errorData.latest_200_date).toMatch(dataTimeRegex) + }) + }) +}) diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js index a3ca6e5a..bf5d9bcd 100644 --- a/test/unit/organisationsController.test.js +++ b/test/unit/organisationsController.test.js @@ -166,79 +166,6 @@ describe('OrganisationsController.js', () => { }) }) - describe('dataset task list', () => { - it('should fetch the dataset tasks and correctly pass them on to the dataset task list page', async () => { - const req = { - params: { lpa: 'example-lpa', dataset: 'example-dataset' }, - resourceStatus: { - resource: 'mock-resource', - endpoint_url: 'http://example.com/resource', - status: '200', - latest_log_entry_date: '', - days_since_200: 0 - }, - orgInfo: { name: 'Example Organisation', organisation: 'ORG' }, - dataset: { name: 'Example Dataset' }, - resource: { resource: 'mock-resource' }, - issues: [ - { - issue: 'Example issue 1', - issue_type: 'Example issue type 1', - field: 'Example issue field 1', - num_issues: 1, - status: 'Error' - }, - { - issue: 'Example issue 2', - issue_type: 'Example issue type 2', - field: 'Example issue field 2', - num_issues: 1, - status: 'Needs fixing' - } - ] - } - const res = { render: vi.fn() } - const next = vi.fn() - - vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce('task message 1').mockReturnValueOnce('task message 2') - - organisationsController.prepareDatasetTaskListTemplateParams(req, res, next) - organisationsController.getDatasetTaskList(req, res, next) - - expect(res.render).toHaveBeenCalledTimes(1) - expect(res.render).toHaveBeenCalledWith('organisations/datasetTaskList.html', { - taskList: [ - { - title: { - text: 'task message 1' - }, - href: '/organisations/example-lpa/example-dataset/Example issue type 1/Example issue field 1', - status: { - tag: { - classes: 'govuk-tag--red', - text: 'Error' - } - } - }, - { - title: { - text: 'task message 2' - }, - href: '/organisations/example-lpa/example-dataset/Example issue type 2/Example issue field 2', - status: { - tag: { - classes: 'govuk-tag--yellow', - text: 'Needs fixing' - } - } - } - ], - organisation: { name: 'Example Organisation', organisation: 'ORG' }, - dataset: { name: 'Example Dataset' } - }) - }) - }) - describe('issue details', () => { const orgInfo = { name: 'mock lpa', organisation: 'ORG' } const dataset = { name: 'mock dataset', dataset: 'mock-dataset' } @@ -462,35 +389,4 @@ describe('OrganisationsController.js', () => { }) }) }) - - describe('getDatasetTaskListError', () => { - it('should render http-error.html template with correct params', async () => { - const resourceStatus = { status: '404', days_since_200: 3, endpoint_url: 'https://example.com', latest_log_entry_date: '2022-01-01T12:00:00.000Z' } - const organisation = { name: 'Example Organisation', organisation: 'ORG' } - const dataset = { name: 'Example Dataset', dataset: 'example-dataset' } - const req = { - params: { lpa: 'example-lpa', dataset: 'example-dataset' }, - resourceStatus, - orgInfo: organisation, - dataset - } - const res = { render: vi.fn() } - const next = vi.fn() - - organisationsController.prepareDatasetTaskListErrorTemplateParams(req, res, next) - organisationsController.getDatasetTaskListError(req, res, next) - - const dataTimeRegex = /\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d{1,3})?Z/ - - expect(res.render).toHaveBeenCalledTimes(1) - const renderArgs = res.render.mock.calls[0] - expect(renderArgs[0]).toEqual('organisations/http-error.html') - expect(renderArgs[1].organisation).toEqual(organisation) - expect(renderArgs[1].dataset).toEqual(dataset) - expect(renderArgs[1].errorData.endpoint_url).toEqual('https://example.com') - expect(renderArgs[1].errorData.http_status).toEqual('404') - expect(renderArgs[1].errorData.latest_log_entry_date).toMatch(dataTimeRegex) - expect(renderArgs[1].errorData.latest_200_date).toMatch(dataTimeRegex) - }) - }) }) From 05df90c61f52261dc6c7a434a5769b7ecf012217 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 25 Sep 2024 11:32:43 +0100 Subject: [PATCH 41/65] make sure to export function for testing --- src/middleware/datasetTaskList.middleware.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/middleware/datasetTaskList.middleware.js b/src/middleware/datasetTaskList.middleware.js index 30dc514a..c8420503 100644 --- a/src/middleware/datasetTaskList.middleware.js +++ b/src/middleware/datasetTaskList.middleware.js @@ -46,7 +46,7 @@ function getStatusTag (status) { * @param {*} next * @returns { { templateParams: object }} */ -const prepareDatasetTaskListTemplateParams = (req, res, next) => { +export const prepareDatasetTaskListTemplateParams = (req, res, next) => { const { issues, entityCount: entityCountRow, params, dataset, orgInfo: organisation } = req const { entity_count: entityCount } = entityCountRow ?? { entity_count: 0 } const { lpa, dataset: datasetId } = params @@ -86,7 +86,7 @@ const getDatasetTaskList = renderTemplate({ * @param {} next * @returns {{ templateParams: object }} */ -const prepareDatasetTaskListErrorTemplateParams = (req, res, next) => { +export const prepareDatasetTaskListErrorTemplateParams = (req, res, next) => { const { orgInfo: organisation, dataset, resourceStatus: resource } = req const daysSince200 = resource.days_since_200 From a1f91104ecabcec185ad5fb58640ed299e0a752b Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 25 Sep 2024 11:32:59 +0100 Subject: [PATCH 42/65] move get started middleware tests to their own file --- src/middleware/getStarted.middleware.js | 2 +- .../middleware/getStarted.middleware.test.js | 44 +++++++++++++++++++ test/unit/organisationsController.test.js | 42 ------------------ 3 files changed, 45 insertions(+), 43 deletions(-) create mode 100644 test/unit/middleware/getStarted.middleware.test.js diff --git a/src/middleware/getStarted.middleware.js b/src/middleware/getStarted.middleware.js index 9bfc123a..4d2d351b 100644 --- a/src/middleware/getStarted.middleware.js +++ b/src/middleware/getStarted.middleware.js @@ -1,7 +1,7 @@ import { fetchDatasetInfo, fetchOrgInfo, logPageError } from './common.middleware.js' import { renderTemplate } from './middleware.builders.js' -const getGetStarted = renderTemplate({ +export const getGetStarted = renderTemplate({ templateParams (req) { const { orgInfo: organisation, dataset } = req return { organisation, dataset } diff --git a/test/unit/middleware/getStarted.middleware.test.js b/test/unit/middleware/getStarted.middleware.test.js new file mode 100644 index 00000000..5a31ec96 --- /dev/null +++ b/test/unit/middleware/getStarted.middleware.test.js @@ -0,0 +1,44 @@ +import { describe, it, vi, expect } from 'vitest' +import { getGetStarted } from '../../../src/middleware/getStarted.middleware.js' + +describe('get-started', () => { + const exampleLpa = { + formattedData: [ + { name: 'Example LPA', organisation: 'LPA' } + ] + } + const exampleDataset = { name: 'Example Dataset', dataset: 'example-dataset' } + + it('should render the get-started template with the correct params', async () => { + const req = { + params: { lpa: 'example-lpa', dataset: 'example-dataset' }, + orgInfo: exampleLpa.formattedData[0], + dataset: exampleDataset + } + const res = { render: vi.fn() } + const next = vi.fn() + + getGetStarted(req, res, next) + + expect(res.render).toHaveBeenCalledTimes(1) + expect(res.render).toHaveBeenCalledWith('organisations/get-started.html', { + organisation: { name: 'Example LPA', organisation: 'LPA' }, + dataset: exampleDataset + }) + }) + + it('should catch and pass errors to the next function', async () => { + const req = { + params: { lpa: 'example-lpa', dataset: 'example-dataset' }, + orgInfo: undefined, // this should fail validation + dataset: exampleDataset + } + const res = { render: vi.fn() } + const next = vi.fn() + + getGetStarted(req, res, next) + + expect(next).toHaveBeenCalledTimes(1) + expect(res.render).toHaveBeenCalledTimes(0) + }) +}) diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js index bf5d9bcd..f67861db 100644 --- a/test/unit/organisationsController.test.js +++ b/test/unit/organisationsController.test.js @@ -124,48 +124,6 @@ describe('OrganisationsController.js', () => { }) }) - describe('get-started', () => { - const exampleLpa = { - formattedData: [ - { name: 'Example LPA', organisation: 'LPA' } - ] - } - const exampleDataset = { name: 'Example Dataset', dataset: 'example-dataset' } - - it('should render the get-started template with the correct params', async () => { - const req = { - params: { lpa: 'example-lpa', dataset: 'example-dataset' }, - orgInfo: exampleLpa.formattedData[0], - dataset: exampleDataset - } - const res = { render: vi.fn() } - const next = vi.fn() - - organisationsController.getGetStarted(req, res, next) - - expect(res.render).toHaveBeenCalledTimes(1) - expect(res.render).toHaveBeenCalledWith('organisations/get-started.html', { - organisation: { name: 'Example LPA', organisation: 'LPA' }, - dataset: exampleDataset - }) - }) - - it('should catch and pass errors to the next function', async () => { - const req = { - params: { lpa: 'example-lpa', dataset: 'example-dataset' }, - orgInfo: undefined, // this should fail validation - dataset: exampleDataset - } - const res = { render: vi.fn() } - const next = vi.fn() - - organisationsController.getGetStarted(req, res, next) - - expect(next).toHaveBeenCalledTimes(1) - expect(res.render).toHaveBeenCalledTimes(0) - }) - }) - describe('issue details', () => { const orgInfo = { name: 'mock lpa', organisation: 'ORG' } const dataset = { name: 'mock dataset', dataset: 'mock-dataset' } From 18cefbdad54823d128d09155e3973be7ea2c8d41 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 25 Sep 2024 12:04:31 +0100 Subject: [PATCH 43/65] move issue details middleware tests to their own file --- src/middleware/issueDetails.middleware.js | 6 +- .../issueDetails.middleware.test.js | 287 ++++++++++++++++++ test/unit/organisationsController.test.js | 226 -------------- 3 files changed, 290 insertions(+), 229 deletions(-) create mode 100644 test/unit/middleware/issueDetails.middleware.test.js diff --git a/src/middleware/issueDetails.middleware.js b/src/middleware/issueDetails.middleware.js index cd44788b..3fee650b 100644 --- a/src/middleware/issueDetails.middleware.js +++ b/src/middleware/issueDetails.middleware.js @@ -6,7 +6,7 @@ import { fetchIf, parallel, renderTemplate } from './middleware.builders.js' import * as v from 'valibot' import { pagination } from '../utils/pagination.js' -const IssueDetailsQueryParams = v.object({ +export const IssueDetailsQueryParams = v.object({ lpa: v.string(), dataset: v.string(), issue_type: v.string(), @@ -182,7 +182,7 @@ const processEntryRow = (issueType, issuesByEntryNumber, row) => { /*** * Middleware. Updates req with `templateParams` */ -function prepareIssueDetailsTemplateParams (req, res, next) { +export function prepareIssueDetailsTemplateParams (req, res, next) { const { entryData, pageNumber, issueEntitiesCount, issuesByEntryNumber, entryNumber, entityCount: entityCountRow } = req const { lpa, dataset: datasetId, issue_type: issueType, issue_field: issueField } = req.params const { entity_count: entityCount } = entityCountRow ?? { entity_count: 0 } @@ -279,7 +279,7 @@ function prepareIssueDetailsTemplateParams (req, res, next) { * Middleware. Renders the issue details page with the list of issues, entry data, * and organisation and dataset details. */ -const getIssueDetails = renderTemplate({ +export const getIssueDetails = renderTemplate({ templateParams: (req) => req.templateParams, template: 'organisations/issueDetails.html', handlerName: 'getIssueDetails' diff --git a/test/unit/middleware/issueDetails.middleware.test.js b/test/unit/middleware/issueDetails.middleware.test.js new file mode 100644 index 00000000..9e79e561 --- /dev/null +++ b/test/unit/middleware/issueDetails.middleware.test.js @@ -0,0 +1,287 @@ +import { describe, it, vi, expect } from 'vitest' +import * as v from 'valibot' + +import performanceDbApi from '../../../src/services/performanceDbApi.js' +import { getIssueDetails, IssueDetailsQueryParams, prepareIssueDetailsTemplateParams } from '../../../src/middleware/issueDetails.middleware.js' + +vi.mock('../../../src/services/performanceDbApi.js') + +describe('issueDetails.middleware.js', () => { + const orgInfo = { name: 'mock lpa', organisation: 'ORG' } + const dataset = { name: 'mock dataset', dataset: 'mock-dataset' } + const entryData = [ + { + field: 'start-date', + value: '02-02-2022', + entry_number: 1 + } + ] + const issues = [ + { + entry_number: 0, + field: 'start-date', + value: '02-02-2022' + } + ] + + describe('prepareIssueDetailsTemplateParams', () => { + it('should correctly set the template params', async () => { + const requestParams = { + lpa: 'test-lpa', + dataset: 'test-dataset', + issue_type: 'test-issue-type', + issue_field: 'test-issue-field', + resourceId: 'test-resource-id', + entityNumber: '1' + } + const req = { + params: requestParams, + // middleware supplies the below + entryNumber: 1, + entityCount: { entity_count: 3 }, + issueEntitiesCount: 1, + pageNumber: 1, + orgInfo, + dataset, + entryData, + issues, + resource: { resource: requestParams.resourceId }, + issuesByEntryNumber: { + 1: [ + { + field: 'start-date', + value: '02-02-2022', + line_number: 1, + entry_number: 1, + message: 'mock message', + issue_type: 'mock type' + } + ] + } + // errorHeading -- set in prepare* fn + } + v.parse(IssueDetailsQueryParams, req.params) + + issues.forEach(issue => { + vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce(`mockMessageFor: ${issue.entry_number}`) + }) + vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce('mock task message 1') + + prepareIssueDetailsTemplateParams(req, {}, () => {}) + + const expectedTempalteParams = { + organisation: { + name: 'mock lpa', + organisation: 'ORG' + }, + dataset: { + name: 'mock dataset', + dataset: 'mock-dataset' + }, + errorHeading: 'mockMessageFor: 0', + issueItems: [ + { + html: 'mock task message 1 in record 1', + href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1' + } + ], + entry: { + title: 'entry: 1', + fields: [ + { + key: { text: 'start-date' }, + value: { html: '

mock message

02-02-2022' }, + classes: 'dl-summary-card-list__row--error' + } + ], + geometries: [] + }, + issueType: 'test-issue-type', + pagination: { + items: [{ + current: true, + href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1', + number: 1, + type: 'item' + }] + }, + issueEntitiesCount: 1, + pageNumber: 1 + } + + expect(req.templateParams).toEqual(expectedTempalteParams) + }) + + it('should correctly set the template params with the correct geometry params', async () => { + const entryData = [ + { + field: 'start-date', + value: '02-02-2022', + entry_number: 1 + }, + { + field: 'geometry', + value: 'POINT(0 0)', + entry_number: 1 + } + ] + const requestParams = { + lpa: 'test-lpa', + dataset: 'test-dataset', + issue_type: 'test-issue-type', + issue_field: 'test-issue-field', + resourceId: 'test-resource-id', + entityNumber: '1' + } + const req = { + params: requestParams, + // middleware supplies the below + entryNumber: 1, + entityCount: { entity_count: 3 }, + issueEntitiesCount: 1, + pageNumber: 1, + orgInfo, + dataset, + entryData, + issues, + resource: { resource: requestParams.resourceId }, + issuesByEntryNumber: { + 1: [ + { + field: 'start-date', + value: '02-02-2022', + line_number: 1, + entry_number: 1, + message: 'mock message', + issue_type: 'mock type' + } + ] + } + // errorHeading -- set in prepare* fn + } + v.parse(IssueDetailsQueryParams, req.params) + + issues.forEach(issue => { + vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce(`mockMessageFor: ${issue.entry_number}`) + }) + vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce('mock task message 1') + + prepareIssueDetailsTemplateParams(req, {}, () => {}) + + const expectedTemplateParams = { + organisation: { + name: 'mock lpa', + organisation: 'ORG' + }, + dataset: { + name: 'mock dataset', + dataset: 'mock-dataset' + }, + errorHeading: 'mockMessageFor: 0', + issueItems: [ + { + html: 'mock task message 1 in record 1', + href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1' + } + ], + entry: { + title: 'entry: 1', + fields: [ + { + key: { text: 'start-date' }, + value: { html: '

mock message

02-02-2022' }, + classes: 'dl-summary-card-list__row--error' + }, + { + classes: '', + key: { + text: 'geometry' + }, + value: { + html: 'POINT(0 0)' + } + } + ], + geometries: ['POINT(0 0)'] + }, + issueType: 'test-issue-type', + pagination: { + items: [{ + current: true, + href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1', + number: 1, + type: 'item' + }] + }, + issueEntitiesCount: 1, + pageNumber: 1 + } + + expect(req.templateParams).toEqual(expectedTemplateParams) + }) + }) + + describe('getIssueDetails', () => { + it('should call render using the provided template params and correct view', () => { + const req = { + templateParams: { + organisation: { + name: 'mock lpa', + organisation: 'ORG' + }, + dataset: { + name: 'mock dataset', + dataset: 'mock-dataset' + }, + errorHeading: 'mockMessageFor: 0', + issueItems: [ + { + html: 'mock task message 1 in record 1', + href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1' + } + ], + entry: { + title: 'entry: 1', + fields: [ + { + key: { text: 'start-date' }, + value: { html: '

mock message

02-02-2022' }, + classes: 'dl-summary-card-list__row--error' + }, + { + classes: '', + key: { + text: 'geometry' + }, + value: { + html: 'POINT(0 0)' + } + } + ], + geometries: ['POINT(0 0)'] + }, + issueType: 'test-issue-type', + pagination: { + items: [{ + current: true, + href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1', + number: 1, + type: 'item' + }] + }, + issueEntitiesCount: 1, + pageNumber: 1 + } + } + + const res = { + render: vi.fn() + } + + getIssueDetails(req, res, () => {}) + + expect(res.render).toHaveBeenCalledTimes(1) + expect(res.render).toHaveBeenCalledWith('organisations/issueDetails.html', req.templateParams) + }) + }) +}) diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js index f67861db..d1a6c6df 100644 --- a/test/unit/organisationsController.test.js +++ b/test/unit/organisationsController.test.js @@ -1,7 +1,5 @@ import { describe, it, vi, expect, beforeEach } from 'vitest' -import * as v from 'valibot' import organisationsController from '../../src/controllers/OrganisationsController.js' -import performanceDbApi from '../../src/services/performanceDbApi.js' vi.mock('../../src/services/performanceDbApi.js') vi.mock('../../src/utils/utils.js', () => { @@ -123,228 +121,4 @@ describe('OrganisationsController.js', () => { })) }) }) - - describe('issue details', () => { - const orgInfo = { name: 'mock lpa', organisation: 'ORG' } - const dataset = { name: 'mock dataset', dataset: 'mock-dataset' } - const entryData = [ - { - field: 'start-date', - value: '02-02-2022', - entry_number: 1 - } - ] - const issues = [ - { - entry_number: 0, - field: 'start-date', - value: '02-02-2022' - } - ] - - it('should call render with the issue details page and the correct params', async () => { - const requestParams = { - lpa: 'test-lpa', - dataset: 'test-dataset', - issue_type: 'test-issue-type', - issue_field: 'test-issue-field', - resourceId: 'test-resource-id', - entityNumber: '1' - } - const req = { - params: requestParams, - // middleware supplies the below - entryNumber: 1, - entityCount: { entity_count: 3 }, - issueEntitiesCount: 1, - pageNumber: 1, - orgInfo, - dataset, - entryData, - issues, - resource: { resource: requestParams.resourceId }, - issuesByEntryNumber: { - 1: [ - { - field: 'start-date', - value: '02-02-2022', - line_number: 1, - entry_number: 1, - message: 'mock message', - issue_type: 'mock type' - } - ] - } - // errorHeading -- set in prepare* fn - } - v.parse(organisationsController.IssueDetailsQueryParams, req.params) - - const res = { - render: vi.fn() - } - const next = vi.fn() - - issues.forEach(issue => { - vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce(`mockMessageFor: ${issue.entry_number}`) - }) - vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce('mock task message 1') - - organisationsController.prepareIssueDetailsTemplateParams(req, {}, () => {}) - organisationsController.getIssueDetails(req, res, next) - - expect(res.render).toHaveBeenCalledTimes(1) - expect(res.render).toHaveBeenCalledWith('organisations/issueDetails.html', { - organisation: { - name: 'mock lpa', - organisation: 'ORG' - }, - dataset: { - name: 'mock dataset', - dataset: 'mock-dataset' - }, - errorHeading: 'mockMessageFor: 0', - issueItems: [ - { - html: 'mock task message 1 in record 1', - href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1' - } - ], - entry: { - title: 'entry: 1', - fields: [ - { - key: { text: 'start-date' }, - value: { html: '

mock message

02-02-2022' }, - classes: 'dl-summary-card-list__row--error' - } - ], - geometries: [] - }, - issueType: 'test-issue-type', - pagination: { - items: [{ - current: true, - href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1', - number: 1, - type: 'item' - }] - }, - issueEntitiesCount: 1, - pageNumber: 1 - }) - }) - - it('should call render with the issue details page with the correct geometry params', async () => { - const entryData = [ - { - field: 'start-date', - value: '02-02-2022', - entry_number: 1 - }, - { - field: 'geometry', - value: 'POINT(0 0)', - entry_number: 1 - } - ] - const requestParams = { - lpa: 'test-lpa', - dataset: 'test-dataset', - issue_type: 'test-issue-type', - issue_field: 'test-issue-field', - resourceId: 'test-resource-id', - entityNumber: '1' - } - const req = { - params: requestParams, - // middleware supplies the below - entryNumber: 1, - entityCount: { entity_count: 3 }, - issueEntitiesCount: 1, - pageNumber: 1, - orgInfo, - dataset, - entryData, - issues, - resource: { resource: requestParams.resourceId }, - issuesByEntryNumber: { - 1: [ - { - field: 'start-date', - value: '02-02-2022', - line_number: 1, - entry_number: 1, - message: 'mock message', - issue_type: 'mock type' - } - ] - } - // errorHeading -- set in prepare* fn - } - v.parse(organisationsController.IssueDetailsQueryParams, req.params) - - const res = { - render: vi.fn() - } - const next = vi.fn() - - issues.forEach(issue => { - vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce(`mockMessageFor: ${issue.entry_number}`) - }) - vi.mocked(performanceDbApi.getTaskMessage).mockReturnValueOnce('mock task message 1') - - organisationsController.prepareIssueDetailsTemplateParams(req, {}, () => {}) - organisationsController.getIssueDetails(req, res, next) - - expect(res.render).toHaveBeenCalledTimes(1) - expect(res.render).toHaveBeenCalledWith('organisations/issueDetails.html', { - organisation: { - name: 'mock lpa', - organisation: 'ORG' - }, - dataset: { - name: 'mock dataset', - dataset: 'mock-dataset' - }, - errorHeading: 'mockMessageFor: 0', - issueItems: [ - { - html: 'mock task message 1 in record 1', - href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1' - } - ], - entry: { - title: 'entry: 1', - fields: [ - { - key: { text: 'start-date' }, - value: { html: '

mock message

02-02-2022' }, - classes: 'dl-summary-card-list__row--error' - }, - { - classes: '', - key: { - text: 'geometry' - }, - value: { - html: 'POINT(0 0)' - } - } - ], - geometries: ['POINT(0 0)'] - }, - issueType: 'test-issue-type', - pagination: { - items: [{ - current: true, - href: '/organisations/test-lpa/test-dataset/test-issue-type/test-issue-field/1', - number: 1, - type: 'item' - }] - }, - issueEntitiesCount: 1, - pageNumber: 1 - }) - }) - }) }) From 1cbaaf47f884386b90c438c49e453103c55053f7 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 25 Sep 2024 12:16:16 +0100 Subject: [PATCH 44/65] moved organisations middleware tests to their own file --- src/middleware/organisations.middleware.js | 4 +- .../organisations.middleware.test.js | 80 +++++++++++++++++++ test/unit/organisationsController.test.js | 58 -------------- 3 files changed, 82 insertions(+), 60 deletions(-) create mode 100644 test/unit/middleware/organisations.middleware.test.js diff --git a/src/middleware/organisations.middleware.js b/src/middleware/organisations.middleware.js index b9b67da5..9efcc4a5 100644 --- a/src/middleware/organisations.middleware.js +++ b/src/middleware/organisations.middleware.js @@ -13,7 +13,7 @@ const fetchOrganisations = fetchMany({ * @param {*} res * @param {*} next */ -const prepareGetOrganisationsTemplateParams = (req, res, next) => { +export const prepareGetOrganisationsTemplateParams = (req, res, next) => { const sortedResults = req.organisations.sort((a, b) => { return a.name.localeCompare(b.name) }) @@ -30,7 +30,7 @@ const prepareGetOrganisationsTemplateParams = (req, res, next) => { next() } -const getOrganisations = renderTemplate({ +export const getOrganisations = renderTemplate({ templateParams: (req) => req.templateParams, template: 'organisations/find.html', handlerName: 'getOrganisations' diff --git a/test/unit/middleware/organisations.middleware.test.js b/test/unit/middleware/organisations.middleware.test.js new file mode 100644 index 00000000..e9b456a0 --- /dev/null +++ b/test/unit/middleware/organisations.middleware.test.js @@ -0,0 +1,80 @@ +import { describe, it, vi, expect } from 'vitest' +import { getOrganisations, prepareGetOrganisationsTemplateParams } from '../../../src/middleware/organisations.middleware' + +describe('organisations.middleware.js', () => { + describe('prepareGetOrganisationsTemplateParams', () => { + it('should correctly sort and restructure the data received from datasette, then set them on req.template params', async () => { + const req = {} + const res = { render: vi.fn() } + const next = vi.fn() + + const datasetteResponse = [ + { name: 'Aardvark Healthcare', organisation: 'Aardvark Healthcare' }, + { name: 'Bath NHS Trust', organisation: 'Bath NHS Trust' }, + { name: 'Bristol Hospital', organisation: 'Bristol Hospital' }, + { name: 'Cardiff Health Board', organisation: 'Cardiff Health Board' }, + { name: 'Derbyshire Healthcare', organisation: 'Derbyshire Healthcare' }, + { name: 'East Sussex NHS Trust', organisation: 'East Sussex NHS Trust' } + ] + + req.organisations = datasetteResponse + prepareGetOrganisationsTemplateParams(req, res, next) + + const expectedTemplatePrams = { + alphabetisedOrgs: { + A: [ + { name: 'Aardvark Healthcare', organisation: 'Aardvark Healthcare' } + ], + B: [ + { name: 'Bath NHS Trust', organisation: 'Bath NHS Trust' }, + { name: 'Bristol Hospital', organisation: 'Bristol Hospital' } + ], + C: [ + { name: 'Cardiff Health Board', organisation: 'Cardiff Health Board' } + ], + D: [ + { name: 'Derbyshire Healthcare', organisation: 'Derbyshire Healthcare' } + ], + E: [ + { name: 'East Sussex NHS Trust', organisation: 'East Sussex NHS Trust' } + ] + } + } + + expect(req.templateParams).toEqual(expectedTemplatePrams) + + expect(next).toHaveBeenCalledOnce() + }) + }) + it('should call render with the find page', async () => { + const req = {} + const res = { render: vi.fn() } + const next = vi.fn() + + req.templateParams = { + alphabetisedOrgs: { + A: [ + { name: 'Aardvark Healthcare', organisation: 'Aardvark Healthcare' } + ], + B: [ + { name: 'Bath NHS Trust', organisation: 'Bath NHS Trust' }, + { name: 'Bristol Hospital', organisation: 'Bristol Hospital' } + ], + C: [ + { name: 'Cardiff Health Board', organisation: 'Cardiff Health Board' } + ], + D: [ + { name: 'Derbyshire Healthcare', organisation: 'Derbyshire Healthcare' } + ], + E: [ + { name: 'East Sussex NHS Trust', organisation: 'East Sussex NHS Trust' } + ] + } + } + + getOrganisations(req, res, next) + + expect(res.render).toHaveBeenCalledTimes(1) + expect(res.render).toHaveBeenCalledWith('organisations/find.html', req.templateParams) + }) +}) diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js index d1a6c6df..b933478e 100644 --- a/test/unit/organisationsController.test.js +++ b/test/unit/organisationsController.test.js @@ -63,62 +63,4 @@ describe('OrganisationsController.js', () => { })) }) }) - - describe('find', () => { - it('should call render with the find page', async () => { - const req = {} - const res = { render: vi.fn() } - const next = vi.fn() - - req.organisations = [] - organisationsController.prepareGetOrganisationsTemplateParams(req, res, next) - organisationsController.getOrganisations(req, res, next) - - expect(res.render).toHaveBeenCalledTimes(1) - expect(res.render).toHaveBeenCalledWith('organisations/find.html', expect.objectContaining({ - alphabetisedOrgs: {} - })) - }) - - it('should correctly sort and restructure the data recieved from datasette, then pass it on to the template', async () => { - const req = {} - const res = { render: vi.fn() } - const next = vi.fn() - - const datasetteResponse = [ - { name: 'Aardvark Healthcare', organisation: 'Aardvark Healthcare' }, - { name: 'Bath NHS Trust', organisation: 'Bath NHS Trust' }, - { name: 'Bristol Hospital', organisation: 'Bristol Hospital' }, - { name: 'Cardiff Health Board', organisation: 'Cardiff Health Board' }, - { name: 'Derbyshire Healthcare', organisation: 'Derbyshire Healthcare' }, - { name: 'East Sussex NHS Trust', organisation: 'East Sussex NHS Trust' } - ] - - req.organisations = datasetteResponse - organisationsController.prepareGetOrganisationsTemplateParams(req, res, next) - organisationsController.getOrganisations(req, res, next) - - expect(res.render).toHaveBeenCalledTimes(1) - expect(res.render).toHaveBeenCalledWith('organisations/find.html', expect.objectContaining({ - alphabetisedOrgs: { - A: [ - { name: 'Aardvark Healthcare', organisation: 'Aardvark Healthcare' } - ], - B: [ - { name: 'Bath NHS Trust', organisation: 'Bath NHS Trust' }, - { name: 'Bristol Hospital', organisation: 'Bristol Hospital' } - ], - C: [ - { name: 'Cardiff Health Board', organisation: 'Cardiff Health Board' } - ], - D: [ - { name: 'Derbyshire Healthcare', organisation: 'Derbyshire Healthcare' } - ], - E: [ - { name: 'East Sussex NHS Trust', organisation: 'East Sussex NHS Trust' } - ] - } - })) - }) - }) }) From b949105f4ad8f8446fca99e400446786102601cf Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 25 Sep 2024 15:06:40 +0100 Subject: [PATCH 45/65] moved overview middleware tests to their own file --- src/middleware/overview.middleware.js | 4 +- .../organisations.middleware.test.js | 2 +- .../middleware/overview.middleware.test.js | 81 +++++++++++++++++++ test/unit/organisationsController.test.js | 66 --------------- 4 files changed, 84 insertions(+), 69 deletions(-) create mode 100644 test/unit/middleware/overview.middleware.test.js delete mode 100644 test/unit/organisationsController.test.js diff --git a/src/middleware/overview.middleware.js b/src/middleware/overview.middleware.js index af951eb6..483927e1 100644 --- a/src/middleware/overview.middleware.js +++ b/src/middleware/overview.middleware.js @@ -32,7 +32,7 @@ async function fetchLpaOverview (req, res, next) { } } -function prepareOverviewTemplateParams (req, res, next) { +export function prepareOverviewTemplateParams (req, res, next) { const { lpaOverview, orgInfo: organisation } = req const datasets = Object.entries(lpaOverview.datasets).map(([key, value]) => { @@ -82,7 +82,7 @@ function prepareOverviewTemplateParams (req, res, next) { next() } -const getOverview = renderTemplate({ +export const getOverview = renderTemplate({ templateParams (req) { if (!req.templateParams) throw new Error('missing templateParams') return req.templateParams diff --git a/test/unit/middleware/organisations.middleware.test.js b/test/unit/middleware/organisations.middleware.test.js index e9b456a0..9650e533 100644 --- a/test/unit/middleware/organisations.middleware.test.js +++ b/test/unit/middleware/organisations.middleware.test.js @@ -46,7 +46,7 @@ describe('organisations.middleware.js', () => { expect(next).toHaveBeenCalledOnce() }) }) - it('should call render with the find page', async () => { + it('should call render with the find page and correct template params', async () => { const req = {} const res = { render: vi.fn() } const next = vi.fn() diff --git a/test/unit/middleware/overview.middleware.test.js b/test/unit/middleware/overview.middleware.test.js new file mode 100644 index 00000000..2b538bf8 --- /dev/null +++ b/test/unit/middleware/overview.middleware.test.js @@ -0,0 +1,81 @@ +import { describe, it, vi, expect } from 'vitest' +import { getOverview, prepareOverviewTemplateParams } from '../../../src/middleware/overview.middleware' + +// vi.mock('../../../src/services/performanceDbApi.js') + +vi.mock('../../../src/utils/utils.js', () => { + return { + dataSubjects: {} + } +}) + +describe('overview.middleware', () => { + const exampleLpa = { + formattedData: [ + { name: 'Example LPA', organisation: 'LPA' } + ] + } + + const perfDbApiResponse = { + name: 'test LPA', + datasets: { + dataset1: { endpoint: 'https://example.com', status: 'Live' }, + dataset2: { endpoint: null, status: 'Needs fixing' }, + dataset3: { endpoint: 'https://example.com', status: 'Error' } + } + } + + describe('prepareOverviewTemplateParams', () => { + it('should render the overview page', async () => { + const req = { + params: { lpa: 'LPA' }, + orgInfo: exampleLpa.formattedData[0], + lpaOverview: perfDbApiResponse + } + const res = { render: vi.fn() } + + prepareOverviewTemplateParams(req, res, () => {}) + + const expectedTemplateParams = { + organisation: { name: 'Example LPA', organisation: 'LPA' }, + datasets: expect.arrayContaining([ + { endpoint: 'https://example.com', status: 'Live', slug: 'dataset1' }, + { endpoint: null, status: 'Needs fixing', slug: 'dataset2' }, + { endpoint: 'https://example.com', status: 'Error', slug: 'dataset3' } + ]), + totalDatasets: 3, + datasetsWithEndpoints: 2, + datasetsWithIssues: 1, + datasetsWithErrors: 1 + } + + expect(req.templateParams).toEqual(expectedTemplateParams) + }) + }) + + describe('getOverview', () => { + it('should call render with the correct template and params', () => { + const req = {} + const res = { render: vi.fn() } + const next = vi.fn() + + req.templateParams = { + organisation: { name: 'Example LPA', organisation: 'LPA' }, + datasets: [ + { endpoint: 'https://example.com', status: 'Live', slug: 'dataset1' }, + { endpoint: null, status: 'Needs fixing', slug: 'dataset2' }, + { endpoint: 'https://example.com', status: 'Error', slug: 'dataset3' } + ], + totalDatasets: 3, + datasetsWithEndpoints: 2, + datasetsWithIssues: 1, + datasetsWithErrors: 1 + } + + getOverview(req, res, next) + + expect(res.render).toHaveBeenCalledTimes(1) + expect(res.render).toHaveBeenCalledWith('organisations/overview.html', req.templateParams) + }) + }) +}) diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js deleted file mode 100644 index b933478e..00000000 --- a/test/unit/organisationsController.test.js +++ /dev/null @@ -1,66 +0,0 @@ -import { describe, it, vi, expect, beforeEach } from 'vitest' -import organisationsController from '../../src/controllers/OrganisationsController.js' - -vi.mock('../../src/services/performanceDbApi.js') -vi.mock('../../src/utils/utils.js', () => { - return { - dataSubjects: {} - } -}) - -vi.mock('../../src/services/datasette.js', () => ({ - default: { - runQuery: vi.fn() - } -})) -vi.mock('../../src/utils/issueMessages/getDatasetTaskList.js') - -describe('OrganisationsController.js', () => { - beforeEach(() => { - vi.resetAllMocks() - }) - - describe('overview', () => { - const exampleLpa = { - formattedData: [ - { name: 'Example LPA', organisation: 'LPA' } - ] - } - - const perfDbApiResponse = { - name: 'test LPA', - datasets: { - dataset1: { endpoint: 'https://example.com', status: 'Live' }, - dataset2: { endpoint: null, status: 'Needs fixing' }, - dataset3: { endpoint: 'https://example.com', status: 'Error' } - } - } - - it('should render the overview page', async () => { - const req = { - params: { lpa: 'LPA' }, - orgInfo: exampleLpa.formattedData[0], - lpaOverview: perfDbApiResponse - } - const res = { render: vi.fn() } - const next = vi.fn() - - organisationsController.prepareOverviewTemplateParams(req, res, () => {}) - organisationsController.getOverview(req, res, next) - - expect(res.render).toHaveBeenCalledTimes(1) - expect(res.render).toHaveBeenCalledWith('organisations/overview.html', expect.objectContaining({ - organisation: { name: 'Example LPA', organisation: 'LPA' }, - datasets: expect.arrayContaining([ - { endpoint: 'https://example.com', status: 'Live', slug: 'dataset1' }, - { endpoint: null, status: 'Needs fixing', slug: 'dataset2' }, - { endpoint: 'https://example.com', status: 'Error', slug: 'dataset3' } - ]), - totalDatasets: 3, - datasetsWithEndpoints: 2, - datasetsWithIssues: 1, - datasetsWithErrors: 1 - })) - }) - }) -}) From b0d99c58344404503e6a415c26faa6596184461a Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 25 Sep 2024 15:34:28 +0100 Subject: [PATCH 46/65] format dates with govuk datetime --- src/views/organisations/dataset-overview.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/views/organisations/dataset-overview.html b/src/views/organisations/dataset-overview.html index 608b868f..add912b9 100644 --- a/src/views/organisations/dataset-overview.html +++ b/src/views/organisations/dataset-overview.html @@ -118,7 +118,7 @@ text: 'Data URL last accessed' }, value: { - text: endpoint.lastAccessed + text: endpoint.lastAccessed | govukDateTime } } %} {% endif %} @@ -131,7 +131,7 @@ text: 'Data URL last updated' }, value: { - text: endpoint.lastUpdated + text: endpoint.lastUpdated | govukDateTime } } %} {{ rows.push(lastUpdatedRow) }} From 8992a3fc5cdb38d16a5aa43798b046f2bedd5075 Mon Sep 17 00:00:00 2001 From: Dilwoar Hussain Date: Wed, 25 Sep 2024 15:38:43 +0100 Subject: [PATCH 47/65] Show overview page unless not started --- src/views/organisations/overview.html | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/views/organisations/overview.html b/src/views/organisations/overview.html index bd200bcd..a0862a24 100644 --- a/src/views/organisations/overview.html +++ b/src/views/organisations/overview.html @@ -69,16 +69,11 @@

Datasets