From 5e073c0ab7731064c74b32681a4893302e96d147 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 10 Jul 2024 15:07:36 +0100 Subject: [PATCH 01/26] added basic lpa-overview page --- src/assets/scss/index.scss | 60 +++++- src/routes/manage.js | 8 + src/serverSetup/routes.js | 4 +- src/views/manage/lpa-overview.html | 295 +++++++++++++++++++++++++++++ 4 files changed, 365 insertions(+), 2 deletions(-) create mode 100644 src/routes/manage.js create mode 100644 src/views/manage/lpa-overview.html diff --git a/src/assets/scss/index.scss b/src/assets/scss/index.scss index 7f667914..14c1bd62 100644 --- a/src/assets/scss/index.scss +++ b/src/assets/scss/index.scss @@ -43,4 +43,62 @@ $govuk-global-styles: true; .app-map { height: 500px; @include govuk-responsive-margin(6, $direction: "bottom"); -} \ No newline at end of file +} + +.task-div { + margin-bottom: 10px; + padding: 10px; +} + +.dataset-summary-grid { + display: flex; + flex-wrap: wrap; + column-gap: govuk-spacing(3); + row-gap: govuk-spacing(6); + align-items: stretch; +} + +.dataset-summary-grid .govuk-summary-card { + width: calc((100% / 3) - govuk-spacing(3)); +} + +.dataset-summary-grid .govuk-summary-card__title-wrapper { + align-items: flex-start; +} + +.dataset-status { + margin-bottom: govuk-spacing(6); + + display: flex; + column-gap: govuk-spacing(6); +} + +.dataset-status--item { + background-color: $govuk-brand-colour; + color: govuk-colour("white"); + + padding: govuk-spacing(2) govuk-spacing(3); + flex-grow: 1; + flex-basis: 0; + + @include govuk-font($size: 24, $weight: bold); +} + +.big-number { + @include govuk-font($size: 80, $weight: bold); + display: block; +} + +.planning-data-actions { + list-style: none; + margin: 0; + padding: 0; + + display: flex; + column-gap: govuk-spacing(2); +} + +code, +code * { + font-family: monospace; +} diff --git a/src/routes/manage.js b/src/routes/manage.js new file mode 100644 index 00000000..8a97aba9 --- /dev/null +++ b/src/routes/manage.js @@ -0,0 +1,8 @@ +import express from 'express' +const router = express.Router(); + +router.get('/lpa-overview', (req, res) => { + res.render('manage/lpa-overview.html', {}) +}); + +export default router diff --git a/src/serverSetup/routes.js b/src/serverSetup/routes.js index 7d56e5d3..4c570a02 100644 --- a/src/serverSetup/routes.js +++ b/src/serverSetup/routes.js @@ -3,11 +3,13 @@ import endpointSubmissionFormFormWisard from '../routes/form-wizard/endpoint-sub import accessibility from '../routes/accessibility.js' import polling from '../routes/api.js' import health from '../routes/health.js' +import manage from '../routes/manage.js' -export function setupRoutes (app) { +export function setupRoutes (app, { nunjucks }) { app.use('/', checkFormWizard) app.use('/submit', endpointSubmissionFormFormWisard) app.use('/accessibility', accessibility) app.use('/api', polling) app.use('/health', health) + app.use('/manage', manage) } diff --git a/src/views/manage/lpa-overview.html b/src/views/manage/lpa-overview.html new file mode 100644 index 00000000..9de83c0a --- /dev/null +++ b/src/views/manage/lpa-overview.html @@ -0,0 +1,295 @@ +{% extends "layouts/main.html" %} + +{% from "govuk/components/breadcrumbs/macro.njk" import govukBreadcrumbs %} +{% from "govuk/components/tag/macro.njk" import govukTag %} + +{% set pageName = organisation.name + " overview" %} + +{% block beforeContent %} +{{ super() }} + +{{ govukBreadcrumbs({ + items: [ + { + text: "Home", + href: "/overview/start" + }, + { + text: "Organisations", + href: "/overview/organisations" + } + ] +}) }} + +{% endblock %} + +{% block content %} + +
+
+ +

+ {{ pageName }} +

+
+
+ +
+
+
+
+ 7/8 + datasets provided +
+ +
+ 2 + datasets with errors +
+ +
+ 2 + datasets with issues +
+
+
+
+ +
+
+ +

Datasets

+ +
+
+ +
+
+

View your task list to fix and improve your datasets

