Skip to content

Commit

Permalink
PP-12444 refactor settings layout
Browse files Browse the repository at this point in the history
- add `service-settings-pane` css class to enforce consistent width across all settings views
- remove `hint-and-body-width` css class
- update base settings layout to include the back link and error summary macros
- update existing views to conform with new layout behaviour
- refactor email notifications custom paragraph controller to match common code style
- fix layout bug where long words would break out of the system messages component
- fix layout bug where long task names would cause the task status to jump onto multiple lines
- fix controller bug where entering a welsh service name that fails validation would cause the 'remove welsh service name' button to unsuccessfully attempt to remove the english service name
  • Loading branch information
nlsteers committed Dec 12, 2024
1 parent 13228c3 commit 716ffa8
Show file tree
Hide file tree
Showing 45 changed files with 498 additions and 729 deletions.
1 change: 1 addition & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ pacts
.project
*.log
*.txt
.env
1 change: 0 additions & 1 deletion app/assets/sass/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,4 @@ $govuk-suppressed-warnings: (
@import "components/system-messages";
@import "components/spinner";
@import "components/service-settings";
@import "components/hint-and-body-width";
@import "components/fieldset-legend-width";
4 changes: 0 additions & 4 deletions app/assets/sass/components/hint-and-body-width.scss

This file was deleted.

7 changes: 7 additions & 0 deletions app/assets/sass/components/service-settings.scss
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
@include govuk-typography-weight-bold();
}

.service-settings-pane {
max-width: 630px;
}

.service-settings-inset-text--grey {
background-color: govuk-colour("light-grey");
}
Expand All @@ -45,6 +49,9 @@
}

.task-list {
.govuk-task-list__name-and-hint {
width: 70% !important;
}
a:visited, a:link {
color: govuk-colour("blue");
}
Expand Down
2 changes: 2 additions & 0 deletions app/assets/sass/components/system-messages.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

.system-message__text {
flex: 1;
overflow-wrap: break-word;
overflow: hidden;
}

.system-message__icon--success {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ const { response } = require('@utils/response')
const formatSimplifiedAccountPathsFor = require('@utils/simplified-account/format/format-simplified-account-paths-for')
const paths = require('@root/paths')
const { updateCustomParagraphByServiceIdAndAccountType } = require('@services/email.service')
const { validateOptionalField } = require('@utils/validation/server-side-form-validations')
const { body, validationResult } = require('express-validator')
const formatValidationErrors = require('@utils/simplified-account/format/format-validation-errors')
const CUSTOM_PARAGRAPH_MAX_LENGTH = 5000

function get (req, res) {
const account = req.account
const service = req.service

response(req, res, 'simplified-account/settings/email-notifications/custom-paragraph', {
customParagraphText: account.rawResponse.email_notifications.PAYMENT_CONFIRMED.template_body,
customParagraph: account.rawResponse.email_notifications.PAYMENT_CONFIRMED.template_body,
serviceName: service.name,
backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.emailNotifications.templates,
service.externalId, account.type),
Expand All @@ -22,22 +23,31 @@ function get (req, res) {
async function postEditCustomParagraph (req, res) {
const serviceExternalId = req.service.externalId
const accountType = req.account.type
const customParagraph = req.body['custom-paragraph']
const validations = [
body('customParagraph').trim().isLength({ max: CUSTOM_PARAGRAPH_MAX_LENGTH }).withMessage(`Custom paragraph name must be ${CUSTOM_PARAGRAPH_MAX_LENGTH} characters or fewer`)
]

const validationResult = validateOptionalField(customParagraph, CUSTOM_PARAGRAPH_MAX_LENGTH, 'custom paragraph')
if (!validationResult.valid) {
await Promise.all(validations.map(validation => validation.run(req)))
const errors = validationResult(req)

if (!errors.isEmpty()) {
const formattedErrors = formatValidationErrors(errors)
return response(req, res, 'simplified-account/settings/email-notifications/custom-paragraph', {
errors: {
customParagraph: validationResult.message
summary: formattedErrors.errorSummary,
formErrors: formattedErrors.formErrors
},
customParagraphText: customParagraph,
customParagraph: req.body.customParagraph,
serviceName: req.service.name,
backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.emailNotifications.templates,
req.service.externalId, accountType),
removeCustomParagraphLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.emailNotifications.removeCustomParagraph,
req.service.externalId, accountType)
})
}

await updateCustomParagraphByServiceIdAndAccountType(serviceExternalId, accountType, customParagraph)
const newCustomParagraph = req.body.customParagraph.trim()
await updateCustomParagraphByServiceIdAndAccountType(serviceExternalId, accountType, newCustomParagraph)
req.flash('messages', { state: 'success', icon: '✓', heading: 'Custom paragraph updated' })
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.emailNotifications.templates,
serviceExternalId, accountType))
Expand All @@ -48,6 +58,7 @@ async function postRemoveCustomParagraph (req, res) {
const accountType = req.account.type

await updateCustomParagraphByServiceIdAndAccountType(serviceExternalId, accountType, '')
req.flash('messages', { state: 'success', icon: '✓', heading: 'Custom paragraph removed' })
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.emailNotifications.templates,
serviceExternalId, accountType))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ describe('Controller: settings/email-notifications/templates/custom-paragraph',
})

it('should pass context data to the response method', () => {
expect(responseStub.args[0][3]).to.have.property('customParagraphText').to.equal('Do this next')
expect(responseStub.args[0][3]).to.have.property('customParagraph').to.equal('Do this next')
expect(responseStub.args[0][3]).to.have.property('serviceName').to.equal(SERVICE_NAME)
expect(responseStub.args[0][3]).to.have.property('backLink').to.contain(paths.simplifiedAccount.settings.emailNotifications.index)
})
})

