Skip to content

Commit

Permalink
PP-13312 simplified account settings - card types (#4383)
Browse files Browse the repository at this point in the history
- card types settings controller and view

---------

Co-authored-by: Nathaniel Steers <[email protected]>
  • Loading branch information
james-peacock-gds and nlsteers authored Dec 19, 2024
1 parent c9643af commit 416c4cb
Show file tree
Hide file tree
Showing 11 changed files with 298 additions and 104 deletions.
11 changes: 11 additions & 0 deletions app/assets/sass/components/service-settings.scss
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,14 @@
color: govuk-colour("black");
}
}

.card-types {
.govuk-checkboxes__item {
border-bottom: 1px solid govuk-colour("mid-grey");
padding-bottom: govuk-spacing(2);
}

.govuk-summary-list__row:first-of-type {
border-top: 1px solid govuk-colour("mid-grey");
}
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,90 @@
const { response } = require('@utils/response')
const { formatCardTypesForTemplate } = require('@utils/simplified-account/format/format-card-types')
const { getAllCardTypes, getAcceptedCardTypesForServiceAndAccountType } = require('@services/card-types.service')
const { getAllCardTypes, getAcceptedCardTypesForServiceAndAccountType, postAcceptedCardsForServiceAndAccountType } = require('@services/card-types.service')
const { formatSimplifiedAccountPathsFor } = require('@utils/simplified-account/format')
const { body, validationResult } = require('express-validator')
const paths = require('@root/paths')
const formatValidationErrors = require('@utils/simplified-account/format/format-validation-errors')

async function get (req, res, next) {
const serviceId = req.service.externalId
const serviceExternalId = req.service.externalId
const accountType = req.account.type
const isAdminUser = req.user.isAdminUserForService(serviceId)
const isAdminUser = req.user.isAdminUserForService(serviceExternalId)
const messages = res.locals?.flash?.messages ?? []
try {
const { card_types: allCards } = await getAllCardTypes()
const { card_types: acceptedCards } = await getAcceptedCardTypesForServiceAndAccountType(serviceId, accountType)
const { card_types: acceptedCards } = await getAcceptedCardTypesForServiceAndAccountType(serviceExternalId, accountType)
const currentAcceptedCardTypeIds = acceptedCards.map(card => card.id)
const cardTypes = formatCardTypesForTemplate(allCards, acceptedCards, req.account, isAdminUser)
response(req, res, 'simplified-account/settings/card-types/index',
{ cardTypes, isAdminUser })
response(req, res, 'simplified-account/settings/card-types/index', {
messages,
cardTypes,
isAdminUser,
currentAcceptedCardTypeIds
})
} catch (err) {
next(err)
}
}

async function post (req, res, next) {
const serviceExternalId = req.service.externalId
const accountType = req.account.type
const currentAcceptedCardTypeIds = req.body.currentAcceptedCardTypeIds?.split(',') || []

let selectedCardTypeIds
const sanitiseToArray = value => Array.isArray(value) ? value : (value ? [value] : [])
const validations = [
body()
.custom((_, { req }) => {
selectedCardTypeIds = sanitiseToArray(req.body.debit).concat(sanitiseToArray(req.body.credit))
if (selectedCardTypeIds < 1) {
throw new Error('You must choose at least one card')
}
return true
})
]
await Promise.all(validations.map(validation => validation.run(req)))
const validationErrors = validationResult(req)
if (!validationErrors.isEmpty()) {
const { card_types: allCards } = await getAllCardTypes()
const cardTypes = formatCardTypesForTemplate(allCards, [], req.account, true)
const formattedValidationErrors = formatValidationErrors(validationErrors)
return response(req, res, 'simplified-account/settings/card-types/index', {
errors: {
summary: formattedValidationErrors.errorSummary
},
cardTypes,
isAdminUser: true,
currentAcceptedCardTypeIds
})
}

const noChangesToAcceptedCardTypes = (
currentAcceptedCardTypeIds.length === selectedCardTypeIds.length &&
currentAcceptedCardTypeIds.every(item => selectedCardTypeIds.includes(item))
)
if (noChangesToAcceptedCardTypes) {
return res.redirect(
formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.cardTypes.index, serviceExternalId, accountType)
)
}

try {
const payload = {
card_types: selectedCardTypeIds
}
await postAcceptedCardsForServiceAndAccountType(serviceExternalId, accountType, payload)
req.flash('messages', { state: 'success', icon: '&check;', heading: 'Accepted card types have been updated' })
return res.redirect(
formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.cardTypes.index, serviceExternalId, accountType)
)
} catch (err) {
next(err)
}
}

