diff --git a/src/controllers/columnMappingController.js b/src/controllers/columnMappingController.js index 2c84d598..16402034 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 f8347927..fa20f79c 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 20c40249..455ab1c6 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,50 @@ 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) + + 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', () => { const rows = buildColumnMappingRows({ columnFieldLog: [ diff --git a/test/unit/redisLoader.test.js b/test/unit/redisLoader.test.js index ca04522e..1325fb68 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() + } + }) })