Skip to content

Commit

Permalink
PP-12444 unswallow unhandled errors (#4408)
Browse files Browse the repository at this point in the history
- rename `/bank-account` path to `/bank-details` in stripe details settings
- prevent unhandled errors in stripe details controllers from being swallowed
- update cypress tests to account for additional stubbing
  • Loading branch information
nlsteers authored Jan 15, 2025
1 parent 87169d9 commit 22f0b5c
Show file tree
Hide file tree
Showing 19 changed files with 167 additions and 132 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@ const formatValidationErrors = require('@utils/simplified-account/format/format-
const { checkTaskCompletion } = require('@middleware/simplified-account')
const { response } = require('@utils/response')
const { body, validationResult } = require('express-validator')
const { updateStripeDetailsBankAccount } = require('@services/stripe-details.service')
const { updateStripeDetailsBankDetails } = require('@services/stripe-details.service')
const { stripeDetailsTasks } = require('@utils/simplified-account/settings/stripe-details/tasks')

const ACCT_NUMBER_ERR_MSG = 'Enter a valid account number like 00733445'

async function get (req, res) {
return response(req, res, 'simplified-account/settings/stripe-details/bank-account/index', {
backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type),
submitLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.bankAccount, req.service.externalId, req.account.type)
submitLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.bankDetails, req.service.externalId, req.account.type)
})
}

Expand All @@ -32,38 +32,37 @@ async function post (req, res, next) {
if (!errors.isEmpty()) {
const formattedErrors = formatValidationErrors(errors)
return postErrorResponse(req, res, {
summary: formattedErrors.errorSummary,
formErrors: formattedErrors.formErrors
summary: formattedErrors.errorSummary, formErrors: formattedErrors.formErrors
})
}

const sortCode = req.body.sortCode.replace(/\D/g, '')
const accountNumber = req.body.accountNumber.replace(/\D/g, '')

try {
await updateStripeDetailsBankAccount(req.service, req.account, sortCode, accountNumber)
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
} catch (error) {
// handle Stripe API errors
switch (error.code) {
case 'bank_account_unusable':
return postErrorResponse(req, res, {
summary: [{ text: 'The bank account provided cannot be used. Contact GOV.UK Pay for assistance.' }]
})
case 'routing_number_invalid':
return postErrorResponse(req, res, {
summary: [{ text: 'Invalid sort code', href: '#sort-code' }],
formErrors: { sortCode: 'The sort code provided is invalid' }
})
case 'account_number_invalid':
return postErrorResponse(req, res, {
summary: [{ text: 'Invalid account number', href: '#account-number' }],
formErrors: { accountNumber: 'The account number provided is invalid' }
})
default:
next(error)
}
}
updateStripeDetailsBankDetails(req.service, req.account, sortCode, accountNumber)
.then(() => {
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
})
.catch((err) => {
switch (err.code) {
case 'bank_account_unusable':
return postErrorResponse(req, res, {
summary: [{ text: 'The bank account provided cannot be used. Contact GOV.UK Pay for assistance.' }]
})
case 'routing_number_invalid':
return postErrorResponse(req, res, {
summary: [{ text: 'Invalid sort code', href: '#sort-code' }],
formErrors: { sortCode: 'The sort code provided is invalid' }
})
case 'account_number_invalid':
return postErrorResponse(req, res, {
summary: [{ text: 'Invalid account number', href: '#account-number' }],
formErrors: { accountNumber: 'The account number provided is invalid' }
})
default:
next(err)
}
})
}

const postErrorResponse = (req, res, errors) => {
Expand All @@ -72,7 +71,7 @@ const postErrorResponse = (req, res, errors) => {
sortCode: req.body.sortCode,
accountNumber: req.body.accountNumber,
backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type),
submitLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.bankAccount, req.service.externalId, req.account.type)
submitLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.bankDetails, req.service.externalId, req.account.type)
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,25 @@ const ACCOUNT_TYPE = 'test'
const SERVICE_ID = 'service-id-123abc'