module.exports = {
get
get,
post
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ const ControllerTestBuilder = require('@test/test-helpers/simplified-account/con
const { expect } = require('chai')
const User = require('@models/User.class')
const userFixtures = require('@test/fixtures/user.fixtures')
const paths = require('@root/paths')

const ACCOUNT_TYPE = 'live'
const SERVICE_ID = 'service-id-123abc'
const SERVICE_EXTERNAL_ID = 'service-id-123abc'

const adminUser = new User(userFixtures.validUserResponse({
external_id: 'user-id-for-admin-user',
service_roles: {
service: {
service: { external_id: SERVICE_ID },
service: { external_id: SERVICE_EXTERNAL_ID },
role: { name: 'admin' }
}
}
Expand All @@ -23,7 +24,7 @@ const viewOnlyUser = new User(userFixtures.validUserResponse(
service_roles: {
service:
{
service: { external_id: SERVICE_ID },
service: { external_id: SERVICE_EXTERNAL_ID },
role: { name: 'view-only' }
}
}
Expand All @@ -42,21 +43,30 @@ const allCardTypes = [{
label: 'Visa',
type: 'CREDIT',
requires3ds: false
},
{
id: 'id-003',
brand: 'amex',
label: 'American Express',
type: 'CREDIT',
requires3ds: false
}]
const acceptedCardTypes = [allCardTypes[0]]

const mockResponse = sinon.spy()
const mockGetAllCardTypes = sinon.stub().resolves({ card_types: allCardTypes })
const mockGetAcceptedCardTypesForServiceAndAccountType = sinon.stub().resolves({ card_types: acceptedCardTypes })
const mockPostAcceptedCardsForServiceAndAccountType = sinon.stub().resolves({})

const { res, nextRequest, call } = new ControllerTestBuilder('@controllers/simplified-account/settings/card-types/card-types.controller')
.withServiceExternalId(SERVICE_ID)
const { req, res, nextRequest, call } = new ControllerTestBuilder('@controllers/simplified-account/settings/card-types/card-types.controller')
.withServiceExternalId(SERVICE_EXTERNAL_ID)
.withAccountType(ACCOUNT_TYPE)
.withStubs({
'@utils/response': { response: mockResponse },
'@services/card-types.service': {
getAllCardTypes: mockGetAllCardTypes,
getAcceptedCardTypesForServiceAndAccountType: mockGetAcceptedCardTypesForServiceAndAccountType
getAcceptedCardTypesForServiceAndAccountType: mockGetAcceptedCardTypesForServiceAndAccountType,
postAcceptedCardsForServiceAndAccountType: mockPostAcceptedCardsForServiceAndAccountType
}
})
.build()
Expand All @@ -83,9 +93,11 @@ describe('Controller: settings/card-types', () => {
it('should pass context data to the response method', () => {
expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('debitCards').length(1)
expect(mockResponse.args[0][3].cardTypes.debitCards[0]).to.deep.include({ text: 'Visa debit', checked: true })
expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('creditCards').length(1)
expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('creditCards').length(2)
expect(mockResponse.args[0][3].cardTypes.creditCards[0]).to.deep.include({ text: 'Visa credit', checked: false })
expect(mockResponse.args[0][3].cardTypes.creditCards[1]).to.deep.include({ text: 'American Express', checked: false })
expect(mockResponse.args[0][3]).to.have.property('isAdminUser').to.equal(true)
expect(mockResponse.args[0][3]).to.have.property('currentAcceptedCardTypeIds').length(1)
})
})

Expand All @@ -108,11 +120,82 @@ describe('Controller: settings/card-types', () => {
})

it('should pass context data to the response method', () => {
expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('Enabled debit cards').to.have.length(1).to.include('Visa debit')
expect(mockResponse.args[0][3].cardTypes).to.have.property('Not enabled debit cards').to.have.length(0)
expect(mockResponse.args[0][3].cardTypes).to.have.property('Enabled credit cards').to.have.length(0)
expect(mockResponse.args[0][3].cardTypes).to.have.property('Not enabled credit cards').to.have.length(1).to.include('Visa credit')
expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('debit/enabled')
.to.have.property('cards').to.deep.equal(['Visa debit'])
expect(mockResponse.args[0][3].cardTypes).to.have.property('debit/disabled').to.have.property('cards').to.have.length(0)
expect(mockResponse.args[0][3].cardTypes).to.have.property('credit/enabled').to.have.property('cards').to.have.length(0)
expect(mockResponse.args[0][3].cardTypes['credit/disabled'].cards).to.deep.equal(['Visa credit', 'American Express'])
expect(mockResponse.args[0][3]).to.have.property('isAdminUser').to.equal(false)
expect(mockResponse.args[0][3]).to.have.property('currentAcceptedCardTypeIds').length(1)
})
})

describe('post to enable an additional card type', () => {
before(() => {
nextRequest({
user: adminUser,
body: { currentAcceptedCardTypeIds: 'id-001', debit: 'id-001', credit: ['id-002', 'id-003'] }
})
call('post')
})

it('should call adminusers to update accepted card types', () => {
expect(mockPostAcceptedCardsForServiceAndAccountType.called).to.be.true // eslint-disable-line
})

it('should redirect to same page with success notification', () => {
expect(req.flash).to.have.been.calledWith('messages', {
state: 'success',
icon: '&check;',
heading: 'Accepted card types have been updated'
})
expect(res.redirect.calledOnce).to.be.true // eslint-disable-line
expect(res.redirect.args[0][0]).to.include(paths.simplifiedAccount.settings.cardTypes.index)
})
})

describe('post an unchanged set of card types', () => {
before(() => {
nextRequest({
user: adminUser,
body: { currentAcceptedCardTypeIds: 'id-001', debit: 'id-001' }
})
call('post')
})

it('should not call adminusers', () => {
expect(mockPostAcceptedCardsForServiceAndAccountType.called).to.be.false // eslint-disable-line no-unused-expressions
})

it('should redirect to same page without a notification', () => {
expect(req.flash).to.have.been.not.called // eslint-disable-line no-unused-expressions
expect(res.redirect.calledOnce).to.be.true // eslint-disable-line no-unused-expressions
expect(res.redirect.args[0][0]).to.include(paths.simplifiedAccount.settings.cardTypes.index)
})
})

describe('post with no card types selected', () => {
before(() => {
nextRequest({
user: adminUser,
body: { currentAcceptedCardTypeIds: 'id-001' }
})
call('post')
})

it('should not call adminusers', () => {
expect(mockPostAcceptedCardsForServiceAndAccountType.called).to.be.false // eslint-disable-line no-unused-expressions
})

it('should should pass context data to the response method with an error', () => {
expect(mockResponse.args[0][3]).to.have.property('errors').to.deep.include({ summary: [{ text: 'You must choose at least one card', href: '#' }] })
expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('debitCards').length(1)
expect(mockResponse.args[0][3].cardTypes.debitCards[0]).to.deep.include({ text: 'Visa debit', checked: false })
expect(mockResponse.args[0][3]).to.have.property('cardTypes').to.have.property('creditCards').length(2)
expect(mockResponse.args[0][3].cardTypes.creditCards[0]).to.deep.include({ text: 'Visa credit', checked: false })
expect(mockResponse.args[0][3].cardTypes.creditCards[1]).to.deep.include({ text: 'American Express', checked: false })
expect(mockResponse.args[0][3]).to.have.property('isAdminUser').to.equal(true)
expect(mockResponse.args[0][3]).to.have.property('currentAcceptedCardTypeIds').length(1)
})
})
})
7 changes: 6 additions & 1 deletion app/services/card-types.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ async function getAcceptedCardTypesForServiceAndAccountType (serviceId, accountT
return connectorClient.getAcceptedCardsForServiceAndAccountType(serviceId, accountType)
}

async function postAcceptedCardsForServiceAndAccountType (serviceId, accountType, payload) {
return connectorClient.postAcceptedCardsForServiceAndAccountType(serviceId, accountType, payload)
}

module.exports = {
getAllCardTypes,
getAcceptedCardTypesForServiceAndAccountType
getAcceptedCardTypesForServiceAndAccountType,
postAcceptedCardsForServiceAndAccountType
}
28 changes: 22 additions & 6 deletions app/services/clients/connector.client.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,14 +295,14 @@ ConnectorClient.prototype = {
},

/**
* Retrieves the accepted card Types for the given external service external id and account type
* @param serviceId (required)
* @param accountType (required)
* Retrieves the accepted card Types for the given service external id and account type
* @param {String} serviceExternalId
* @param {String} accountType
* @returns {Promise<Object>}
*/
getAcceptedCardsForServiceAndAccountType: async function (serviceId, accountType) {
const url = `${this.connectorUrl}/v1/frontend/service/{serviceId}/account/{accountType}/card-types`
.replace('{serviceId}', encodeURIComponent(serviceId))
getAcceptedCardsForServiceAndAccountType: async function (serviceExternalId, accountType) {
const url = `${this.connectorUrl}/v1/frontend/service/{serviceExternalId}/account/{accountType}/card-types`
.replace('{serviceExternalId}', encodeURIComponent(serviceExternalId))
.replace('{accountType}', encodeURIComponent(accountType))
configureClient(client, url)
const response = await client.get(url, 'get accepted card types for account')
Expand All @@ -323,6 +323,22 @@ ConnectorClient.prototype = {
return response.data
},

/**
* Updates the accepted card Types for the given service and account type
* @param {String} serviceExternalId
* @param {String} accountType
* @param {{card_types: string|string[]}} payload
* @returns {Promise<Object>}
*/
postAcceptedCardsForServiceAndAccountType: async function (serviceExternalId, accountType, payload) {
const url = `${this.connectorUrl}/v1/frontend/service/{serviceExternalId}/account/{accountType}/card-types`
.replace('{serviceExternalId}', encodeURIComponent(serviceExternalId))
.replace('{accountType}', encodeURIComponent(accountType))
configureClient(client, url)
const response = await client.post(url, payload, 'post accepted card types for account')
return response.data
},

/**
* Retrieves all card types
* @returns {Promise<Object>}
Expand Down
1 change: 1 addition & 0 deletions app/simplified-account-routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ simplifiedAccount.post(paths.simplifiedAccount.settings.organisationDetails.edit

// card types
simplifiedAccount.get(paths.simplifiedAccount.settings.cardTypes.index, permission('transactions:read'), serviceSettingsController.cardTypes.get)
simplifiedAccount.post(paths.simplifiedAccount.settings.cardTypes.index, permission('payment-types:update'), serviceSettingsController.cardTypes.post)

// worldpay details
simplifiedAccount.get(paths.simplifiedAccount.settings.worldpayDetails.index, permission('gateway-credentials:read'), serviceSettingsController.worldpayDetails.get)
Expand Down
24 changes: 18 additions & 6 deletions app/utils/simplified-account/format/format-card-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,26 @@ const formatCardTypesForAdminTemplate = (allCards, acceptedCards, account) => {
const formatCardTypesForNonAdminTemplate = (allCards, acceptedCards) => {
const acceptedCardTypeIds = acceptedCards.map(card => card.id)
const formattedCardTypes = {
'Enabled debit cards': [],
'Not enabled debit cards': [],
'Enabled credit cards': [],
'Not enabled credit cards': []
'debit/enabled': {
cards: [],
heading: 'Enabled debit cards'
},
'debit/disabled': {
cards: [],
heading: 'Not enabled debit cards'
},
'credit/enabled': {
cards: [],
heading: 'Enabled credit cards'
},
'credit/disabled': {
cards: [],
heading: 'Not enabled credit cards'
}
}
allCards.forEach(card => {
const cardIsEnabled = acceptedCardTypeIds.includes(card.id) ? 'Enabled' : 'Not enabled'
formattedCardTypes[`${cardIsEnabled} ${card.type.toLowerCase()} cards`].push(formatLabel(card))
const cardIsEnabled = acceptedCardTypeIds.includes(card.id) ? 'enabled' : 'disabled'
formattedCardTypes[`${card.type.toLowerCase()}/${cardIsEnabled}`].cards.push(formatLabel(card))
})
return formattedCardTypes
}
Expand Down
8 changes: 4 additions & 4 deletions app/utils/simplified-account/format/format-card-types.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ describe('format-card-types for template', () => {
const acceptedCards = [...allCards].filter(card => card.id !== 'id-001' && card.id !== 'id-002')
const account = { requires3ds: true }
const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, false)
expect(cards['Enabled debit cards']).to.have.length(2).to.deep.equal(['Mastercard debit', 'Maestro'])
expect(cards['Not enabled debit cards']).to.have.length(1).to.deep.equal(['Visa debit'])
expect(cards['Enabled credit cards']).to.have.length(2).to.deep.equal(['American Express', 'JCB'])
expect(cards['Not enabled credit cards']).to.have.length(1).to.deep.equal(['Visa credit'])
expect(cards['debit/enabled']).to.deep.equal({ cards: ['Mastercard debit', 'Maestro'], heading: 'Enabled debit cards' })
expect(cards['debit/disabled']).to.deep.equal({ cards: ['Visa debit'], heading: 'Not enabled debit cards' })
expect(cards['credit/enabled']).to.deep.equal({ cards: ['American Express', 'JCB'], heading: 'Enabled credit cards' })
expect(cards['credit/disabled']).to.deep.equal({ cards: ['Visa credit'], heading: 'Not enabled credit cards' })
})
})
})
Loading

0 comments on commit 416c4cb

Please sign in to comment.