+ +
+
+ +{% endblock %} \ No newline at end of file From 5b83f4923bfd0e0b30d3ea8db393cb7ba7d2b89b Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 10 Jul 2024 15:07:44 +0100 Subject: [PATCH 02/26] linting --- src/routes/manage.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/routes/manage.js b/src/routes/manage.js index 8a97aba9..7b843091 100644 --- a/src/routes/manage.js +++ b/src/routes/manage.js @@ -1,8 +1,8 @@ import express from 'express' -const router = express.Router(); +const router = express.Router() router.get('/lpa-overview', (req, res) => { - res.render('manage/lpa-overview.html', {}) -}); + res.render('manage/lpa-overview.html', {}) +}) export default router From a46e291248e7adabbe6c49e3f1eed50bc7f859bb Mon Sep 17 00:00:00 2001 From: George Goodall Date: Wed, 10 Jul 2024 15:39:20 +0100 Subject: [PATCH 03/26] remove unused param --- src/serverSetup/routes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/serverSetup/routes.js b/src/serverSetup/routes.js index 4c570a02..ad40b7a6 100644 --- a/src/serverSetup/routes.js +++ b/src/serverSetup/routes.js @@ -5,7 +5,7 @@ import polling from '../routes/api.js' import health from '../routes/health.js' import manage from '../routes/manage.js' -export function setupRoutes (app, { nunjucks }) { +export function setupRoutes (app) { app.use('/', checkFormWizard) app.use('/submit', endpointSubmissionFormFormWisard) app.use('/accessibility', accessibility) From 7cc54a7b75bc6d6997307fe1d04e30284bf7c984 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Thu, 11 Jul 2024 14:05:28 +0100 Subject: [PATCH 04/26] move services to service folder --- src/controllers/CheckAnswersController.js | 2 +- src/controllers/lpaDetailsController.js | 2 +- src/controllers/resultsController.js | 2 +- src/controllers/statusController.js | 2 +- src/controllers/submitUrlController.js | 2 +- src/controllers/uploadFileController.js | 2 +- src/routes/api.js | 2 +- src/{utils => services}/asyncRequestApi.js | 0 src/{utils => services}/fetchLocalAuthorities.js | 0 src/{utils => services}/mailClient.js | 0 test/unit/checkAnswersController.test.js | 4 ++-- test/unit/fetchLocalAuthorities.test.js | 2 +- test/unit/lpaDetailsController.test.js | 4 ++-- test/unit/publishRequestAPI.test.js | 2 +- test/unit/resultsController.test.js | 4 ++-- test/unit/statusController.test.js | 4 ++-- test/unit/submitUrlController.test.js | 4 ++-- test/unit/uploadFileController.test.js | 4 ++-- 18 files changed, 21 insertions(+), 21 deletions(-) rename src/{utils => services}/asyncRequestApi.js (100%) rename src/{utils => services}/fetchLocalAuthorities.js (100%) rename src/{utils => services}/mailClient.js (100%) diff --git a/src/controllers/CheckAnswersController.js b/src/controllers/CheckAnswersController.js index 22766cbe..1f5602e8 100644 --- a/src/controllers/CheckAnswersController.js +++ b/src/controllers/CheckAnswersController.js @@ -1,5 +1,5 @@ import PageController from './pageController.js' -import notifyClient from '../utils/mailClient.js' +import notifyClient from '../services/mailClient.js' import config from '../../config/index.js' const dataManagementEmail = process.env.DATA_MANAGEMENT_EMAIL || config.email.dataManagementEmail diff --git a/src/controllers/lpaDetailsController.js b/src/controllers/lpaDetailsController.js index 6301564a..6d391848 100644 --- a/src/controllers/lpaDetailsController.js +++ b/src/controllers/lpaDetailsController.js @@ -1,5 +1,5 @@ import PageController from './pageController.js' -import { fetchLocalAuthorities } from '../utils/fetchLocalAuthorities.js' +import { fetchLocalAuthorities } from '../services/fetchLocalAuthorities.js' class LpaDetailsController extends PageController { async locals (req, res, next) { diff --git a/src/controllers/resultsController.js b/src/controllers/resultsController.js index 5c4cc7b9..8c290c73 100644 --- a/src/controllers/resultsController.js +++ b/src/controllers/resultsController.js @@ -1,5 +1,5 @@ import PageController from './pageController.js' -import { getRequestData } from '../utils/asyncRequestApi.js' +import { getRequestData } from '../services/asyncRequestApi.js' const failedFileRequestTemplate = 'results/failedFileRequest' const failedUrlRequestTemplate = 'results/failedUrlRequest' diff --git a/src/controllers/statusController.js b/src/controllers/statusController.js index e357b3ab..a5cba75a 100644 --- a/src/controllers/statusController.js +++ b/src/controllers/statusController.js @@ -1,5 +1,5 @@ import PageController from './pageController.js' -import { getRequestData } from '../utils/asyncRequestApi.js' +import { getRequestData } from '../services/asyncRequestApi.js' import { finishedProcessingStatuses } from '../utils/utils.js' class StatusController extends PageController { diff --git a/src/controllers/submitUrlController.js b/src/controllers/submitUrlController.js index 3a06e2a4..18f4d8e9 100644 --- a/src/controllers/submitUrlController.js +++ b/src/controllers/submitUrlController.js @@ -1,5 +1,5 @@ import UploadController from './uploadController.js' -import { postUrlRequest } from '../utils/asyncRequestApi.js' +import { postUrlRequest } from '../services/asyncRequestApi.js' import { URL } from 'url' import logger from '../utils/logger.js' import axios from 'axios' diff --git a/src/controllers/uploadFileController.js b/src/controllers/uploadFileController.js index 8b464e46..8302c353 100644 --- a/src/controllers/uploadFileController.js +++ b/src/controllers/uploadFileController.js @@ -8,7 +8,7 @@ import multer from 'multer' import { promises as fs, createReadStream } from 'fs' import config from '../../config/index.js' import logger from '../utils/logger.js' -import { postFileRequest } from '../utils/asyncRequestApi.js' +import { postFileRequest } from '../services/asyncRequestApi.js' import { allowedFileTypes } from '../utils/utils.js' AWS.config.update({ diff --git a/src/routes/api.js b/src/routes/api.js index d0ce754b..4dc01638 100644 --- a/src/routes/api.js +++ b/src/routes/api.js @@ -1,5 +1,5 @@ import express from 'express' -import { getRequestData } from '../utils/asyncRequestApi.js' +import { getRequestData } from '../services/asyncRequestApi.js' const router = express.Router() diff --git a/src/utils/asyncRequestApi.js b/src/services/asyncRequestApi.js similarity index 100% rename from src/utils/asyncRequestApi.js rename to src/services/asyncRequestApi.js diff --git a/src/utils/fetchLocalAuthorities.js b/src/services/fetchLocalAuthorities.js similarity index 100% rename from src/utils/fetchLocalAuthorities.js rename to src/services/fetchLocalAuthorities.js diff --git a/src/utils/mailClient.js b/src/services/mailClient.js similarity index 100% rename from src/utils/mailClient.js rename to src/services/mailClient.js diff --git a/test/unit/checkAnswersController.test.js b/test/unit/checkAnswersController.test.js index f0988183..9323b253 100644 --- a/test/unit/checkAnswersController.test.js +++ b/test/unit/checkAnswersController.test.js @@ -1,10 +1,10 @@ /* eslint-disable new-cap */ import { describe, it, vi, expect, beforeEach } from 'vitest' -import notifyClient from '../../src/utils/mailClient.js' +import notifyClient from '../../src/services/mailClient.js' import config from '../../config/index.js' -vi.mock('../../src/utils/mailClient.js') +vi.mock('../../src/services/mailClient.js') function makeRequest () { return { diff --git a/test/unit/fetchLocalAuthorities.test.js b/test/unit/fetchLocalAuthorities.test.js index 9b3c73f2..23380347 100644 --- a/test/unit/fetchLocalAuthorities.test.js +++ b/test/unit/fetchLocalAuthorities.test.js @@ -1,6 +1,6 @@ import axios from 'axios' import { vi, it, describe, expect } from 'vitest' -import { fetchLocalAuthorities } from '../../src/utils/fetchLocalAuthorities' +import { fetchLocalAuthorities } from '../../src/services/fetchLocalAuthorities' // Mock axios.get to return a fake response vi.mock('axios') diff --git a/test/unit/lpaDetailsController.test.js b/test/unit/lpaDetailsController.test.js index 21355a83..2d98156b 100644 --- a/test/unit/lpaDetailsController.test.js +++ b/test/unit/lpaDetailsController.test.js @@ -4,14 +4,14 @@ import PageController from '../../src/controllers/pageController.js' import { vi, it, describe, expect, beforeEach, afterEach } from 'vitest' -vi.mock('../../src/utils/fetchLocalAuthorities.js') +vi.mock('../../src/services/fetchLocalAuthorities.js') describe('lpaDetailsController', async () => { let fetchLocalAuthorities let controller beforeEach(async () => { - fetchLocalAuthorities = await import('../../src/utils/fetchLocalAuthorities') + fetchLocalAuthorities = await import('../../src/services/fetchLocalAuthorities') const LpaDetailsController = await import('../../src/controllers/lpaDetailsController.js') controller = new LpaDetailsController.default({ route: '/lpa-details' diff --git a/test/unit/publishRequestAPI.test.js b/test/unit/publishRequestAPI.test.js index 70cda772..da200154 100644 --- a/test/unit/publishRequestAPI.test.js +++ b/test/unit/publishRequestAPI.test.js @@ -1,5 +1,5 @@ import { it, describe, expect, afterEach, vi } from 'vitest' -import { postFileRequest, postUrlRequest, getRequestData } from '../../src/utils/asyncRequestApi.js' +import { postFileRequest, postUrlRequest, getRequestData } from '../../src/services/asyncRequestApi.js' import axios from 'axios' import RequestData from '../../src/models/requestData.js' import config from '../../config/index.js' diff --git a/test/unit/resultsController.test.js b/test/unit/resultsController.test.js index 7d95eb5e..6a2400ad 100644 --- a/test/unit/resultsController.test.js +++ b/test/unit/resultsController.test.js @@ -2,7 +2,7 @@ import ResultsController from '../../src/controllers/resultsController.js' import { describe, it, vi, expect, beforeEach } from 'vitest' describe('ResultsController', () => { - vi.mock('@/utils/asyncRequestApi.js') + vi.mock('@/services/asyncRequestApi.js') let asyncRequestApi let resultsController @@ -14,7 +14,7 @@ describe('ResultsController', () => { } beforeEach(async () => { - asyncRequestApi = await import('@/utils/asyncRequestApi') + asyncRequestApi = await import('@/services/asyncRequestApi') resultsController = new ResultsController({ route: '/results' diff --git a/test/unit/statusController.test.js b/test/unit/statusController.test.js index 19280175..d5ceaea5 100644 --- a/test/unit/statusController.test.js +++ b/test/unit/statusController.test.js @@ -2,13 +2,13 @@ import StatusController from '../../src/controllers/statusController.js' import { describe, it, vi, expect, beforeEach } from 'vitest' describe('StatusController', () => { - vi.mock('@/utils/asyncRequestApi.js') + vi.mock('@/services/asyncRequestApi.js') let asyncRequestApi let statusController beforeEach(async () => { - asyncRequestApi = await import('@/utils/asyncRequestApi') + asyncRequestApi = await import('@/services/asyncRequestApi') statusController = new StatusController({ route: '/status' diff --git a/test/unit/submitUrlController.test.js b/test/unit/submitUrlController.test.js index 6b31e205..6f84397f 100644 --- a/test/unit/submitUrlController.test.js +++ b/test/unit/submitUrlController.test.js @@ -3,7 +3,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import SubmitUrlController from '../../src/controllers/submitUrlController.js' describe('SubmitUrlController', async () => { - vi.mock('@/utils/asyncRequestApi.js') + vi.mock('@/services/asyncRequestApi.js') let submitUrlController let asyncRequestApi @@ -23,7 +23,7 @@ describe('SubmitUrlController', async () => { }) beforeEach(async () => { - asyncRequestApi = await import('@/utils/asyncRequestApi') + asyncRequestApi = await import('@/services/asyncRequestApi') asyncRequestApi.postUrlRequest = vi.fn() submitUrlController = new SubmitUrlController({ diff --git a/test/unit/uploadFileController.test.js b/test/unit/uploadFileController.test.js index beff51f6..2ae08dd7 100644 --- a/test/unit/uploadFileController.test.js +++ b/test/unit/uploadFileController.test.js @@ -6,7 +6,7 @@ describe('UploadFileController', () => { let uploadFileController let asyncRequestApi - vi.mock('@/utils/asyncRequestApi.js') + vi.mock('@/services/asyncRequestApi.js') vi.mock('fs', async () => { const actual = await vi.importActual('fs') @@ -36,7 +36,7 @@ describe('UploadFileController', () => { }) beforeEach(async () => { - asyncRequestApi = await import('@/utils/asyncRequestApi') + asyncRequestApi = await import('@/services/asyncRequestApi') asyncRequestApi.postFileRequest = vi.fn().mockResolvedValue('1234') uploadFileController = new UploadFileController({ From 085e917759cbdbca118965073b4b5005ae50ed98 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Thu, 11 Jul 2024 14:09:28 +0100 Subject: [PATCH 05/26] added a controller and performanceDbApi service --- src/controllers/LpaOverviewController.js | 15 +++++++++++++++ src/routes/manage.js | 6 +++--- src/services/performanceDbApi.js | 5 +++++ 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 src/controllers/LpaOverviewController.js create mode 100644 src/services/performanceDbApi.js diff --git a/src/controllers/LpaOverviewController.js b/src/controllers/LpaOverviewController.js new file mode 100644 index 00000000..be3e4e98 --- /dev/null +++ b/src/controllers/LpaOverviewController.js @@ -0,0 +1,15 @@ +import performanceDbApi from '../services/performanceDbApi' // Assume you have an API service module + +class LpaOverviewController { + async getOverview (req, res, next) { + try { + const response = await performanceDbApi.getLpaOverview() // Make API request + const data = response.data + res.render('manage/lpa-overview.html', { data }) + } catch (error) { + next(error) + } + } +} + +export default LpaOverviewController diff --git a/src/routes/manage.js b/src/routes/manage.js index 7b843091..ed19175b 100644 --- a/src/routes/manage.js +++ b/src/routes/manage.js @@ -1,8 +1,8 @@ import express from 'express' +import LpaOverviewController from '../controllers/LpaOverviewController' + const router = express.Router() -router.get('/lpa-overview', (req, res) => { - res.render('manage/lpa-overview.html', {}) -}) +router.get('/lpa-overview', LpaOverviewController.getOverview) export default router diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js new file mode 100644 index 00000000..ef33bab9 --- /dev/null +++ b/src/services/performanceDbApi.js @@ -0,0 +1,5 @@ +export default { + getLpaOverview: async () => { + return {} + } +} \ No newline at end of file From ed3091b86300b915d0d55d4d0a7da0272357389d Mon Sep 17 00:00:00 2001 From: George Goodall Date: Thu, 11 Jul 2024 14:09:37 +0100 Subject: [PATCH 06/26] linting --- src/services/performanceDbApi.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index ef33bab9..2dbe8d81 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -1,5 +1,5 @@ export default { - getLpaOverview: async () => { - return {} - } -} \ No newline at end of file + getLpaOverview: async () => { + return {} + } +} From 0a489c5a0cde89ca0a58f779970275495c0253c6 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Thu, 11 Jul 2024 15:09:36 +0100 Subject: [PATCH 07/26] now get mock data from the mock api, and use it to populate the template --- src/controllers/LpaOverviewController.js | 30 ++- src/routes/manage.js | 4 +- src/services/performanceDbApi.js | 46 +++- src/views/manage/lpa-overview.html | 290 ++++++----------------- 4 files changed, 142 insertions(+), 228 deletions(-) diff --git a/src/controllers/LpaOverviewController.js b/src/controllers/LpaOverviewController.js index be3e4e98..2e902e8c 100644 --- a/src/controllers/LpaOverviewController.js +++ b/src/controllers/LpaOverviewController.js @@ -1,11 +1,33 @@ -import performanceDbApi from '../services/performanceDbApi' // Assume you have an API service module +import performanceDbApi from '../services/performanceDbApi.js' // Assume you have an API service module -class LpaOverviewController { +const LpaOverviewController = { async getOverview (req, res, next) { try { - const response = await performanceDbApi.getLpaOverview() // Make API request + const lpa = req.params.lpa + + const response = await performanceDbApi.getLpaOverview(lpa) // Make API request const data = response.data - res.render('manage/lpa-overview.html', { data }) + + const datasets = Object.entries(data.datasets).map(([key, value]) => { + return { ...value, slug: key } + }) + const totalDatasets = datasets.length + const datasetsWithEndpoints = datasets.filter(item => item.endpoint !== null).length + const datasetsWithIssues = datasets.filter(item => item.issue).length + const datasetsWithErrors = datasets.filter(item => item.error).length + + const params = { + organisation: { + name: data.name + }, + datasets, + totalDatasets, + datasetsWithEndpoints, + datasetsWithIssues, + datasetsWithErrors + } + + res.render('manage/lpa-overview.html', params) } catch (error) { next(error) } diff --git a/src/routes/manage.js b/src/routes/manage.js index ed19175b..a4cb31eb 100644 --- a/src/routes/manage.js +++ b/src/routes/manage.js @@ -1,8 +1,8 @@ import express from 'express' -import LpaOverviewController from '../controllers/LpaOverviewController' +import LpaOverviewController from '../controllers/LpaOverviewController.js' const router = express.Router() -router.get('/lpa-overview', LpaOverviewController.getOverview) +router.get('/:lpa/overview', LpaOverviewController.getOverview) export default router diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 2dbe8d81..c111a15b 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -1,5 +1,47 @@ export default { - getLpaOverview: async () => { - return {} + getLpaOverview: async (lpa) => { + return { + data: { + name: 'Borechester City Council', + datasets: { + 'article-4-direction': { + endpoint: null + }, + 'article-4-direction-area': { + endpoint: null + }, + 'conservation-area': { + endpoint: 'http://conservation-area.json', + error: null, + issue: 'Endpoint has not been updated since 21 May 2023' + }, + 'conservation-area-document': { + endpoint: 'http://conservation-area-document.json', + error: null, + issue: null + }, + 'listed-building-outline': { + endpoint: 'http://listed-building-outline.json', + error: null, + issue: null + }, + tree: { + endpoint: 'http://tree.json', + error: null, + issue: 'There are 20 issues in this dataset' + }, + 'tree-preservation-order': { + endpoint: 'http://tree-preservation-order.json', + error: 'Error connecting to endpoint', + issue: null + }, + 'tree-preservation-zone': { + endpoint: 'http://tree-preservation-zone.json', + error: 'Error connecting to endpoint', + issue: null + } + } + } + } } } diff --git a/src/views/manage/lpa-overview.html b/src/views/manage/lpa-overview.html index 9de83c0a..fd44e1ca 100644 --- a/src/views/manage/lpa-overview.html +++ b/src/views/manage/lpa-overview.html @@ -38,17 +38,17 @@

- 7/8 + {{datasetsWithEndpoints}}/{{totalDatasets}} datasets provided
- 2 + {{datasetsWithErrors}} datasets with errors
- 2 + {{datasetsWithIssues}} datasets with issues
@@ -60,227 +60,78 @@

Datasets

-
- {{govukTag({ - text: "Issues", - classes: "govuk-tag--yellow" - })}} -
- - -
  • -
    -

    - - Tree preservation order - -

    -
    -

    Error connecting to endpoint

    - - +
    + {% if dataset.endpoint is null %} + {{govukTag({ + text: "Not provided", + classes: "govuk-tag--grey" + })}} + {% elif dataset.error %} + {{govukTag({ + text: "Error", + classes: "govuk-tag--red" + })}} + {% elif dataset.issue %} + {{govukTag({ + text: "Issues", + classes: "govuk-tag--yellow" + })}} + {% else %} + {{govukTag({ + text: "No issues", + classes: "govuk-tag--green" + })}} + {% endif %}
    -
    - -
    - {{govukTag({ - text: "Error", - classes: "govuk-tag--red" - })}} -
    -
    • - -
  • -
    -

    - - Tree preservation zone - -

    -
    -

    Error connecting to endpoint

    +
    • + {% endfor %} - - - - -
    - {{govukTag({ - text: "Error", - classes: "govuk-tag--red" - })}} -
    - @@ -288,7 +139,6 @@

    View your task list to fix and improve your datasets

    -
    From 3599715e591a05183895486529d2fd0afec74385 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 11:45:45 +0100 Subject: [PATCH 08/26] add make dataset slug to readable name filter factory function and move into separate file for easier testing --- src/filters/filters.js | 25 ++---------- .../makeDatasetSlugToReadableNameFilter.js | 38 +++++++++++++++++++ 2 files changed, 42 insertions(+), 21 deletions(-) create mode 100644 src/filters/makeDatasetSlugToReadableNameFilter.js diff --git a/src/filters/filters.js b/src/filters/filters.js index e9e8e5bd..c8db12ca 100644 --- a/src/filters/filters.js +++ b/src/filters/filters.js @@ -4,33 +4,16 @@ import validationMessageLookup from './validationMessageLookup.js' import toErrorList from './toErrorList.js' import prettifyColumnName from './prettifyColumnName.js' import getFullServiceName from './getFullServiceName.js' +import { makeDatasetSlugToReadableNameFilter, createDatasetMapping } from './makeDatasetSlugToReadableNameFilter.js' const { govukMarkdown } = xGovFilters -/** - * - * @param {*} dataSubjects - * @returns {Map} - */ -function createDatasetMapping (dataSubjects) { - const mapping = new Map() - for (const data of Object.values(dataSubjects)) { - for (const dataset of data.dataSets) { - mapping.set(dataset.value, dataset.text) - } - } - return mapping -} + const addFilters = (nunjucksEnv, { dataSubjects }) => { const datasetNameMapping = createDatasetMapping(dataSubjects) - nunjucksEnv.addFilter('datasetSlugToReadableName', function (slug) { - const name = datasetNameMapping.get(slug) - if (!name) { - throw new Error(`Can't find a name for ${slug}`) - } - return name - }) + const datasetSlugToReadableName = makeDatasetSlugToReadableNameFilter(datasetNameMapping) + nunjucksEnv.addFilter('datasetSlugToReadableName', datasetSlugToReadableName) nunjucksEnv.addFilter('govukMarkdown', govukMarkdown) nunjucksEnv.addFilter('getkeys', getkeys) diff --git a/src/filters/makeDatasetSlugToReadableNameFilter.js b/src/filters/makeDatasetSlugToReadableNameFilter.js new file mode 100644 index 00000000..76e1f86b --- /dev/null +++ b/src/filters/makeDatasetSlugToReadableNameFilter.js @@ -0,0 +1,38 @@ +/** + * Creates a filter function that takes a dataset slug as input and returns its corresponding readable name. + * The filter function uses a provided dataset name mapping to look up the readable name. + * + * @param {Map} datasetNameMapping - A map of dataset slugs to their corresponding readable names. + * @returns {(slug: string) => string} - A filter function that takes a dataset slug as input and returns its corresponding readable name. + */ +export const makeDatasetSlugToReadableNameFilter = (datasetNameMapping) => { + /** + * A filter function that takes a dataset slug as input and returns its corresponding readable name. + * + * @param {string} slug - The dataset slug to look up. + * @returns {string} - The readable name corresponding to the provided slug. + * @throws {Error} - If the provided slug is not found in the dataset name mapping. + */ + return (slug) => { + const name = datasetNameMapping.get(slug) + if (!name) { + throw new Error(`Can't find a name for ${slug}`) + } + return name + } +} + +/** + * + * @param {*} dataSubjects + * @returns {Map} + */ +export const createDatasetMapping = (dataSubjects) => { + const mapping = new Map() + for (const data of Object.values(dataSubjects)) { + for (const dataset of data.dataSets) { + mapping.set(dataset.value, dataset.text) + } + } + return mapping +} \ No newline at end of file From 96f3e9189c1ecdac68ef72a3e101705bba8dc541 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 11:45:57 +0100 Subject: [PATCH 09/26] linting --- src/filters/filters.js | 2 -- src/filters/makeDatasetSlugToReadableNameFilter.js | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/filters/filters.js b/src/filters/filters.js index c8db12ca..18c06ba9 100644 --- a/src/filters/filters.js +++ b/src/filters/filters.js @@ -8,8 +8,6 @@ import { makeDatasetSlugToReadableNameFilter, createDatasetMapping } from './mak const { govukMarkdown } = xGovFilters - - const addFilters = (nunjucksEnv, { dataSubjects }) => { const datasetNameMapping = createDatasetMapping(dataSubjects) const datasetSlugToReadableName = makeDatasetSlugToReadableNameFilter(datasetNameMapping) diff --git a/src/filters/makeDatasetSlugToReadableNameFilter.js b/src/filters/makeDatasetSlugToReadableNameFilter.js index 76e1f86b..d0358669 100644 --- a/src/filters/makeDatasetSlugToReadableNameFilter.js +++ b/src/filters/makeDatasetSlugToReadableNameFilter.js @@ -1,14 +1,14 @@ /** * Creates a filter function that takes a dataset slug as input and returns its corresponding readable name. * The filter function uses a provided dataset name mapping to look up the readable name. - * + * * @param {Map} datasetNameMapping - A map of dataset slugs to their corresponding readable names. * @returns {(slug: string) => string} - A filter function that takes a dataset slug as input and returns its corresponding readable name. */ export const makeDatasetSlugToReadableNameFilter = (datasetNameMapping) => { /** * A filter function that takes a dataset slug as input and returns its corresponding readable name. - * + * * @param {string} slug - The dataset slug to look up. * @returns {string} - The readable name corresponding to the provided slug. * @throws {Error} - If the provided slug is not found in the dataset name mapping. @@ -35,4 +35,4 @@ export const createDatasetMapping = (dataSubjects) => { } } return mapping -} \ No newline at end of file +} From 62078a10f4bf498804de3564531e3a5379787430 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 11:46:15 +0100 Subject: [PATCH 10/26] minor template tweeks --- src/views/manage/lpa-overview.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/views/manage/lpa-overview.html b/src/views/manage/lpa-overview.html index fd44e1ca..194bbe18 100644 --- a/src/views/manage/lpa-overview.html +++ b/src/views/manage/lpa-overview.html @@ -4,6 +4,7 @@ {% from "govuk/components/tag/macro.njk" import govukTag %} {% set pageName = organisation.name + " overview" %} +{% set serviceType = 'Manage' %} {% block beforeContent %} {{ super() }} @@ -89,7 +90,7 @@

    {% endif %} {% if dataset.error %}
  • - Fix Errors + Fix errors
    • {% elseif dataset.issue %}
  • From 3eeecfe9f7a472b3419dcabb093dbd94b0c29832 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 12:00:26 +0100 Subject: [PATCH 11/26] added template unit tests for lpa overview page --- test/unit/lpaOverviewPage.test.js | 158 ++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 test/unit/lpaOverviewPage.test.js diff --git a/test/unit/lpaOverviewPage.test.js b/test/unit/lpaOverviewPage.test.js new file mode 100644 index 00000000..b12efc6b --- /dev/null +++ b/test/unit/lpaOverviewPage.test.js @@ -0,0 +1,158 @@ +import { describe, it, expect } from 'vitest' +import config from '../../config/index.js' +import nunjucks from 'nunjucks' +import addFilters from '../../src/filters/filters' +import { runGenericPageTests } from './generic-page.js' +import jsdom from 'jsdom' +import { dataSubjects } from '../../src/utils/utils.js' +import { makeDatasetSlugToReadableNameFilter, createDatasetMapping } from '../../src/filters/makeDatasetSlugToReadableNameFilter.js' + +const nunjucksEnv = nunjucks.configure([ + 'src/views', + 'src/views/check', + 'src/views/submit', + 'node_modules/govuk-frontend/dist/', + 'node_modules/@x-govuk/govuk-prototype-components/' +], { + dev: true, + noCache: true, + watch: true +}) + +addFilters(nunjucksEnv, { dataSubjects }) + +describe('LPA Overview Page', () => { + const params = { + organisation: { + name: 'mock org' + }, + serviceName: config.serviceName, + datasetsWithEndpoints: 2, + totalDatasets: 8, + datasetsWithErrors: 2, + datasetsWithIssues: 2, + datasets: [ + { + slug: 'article-4-direction', + endpoint: null + }, + { + slug: 'article-4-direction-area', + endpoint: null + }, + { + slug: 'conservation-area', + endpoint: 'http://conservation-area.json', + error: null, + issue: 'Endpoint has not been updated since 21 May 2023' + }, + { + slug: 'conservation-area-document', + endpoint: 'http://conservation-area-document.json', + error: null, + issue: null + }, + { + slug: 'listed-building-outline', + endpoint: 'http://listed-building-outline.json', + error: null, + issue: null + }, + { + slug: 'tree', + endpoint: 'http://tree.json', + error: null, + issue: 'There are 20 issues in this dataset' + }, + { + slug: 'tree-preservation-order', + endpoint: 'http://tree-preservation-order.json', + error: 'Error connecting to endpoint', + issue: null + }, + { + slug: 'tree-preservation-zone', + endpoint: 'http://tree-preservation-zone.json', + error: 'Error connecting to endpoint', + issue: null + } + ] + } + const html = nunjucks.render('manage/lpa-overview.html', params) + + const dom = new jsdom.JSDOM(html) + const document = dom.window.document + + runGenericPageTests(html, { + pageTitle: 'mock org overview - Manage planning and housing data for England', + serviceName: config.serviceName + }) + + const statsBoxes = document.querySelector('.dataset-status').children + it('Datasets provided gives the correct value', () => { + expect(statsBoxes[0].textContent).toContain('2/8') + expect(statsBoxes[0].textContent).toContain('datasets provided') + }) + + it('Datasets with errors gives the correct value', () => { + expect(statsBoxes[1].textContent).toContain('2') + expect(statsBoxes[1].textContent).toContain('datasets with errors') + }) + + it('Datasets with issues gives the correct value', () => { + expect(statsBoxes[2].textContent).toContain('2') + expect(statsBoxes[2].textContent).toContain('datasets with issues') + }) + + const datasetCards = document.querySelector('.govuk-task-list').children + it('The correct number of dataset cards are rendered with the correct titles', () => { + expect(datasetCards.length).toEqual(params.datasets.length) + + const datasetMapping = createDatasetMapping(dataSubjects) + const datasetSlugToReadableName = makeDatasetSlugToReadableNameFilter(datasetMapping) + + params.datasets.forEach((dataset, i) => { + expect(datasetCards[i].querySelector('.govuk-heading-m').textContent).toContain(datasetSlugToReadableName(dataset.slug)) + }) + }) + + it('The dataset cards are rendered with the correct hints', () => { + params.datasets.forEach((dataset, i) => { + const expectedHint = !dataset.endpoint ? 'Endpoint not provided' : dataset.error ? dataset.error : dataset.issue ? dataset.issue : 'Endpoint provided' + expect(datasetCards[i].querySelector('.govuk-task-list__hint').textContent).toContain(expectedHint) + }) + }) + + it('Renders the correct actions on each dataset card', () => { + params.datasets.forEach((dataset, i) => { + const expectedActions = [] + if (!dataset.endpoint) { + expectedActions.push({ text: 'Add endpoint', href: '/taskLists/taskChecklist' }) + } + if (dataset.error) { + expectedActions.push({ text: 'Fix errors', href: '/taskLists/taskChecklist' }) + } else if (dataset.issue) { + expectedActions.push({ text: 'Fix issues', href: '/taskLists/taskChecklist' }) + } + if (dataset.endpoint) { + expectedActions.push({ text: 'View data', href: '/taskLists/taskChecklist' }) + } + + const actions = datasetCards[i].querySelector('.planning-data-actions').children + expectedActions.forEach((expectedAction, j) => { + expect(actions[j].textContent, `expect action ${expectedAction.text} for dataset ${dataset.slug}`).toContain(expectedAction.text) + const actionLink = actions[j].querySelector('a') + expect(actionLink.href).toBe(expectedAction.href) + }) + }) + }) + + it('Renders the correct status on each dataset card', () => { + params.datasets.forEach((dataset, i) => { + const expectedHint = !dataset.endpoint ? 'Not provided' : dataset.error ? 'Error' : dataset.issue ? 'Issues' : 'No issues' + + const statusIndicator = datasetCards[i].querySelector('.govuk-task-list__status') + expect(statusIndicator.textContent).toContain(expectedHint) + }) + }) +}) From 1273183e59f2f372e284f269a4d724ae434e5492 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 13:32:07 +0100 Subject: [PATCH 12/26] added tests for the lpa overview controller --- test/unit/lpaOverviewController.test.js | 61 +++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 test/unit/lpaOverviewController.test.js diff --git a/test/unit/lpaOverviewController.test.js b/test/unit/lpaOverviewController.test.js new file mode 100644 index 00000000..01b98349 --- /dev/null +++ b/test/unit/lpaOverviewController.test.js @@ -0,0 +1,61 @@ +import { describe, it, vi, expect, beforeEach } from 'vitest'; +import LpaOverviewController from '../../src/controllers/LpaOverviewController.js'; +import performanceDbApi from '../../src/services/performanceDbApi.js'; + +vi.mock('../../src/services/performanceDbApi.js') + +describe('LpaOverviewController', () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it('should render the lpa overview page', async () => { + const req = { params: { lpa: 'test-lpa' } }; + const res = { render: vi.fn() }; + const next = vi.fn(); + + const expectedResponse = { + data: { + name: 'Test LPA', + datasets: { + dataset1: { endpoint: 'https://example.com', issue: false, error: false }, + dataset2: { endpoint: null, issue: true, error: false }, + dataset3: { endpoint: 'https://example.com', issue: false, error: true }, + }, + }, + }; + + performanceDbApi.getLpaOverview = vi.fn().mockResolvedValue(expectedResponse); + + await LpaOverviewController.getOverview(req, res, next); + + expect(res.render).toHaveBeenCalledTimes(1); + expect(res.render).toHaveBeenCalledWith('manage/lpa-overview.html', expect.objectContaining({ + organisation: { name: 'Test LPA' }, + datasets: expect.arrayContaining([ + { endpoint: 'https://example.com', issue: false, error: false, slug: 'dataset1' }, + { endpoint: null, issue: true, error: false, slug: 'dataset2' }, + { endpoint: 'https://example.com', issue: false, error: true, slug: 'dataset3' }, + ]), + totalDatasets: 3, + datasetsWithEndpoints: 2, + datasetsWithIssues: 1, + datasetsWithErrors: 1, + })); + }); + + it('should catch and pass errors to the next function', async () => { + const req = { params: { lpa: 'test-lpa' } }; + const res = { }; + const next = vi.fn(); + + const error = new Error('Test error'); + + vi.mocked(performanceDbApi.getLpaOverview).mockRejectedValue(error); + + await LpaOverviewController.getOverview(req, res, next); + + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(error); + }); +}); From 0fe19e345a2099fc6fddabf0c19ce236f0239a65 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 13:32:19 +0100 Subject: [PATCH 13/26] linting --- test/unit/lpaOverviewController.test.js | 58 ++++++++++++------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/test/unit/lpaOverviewController.test.js b/test/unit/lpaOverviewController.test.js index 01b98349..d55223d1 100644 --- a/test/unit/lpaOverviewController.test.js +++ b/test/unit/lpaOverviewController.test.js @@ -1,18 +1,18 @@ -import { describe, it, vi, expect, beforeEach } from 'vitest'; -import LpaOverviewController from '../../src/controllers/LpaOverviewController.js'; -import performanceDbApi from '../../src/services/performanceDbApi.js'; +import { describe, it, vi, expect, beforeEach } from 'vitest' +import LpaOverviewController from '../../src/controllers/LpaOverviewController.js' +import performanceDbApi from '../../src/services/performanceDbApi.js' vi.mock('../../src/services/performanceDbApi.js') describe('LpaOverviewController', () => { beforeEach(() => { - vi.resetAllMocks(); - }); + vi.resetAllMocks() + }) it('should render the lpa overview page', async () => { - const req = { params: { lpa: 'test-lpa' } }; - const res = { render: vi.fn() }; - const next = vi.fn(); + const req = { params: { lpa: 'test-lpa' } } + const res = { render: vi.fn() } + const next = vi.fn() const expectedResponse = { data: { @@ -20,42 +20,42 @@ describe('LpaOverviewController', () => { datasets: { dataset1: { endpoint: 'https://example.com', issue: false, error: false }, dataset2: { endpoint: null, issue: true, error: false }, - dataset3: { endpoint: 'https://example.com', issue: false, error: true }, - }, - }, - }; + dataset3: { endpoint: 'https://example.com', issue: false, error: true } + } + } + } - performanceDbApi.getLpaOverview = vi.fn().mockResolvedValue(expectedResponse); + performanceDbApi.getLpaOverview = vi.fn().mockResolvedValue(expectedResponse) - await LpaOverviewController.getOverview(req, res, next); + await LpaOverviewController.getOverview(req, res, next) - expect(res.render).toHaveBeenCalledTimes(1); + expect(res.render).toHaveBeenCalledTimes(1) expect(res.render).toHaveBeenCalledWith('manage/lpa-overview.html', expect.objectContaining({ organisation: { name: 'Test LPA' }, datasets: expect.arrayContaining([ { endpoint: 'https://example.com', issue: false, error: false, slug: 'dataset1' }, { endpoint: null, issue: true, error: false, slug: 'dataset2' }, - { endpoint: 'https://example.com', issue: false, error: true, slug: 'dataset3' }, + { endpoint: 'https://example.com', issue: false, error: true, slug: 'dataset3' } ]), totalDatasets: 3, datasetsWithEndpoints: 2, datasetsWithIssues: 1, - datasetsWithErrors: 1, - })); - }); + datasetsWithErrors: 1 + })) + }) it('should catch and pass errors to the next function', async () => { - const req = { params: { lpa: 'test-lpa' } }; - const res = { }; - const next = vi.fn(); + const req = { params: { lpa: 'test-lpa' } } + const res = { } + const next = vi.fn() - const error = new Error('Test error'); + const error = new Error('Test error') - vi.mocked(performanceDbApi.getLpaOverview).mockRejectedValue(error); + vi.mocked(performanceDbApi.getLpaOverview).mockRejectedValue(error) - await LpaOverviewController.getOverview(req, res, next); + await LpaOverviewController.getOverview(req, res, next) - expect(next).toHaveBeenCalledTimes(1); - expect(next).toHaveBeenCalledWith(error); - }); -}); + expect(next).toHaveBeenCalledTimes(1) + expect(next).toHaveBeenCalledWith(error) + }) +}) From 7901b6f1d8e138eeed441c03b7887a209a85e38a Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 13:46:57 +0100 Subject: [PATCH 14/26] change in response to rolands pr comment --- src/controllers/LpaOverviewController.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/controllers/LpaOverviewController.js b/src/controllers/LpaOverviewController.js index 2e902e8c..0a9e0c0c 100644 --- a/src/controllers/LpaOverviewController.js +++ b/src/controllers/LpaOverviewController.js @@ -1,4 +1,5 @@ import performanceDbApi from '../services/performanceDbApi.js' // Assume you have an API service module +import logger from '../utils/logger.js' const LpaOverviewController = { async getOverview (req, res, next) { @@ -12,9 +13,12 @@ const LpaOverviewController = { return { ...value, slug: key } }) const totalDatasets = datasets.length - const datasetsWithEndpoints = datasets.filter(item => item.endpoint !== null).length - const datasetsWithIssues = datasets.filter(item => item.issue).length - const datasetsWithErrors = datasets.filter(item => item.error).length + const [datasetsWithEndpoints, datasetsWithIssues, datasetsWithErrors] = datasets.reduce((accumulator, dataset) => { + if (dataset.endpoint !== null) accumulator[0]++ + if (dataset.issue) accumulator[1]++ + if (dataset.error) accumulator[2]++ + return accumulator + }, [0, 0, 0]) const params = { organisation: { @@ -29,6 +33,7 @@ const LpaOverviewController = { res.render('manage/lpa-overview.html', params) } catch (error) { + logger.error(error) next(error) } } From 9f28f4ee2a999526ec45cb7d6b5c79c33a548f9a Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 14:45:27 +0100 Subject: [PATCH 15/26] get data from datasette --- src/controllers/LpaOverviewController.js | 7 +- .../makeDatasetSlugToReadableNameFilter.js | 7 +- src/services/datasette.js | 19 +++ src/services/performanceDbApi.js | 119 ++++++++++++------ 4 files changed, 107 insertions(+), 45 deletions(-) create mode 100644 src/services/datasette.js diff --git a/src/controllers/LpaOverviewController.js b/src/controllers/LpaOverviewController.js index 0a9e0c0c..e216c7e1 100644 --- a/src/controllers/LpaOverviewController.js +++ b/src/controllers/LpaOverviewController.js @@ -6,10 +6,9 @@ const LpaOverviewController = { try { const lpa = req.params.lpa - const response = await performanceDbApi.getLpaOverview(lpa) // Make API request - const data = response.data + const lpaOverview = await performanceDbApi.getLpaOverview(lpa) // Make API request - const datasets = Object.entries(data.datasets).map(([key, value]) => { + const datasets = Object.entries(lpaOverview.datasets).map(([key, value]) => { return { ...value, slug: key } }) const totalDatasets = datasets.length @@ -22,7 +21,7 @@ const LpaOverviewController = { const params = { organisation: { - name: data.name + name: lpaOverview.name }, datasets, totalDatasets, diff --git a/src/filters/makeDatasetSlugToReadableNameFilter.js b/src/filters/makeDatasetSlugToReadableNameFilter.js index d0358669..07c6d752 100644 --- a/src/filters/makeDatasetSlugToReadableNameFilter.js +++ b/src/filters/makeDatasetSlugToReadableNameFilter.js @@ -1,3 +1,5 @@ +import logger from '../utils/logger.js' + /** * Creates a filter function that takes a dataset slug as input and returns its corresponding readable name. * The filter function uses a provided dataset name mapping to look up the readable name. @@ -16,7 +18,10 @@ export const makeDatasetSlugToReadableNameFilter = (datasetNameMapping) => { return (slug) => { const name = datasetNameMapping.get(slug) if (!name) { - throw new Error(`Can't find a name for ${slug}`) + // throw new Error(`Can't find a name for ${slug}`) + // ToDo: work out what to do here? potentially update it with data from datasette + logger.error(`can't find a name for ${slug}`) + return slug } return name } diff --git a/src/services/datasette.js b/src/services/datasette.js new file mode 100644 index 00000000..137146e7 --- /dev/null +++ b/src/services/datasette.js @@ -0,0 +1,19 @@ +import axios from 'axios' +import logger from '../utils/logger.js' + +const datasetteUrl = 'https://datasette.planning.data.gov.uk' +const database = 'digital-land' + +export default { + runQuery: async (query) => { + const encodedQuery = encodeURIComponent(query) + const url = `${datasetteUrl}/${database}.json?sql=${encodedQuery}` + try { + const response = await axios.get(url) + return response.data + } catch (error) { + logger.error(error) + throw error + } + } +} diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index c111a15b..58db6b5c 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -1,47 +1,86 @@ +import datasette from './datasette.js' + +/* + +*/ + export default { - getLpaOverview: async (lpa) => { + getLpaOverviewOld: async (lpa) => { return { - data: { - name: 'Borechester City Council', - datasets: { - 'article-4-direction': { - endpoint: null - }, - 'article-4-direction-area': { - endpoint: null - }, - 'conservation-area': { - endpoint: 'http://conservation-area.json', - error: null, - issue: 'Endpoint has not been updated since 21 May 2023' - }, - 'conservation-area-document': { - endpoint: 'http://conservation-area-document.json', - error: null, - issue: null - }, - 'listed-building-outline': { - endpoint: 'http://listed-building-outline.json', - error: null, - issue: null - }, - tree: { - endpoint: 'http://tree.json', - error: null, - issue: 'There are 20 issues in this dataset' - }, - 'tree-preservation-order': { - endpoint: 'http://tree-preservation-order.json', - error: 'Error connecting to endpoint', - issue: null - }, - 'tree-preservation-zone': { - endpoint: 'http://tree-preservation-zone.json', - error: 'Error connecting to endpoint', - issue: null - } + name: 'Borechester City Council', + datasets: { + 'article-4-direction': { + endpoint: null + }, + 'article-4-direction-area': { + endpoint: null + }, + 'conservation-area': { + endpoint: 'http://conservation-area.json', + error: null, + issue: 'Endpoint has not been updated since 21 May 2023' + }, + 'conservation-area-document': { + endpoint: 'http://conservation-area-document.json', + error: null, + issue: null + }, + 'listed-building-outline': { + endpoint: 'http://listed-building-outline.json', + error: null, + issue: null + }, + tree: { + endpoint: 'http://tree.json', + error: null, + issue: 'There are 20 issues in this dataset' + }, + 'tree-preservation-order': { + endpoint: 'http://tree-preservation-order.json', + error: 'Error connecting to endpoint', + issue: null + }, + 'tree-preservation-zone': { + endpoint: 'http://tree-preservation-zone.json', + error: 'Error connecting to endpoint', + issue: null } } } + }, + + getLpaOverview: async (lpa) => { + const query = + `SELECT + p.organisation, + o.name, + p.dataset, + rle.endpoint + FROM + provision p + INNER JOIN + organisation o ON o.organisation = p.organisation + LEFT JOIN + reporting_latest_endpoints rle + ON REPLACE(rle.organisation, '-eng', '') = p.organisation + AND rle.pipeline = p.dataset + WHERE p.organisation = '${lpa}' + ORDER BY + p.organisation, + o.name` + + const result = await datasette.runQuery(query) + + const datasets = result.rows.reduce((accumulator, row) => { + accumulator[row[2]] = { + endpoint: row[3] + } + return accumulator + }, {}) + + return { + name: result.rows[0][1], + datasets + } } } From 4605bcab7c671ceee41a06b4dd086bd566ed816d Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 14:59:28 +0100 Subject: [PATCH 16/26] now get errors with endpoints --- src/services/performanceDbApi.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 58db6b5c..d7ffa4ca 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -55,7 +55,9 @@ export default { p.organisation, o.name, p.dataset, - rle.endpoint + rle.endpoint, + rle.status, + rle.exception FROM provision p INNER JOIN @@ -72,8 +74,13 @@ export default { const result = await datasette.runQuery(query) const datasets = result.rows.reduce((accumulator, row) => { + let error + if (row[4] !== 200 || row[5] !== '') { + error = row[5] !== '' ? row[5] : `endpoint returned with a status of ${row[4]}` + } accumulator[row[2]] = { - endpoint: row[3] + endpoint: row[3], + error } return accumulator }, {}) From 85100c1347afb290b76ed34b0d2537736bef4e6d Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 15:40:01 +0100 Subject: [PATCH 17/26] updated with samritis new query: now showing issues --- src/services/performanceDbApi.js | 103 +++++++++++++++++++++++-------- 1 file changed, 77 insertions(+), 26 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index d7ffa4ca..33cfac2f 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -50,37 +50,88 @@ export default { }, getLpaOverview: async (lpa) => { - const query = - `SELECT - p.organisation, - o.name, - p.dataset, - rle.endpoint, - rle.status, - rle.exception - FROM - provision p - INNER JOIN - organisation o ON o.organisation = p.organisation - LEFT JOIN - reporting_latest_endpoints rle - ON REPLACE(rle.organisation, '-eng', '') = p.organisation - AND rle.pipeline = p.dataset - WHERE p.organisation = '${lpa}' - ORDER BY - p.organisation, - o.name` + const query = ` + SELECT + p.organisation, + o.name, + p.dataset, + rle.pipeline, + rle.endpoint, + rle.resource, + rle.exception, + rle.status as http_status, + case + when (rle.status != '200') then 'Error' + when (it.severity = 'error') then 'Issue' + when (it.severity = 'warning') then 'Warning' + else 'No issues' + end as status, + case + when (it.severity = 'info') then '' + else i.issue_type + end as issue_type, + case + when (it.severity = 'info') then '' + else it.severity + end as severity, + it.responsibility, + COUNT( + case + when it.severity != 'info' then 1 + else null + end + ) as issue_count +FROM + provision p +LEFT JOIN + organisation o ON o.organisation = p.organisation +LEFT JOIN + reporting_latest_endpoints rle + ON REPLACE(rle.organisation, '-eng', '') = p.organisation + AND rle.pipeline = p.dataset +LEFT JOIN + issue i ON rle.resource = i.resource AND rle.pipeline = i.dataset +LEFT JOIN + issue_type it ON i.issue_type = it.issue_type AND it.severity != 'info' +WHERE + p.organisation = '${lpa}' +GROUP BY + p.organisation, + p.dataset, + o.name, + rle.pipeline, + rle.endpoint +ORDER BY + p.organisation, + o.name; +` const result = await datasette.runQuery(query) - const datasets = result.rows.reduce((accumulator, row) => { + // convert the rows into an easier to access format + const columns = result.columns + const rows = result.rows.map((row) => { + return row.reduce((acc, val, index) => { + acc[columns[index]] = val; + return acc; + }, {}); + }) + + const datasets = rows.reduce((accumulator, row) => { let error - if (row[4] !== 200 || row[5] !== '') { - error = row[5] !== '' ? row[5] : `endpoint returned with a status of ${row[4]}` + if (row.http_status !== '200' || row.exception !== '') { + error = row.exception !== '' ? row.exception : `endpoint returned with a status of ${row.http_status}` + } + + let issue + if (row.issue_count > 0){ + issue = `There are ${row.issue_count} issues in this dataset` } - accumulator[row[2]] = { - endpoint: row[3], - error + + accumulator[row.dataset] = { + endpoint: row.endpoint, + error, + issue } return accumulator }, {}) From f7ede6c3ad02395eb71222d0d2978c595238d5c2 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 15:40:21 +0100 Subject: [PATCH 18/26] linting --- src/services/performanceDbApi.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 33cfac2f..747b56d1 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -112,9 +112,9 @@ ORDER BY const columns = result.columns const rows = result.rows.map((row) => { return row.reduce((acc, val, index) => { - acc[columns[index]] = val; - return acc; - }, {}); + acc[columns[index]] = val + return acc + }, {}) }) const datasets = rows.reduce((accumulator, row) => { @@ -124,7 +124,7 @@ ORDER BY } let issue - if (row.issue_count > 0){ + if (row.issue_count > 0) { issue = `There are ${row.issue_count} issues in this dataset` } From dbb045189e9cd2343eeab692febec8e5316a2f70 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 16:27:46 +0100 Subject: [PATCH 19/26] refactor lpa overview controller and add js doc --- src/controllers/LpaOverviewController.js | 27 ++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/controllers/LpaOverviewController.js b/src/controllers/LpaOverviewController.js index e216c7e1..b665493f 100644 --- a/src/controllers/LpaOverviewController.js +++ b/src/controllers/LpaOverviewController.js @@ -1,16 +1,35 @@ import performanceDbApi from '../services/performanceDbApi.js' // Assume you have an API service module import logger from '../utils/logger.js' +import { dataSubjects } from '../utils/utils.js' + +// get a list of available datasets +const availableDatasets = Object.values(dataSubjects) + .flatMap(dataSubject => + dataSubject.dataSets + .filter(dataset => dataset.available) + .map(dataset => dataset.value) + ) const LpaOverviewController = { + /** + * Get LPA overview data and render the overview page + * @param {Request} req - Express request object + * @param {Response} res - Express response object + * @param {NextFunction} next - Express next function + * @returns {Promise} - Returns a promise that resolves when the overview page is rendered + */ async getOverview (req, res, next) { try { const lpa = req.params.lpa - const lpaOverview = await performanceDbApi.getLpaOverview(lpa) // Make API request + // Make API request + const lpaOverview = await performanceDbApi.getLpaOverview(lpa) + + const datasets = availableDatasets.map((dataset) => ({ + slug: dataset, + ...(lpaOverview.datasets[dataset] || { endpoint: null }) + })) - const datasets = Object.entries(lpaOverview.datasets).map(([key, value]) => { - return { ...value, slug: key } - }) const totalDatasets = datasets.length const [datasetsWithEndpoints, datasetsWithIssues, datasetsWithErrors] = datasets.reduce((accumulator, dataset) => { if (dataset.endpoint !== null) accumulator[0]++ From 7609f6e1a174b6fbcccd7c5389f864597eedf577 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 16:27:57 +0100 Subject: [PATCH 20/26] remove old method --- src/services/performanceDbApi.js | 44 -------------------------------- 1 file changed, 44 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index 747b56d1..a299fa5f 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -5,50 +5,6 @@ import datasette from './datasette.js' */ export default { - getLpaOverviewOld: async (lpa) => { - return { - name: 'Borechester City Council', - datasets: { - 'article-4-direction': { - endpoint: null - }, - 'article-4-direction-area': { - endpoint: null - }, - 'conservation-area': { - endpoint: 'http://conservation-area.json', - error: null, - issue: 'Endpoint has not been updated since 21 May 2023' - }, - 'conservation-area-document': { - endpoint: 'http://conservation-area-document.json', - error: null, - issue: null - }, - 'listed-building-outline': { - endpoint: 'http://listed-building-outline.json', - error: null, - issue: null - }, - tree: { - endpoint: 'http://tree.json', - error: null, - issue: 'There are 20 issues in this dataset' - }, - 'tree-preservation-order': { - endpoint: 'http://tree-preservation-order.json', - error: 'Error connecting to endpoint', - issue: null - }, - 'tree-preservation-zone': { - endpoint: 'http://tree-preservation-zone.json', - error: 'Error connecting to endpoint', - issue: null - } - } - } - }, - getLpaOverview: async (lpa) => { const query = ` SELECT From 8511c19a54c6b45ad89aa3b5049feef75e25843e Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 16:29:46 +0100 Subject: [PATCH 21/26] add jsdoc to performanceDbApi --- src/services/performanceDbApi.js | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/services/performanceDbApi.js b/src/services/performanceDbApi.js index a299fa5f..17fe893a 100644 --- a/src/services/performanceDbApi.js +++ b/src/services/performanceDbApi.js @@ -1,10 +1,32 @@ +/** + * Performance DB API service + */ import datasette from './datasette.js' -/* +/** + * @typedef {object} Dataset + * @property {string} endpoint + * @property {?string} error + * @property {?string} issue + */ -*/ +/** + * @typedef {object} LpaOverview + * @property {string} name + * @property {{ [dataset: string]: Dataset }} datasets + */ +/** + * Performance DB API service + * @export + * @default + */ export default { + /** + * Get LPA overview + * @param {string} lpa - LPA ID + * @returns {Promise} LPA overview + */ getLpaOverview: async (lpa) => { const query = ` SELECT From 3434d835290311bc207fd17ba27153a7537810d4 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 12 Jul 2024 16:55:11 +0100 Subject: [PATCH 22/26] update to show all available datasets and include 8 essential ones --- src/controllers/LpaOverviewController.js | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/controllers/LpaOverviewController.js b/src/controllers/LpaOverviewController.js index b665493f..497624e4 100644 --- a/src/controllers/LpaOverviewController.js +++ b/src/controllers/LpaOverviewController.js @@ -25,10 +25,24 @@ const LpaOverviewController = { // Make API request const lpaOverview = await performanceDbApi.getLpaOverview(lpa) - const datasets = availableDatasets.map((dataset) => ({ - slug: dataset, - ...(lpaOverview.datasets[dataset] || { endpoint: null }) - })) + // restructure datasets to usable format + const datasets = Object.entries(lpaOverview.datasets).map(([key, value]) => { + return { + slug: key, + ...value + } + }) + + // add in any of the missing key 8 datasets + const keys = Object.keys(lpaOverview.datasets) + availableDatasets.forEach(dataset => { + if (!keys.includes(dataset)) { + datasets.push({ + slug: dataset, + endpoint: null + }) + } + }) const totalDatasets = datasets.length const [datasetsWithEndpoints, datasetsWithIssues, datasetsWithErrors] = datasets.reduce((accumulator, dataset) => { From 05447ab0ba4f93ad94981ef8fe30576b9311fccb Mon Sep 17 00:00:00 2001 From: George Goodall Date: Mon, 15 Jul 2024 11:25:30 +0100 Subject: [PATCH 23/26] started work on fixing tests --- test/unit/lpaOverviewController.test.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/test/unit/lpaOverviewController.test.js b/test/unit/lpaOverviewController.test.js index d55223d1..4af280b2 100644 --- a/test/unit/lpaOverviewController.test.js +++ b/test/unit/lpaOverviewController.test.js @@ -15,13 +15,11 @@ describe('LpaOverviewController', () => { const next = vi.fn() const expectedResponse = { - data: { - name: 'Test LPA', - datasets: { - dataset1: { endpoint: 'https://example.com', issue: false, error: false }, - dataset2: { endpoint: null, issue: true, error: false }, - dataset3: { endpoint: 'https://example.com', issue: false, error: true } - } + name: 'Test LPA', + datasets: { + dataset1: { endpoint: 'https://example.com', issue: false, error: false }, + dataset2: { endpoint: null, issue: true, error: false }, + dataset3: { endpoint: 'https://example.com', issue: false, error: true } } } From fd96dd45d060359f3b3111e9308368698029ae5a Mon Sep 17 00:00:00 2001 From: George Goodall Date: Mon, 15 Jul 2024 11:55:29 +0100 Subject: [PATCH 24/26] fix test --- test/unit/lpaOverviewController.test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/unit/lpaOverviewController.test.js b/test/unit/lpaOverviewController.test.js index 4af280b2..073b5c6d 100644 --- a/test/unit/lpaOverviewController.test.js +++ b/test/unit/lpaOverviewController.test.js @@ -3,6 +3,11 @@ import LpaOverviewController from '../../src/controllers/LpaOverviewController.j import performanceDbApi from '../../src/services/performanceDbApi.js' vi.mock('../../src/services/performanceDbApi.js') +vi.mock('../../src/utils/utils.js', () => { + return { + dataSubjects: {} + } +}) describe('LpaOverviewController', () => { beforeEach(() => { From b87b9fca4d2252e147309344c739fcad3fa69e9a Mon Sep 17 00:00:00 2001 From: George Goodall Date: Thu, 1 Aug 2024 09:25:44 +0100 Subject: [PATCH 25/26] update logger level from error to warning --- src/filters/makeDatasetSlugToReadableNameFilter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filters/makeDatasetSlugToReadableNameFilter.js b/src/filters/makeDatasetSlugToReadableNameFilter.js index 07c6d752..3180a360 100644 --- a/src/filters/makeDatasetSlugToReadableNameFilter.js +++ b/src/filters/makeDatasetSlugToReadableNameFilter.js @@ -20,7 +20,7 @@ export const makeDatasetSlugToReadableNameFilter = (datasetNameMapping) => { if (!name) { // throw new Error(`Can't find a name for ${slug}`) // ToDo: work out what to do here? potentially update it with data from datasette - logger.error(`can't find a name for ${slug}`) + logger.warning(`can't find a name for ${slug}`) return slug } return name From 7eb9136ae1596c5e0c135c58df516de556c9e2a1 Mon Sep 17 00:00:00 2001 From: Roland Sadowski Date: Thu, 1 Aug 2024 09:51:16 +0100 Subject: [PATCH 26/26] log 'warning' instead of 'error' --- src/services/datasette.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/datasette.js b/src/services/datasette.js index 137146e7..f7307a99 100644 --- a/src/services/datasette.js +++ b/src/services/datasette.js @@ -12,7 +12,7 @@ export default { const response = await axios.get(url) return response.data } catch (error) { - logger.error(error) + logger.warn(error) throw error } }