From 6b789d2ca288070fe2e1bc93445b1fcf16d2fe87 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 2 Aug 2024 15:37:02 +0100 Subject: [PATCH 1/6] reduce wait time between updating list filter --- src/assets/js/list-filter.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/assets/js/list-filter.js b/src/assets/js/list-filter.js index edc18066..02b0a2d9 100644 --- a/src/assets/js/list-filter.js +++ b/src/assets/js/list-filter.js @@ -5,6 +5,8 @@ /* eslint-disable no-var */ //= require govuk_publishing_components/vendor/polyfills/closest +const keyPauseTime = 20 + window.GOVUK = window.GOVUK || {} window.GOVUK.Modules = window.GOVUK.Modules || {}; @@ -34,7 +36,7 @@ window.GOVUK.Modules = window.GOVUK.Modules || {}; clearTimeout(this.filterTimeout) this.filterTimeout = setTimeout(function () { this.$module.filterList(searchTerm) - }.bind(this), 200) + }.bind(this), keyPauseTime) }.bind(this)) } From 6c75e2e282ed007c121eb58b83eea006a1662983 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 2 Aug 2024 15:37:19 +0100 Subject: [PATCH 2/6] remove line --- src/views/organisations/find.html | 1 - 1 file changed, 1 deletion(-) diff --git a/src/views/organisations/find.html b/src/views/organisations/find.html index 8f169529..15609fd2 100644 --- a/src/views/organisations/find.html +++ b/src/views/organisations/find.html @@ -28,7 +28,6 @@

{{ pageName }}

