From 2b8d2c7b2d79a1a4a104adccdd38598c76721173 Mon Sep 17 00:00:00 2001 From: Nathaniel Steers Date: Fri, 28 Feb 2025 17:59:43 +0000 Subject: [PATCH] PP-13525 show only one test account for degatewayed users - update `my-services` controller to enable conditional filtering of test gateway accounts when user is opted into degateway - simplify `my services` styles and views - remove reference to `service-switcher` now that accounts are presented as links, not forms - update routes and paths - remove unnecessary call to connector when determining if the service is a Worldpay test service in the `dashboard activity` controller - remove old `ui` tests, prefer cypress and mocha - fix issue where account and service resolution middleware would throw an error when adminusers and connector disagree about which gateway account should be returned for a service - the middleware now logs a warning when there is a mismatch between connector and adminusers, we prefer connector as the source of truth in this situation --- dev-server.mjs | 13 +- src/assets/sass/components/my-services.scss | 65 +++-- .../create-service.controller.js | 6 +- .../create-service/get.controller.test.js | 2 +- .../create-service/post.controller.test.js | 2 +- .../select-organisation-type.controller.js | 12 +- .../select-organisation-type.test.js | 12 +- .../dashboard-activity.controller.js | 14 +- .../dashboard-activity.controller.test.js | 1 + .../edit-service-name.controller.js | 4 +- .../invite-validation.controller.js | 2 +- .../invite-validation.controller.test.js | 2 +- .../my-services/get-index.controller.js | 85 ------ src/controllers/my-services/index.js | 4 - .../my-services/post-index.controller.js | 22 -- src/controllers/root/index.controller.js | 2 +- .../service-switch.controller.it.test.js | 267 ------------------ .../simplified-account/services/index.js | 1 + .../services/my-services.controller.js | 140 +++++++++ ...onsible-person-check-answers.controller.js | 2 +- .../subscribe-service.controller.js | 4 +- .../subscribe-service.controller.test.js | 2 +- .../default-view-decider.middleware.js | 7 +- .../simplified-account-strategy.middleware.js | 18 +- ...lified-account-strategy.middleware.test.js | 54 +++- src/models/GatewayAccount.class.js | 17 -- src/models/Service.class.js | 1 + src/models/User.class.js | 5 +- src/paths.js | 15 +- src/routes.js | 19 +- src/services/auth.service.js | 6 +- src/services/gateway-accounts.service.js | 14 + src/services/service.service.js | 15 +- src/services/service.service.test.js | 44 --- src/simplified-account-routes.js | 4 +- .../my-services/service-presentation-utils.js | 54 ++++ src/views/includes/phase-banner.njk | 39 ++- .../includes/propositional-navigation.njk | 2 +- src/views/payouts/list.njk | 2 +- src/views/registration/success.njk | 2 +- src/views/request-psp-test-account/index.njk | 6 +- src/views/request-to-go-live/index.njk | 4 +- src/views/services/_service-section.njk | 56 ---- src/views/services/_service-switch.njk | 21 -- src/views/services/add-service.njk | 2 +- .../services/my-services/_service-section.njk | 73 +++++ .../services/my-services}/index.njk | 17 +- src/views/transaction-detail/index.njk | 2 +- src/views/transactions/index.njk | 2 +- .../complete-invite-for-existing-user.cy.js | 2 +- .../integration/my-services/my-services.cy.js | 24 +- .../my-services/organisation-details.cy.js | 2 +- .../cypress/utils/request-to-go-live-utils.js | 3 +- test/ui/service-switcher.ui.test.js | 113 -------- 54 files changed, 498 insertions(+), 811 deletions(-) delete mode 100644 src/controllers/my-services/get-index.controller.js delete mode 100644 src/controllers/my-services/index.js delete mode 100644 src/controllers/my-services/post-index.controller.js delete mode 100644 src/controllers/service-switch.controller.it.test.js create mode 100644 src/controllers/simplified-account/services/index.js create mode 100644 src/controllers/simplified-account/services/my-services.controller.js create mode 100644 src/utils/simplified-account/services/my-services/service-presentation-utils.js delete mode 100644 src/views/services/_service-section.njk delete mode 100644 src/views/services/_service-switch.njk create mode 100644 src/views/simplified-account/services/my-services/_service-section.njk rename src/views/{services => simplified-account/services/my-services}/index.njk (78%) delete mode 100644 test/ui/service-switcher.ui.test.js diff --git a/dev-server.mjs b/dev-server.mjs index c7bbf1bcb3..7631861b2d 100644 --- a/dev-server.mjs +++ b/dev-server.mjs @@ -3,17 +3,18 @@ import { spawn } from 'child_process' import { clientBuild, serverBuild } from './esbuild.config.mjs' import { rm, access, constants } from 'node:fs' -const args = ['dist/application.js'] +const args = ['--enable-source-maps', '--inspect', 'dist/application.js'] let server const startServer = async () => { if (server) server.kill() + console.log(`💻\x1b[32m node\x1b[0m\x1b[33m ${args.join(' ')}\x1b[0m`) server = spawn('node', args, { stdio: 'inherit' }) } -async function startDevServer() { +async function startDevServer () { const clientCtx = await context(clientBuild) const serverCtx = await context({ @@ -22,7 +23,7 @@ async function startDevServer() { ...serverBuild.plugins, { name: 'server-rebuild', - setup(build) { + setup (build) { build.onEnd(async result => { if (result.errors.length === 0) { await startServer() @@ -35,7 +36,7 @@ async function startDevServer() { await Promise.all([ clientCtx.watch(), - serverCtx.watch(), + serverCtx.watch() ]) const cleanup = async () => { @@ -53,14 +54,14 @@ async function startDevServer() { process.on('SIGTERM', cleanup) } -await rm('dist', { recursive: true, force: true }, async () => { +rm('dist', { recursive: true, force: true }, async () => { console.log('✅ [dist] cleared') if (process.env.NODE_ENV === 'test') { console.log('🧪 [cypress/test.env] loaded environment') args.unshift('-r', 'dotenv/config') args.push('dotenv_config_path=test/cypress/test.env') } else { - await access('/.dockerenv', constants.R_OK, (err) => { + access('/.dockerenv', constants.R_OK, (err) => { if (err) { console.log('🔩 [.env] loaded environment') args.unshift('-r', 'dotenv/config') diff --git a/src/assets/sass/components/my-services.scss b/src/assets/sass/components/my-services.scss index a54f7e6425..6f9a96606d 100644 --- a/src/assets/sass/components/my-services.scss +++ b/src/assets/sass/components/my-services.scss @@ -1,28 +1,3 @@ -.service-switcher{ - &.live { - position: relative; - margin-bottom: govuk-spacing(3); - padding-left: 2rem; - @include govuk-font($size: 19, $weight: bold); - - &:before { - content: ""; - position: absolute; - top: 50%; - left: 0; - -webkit-transform:translate(0, -50%); - -moz-transform:translate(0, -50%); - -ms-transform:translate(0, -50%); - -o-transform:translate(0, -50%); - transform:translate(0, -50%); - width: 1.5rem; - height: 1.5rem; - border-radius: 100%; - background-color: govuk-colour("green"); - } - } -} - .reports { margin-top: govuk-spacing(2); margin-bottom: govuk-spacing(4); @@ -32,7 +7,8 @@ margin-bottom: govuk-spacing(6); } -.service_list_item { +.service-section { + margin-top: govuk-spacing(1); hr { border: none; border-top: 1px solid $govuk-border-colour; @@ -41,6 +17,39 @@ } } -.service_list_item { - margin-top: govuk-spacing(1); +.govuk-tag--worldpay-test-service { + max-width: 200px; +} + +.service-section__links { + margin-bottom: govuk-spacing(4); + li:first-child { + margin-top: 0; + margin-bottom: govuk-spacing(4); + } + li { + margin-bottom: govuk-spacing(1); + } +} + +.service-section__link--test { + @include govuk-font($size: 16); +} + +.service-section__link--live { + position: relative; + padding-left: 2rem; + @include govuk-font($size: 19, $weight: bold); + + &:before { + content: ""; + position: absolute; + top: 50%; + left: 0; + transform: translateY(-50%); + width: 1.5rem; + height: 1.5rem; + border-radius: 50%; + background-color: govuk-colour("green"); + } } diff --git a/src/controllers/create-service/create-service.controller.js b/src/controllers/create-service/create-service.controller.js index 06fa24af5f..57d9fb2745 100644 --- a/src/controllers/create-service/create-service.controller.js +++ b/src/controllers/create-service/create-service.controller.js @@ -12,8 +12,8 @@ function get (req, res) { const createServiceState = _.get(req, 'session.pageData.createService', {}) const context = { ...createServiceState, - back_link: paths.serviceSwitcher.index, - submit_link: paths.serviceSwitcher.create.selectOrgType + back_link: paths.services.index, + submit_link: paths.services.create.selectOrgType } _.unset(req, 'session.pageData.createService') return response(req, res, 'services/add-service', context) @@ -28,7 +28,7 @@ async function post (req, res, next) { _.set(req, 'session.pageData.createService.errors', { organisation_type: 'Organisation type is required' }) - return res.redirect(paths.serviceSwitcher.create.selectOrgType) + return res.redirect(paths.services.create.selectOrgType) } try { diff --git a/src/controllers/create-service/get.controller.test.js b/src/controllers/create-service/get.controller.test.js index 0dda19e2f9..07e4ee6c6d 100644 --- a/src/controllers/create-service/get.controller.test.js +++ b/src/controllers/create-service/get.controller.test.js @@ -37,7 +37,7 @@ describe('Controller: createService, Method: get', () => { }) it('should pass pageData to the responses.response method that has properly formatted \'submit_link\' and \'my_services\' properties', () => { - expect(mockResponses.response.args[0][3]).to.have.property('submit_link').to.equal('/my-services/create/select-org-type') + expect(mockResponses.response.args[0][3]).to.have.property('submit_link').to.equal('/services/create/select-org-type') expect(mockResponses.response.args[0][3]).to.have.property('back_link').to.equal('/my-services') }) }) diff --git a/src/controllers/create-service/post.controller.test.js b/src/controllers/create-service/post.controller.test.js index 593110280d..ba30fed454 100644 --- a/src/controllers/create-service/post.controller.test.js +++ b/src/controllers/create-service/post.controller.test.js @@ -157,7 +157,7 @@ describe('Controller: createService, Method: post', () => { }) it('should redirect back to select org type', () => { - sinon.assert.calledWith(res.redirect, paths.serviceSwitcher.create.selectOrgType) + sinon.assert.calledWith(res.redirect, paths.services.create.selectOrgType) }) it('should set error information on the session', () => { diff --git a/src/controllers/create-service/select-organisation-type/select-organisation-type.controller.js b/src/controllers/create-service/select-organisation-type/select-organisation-type.controller.js index 7545cb5e33..eb9dbb2824 100644 --- a/src/controllers/create-service/select-organisation-type/select-organisation-type.controller.js +++ b/src/controllers/create-service/select-organisation-type/select-organisation-type.controller.js @@ -12,12 +12,12 @@ function get (req, res) { const createServiceState = _.get(req, 'session.pageData.createService', {}) if (!createServiceState.current_name) { logger.info('Page accessed out of sequence, redirecting to my-services/create') - return res.redirect(paths.serviceSwitcher.create.index) + return res.redirect(paths.services.create.index) } const context = { ...createServiceState, - back_link: paths.serviceSwitcher.create.index, - submit_link: paths.serviceSwitcher.create.index + back_link: paths.services.create.index, + submit_link: paths.services.create.index } _.unset(req, 'session.pageData.createService.errors') return response(req, res, 'services/select-org-type', context) @@ -32,11 +32,11 @@ function post (req, res) { const errors = validateServiceName(req.body['service-name'], req.body['service-name-cy']) if (!_.isEmpty(errors)) { _.set(req, 'session.pageData.createService.errors', errors) - return res.redirect(paths.serviceSwitcher.create.index) + return res.redirect(paths.services.create.index) } const context = { - back_link: paths.serviceSwitcher.create.index, - submit_link: paths.serviceSwitcher.create.index + back_link: paths.services.create.index, + submit_link: paths.services.create.index } return response(req, res, 'services/select-org-type', context) } diff --git a/src/controllers/create-service/select-organisation-type/select-organisation-type.test.js b/src/controllers/create-service/select-organisation-type/select-organisation-type.test.js index bb0170b865..2cde7cdd58 100644 --- a/src/controllers/create-service/select-organisation-type/select-organisation-type.test.js +++ b/src/controllers/create-service/select-organisation-type/select-organisation-type.test.js @@ -25,7 +25,7 @@ describe('Controller: selectOrganisationType, Method: get', () => { redirect: sinon.spy() } selectOrganisationTypeController.get(req, res) - sinon.assert.calledWith(res.redirect, paths.serviceSwitcher.create.index) + sinon.assert.calledWith(res.redirect, paths.services.create.index) }) }) @@ -60,8 +60,8 @@ describe('Controller: selectOrganisationType, Method: get', () => { expect(responseContext).to.have.property('errors').to.deep.equal({ organisation_type: 'Organisation type is required' }) - expect(responseContext).to.have.property('back_link').to.equal(paths.serviceSwitcher.create.index) - expect(responseContext).to.have.property('submit_link').to.equal(paths.serviceSwitcher.create.index) + expect(responseContext).to.have.property('back_link').to.equal(paths.services.create.index) + expect(responseContext).to.have.property('submit_link').to.equal(paths.services.create.index) }) it('should remove errors pageData from the session', () => { @@ -93,8 +93,8 @@ describe('Controller: selectOrganisationType, Method: post', () => { expect(createServiceState).to.have.property('current_name_cy').to.equal(WELSH_SERVICE_NAME) expect(createServiceState).to.have.property('service_selected_cy').to.equal(true) const responseContext = mockResponse.response.args[0][3] - expect(responseContext).to.have.property('back_link').to.equal(paths.serviceSwitcher.create.index) - expect(responseContext).to.have.property('submit_link').to.equal(paths.serviceSwitcher.create.index) + expect(responseContext).to.have.property('back_link').to.equal(paths.services.create.index) + expect(responseContext).to.have.property('submit_link').to.equal(paths.services.create.index) }) }) describe('when request fails validation', () => { @@ -116,7 +116,7 @@ describe('Controller: selectOrganisationType, Method: post', () => { it('should redirect to the create service controller and set the session data', () => { expect(mockResponse.response.called).to.equal(false) - sinon.assert.calledWith(res.redirect, paths.serviceSwitcher.create.index) + sinon.assert.calledWith(res.redirect, paths.services.create.index) const expectedErrors = req.session.pageData.createService.errors expect(expectedErrors).to.have.property('service_name').to.equal('Service name must be 50 characters or fewer') expect(expectedErrors).to.have.property('service_name_cy').to.equal('Welsh service name must be 50 characters or fewer') diff --git a/src/controllers/dashboard/dashboard-activity.controller.js b/src/controllers/dashboard/dashboard-activity.controller.js index 1822a1a737..a0e5753983 100644 --- a/src/controllers/dashboard/dashboard-activity.controller.js +++ b/src/controllers/dashboard/dashboard-activity.controller.js @@ -26,7 +26,7 @@ const { DENIED } = require('@models/constants/go-live-stage') const pspTestAccountStage = require('@models/constants/psp-test-account-stage') -const serviceService = require('../../services/service.service') +const { WORLDPAY } = require('@models/constants/payment-providers') const links = { demoPayment: 0, @@ -102,11 +102,11 @@ const displayRequestTestStripeAccountLink = (service, account, user) => { user.hasPermission(service.externalId, 'psp-test-account-stage:update') } -async function isWorldpayTestService (gatewayAccountIds) { - const gatewayAccounts = await serviceService.getGatewayAccounts(gatewayAccountIds) - logger.info(`returned gateway accounts: ${JSON.stringify(gatewayAccounts)}`) - return gatewayAccounts.length === 1 && gatewayAccounts[0].type === 'test' && - gatewayAccounts[0].payment_provider.toUpperCase() === 'WORLDPAY' +async function isWorldpayTestService (service, account) { + return service.gatewayAccountIds.length === 1 && + parseInt(account.gateway_account_id) === parseInt(service.gatewayAccountIds[0]) && + account.type === 'test' && + account.payment_provider === WORLDPAY } module.exports = async (req, res) => { @@ -114,7 +114,7 @@ module.exports = async (req, res) => { const messages = res.locals.flash.messages const period = _.get(req, 'query.period', 'today') const telephonePaymentLink = await getTelephonePaymentLink(req.user, req.service, gatewayAccountId) - const worldpayTestService = await isWorldpayTestService(req.service.gatewayAccountIds) + const worldpayTestService = await isWorldpayTestService(req.service, req.account) const linksToDisplay = getLinksToDisplay(req.service, req.account, req.user, telephonePaymentLink) const model = { serviceId: req.service.externalId, diff --git a/src/controllers/dashboard/dashboard-activity.controller.test.js b/src/controllers/dashboard/dashboard-activity.controller.test.js index 5e93e49095..9056ccc6b9 100644 --- a/src/controllers/dashboard/dashboard-activity.controller.test.js +++ b/src/controllers/dashboard/dashboard-activity.controller.test.js @@ -35,6 +35,7 @@ describe('Controller: Dashboard activity', () => { req = { account, service: { + gatewayAccountIds: serviceGatewayAccountIds, currentGoLiveStage: null }, user diff --git a/src/controllers/edit-service-name/edit-service-name.controller.js b/src/controllers/edit-service-name/edit-service-name.controller.js index 77729959f4..c4f72aca26 100644 --- a/src/controllers/edit-service-name/edit-service-name.controller.js +++ b/src/controllers/edit-service-name/edit-service-name.controller.js @@ -23,7 +23,7 @@ function getServiceName (req, res) { } pageData.submit_link = formatServicePathsFor(paths.service.editServiceName.update, req.service.externalId) - pageData.my_services = paths.serviceSwitcher.index + pageData.my_services = paths.services.index return responses.response(req, res, 'services/edit-service-name', pageData) } @@ -56,7 +56,7 @@ async function postEditServiceName (req, res, next) { } else { try { await serviceService.updateServiceName(serviceExternalId, serviceName, serviceNameCy) - res.redirect(paths.serviceSwitcher.index) + res.redirect(paths.services.index) } catch (err) { next(err) } diff --git a/src/controllers/invite-validation.controller.js b/src/controllers/invite-validation.controller.js index 7cf742feab..abfb96ec69 100644 --- a/src/controllers/invite-validation.controller.js +++ b/src/controllers/invite-validation.controller.js @@ -33,7 +33,7 @@ async function validateInvite (req, res, next) { if (invite.is_invite_to_join_service) { res.redirect(paths.invite.subscribeService) } else { - res.redirect(paths.serviceSwitcher.index) + res.redirect(paths.services.index) } } else { res.redirect(paths.register.password) diff --git a/src/controllers/invite-validation.controller.test.js b/src/controllers/invite-validation.controller.test.js index 93e6548bcf..e79a6a751f 100644 --- a/src/controllers/invite-validation.controller.test.js +++ b/src/controllers/invite-validation.controller.test.js @@ -72,7 +72,7 @@ describe('Invite validation controller', function () { await controller.validateInvite(req, res, next) sinon.assert.calledWith(getValidatedInviteSpy, code) - sinon.assert.calledWith(res.redirect, paths.serviceSwitcher.index) + sinon.assert.calledWith(res.redirect, paths.services.index) sinon.assert.notCalled(next) }) diff --git a/src/controllers/my-services/get-index.controller.js b/src/controllers/my-services/get-index.controller.js deleted file mode 100644 index 47e82b7801..0000000000 --- a/src/controllers/my-services/get-index.controller.js +++ /dev/null @@ -1,85 +0,0 @@ -'use strict' - -const lodash = require('lodash') - -const { response } = require('../../utils/response') -const serviceService = require('../../services/service.service') -const { filterGatewayAccountIds } = require('../../utils/permissions') -const getHeldPermissions = require('../../utils/get-held-permissions') -const { DEFAULT_SERVICE_NAME } = require('../../utils/constants') - -function hasStripeAccount (gatewayAccounts) { - return gatewayAccounts.some(gatewayAccount => - gatewayAccount.payment_provider === 'stripe') -} - -function sortServicesByLiveThenName (a, b) { - const aHasLive = a.gatewayAccounts.some(account => account.type === 'live') - const bHasLive = b.gatewayAccounts.some(account => account.type === 'live') - const aName = a.name.toLowerCase() - const bName = b.name.toLowerCase() - - // live comes before test, then sort by name ascending - if (aHasLive && !bHasLive) { return -1 } - if (!aHasLive && bHasLive) { return 1 } - if (aName < bName) { return -1 } - if (aName > bName) { return 1 } - return 0 -} - -function isWorldpayTestService (gatewayAccounts) { - return gatewayAccounts.length === 1 && gatewayAccounts[0].type === 'test' && - gatewayAccounts[0].payment_provider.toUpperCase() === 'WORLDPAY' -} - -module.exports = async function getServiceList (req, res) { - const servicesRoles = lodash.get(req, 'user.serviceRoles', []) - const newServiceId = res.locals.flash && res.locals.flash.inviteSuccessServiceId && - res.locals.flash.inviteSuccessServiceId[0] - - const aggregatedGatewayAccountIds = servicesRoles - .flatMap(servicesRole => servicesRole.service.gatewayAccountIds) - - let aggregatedGatewayAccounts = [] - if (aggregatedGatewayAccountIds.length) { - aggregatedGatewayAccounts = await serviceService.getGatewayAccounts(aggregatedGatewayAccountIds) - } - const servicesData = servicesRoles - .map(serviceRole => { - const gatewayAccounts = aggregatedGatewayAccounts.filter(gatewayAccount => - serviceRole.service.gatewayAccountIds.includes(gatewayAccount.id.toString())) - const isAdminUser = req.user.isAdminUserForService(serviceRole.service.externalId) - - const serviceData = { - name: serviceRole.service.name === DEFAULT_SERVICE_NAME ? 'Temporary Service Name' : serviceRole.service.name, - id: serviceRole.service.id, - external_id: serviceRole.service.externalId, - gatewayAccounts: lodash.sortBy(gatewayAccounts, 'type', 'asc'), - permissions: getHeldPermissions(serviceRole.role.permissions.map(permission => permission.name)), - isAdminUser, - isWorldpayTestService: isWorldpayTestService(gatewayAccounts) - } - return serviceData - }) - .sort((a, b) => sortServicesByLiveThenName(a, b)) - - const data = { - services: servicesData, - services_singular: servicesData.length === 1, - env: process.env, - has_account_with_payouts: hasStripeAccount(aggregatedGatewayAccounts), - has_live_account: filterGatewayAccountIds(aggregatedGatewayAccounts, true).length - } - - if (newServiceId) { - servicesData.find(service => { - if (service.external_id === newServiceId) { - data.new_service_name = service.name - return true - } - return false - }) - } - - return response(req, res, 'services/index', data) -} diff --git a/src/controllers/my-services/index.js b/src/controllers/my-services/index.js deleted file mode 100644 index fa69a606e6..0000000000 --- a/src/controllers/my-services/index.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict' - -exports.getIndex = require('./get-index.controller') -exports.postIndex = require('./post-index.controller') diff --git a/src/controllers/my-services/post-index.controller.js b/src/controllers/my-services/post-index.controller.js deleted file mode 100644 index 29b44590b2..0000000000 --- a/src/controllers/my-services/post-index.controller.js +++ /dev/null @@ -1,22 +0,0 @@ -const _ = require('lodash') - -const logger = require('../../utils/logger')(__filename) -const paths = require('../../paths') -const formatAccountPathsFor = require('../../utils/format-account-paths-for') - -const validAccountId = (accountId, user) => { - const gatewayAccountIds = _.flattenDeep(_.concat(user.serviceRoles.map(serviceRole => serviceRole.service.gatewayAccountIds))) - return accountId && gatewayAccountIds.indexOf(accountId) !== -1 -} - -module.exports = (req, res) => { - const gatewayAccountId = req.body && req.body.gatewayAccountId - const gatewayAccountExternalId = req.body && req.body.gatewayAccountExternalId - - if (validAccountId(gatewayAccountId, req.user)) { - res.redirect(302, formatAccountPathsFor(paths.account.dashboard.index, gatewayAccountExternalId)) - } else { - logger.warn(`Attempted to switch to invalid account ${gatewayAccountId}`) - res.redirect(302, paths.serviceSwitcher.index) - } -} diff --git a/src/controllers/root/index.controller.js b/src/controllers/root/index.controller.js index 75488d81e7..62b23bf669 100644 --- a/src/controllers/root/index.controller.js +++ b/src/controllers/root/index.controller.js @@ -3,7 +3,7 @@ const paths = require('../../paths') function get (req, res) { - res.redirect(paths.serviceSwitcher.index) + res.redirect(paths.services.index) } module.exports = { diff --git a/src/controllers/service-switch.controller.it.test.js b/src/controllers/service-switch.controller.it.test.js deleted file mode 100644 index 9829f5043e..0000000000 --- a/src/controllers/service-switch.controller.it.test.js +++ /dev/null @@ -1,267 +0,0 @@ -'use strict' - -const chai = require('chai') -const nock = require('nock') -const sinon = require('sinon') -const _ = require('lodash') -const chaiAsPromised = require('chai-as-promised') -const { expect } = require('chai') - -const myServicesController = require('./my-services') -const User = require('../models/User.class') -const userFixtures = require('../../test/fixtures/user.fixtures') -const gatewayAccountFixtures = require('../../test/fixtures/gateway-account.fixtures') - -chai.use(chaiAsPromised) - -const connectorMock = nock(process.env.CONNECTOR_URL) -const ACCOUNTS_API_PATH = '/v1/api/accounts' - -describe('My services controller', () => { - describe('service list', function () { - const gatewayAccountsByService = { - service1: [{ - gateway_account_id: '2', - type: 'test' - } - ], - service2: [ - { - gateway_account_id: '3', - type: 'live' - }, - { - gateway_account_id: '6', - type: 'test' - } - ], - service3: [ - { - gateway_account_id: '4', - type: 'test' - }, - { - gateway_account_id: '9', - type: 'live' - } - ], - service4: [ - { - gateway_account_id: '5', - type: 'test' - } - ] - } - const user = new User(userFixtures.validUserResponse({ - service_roles: [ - { - service: { - id: 201, - name: 'A service', - external_id: 'service-external-id-1', - gateway_account_ids: gatewayAccountsByService.service1.map(account => account.gateway_account_id) - } - }, - { - service: { - id: 12, - name: 'C service', - external_id: 'service-external-id-2', - gateway_account_ids: gatewayAccountsByService.service2.map(account => account.gateway_account_id) - } - }, - { - service: { - id: 122, - name: 'B service', - external_id: 'service-external-id-3', - gateway_account_ids: gatewayAccountsByService.service3.map(account => account.gateway_account_id) - } - }, - { - service: { - id: 120, - name: 'a service 2', - external_id: 'service-external-id-4', - gateway_account_ids: gatewayAccountsByService.service4.map(account => account.gateway_account_id) - } - }] - })) - - const accounts = Object.values(gatewayAccountsByService).flat() - const accountIds = accounts.map(account => account.gateway_account_id) - let res - - before(async () => { - connectorMock.get(ACCOUNTS_API_PATH + `?accountIds=${accountIds.join(',')}`) - .reply(200, gatewayAccountFixtures.validGatewayAccountsResponse({ accounts })) - - res = { - render: sinon.spy(), - locals: {} - } - const req = { - user - } - await myServicesController.getIndex(req, res) - }) - - after(() => { - nock.cleanAll() - }) - - it('should call render', () => { - sinon.assert.called(res.render) - expect(res.render.firstCall.args[0]).to.equal('services/index') - }) - - it('should return a list sorted by live/test then alphabetically', () => { - const { services } = res.render.firstCall.args[1] - expect(services).to.have.length(4) - expect(services.map(service => service.external_id)).to.have.ordered.members([ - 'service-external-id-3', 'service-external-id-2', 'service-external-id-1', 'service-external-id-4' - ]) - }) - - it('should have gateway accounts sorted by type within services', () => { - it('should have gateway accounts sorted by live/test within services', () => { - const { services } = res.render.firstCall.args[1] - expect(services[0].gatewayAccounts.map(a => a.id)).to.have.length(2) - .and.to.have.ordered.members(['9', '4']) - expect(services[1].gatewayAccounts.map(a => a.id)).to.have.length(2) - .and.to.have.ordered.members(['3', '6']) - expect(services[2].gatewayAccounts.map(a => a.id)).to.have.length(1) - .and.to.have.ordered.members(['2']) - expect(services[3].gatewayAccounts.map(a => a.id)).to.have.length(1) - .and.to.have.ordered.members(['5']) - }) - }) - }) - - describe('switching', function () { - it('should redirect to / with correct account id set', function () { - const session = {} - const gatewayAccountExternalId = 'some-external-id' - - const req = { - originalUrl: 'http://bob.com?accountId=6', - user: new User(userFixtures.validUserResponse({ - username: 'bob', - service_roles: [{ - service: { - gateway_account_ids: ['6', '5'] - } - }] - })), - session, - body: { - gatewayAccountId: '6', - gatewayAccountExternalId - } - } - - const res = { - redirect: function () { - expect(arguments[0]).to.equal(302) - expect(arguments[1]).to.equal(`/account/${gatewayAccountExternalId}/dashboard`) - } - } - - myServicesController.postIndex(req, res) - }) - - it('should not switch id if user not authorised to see account id', function () { - const session = {} - - const req = { - originalUrl: 'http://bob.com?accountId=6', - user: new User(userFixtures.validUserResponse({ - username: 'bob', - gateway_account_ids: ['8', '666'] - })), - session - } - - const res = { - redirect: function () { - expect(session).to.deep.equal({}) - expect(arguments[0]).to.equal(302) - expect(arguments[1]).to.equal('/my-services') - } - } - - myServicesController.postIndex(req, res) - }) - }) - - describe('display added to the new service msg', function () { - beforeEach(() => { - nock.cleanAll() - }) - - it('should render a list of services when user has multiple services and display added to new service message', function (done) { - const service1gatewayAccountIds = ['2', '5'] - const newServiceGatewayAccountIds = ['3', '6', '7'] - const gatewayAccountIds = _.concat(service1gatewayAccountIds, newServiceGatewayAccountIds) - - connectorMock.get(ACCOUNTS_API_PATH + `?accountIds=${gatewayAccountIds.join(',')}`) - .reply(200, { - accounts: gatewayAccountIds.map(iter => gatewayAccountFixtures.validGatewayAccountResponse({ - gateway_account_id: iter, - service_name: `account ${iter}`, - type: _.sample(['test', 'live']) - })) - }) - - const newServiceName = 'My New Service' - const newServiceExternalId = 'service-external-id-2' - - const req = { - user: new User(userFixtures.validUserResponse({ - username: 'bob', - service_roles: [ - { - service: { - name: 'My Service 1', - external_id: 'service-external-id-1', - gateway_account_ids: service1gatewayAccountIds - }, - role: { - name: 'admin', - permissions: [{ name: 'blah-blah:blah' }] - } - }, - { - service: { - name: newServiceName, - external_id: newServiceExternalId, - gateway_account_ids: newServiceGatewayAccountIds - }, - role: { - name: 'admin', - permissions: [{ name: 'blah-blah:blah' }] - } - }] - })), - session: {} - } - - const res = { - locals: { - flash: { - inviteSuccessServiceId: [newServiceExternalId] - } - }, - render: function () { - const path = arguments[0] - const renderData = arguments[1] - expect(path).to.equal('services/index') - expect(renderData.new_service_name).to.be.equal(newServiceName) - done() - } - } - - myServicesController.getIndex(req, res) - }) - }) -}) diff --git a/src/controllers/simplified-account/services/index.js b/src/controllers/simplified-account/services/index.js new file mode 100644 index 0000000000..7b055b3e50 --- /dev/null +++ b/src/controllers/simplified-account/services/index.js @@ -0,0 +1 @@ +module.exports.myServices = require('@controllers/simplified-account/services/my-services.controller') diff --git a/src/controllers/simplified-account/services/my-services.controller.js b/src/controllers/simplified-account/services/my-services.controller.js new file mode 100644 index 0000000000..e91f137402 --- /dev/null +++ b/src/controllers/simplified-account/services/my-services.controller.js @@ -0,0 +1,140 @@ +const { response } = require('@utils/response') +const paths = require('@root/paths') +const { STRIPE, SANDBOX, WORLDPAY } = require('@models/constants/payment-providers') +const getHeldPermissions = require('@utils/get-held-permissions') +const logger = require('@utils/logger')(__filename) +const gatewayAccountsService = require('@services/gateway-accounts.service') +const { DEFAULT_SERVICE_NAME } = require('@utils/constants') +const { formattedPathFor } = require('@root/paths') +const { accountLinksGenerator, sortByLiveThenName, isWorldpayTestService } = require('@utils/simplified-account/services/my-services/service-presentation-utils') + +async function get (req, res) { + const userServiceRoles = req.user.serviceRoles + const flags = { + userIsDegatewayed: req.user.isDegatewayed() + } + + if (res.locals?.flash?.inviteSuccessServiceId?.[0]) { + flags.recentlyInvitedServiceExternalId = res.locals.flash.inviteSuccessServiceId[0] + } + + const gatewayAccountIds = userServiceRoles.flatMap(role => { + if (role?.service?.gatewayAccountIds && Array.isArray(role.service.gatewayAccountIds)) { + return role.service.gatewayAccountIds + } + return [] + }) + + let services = [] + if (gatewayAccountIds.length > 0) { + const gatewayAccounts = await gatewayAccountsService.getGatewayAccountsByIds(gatewayAccountIds) + services = mergeServicesWithGatewayAccounts(userServiceRoles, gatewayAccounts, flags) + .sort((a, b) => sortByLiveThenName(a, b)) + } + + const pathFilter = flags.hasLiveAccount ? 'live' : 'test' + + return response(req, res, 'simplified-account/services/my-services/index', { + createServicePath: paths.services.create.index, + allServiceTransactionsPath: formattedPathFor(paths.allServiceTransactions.indexStatusFilter, pathFilter), + payoutsPath: formattedPathFor(paths.payouts.listStatusFilter, pathFilter), + services, + flags + }) +} + +const mergeServicesWithGatewayAccounts = (services, gatewayAccounts, flags) => { + return services.map(serviceRole => { + const { service, role } = serviceRole + + if (flags.recentlyInvitedServiceExternalId === service.externalId) { + flags.recentlyInvitedServiceName = service.name + } + + const mappedGatewayAccounts = service.gatewayAccountIds + .map(id => gatewayAccounts[id]) + .filter(account => account !== undefined) + + const mappedLiveGatewayAccounts = mappedGatewayAccounts + .filter(account => account.type === 'live') + + let mappedTestGatewayAccounts = mappedGatewayAccounts + .filter(account => account.type === 'test') + + if (flags.userIsDegatewayed) { + mappedTestGatewayAccounts = filterTestGatewaysDegatewayView(mappedTestGatewayAccounts, service) + } + + if (mappedLiveGatewayAccounts.length > 0) { + flags.hasLiveAccount = true + } + + const associatedGatewayAccounts = [ + ...mappedLiveGatewayAccounts, + ...mappedTestGatewayAccounts + ].map(account => { + if (account.paymentProvider === STRIPE) { + flags.hasAccountWithPayouts = true + } + return { + id: account.id, + externalId: account.externalId, + type: account.type, + paymentProvider: account.paymentProvider, + allowMoto: account.allowMoto, + providerSwitchEnabled: account.providerSwitchEnabled, + recurringEnabled: account.recurringEnabled, + links: accountLinksGenerator(account, service, flags.userIsDegatewayed) + } + }) + .sort((a, b) => { + // ensure live gateway is first in array + if (a.type === 'live') return -1 + return b.type === 'live' ? 1 : 0 + }) + + const serviceName = service.serviceName.en || service.name + + return { + externalId: service.externalId, + name: serviceName === DEFAULT_SERVICE_NAME ? 'Temporary Service Name' : serviceName, + goLiveStage: service.currentGoLiveStage, + currentPspTestAccountStage: service.currentPspTestAccountStage, + createdDate: service.createdDate, + isWorldpayTestService: isWorldpayTestService(associatedGatewayAccounts), + userIsAdminForService: role.name === 'admin', + permissions: getHeldPermissions(role.permissions.map(permission => permission.name)), + gatewayAccounts: associatedGatewayAccounts + } + }) +} + +// PP-13525 return exactly one test account. PSP preference: Stripe -> Sandbox -> Worldpay +const filterTestGatewaysDegatewayView = (testGatewayAccounts, service) => { + return testGatewayAccounts + .filter((account, _, accounts) => { + if (accounts.length === 0) { + logger.warn(`Service has no associated test gateway [service_external_id: ${service.externalId}]`) + return false + } + if (accounts.length === 1) { + if ([STRIPE, SANDBOX, WORLDPAY].includes(accounts[0].paymentProvider)) return true + logger.warn(`Resolved test account is not of supported type [service_external_id: ${service.externalId}, payment_provider: ${accounts[0].paymentProvider}]`) + return false + } + for (const provider of [STRIPE, SANDBOX, WORLDPAY]) { + const testAccountsByProvider = accounts.filter(testAccount => testAccount.paymentProvider === provider) + if (testAccountsByProvider.length > 1) { + logger.warn(`Multiple ${provider} test accounts found for service [external_id: ${service.externalId}]`) + // if for some reason there is more than one test account with the same provider, use the ID to work out the newest one + testAccountsByProvider.sort((a, b) => parseInt(b.id) - parseInt(a.id)) + return account.id === testAccountsByProvider[0].id + } + } + return false + }) +} + +module.exports = { + get +} diff --git a/src/controllers/simplified-account/settings/stripe-details/responsible-person/responsible-person-check-answers.controller.js b/src/controllers/simplified-account/settings/stripe-details/responsible-person/responsible-person-check-answers.controller.js index efdd233059..0a5ca36825 100644 --- a/src/controllers/simplified-account/settings/stripe-details/responsible-person/responsible-person-check-answers.controller.js +++ b/src/controllers/simplified-account/settings/stripe-details/responsible-person/responsible-person-check-answers.controller.js @@ -12,7 +12,7 @@ const _ = require('lodash') async function get (req, res) { const { name, dob, address, contact } = _.get(req, FORM_STATE_KEY, {}) if (!Object.values({ name, dob, address, contact }).every(Boolean)) { - res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.responsiblePerson.index, req.service.externalId, req.account.type)) + return res.redirect(formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.responsiblePerson.index, req.service.externalId, req.account.type)) } return response(req, res, 'simplified-account/settings/stripe-details/responsible-person/check-your-answers', { backLink: formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.stripeDetails.responsiblePerson.contactDetails, req.service.externalId, req.account.type), diff --git a/src/controllers/subscribe-service.controller.js b/src/controllers/subscribe-service.controller.js index e42582ae63..b06890b653 100644 --- a/src/controllers/subscribe-service.controller.js +++ b/src/controllers/subscribe-service.controller.js @@ -16,13 +16,13 @@ const subscribeService = async function subscribeService (req, res, next) { logger.info('Attempt to accept invite for a different user', { invite_code: inviteCode }) - return res.redirect(303, paths.serviceSwitcher.index) + return res.redirect(303, paths.services.index) } try { const completeResponse = await adminusersClient.completeInvite(inviteCode, SMS) req.flash('inviteSuccessServiceId', completeResponse.service_external_id) - return res.redirect(303, paths.serviceSwitcher.index) + return res.redirect(303, paths.services.index) } catch (err) { if (err.errorCode === 410) { return next(new ExpiredInviteError(`Invite with code ${sessionData.code} has expired`)) diff --git a/src/controllers/subscribe-service.controller.test.js b/src/controllers/subscribe-service.controller.test.js index 3c856d7f18..24585d6ee8 100644 --- a/src/controllers/subscribe-service.controller.test.js +++ b/src/controllers/subscribe-service.controller.test.js @@ -83,7 +83,7 @@ describe('Subscribe service controller', () => { const controller = getController(completeInviteSuccessStub) await controller.subscribeService(req, res, next) sinon.assert.notCalled(completeInviteSuccessStub) - sinon.assert.calledWith(res.redirect, 303, paths.serviceSwitcher.index) + sinon.assert.calledWith(res.redirect, 303, paths.services.index) }) }) diff --git a/src/middleware/simplified-account/settings/default-view-decider.middleware.js b/src/middleware/simplified-account/settings/default-view-decider.middleware.js index 685e402e74..00a5395b41 100644 --- a/src/middleware/simplified-account/settings/default-view-decider.middleware.js +++ b/src/middleware/simplified-account/settings/default-view-decider.middleware.js @@ -3,17 +3,16 @@ const { formatSimplifiedAccountPathsFor } = require('@utils/simplified-account/f const paths = require('@root/paths') const serviceSettingsController = require('@controllers/simplified-account/settings') -module.exports = function (req, res, next) { +module.exports = function (req, res) { const account = req.account const service = req.service const isServiceAdmin = req.user.isAdminUserForService(service.externalId) const useEmailNotificationsController = !isServiceAdmin || (account.type === 'test' && service.currentGoLiveStage === LIVE) if (useEmailNotificationsController) { req.url = formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.emailNotifications.index, service.externalId, account.type) - req.selectedController = serviceSettingsController.emailNotifications.getEmailNotificationsSettingsPage + return serviceSettingsController.emailNotifications.getEmailNotificationsSettingsPage(req, res) } else { req.url = formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.serviceName.index, service.externalId, account.type) - req.selectedController = serviceSettingsController.serviceName.get + return serviceSettingsController.serviceName.get(req, res) } - next() } diff --git a/src/middleware/simplified-account/simplified-account-strategy.middleware.js b/src/middleware/simplified-account/simplified-account-strategy.middleware.js index c27cf9a4e4..484269fcc9 100644 --- a/src/middleware/simplified-account/simplified-account-strategy.middleware.js +++ b/src/middleware/simplified-account/simplified-account-strategy.middleware.js @@ -9,13 +9,25 @@ const connectorClient = new Connector(process.env.CONNECTOR_URL) function getService (user, serviceExternalId, gatewayAccountId) { let service + let matchedByExternalId const serviceRoles = _.get(user, 'serviceRoles', []) if (serviceRoles.length > 0 && serviceExternalId) { service = _.get(serviceRoles.find(serviceRole => { - return (serviceRole.service.externalId === serviceExternalId && - (!gatewayAccountId || serviceRole.service.gatewayAccountIds.includes(String(gatewayAccountId)))) - }), 'service') + const externalIdMatch = serviceRole.service.externalId === serviceExternalId + if (externalIdMatch) { + matchedByExternalId = serviceRole + if (gatewayAccountId && !serviceRole.service.gatewayAccountIds.includes(`${gatewayAccountId}`)) { + /* + if you're here debugging this error message, it means that connector returned a gateway account for the + serviceExtId/account type that adminusers does not know about and probably needs relinking + */ + logger.warn(`Resolved gateway account is not present on service [service_external_id: ${serviceExternalId}, gateway_account_id: ${gatewayAccountId}]`) + return false + } + } + return externalIdMatch + }) || matchedByExternalId, 'service') } return service diff --git a/src/middleware/simplified-account/simplified-account-strategy.middleware.test.js b/src/middleware/simplified-account/simplified-account-strategy.middleware.test.js index 00f403e024..e365aa9c08 100644 --- a/src/middleware/simplified-account/simplified-account-strategy.middleware.test.js +++ b/src/middleware/simplified-account/simplified-account-strategy.middleware.test.js @@ -26,17 +26,18 @@ const buildUser = (serviceExternalId, gatewayAccountIds) => { const setupSimplifiedAccountStrategyTest = function (options) { const { - gatewayAccountID, + gatewayAccountId, gatewayAccountExternalId, paymentProvider, serviceExternalId, + serviceGatewayAccountIds, accountType, errorCode } = options req = { params: { serviceExternalId, accountType }, - user: buildUser(serviceExternalId, [`${gatewayAccountID}`]) + user: buildUser(serviceExternalId, serviceGatewayAccountIds || [`${gatewayAccountId}`]) } next = sinon.spy() @@ -45,7 +46,7 @@ const setupSimplifiedAccountStrategyTest = function (options) { connectorGetAccountMock = sinon.stub().rejects({ errorCode }) } else { connectorGetAccountMock = sinon.stub().resolves(new GatewayAccount({ - gateway_account_id: gatewayAccountID, + gateway_account_id: gatewayAccountId, external_id: gatewayAccountExternalId, payment_provider: paymentProvider })) @@ -62,21 +63,36 @@ const setupSimplifiedAccountStrategyTest = function (options) { } } + const loggerMock = { + info: sinon.stub(), + error: sinon.stub(), + debug: sinon.stub(), + warn: sinon.stub() + } + + const loggerStub = sinon.stub().callsFake(_ => { + return loggerMock + }) + const simplifiedAccountStrategy = proxyquire( path.join(__dirname, './simplified-account-strategy.middleware'), - { '@services/clients/connector.client.js': connectorMock } + { + '@services/clients/connector.client.js': connectorMock, + '@utils/logger': loggerStub + } ) return { simplifiedAccountStrategy, - connectorGetAccountMock + connectorGetAccountMock, + loggerMock } } describe('Middleware: getSimplifiedAccount', () => { it('should set gateway account and service on request object', async () => { const { simplifiedAccountStrategy, connectorGetAccountMock } = setupSimplifiedAccountStrategyTest({ - gatewayAccountID: '1', + gatewayAccountId: '1', gatewayAccountExternalId: A_GATEWAY_EXTERNAL_ID, paymentProvider: 'worldpay', serviceExternalId: A_SERVICE_EXTERNAL_ID, @@ -96,7 +112,7 @@ describe('Middleware: getSimplifiedAccount', () => { }) it('should error if service external ID or gateway account type cannot be resolved from request parameters', async () => { const { simplifiedAccountStrategy } = setupSimplifiedAccountStrategyTest({ - gatewayAccountID: '1', + gatewayAccountId: '1', gatewayAccountExternalId: A_GATEWAY_EXTERNAL_ID, paymentProvider: 'worldpay', serviceExternalId: A_SERVICE_EXTERNAL_ID, @@ -114,7 +130,7 @@ describe('Middleware: getSimplifiedAccount', () => { }) it('should error if gateway account lookup fails for account type', async () => { const { simplifiedAccountStrategy } = setupSimplifiedAccountStrategyTest({ - gatewayAccountID: '1', + gatewayAccountId: '1', serviceExternalId: A_SERVICE_EXTERNAL_ID, accountType: 'test', errorCode: '404' @@ -126,10 +142,24 @@ describe('Middleware: getSimplifiedAccount', () => { sinon.assert.calledOnce(next) sinon.assert.calledWith(next, expectedError) }) + it('should warn if gateway account id is not present on service', async () => { + const { simplifiedAccountStrategy, loggerMock } = setupSimplifiedAccountStrategyTest({ + gatewayAccountId: '1', + gatewayAccountExternalId: A_GATEWAY_EXTERNAL_ID, + serviceGatewayAccountIds: ['2', '3'], + serviceExternalId: A_SERVICE_EXTERNAL_ID, + accountType: 'test' + }) + await simplifiedAccountStrategy(req, res, next) + expect(loggerMock.warn).to.have.been + .calledWith(`Resolved gateway account is not present on service [service_external_id: ${A_SERVICE_EXTERNAL_ID}, gateway_account_id: 1]`) + expect(req.account.externalId).to.equal(A_GATEWAY_EXTERNAL_ID) + expect(req.service.externalId).to.equal(A_SERVICE_EXTERNAL_ID) + }) describe('extend gateway account data with disableToggle3ds field', () => { it('should extend the account data with disableToggle3ds set to false if payment provider is worldpay', async () => { const { simplifiedAccountStrategy } = setupSimplifiedAccountStrategyTest({ - gatewayAccountID: '1', + gatewayAccountId: '1', gatewayAccountExternalId: A_GATEWAY_EXTERNAL_ID, paymentProvider: 'worldpay', serviceExternalId: A_SERVICE_EXTERNAL_ID, @@ -141,7 +171,7 @@ describe('Middleware: getSimplifiedAccount', () => { }) it('should extend the account data with disableToggle3ds set to true if payment provider is stripe', async () => { const { simplifiedAccountStrategy } = setupSimplifiedAccountStrategyTest({ - gatewayAccountID: '1', + gatewayAccountId: '1', gatewayAccountExternalId: A_GATEWAY_EXTERNAL_ID, paymentProvider: 'stripe', serviceExternalId: A_SERVICE_EXTERNAL_ID, @@ -156,7 +186,7 @@ describe('Middleware: getSimplifiedAccount', () => { ['worldpay', 'stripe'].forEach(function (paymentProvider) { it('should extend the account data with supports3ds set to true if payment provider is ' + paymentProvider, async () => { const { simplifiedAccountStrategy } = setupSimplifiedAccountStrategyTest({ - gatewayAccountID: '1', + gatewayAccountId: '1', gatewayAccountExternalId: A_GATEWAY_EXTERNAL_ID, paymentProvider, serviceExternalId: A_SERVICE_EXTERNAL_ID, @@ -169,7 +199,7 @@ describe('Middleware: getSimplifiedAccount', () => { }) it('should extend the account data with supports3ds set to false if payment provider is sandbox', async () => { const { simplifiedAccountStrategy } = setupSimplifiedAccountStrategyTest({ - gatewayAccountID: '1', + gatewayAccountId: '1', gatewayAccountExternalId: A_GATEWAY_EXTERNAL_ID, paymentProvider: 'sandbox', serviceExternalId: A_SERVICE_EXTERNAL_ID, diff --git a/src/models/GatewayAccount.class.js b/src/models/GatewayAccount.class.js index 69fd6884b3..d69dc840d3 100644 --- a/src/models/GatewayAccount.class.js +++ b/src/models/GatewayAccount.class.js @@ -107,23 +107,6 @@ class GatewayAccount { return false } } - - /** - * Returns a minimal representation of the gateway account - * @returns {Object} A minimal representation of the gateway account - */ - toMinimalJson () { - // until we have external ids for card accounts, the external id is the internal one - return { - id: this.id, - external_id: this.externalId, - payment_provider: this.paymentProvider, - service_name: this.name, - type: this.type, - provider_switch_enabled: this.providerSwitchEnabled, - recurring_enabled: this.recurringEnabled - } - } } module.exports = GatewayAccount diff --git a/src/models/Service.class.js b/src/models/Service.class.js index b789a72e1f..766e69e6f4 100644 --- a/src/models/Service.class.js +++ b/src/models/Service.class.js @@ -7,6 +7,7 @@ * en: string, * cy: string * }} serviceName - object containing english and welsh service names + * @property {[string]} gatewayAccountIds - gateway account ids associated with service */ class Service { constructor (serviceData) { diff --git a/src/models/User.class.js b/src/models/User.class.js index 7bc4463e28..62628a8abb 100644 --- a/src/models/User.class.js +++ b/src/models/User.class.js @@ -14,7 +14,7 @@ const DEGATEWAY_FLAG = process.env.DEGATEWAY_FLAG === 'true' * @property {number} sessionVersion - The user's current session version * @property {string} otpKey - The user's OTP key * @property {string} telephoneNumber - The user's telephone number - * @property {boolean} disabled - Whether or not the user is disabled + * @property {boolean} disabled - Whether the user is disabled * @property {ServiceRole[]} serviceRoles - An array of the user's serviceRoles * @property {boolean} internalUser - Whether the user is internal * @property {number} numberOfLiveServices - number of live services user is associated with @@ -28,7 +28,7 @@ class User { * @param {string} userData.email - The user's email address * @param {string} userData.otp_key - The user's OTP key * @param {string} userData.telephone_number - The user's telephone number - * @param {boolean} userData.disabled - Whether or not the user's account is locked + * @param {boolean} userData.disabled - Whether the user's account is locked * @param {number} userData.session_version - The user's current session version * @param {Object[]} userData.service_roles - An array of the user's serviceRoles * @param {Object} userData.service_roles[].service - A raw service object see {@link Service.constructor} @@ -92,6 +92,7 @@ class User { /** * @method hasPermission + * @param {String} serviceExternalId * @param {String} permissionName name of permission * @returns {boolean} Whether or not the user has the given permission */ diff --git a/src/paths.js b/src/paths.js index 7e2b091fa9..c10845db2b 100644 --- a/src/paths.js +++ b/src/paths.js @@ -288,6 +288,13 @@ module.exports = { invite: '/team-members/invite' } }, + services: { + index: '/my-services', + create: { + index: '/services/create', + selectOrgType: '/services/create/select-org-type' + } + }, index: '/', allServiceTransactions: { index: '/all-service-transactions', @@ -319,14 +326,6 @@ module.exports = { } } }, - serviceSwitcher: { - index: '/my-services', - switch: '/my-services/switch', - create: { - index: '/my-services/create', - selectOrgType: '/my-services/create/select-org-type' - } - }, invite: { validateInvite: '/invites/:code', subscribeService: '/subscribe' diff --git a/src/routes.js b/src/routes.js index f8a77f2fc1..aa80c34139 100644 --- a/src/routes.js +++ b/src/routes.js @@ -39,7 +39,6 @@ const apiKeysController = require('./controllers/api-keys') const digitalWalletController = require('./controllers/digital-wallet') const emailNotificationsController = require('./controllers/email-notifications/email-notifications.controller') const forgotPasswordController = require('./controllers/forgotten-password.controller') -const myServicesController = require('./controllers/my-services') const editServiceNameController = require('./controllers/edit-service-name/edit-service-name.controller') const serviceUsersController = require('./controllers/service-users.controller') const organisationDetailsController = require('./controllers/organisation-details.controller') @@ -90,6 +89,7 @@ const agreementsController = require('./controllers/agreements/agreements.contro const organisationUrlController = require('./controllers/switch-psp/organisation-url') const registrationController = require('./controllers/registration/registration.controller') const privacyController = require('./controllers/privacy/privacy.controller') +const servicesController = require('./controllers/simplified-account/services') const simplifiedAccountRoutes = require('./simplified-account-routes') @@ -103,7 +103,7 @@ const { policyPage, payouts, register, - serviceSwitcher, + services, staticPaths, user } = paths @@ -221,13 +221,12 @@ module.exports.bind = function (app) { // OUTSIDE OF SERVICE ROUTES // ------------------------- - // Service switcher - app.get(serviceSwitcher.index, userIsAuthorised, myServicesController.getIndex) - app.post(serviceSwitcher.switch, userIsAuthorised, myServicesController.postIndex) - app.get(serviceSwitcher.create.index, userIsAuthorised, createServiceController.get) - app.post(serviceSwitcher.create.index, userIsAuthorised, createServiceController.post) - app.post(serviceSwitcher.create.selectOrgType, userIsAuthorised, selectOrgTypeController.post) - app.get(serviceSwitcher.create.selectOrgType, userIsAuthorised, selectOrgTypeController.get) + // My services + app.get(services.index, userIsAuthorised, servicesController.myServices.get) + app.get(services.create.index, userIsAuthorised, createServiceController.get) + app.post(services.create.index, userIsAuthorised, createServiceController.post) + app.post(services.create.selectOrgType, userIsAuthorised, selectOrgTypeController.post) + app.get(services.create.selectOrgType, userIsAuthorised, selectOrgTypeController.get) // All service transactions app.get(allServiceTransactions.index, userIsAuthorised, allTransactionsController.getController) @@ -515,7 +514,7 @@ module.exports.bind = function (app) { return } - res.redirect(serviceSwitcher.index) + res.redirect(services.index) return } logger.info('Page not found', { diff --git a/src/services/auth.service.js b/src/services/auth.service.js index 86474cc7c5..b33a6e63e1 100644 --- a/src/services/auth.service.js +++ b/src/services/auth.service.js @@ -125,8 +125,8 @@ function addUserFieldsToLogContext (req, res, next) { } function initialise (app) { - app.use(passport.initialize()) - app.use(passport.session()) + app.use(passport.initialize({})) + app.use(passport.session({})) passport.use('local', new LocalStrategy({ usernameField: 'username', passReqToCallback: true }, localStrategyAuth)) passport.use('local2Fa', new CustomStrategy(localStrategy2Fa)) passport.use('localStrategyLoginDirectAfterRegistration', new CustomStrategy(localStrategyLoginDirectAfterRegistration)) @@ -147,5 +147,5 @@ function deserializeUser (req, externalId, done) { } function serializeUser (user, done) { - done(null, user && user.externalId) + done(null, user?.externalId) } diff --git a/src/services/gateway-accounts.service.js b/src/services/gateway-accounts.service.js index 0b42e57ec9..af29d5f7a8 100644 --- a/src/services/gateway-accounts.service.js +++ b/src/services/gateway-accounts.service.js @@ -1,6 +1,20 @@ const { ConnectorClient } = require('@services/clients/connector.client') +const GatewayAccount = require('@models/GatewayAccount.class') const connectorClient = new ConnectorClient(process.env.CONNECTOR_URL) +async function getGatewayAccountsByIds (gatewayAccountIds) { + const gatewayAccounts = await connectorClient.getAccounts({ + gatewayAccountIds + }) + + return gatewayAccounts.accounts.reduce((acc, gatewayAccount) => { + const account = new GatewayAccount(gatewayAccount) + acc[account.id] = account + return acc + }, {}) +} + module.exports = { + getGatewayAccountsByIds, postSwitchPSP: connectorClient.postSwitchPSPByServiceExternalIdAndAccountType.bind(connectorClient) } diff --git a/src/services/service.service.js b/src/services/service.service.js index dc6531efdb..0c808c5f79 100644 --- a/src/services/service.service.js +++ b/src/services/service.service.js @@ -5,22 +5,12 @@ const lodash = require('lodash') const logger = require('../utils/logger')(__filename) const getAdminUsersClient = require('./clients/adminusers.client') const { ConnectorClient } = require('./clients/connector.client') -const CardGatewayAccount = require('@models/GatewayAccount.class') -const Service = require('../models/Service.class') +const Service = require('@models/Service.class') const connectorClient = new ConnectorClient(process.env.CONNECTOR_URL) const adminUsersClient = getAdminUsersClient() -const { DEFAULT_SERVICE_NAME } = require('../utils/constants') +const { DEFAULT_SERVICE_NAME } = require('@utils/constants') const { CREATED } = require('@models/constants/psp-test-account-stage') -async function getGatewayAccounts (gatewayAccountIds) { - const cardGatewayAccounts = await connectorClient.getAccounts({ - gatewayAccountIds - }) - - return cardGatewayAccounts.accounts - .map(gatewayAccount => new CardGatewayAccount(gatewayAccount).toMinimalJson()) -} - async function updateServiceName (serviceExternalId, serviceName, serviceNameCy) { if (!serviceExternalId) { return Promise.reject(new Error('argument: \'serviceExternalId\' cannot be undefined')) @@ -96,7 +86,6 @@ function addGovUkAgreementEmailAddress (serviceExternalId, userExternalId) { } module.exports = { - getGatewayAccounts, updateService, updateServiceName, createService, diff --git a/src/services/service.service.test.js b/src/services/service.service.test.js index 58971974a1..0ef7b5a908 100644 --- a/src/services/service.service.test.js +++ b/src/services/service.service.test.js @@ -1,60 +1,16 @@ -'use strict' - -const _ = require('lodash') const proxyquire = require('proxyquire') const chai = require('chai') const chaiAsPromised = require('chai-as-promised') chai.use(chaiAsPromised) const sinon = require('sinon') -const gatewayAccountFixtures = require('../../test/fixtures/gateway-account.fixtures') - const expect = chai.expect -const gatewayAccountId1 = '1' -const gatewayAccountId2 = '2' -const nonExistentId = '3' - let connectorClientStub let adminusersClientStub let serviceService -const getGatewayAccounts = function () { - return { - getAccounts: function (obj) { - return new Promise(function (resolve) { - resolve({ - accounts: obj.gatewayAccountIds.filter(fil => fil !== nonExistentId).map(iter => gatewayAccountFixtures.validGatewayAccountResponse({ - gateway_account_id: iter, - service_name: `account ${iter}`, - type: _.sample(['test', 'live']) - })) - }) - }) - } - } -} - describe('service service', function () { - describe('when getting gateway accounts', function () { - it('should return card gateway accounts only for the valid ids', async function () { - connectorClientStub = { - ConnectorClient: getGatewayAccounts - } - - serviceService = proxyquire('./service.service', - { - '../services/clients/connector.client': connectorClientStub - }) - - const gatewayAccounts = await serviceService.getGatewayAccounts([gatewayAccountId1, gatewayAccountId2, nonExistentId]) - - expect(gatewayAccounts).to.have.lengthOf(2) - expect(gatewayAccounts.map(accountObj => accountObj.id || accountObj.gateway_account_external_id)) - .to.have.all.members(['1', '2']) - }) - }) - describe('when editing service name', function () { it('should update service name', async function () { const externalServiceId = 'sdfjksdnfkjn' diff --git a/src/simplified-account-routes.js b/src/simplified-account-routes.js index 96297663fc..010927c05f 100644 --- a/src/simplified-account-routes.js +++ b/src/simplified-account-routes.js @@ -23,9 +23,7 @@ const simplifiedAccount = new Router({ mergeParams: true }) simplifiedAccount.use(simplifiedAccountOptIn, simplifiedAccountStrategy, userIsAuthorised) // settings index -simplifiedAccount.get(paths.simplifiedAccount.settings.index, defaultViewDecider, (req, res) => { - req.selectedController(req, res) -}) +simplifiedAccount.get(paths.simplifiedAccount.settings.index, defaultViewDecider) // service name simplifiedAccount.get(paths.simplifiedAccount.settings.serviceName.index, enforceLiveAccountOnly, permission('service-name:update'), serviceSettingsController.serviceName.get) diff --git a/src/utils/simplified-account/services/my-services/service-presentation-utils.js b/src/utils/simplified-account/services/my-services/service-presentation-utils.js new file mode 100644 index 0000000000..9724f41ae6 --- /dev/null +++ b/src/utils/simplified-account/services/my-services/service-presentation-utils.js @@ -0,0 +1,54 @@ +const formatAccountPathsFor = require('@utils/format-account-paths-for') +const paths = require('@root/paths') +const { formatSimplifiedAccountPathsFor } = require('@utils/simplified-account/format') +const formatServicePathsFor = require('@utils/format-service-paths-for') +const { WORLDPAY } = require('@models/constants/payment-providers') + +/** + * @param {GatewayAccount} account + * @param {GOVUKPayService} service + * @param {boolean} userIsDegatewayed + * @returns {{dashboardLink: String, editServiceNameLink: String, manageTeamMembersLink: String, organisationDetailsLink: String}} + */ +const accountLinksGenerator = (account, service, userIsDegatewayed) => { + return { + dashboardLink: formatAccountPathsFor(paths.account.dashboard.index, account.externalId), + editServiceNameLink: userIsDegatewayed + ? formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.serviceName.index, service.externalId, account.type) + : formatServicePathsFor(paths.service.editServiceName.index, service.externalId), + manageTeamMembersLink: userIsDegatewayed + ? formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.teamMembers.index, service.externalId, account.type) + : formatServicePathsFor(paths.service.teamMembers.index, service.externalId), + organisationDetailsLink: userIsDegatewayed + ? formatSimplifiedAccountPathsFor(paths.simplifiedAccount.settings.organisationDetails.index, service.externalId, account.type) + : formatServicePathsFor(paths.service.organisationDetails.index, service.externalId) + } +} + +/** + * Sorts objects by presence of 'live' gateway accounts, then alphabetically by name. + * @param {{gatewayAccounts: Array<{type: string}>, name: string}} a - First object to compare + * @param {{gatewayAccounts: Array<{type: string}>, name: string}} b - Second object to compare + * @returns {number} Sorting order: objects with live accounts first, then alphabetical + */ +const sortByLiveThenName = (a, b) => { + const aHasLive = a.gatewayAccounts.some(account => account.type === 'live') + const bHasLive = b.gatewayAccounts.some(account => account.type === 'live') + if (aHasLive !== bHasLive) return bHasLive ? 1 : -1 + return a.name.toLowerCase().localeCompare(b.name.toLowerCase()) +} + +/** + * @param {Array<{type: string, paymentProvider: string}>} gatewayAccounts + * @returns {boolean} + */ +const isWorldpayTestService = (gatewayAccounts) => { + return gatewayAccounts.length === 1 && gatewayAccounts[0].type === 'test' && + gatewayAccounts[0].paymentProvider === WORLDPAY +} + +module.exports = { + accountLinksGenerator, + sortByLiveThenName, + isWorldpayTestService +} diff --git a/src/views/includes/phase-banner.njk b/src/views/includes/phase-banner.njk index c3559cd51a..7be5b87800 100644 --- a/src/views/includes/phase-banner.njk +++ b/src/views/includes/phase-banner.njk @@ -1,6 +1,6 @@ {% from "../macro/breadcrumbs.njk" import breadcrumbs %} {% set accountLabelWithTag %} - {{currentService.name}} + {{ currentService.name }} {% if currentGatewayAccount %} {% set paymentProvider = currentGatewayAccount.payment_provider or currentGatewayAccount.paymentProvider %} {% set normalisedPaymentProvider = paymentProvider.toLowerCase() %} @@ -32,7 +32,7 @@ - information needed {% endif %} {% if isAdminUser and (currentGatewayAccount.provider_switch_enabled or currentGatewayAccount.providerSwitchEnabled) %} - - switch psp + - switch psp {% endif %} {% endset %} {{ tagText }} @@ -40,31 +40,30 @@ {% endset %} {% set breadcrumbItems = [ - { text: "My services", href: routes.serviceSwitcher.index } + { text: "My services", href: routes.services.index } ] %} {% if not hideServiceHeader and currentService.name %} -{% set breadcrumbItems = (breadcrumbItems.push({ - html: accountLabelWithTag -}), breadcrumbItems) %} + {% set breadcrumbItems = (breadcrumbItems.push({ + html: accountLabelWithTag + }), breadcrumbItems) %} {% endif %} {{ breadcrumbs(breadcrumbItems) }} {% if not hideServiceNav and not hideServiceHeader %} -
- +
{% endif %} diff --git a/src/views/includes/propositional-navigation.njk b/src/views/includes/propositional-navigation.njk index e740cad6fc..2040f5ac2f 100644 --- a/src/views/includes/propositional-navigation.njk +++ b/src/views/includes/propositional-navigation.njk @@ -3,7 +3,7 @@ Menu