Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply more specific date error messages #2022

Merged
merged 24 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
cda7d6c
Apply more specific date error messages
irisfaraway Aug 19, 2024
5687ff7
Apply new error messages to DOB form
irisfaraway Aug 19, 2024
ec23329
Alternate message for BOBO journey
irisfaraway Aug 20, 2024
92983cc
Changed our minds, no alt message
irisfaraway Aug 20, 2024
b3ae1de
Highlight invalid fields in red
irisfaraway Aug 20, 2024
a29c006
Only validate whole date if fields are valid
irisfaraway Aug 20, 2024
e1d58f9
Update DOB validation on renewals
irisfaraway Sep 2, 2024
51b7796
Add new validation to licence start time
irisfaraway Sep 2, 2024
e7ddd68
Split up licence start validator
irisfaraway Sep 3, 2024
67d2e45
Refactor to address Sonarcloud issues
irisfaraway Sep 19, 2024
988783a
Refactor for Sonarcloud, clarify field name
irisfaraway Sep 19, 2024
89d8557
Remove extra line break, actually trying to force Sonar to re-analyse
jaucourt Oct 25, 2024
397ec2c
Generalise date validation and apply to date of birth page
jaucourt Nov 5, 2024
53b6fd0
Amended invalid date validation as it wasn't doing what was expected …
jaucourt Nov 8, 2024
3502c60
Lint fix
jaucourt Nov 8, 2024
0551751
Refactor dateSchemaInput to map zero-length strings to undefined valu…
jaucourt Nov 13, 2024
8a950cb
Added validation for licence to start page
jaucourt Nov 15, 2024
7f0c55e
Remove blank lines in english resource file
jaucourt Nov 15, 2024
63ef199
Refactor date of birth and start date validators to be reusable funct…
jaucourt Nov 15, 2024
77f0833
Amend templates to have correct error class
jaucourt Nov 15, 2024
b6fc2fe
Improve test for licence to start page
jaucourt Nov 15, 2024
d182510
Make the error shown by the date input the same as in the error summary
jaucourt Nov 20, 2024
f891b50
Add function to get error flags for a date, to pass through to the te…
jaucourt Dec 9, 2024
54bfb0c
Renamed getErrorFlags function to getDateErrorFlags
jaucourt Dec 9, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { getData } from '../route'
import pageRoute from '../../../../routes/page-route.js'
import { nextPage } from '../../../../routes/next-page.js'
import { DATE_OF_BIRTH, LICENCE_FOR } from '../../../../uri.js'
import { dateOfBirthValidator } from '../../../../schema/validators/validators.js'
import { dateOfBirthValidator, getErrorFlags } from '../../../../schema/validators/validators.js'

jest.mock('../../../../routes/next-page.js')
jest.mock('../../../../routes/page-route.js')
Expand Down Expand Up @@ -81,6 +81,19 @@ describe('name > route', () => {
expect(result.error).toBeUndefined()
})

it('adds return value of getErrorFlags to the page data', async () => {
const errorFlags = { unique: Symbol('error-flags') }
getErrorFlags.mockReturnValueOnce(errorFlags)
const result = await getData(mockRequest())
expect(result).toEqual(expect.objectContaining(errorFlags))
})

it('passes error to getErrorFlags', async () => {
const error = Symbol('error')
await getData(mockRequest({ pageGet: async () => ({ error }) }))
expect(getErrorFlags).toHaveBeenCalledWith(error)
})