describe('postRemoveCustomParagraph', () => {
before(() => {
const body = { 'custom-paragraph': 'a test custom paragraph' }
const body = { customParagraph: 'a test custom paragraph' }
setupTest(body)
customParagraphController.postRemoveCustomParagraph(req, res)
})
Expand All @@ -105,7 +105,7 @@ describe('Controller: settings/email-notifications/templates/custom-paragraph',

describe('postEditCustomParagraph', () => {
before(() => {
const body = { 'custom-paragraph': 'a test custom paragraph' }
const body = { customParagraph: 'a test custom paragraph' }
setupTest(body)
customParagraphController.postEditCustomParagraph(req, res)
})
Expand All @@ -125,7 +125,7 @@ describe('Controller: settings/email-notifications/templates/custom-paragraph',
describe('with validation error', () => {
const invalidText = 'hi'.repeat(5000)
before(() => {
const body = { 'custom-paragraph': invalidText }
const body = { customParagraph: invalidText }
setupTest(body)
customParagraphController.postEditCustomParagraph(req, res)
})
Expand All @@ -141,12 +141,23 @@ describe('Controller: settings/email-notifications/templates/custom-paragraph',
})

it('should pass context data to the response method', () => {
expect(responseStub.args[0][3]).to.have.property('errors').to.deep.equal({
customParagraph: 'Custom paragraph must be 5000 characters or fewer'
})
expect(responseStub.args[0][3]).to.have.property('customParagraphText').to.equal(invalidText)
const errors = responseStub.args[0][3].errors
expect(errors.summary).to.deep.equal(
[
{
text: 'Custom paragraph name must be 5000 characters or fewer',
href: '#custom-paragraph'
}
])
expect(errors.formErrors).to.deep.equal(
{
customParagraph: 'Custom paragraph name must be 5000 characters or fewer'
}
)
expect(responseStub.args[0][3]).to.have.property('customParagraph').to.equal(invalidText)
expect(responseStub.args[0][3]).to.have.property('serviceName').to.equal(SERVICE_NAME)
expect(responseStub.args[0][3]).to.have.property('backLink').to.contain(paths.simplifiedAccount.settings.emailNotifications.index)
expect(responseStub.args[0][3]).to.have.property('removeCustomParagraphLink').to.contain(paths.simplifiedAccount.settings.emailNotifications.removeCustomParagraph)
})
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ function getEditServiceName (req, res) {
async function postEditServiceName (req, res) {
const editCy = req.body.cy === 'true'
const validations = [
body('serviceNameInput').trim().isLength({ max: SERVICE_NAME_MAX_LENGTH }).withMessage(`Service name must be ${SERVICE_NAME_MAX_LENGTH} characters or fewer`)
body('serviceName').trim().isLength({ max: SERVICE_NAME_MAX_LENGTH }).withMessage(`Service name must be ${SERVICE_NAME_MAX_LENGTH} characters or fewer`)
]
// we don't check presence for welsh names
if (!editCy) {
validations.push(body('serviceNameInput').trim().notEmpty().withMessage('Service name is required'))
validations.push(body('serviceName').trim().notEmpty().withMessage('Service name is required'))
}

await Promise.all(validations.map(validation => validation.run(req)))
Expand All @@ -53,13 +53,14 @@ async function postEditServiceName (req, res) {
formErrors: formattedErrors.formErrors
},
editCy,
serviceName: req.body.serviceNameInput,
serviceName: req.body.serviceName,
backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.serviceName.index, req.service.externalId, req.account.type),
submitLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.serviceName.edit, req.service.externalId, req.account.type)
submitLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.serviceName.edit, req.service.externalId, req.account.type),
removeCyLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.serviceName.removeCy, req.service.externalId, req.account.type)
})
}

const newServiceName = req.body.serviceNameInput.trim()
const newServiceName = req.body.serviceName.trim()
editCy ? await updateServiceName(req.service.externalId, req.service.serviceName.en, newServiceName) : await updateServiceName(req.service.externalId, newServiceName, req.service.serviceName.cy)
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.serviceName.index, req.service.externalId, req.account.type))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ describe('Controller: settings/service-name', () => {
before(() => {
setupTest('postEditServiceName', {}, {
body: {
serviceNameInput: 'New English Name',
serviceName: 'New English Name',
cy: 'false'
}
})
Expand All @@ -143,7 +143,7 @@ describe('Controller: settings/service-name', () => {
before(() => {
setupTest('postEditServiceName', {}, {
body: {
serviceNameInput: 'Enw Cymraeg newydd',
serviceName: 'Enw Cymraeg newydd',
cy: 'true'
}
})
Expand Down Expand Up @@ -186,6 +186,7 @@ describe('Controller: settings/service-name', () => {
expect(responseStub.calledOnce).to.be.true // eslint-disable-line
const [, , template, context] = responseStub.args[0]
expect(template).to.equal('simplified-account/settings/service-name/edit-service-name')
expect(context).to.have.property('removeCyLink').to.contain(paths.simplifiedAccount.settings.serviceName.removeCy)
expect(context.errors).to.deep.equal({
summary: ['Error summary'],
formErrors: { serviceNameInput: 'Error message' }
Expand Down
13 changes: 9 additions & 4 deletions app/simplified-account-routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,20 @@ simplifiedAccount.get(paths.simplifiedAccount.settings.cardTypes.index, permissi
// stripe details
const stripeDetailsPath = paths.simplifiedAccount.settings.stripeDetails
const stripeDetailsRouter = new Router({ mergeParams: true })
.use(enforcePaymentProviderType(STRIPE), permission('stripe-account-details:update'))
.use(enforceLiveAccountOnly, enforcePaymentProviderType(STRIPE), permission('stripe-account-details:update'))
stripeDetailsRouter.get(stripeDetailsPath.index, serviceSettingsController.stripeDetails.get)

stripeDetailsRouter.get(stripeDetailsPath.bankAccount, serviceSettingsController.stripeDetails.bankAccount.get)
stripeDetailsRouter.post(stripeDetailsPath.bankAccount, serviceSettingsController.stripeDetails.bankAccount.post)
// -- new stuff

stripeDetailsRouter.get(stripeDetailsPath.companyNumber, serviceSettingsController.stripeDetails.companyNumber.get)
stripeDetailsRouter.post(stripeDetailsPath.companyNumber, serviceSettingsController.stripeDetails.companyNumber.post)

stripeDetailsRouter.get(stripeDetailsPath.organisationDetails.index, serviceSettingsController.stripeDetails.organisationDetails.get)
stripeDetailsRouter.post(stripeDetailsPath.organisationDetails.index, serviceSettingsController.stripeDetails.organisationDetails.post)
stripeDetailsRouter.get(stripeDetailsPath.organisationDetails.update, serviceSettingsController.stripeDetails.organisationDetails.update.get)
stripeDetailsRouter.post(stripeDetailsPath.organisationDetails.update, serviceSettingsController.stripeDetails.organisationDetails.update.post)
// -- responsible person

stripeDetailsRouter.get(stripeDetailsPath.responsiblePerson.index, serviceSettingsController.stripeDetails.responsiblePerson.get)
stripeDetailsRouter.post(stripeDetailsPath.responsiblePerson.index, serviceSettingsController.stripeDetails.responsiblePerson.post)
stripeDetailsRouter.get(stripeDetailsPath.responsiblePerson.homeAddress, serviceSettingsController.stripeDetails.responsiblePerson.homeAddress.get)
Expand All @@ -95,13 +97,16 @@ stripeDetailsRouter.get(stripeDetailsPath.responsiblePerson.contactDetails, serv
stripeDetailsRouter.post(stripeDetailsPath.responsiblePerson.contactDetails, serviceSettingsController.stripeDetails.responsiblePerson.contactDetails.post)
stripeDetailsRouter.get(stripeDetailsPath.responsiblePerson.checkYourAnswers, serviceSettingsController.stripeDetails.responsiblePerson.checkYourAnswers.get)
stripeDetailsRouter.post(stripeDetailsPath.responsiblePerson.checkYourAnswers, serviceSettingsController.stripeDetails.responsiblePerson.checkYourAnswers.post)
// --

stripeDetailsRouter.get(stripeDetailsPath.vatNumber, serviceSettingsController.stripeDetails.vatNumber.get)
stripeDetailsRouter.post(stripeDetailsPath.vatNumber, serviceSettingsController.stripeDetails.vatNumber.post)

stripeDetailsRouter.get(stripeDetailsPath.director, serviceSettingsController.stripeDetails.director.get)
stripeDetailsRouter.post(stripeDetailsPath.director, serviceSettingsController.stripeDetails.director.post)

stripeDetailsRouter.get(stripeDetailsPath.governmentEntityDocument, serviceSettingsController.stripeDetails.governmentEntityDocument.get)
stripeDetailsRouter.post(stripeDetailsPath.governmentEntityDocument, [upload.single(GOV_ENTITY_DOC_FORM_FIELD_NAME), ...serviceSettingsController.stripeDetails.governmentEntityDocument.post])

simplifiedAccount.use(stripeDetailsRouter)

module.exports = simplifiedAccount
Loading

0 comments on commit 716ffa8

Please sign in to comment.