From 62a37a94048322c319daa710cf88c748c033bd21 Mon Sep 17 00:00:00 2001 From: James Peacock Date: Mon, 2 Dec 2024 13:52:57 +0000 Subject: [PATCH 1/7] PP-13312 Get controller for card types --- .../card-types/card-types.controller.js | 17 ++- .../card-types/card-types.controller.test.js | 131 ++++++++++++++++++ app/services/card-types.service.js | 17 +++ app/services/clients/connector.client.js | 15 ++ app/simplified-account-routes.js | 3 + .../format/format-card-types.js | 72 ++++++++++ .../format/format-card-types.test.js | 111 +++++++++++++++ .../card-types/cardTypesCheckboxes.njk | 44 ++++++ .../settings/card-types/cardTypesList.njk | 21 +++ .../settings/card-types/index.njk | 18 ++- 10 files changed, 446 insertions(+), 3 deletions(-) create mode 100644 app/controllers/simplified-account/settings/card-types/card-types.controller.test.js create mode 100644 app/services/card-types.service.js create mode 100644 app/utils/simplified-account/format/format-card-types.js create mode 100644 app/utils/simplified-account/format/format-card-types.test.js create mode 100644 app/views/simplified-account/settings/card-types/cardTypesCheckboxes.njk create mode 100644 app/views/simplified-account/settings/card-types/cardTypesList.njk 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 199f1a357..ea8a5dc9e 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,7 +1,20 @@ const { response } = require('@utils/response') +const { formatCardTypesForTemplate } = require('@utils/simplified-account/format/format-card-types') +const { getAllCardTypes, getAcceptedCardTypesForServiceAndAccountType } = require('@services/card-types.service') -function get (req, res) { - response(req, res, 'simplified-account/settings/card-types/index', { }) +async function get (req, res, next) { + const serviceId = req.service.externalId + const accountType = req.account.type + const isAdminUser = req.user.isAdminUserForService(serviceId) + try { + const { card_types: allCards } = await getAllCardTypes() + const { card_types: acceptedCards } = await getAcceptedCardTypesForServiceAndAccountType(serviceId, accountType) + const cardTypes = formatCardTypesForTemplate(allCards, acceptedCards, req.account, isAdminUser) + response(req, res, 'simplified-account/settings/card-types/index', + { cardTypes, isAdminUser }) + } catch (err) { + next(err) + } } module.exports = { 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 new file mode 100644 index 000000000..34ce0302e --- /dev/null +++ b/app/controllers/simplified-account/settings/card-types/card-types.controller.test.js @@ -0,0 +1,131 @@ +const sinon = require('sinon') +const { expect } = require('chai') +const User = require('@models/User.class') +const userFixtures = require('@test/fixtures/user.fixtures') +const proxyquire = require('proxyquire') + +const ACCOUNT_TYPE = 'live' +const SERVICE_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 }, + role: { name: 'admin' } + } + } +})) +const viewOnlyUser = new User(userFixtures.validUserResponse( + { + external_id: 'user-id-for-view-only-user', + service_roles: { + service: + { + service: { external_id: SERVICE_ID }, + role: { name: 'view-only' } + } + } + })) + +const allCardTypes = [{ + id: 'id-001', + brand: 'visa', + label: 'Visa', + type: 'DEBIT', + requires3ds: false +}, +{ + id: 'id-002', + brand: 'visa', + label: 'Visa', + type: 'CREDIT', + requires3ds: false +}] + +let req, res, responseStub, getAllCardTypesStub, getAcceptedCardTypesForServiceAndAccountTypeStub, cardTypesController + +const getController = (stubs = {}) => { + return proxyquire('./card-types.controller', { + '@utils/response': { response: stubs.response }, + '@services/card-types.service': { + getAllCardTypes: stubs.getAllCardTypes, + getAcceptedCardTypesForServiceAndAccountType: stubs.getAcceptedCardTypesForServiceAndAccountType + } + }) +} + +const setupTest = (method, user, additionalReqProps = {}, additionalStubs = {}) => { + responseStub = sinon.spy() + getAllCardTypesStub = sinon.stub().returns({ card_types: allCardTypes }) + getAcceptedCardTypesForServiceAndAccountTypeStub = sinon.stub().resolves({ card_types: [allCardTypes[0]] }) + + cardTypesController = getController({ + response: responseStub, + getAllCardTypes: getAllCardTypesStub, + getAcceptedCardTypesForServiceAndAccountType: getAcceptedCardTypesForServiceAndAccountTypeStub, + ...additionalStubs + }) + res = { + redirect: sinon.spy() + } + req = { + user: user, + service: { + externalId: SERVICE_ID + }, + account: { + type: ACCOUNT_TYPE + }, + ...additionalReqProps + } + cardTypesController[method](req, res) +} + +describe('Controller: settings/card-types', () => { + describe('get for admin user', () => { + before(() => setupTest('get', adminUser)) + + it('should call the response method', () => { + expect(responseStub.called).to.be.true // eslint-disable-line + }) + + it('should pass req, res and template path to the response method', () => { + expect(responseStub.args[0][0]).to.deep.equal(req) + expect(responseStub.args[0][1]).to.deep.equal(res) + expect(responseStub.args[0][2]).to.equal('simplified-account/settings/card-types/index') + }) + + it('should pass context data to the response method', () => { + expect(responseStub.args[0][3]).to.have.property('cardTypes').to.have.property('debitCards').length(1) + expect(responseStub.args[0][3].cardTypes.debitCards[0]).to.have.property('text').to.equal('Visa debit') + expect(responseStub.args[0][3].cardTypes.debitCards[0]).to.have.property('checked').to.equal(true) + expect(responseStub.args[0][3]).to.have.property('cardTypes').to.have.property('creditCards').length(1) + expect(responseStub.args[0][3].cardTypes.creditCards[0]).to.have.property('text').to.equal('Visa credit') + expect(responseStub.args[0][3].cardTypes.creditCards[0]).to.have.property('checked').to.equal(false) + expect(responseStub.args[0][3]).to.have.property('isAdminUser').to.equal(true) + }) + }) + + describe('get for non-admin user', () => { + before(() => setupTest('get', viewOnlyUser)) + + it('should call the response method', () => { + expect(responseStub.called).to.be.true // eslint-disable-line + }) + + it('should pass req, res and template path to the response method', () => { + expect(responseStub.args[0][0]).to.deep.equal(req) + expect(responseStub.args[0][1]).to.deep.equal(res) + expect(responseStub.args[0][2]).to.equal('simplified-account/settings/card-types/index') + }) + + it('should pass context data to the response method', () => { + expect(responseStub.args[0][3]).to.have.property('cardTypes').to.have.property('Enabled debit cards').to.have.length(1).to.include('Visa debit') + expect(responseStub.args[0][3].cardTypes).to.have.property('Not enabled debit cards').to.have.length(0) + expect(responseStub.args[0][3].cardTypes).to.have.property('Enabled credit cards').to.have.length(0) + expect(responseStub.args[0][3].cardTypes).to.have.property('Not enabled credit cards').to.have.length(1).to.include('Visa credit') + expect(responseStub.args[0][3]).to.have.property('isAdminUser').to.equal(false) + }) + }) +}) diff --git a/app/services/card-types.service.js b/app/services/card-types.service.js new file mode 100644 index 000000000..dc0de9d7a --- /dev/null +++ b/app/services/card-types.service.js @@ -0,0 +1,17 @@ +'use strict' + +const ConnectorClient = require('./clients/connector.client.js').ConnectorClient +const connectorClient = new ConnectorClient(process.env.CONNECTOR_URL) + +async function getAllCardTypes () { + return connectorClient.getAllCardTypes() +} + +async function getAcceptedCardTypesForServiceAndAccountType (serviceId, accountType) { + return connectorClient.getAcceptedCardsForServiceAndAccountType(serviceId, accountType) +} + +module.exports = { + getAllCardTypes, + getAcceptedCardTypesForServiceAndAccountType +} diff --git a/app/services/clients/connector.client.js b/app/services/clients/connector.client.js index a559dc64d..933d6469f 100644 --- a/app/services/clients/connector.client.js +++ b/app/services/clients/connector.client.js @@ -257,6 +257,21 @@ ConnectorClient.prototype = { return response.data }, + /** + * Retrieves the accepted card Types for the given account + * @param serviceId (required) + * @param accountType (required) + * @returns {Promise} + */ + getAcceptedCardsForServiceAndAccountType: async function (serviceId, accountType) { + const url = `${this.connectorUrl}/v1/frontend/service/{serviceId}/account/{accountType}/card-types` + .replace('{serviceId}', encodeURIComponent(serviceId)) + .replace('{accountType}', encodeURIComponent(accountType)) + configureClient(client, url) + const response = await client.get(url, 'get accepted card types for account') + return response.data + }, + /** * Updates the accepted card Types for to the given gateway account * @param gatewayAccountId (required) diff --git a/app/simplified-account-routes.js b/app/simplified-account-routes.js index 246980a47..8b477b27f 100644 --- a/app/simplified-account-routes.js +++ b/app/simplified-account-routes.js @@ -69,6 +69,9 @@ simplifiedAccount.get(paths.simplifiedAccount.settings.worldpayDetails.index, pe simplifiedAccount.get(paths.simplifiedAccount.settings.worldpayDetails.credentials, permission('gateway-credentials:update'), serviceSettingsController.worldpayDetails.worldpayCredentials.get) simplifiedAccount.post(paths.simplifiedAccount.settings.worldpayDetails.credentials, permission('gateway-credentials:update'), serviceSettingsController.worldpayDetails.worldpayCredentials.post) +// card types +simplifiedAccount.get(paths.simplifiedAccount.settings.cardTypes.index, permission('transactions:read'), serviceSettingsController.cardTypes.get) + // stripe details const stripeDetailsPath = paths.simplifiedAccount.settings.stripeDetails const stripeDetailsRouter = new Router({ mergeParams: true }) diff --git a/app/utils/simplified-account/format/format-card-types.js b/app/utils/simplified-account/format/format-card-types.js new file mode 100644 index 000000000..70e000f63 --- /dev/null +++ b/app/utils/simplified-account/format/format-card-types.js @@ -0,0 +1,72 @@ +const formatLabel = (card) => { + if (card.brand === 'visa' || card.brand === 'master-card') { + return `${card.label} ${card.type.toLowerCase()}` + } else { + return card.brand === 'jcb' ? card.label.toUpperCase() : card.label + } +} + +const formatCardTypesForAdminTemplate = (allCards, acceptedCards, account) => { + const cardDataChecklistItem = (card) => { + return { + value: card.id, + text: formatLabel(card), + checked: acceptedCards.filter(accepted => accepted.id === card.id).length !== 0, + requires3ds: card.requires3ds + } + } + const disableCheckboxIf3dsRequiredButNotEnabled = (cardTypeChecklistItem) => { + if (cardTypeChecklistItem.requires3ds && !account.requires3ds) { + cardTypeChecklistItem.disabled = true + cardTypeChecklistItem.hint = { + html: account.type === 'test' ? `${cardTypeChecklistItem.text} is not available on test accounts` : `${cardTypeChecklistItem.text} cannot be used because 3D Secure is not available. Please contact support` + } + } + return cardTypeChecklistItem + } + const addHintForAmexAndUnionpay = (cardTypeChecklistItem) => { + if (['American Express', 'Union Pay'].includes(cardTypeChecklistItem.text)) { + if (account.paymentProvider === 'worldpay') { + cardTypeChecklistItem.hint = { + html: 'You must have already enabled this with Worldpay' + } + } + } + return cardTypeChecklistItem + } + const debitCardChecklistItems = allCards.filter(card => card.type === 'DEBIT') + .map(card => cardDataChecklistItem(card)) + .map(cardTypeChecklistItem => disableCheckboxIf3dsRequiredButNotEnabled(cardTypeChecklistItem)) + + const creditCardChecklistItems = allCards.filter(card => card.type === 'CREDIT') + .map(card => cardDataChecklistItem(card)) + .map(cardTypeChecklistItem => addHintForAmexAndUnionpay(cardTypeChecklistItem)) + + return { debitCards: debitCardChecklistItems, creditCards: creditCardChecklistItems } +} + +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': [] + } + allCards.forEach(card => { + const cardIsEnabled = acceptedCardTypeIds.includes(card.id) ? 'Enabled' : 'Not enabled' + formattedCardTypes[`${cardIsEnabled} ${card.type.toLowerCase()} cards`].push(formatLabel(card)) + }) + return formattedCardTypes +} + +const formatCardTypesForTemplate = (allCards, acceptedCards, account, isAdminUser) => { + if (isAdminUser) { + return formatCardTypesForAdminTemplate(allCards, acceptedCards, account) + } + return formatCardTypesForNonAdminTemplate(allCards, acceptedCards) +} + +module.exports = { + formatCardTypesForTemplate +} diff --git a/app/utils/simplified-account/format/format-card-types.test.js b/app/utils/simplified-account/format/format-card-types.test.js new file mode 100644 index 000000000..f33140526 --- /dev/null +++ b/app/utils/simplified-account/format/format-card-types.test.js @@ -0,0 +1,111 @@ +const { expect } = require('chai') +const { formatCardTypesForTemplate } = require('@utils/simplified-account/format/format-card-types') + +const allCards = [ + { + id: 'id-001', + brand: 'visa', + label: 'Visa', + type: 'DEBIT', + requires3ds: false + }, + { + id: 'id-002', + brand: 'visa', + label: 'Visa', + type: 'CREDIT', + requires3ds: false + }, + { + id: 'id-003', + brand: 'master-card', + label: 'Mastercard', + type: 'DEBIT', + requires3ds: false + }, + { + id: 'id-004', + brand: 'american-express', + label: 'American Express', + type: 'CREDIT', + requires3ds: false + }, + { + id: 'id-005', + brand: 'jcb', + label: 'Jcb', + type: 'CREDIT', + requires3ds: false + }, + { + id: 'id-006', + brand: 'maestro', + label: 'Maestro', + type: 'DEBIT', + requires3ds: true + } +] +describe('format-card-types for template', () => { + describe('present checkboxes for admin user', () => { + it('should return all card types with checked boxes if they are all accepted', () => { + const acceptedCards = [...allCards] + const account = { requires3ds: true } + const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, true) + expect(cards).to.have.property('debitCards').to.have.length(3) + expect(cards.debitCards[0].text).to.equal('Visa debit') + expect(cards.debitCards[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.debitCards[1].text).to.equal('Mastercard debit') + expect(cards.debitCards[1].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.debitCards[2].text).to.equal('Maestro') + expect(cards.debitCards[2].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards).to.have.property('creditCards').to.have.length(3) + expect(cards.creditCards[0].text).to.equal('Visa credit') + expect(cards.creditCards[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.creditCards[1].text).to.equal('American Express') + expect(cards.creditCards[1].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.creditCards[2].text).to.equal('JCB') + expect(cards.creditCards[2].checked).to.be.true // eslint-disable-line no-unused-expressions + }) + + it('should return unchecked boxes for not accepted card types', () => { + const acceptedCards = [...allCards].filter(card => card.id !== 'id-001' && card.id !== 'id-002') + const account = { requires3ds: true } + const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, true) + expect(cards).to.have.property('debitCards').to.have.length(3) + expect(cards.debitCards.filter(card => card.text === 'Visa debit')[0].checked).to.be.false // eslint-disable-line no-unused-expressions + expect(cards.debitCards.filter(card => card.text === 'Mastercard debit')[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.debitCards.filter(card => card.text === 'Maestro')[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards).to.have.property('creditCards').to.have.length(3) + expect(cards.debitCards.filter(card => card.text === 'Visa credit')[0].checked).to.be.false // eslint-disable-line no-unused-expressions + expect(cards.debitCards.filter(card => card.text === 'American Express')[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.debitCards.filter(card => card.text === 'JCB')[0].checked).to.be.true // eslint-disable-line no-unused-expressions + }) + + it('should set checkbox to disabled for requires3ds card types if 3ds not enabled on account', () => { + const acceptedCards = [...allCards] + const account = { requires3ds: false } + const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, true) + expect(cards.debitCards.filter(card => card.text === 'Maestro')[0]).to.have.property('disabled').to.be.true // eslint-disable-line no-unused-expressions + }) + + it('should add hint to American Express if payment provider is Worldpay and account is live', () => { + const acceptedCards = [...allCards] + const account = { requires3ds: true, paymentProvider: 'worldpay', type: 'live' } + const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, true) + expect(cards.creditCards.filter(card => card.text === 'American Express')[0]) + .to.have.property('hint').to.deep.equal({ html: 'You must have already enabled this with Worldpay' }) + }) + }) + + describe('present read-only list for non-admin user', () => { + it('should return all card types arranged by type and whether accepted or not', () => { + 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']) + }) + }) +}) diff --git a/app/views/simplified-account/settings/card-types/cardTypesCheckboxes.njk b/app/views/simplified-account/settings/card-types/cardTypesCheckboxes.njk new file mode 100644 index 000000000..8e2a14d7a --- /dev/null +++ b/app/views/simplified-account/settings/card-types/cardTypesCheckboxes.njk @@ -0,0 +1,44 @@ + +

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 new file mode 100644 index 000000000..c481b71df --- /dev/null +++ b/app/views/simplified-account/settings/card-types/cardTypesList.njk @@ -0,0 +1,21 @@ +{% 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 447667725..ab11152a3 100644 --- a/app/views/simplified-account/settings/card-types/index.njk +++ b/app/views/simplified-account/settings/card-types/index.njk @@ -5,4 +5,20 @@ {% endblock %} {% block settingsContent %} -{% endblock %} + {% if not isAdminUser %} + {{ govukInsetText({ + text: "You don’t have permission to manage settings. Contact your service admin if you would like to manage 3D Secure, + accepted card types, email notifications, billing address or mask card numbers or security codes for MOTO services.", + classes: "service-settings-inset-text--grey" + }) }} + {% endif %} + +

Card types

+ + {% if isAdminUser %} + {% include "./cardTypesCheckboxes.njk" %} + {% else %} + {% include "./cardTypesList.njk" %} + {% endif %} + +{% endblock %} From 988891ae0fa9135bef1a3b4c92ea900065e5f095 Mon Sep 17 00:00:00 2001 From: James Peacock Date: Thu, 5 Dec 2024 13:07:32 +0000 Subject: [PATCH 2/7] PP-13312 Fix test --- .../simplified-account/format/format-card-types.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 f33140526..e95df396c 100644 --- a/app/utils/simplified-account/format/format-card-types.test.js +++ b/app/utils/simplified-account/format/format-card-types.test.js @@ -76,9 +76,9 @@ describe('format-card-types for template', () => { expect(cards.debitCards.filter(card => card.text === 'Mastercard debit')[0].checked).to.be.true // eslint-disable-line no-unused-expressions expect(cards.debitCards.filter(card => card.text === 'Maestro')[0].checked).to.be.true // eslint-disable-line no-unused-expressions expect(cards).to.have.property('creditCards').to.have.length(3) - expect(cards.debitCards.filter(card => card.text === 'Visa credit')[0].checked).to.be.false // eslint-disable-line no-unused-expressions - expect(cards.debitCards.filter(card => card.text === 'American Express')[0].checked).to.be.true // eslint-disable-line no-unused-expressions - expect(cards.debitCards.filter(card => card.text === 'JCB')[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.creditCards.filter(card => card.text === 'Visa credit')[0].checked).to.be.false // eslint-disable-line no-unused-expressions + expect(cards.creditCards.filter(card => card.text === 'American Express')[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.creditCards.filter(card => card.text === 'JCB')[0].checked).to.be.true // eslint-disable-line no-unused-expressions }) it('should set checkbox to disabled for requires3ds card types if 3ds not enabled on account', () => { From 9c770fd5f4541f8e9a9a6cc86718f91f664b44a8 Mon Sep 17 00:00:00 2001 From: James Peacock Date: Fri, 6 Dec 2024 11:31:53 +0000 Subject: [PATCH 3/7] PP-13312 [squash] refactor format-card-types --- .../format/format-card-types.js | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/app/utils/simplified-account/format/format-card-types.js b/app/utils/simplified-account/format/format-card-types.js index 70e000f63..1a57970a9 100644 --- a/app/utils/simplified-account/format/format-card-types.js +++ b/app/utils/simplified-account/format/format-card-types.js @@ -1,46 +1,52 @@ const formatLabel = (card) => { if (card.brand === 'visa' || card.brand === 'master-card') { return `${card.label} ${card.type.toLowerCase()}` - } else { - return card.brand === 'jcb' ? card.label.toUpperCase() : card.label } + return card.brand === 'jcb' ? card.label.toUpperCase() : card.label } -const formatCardTypesForAdminTemplate = (allCards, acceptedCards, account) => { - const cardDataChecklistItem = (card) => { - return { - value: card.id, - text: formatLabel(card), - checked: acceptedCards.filter(accepted => accepted.id === card.id).length !== 0, - requires3ds: card.requires3ds - } +const createCardTypeChecklistItem = (card, acceptedCards) => { + return { + value: card.id, + text: formatLabel(card), + checked: acceptedCards.filter(accepted => accepted.id === card.id).length !== 0, + requires3ds: card.requires3ds } - const disableCheckboxIf3dsRequiredButNotEnabled = (cardTypeChecklistItem) => { - if (cardTypeChecklistItem.requires3ds && !account.requires3ds) { - cardTypeChecklistItem.disabled = true - cardTypeChecklistItem.hint = { - html: account.type === 'test' ? `${cardTypeChecklistItem.text} is not available on test accounts` : `${cardTypeChecklistItem.text} cannot be used because 3D Secure is not available. Please contact support` +} + +const disableCheckboxIf3dsRequiredButNotEnabled = (cardTypeChecklistItem, accountType, accountRequires3ds) => { + if (cardTypeChecklistItem.requires3ds && !accountRequires3ds) { + return { + ...cardTypeChecklistItem, + disabled: true, + hint: { + html: accountType === 'test' ? `${cardTypeChecklistItem.text} is not available on test accounts` : `${cardTypeChecklistItem.text} cannot be used because 3D Secure is not available. Please contact support` } } - return cardTypeChecklistItem } - const addHintForAmexAndUnionpay = (cardTypeChecklistItem) => { - if (['American Express', 'Union Pay'].includes(cardTypeChecklistItem.text)) { - if (account.paymentProvider === 'worldpay') { - cardTypeChecklistItem.hint = { - html: 'You must have already enabled this with Worldpay' - } + return cardTypeChecklistItem +} + +const addHintForAmexAndUnionpayIfWorldpay = (cardTypeChecklistItem, paymentProvider) => { + if (['American Express', 'Union Pay'].includes(cardTypeChecklistItem.text) && paymentProvider === 'worldpay') { + return { + ...cardTypeChecklistItem, + hint: { + html: 'You must have already enabled this with Worldpay' } } - return cardTypeChecklistItem } + return cardTypeChecklistItem +} + +const formatCardTypesForAdminTemplate = (allCards, acceptedCards, account) => { const debitCardChecklistItems = allCards.filter(card => card.type === 'DEBIT') - .map(card => cardDataChecklistItem(card)) - .map(cardTypeChecklistItem => disableCheckboxIf3dsRequiredButNotEnabled(cardTypeChecklistItem)) + .map(card => createCardTypeChecklistItem(card, acceptedCards)) + .map(cardTypeChecklistItem => disableCheckboxIf3dsRequiredButNotEnabled(cardTypeChecklistItem, account.type, account.requires3ds)) const creditCardChecklistItems = allCards.filter(card => card.type === 'CREDIT') - .map(card => cardDataChecklistItem(card)) - .map(cardTypeChecklistItem => addHintForAmexAndUnionpay(cardTypeChecklistItem)) + .map(card => createCardTypeChecklistItem(card, acceptedCards)) + .map(cardTypeChecklistItem => addHintForAmexAndUnionpayIfWorldpay(cardTypeChecklistItem, account.paymentProvider)) return { debitCards: debitCardChecklistItems, creditCards: creditCardChecklistItems } } From 5d4726bd622643440aeced39f299df578eb70c18 Mon Sep 17 00:00:00 2001 From: James Peacock Date: Fri, 6 Dec 2024 12:54:10 +0000 Subject: [PATCH 4/7] PP-13312 [squash] update tests --- .../format/format-card-types.test.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) 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 e95df396c..ae7348056 100644 --- a/app/utils/simplified-account/format/format-card-types.test.js +++ b/app/utils/simplified-account/format/format-card-types.test.js @@ -81,11 +81,22 @@ describe('format-card-types for template', () => { expect(cards.creditCards.filter(card => card.text === 'JCB')[0].checked).to.be.true // eslint-disable-line no-unused-expressions }) - it('should set checkbox to disabled for requires3ds card types if 3ds not enabled on account', () => { + it('should set checkbox to disabled for requires3ds card types if 3ds not enabled for test account', () => { const acceptedCards = [...allCards] - const account = { requires3ds: false } + const account = { requires3ds: false, type: 'test' } const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, true) expect(cards.debitCards.filter(card => card.text === 'Maestro')[0]).to.have.property('disabled').to.be.true // eslint-disable-line no-unused-expressions + expect(cards.debitCards.filter(card => card.text === 'Maestro')[0]).to.have.property('hint') + .to.deep.equal({ html: 'Maestro is not available on test accounts' }) + }) + + it('should set checkbox to disabled for requires3ds card types if 3ds not enabled for live account', () => { + const acceptedCards = [...allCards] + const account = { requires3ds: false, type: 'live' } + const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, true) + expect(cards.debitCards.filter(card => card.text === 'Maestro')[0]).to.have.property('disabled').to.be.true // eslint-disable-line no-unused-expressions + expect(cards.debitCards.filter(card => card.text === 'Maestro')[0]).to.have.property('hint') + .to.deep.equal({ html: 'Maestro cannot be used because 3D Secure is not available. Please contact support' }) }) it('should add hint to American Express if payment provider is Worldpay and account is live', () => { From 5435facbac051eb3ac71699192cc84dfa109222b Mon Sep 17 00:00:00 2001 From: James Peacock Date: Fri, 6 Dec 2024 14:32:10 +0000 Subject: [PATCH 5/7] PP-13312 response to comments --- app/services/clients/connector.client.js | 2 +- .../format/format-card-types.js | 4 ++- .../format/format-card-types.test.js | 30 ++++++++----------- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/app/services/clients/connector.client.js b/app/services/clients/connector.client.js index 933d6469f..350965216 100644 --- a/app/services/clients/connector.client.js +++ b/app/services/clients/connector.client.js @@ -258,7 +258,7 @@ ConnectorClient.prototype = { }, /** - * Retrieves the accepted card Types for the given account + * Retrieves the accepted card Types for the given external service external id and account type * @param serviceId (required) * @param accountType (required) * @returns {Promise} diff --git a/app/utils/simplified-account/format/format-card-types.js b/app/utils/simplified-account/format/format-card-types.js index 1a57970a9..fcd4fc424 100644 --- a/app/utils/simplified-account/format/format-card-types.js +++ b/app/utils/simplified-account/format/format-card-types.js @@ -1,3 +1,5 @@ +const cardsThatNeedToBeEnabledOnWorldpay = ['American Express', 'Union Pay'] + const formatLabel = (card) => { if (card.brand === 'visa' || card.brand === 'master-card') { return `${card.label} ${card.type.toLowerCase()}` @@ -28,7 +30,7 @@ const disableCheckboxIf3dsRequiredButNotEnabled = (cardTypeChecklistItem, accoun } const addHintForAmexAndUnionpayIfWorldpay = (cardTypeChecklistItem, paymentProvider) => { - if (['American Express', 'Union Pay'].includes(cardTypeChecklistItem.text) && paymentProvider === 'worldpay') { + if (cardsThatNeedToBeEnabledOnWorldpay.includes(cardTypeChecklistItem.text) && paymentProvider === 'worldpay') { return { ...cardTypeChecklistItem, hint: { 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 ae7348056..46aec29d0 100644 --- a/app/utils/simplified-account/format/format-card-types.test.js +++ b/app/utils/simplified-account/format/format-card-types.test.js @@ -52,19 +52,13 @@ describe('format-card-types for template', () => { const account = { requires3ds: true } const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, true) expect(cards).to.have.property('debitCards').to.have.length(3) - expect(cards.debitCards[0].text).to.equal('Visa debit') - expect(cards.debitCards[0].checked).to.be.true // eslint-disable-line no-unused-expressions - expect(cards.debitCards[1].text).to.equal('Mastercard debit') - expect(cards.debitCards[1].checked).to.be.true // eslint-disable-line no-unused-expressions - expect(cards.debitCards[2].text).to.equal('Maestro') - expect(cards.debitCards[2].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.debitCards[0]).to.deep.include({ text: 'Visa debit', checked: true }) + expect(cards.debitCards[1]).to.deep.include({ text: 'Mastercard debit', checked: true }) + expect(cards.debitCards[2]).to.deep.include({ text: 'Maestro', checked: true }) expect(cards).to.have.property('creditCards').to.have.length(3) - expect(cards.creditCards[0].text).to.equal('Visa credit') - expect(cards.creditCards[0].checked).to.be.true // eslint-disable-line no-unused-expressions - expect(cards.creditCards[1].text).to.equal('American Express') - expect(cards.creditCards[1].checked).to.be.true // eslint-disable-line no-unused-expressions - expect(cards.creditCards[2].text).to.equal('JCB') - expect(cards.creditCards[2].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.creditCards[0]).to.deep.include({ text: 'Visa credit', checked: true }) + expect(cards.creditCards[1]).to.deep.include({ text: 'American Express', checked: true }) + expect(cards.creditCards[2]).to.deep.include({ text: 'JCB', checked: true }) }) it('should return unchecked boxes for not accepted card types', () => { @@ -72,13 +66,13 @@ describe('format-card-types for template', () => { const account = { requires3ds: true } const cards = formatCardTypesForTemplate(allCards, acceptedCards, account, true) expect(cards).to.have.property('debitCards').to.have.length(3) - expect(cards.debitCards.filter(card => card.text === 'Visa debit')[0].checked).to.be.false // eslint-disable-line no-unused-expressions - expect(cards.debitCards.filter(card => card.text === 'Mastercard debit')[0].checked).to.be.true // eslint-disable-line no-unused-expressions - expect(cards.debitCards.filter(card => card.text === 'Maestro')[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.debitCards[0]).to.deep.include({ text: 'Visa debit', checked: false }) + expect(cards.debitCards[1]).to.deep.include({ text: 'Mastercard debit', checked: true }) + expect(cards.debitCards[2]).to.deep.include({ text: 'Maestro', checked: true }) expect(cards).to.have.property('creditCards').to.have.length(3) - expect(cards.creditCards.filter(card => card.text === 'Visa credit')[0].checked).to.be.false // eslint-disable-line no-unused-expressions - expect(cards.creditCards.filter(card => card.text === 'American Express')[0].checked).to.be.true // eslint-disable-line no-unused-expressions - expect(cards.creditCards.filter(card => card.text === 'JCB')[0].checked).to.be.true // eslint-disable-line no-unused-expressions + expect(cards.creditCards[0]).to.deep.include({ text: 'Visa credit', checked: false }) + expect(cards.creditCards[1]).to.deep.include({ text: 'American Express', checked: true }) + expect(cards.creditCards[2]).to.deep.include({ text: 'JCB', checked: true }) }) it('should set checkbox to disabled for requires3ds card types if 3ds not enabled for test account', () => { From 0f308191c67764dd340710ba9346849f829e0e35 Mon Sep 17 00:00:00 2001 From: James Peacock Date: Fri, 6 Dec 2024 15:08:17 +0000 Subject: [PATCH 6/7] PP-13312 [squash] improve tests as recommended --- .../card-types/card-types.controller.test.js | 106 +++++------------- .../ControllerTestBuilder.class.js | 5 + 2 files changed, 31 insertions(+), 80 deletions(-) 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 34ce0302e..dbf3a3550 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 @@ -1,8 +1,8 @@ const sinon = require('sinon') +const ControllerTestBuilder = require('@test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class') const { expect } = require('chai') const User = require('@models/User.class') const userFixtures = require('@test/fixtures/user.fixtures') -const proxyquire = require('proxyquire') const ACCOUNT_TYPE = 'live' const SERVICE_ID = 'service-id-123abc' @@ -16,17 +16,6 @@ const adminUser = new User(userFixtures.validUserResponse({ } } })) -const viewOnlyUser = new User(userFixtures.validUserResponse( - { - external_id: 'user-id-for-view-only-user', - service_roles: { - service: - { - service: { external_id: SERVICE_ID }, - role: { name: 'view-only' } - } - } - })) const allCardTypes = [{ id: 'id-001', @@ -42,90 +31,47 @@ const allCardTypes = [{ type: 'CREDIT', requires3ds: false }] +const acceptedCardTypes = [allCardTypes[0]] -let req, res, responseStub, getAllCardTypesStub, getAcceptedCardTypesForServiceAndAccountTypeStub, cardTypesController +const mockResponse = sinon.spy() +const mockGetAllCardTypes = sinon.stub().resolves({ card_types: allCardTypes }) +const mockGetAcceptedCardTypesForServiceAndAccountType = sinon.stub().resolves({ card_types: acceptedCardTypes }) -const getController = (stubs = {}) => { - return proxyquire('./card-types.controller', { - '@utils/response': { response: stubs.response }, +const { req, res, call } = new ControllerTestBuilder('@controllers/simplified-account/settings/card-types/card-types.controller') + .withServiceExternalId(SERVICE_ID) + .withAccountType(ACCOUNT_TYPE) + .withUser(adminUser) + .withStubs({ + '@utils/response': { response: mockResponse }, '@services/card-types.service': { - getAllCardTypes: stubs.getAllCardTypes, - getAcceptedCardTypesForServiceAndAccountType: stubs.getAcceptedCardTypesForServiceAndAccountType + getAllCardTypes: mockGetAllCardTypes, + getAcceptedCardTypesForServiceAndAccountType: mockGetAcceptedCardTypesForServiceAndAccountType } }) -} - -const setupTest = (method, user, additionalReqProps = {}, additionalStubs = {}) => { - responseStub = sinon.spy() - getAllCardTypesStub = sinon.stub().returns({ card_types: allCardTypes }) - getAcceptedCardTypesForServiceAndAccountTypeStub = sinon.stub().resolves({ card_types: [allCardTypes[0]] }) - - cardTypesController = getController({ - response: responseStub, - getAllCardTypes: getAllCardTypesStub, - getAcceptedCardTypesForServiceAndAccountType: getAcceptedCardTypesForServiceAndAccountTypeStub, - ...additionalStubs - }) - res = { - redirect: sinon.spy() - } - req = { - user: user, - service: { - externalId: SERVICE_ID - }, - account: { - type: ACCOUNT_TYPE - }, - ...additionalReqProps - } - cardTypesController[method](req, res) -} + .build() describe('Controller: settings/card-types', () => { - describe('get for admin user', () => { - before(() => setupTest('get', adminUser)) - - it('should call the response method', () => { - expect(responseStub.called).to.be.true // eslint-disable-line + describe('get', () => { + before(() => { + call('get') }) - it('should pass req, res and template path to the response method', () => { - expect(responseStub.args[0][0]).to.deep.equal(req) - expect(responseStub.args[0][1]).to.deep.equal(res) - expect(responseStub.args[0][2]).to.equal('simplified-account/settings/card-types/index') - }) - - it('should pass context data to the response method', () => { - expect(responseStub.args[0][3]).to.have.property('cardTypes').to.have.property('debitCards').length(1) - expect(responseStub.args[0][3].cardTypes.debitCards[0]).to.have.property('text').to.equal('Visa debit') - expect(responseStub.args[0][3].cardTypes.debitCards[0]).to.have.property('checked').to.equal(true) - expect(responseStub.args[0][3]).to.have.property('cardTypes').to.have.property('creditCards').length(1) - expect(responseStub.args[0][3].cardTypes.creditCards[0]).to.have.property('text').to.equal('Visa credit') - expect(responseStub.args[0][3].cardTypes.creditCards[0]).to.have.property('checked').to.equal(false) - expect(responseStub.args[0][3]).to.have.property('isAdminUser').to.equal(true) - }) - }) - - describe('get for non-admin user', () => { - before(() => setupTest('get', viewOnlyUser)) - it('should call the response method', () => { - expect(responseStub.called).to.be.true // eslint-disable-line + expect(mockResponse.called).to.be.true // eslint-disable-line }) it('should pass req, res and template path to the response method', () => { - expect(responseStub.args[0][0]).to.deep.equal(req) - expect(responseStub.args[0][1]).to.deep.equal(res) - expect(responseStub.args[0][2]).to.equal('simplified-account/settings/card-types/index') + expect(mockResponse.args[0][0]).to.deep.equal(req) + expect(mockResponse.args[0][1]).to.deep.equal(res) + expect(mockResponse.args[0][2]).to.equal('simplified-account/settings/card-types/index') }) it('should pass context data to the response method', () => { - expect(responseStub.args[0][3]).to.have.property('cardTypes').to.have.property('Enabled debit cards').to.have.length(1).to.include('Visa debit') - expect(responseStub.args[0][3].cardTypes).to.have.property('Not enabled debit cards').to.have.length(0) - expect(responseStub.args[0][3].cardTypes).to.have.property('Enabled credit cards').to.have.length(0) - expect(responseStub.args[0][3].cardTypes).to.have.property('Not enabled credit cards').to.have.length(1).to.include('Visa credit') - expect(responseStub.args[0][3]).to.have.property('isAdminUser').to.equal(false) + 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].cardTypes.creditCards[0]).to.deep.include({ text: 'Visa credit', checked: false }) + expect(mockResponse.args[0][3]).to.have.property('isAdminUser').to.equal(true) }) }) }) diff --git a/test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class.js b/test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class.js index b6750aff3..d709e2e89 100644 --- a/test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class.js +++ b/test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class.js @@ -24,6 +24,11 @@ module.exports = class ControllerTestBuilder { return this } + withUser (user) { + this.req.user = user + return this + } + withAccount (account) { this.req.account = account return this From 576877a9c01d2840d70045454970bf19c1008399 Mon Sep 17 00:00:00 2001 From: James Peacock Date: Fri, 6 Dec 2024 15:34:39 +0000 Subject: [PATCH 7/7] PP-13312 [squash] add test for nonadmin --- .../card-types/card-types.controller.test.js | 49 +++++++++++++++++-- .../ControllerTestBuilder.class.js | 5 -- 2 files changed, 45 insertions(+), 9 deletions(-) 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 dbf3a3550..f68a4bde7 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 @@ -17,6 +17,18 @@ const adminUser = new User(userFixtures.validUserResponse({ } })) +const viewOnlyUser = new User(userFixtures.validUserResponse( + { + external_id: 'user-id-for-view-only-user', + service_roles: { + service: + { + service: { external_id: SERVICE_ID }, + role: { name: 'view-only' } + } + } + })) + const allCardTypes = [{ id: 'id-001', brand: 'visa', @@ -37,10 +49,9 @@ const mockResponse = sinon.spy() const mockGetAllCardTypes = sinon.stub().resolves({ card_types: allCardTypes }) const mockGetAcceptedCardTypesForServiceAndAccountType = sinon.stub().resolves({ card_types: acceptedCardTypes }) -const { req, res, call } = new ControllerTestBuilder('@controllers/simplified-account/settings/card-types/card-types.controller') +const { res, nextRequest, call } = new ControllerTestBuilder('@controllers/simplified-account/settings/card-types/card-types.controller') .withServiceExternalId(SERVICE_ID) .withAccountType(ACCOUNT_TYPE) - .withUser(adminUser) .withStubs({ '@utils/response': { response: mockResponse }, '@services/card-types.service': { @@ -51,8 +62,11 @@ const { req, res, call } = new ControllerTestBuilder('@controllers/simplified-ac .build() describe('Controller: settings/card-types', () => { - describe('get', () => { + describe('get for admin user', () => { before(() => { + nextRequest({ + user: adminUser + }) call('get') }) @@ -61,7 +75,7 @@ describe('Controller: settings/card-types', () => { }) it('should pass req, res and template path to the response method', () => { - expect(mockResponse.args[0][0]).to.deep.equal(req) + expect(mockResponse.args[0][0].user).to.deep.equal(adminUser) expect(mockResponse.args[0][1]).to.deep.equal(res) expect(mockResponse.args[0][2]).to.equal('simplified-account/settings/card-types/index') }) @@ -74,4 +88,31 @@ describe('Controller: settings/card-types', () => { expect(mockResponse.args[0][3]).to.have.property('isAdminUser').to.equal(true) }) }) + + describe('get for non-admin user', () => { + before(() => { + nextRequest({ + user: viewOnlyUser + }) + call('get') + }) + + it('should call the response method', () => { + expect(mockResponse.called).to.be.true // eslint-disable-line + }) + + it('should pass req, res and template path to the response method', () => { + expect(mockResponse.args[0][0].user).to.deep.equal(viewOnlyUser) + expect(mockResponse.args[0][1]).to.deep.equal(res) + expect(mockResponse.args[0][2]).to.equal('simplified-account/settings/card-types/index') + }) + + 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('isAdminUser').to.equal(false) + }) + }) }) diff --git a/test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class.js b/test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class.js index d709e2e89..b6750aff3 100644 --- a/test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class.js +++ b/test/test-helpers/simplified-account/controllers/ControllerTestBuilder.class.js @@ -24,11 +24,6 @@ module.exports = class ControllerTestBuilder { return this } - withUser (user) { - this.req.user = user - return this - } - withAccount (account) { this.req.account = account return this