diff --git a/app/assets/sass/components/service-settings.scss b/app/assets/sass/components/service-settings.scss index 63333dac0..22a54f75c 100644 --- a/app/assets/sass/components/service-settings.scss +++ b/app/assets/sass/components/service-settings.scss @@ -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"); + } +} diff --git a/app/controllers/simplified-account/settings/card-types/card-types.controller.js b/app/controllers/simplified-account/settings/card-types/card-types.controller.js index ea8a5dc9e..c8974759c 100644 --- a/app/controllers/simplified-account/settings/card-types/card-types.controller.js +++ b/app/controllers/simplified-account/settings/card-types/card-types.controller.js @@ -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: '✓', 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 } diff --git a/app/controllers/simplified-account/settings/card-types/card-types.controller.test.js b/app/controllers/simplified-account/settings/card-types/card-types.controller.test.js index f68a4bde7..5a25b0c8a 100644 --- a/app/controllers/simplified-account/settings/card-types/card-types.controller.test.js +++ b/app/controllers/simplified-account/settings/card-types/card-types.controller.test.js @@ -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' } } } @@ -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' } } } @@ -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() @@ -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) }) }) @@ -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: '✓', + 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) }) }) }) diff --git a/app/services/card-types.service.js b/app/services/card-types.service.js index dc0de9d7a..ae9adce9c 100644 --- a/app/services/card-types.service.js +++ b/app/services/card-types.service.js @@ -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 } diff --git a/app/services/clients/connector.client.js b/app/services/clients/connector.client.js index 1fdae8440..28ceaf7f8 100644 --- a/app/services/clients/connector.client.js +++ b/app/services/clients/connector.client.js @@ -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} */ - 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') @@ -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} + */ + 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} diff --git a/app/simplified-account-routes.js b/app/simplified-account-routes.js index a6f92da68..967b40131 100644 --- a/app/simplified-account-routes.js +++ b/app/simplified-account-routes.js @@ -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) diff --git a/app/utils/simplified-account/format/format-card-types.js b/app/utils/simplified-account/format/format-card-types.js index fcd4fc424..cb21ea7cd 100644 --- a/app/utils/simplified-account/format/format-card-types.js +++ b/app/utils/simplified-account/format/format-card-types.js @@ -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 } diff --git a/app/utils/simplified-account/format/format-card-types.test.js b/app/utils/simplified-account/format/format-card-types.test.js index 46aec29d0..2e4985805 100644 --- a/app/utils/simplified-account/format/format-card-types.test.js +++ b/app/utils/simplified-account/format/format-card-types.test.js @@ -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' }) }) }) }) diff --git a/app/views/simplified-account/settings/card-types/cardTypesCheckboxes.njk b/app/views/simplified-account/settings/card-types/cardTypesCheckboxes.njk deleted file mode 100644 index 8e2a14d7a..000000000 --- a/app/views/simplified-account/settings/card-types/cardTypesCheckboxes.njk +++ /dev/null @@ -1,44 +0,0 @@ - -

Choose which credit and debit cards you want to accept.

- -
- - {{ - govukCheckboxes({ - idPrefix: "debit", - name: "debit", - classes: "with-border govuk-!-padding-top-3 pay-!-border-top", - fieldset: { - legend: { - text: "Debit cards", - classes: "govuk-fieldset__legend--s" - } - }, - items: cardTypes.debitCards - }) - }} - - {{ - govukCheckboxes({ - idPrefix: "credit", - name: "credit", - classes: "with-border govuk-!-padding-top-3 pay-!-border-top", - fieldset: { - legend: { - text: "Credit cards", - classes: "govuk-fieldset__legend--s" - } - }, - items: cardTypes.creditCards - }) - }} - - {{ - govukButton({ - text: 'Save changes', - attributes: { - id: "save-card-types" - } - }) - }} -
diff --git a/app/views/simplified-account/settings/card-types/cardTypesList.njk b/app/views/simplified-account/settings/card-types/cardTypesList.njk deleted file mode 100644 index c481b71df..000000000 --- a/app/views/simplified-account/settings/card-types/cardTypesList.njk +++ /dev/null @@ -1,21 +0,0 @@ -{% for category, items in cardTypes %} - {% if (items.length > 0) %} -

- {{ category }} -

- {% set cardList = [] %} - {% for card in items %} - {% set cardListItem = { - key: { - text: card, - classes: "govuk-!-display-none" - }, - value: { - html: card - } - } %} - {% set cardList = (cardList.push(cardListItem), cardList) %} - {% endfor %} - {{ govukSummaryList({ rows: cardList }) }} - {% endif %} -{% endfor %} diff --git a/app/views/simplified-account/settings/card-types/index.njk b/app/views/simplified-account/settings/card-types/index.njk index ab11152a3..a5bd7285b 100644 --- a/app/views/simplified-account/settings/card-types/index.njk +++ b/app/views/simplified-account/settings/card-types/index.njk @@ -13,12 +13,75 @@ }) }} {% endif %} -

Card types

+

Card types

{% if isAdminUser %} - {% include "./cardTypesCheckboxes.njk" %} +

Choose which credit and debit cards you want to accept.

+
+ + + {{ + govukCheckboxes({ + idPrefix: "debit", + name: "debit", + fieldset: { + legend: { + text: "Debit cards", + classes: "govuk-fieldset__legend--s" + } + }, + classes: "card-types govuk-!-padding-top-2", + items: cardTypes.debitCards + }) + }} + + {{ + govukCheckboxes({ + idPrefix: "credit", + name: "credit", + fieldset: { + legend: { + text: "Credit cards", + classes: "govuk-fieldset__legend--s" + } + }, + classes: "card-types govuk-!-padding-top-2", + items: cardTypes.creditCards + }) + }} + + {{ + govukButton({ + text: 'Save changes', + attributes: { + id: "save-card-types" + } + }) + }} +
{% else %} - {% include "./cardTypesList.njk" %} + {% for category, item in cardTypes %} + {% if (item.cards.length > 0) %} +

+ {{ item.heading }} +

+ {% set cardList = [] %} + {% for cardBrand in item.cards %} + {% set cardListItem = { + key: { + classes: "govuk-!-display-none" + }, + value: { + text: cardBrand + } + } %} + {% set cardList = (cardList.push(cardListItem), cardList) %} + {% endfor %} + {{ govukSummaryList({ + rows: cardList, + classes: "card-types govuk-!-margin-bottom-4" + }) }} + {% endif %} + {% endfor %} {% endif %} - {% endblock %}