Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/controllers/CheckAnswersController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion src/controllers/datasetDetailsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/controllers/lpaDetailsController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)
}
Expand Down
10 changes: 6 additions & 4 deletions src/controllers/pageController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
5 changes: 5 additions & 0 deletions src/filters/makeDatasetSlugToReadableNameFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion test/unit/PageController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
14 changes: 14 additions & 0 deletions test/unit/checkAnswersController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' }
Expand Down
11 changes: 11 additions & 0 deletions test/unit/lpaDetailsController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'))

Expand Down
7 changes: 7 additions & 0 deletions test/unit/makeDatasetSlugToReadableNameFilter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
Loading