From 330eb07912f013b102a279fa9fc9d439913d7e51 Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Mon, 29 Jun 2026 14:30:33 +0100 Subject: [PATCH 1/9] feat: Implement endpoint duplication check in CheckConfirmationController - Update CheckConfirmationController to check if an endpoint is already being collected for a dataset. - Modify confirmation.html to display appropriate messages based on endpoint collection status. - Add unit tests for CheckConfirmationController and endpointAlreadyCollectedForDataset functionality. - Introduce endpointAlreadyCollectedForDataset utility to query the database for existing endpoints. --- .../checkConfirmationController.js | 33 +++++- .../endpointAlreadyCollected.js | 24 ++++ src/views/check/confirmation.html | 28 ++++- test/unit/check/confirmationPage.test.js | 39 +++++++ test/unit/checkConfirmationController.test.js | 106 ++++++++++++++++++ .../utils/endpointAlreadyCollected.test.js | 54 +++++++++ 6 files changed, 281 insertions(+), 3 deletions(-) create mode 100644 src/utils/datasetteQueries/endpointAlreadyCollected.js create mode 100644 test/unit/checkConfirmationController.test.js create mode 100644 test/unit/utils/endpointAlreadyCollected.test.js diff --git a/src/controllers/checkConfirmationController.js b/src/controllers/checkConfirmationController.js index b08339075..773febb5e 100644 --- a/src/controllers/checkConfirmationController.js +++ b/src/controllers/checkConfirmationController.js @@ -1,12 +1,41 @@ import PageController from './pageController.js' +import { getRequestData } from '../services/asyncRequestApi.js' +import { endpointAlreadyCollectedForDataset } from '../utils/datasetteQueries/endpointAlreadyCollected.js' +import logger from '../utils/logger.js' +import { types } from '../utils/logging.js' class CheckConfirmationController extends PageController { - locals (req, res, next) { + async locals (req, res, next) { const isUrlCheck = req.sessionModel.get('upload-method') === 'url' if (isUrlCheck) { const requestId = req.sessionModel.get('request_id') req.form.options.requestId = requestId - req.session.checkRequestId = requestId + try { + const requestData = await getRequestData(requestId) + const params = requestData.getParams() ?? {} + if (params.dataset) { + req.sessionModel.set('dataset', params.dataset) + } + if (params.organisationName) { + req.sessionModel.set('orgId', params.organisationName) + } + req.form.options.alreadyCollectingEndpoint = await endpointAlreadyCollectedForDataset({ + endpointUrl: params.url, + dataset: params.dataset + }) + } catch (error) { + logger.warn('CheckConfirmationController: could not check whether endpoint is already collected', { + type: types.App, + requestId, + errorMessage: error.message + }) + } + + if (req.form.options.alreadyCollectingEndpoint) { + delete req.session.checkRequestId + } else { + req.session.checkRequestId = requestId + } } super.locals(req, res, next) } diff --git a/src/utils/datasetteQueries/endpointAlreadyCollected.js b/src/utils/datasetteQueries/endpointAlreadyCollected.js new file mode 100644 index 000000000..f08796dd5 --- /dev/null +++ b/src/utils/datasetteQueries/endpointAlreadyCollected.js @@ -0,0 +1,24 @@ +import datasette from '../../services/datasette.js' + +function sqlString (value) { + return String(value).replaceAll("'", "''") +} + +export async function endpointAlreadyCollectedForDataset ({ endpointUrl, dataset }) { + if (!endpointUrl || !dataset) return false + + const sql = /* sql */ ` + SELECT 1 + FROM endpoint e + JOIN resource_endpoint re ON e.endpoint = re.endpoint + JOIN resource r ON re.resource = r.resource + JOIN resource_dataset rd ON r.resource = rd.resource + WHERE e.endpoint_url = '${sqlString(endpointUrl)}' + AND rd.dataset = '${sqlString(dataset)}' + AND (e.end_date IS NULL OR e.end_date = '') + AND (r.end_date IS NULL OR r.end_date = '') + LIMIT 1` + + const response = await datasette.runQuery(sql) + return response.formattedData.length > 0 +} diff --git a/src/views/check/confirmation.html b/src/views/check/confirmation.html index 4bb4f5856..94ce77ceb 100644 --- a/src/views/check/confirmation.html +++ b/src/views/check/confirmation.html @@ -6,12 +6,22 @@ {% set serviceType = 'Check' %} -{% if options.requestId %} +{% if options.alreadyCollectingEndpoint %} + {% set pageName = "Data checked" %} +{% elif options.requestId %} {% set pageName = "Provide your data" %} {% else %} {% set pageName = "Publish your data" %} {% endif %} +{% if options.deepLink %} + {% set datasetOverviewHref = options.deepLink.referrer %} +{% elif options.orgId and options.dataset %} + {% set datasetOverviewHref = "/organisations/" + options.orgId + "/" + options.dataset %} +{% else %} + {% set datasetOverviewHref = "/" %} +{% endif %} + {% block beforeContent %} {{ govukBackLink({ text: "Back", @@ -24,6 +34,20 @@ # What happens next + {% if options.alreadyCollectingEndpoint %} + + We are already collecting data from your endpoint URL, so we will update your changes automatically. + + You do not need to submit your endpoint URL again. + + {% if options.dataset %} +

Return to {{ options.dataset | datasetSlugToReadableName }} overview

+ {% else %} +

Return to Home

+ {% endif %} + + {% else %} + {% if options.requestId %} ## 1. Make sure that your data is published on your website @@ -76,6 +100,8 @@ {% endif %} + {% endif %} + ## Give feedback [Give feedback about this service]({{feedbackLink}}) (takes 30 seconds). diff --git a/test/unit/check/confirmationPage.test.js b/test/unit/check/confirmationPage.test.js index 3a8a94977..360cad752 100644 --- a/test/unit/check/confirmationPage.test.js +++ b/test/unit/check/confirmationPage.test.js @@ -51,4 +51,43 @@ describe('Check confirmation View', () => { expect(submitLink.getAttribute('href')).toBe('/submit/lpa-details') }) }) + + describe('when the endpoint is already being collected for the dataset', () => { + const templateParams = { + options: { + ...baseOptions, + requestId: 'abc-123', + alreadyCollectingEndpoint: true, + orgId: 'local-authority:ABC', + dataset: 'brownfield-land' + } + } + const html = stripWhitespace(nunjucks.render('check/confirmation.html', templateParams)) + const dom = new JSDOM(html) + const doc = dom.window.document + + runGenericPageTests(html, { + pageTitle: 'Data checked - Check your planning data' + }) + + it('should render the data checked panel', () => { + const regex = new RegExp('

', 'g') + expect(html).toMatch(regex) + }) + + it('should render the already collecting content', () => { + expect(doc.body.textContent).toContain('We are already collecting data from your endpoint URL, so we will update your changes automatically.') + expect(doc.body.textContent).toContain('You do not need to submit your endpoint URL again.') + }) + + it('should not render the submit link', () => { + expect(doc.querySelector('a.submit-link')).toBeNull() + }) + + it('should render a dataset overview link', () => { + const overviewLink = doc.querySelector('a[href="/organisations/local-authority:ABC/brownfield-land"]') + expect(overviewLink).not.toBeNull() + expect(overviewLink.textContent.trim()).toBe('Return to brownfield-land overview') + }) + }) }) diff --git a/test/unit/checkConfirmationController.test.js b/test/unit/checkConfirmationController.test.js new file mode 100644 index 000000000..58d57dc73 --- /dev/null +++ b/test/unit/checkConfirmationController.test.js @@ -0,0 +1,106 @@ +/* eslint-disable no-import-assign */ +import PageController from '../../src/controllers/pageController.js' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('../../src/services/asyncRequestApi.js') +vi.mock('../../src/utils/datasetteQueries/endpointAlreadyCollected.js') +vi.mock('../../src/utils/logger.js', () => ({ + default: { + warn: vi.fn() + } +})) + +describe('CheckConfirmationController', () => { + let Controller + let getRequestData + let endpointAlreadyCollectedForDataset + let controller + let req + let res + let next + let sessionValues + + beforeEach(async () => { + vi.clearAllMocks() + const asyncRequestApi = await import('../../src/services/asyncRequestApi.js') + const endpointAlreadyCollected = await import('../../src/utils/datasetteQueries/endpointAlreadyCollected.js') + const CheckConfirmationController = await import('../../src/controllers/checkConfirmationController.js') + Controller = CheckConfirmationController.default + getRequestData = asyncRequestApi.getRequestData + endpointAlreadyCollectedForDataset = endpointAlreadyCollected.endpointAlreadyCollectedForDataset + controller = new Controller({ route: '/confirmation' }) + sessionValues = new Map([ + ['upload-method', 'url'], + ['request_id', 'request-123'] + ]) + req = { + originalUrl: '/check/confirmation', + session: {}, + form: { options: {} }, + sessionModel: { + get: vi.fn(key => sessionValues.get(key)), + set: vi.fn((key, value) => sessionValues.set(key, value)) + } + } + res = {} + next = vi.fn() + getRequestData.mockResolvedValue({ + getParams: () => ({ + type: 'check_url', + url: 'https://example.com/data.csv', + dataset: 'brownfield-land', + organisationName: 'local-authority:ABC' + }) + }) + endpointAlreadyCollectedForDataset.mockResolvedValue(false) + }) + + it('sets request data context and keeps the submit handoff when endpoint is not already collected', async () => { + const superLocalsSpy = vi.spyOn(PageController.prototype, 'locals') + + await controller.locals(req, res, next) + + expect(getRequestData).toHaveBeenCalledWith('request-123') + expect(endpointAlreadyCollectedForDataset).toHaveBeenCalledWith({ + endpointUrl: 'https://example.com/data.csv', + dataset: 'brownfield-land' + }) + expect(req.sessionModel.set).toHaveBeenCalledWith('dataset', 'brownfield-land') + expect(req.sessionModel.set).toHaveBeenCalledWith('orgId', 'local-authority:ABC') + expect(req.form.options.requestId).toBe('request-123') + expect(req.form.options.alreadyCollectingEndpoint).toBe(false) + expect(req.session.checkRequestId).toBe('request-123') + expect(superLocalsSpy).toHaveBeenCalledWith(req, res, next) + }) + + it('clears the submit handoff when endpoint is already collected for the dataset', async () => { + req.session.checkRequestId = 'old-request-id' + endpointAlreadyCollectedForDataset.mockResolvedValue(true) + + await controller.locals(req, res, next) + + expect(req.form.options.alreadyCollectingEndpoint).toBe(true) + expect(req.session.checkRequestId).toBeUndefined() + }) + + it('keeps current confirmation behaviour when the request is not a URL check', async () => { + sessionValues.set('upload-method', 'file') + + await controller.locals(req, res, next) + + expect(getRequestData).not.toHaveBeenCalled() + expect(endpointAlreadyCollectedForDataset).not.toHaveBeenCalled() + expect(req.form.options.requestId).toBeUndefined() + expect(req.session.checkRequestId).toBeUndefined() + }) + + it('fails open and keeps the submit handoff when request lookup fails', async () => { + getRequestData.mockRejectedValue(new Error('API error')) + + await controller.locals(req, res, next) + + expect(req.form.options.requestId).toBe('request-123') + expect(req.form.options.alreadyCollectingEndpoint).toBeUndefined() + expect(req.session.checkRequestId).toBe('request-123') + }) +}) diff --git a/test/unit/utils/endpointAlreadyCollected.test.js b/test/unit/utils/endpointAlreadyCollected.test.js new file mode 100644 index 000000000..f9d29d345 --- /dev/null +++ b/test/unit/utils/endpointAlreadyCollected.test.js @@ -0,0 +1,54 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' +import datasette from '../../../src/services/datasette.js' +import { endpointAlreadyCollectedForDataset } from '../../../src/utils/datasetteQueries/endpointAlreadyCollected.js' + +vi.mock('../../../src/services/datasette.js', () => ({ + default: { + runQuery: vi.fn() + } +})) + +describe('endpointAlreadyCollectedForDataset', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns true when Datasette finds a matching active endpoint resource for the dataset', async () => { + datasette.runQuery.mockResolvedValue({ formattedData: [{ 1: 1 }] }) + + await expect(endpointAlreadyCollectedForDataset({ + endpointUrl: 'https://example.com/data.csv', + dataset: 'brownfield-land' + })).resolves.toBe(true) + }) + + it('returns false when Datasette does not find a matching endpoint resource for the dataset', async () => { + datasette.runQuery.mockResolvedValue({ formattedData: [] }) + + await expect(endpointAlreadyCollectedForDataset({ + endpointUrl: 'https://example.com/data.csv', + dataset: 'article-4-direction' + })).resolves.toBe(false) + }) + + it('escapes endpoint URL and dataset values in the query', async () => { + datasette.runQuery.mockResolvedValue({ formattedData: [] }) + + await endpointAlreadyCollectedForDataset({ + endpointUrl: "https://example.com/data's.csv", + dataset: "dataset's" + }) + + expect(datasette.runQuery.mock.calls[0][0]).toContain("https://example.com/data''s.csv") + expect(datasette.runQuery.mock.calls[0][0]).toContain("dataset''s") + }) + + it('does not query Datasette when endpoint URL or dataset is missing', async () => { + await expect(endpointAlreadyCollectedForDataset({ + endpointUrl: '', + dataset: 'brownfield-land' + })).resolves.toBe(false) + + expect(datasette.runQuery).not.toHaveBeenCalled() + }) +}) From d4873b0db08da141c8b59c8492674610254342f7 Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Tue, 30 Jun 2026 11:13:39 +0100 Subject: [PATCH 2/9] fix: Enhance endpoint duplication check to include organisation parameter --- package-lock.json | 119 +++++------------- .../checkConfirmationController.js | 3 +- .../endpointAlreadyCollected.js | 6 +- test/unit/checkConfirmationController.test.js | 3 +- .../utils/endpointAlreadyCollected.test.js | 20 ++- 5 files changed, 56 insertions(+), 95 deletions(-) diff --git a/package-lock.json b/package-lock.json index 634d62283..d9bd30eef 100644 --- a/package-lock.json +++ b/package-lock.json @@ -69,7 +69,7 @@ "zaproxy": "^2.0.0-rc.5" }, "engines": { - "node": ">=22.5.1" + "node": ">=22.12.0" } }, "node_modules/@asamuzakjp/css-color": { @@ -2475,9 +2475,6 @@ "cpu": [ "arm" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2498,9 +2495,6 @@ "cpu": [ "arm" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2521,9 +2515,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2544,9 +2535,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2567,9 +2555,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2590,9 +2575,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3092,9 +3074,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3112,9 +3091,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3132,9 +3108,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3152,9 +3125,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3172,9 +3142,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3192,9 +3159,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3373,9 +3337,6 @@ "arm" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3390,9 +3351,6 @@ "arm" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3407,9 +3365,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3424,9 +3379,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3441,9 +3393,6 @@ "loong64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3458,9 +3407,6 @@ "loong64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3475,9 +3421,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3492,9 +3435,6 @@ "ppc64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3509,9 +3449,6 @@ "riscv64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3526,9 +3463,6 @@ "riscv64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -3543,9 +3477,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3560,9 +3491,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -3577,9 +3505,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -5661,6 +5586,7 @@ "version": "1.8.1", "resolved": "https://registry.npmjs.org/b4a/-/b4a-1.8.1.tgz", "integrity": "sha512-aiqre1Nr0B/6DgE2N5vwTc+2/oQZ4Wh1t4NznYY4E00y8LCt6NqdRv81so00oo27D8MVKTpUa/MwUUtBLXCoDw==", + "dev": true, "license": "Apache-2.0", "peerDependencies": { "react-native-b4a": "*" @@ -5681,6 +5607,7 @@ "version": "2.9.1", "resolved": "https://registry.npmjs.org/bare-events/-/bare-events-2.9.1.tgz", "integrity": "sha512-Z0oHEHAFDZkffN8Qc39zNZjQlMDkPJRyyyZieU1VH7u8c5S+qHZ2S8ixdKIAxEjfHO7FJxXmJWgteOghVanIsg==", + "dev": true, "license": "Apache-2.0", "peerDependencies": { "bare-abort-controller": "*" @@ -5695,6 +5622,7 @@ "version": "4.7.2", "resolved": "https://registry.npmjs.org/bare-fs/-/bare-fs-4.7.2.tgz", "integrity": "sha512-aTvMFUWkBmjzKtEQMDGGDNF8bkfpD5N1b/FCwt7A3wrU4t1o/e/85Wzkluh6JlODCjqVESYCkQCdTXqZ9G7VFg==", + "dev": true, "license": "Apache-2.0", "dependencies": { "bare-events": "^2.5.4", @@ -5719,6 +5647,7 @@ "version": "3.9.1", "resolved": "https://registry.npmjs.org/bare-os/-/bare-os-3.9.1.tgz", "integrity": "sha512-6M5XjcnsygQNPMCMPXSK379xrJFiZ/AEMNBmFEmQW8d/789VQATvriyi5r0HYTL9TkQ26rn3kgdTG3aisbrXkQ==", + "dev": true, "license": "Apache-2.0", "engines": { "bare": ">=1.14.0" @@ -5728,6 +5657,7 @@ "version": "3.0.1", "resolved": "https://registry.npmjs.org/bare-path/-/bare-path-3.0.1.tgz", "integrity": "sha512-ghj2DSK/2e99a1anTVPCV4m4YIYtrbXhfM7V3D7XZLOTsybnYyaJloymGqssQc8l/or0UoDyRtNQkmkEF/ysgQ==", + "dev": true, "license": "Apache-2.0", "dependencies": { "bare-os": "^3.0.1" @@ -5737,6 +5667,7 @@ "version": "2.13.3", "resolved": "https://registry.npmjs.org/bare-stream/-/bare-stream-2.13.3.tgz", "integrity": "sha512-Kc+brLqvEqGkjyfiwJmImAOqLZL7OsoLKuavx+hJjgVV3nLTOjloJyPMFxjUPerGGHrNH0fLU06jjykMLWrERQ==", + "dev": true, "license": "Apache-2.0", "dependencies": { "b4a": "^1.8.1", @@ -5764,6 +5695,7 @@ "version": "2.4.5", "resolved": "https://registry.npmjs.org/bare-url/-/bare-url-2.4.5.tgz", "integrity": "sha512-K+y9xF1tN+CdPu4qWwr0QiK1Al07eFPGYK5M2pDXcmHdMdgC/tT/bpmMe1hrmRHaidKLkXrC+cRNYf3XVDUhSQ==", + "dev": true, "license": "Apache-2.0", "dependencies": { "bare-path": "^3.0.0" @@ -8903,6 +8835,7 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/events-universal/-/events-universal-1.0.1.tgz", "integrity": "sha512-LUd5euvbMLpwOF8m6ivPCbhQeSiYVNb8Vs0fQ8QjXo0JTkEHpz8pxdQf0gStltaPpw0Cca8b39KxvK9cfKRiAw==", + "dev": true, "license": "Apache-2.0", "dependencies": { "bare-events": "^2.7.0" @@ -9010,6 +8943,7 @@ "version": "1.3.2", "resolved": "https://registry.npmjs.org/fast-fifo/-/fast-fifo-1.3.2.tgz", "integrity": "sha512-/d9sfos4yxzpwkDkuN7k2SqFKtYNmCTzgfEpz82x34IM9/zc8KGxQoXg1liNC/izpRM/MBdt44Nmx41ZWqk+FQ==", + "dev": true, "license": "MIT" }, "node_modules/fast-json-stable-stringify": { @@ -9360,6 +9294,7 @@ "version": "10.1.0", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-10.1.0.tgz", "integrity": "sha512-oRXApq54ETRj4eMiFzGnHWGy+zo5raudjuxN0b8H7s/RU2oW0Wvsx9O0ACRN/kRq9E8Vu/ReskGB5o3ji+FzHQ==", + "dev": true, "license": "MIT", "dependencies": { "graceful-fs": "^4.2.0", @@ -14051,6 +13986,7 @@ "version": "4.2.11", "resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.2.11.tgz", "integrity": "sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==", + "dev": true, "license": "ISC" }, "node_modules/graphemer": { @@ -14575,6 +14511,7 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/ignore-by-default/-/ignore-by-default-1.0.1.tgz", "integrity": "sha512-Ius2VYcGNk7T90CppJqcIkS5ooHUZyIQK+ClZfMfMNFEF9VSE73Fq+906u/CWu92x4gzZMWOwfFYckPObzdEbA==", + "dev": true, "license": "ISC" }, "node_modules/immutable": { @@ -15781,6 +15718,7 @@ "version": "6.2.1", "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-6.2.1.tgz", "integrity": "sha512-zwOTdL3rFQ/lRdBnntKVOX6k5cKJwEc1HdilT71BWEu7J41gXIB2MRp+vxduPSwZJPWBxEzv4yH1wYLJGUHX4Q==", + "dev": true, "license": "MIT", "dependencies": { "universalify": "^2.0.0" @@ -16107,9 +16045,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -16131,9 +16066,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -16155,9 +16087,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -16179,9 +16108,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -16978,6 +16904,7 @@ "version": "3.1.14", "resolved": "https://registry.npmjs.org/nodemon/-/nodemon-3.1.14.tgz", "integrity": "sha512-jakjZi93UtB3jHMWsXL68FXSAosbLfY0In5gtKq3niLSkrWznrVBzXFNOEMJUfc9+Ke7SHWoAZsiMkNP3vq6Jw==", + "dev": true, "license": "MIT", "dependencies": { "chokidar": "^3.5.2", @@ -17006,6 +16933,7 @@ "version": "4.0.4", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-4.0.4.tgz", "integrity": "sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==", + "dev": true, "license": "MIT", "engines": { "node": "18 || 20 || >=22" @@ -17015,6 +16943,7 @@ "version": "5.0.6", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz", "integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==", + "dev": true, "license": "MIT", "dependencies": { "balanced-match": "^4.0.2" @@ -17027,6 +16956,7 @@ "version": "4.4.3", "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.3.tgz", "integrity": "sha512-RGwwWnwQvkVfavKVt22FGLw+xYSdzARwm0ru6DhTVA3umU5hZc28V3kO4stgYryrTlLpuvgI9GiijltAjNbcqA==", + "dev": true, "license": "MIT", "dependencies": { "ms": "^2.1.3" @@ -17044,6 +16974,7 @@ "version": "3.0.0", "resolved": "https://registry.npmjs.org/has-flag/-/has-flag-3.0.0.tgz", "integrity": "sha512-sKJf1+ceQBr4SMkvQnBDNDtf4TXpVhVGateu0t918bl30FnbE2m4vNLX+VWe/dpjlb+HugGYzW7uQXH98HPEYw==", + "dev": true, "license": "MIT", "engines": { "node": ">=4" @@ -17053,6 +16984,7 @@ "version": "10.2.5", "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.2.5.tgz", "integrity": "sha512-MULkVLfKGYDFYejP07QOurDLLQpcjk7Fw+7jXS2R2czRQzR56yHRveU5NDJEOviH+hETZKSkIk5c+T23GjFUMg==", + "dev": true, "license": "BlueOak-1.0.0", "dependencies": { "brace-expansion": "^5.0.5" @@ -17068,12 +17000,14 @@ "version": "2.1.3", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", + "dev": true, "license": "MIT" }, "node_modules/nodemon/node_modules/supports-color": { "version": "5.5.0", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.5.0.tgz", "integrity": "sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==", + "dev": true, "license": "MIT", "dependencies": { "has-flag": "^3.0.0" @@ -18328,6 +18262,7 @@ "version": "1.1.8", "resolved": "https://registry.npmjs.org/pstree.remy/-/pstree.remy-1.1.8.tgz", "integrity": "sha512-77DZwxQmxKnu3aR542U+X8FypNzbfJ+C5XQDk3uWjWxn6151aIMGthWYRXTqT1E5oJvg+ljaa2OJi+VfvCOQ8w==", + "dev": true, "license": "MIT" }, "node_modules/pump": { @@ -19704,6 +19639,7 @@ "version": "2.0.0", "resolved": "https://registry.npmjs.org/simple-update-notifier/-/simple-update-notifier-2.0.0.tgz", "integrity": "sha512-a2B9Y0KlNXl9u/vsW6sTIu9vGEpfKu2wRV6l1H3XEas/0gUIzGzBoP/IouTcUQbm9JWZLH3COxyn03TYlFax6w==", + "dev": true, "license": "MIT", "dependencies": { "semver": "^7.5.3" @@ -20058,6 +19994,7 @@ "version": "2.28.0", "resolved": "https://registry.npmjs.org/streamx/-/streamx-2.28.0.tgz", "integrity": "sha512-1Yowhzjf0ivGMrTIkY9hav5TxobO9qIVqUE41fiCGMGgc3CLlf4MY+9AHmZqBWgDTue0fY9zWjYFVyf6Diuobw==", + "dev": true, "license": "MIT", "dependencies": { "events-universal": "^1.0.0", @@ -20502,6 +20439,7 @@ "version": "3.2.0", "resolved": "https://registry.npmjs.org/tar-stream/-/tar-stream-3.2.0.tgz", "integrity": "sha512-ojzvCvVaNp6aOTFmG7jaRD0meowIAuPc3cMMhSgKiVWws1GyHbGd/xvnyuRKcKlMpt3qvxx6r0hreCNITP9hIg==", + "dev": true, "license": "MIT", "dependencies": { "b4a": "^1.6.4", @@ -20514,6 +20452,7 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/teex/-/teex-1.0.1.tgz", "integrity": "sha512-eYE6iEI62Ni1H8oIa7KlDU6uQBtqr4Eajni3wX7rpfXD8ysFx8z0+dri+KWEPWpBsxXfxu58x/0jvTVT1ekOSg==", + "dev": true, "license": "MIT", "dependencies": { "streamx": "^2.12.5" @@ -20676,6 +20615,7 @@ "version": "1.2.7", "resolved": "https://registry.npmjs.org/text-decoder/-/text-decoder-1.2.7.tgz", "integrity": "sha512-vlLytXkeP4xvEq2otHeJfSQIRyWxo/oZGEbXrtEEF9Hnmrdly59sUbzZ/QgyWuLYHctCHxFF4tRQZNQ9k60ExQ==", + "dev": true, "license": "Apache-2.0", "dependencies": { "b4a": "^1.6.4" @@ -20844,6 +20784,7 @@ "version": "3.1.1", "resolved": "https://registry.npmjs.org/touch/-/touch-3.1.1.tgz", "integrity": "sha512-r0eojU4bI8MnHr8c5bNo7lJDdI2qXlWWJk6a9EAFG7vbhTjElYhBVS3/miuE0uOuoLdb8Mc/rVfsmm6eo5o9GA==", + "dev": true, "license": "ISC", "bin": { "nodetouch": "bin/nodetouch.js" @@ -21167,6 +21108,7 @@ "version": "2.0.5", "resolved": "https://registry.npmjs.org/undefsafe/-/undefsafe-2.0.5.tgz", "integrity": "sha512-WxONCrssBM8TSPRqN5EmsjVrsv4A8X12J4ArBiiayv3DyyG3ZlIg6yysuuSYdZsVz3TKcTg2fd//Ujd4CHV1iA==", + "dev": true, "license": "MIT" }, "node_modules/underscore": { @@ -21211,6 +21153,7 @@ "version": "2.0.1", "resolved": "https://registry.npmjs.org/universalify/-/universalify-2.0.1.tgz", "integrity": "sha512-gptHNQghINnc/vTGIk0SOFGFNXw7JVrlRUtConJRlvaw6DuX0wO5Jeko9sWrMBhh+PsYAZ7oXAiOnf/UKogyiw==", + "dev": true, "license": "MIT", "engines": { "node": ">= 10.0.0" diff --git a/src/controllers/checkConfirmationController.js b/src/controllers/checkConfirmationController.js index 773febb5e..0b9e115c7 100644 --- a/src/controllers/checkConfirmationController.js +++ b/src/controllers/checkConfirmationController.js @@ -21,7 +21,8 @@ class CheckConfirmationController extends PageController { } req.form.options.alreadyCollectingEndpoint = await endpointAlreadyCollectedForDataset({ endpointUrl: params.url, - dataset: params.dataset + dataset: params.dataset, + organisation: params.organisationName }) } catch (error) { logger.warn('CheckConfirmationController: could not check whether endpoint is already collected', { diff --git a/src/utils/datasetteQueries/endpointAlreadyCollected.js b/src/utils/datasetteQueries/endpointAlreadyCollected.js index f08796dd5..a7febdfcc 100644 --- a/src/utils/datasetteQueries/endpointAlreadyCollected.js +++ b/src/utils/datasetteQueries/endpointAlreadyCollected.js @@ -4,8 +4,8 @@ function sqlString (value) { return String(value).replaceAll("'", "''") } -export async function endpointAlreadyCollectedForDataset ({ endpointUrl, dataset }) { - if (!endpointUrl || !dataset) return false +export async function endpointAlreadyCollectedForDataset ({ endpointUrl, dataset, organisation }) { + if (!endpointUrl || !dataset || !organisation) return false const sql = /* sql */ ` SELECT 1 @@ -13,8 +13,10 @@ export async function endpointAlreadyCollectedForDataset ({ endpointUrl, dataset JOIN resource_endpoint re ON e.endpoint = re.endpoint JOIN resource r ON re.resource = r.resource JOIN resource_dataset rd ON r.resource = rd.resource + JOIN resource_organisation ro ON r.resource = ro.resource WHERE e.endpoint_url = '${sqlString(endpointUrl)}' AND rd.dataset = '${sqlString(dataset)}' + AND REPLACE(ro.organisation, '-eng', '') = '${sqlString(organisation)}' AND (e.end_date IS NULL OR e.end_date = '') AND (r.end_date IS NULL OR r.end_date = '') LIMIT 1` diff --git a/test/unit/checkConfirmationController.test.js b/test/unit/checkConfirmationController.test.js index 58d57dc73..d8249330d 100644 --- a/test/unit/checkConfirmationController.test.js +++ b/test/unit/checkConfirmationController.test.js @@ -63,7 +63,8 @@ describe('CheckConfirmationController', () => { expect(getRequestData).toHaveBeenCalledWith('request-123') expect(endpointAlreadyCollectedForDataset).toHaveBeenCalledWith({ endpointUrl: 'https://example.com/data.csv', - dataset: 'brownfield-land' + dataset: 'brownfield-land', + organisation: 'local-authority:ABC' }) expect(req.sessionModel.set).toHaveBeenCalledWith('dataset', 'brownfield-land') expect(req.sessionModel.set).toHaveBeenCalledWith('orgId', 'local-authority:ABC') diff --git a/test/unit/utils/endpointAlreadyCollected.test.js b/test/unit/utils/endpointAlreadyCollected.test.js index f9d29d345..f1e2cb63d 100644 --- a/test/unit/utils/endpointAlreadyCollected.test.js +++ b/test/unit/utils/endpointAlreadyCollected.test.js @@ -18,7 +18,8 @@ describe('endpointAlreadyCollectedForDataset', () => { await expect(endpointAlreadyCollectedForDataset({ endpointUrl: 'https://example.com/data.csv', - dataset: 'brownfield-land' + dataset: 'brownfield-land', + organisation: 'local-authority:ABC' })).resolves.toBe(true) }) @@ -27,7 +28,8 @@ describe('endpointAlreadyCollectedForDataset', () => { await expect(endpointAlreadyCollectedForDataset({ endpointUrl: 'https://example.com/data.csv', - dataset: 'article-4-direction' + dataset: 'article-4-direction', + organisation: 'local-authority:ABC' })).resolves.toBe(false) }) @@ -36,11 +38,13 @@ describe('endpointAlreadyCollectedForDataset', () => { await endpointAlreadyCollectedForDataset({ endpointUrl: "https://example.com/data's.csv", - dataset: "dataset's" + dataset: "dataset's", + organisation: "local-authority:O'RG" }) expect(datasette.runQuery.mock.calls[0][0]).toContain("https://example.com/data''s.csv") expect(datasette.runQuery.mock.calls[0][0]).toContain("dataset''s") + expect(datasette.runQuery.mock.calls[0][0]).toContain("local-authority:O''RG") }) it('does not query Datasette when endpoint URL or dataset is missing', async () => { @@ -51,4 +55,14 @@ describe('endpointAlreadyCollectedForDataset', () => { expect(datasette.runQuery).not.toHaveBeenCalled() }) + + it('does not query Datasette when organisation is missing', async () => { + await expect(endpointAlreadyCollectedForDataset({ + endpointUrl: 'https://example.com/data.csv', + dataset: 'brownfield-land', + organisation: '' + })).resolves.toBe(false) + + expect(datasette.runQuery).not.toHaveBeenCalled() + }) }) From e7cad144242f05eeba96828f3c8674c366d3b3f5 Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Tue, 30 Jun 2026 11:20:24 +0100 Subject: [PATCH 3/9] test: Add checks for organisation in endpoint duplication logic and update confirmation view links --- src/views/check/confirmation.html | 6 ++--- test/unit/check/confirmationPage.test.js | 18 +++++++++++++ .../utils/endpointAlreadyCollected.test.js | 26 +++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/src/views/check/confirmation.html b/src/views/check/confirmation.html index 94ce77ceb..cdeaee432 100644 --- a/src/views/check/confirmation.html +++ b/src/views/check/confirmation.html @@ -18,8 +18,6 @@ {% set datasetOverviewHref = options.deepLink.referrer %} {% elif options.orgId and options.dataset %} {% set datasetOverviewHref = "/organisations/" + options.orgId + "/" + options.dataset %} -{% else %} - {% set datasetOverviewHref = "/" %} {% endif %} {% block beforeContent %} @@ -40,10 +38,10 @@ You do not need to submit your endpoint URL again. - {% if options.dataset %} + {% if datasetOverviewHref and options.dataset %}

Return to {{ options.dataset | datasetSlugToReadableName }} overview

{% else %} -

Return to Home

+

Return to Home

{% endif %} {% else %} diff --git a/test/unit/check/confirmationPage.test.js b/test/unit/check/confirmationPage.test.js index 360cad752..4a1f09218 100644 --- a/test/unit/check/confirmationPage.test.js +++ b/test/unit/check/confirmationPage.test.js @@ -89,5 +89,23 @@ describe('Check confirmation View', () => { expect(overviewLink).not.toBeNull() expect(overviewLink.textContent.trim()).toBe('Return to brownfield-land overview') }) + + it('should render a home link when an overview URL cannot be built', () => { + const html = stripWhitespace(nunjucks.render('check/confirmation.html', { + options: { + ...baseOptions, + requestId: 'abc-123', + alreadyCollectingEndpoint: true, + orgId: undefined, + dataset: 'brownfield-land' + } + })) + const doc = new JSDOM(html).window.document + const homeLink = doc.querySelector('a[href="/"]') + + expect(homeLink).not.toBeNull() + expect(homeLink.textContent.trim()).toBe('Return to Home') + expect(doc.body.textContent).not.toContain('Return to brownfield-land overview') + }) }) }) diff --git a/test/unit/utils/endpointAlreadyCollected.test.js b/test/unit/utils/endpointAlreadyCollected.test.js index f1e2cb63d..5aaed1e7e 100644 --- a/test/unit/utils/endpointAlreadyCollected.test.js +++ b/test/unit/utils/endpointAlreadyCollected.test.js @@ -33,6 +33,32 @@ describe('endpointAlreadyCollectedForDataset', () => { })).resolves.toBe(false) }) + it('returns false when the same endpoint URL and dataset belong to a different organisation', async () => { + datasette.runQuery.mockImplementation(query => ({ + formattedData: query.includes("local-authority:ABC'") ? [{ 1: 1 }] : [] + })) + + await expect(endpointAlreadyCollectedForDataset({ + endpointUrl: 'https://example.com/data.csv', + dataset: 'brownfield-land', + organisation: 'local-authority:XYZ' + })).resolves.toBe(false) + + expect(datasette.runQuery.mock.calls[0][0]).toContain("local-authority:XYZ'") + }) + + it('returns true when the same endpoint URL and dataset belong to the same organisation', async () => { + datasette.runQuery.mockImplementation(query => ({ + formattedData: query.includes("local-authority:ABC'") ? [{ 1: 1 }] : [] + })) + + await expect(endpointAlreadyCollectedForDataset({ + endpointUrl: 'https://example.com/data.csv', + dataset: 'brownfield-land', + organisation: 'local-authority:ABC' + })).resolves.toBe(true) + }) + it('escapes endpoint URL and dataset values in the query', async () => { datasette.runQuery.mockResolvedValue({ formattedData: [] }) From 9fac5cf334f709900015175a9868b6a00be0a823 Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 1 Jul 2026 08:42:00 +0100 Subject: [PATCH 4/9] fix: Update heading levels for clarity in confirmation view --- src/views/check/confirmation.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/views/check/confirmation.html b/src/views/check/confirmation.html index cdeaee432..4708b480a 100644 --- a/src/views/check/confirmation.html +++ b/src/views/check/confirmation.html @@ -30,7 +30,7 @@ {% set content %} - # What happens next + ## What happens next {% if options.alreadyCollectingEndpoint %} @@ -48,11 +48,11 @@ {% if options.requestId %} - ## 1. Make sure that your data is published on your website + ### 1. Make sure that your data is published on your website {% else %} - ## 1. Publish data to your website + ### 1. Publish data to your website {% endif %} @@ -63,7 +63,7 @@ {% if options.requestId %} - ## 2. Provide your data + ### 2. Provide your data You need to submit: @@ -84,7 +84,7 @@ {% else %} - ## 2. Provide your data + ### 2. Provide your data After you publish, you should [provide your data to the Planning Data Platform](/check/url). From 77faa7ce6d9f8ce66db687ebc8eee1b3663c665a Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 1 Jul 2026 09:18:36 +0100 Subject: [PATCH 5/9] fix: issue with column mapping showing inconsistently --- src/assets/js/statusPage.js | 2 +- src/controllers/statusController.js | 6 ++- src/routes/api.js | 17 ++++++-- src/views/check/statusPage/status.html | 2 +- test/unit/routes.api.test.js | 58 ++++++++++++++++++++++++++ test/unit/statusController.test.js | 20 +++++++++ 6 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 test/unit/routes.api.test.js diff --git a/src/assets/js/statusPage.js b/src/assets/js/statusPage.js index 9cc290b86..155138c58 100644 --- a/src/assets/js/statusPage.js +++ b/src/assets/js/statusPage.js @@ -8,7 +8,7 @@ const finishedProcessingStatuses = [ export default class StatusPage { constructor (pollingInterval, maxPollAttempts) { - this.pollingInterval = pollingInterval || 1000 + this.pollingInterval = pollingInterval || 3000 this.maxPollAttempts = maxPollAttempts || 30 this.pollingOffset = 400 this.pollAttempts = 0 diff --git a/src/controllers/statusController.js b/src/controllers/statusController.js index d768251cd..295c4e3a5 100644 --- a/src/controllers/statusController.js +++ b/src/controllers/statusController.js @@ -81,7 +81,11 @@ class StatusController extends PageController { req.form.options.messageTexts = messageTexts req.form.options.buttonTexts = buttonTexts req.form.options.buttonAriaLabels = buttonAriaLabels - req.form.options.pollingEndpoint = `/api/status/${req.form.options.data.id}` + const pollingParams = new URLSearchParams() + const uniqueDatasetFields = req.uniqueDatasetFields || [] + uniqueDatasetFields.forEach(field => pollingParams.append('field', field)) + const pollingQuery = pollingParams.toString() + req.form.options.pollingEndpoint = `/api/status/${req.form.options.data.id}${pollingQuery ? `?${pollingQuery}` : ''}` const now = new Date() req.form.options.lastUpdated = now.toLocaleTimeString([], { hour: 'numeric', minute: '2-digit', hour12: true, timeZone: 'Europe/London' }).replace(' ', '').toLowerCase() + ' on ' + diff --git a/src/routes/api.js b/src/routes/api.js index 546830ed6..7b2c802f3 100644 --- a/src/routes/api.js +++ b/src/routes/api.js @@ -15,7 +15,7 @@ const router = express.Router() * @returns {Promise} Returns a JSON object with the request data if successful. * If an error occurs, returns a JSON object with an error message and sets the HTTP status code to 500. */ -router.get('/status/:result_id', async (req, res) => { +export async function getStatus (req, res) { res.set('Cache-Control', 'no-store') try { const resultData = await getRequestData(req.params.result_id) @@ -24,7 +24,8 @@ router.get('/status/:result_id', async (req, res) => { // compute whether we should show column mapping when the request finished if (typeof resultData.isComplete === 'function' && resultData.isComplete() && !resultData.isFailed?.()) { try { - const show = await shouldShowColumnMapping(resultData, []) + const uniqueDatasetFields = getUniqueDatasetFieldsFromQuery(req.query) + const show = await shouldShowColumnMapping(resultData, uniqueDatasetFields) payload.showColumnMapping = show if (show) payload.columnMappingUrl = `/check/column-mapping/${resultData.id}` } catch (e) { @@ -35,7 +36,17 @@ router.get('/status/:result_id', async (req, res) => { } catch (error) { return res.status(500).json({ error }) } -}) +} + +router.get('/status/:result_id', getStatus) + +export function getUniqueDatasetFieldsFromQuery (query = {}) { + if (!query.field) return [] + + return Array.isArray(query.field) + ? query.field + : [query.field] +} /** * Retrieves the boundary data for a local planning authority (LPA) by boundary ID. diff --git a/src/views/check/statusPage/status.html b/src/views/check/statusPage/status.html index a429bca41..c038cd3c7 100644 --- a/src/views/check/statusPage/status.html +++ b/src/views/check/statusPage/status.html @@ -59,7 +59,7 @@ {{ super() }} diff --git a/test/unit/routes.api.test.js b/test/unit/routes.api.test.js new file mode 100644 index 000000000..3234200a5 --- /dev/null +++ b/test/unit/routes.api.test.js @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { getStatus, getUniqueDatasetFieldsFromQuery } from '../../src/routes/api.js' +import { getRequestData } from '../../src/services/asyncRequestApi.js' +import { shouldShowColumnMapping } from '../../src/services/columnMappingDecider.js' + +vi.mock('../../src/services/asyncRequestApi.js', () => ({ + getRequestData: vi.fn() +})) + +vi.mock('../../src/services/columnMappingDecider.js', () => ({ + shouldShowColumnMapping: vi.fn() +})) + +describe('api routes', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('passes dataset fields from the query string into the column mapping decider', async () => { + const requestData = { + id: 'request-123', + status: 'COMPLETE', + isComplete: () => true, + isFailed: () => false + } + + vi.mocked(getRequestData).mockResolvedValue(requestData) + vi.mocked(shouldShowColumnMapping).mockResolvedValue(true) + + const req = { + params: { result_id: 'request-123' }, + query: { field: ['reference', 'geometry'] } + } + const res = { + statusCode: 200, + body: null, + set: vi.fn(), + status: vi.fn((statusCode) => { + res.statusCode = statusCode + return res + }), + json: vi.fn((body) => { + res.body = body + return res + }) + } + + await getStatus(req, res) + + expect(shouldShowColumnMapping).toHaveBeenCalledWith(requestData, ['reference', 'geometry']) + expect(res.body.showColumnMapping).toBe(true) + expect(res.body.columnMappingUrl).toBe('/check/column-mapping/request-123') + }) + + it('normalises a single field query parameter to an array', () => { + expect(getUniqueDatasetFieldsFromQuery({ field: 'reference' })).toEqual(['reference']) + }) +}) diff --git a/test/unit/statusController.test.js b/test/unit/statusController.test.js index 029259868..5d945ad36 100644 --- a/test/unit/statusController.test.js +++ b/test/unit/statusController.test.js @@ -73,6 +73,26 @@ describe('StatusController', () => { expect(req.form.options.data).toBe(mockResult) }) + + it('should include unique dataset fields in the polling endpoint', async () => { + const mockResult = { id: 'test_id', status: 'PROCESSING', response: { test: 'test' }, hasErrors: () => false } + asyncRequestApi.getRequestData = vi.fn().mockResolvedValue(mockResult) + + const req = { + form: { + options: {} + }, + params: { id: 'fake_id' }, + uniqueDatasetFields: ['reference', 'geometry', 'entry-date'] + } + + const res = {} + const next = vi.fn() + + await statusController.locals(req, res, next) + + expect(req.form.options.pollingEndpoint).toBe('/api/status/test_id?field=reference&field=geometry&field=entry-date') + }) }) describe('post', () => { From 7236e023855a17ac2ca0a3907e30f6e4ccd4803c Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 1 Jul 2026 09:24:11 +0100 Subject: [PATCH 6/9] test: Add checks for empty dataset in endpointAlreadyCollected function and update confirmation view link expectations --- test/unit/check/confirmationPage.test.js | 3 ++- test/unit/statusPage.test.js | 12 ++++++------ test/unit/utils/endpointAlreadyCollected.test.js | 5 +++++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/test/unit/check/confirmationPage.test.js b/test/unit/check/confirmationPage.test.js index 4a1f09218..28c9358b2 100644 --- a/test/unit/check/confirmationPage.test.js +++ b/test/unit/check/confirmationPage.test.js @@ -102,10 +102,11 @@ describe('Check confirmation View', () => { })) const doc = new JSDOM(html).window.document const homeLink = doc.querySelector('a[href="/"]') + const overviewLink = doc.querySelector('a[href^="/organisations/"]') expect(homeLink).not.toBeNull() expect(homeLink.textContent.trim()).toBe('Return to Home') - expect(doc.body.textContent).not.toContain('Return to brownfield-land overview') + expect(overviewLink).toBeNull() }) }) }) diff --git a/test/unit/statusPage.test.js b/test/unit/statusPage.test.js index 287e1431d..0937089b3 100644 --- a/test/unit/statusPage.test.js +++ b/test/unit/statusPage.test.js @@ -75,7 +75,7 @@ describe('StatusPage', () => { json: () => Promise.resolve(mockResponse) }) statusPage.beginPolling('http://test.com', '123') - await vi.advanceTimersByTimeAsync(1000) + await vi.advanceTimersByTimeAsync(statusPage.pollingInterval) await Promise.resolve() // wait for promises to resolve expect(statusPage.heading.textContent).toBe(headingTexts.checked) @@ -91,7 +91,7 @@ describe('StatusPage', () => { }) statusPage.beginPolling('http://test.com', '123') - await vi.advanceTimersByTimeAsync(1000) + await vi.advanceTimersByTimeAsync(statusPage.pollingInterval) await Promise.resolve() expect(statusPage.heading.textContent).toBe(headingTexts.columnMapping) @@ -108,7 +108,7 @@ describe('StatusPage', () => { json: () => Promise.resolve(mockResponse) }) statusPage.beginPolling('http://test.com', '123') - await vi.advanceTimersByTimeAsync(1000) + await vi.advanceTimersByTimeAsync(statusPage.pollingInterval) await Promise.resolve() // wait for promises to resolve expect(statusPage.heading.textContent).toBe(headingTexts.checked) expect(statusPage.continueButton.style.display).toBe('block') @@ -120,18 +120,18 @@ describe('StatusPage', () => { json: () => Promise.resolve(mockResponse) }) statusPage.beginPolling('http://test.com', '123') - await vi.advanceTimersByTimeAsync(1000) + await vi.advanceTimersByTimeAsync(statusPage.pollingInterval) await Promise.resolve() // wait for promises to resolve expect(statusPage.heading.textContent).toBe(headingTexts.checking) expect(statusPage.continueButton.style.display).toBe('none') - await vi.advanceTimersByTimeAsync(1000) + await vi.advanceTimersByTimeAsync(statusPage.pollingInterval) await Promise.resolve() expect(statusPage.heading.textContent).toBe(headingTexts.checking) expect(statusPage.continueButton.style.display).toBe('none') global.fetch.mockResolvedValueOnce({ json: () => Promise.resolve({ status: 'COMPLETE' }) }) - await vi.advanceTimersByTimeAsync(1000) + await vi.advanceTimersByTimeAsync(statusPage.pollingInterval) await Promise.resolve() expect(statusPage.heading.textContent).toBe(headingTexts.checked) diff --git a/test/unit/utils/endpointAlreadyCollected.test.js b/test/unit/utils/endpointAlreadyCollected.test.js index 5aaed1e7e..d489db124 100644 --- a/test/unit/utils/endpointAlreadyCollected.test.js +++ b/test/unit/utils/endpointAlreadyCollected.test.js @@ -79,6 +79,11 @@ describe('endpointAlreadyCollectedForDataset', () => { dataset: 'brownfield-land' })).resolves.toBe(false) + await expect(endpointAlreadyCollectedForDataset({ + endpointUrl: 'https://example.com/data.csv', + dataset: '' + })).resolves.toBe(false) + expect(datasette.runQuery).not.toHaveBeenCalled() }) From d23362bfe41d77b742f12bc3ae002b14d339371b Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 1 Jul 2026 10:24:52 +0100 Subject: [PATCH 7/9] fix: Update confirmation message for clarity on endpoint submission --- src/views/check/confirmation.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/views/check/confirmation.html b/src/views/check/confirmation.html index 4708b480a..ce484b9a9 100644 --- a/src/views/check/confirmation.html +++ b/src/views/check/confirmation.html @@ -34,9 +34,9 @@ {% if options.alreadyCollectingEndpoint %} - We are already collecting data from your endpoint URL, so we will update your changes automatically. + We are already collecting data from this endpoint URL, so we will process any changes you make automatically.. - You do not need to submit your endpoint URL again. + You do not need to submit this endpoint URL again. {% if datasetOverviewHref and options.dataset %}

Return to {{ options.dataset | datasetSlugToReadableName }} overview

From 75e65a9af25f1db5e63939e0b346b99ccdfdd73b Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 1 Jul 2026 11:51:51 +0100 Subject: [PATCH 8/9] fix: Improve confirmation message clarity and heading structure in confirmation view --- src/views/check/confirmation.html | 8 +++---- test/unit/check/confirmationPage.test.js | 30 +++++++++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/views/check/confirmation.html b/src/views/check/confirmation.html index ce484b9a9..25a2d9eff 100644 --- a/src/views/check/confirmation.html +++ b/src/views/check/confirmation.html @@ -34,12 +34,12 @@ {% if options.alreadyCollectingEndpoint %} - We are already collecting data from this endpoint URL, so we will process any changes you make automatically.. + We are already collecting data from this endpoint URL, so we will process any changes you make automatically. You do not need to submit this endpoint URL again. {% if datasetOverviewHref and options.dataset %} -

Return to {{ options.dataset | datasetSlugToReadableName }} overview

+

Return to {{ options.dataset | datasetSlugToReadableName | replace("-", " ") }} overview

{% else %}

Return to Home

{% endif %} @@ -76,7 +76,7 @@
Provide your data {% if options.deepLink %} - Return to {{ options.deepLink.dataset | datasetSlugToReadableName }} overview + Return to {{ options.deepLink.dataset | datasetSlugToReadableName | replace("-", " ") }} overview {% else %} Return to Home {% endif %} @@ -91,7 +91,7 @@ [Find out more about how to check and provide your data](/guidance) {% if options.deepLink %} -

Return to {{ options.deepLink.dataset | datasetSlugToReadableName }} overview

+

Return to {{ options.deepLink.dataset | datasetSlugToReadableName | replace("-", " ") }} overview

{% else %}

Return to Home

{% endif %} diff --git a/test/unit/check/confirmationPage.test.js b/test/unit/check/confirmationPage.test.js index 28c9358b2..0df63e386 100644 --- a/test/unit/check/confirmationPage.test.js +++ b/test/unit/check/confirmationPage.test.js @@ -32,6 +32,15 @@ describe('Check confirmation View', () => { it('should not render the submit link when requestId is absent', () => { expect(doc.querySelector('a.submit-link')).toBeNull() }) + + it('should render the standard confirmation heading structure', () => { + expect(doc.querySelector('main h1')?.textContent.trim()).toBe('Publish your data') + expect(doc.querySelector('main h2')?.textContent.trim()).toBe('What happens next') + expect([...doc.querySelectorAll('main h3')].map(heading => heading.textContent.trim())).toEqual([ + '1. Publish data to your website', + '2. Provide your data' + ]) + }) }) describe('with requestId', () => { @@ -50,6 +59,15 @@ describe('Check confirmation View', () => { expect(submitLink).not.toBeNull() expect(submitLink.getAttribute('href')).toBe('/submit/lpa-details') }) + + it('should render the standard confirmation heading structure', () => { + expect(doc.querySelector('main h1')?.textContent.trim()).toBe('Provide your data') + expect(doc.querySelector('main h2')?.textContent.trim()).toBe('What happens next') + expect([...doc.querySelectorAll('main h3')].map(heading => heading.textContent.trim())).toEqual([ + '1. Make sure that your data is published on your website', + '2. Provide your data' + ]) + }) }) describe('when the endpoint is already being collected for the dataset', () => { @@ -76,8 +94,8 @@ describe('Check confirmation View', () => { }) it('should render the already collecting content', () => { - expect(doc.body.textContent).toContain('We are already collecting data from your endpoint URL, so we will update your changes automatically.') - expect(doc.body.textContent).toContain('You do not need to submit your endpoint URL again.') + expect(doc.body.textContent).toContain('We are already collecting data from this endpoint URL, so we will process any changes you make automatically.') + expect(doc.body.textContent).toContain('You do not need to submit this endpoint URL again.') }) it('should not render the submit link', () => { @@ -87,7 +105,13 @@ describe('Check confirmation View', () => { it('should render a dataset overview link', () => { const overviewLink = doc.querySelector('a[href="/organisations/local-authority:ABC/brownfield-land"]') expect(overviewLink).not.toBeNull() - expect(overviewLink.textContent.trim()).toBe('Return to brownfield-land overview') + expect(overviewLink.textContent.trim()).toBe('Return to brownfield land overview') + }) + + it('should render What happens next as a second-level heading', () => { + expect(doc.querySelector('main h1')?.textContent.trim()).toBe('Data checked') + expect(doc.querySelector('main h2')?.textContent.trim()).toBe('What happens next') + expect([...doc.querySelectorAll('main h1')]).toHaveLength(1) }) it('should render a home link when an overview URL cannot be built', () => { From 548b7de363dd69dbfd8184b4d32574e1834577cc Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 1 Jul 2026 13:30:51 +0100 Subject: [PATCH 9/9] fix: Remove REPLACE function from organisation query to match directly --- .../datasetteQueries/endpointAlreadyCollected.js | 2 +- test/unit/utils/endpointAlreadyCollected.test.js | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/utils/datasetteQueries/endpointAlreadyCollected.js b/src/utils/datasetteQueries/endpointAlreadyCollected.js index a7febdfcc..107ce31fa 100644 --- a/src/utils/datasetteQueries/endpointAlreadyCollected.js +++ b/src/utils/datasetteQueries/endpointAlreadyCollected.js @@ -16,7 +16,7 @@ export async function endpointAlreadyCollectedForDataset ({ endpointUrl, dataset JOIN resource_organisation ro ON r.resource = ro.resource WHERE e.endpoint_url = '${sqlString(endpointUrl)}' AND rd.dataset = '${sqlString(dataset)}' - AND REPLACE(ro.organisation, '-eng', '') = '${sqlString(organisation)}' + AND ro.organisation = '${sqlString(organisation)}' AND (e.end_date IS NULL OR e.end_date = '') AND (r.end_date IS NULL OR r.end_date = '') LIMIT 1` diff --git a/test/unit/utils/endpointAlreadyCollected.test.js b/test/unit/utils/endpointAlreadyCollected.test.js index d489db124..c4825f7fb 100644 --- a/test/unit/utils/endpointAlreadyCollected.test.js +++ b/test/unit/utils/endpointAlreadyCollected.test.js @@ -73,6 +73,19 @@ describe('endpointAlreadyCollectedForDataset', () => { expect(datasette.runQuery.mock.calls[0][0]).toContain("local-authority:O''RG") }) + it('matches organisation directly', async () => { + datasette.runQuery.mockResolvedValue({ formattedData: [] }) + + await endpointAlreadyCollectedForDataset({ + endpointUrl: 'https://example.com/data.csv', + dataset: 'brownfield-land', + organisation: 'local-authority:ABC' + }) + + expect(datasette.runQuery.mock.calls[0][0]).toContain("ro.organisation = 'local-authority:ABC'") + expect(datasette.runQuery.mock.calls[0][0]).not.toContain('REPLACE') + }) + it('does not query Datasette when endpoint URL or dataset is missing', async () => { await expect(endpointAlreadyCollectedForDataset({ endpointUrl: '',