diff --git a/src/controllers/CheckAnswersController.js b/src/controllers/CheckAnswersController.js index 2749fe03..4034f806 100644 --- a/src/controllers/CheckAnswersController.js +++ b/src/controllers/CheckAnswersController.js @@ -10,7 +10,8 @@ import { getDatasets } from '../utils/utils.js' class CheckAnswersController extends PageController { async locals (req, res, next) { const requestId = req.sessionModel.get('requestId') - if (!requestId) { + const dataset = req.sessionModel.get('dataset') + if (!requestId || !dataset) { return res.redirect('/check/url') } try { diff --git a/src/controllers/datasetDetailsController.js b/src/controllers/datasetDetailsController.js index 7a39983e..808ee3b7 100644 --- a/src/controllers/datasetDetailsController.js +++ b/src/controllers/datasetDetailsController.js @@ -3,7 +3,8 @@ import PageController from './pageController.js' class DatasetDetailsController extends PageController { locals (req, res, next) { const requestId = req.sessionModel.get('requestId') - if (!requestId) { + const dataset = req.sessionModel.get('dataset') + if (!requestId || !dataset) { return res.redirect('/check/url') } super.locals(req, res, next) diff --git a/src/controllers/lpaDetailsController.js b/src/controllers/lpaDetailsController.js index 85c4b6a3..cb488428 100644 --- a/src/controllers/lpaDetailsController.js +++ b/src/controllers/lpaDetailsController.js @@ -9,11 +9,11 @@ class LpaDetailsController extends PageController { try { const requestData = await getRequestData(requestId) - if (requestData?.getParams()?.type !== 'check_url') { + const params = requestData?.getParams() + if (params?.type !== 'check_url' || !params.dataset) { return res.redirect('/check/url') } // Populate submit wizard session from the check request params - const params = requestData.getParams() const orgId = params.organisationName req.sessionModel.set('requestId', requestId) req.sessionModel.set('lpa', orgIdToName(orgId)) @@ -33,7 +33,7 @@ class LpaDetailsController extends PageController { value: name })) - req.form.options.lastPage = `/check/results/${req.session.checkRequestId}/1` + req.form.options.lastPage = `/check/results/${requestId}/1` super.locals(req, res, next) } diff --git a/src/controllers/pageController.js b/src/controllers/pageController.js index 30ba1d39..3f08c868 100644 --- a/src/controllers/pageController.js +++ b/src/controllers/pageController.js @@ -79,10 +79,12 @@ class PageController extends Controller { } const dataset = req?.sessionModel?.get('dataset') - try { - req.form.options.datasetName = datasetSlugToReadableName(dataset) - } catch (e) { - logger.warn(`Failed to get readable dataset name from slug: ${dataset}`) + if (dataset) { + try { + req.form.options.datasetName = datasetSlugToReadableName(dataset) + } catch (e) { + logger.warn(`Failed to get readable dataset name from slug: ${dataset}`) + } } const errors = req?.sessionModel?.get('errors') diff --git a/src/filters/makeDatasetSlugToReadableNameFilter.js b/src/filters/makeDatasetSlugToReadableNameFilter.js index d99b3dcd..49def78b 100644 --- a/src/filters/makeDatasetSlugToReadableNameFilter.js +++ b/src/filters/makeDatasetSlugToReadableNameFilter.js @@ -20,6 +20,11 @@ export const makeDatasetSlugToReadableNameFilter = (datasetNameMapping) => { const lowercaseFirst = (str) => str.charAt(0).toLowerCase() + str.slice(1) return (slug, capitalize = false) => { + if (!slug) { + logger.debug(`can't find a dataset name for ${slug}`) + return slug + } + const name = datasetNameMapping.get(slug) if (!name) { // ToDo: work out what to do here? potentially update it with data from datasette diff --git a/test/unit/PageController.test.js b/test/unit/PageController.test.js index 9a767ecb..b6076cc1 100644 --- a/test/unit/PageController.test.js +++ b/test/unit/PageController.test.js @@ -128,9 +128,11 @@ describe('PageController', () => { }) it('handles missing dataset gracefully', () => { - req.sessionModel.get.mockReturnValue({}) + vi.clearAllMocks() + req.sessionModel.get.mockReturnValue(undefined) pageController.locals(req, {}, vi.fn()) expect(req.form.options.datasetName).toBeUndefined() + expect(datasetSlugToReadableName.datasetSlugToReadableName).not.toHaveBeenCalled() }) it('handles datasetSlugToReadableName errors gracefully', () => { diff --git a/test/unit/checkAnswersController.test.js b/test/unit/checkAnswersController.test.js index a22734bf..a0fc7f32 100644 --- a/test/unit/checkAnswersController.test.js +++ b/test/unit/checkAnswersController.test.js @@ -37,6 +37,20 @@ describe('CheckAnswersController', () => { vi.clearAllMocks() }) + describe('locals', () => { + it('should redirect to /check/url when dataset is missing', async () => { + req.sessionModel.get.mockImplementation(key => ({ + requestId: 'existing-request-id' + }[key])) + + await controller.locals(req, res, next) + + expect(res.redirect).toHaveBeenCalledWith('/check/url') + expect(getRequestData).not.toHaveBeenCalled() + expect(next).not.toHaveBeenCalled() + }) + }) + describe('POST to CheckAnswersController', () => { it('should create a Jira issue and set session data on success', async () => { const issue = { issueKey: 'TEST-123' } diff --git a/test/unit/lpaDetailsController.test.js b/test/unit/lpaDetailsController.test.js index ab8ad6a5..ea24903a 100644 --- a/test/unit/lpaDetailsController.test.js +++ b/test/unit/lpaDetailsController.test.js @@ -88,6 +88,17 @@ describe('lpaDetailsController', async () => { expect(next).not.toHaveBeenCalled() }) + it('should redirect to /check/url when request params do not include a dataset', async () => { + getRequestData.mockResolvedValue({ + getParams: () => ({ type: 'check_url', organisationName: 'Mock LPA' }) + }) + + await controller.locals(req, res, next) + + expect(res.redirect).toHaveBeenCalledWith('/check/url') + expect(next).not.toHaveBeenCalled() + }) + it('should redirect to /check/url when getRequestData throws', async () => { getRequestData.mockRejectedValue(new Error('API error')) diff --git a/test/unit/makeDatasetSlugToReadableNameFilter.test.js b/test/unit/makeDatasetSlugToReadableNameFilter.test.js index b97f164f..11d4afac 100644 --- a/test/unit/makeDatasetSlugToReadableNameFilter.test.js +++ b/test/unit/makeDatasetSlugToReadableNameFilter.test.js @@ -24,4 +24,11 @@ describe('makeDatasetSlugToReadableNameFilter', () => { expect(filter('Unknown-slug')).toBe('unknown-slug') expect(filter('Unknown-slug', true)).toBe('Unknown-slug') }) + + it('returns a missing slug without trying to capitalize it', () => { + expect(filter(undefined)).toBeUndefined() + expect(filter(undefined, true)).toBeUndefined() + expect(filter(null)).toBeNull() + expect(filter(null, true)).toBeNull() + }) })