From 202372614197cfb2f14c2f0dd06d3a05b5f5f8dd Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 22 Jul 2026 15:56:00 +0100 Subject: [PATCH 1/2] refactor: improve column mapping request handling and add tests for missing params --- src/controllers/columnMappingController.js | 29 +++++++++++++------ src/utils/redisLoader.js | 3 +- test/unit/columnMappingController.test.js | 33 +++++++++++++++++++++- test/unit/redisLoader.test.js | 33 ++++++++++++++++++++++ 4 files changed, 88 insertions(+), 10 deletions(-) diff --git a/src/controllers/columnMappingController.js b/src/controllers/columnMappingController.js index 2c84d5984..164020342 100644 --- a/src/controllers/columnMappingController.js +++ b/src/controllers/columnMappingController.js @@ -6,6 +6,7 @@ import { processSpecificationMiddlewares } from '../middleware/common.middleware import platformApi from '../services/platformApi.js' import { types } from '../utils/logging.js' import logger from '../utils/logger.js' +import { MiddlewareError } from '../utils/errors.js' const LOCK_DETECTED_GEOMETRY_MAPPINGS = true const GEOMETRY_FIELDS = ['geometry', 'point'] @@ -14,14 +15,7 @@ class ColumnMappingController extends PageController { middlewareSetup () { super.middlewareSetup() this.use(getRequestDataMiddleware) - this.use(async (req, res, next) => { - const { requestData } = req.locals - const params = requestData.getParams() ?? {} - if (await isStatutoryDataset(params.organisationName, params.dataset)) { - return res.status(404).render('errors/404.html') - } - next() - }) + this.use(handleUnavailableColumnMappingRequest) this.use(updateSessionFromRequestData) // Populate req.params and dataset, then run specification processing middlewares this.use(async (req, res, next) => { @@ -121,6 +115,25 @@ class ColumnMappingController extends PageController { } } +export async function handleUnavailableColumnMappingRequest (req, res, next) { + const { requestData } = req.locals + const params = requestData?.getParams?.() ?? {} + const { organisationName, dataset } = params + + if (!organisationName || !dataset) { + return next(new MiddlewareError('Column mapping request params not found', 404)) + } + + if (await isStatutoryDataset({ + organisation: organisationName, + dataset + })) { + return next(new MiddlewareError('Column mapping not found', 404)) + } + + next() +} + /** * Build the options object passed to the column-mapping template. * Returns UI-friendly data including expected mapping rows, selectable uploaded diff --git a/src/utils/redisLoader.js b/src/utils/redisLoader.js index f83479273..fa20f79c5 100644 --- a/src/utils/redisLoader.js +++ b/src/utils/redisLoader.js @@ -93,7 +93,8 @@ export async function getProvisionReasonsForDataset ({ organisation, dataset }) return provisionReasons } -export async function isStatutoryDataset ({ organisation, dataset }) { +export async function isStatutoryDataset (requestParams = {}) { + const { organisation, dataset } = requestParams ?? {} const provisionReasons = await getProvisionReasonsForDataset({ organisation, dataset }) return provisionReasons.includes('statutory') } diff --git a/test/unit/columnMappingController.test.js b/test/unit/columnMappingController.test.js index 20c40249a..aaf599ec0 100644 --- a/test/unit/columnMappingController.test.js +++ b/test/unit/columnMappingController.test.js @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { applySubmittedFieldSelections, applyDetectedGeometryColumnMapping, @@ -9,10 +9,41 @@ import { buildSelectableColumns, detectGeometryColumnMapping, getBracketFields, + handleUnavailableColumnMappingRequest, validateColumnMapping } from '../../src/controllers/columnMappingController.js' +import { MiddlewareError } from '../../src/utils/errors.js' describe('columnMappingController helpers', () => { + it('returns a 404 when column mapping request params are missing', async () => { + const res = {} + const next = vi.fn() + + await handleUnavailableColumnMappingRequest({ + locals: { + requestData: { + getParams: () => ({ dataset: 'tree' }) + } + } + }, res, next) + + expect(next).toHaveBeenCalledWith(expect.any(MiddlewareError)) + expect(next.mock.calls[0][0].statusCode).toBe(404) + + vi.clearAllMocks() + + await handleUnavailableColumnMappingRequest({ + locals: { + requestData: { + getParams: () => ({ organisationName: 'local-authority:TST' }) + } + } + }, res, next) + + expect(next).toHaveBeenCalledWith(expect.any(MiddlewareError)) + expect(next.mock.calls[0][0].statusCode).toBe(404) + }) + it('builds rows from mapped, missing and unmapped columns', () => { const rows = buildColumnMappingRows({ columnFieldLog: [ diff --git a/test/unit/redisLoader.test.js b/test/unit/redisLoader.test.js index ca04522e7..1325fb68a 100644 --- a/test/unit/redisLoader.test.js +++ b/test/unit/redisLoader.test.js @@ -230,4 +230,37 @@ describe('getDatasetNameMap', () => { vi.resetModules() } }) + + it('should return false for missing statutory dataset params without querying Datasette', async () => { + const mockDatasette = { + default: { + runQuery: vi.fn() + } + } + + try { + vi.resetModules() + vi.doMock('../../config/index.js', () => ({ + default: { + redis: false, + mainWebsiteUrl: config.mainWebsiteUrl + } + })) + vi.doMock('../../src/services/datasette.js', () => mockDatasette) + + const { isStatutoryDataset } = await import('../../src/utils/redisLoader.js') + + await expect(isStatutoryDataset(null)).resolves.toBe(false) + await expect(isStatutoryDataset()).resolves.toBe(false) + await expect(isStatutoryDataset({ + organisation: 'local-authority:TST' + })).resolves.toBe(false) + + expect(mockDatasette.default.runQuery).not.toHaveBeenCalled() + } finally { + vi.unmock('../../config/index.js') + vi.unmock('../../src/services/datasette.js') + vi.resetModules() + } + }) }) From dcbe120de644858f94a561ec2cd0c568ab19bfea Mon Sep 17 00:00:00 2001 From: Gibah Joseph Date: Wed, 22 Jul 2026 16:04:37 +0100 Subject: [PATCH 2/2] test: add handling for unavailable column mapping requests --- test/unit/columnMappingController.test.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/unit/columnMappingController.test.js b/test/unit/columnMappingController.test.js index aaf599ec0..455ab1c6d 100644 --- a/test/unit/columnMappingController.test.js +++ b/test/unit/columnMappingController.test.js @@ -42,6 +42,15 @@ describe('columnMappingController helpers', () => { expect(next).toHaveBeenCalledWith(expect.any(MiddlewareError)) expect(next.mock.calls[0][0].statusCode).toBe(404) + + vi.clearAllMocks() + + await handleUnavailableColumnMappingRequest({ + locals: {} + }, res, next) + + expect(next).toHaveBeenCalledWith(expect.any(MiddlewareError)) + expect(next.mock.calls[0][0].statusCode).toBe(404) }) it('builds rows from mapped, missing and unmapped columns', () => {