const STRIPE_DETAILS_INDEX_PATH = formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, SERVICE_ID, ACCOUNT_TYPE)
const STRIPE_DETAILS_BANK_ACCOUNT_PATH = formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.bankAccount, SERVICE_ID, ACCOUNT_TYPE)
const STRIPE_DETAILS_BANK_ACCOUNT_PATH = formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.bankDetails, SERVICE_ID, ACCOUNT_TYPE)

let req, res, next, responseStub, updateStripeDetailsBankAccountStub, bankAccountController
let req, res, next, responseStub, updateStripeDetailsBankDetailsStub, bankDetailsController

const getController = (stubs = {}) => {
return proxyquire('./bank-account.controller', {
return proxyquire('./bank-details.controller', {
'@utils/response': { response: stubs.response },
'@services/stripe-details.service': {
updateStripeDetailsBankAccount: stubs.updateStripeDetailsBankAccount
updateStripeDetailsBankDetails: stubs.updateStripeDetailsBankDetails
}
})
}

const setupTest = (method, additionalStubs = {}, additionalResProps = {}, additionalReqProps = {}) => {
responseStub = sinon.spy()
updateStripeDetailsBankAccountStub = sinon.stub().resolves()
bankAccountController = getController({
updateStripeDetailsBankDetailsStub = sinon.stub().resolves()
bankDetailsController = getController({
response: responseStub,
updateStripeDetailsBankAccount: updateStripeDetailsBankAccountStub,
updateStripeDetailsBankDetails: updateStripeDetailsBankDetailsStub,
...additionalStubs
})
res = {
Expand All @@ -43,10 +43,10 @@ const setupTest = (method, additionalStubs = {}, additionalResProps = {}, additi
...additionalReqProps
}
next = sinon.spy()
bankAccountController[method][1](req, res, next)
bankDetailsController[method][1](req, res, next)
}

describe('Controller: settings/stripe-details/bank-account', () => {
describe('Controller: settings/stripe-details/bank-details', () => {
describe('get', () => {
before(() => setupTest('get'))

Expand Down Expand Up @@ -78,7 +78,7 @@ describe('Controller: settings/stripe-details/bank-account', () => {
}))

it('should submit bank details to the stripe details service', () => {
expect(updateStripeDetailsBankAccountStub.calledWith(req.service, req.account, VALID_SORT_CODE, VALID_ACCOUNT_NUMBER)).to.be.true // eslint-disable-line
expect(updateStripeDetailsBankDetailsStub.calledWith(req.service, req.account, VALID_SORT_CODE, VALID_ACCOUNT_NUMBER)).to.be.true // eslint-disable-line
})

it('should redirect to the stripe details index page', () => {
Expand Down Expand Up @@ -181,7 +181,7 @@ describe('Controller: settings/stripe-details/bank-account', () => {
describe('for unusable bank account', () => {
before(() => {
setupTest('post',
{ updateStripeDetailsBankAccount: sinon.stub().rejects({ code: 'bank_account_unusable' }) },
{ updateStripeDetailsBankDetails: sinon.stub().rejects({ code: 'bank_account_unusable' }) },
{},
{ body: { sortCode: VALID_SORT_CODE, accountNumber: VALID_ACCOUNT_NUMBER } }
)
Expand All @@ -199,7 +199,7 @@ describe('Controller: settings/stripe-details/bank-account', () => {
describe('for invalid sort code', () => {
before(() => {
setupTest('post',
{ updateStripeDetailsBankAccount: sinon.stub().rejects({ code: 'routing_number_invalid' }) },
{ updateStripeDetailsBankDetails: sinon.stub().rejects({ code: 'routing_number_invalid' }) },
{},
{ body: { sortCode: VALID_SORT_CODE, accountNumber: VALID_ACCOUNT_NUMBER } }
)
Expand All @@ -218,7 +218,7 @@ describe('Controller: settings/stripe-details/bank-account', () => {
describe('for invalid account number', () => {
before(() => {
setupTest('post',
{ updateStripeDetailsBankAccount: sinon.stub().rejects({ code: 'account_number_invalid' }) },
{ updateStripeDetailsBankDetails: sinon.stub().rejects({ code: 'account_number_invalid' }) },
{},
{ body: { sortCode: VALID_SORT_CODE, accountNumber: VALID_ACCOUNT_NUMBER } }
)
Expand All @@ -237,7 +237,7 @@ describe('Controller: settings/stripe-details/bank-account', () => {
describe('for unhandled errors with codes', () => {
before(() => {
setupTest('post',
{ updateStripeDetailsBankAccount: sinon.stub().rejects({ code: 'unhandled_error' }) },
{ updateStripeDetailsBankDetails: sinon.stub().rejects({ code: 'unhandled_error' }) },
{},
{ body: { sortCode: VALID_SORT_CODE, accountNumber: VALID_ACCOUNT_NUMBER } }
)
Expand All @@ -252,7 +252,7 @@ describe('Controller: settings/stripe-details/bank-account', () => {
describe('for any other errors', () => {
before(() => {
setupTest('post',
{ updateStripeDetailsBankAccount: sinon.stub().rejects({ foo: 'bar' }) },
{ updateStripeDetailsBankDetails: sinon.stub().rejects({ foo: 'bar' }) },
{},
{ body: { sortCode: VALID_SORT_CODE, accountNumber: VALID_ACCOUNT_NUMBER } }
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,20 @@ async function post (req, res, next) {
}
}

try {
await updateStripeDetailsCompanyNumber(req.service, req.account, hasCompanyNumber
? req.body.companyNumber.replace(/\s/g, '').toUpperCase()
: false)
} catch (err) {
if (err.type === 'StripeInvalidRequestError') {
return postErrorResponse(req, res, {
summary: [{ text: 'There is a problem with your Company number. Please check your answer and try again.' }]
})
}
next(err)
}
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
updateStripeDetailsCompanyNumber(req.service, req.account, hasCompanyNumber
? req.body.companyNumber.replace(/\s/g, '').toUpperCase()
: false)
.then(() => {
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
})
.catch((err) => {
if (err.type === 'StripeInvalidRequestError') {
return postErrorResponse(req, res, {
summary: [{ text: 'There is a problem with your Company number. Please check your answer and try again.' }]
})
}
next(err)
})
}

const postErrorResponse = (req, res, errors) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,18 @@ async function post (req, res, next) {
email: req.body.workEmail.trim()
}

try {
await updateStripeDetailsDirector(req.service, req.account, director)
} catch (err) {
if (err.type === 'StripeInvalidRequestError') {
return postErrorResponse(req, res, {
summary: [{ text: 'There is a problem with the information you\'ve submitted. We\'ve not been able to save your details. Email govuk-pay-support@digital.cabinet-office.gov.uk for help.' }]
})
}
next(err)
}
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
updateStripeDetailsDirector(req.service, req.account, director)
.then(() => {
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
})
.catch((err) => {
if (err.type === 'StripeInvalidRequestError') {
return postErrorResponse(req, res, {
summary: [{ text: 'There is a problem with the information you\'ve submitted. We\'ve not been able to save your details. Email govuk-pay-support@digital.cabinet-office.gov.uk for help.' }]
})
}
next(err)
})
}

const postErrorResponse = (req, res, errors) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,29 @@ async function post (req, res, next) {
formErrors: formattedErrors.formErrors
})
}
try {
const file = req.file
await updateStripeDetailsUploadEntityDocument(req.service, req.account, file)
} catch (err) {
if (err.type === 'StripeInvalidRequestError' && err.param === 'file') {
return postErrorResponse(req, res, {
summary: [{ text: 'Error uploading file to stripe. Try uploading a file with one of the following types: pdf, jpeg, png', href: '#government-entity-document' }]

const file = req.file
updateStripeDetailsUploadEntityDocument(req.service, req.account, file)
.then(() => {
req.flash('messages', {
state: 'success',
icon: '✓',
heading: 'Service connected to Stripe',
body: 'This service can now take payments'
})
}
return next(err)
}
req.flash('messages', { state: 'success', icon: '✓', heading: 'Service connected to Stripe', body: 'This service can now take payments' })
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
})
.catch((err) => {
if (err.type === 'StripeInvalidRequestError' && err.param === 'file') {
return postErrorResponse(req, res, {
summary: [{
text: 'Error uploading file to stripe. Try uploading a file with one of the following types: pdf, jpeg, png',
href: '#government-entity-document'
}]
})
}
next(err)
})
}

const postErrorResponse = (req, res, errors) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ async function post (req, res) {
...(addressLine2 && { address_line2: addressLine2 })
}

await updateStripeDetailsOrganisationNameAndAddress(req.service, req.account, newOrgDetails)
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
updateStripeDetailsOrganisationNameAndAddress(req.service, req.account, newOrgDetails)
.then(() => {
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
})
}

const postErrorResponse = (req, res, errors) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ async function post (req, res) {
}
const isCorrect = req.body.confirmOrgDetails === 'true'
if (isCorrect) {
await updateConnectorStripeProgress(req.service, req.account, 'organisation_details')
updateConnectorStripeProgress(req.service, req.account, 'organisation_details')
.then(() => {
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
})
} else {
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.organisationDetails.update, req.service.externalId, req.account.type))
}
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
}

const postErrorResponse = (req, res, errors) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,25 +44,26 @@ async function post (req, res, next) {
address_postcode: address.homeAddressPostcode,
...(address.homeAddressLine2 && { address_line2: address.homeAddressLine2 })
}
try {
await updateStripeDetailsResponsiblePerson(req.service, req.account, responsiblePerson)
} catch (err) {
if (err.type === 'StripeInvalidRequestError') {
switch (err.param) {
case 'phone':
return postErrorResponse(req, res, {
summary: [{ text: 'There is a problem with your telephone number. Please check your answer and try again.' }]
})
default:
return postErrorResponse(req, res, {
summary: [{ text: 'There is a problem with the information you\'ve submitted. We\'ve not been able to save your details. Email govuk-pay-support@digital.cabinet-office.gov.uk for help.' }]
})
updateStripeDetailsResponsiblePerson(req.service, req.account, responsiblePerson)
.then(() => {
_.unset(req, FORM_STATE_KEY)
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
})
.catch((err) => {
if (err.type === 'StripeInvalidRequestError') {
switch (err.param) {
case 'phone':
return postErrorResponse(req, res, {
summary: [{ text: 'There is a problem with your telephone number. Please check your answer and try again.' }]
})
default:
return postErrorResponse(req, res, {
summary: [{ text: 'There is a problem with the information you\'ve submitted. We\'ve not been able to save your details. Email govuk-pay-support@digital.cabinet-office.gov.uk for help.' }]
})
}
}
}
next(err)
}
_.unset(req, FORM_STATE_KEY)
res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.index, req.service.externalId, req.account.type))
next(err)
})
}

const postErrorResponse = (req, res, errors) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ async function get (req, res) {
}

module.exports.get = get
module.exports.bankAccount = require('./bank-account/bank-account.controller')
module.exports.bankDetails = require('./bank-details/bank-details.controller')
module.exports.companyNumber = require('./company-number/company-number.controller')
module.exports.director = require('./director/director.controller')
module.exports.governmentEntityDocument = require('./government-entity-document/government-entity-document.controller')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('Controller: settings/stripe-details', () => {
expect(stripeDetailsTasks).to.have.all.keys('bankAccount', 'vatNumber', 'governmentEntityDocument')
expect(stripeDetailsTasks.bankAccount).to.deep.equal({
friendlyName: 'Organisation\'s bank details',
href: `/simplified/service/${SERVICE_ID}/account/${ACCOUNT_TYPE}/settings/stripe-details/bank-account`,
href: `/simplified/service/${SERVICE_ID}/account/${ACCOUNT_TYPE}/settings/stripe-details/bank-details`,
status: true
})
expect(stripeDetailsTasks.vatNumber).to.deep.equal({
Expand Down
Loading

0 comments on commit 22f0b5c

Please sign in to comment.