it('passes correct page name when getting page cache', async () => {
const pageGet = jest.fn(() => ({}))
await getData(mockRequest({ pageGet }))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
'invalid-date': {
'any.custom': { ref: '#date-of-birth-day', text: mssgs.dob_error_date_real }
},
'birthDate': {
'date-range': {
'date.min': { ref: '#date-of-birth-day', text: mssgs.dob_error_year_min },
'date.max': { ref: '#date-of-birth-day', text: mssgs.dob_error_year_max }
}
Expand All @@ -47,21 +47,21 @@
{
label: mssgs.dob_day,
name: 'day',
classes: "govuk-input--width-2 govuk-input--error" if error['full-date'] or error['day-and-month'] or error['day-and-year'] or error['day'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-2",
classes: "govuk-input--width-2 govuk-input--error" if data.isDayError else "govuk-input--width-2",
value: payload['date-of-birth-day'],
attributes: { maxlength : 2 }
},
{
label: mssgs.dob_month,
name: 'month',
classes: "govuk-input--width-2 govuk-input--error" if error['full-date'] or error['day-and-month'] or error['month-and-year'] or error['month'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-2",
classes: "govuk-input--width-2 govuk-input--error" if data.isMonthError else "govuk-input--width-2",
value: payload['date-of-birth-month'],
attributes: { maxlength : 2 }
},
{
label: mssgs.dob_year,
name: 'year',
classes: "govuk-input--width-4 govuk-input--error" if error['full-date'] or error['month-and-year'] or error['day-and-year'] or error['year'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-4",
classes: "govuk-input--width-4 govuk-input--error" if data.isYearError else "govuk-input--width-4",
value: payload['date-of-birth-year'],
attributes: { maxlength : 4 }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { DATE_OF_BIRTH, LICENCE_FOR } from '../../../uri.js'
import pageRoute from '../../../routes/page-route.js'
import { nextPage } from '../../../routes/next-page.js'
import GetDataRedirect from '../../../handlers/get-data-redirect.js'
import { dateOfBirthValidator } from '../../../schema/validators/validators.js'
import { dateOfBirthValidator, getErrorFlags } from '../../../schema/validators/validators.js'

const redirectToStartOfJourney = status => {
if (!status[LICENCE_FOR.page]) {
Expand All @@ -14,16 +14,16 @@ export const getData = async request => {
const { isLicenceForYou } = await request.cache().helpers.transaction.getCurrentPermission()
const status = await request.cache().helpers.status.getCurrentPermission()
const page = await request.cache().helpers.page.getCurrentPermission(DATE_OF_BIRTH.page)
const pageData = { isLicenceForYou, ...getErrorFlags(page?.error) }

redirectToStartOfJourney(status)

if (page?.error) {
const [errorKey] = Object.keys(page.error)
const errorValue = page.error[errorKey]

return { isLicenceForYou, error: { errorKey, errorValue } }
pageData.error = { errorKey, errorValue }
}
return { isLicenceForYou }
return pageData
}

export default pageRoute(DATE_OF_BIRTH.page, DATE_OF_BIRTH.uri, dateOfBirthValidator, nextPage, getData)
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import pageRoute from '../../../../routes/page-route.js'
import { nextPage } from '../../../../routes/next-page.js'
import { getData } from '../route'
import { LICENCE_TO_START } from '../../../../uri.js'
import { startDateValidator } from '../../../../schema/validators/validators.js'
import { startDateValidator, getErrorFlags } from '../../../../schema/validators/validators.js'

jest.mock('../../../../routes/next-page.js')
jest.mock('../../../../routes/page-route.js')
Expand All @@ -14,6 +14,7 @@ jest.mock('../../../../uri.js', () => ({
uri: Symbol('/licence-to-start')
}
}))
jest.mock('../../../../schema/validators/validators.js')

describe('licence-to-start > route', () => {
const getMockRequest = (isLicenceForYou = true, pageGet = () => {}) => ({
Expand All @@ -32,6 +33,10 @@ describe('licence-to-start > route', () => {
})

describe('getData', () => {
beforeEach(() => {
getErrorFlags.mockClear()
})

it('should return isLicenceForYou as true, if isLicenceForYou is true on the transaction cache', async () => {
const request = getMockRequest()
const result = await getData(request)
Expand Down Expand Up @@ -66,6 +71,19 @@ describe('licence-to-start > route', () => {
await getData(getMockRequest(undefined, pageGet))
expect(pageGet).toHaveBeenCalledWith(LICENCE_TO_START.page)
})

it('adds return value of getErrorFlags to the page data', async () => {
const errorFlags = { unique: Symbol('error-flags') }
getErrorFlags.mockReturnValueOnce(errorFlags)
const result = await getData(getMockRequest())
expect(result).toEqual(expect.objectContaining(errorFlags))
})

it('passes error to getErrorFlags', async () => {
const error = Symbol('error')
await getData(getMockRequest(undefined, async () => ({ error })))
expect(getErrorFlags).toHaveBeenCalledWith(error)
})
})

describe('default', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
'invalid-date': {
'any.custom': { ref: '#licence-start-date-day', text: mssgs.licence_start_error_date_real }
},
'startDate': {
'date-range': {
'date.min': { ref: '#licence-start-date-day', text: mssgs.licence_start_error_within + data.advancedPurchaseMaxDays + mssgs.licence_start_days },
'date.max': { ref: '#licence-start-date-day', text: mssgs.licence_start_error_within + data.advancedPurchaseMaxDays + mssgs.licence_start_days }
}
Expand All @@ -46,21 +46,21 @@
{
name: "day",
label: mssgs.dob_day,
classes: "govuk-input--width-2 govuk-input--error" if error['full-date'] or error['day-and-month'] or error['day-and-year'] or error['day'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-2",
classes: "govuk-input--width-2 govuk-input--error" if data.isDayError else "govuk-input--width-2",
value: payload['licence-start-date-day'],
attributes: { maxlength : 2 }
},
{
name: "month",
label: mssgs.dob_month,
classes: "govuk-input--width-2 govuk-input--error" if error['full-date'] or error['day-and-month'] or error['month-and-year'] or error['month'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-2",
classes: "govuk-input--width-2 govuk-input--error" if data.isMonthError else "govuk-input--width-2",
value: payload['licence-start-date-month'],
attributes: { maxlength : 2 }
},
{
name: "year",
label: mssgs.dob_year,
classes: "govuk-input--width-4 govuk-input--error" if error['full-date'] or error['month-and-year'] or error['day-and-year'] or error['year'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-4",
classes: "govuk-input--width-4 govuk-input--error" if data.isYearError else "govuk-input--width-4",
value: payload['licence-start-date-year'],
attributes: { maxlength : 4 }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { START_AFTER_PAYMENT_MINUTES, ADVANCED_PURCHASE_MAX_DAYS, SERVICE_LOCAL_
import { LICENCE_TO_START } from '../../../uri.js'
import pageRoute from '../../../routes/page-route.js'
import { nextPage } from '../../../routes/next-page.js'
import { startDateValidator } from '../../../schema/validators/validators.js'
import { getErrorFlags, startDateValidator } from '../../../schema/validators/validators.js'

export const getData = async request => {
const fmt = 'DD MM YYYY'
Expand All @@ -15,7 +15,8 @@ export const getData = async request => {
minStartDate: moment().tz(SERVICE_LOCAL_TIME).format(fmt),
maxStartDate: moment().tz(SERVICE_LOCAL_TIME).add(ADVANCED_PURCHASE_MAX_DAYS, 'days').format(fmt),
advancedPurchaseMaxDays: ADVANCED_PURCHASE_MAX_DAYS,
startAfterPaymentMinutes: START_AFTER_PAYMENT_MINUTES
startAfterPaymentMinutes: START_AFTER_PAYMENT_MINUTES,
...getErrorFlags(page?.error)
}

if (page?.error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ describe('The easy renewal identification page', () => {
referenceNumber: 'ABC123'
}),
setCurrentPermission: () => {}
},
page: {
getCurrentPermission: async () => ({})
}
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import pageRoute from '../../../../routes/page-route.js'
import { addLanguageCodeToUri } from '../../../../processors/uri-helper.js'
import { getData, validator } from '../route.js'
import { IDENTIFY, NEW_TRANSACTION } from '../../../../uri.js'
import { dateOfBirthValidator } from '../../../../schema/validators/validators.js'
import { dateOfBirthValidator, getErrorFlags } from '../../../../schema/validators/validators.js'

jest.mock('../../../../routes/page-route.js', () => jest.fn())
jest.mock('../../../../uri.js', () => ({
Expand All @@ -14,13 +14,16 @@ jest.mock('../../../../processors/uri-helper.js')
jest.mock('../../../../schema/validators/validators.js')

describe('getData', () => {
const getMockRequest = referenceNumber => ({
const getMockRequest = (referenceNumber, pageGet = async () => ({})) => ({
cache: () => ({
helpers: {
status: {
getCurrentPermission: () => ({
referenceNumber: referenceNumber
})
},
page: {
getCurrentPermission: pageGet
}
}
})
Expand All @@ -44,6 +47,25 @@ describe('getData', () => {
const result = await getData(getMockRequest(referenceNumber))
expect(result.referenceNumber).toEqual(referenceNumber)
})

it('adds return value of getErrorFlags to the page data', async () => {
const errorFlags = { unique: Symbol('error-flags') }
getErrorFlags.mockReturnValueOnce(errorFlags)
const result = await getData(getMockRequest())
expect(result).toEqual(expect.objectContaining(errorFlags))
})

it('passes error to getErrorFlags', async () => {
const error = Symbol('error')
await getData(getMockRequest(undefined, async () => ({ error })))
expect(getErrorFlags).toHaveBeenCalledWith(error)
})

it('passes correct page name when getting page cache', async () => {
const pageGet = jest.fn(() => ({}))
await getData(getMockRequest(undefined, pageGet))
expect(pageGet).toHaveBeenCalledWith(IDENTIFY.page)
})
})

describe('default', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
'invalid-date': {
'any.custom': { ref: '#date-of-birth-day', text: mssgs.dob_error_date_real }
},
'birthDate': {
'date-range': {
'date.min': { ref: '#date-of-birth-day', text: mssgs.dob_error_year_min },
'date.max': { ref: '#date-of-birth-day', text: mssgs.dob_error_year_max }
}
Expand All @@ -63,21 +63,21 @@
{
label: mssgs.dob_day,
name: "day",
classes: "govuk-input--width-2 govuk-input--error" if error['full-date'] or error['day-and-month'] or error['day-and-year'] or error['day'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-2",
classes: "govuk-input--width-2 govuk-input--error" if data.isDayError else "govuk-input--width-2",
value: payload['date-of-birth-day'],
attributes: { maxlength : 2 }
},
{
label: mssgs.dob_month,
name: "month",
classes: "govuk-input--width-2 govuk-input--error" if error['full-date'] or error['day-and-month'] or error['month-and-year'] or error['month'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-2",
classes: "govuk-input--width-2 govuk-input--error" if data.isMonthError else "govuk-input--width-2",
value: payload['date-of-birth-month'],
attributes: { maxlength : 2 }
},
{
label: mssgs.dob_year,
name: "year",
classes: "govuk-input--width-4 govuk-input--error" if error['full-date'] or error['month-and-year'] or error['day-and-year'] or error['year'] or error['invalid-date'] or error['birthDate'] else "govuk-input--width-4",
classes: "govuk-input--width-4 govuk-input--error" if data.isYearError else "govuk-input--width-4",
value: payload['date-of-birth-year'],
attributes: { maxlength : 4 }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import Joi from 'joi'
import { validation } from '@defra-fish/business-rules-lib'
import { addLanguageCodeToUri } from '../../../processors/uri-helper.js'
import GetDataRedirect from '../../../handlers/get-data-redirect.js'
import { dateOfBirthValidator } from '../../../schema/validators/validators.js'
import { dateOfBirthValidator, getErrorFlags } from '../../../schema/validators/validators.js'

export const getData = async request => {
// If we are supplied a permission number, validate it or throw 400
const permission = await request.cache().helpers.status.getCurrentPermission()
const page = await request.cache().helpers.page.getCurrentPermission(IDENTIFY.page)

if (permission.referenceNumber) {
const validatePermissionNumber = validation.permission
Expand All @@ -24,7 +25,8 @@ export const getData = async request => {
referenceNumber: permission.referenceNumber,
uri: {
new: addLanguageCodeToUri(request, NEW_TRANSACTION.uri)
}
},
...getErrorFlags(page?.error)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Joi from 'joi'
import { dateOfBirthValidator, startDateValidator } from '../validators.js'
import { dateOfBirthValidator, startDateValidator, getErrorFlags } from '../validators.js'
import moment from 'moment-timezone'
const dateSchema = require('../../date.schema.js')

Expand Down Expand Up @@ -180,3 +180,29 @@ describe('startDate validator', () => {
expect(() => startDateValidator(samplePayload)).toThrow()
})
})

describe('getErrorFlags', () => {
it('sets all error flags to be false when there are no errors', () => {
const result = getErrorFlags()
expect(result).toEqual({ isDayError: false, isMonthError: false, isYearError: false })
})

it.each([
['full-date', { isDayError: true, isMonthError: true, isYearError: true }],
['day-and-month', { isDayError: true, isMonthError: true, isYearError: false }],
['month-and-year', { isDayError: false, isMonthError: true, isYearError: true }],
['day-and-year', { isDayError: true, isMonthError: false, isYearError: true }],
['day', { isDayError: true, isMonthError: false, isYearError: false }],
['month', { isDayError: false, isMonthError: true, isYearError: false }],
['year', { isDayError: false, isMonthError: false, isYearError: true }],
['invalid-date', { isDayError: true, isMonthError: true, isYearError: true }],
['date-range', { isDayError: true, isMonthError: true, isYearError: true }],
['non-numeric', { isDayError: true, isMonthError: true, isYearError: true }]
])('when error is %s, should set %o in flags', (errorKey, expected) => {
const error = { [errorKey]: 'anything.at.all' }

const result = getErrorFlags(error)

expect(result).toEqual(expect.objectContaining(expected))
})
})
Loading
Loading