Skip to content

Commit

Permalink
SDIT-2256 PR updates and additional tests (#10)
Browse files Browse the repository at this point in the history
* SDIT-2256 PR updates and additional tests

* SDIT-2256 Fix linting issue
  • Loading branch information
karenmillermoj authored Nov 26, 2024
1 parent 7a2f0ba commit 7d7abab
Show file tree
Hide file tree
Showing 15 changed files with 62 additions and 25 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ To start the main services excluding the historical prisoner app:
Create an environment file by copying `.env.example` -> `.env`
Environment variables set in here will be available when running `start:dev`

Install dependencies using `npm install`, ensuring you are using `node v20`
Install dependencies using `npm install`, ensuring you are using `node v22`

Note: Using `nvm` (or [fnm](https://github.com/Schniz/fnm)), run `nvm install --latest-npm` within the repository folder
to use the correct version of node, and the latest version of npm. This matches the `engines` config in `package.json`
Expand Down
17 changes: 16 additions & 1 deletion integration_tests/e2e/disclaimer.cy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Page from '../pages/page'
import Disclaimer from '../pages/disclaimer'
import Search from '../pages/search'

context('Sign In', () => {
beforeEach(() => {
Expand All @@ -22,7 +23,21 @@ context('Sign In', () => {
navigateToDisclaimerPage()
const disclaimerPage = Page.verifyOnPage(Disclaimer)
disclaimerPage.confirmButton().click()
Page.verifyOnPage(Disclaimer)
const disclaimerPageUpdate = Page.verifyOnPage(Disclaimer)
disclaimerPageUpdate
.errorSummaryList()
.find('li')
.then($errors => {
expect($errors.get(0).innerText).to.contain('You must confirm that you understand the disclaimer')
})
})

it('Will successfully move to the search screen if disclaimer checkbox selected', () => {
navigateToDisclaimerPage()
const disclaimerPage = Page.verifyOnPage(Disclaimer)
disclaimerPage.disclaimerCheckbox.click()
disclaimerPage.confirmButton().click()
Page.verifyOnPage(Search)
})

const navigateToDisclaimerPage = () => {
Expand Down
19 changes: 19 additions & 0 deletions integration_tests/e2e/search.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import Page from '../pages/page'
import Disclaimer from '../pages/disclaimer'
import Search from '../pages/search'

context('Sign In', () => {
beforeEach(() => {
cy.task('reset')
cy.task('stubSignIn', { roles: ['ROLE_HPA_USER'] })
cy.signIn()
const disclaimerPage = Page.verifyOnPage(Disclaimer)
disclaimerPage.disclaimerCheckbox.click()
disclaimerPage.confirmButton().click()
})

it('Will show the search form with name/age selected', () => {
const prisonerSearchPage = Page.verifyOnPage(Search)
prisonerSearchPage.searchSelectRadioButton('Name/age').should('be.checked')
})
})
2 changes: 2 additions & 0 deletions integration_tests/pages/disclaimer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ export default class Disclaimer extends Page {

disclaimerCheckbox = cy.contains('label', 'I confirm that I understand').prev()

errorSummaryList = (): PageElement => cy.get('[data-module="govuk-error-summary"]')

confirmButton = (): PageElement => cy.get('button[type="submit"]')

headerUserName = (): PageElement => cy.get('[data-qa=header-user-name]')
Expand Down
9 changes: 9 additions & 0 deletions integration_tests/pages/search.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import Page from './page'

export default class Search extends Page {
constructor() {
super('Prisoner Search')
}

searchSelectRadioButton = text => cy.contains('label', text).prev()
}
2 changes: 1 addition & 1 deletion server/middleware/setUpAuthentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default function setupAuthentication() {

router.get('/sign-in/callback', (req, res, next) =>
passport.authenticate('oauth2', {
successReturnToOrRedirect: req.session.returnTo || '/disclaimer',
successReturnToOrRedirect: '/disclaimer',
failureRedirect: '/autherror',
})(req, res, next),
)
Expand Down
3 changes: 0 additions & 3 deletions server/middleware/validationMiddleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ export type fieldErrors = {
[field: string | number | symbol]: string[] | undefined
}

export const customErrorOrderBuilder = (errorSummaryList: { href: string }[], order: string[]) =>
order.map(key => errorSummaryList.find(error => error.href === `#${key}`)).filter(Boolean)

export const buildErrorSummaryList = (array: fieldErrors) => {
if (!array) return null

Expand Down
6 changes: 3 additions & 3 deletions server/routes/disclaimer/disclaimer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export default function routes(router: Router, auditService: AuditService): Rout

get('/disclaimer', async (req, res, next) => {
if (req.session.disclaimerConfirmed) {
res.render('pages/search')
res.redirect('search')
} else {
await auditService.logPageView(Page.LOG_IN, { who: res.locals.user.username, correlationId: req.id })
res.render('pages/disclaimer')
Expand All @@ -20,10 +20,10 @@ export default function routes(router: Router, auditService: AuditService): Rout
post('/disclaimer', async (req, res) => {
if (!req.body.disclaimer) {
res.status(400)
const validationErrors: HmppsError[] = [
const errors: HmppsError[] = [
{ href: '#disclaimer', text: 'You must confirm that you understand the disclaimer' },
]
res.render('pages/disclaimer', { validationErrors })
res.render('pages/disclaimer', { errors })
} else {
req.session.disclaimerConfirmed = true
logger.info('Disclaimer accepted - redirecting to search', { userId: res.locals.user.username })
Expand Down
2 changes: 1 addition & 1 deletion server/routes/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('GET /', () => {
return request(app)
.get('/')
.expect(res => {
expect(res.text).toContain('Redirecting to /disclaimer')
expect(res.text).toContain('Redirecting to /search')
})
})
})
2 changes: 1 addition & 1 deletion server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default function routes({ auditService, historicalPrisonerService }: Serv
const get = (path: string | string[], handler: RequestHandler) => router.get(path, asyncMiddleware(handler))

get('/', async (req, res, next) => {
res.redirect('/disclaimer')
return res.redirect('/search')
})

disclaimerRoutes(router, auditService)
Expand Down
8 changes: 2 additions & 6 deletions server/routes/search/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,9 @@ export default function routes(

get('/search', async (req, res, next) => {
logger.debug('search /')
if (req.session.disclaimerConfirmed) {
await auditService.logPageView(Page.SEARCH, { who: res.locals.user.username, correlationId: req.id })
await auditService.logPageView(Page.SEARCH, { who: res.locals.user.username, correlationId: req.id })

res.render('pages/search')
} else {
res.render('pages/disclaimer')
}
res.render('pages/search')
})

post('/search', async (req, res) => {
Expand Down
3 changes: 1 addition & 2 deletions server/utils/nunjucksSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import fs from 'fs'
import { initialiseName } from './utils'
import config from '../config'
import logger from '../../logger'
import { buildErrorSummaryList, customErrorOrderBuilder, findError } from '../middleware/validationMiddleware'
import { buildErrorSummaryList, findError } from '../middleware/validationMiddleware'

export default function nunjucksSetup(app: express.Express): void {
app.set('view engine', 'njk')
Expand Down Expand Up @@ -39,7 +39,6 @@ export default function nunjucksSetup(app: express.Express): void {
)
njkEnv.addFilter('buildErrorSummaryList', buildErrorSummaryList)
njkEnv.addFilter('findError', findError)
njkEnv.addFilter('customErrorOrderBuilder', customErrorOrderBuilder)
njkEnv.addFilter('initialiseName', initialiseName)
njkEnv.addFilter('assetMap', (url: string) => assetManifest[url] || url)
}
2 changes: 1 addition & 1 deletion server/views/pages/disclaimer.njk
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
text: 'I confirm that I understand'
}
],
errorMessage: validationErrors | findError('disclaimer')
errorMessage: errors | findError('disclaimer')
}) }}
</div>

Expand Down
6 changes: 3 additions & 3 deletions server/views/pages/search.njk
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
label: {
text: "First name or initial"
},
errorMessage: validationErrors.firstName
errorMessage: errors.firstName
}) }}
{{ govukInput({
id: "last-name",
Expand All @@ -33,7 +33,7 @@
label: {
text: "Surname"
},
errorMessage: validationErrors.contactByEmail
errorMessage: errors.contactByEmail
}) }}
{{ govukDateInput({
id: "date-of-birth",
Expand All @@ -52,7 +52,7 @@
label: {
text: "Or, enter an age or age range"
},
errorMessage: validationErrors.age,
errorMessage: errors.age,
hint: {
text: "For example, 32-34"
}
Expand Down
4 changes: 2 additions & 2 deletions server/views/partials/layout.njk
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% extends "govuk/template.njk" %}
{% from "govuk/components/error-summary/macro.njk" import govukErrorSummary %}
{% set errorSummaryList = validationErrors %}
{% set errorSummaryList = errors %}

{% block head %}
<link href="{{ '/assets/css/app.css' | assetMap }}" rel="stylesheet"/>
Expand All @@ -18,7 +18,7 @@
<div class="govuk-grid-column-two-thirds">
{{ govukErrorSummary({
titleText: "There is a problem",
errorList: (errorSummaryList | customErrorOrderBuilder(customErrorOrder)) if customErrorOrder else errorSummaryList,
errorList: errorSummaryList,
classes: 'govuk-!-margin-top-3 govuk-!-margin-bottom-0'
}) }}
</div>
Expand Down

0 comments on commit 7d7abab

Please sign in to comment.