- {% for letter, orgs in alphabetisedOrgs %}
From d8b66fb6e492c29ac263430c04a42be73ba39200 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 2 Aug 2024 15:39:01 +0100 Subject: [PATCH 3/6] get organisations from datasette --- src/controllers/OrganisationsController.js | 97 +++++----------------- 1 file changed, 22 insertions(+), 75 deletions(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 5b65911c..301c717c 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -1,3 +1,4 @@ +import datasette from '../services/datasette.js' 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' @@ -70,85 +71,31 @@ const organisationsController = { } }, + /** + * Handles the GET /organisations request + * + * @param {Request} req + * @param {Response} res + * @param {NextFunction} next + */ async getOrganisations (req, res, next) { - const alphabetisedOrgs = { - A: [ - { - name: 'Aberdeen' - }, - { - name: 'Aylesbury' - }, - { - name: 'Ashford' - } - ], - B: [ - { - name: 'Bath' - }, - { - name: 'Birmingham' - }, - { - name: 'Brighton' - } - ], - C: [ - { - name: 'Cambridge' - }, - { - name: 'Cardiff' - }, - { - name: 'Cheltenham' - }, - { - name: 'Chester' - } - ], - D: [ - { - name: 'Derby' - }, - { - name: 'Dundee' - } - ], - E: [ - { - name: 'Edinburgh' - }, - { - name: 'Epsom' - } - ], - G: [ - { - name: 'Glasgow' - }, - { - name: 'Gloucester' - } - ], - H: [ - { - name: 'Hull' - } - ], - L: [ - { - name: 'Leeds' - }, - { - name: 'London' - } - ] - } + const sql = 'select name, organisation from organisation' + const result = await datasette.runQuery(sql) + + const sortedResults = result.formattedData.sort((a, b) => { + return a.name.localeCompare(b.name) + }) + + const alphabetisedOrgs = sortedResults.reduce((acc, current) => { + const firstLetter = current.name.charAt(0).toUpperCase() + acc[firstLetter] = acc[firstLetter] || [] + acc[firstLetter].push(current) + return acc + }, {}) res.render('organisations/find.html', { alphabetisedOrgs }) } + } export default organisationsController From f626da0388b83cf3742632bf662100e221c1e635 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 2 Aug 2024 15:58:01 +0100 Subject: [PATCH 4/6] updated tests --- src/controllers/OrganisationsController.js | 29 ++++---- test/unit/organisationsController.test.js | 77 ++++++++++++++++++++-- 2 files changed, 89 insertions(+), 17 deletions(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 301c717c..5c3c16fe 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -79,21 +79,26 @@ const organisationsController = { * @param {NextFunction} next */ async getOrganisations (req, res, next) { - const sql = 'select name, organisation from organisation' - const result = await datasette.runQuery(sql) + try { + const sql = 'select name, organisation from organisation' + const result = await datasette.runQuery(sql) - const sortedResults = result.formattedData.sort((a, b) => { - return a.name.localeCompare(b.name) - }) + const sortedResults = result.formattedData.sort((a, b) => { + return a.name.localeCompare(b.name) + }) - const alphabetisedOrgs = sortedResults.reduce((acc, current) => { - const firstLetter = current.name.charAt(0).toUpperCase() - acc[firstLetter] = acc[firstLetter] || [] - acc[firstLetter].push(current) - return acc - }, {}) + const alphabetisedOrgs = sortedResults.reduce((acc, current) => { + const firstLetter = current.name.charAt(0).toUpperCase() + acc[firstLetter] = acc[firstLetter] || [] + acc[firstLetter].push(current) + return acc + }, {}) - res.render('organisations/find.html', { alphabetisedOrgs }) + res.render('organisations/find.html', { alphabetisedOrgs }) + } catch (err) { + logger.error(err) + next(err) + } } } diff --git a/test/unit/organisationsController.test.js b/test/unit/organisationsController.test.js index a36c3be7..67dbc892 100644 --- a/test/unit/organisationsController.test.js +++ b/test/unit/organisationsController.test.js @@ -1,6 +1,7 @@ import { describe, it, vi, expect, beforeEach } from 'vitest' -import LpaOverviewController from '../../src/controllers/OrganisationsController.js' +import organisationsController from '../../src/controllers/OrganisationsController.js' import performanceDbApi from '../../src/services/performanceDbApi.js' +import datasette from '../../src/services/datasette.js' vi.mock('../../src/services/performanceDbApi.js') vi.mock('../../src/utils/utils.js', () => { @@ -8,6 +9,7 @@ vi.mock('../../src/utils/utils.js', () => { dataSubjects: {} } }) +vi.mock('../../src/services/datasette.js') describe('OrganisationsController.js', () => { beforeEach(() => { @@ -31,7 +33,7 @@ describe('OrganisationsController.js', () => { performanceDbApi.getLpaOverview = vi.fn().mockResolvedValue(expectedResponse) - await LpaOverviewController.getOverview(req, res, next) + await organisationsController.getOverview(req, res, next) expect(res.render).toHaveBeenCalledTimes(1) expect(res.render).toHaveBeenCalledWith('organisations/overview.html', expect.objectContaining({ @@ -57,7 +59,7 @@ describe('OrganisationsController.js', () => { vi.mocked(performanceDbApi.getLpaOverview).mockRejectedValue(error) - await LpaOverviewController.getOverview(req, res, next) + await organisationsController.getOverview(req, res, next) expect(next).toHaveBeenCalledTimes(1) expect(next).toHaveBeenCalledWith(error) @@ -65,10 +67,75 @@ describe('OrganisationsController.js', () => { }) describe('find', () => { - it.todo('should render the find page', () => { + it('should call render with the find page', async () => { + const req = {} + const res = { render: vi.fn() } + const next = vi.fn() + + vi.mocked(datasette.runQuery).mockResolvedValue({ formattedData: [] }) + + await 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' } + ] + + vi.mocked(datasette.runQuery).mockResolvedValue({ formattedData: datasetteResponse }) + await 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' } + ] + } + })) }) - it.todo('should catch errors and pass them onto the next function') + it('should catch errors and pass them onto the next function', async () => { + const req = {} + const res = {} + const next = vi.fn() + + const error = new Error('Test error') + + vi.mocked(datasette.runQuery).mockRejectedValue(error) + + await organisationsController.getOrganisations(req, res, next) + + expect(next).toHaveBeenCalledTimes(1) + expect(next).toHaveBeenCalledWith(error) + }) }) }) From c798a1e632888d9a28dcf1fcf14263c2938adeed Mon Sep 17 00:00:00 2001 From: George Goodall Date: Fri, 2 Aug 2024 16:05:22 +0100 Subject: [PATCH 5/6] make sure feature deploy workflow is correct --- .github/workflows/featureDeploy.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/featureDeploy.yml b/.github/workflows/featureDeploy.yml index acd59b32..7e77d25c 100644 --- a/.github/workflows/featureDeploy.yml +++ b/.github/workflows/featureDeploy.yml @@ -30,10 +30,10 @@ jobs: strategy: matrix: environment: ${{ fromJSON(needs.detect-environments.outputs.environments) }} - if: ${{ matrix.environment != 'production' }} + if: ${{ inputs.environment != 'production' }} uses: ./.github/workflows/deploy.yml with: - environment: '${{ matrix.environment }}' + environment: '${{ inputs.environment }}' secrets: inherit From 52e23fa423107f28bd04156f6021b67f4a1ae594 Mon Sep 17 00:00:00 2001 From: George Goodall Date: Tue, 6 Aug 2024 12:29:58 +0100 Subject: [PATCH 6/6] Update OrganisationsController.js error->warn --- src/controllers/OrganisationsController.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/OrganisationsController.js b/src/controllers/OrganisationsController.js index 5c3c16fe..c4d388fc 100644 --- a/src/controllers/OrganisationsController.js +++ b/src/controllers/OrganisationsController.js @@ -96,7 +96,7 @@ const organisationsController = { res.render('organisations/find.html', { alphabetisedOrgs }) } catch (err) { - logger.error(err) + logger.warn(err) next(err) } }