From f246eb715de29ab73a96440e6501ab0c08866712 Mon Sep 17 00:00:00 2001 From: Stephen Daly Date: Mon, 3 Jul 2023 16:53:04 +0100 Subject: [PATCH] PP-11244 Prepare for changing find charge by token response Want to change the response schema for the connector GET `/v1/frontend/tokens/{chargeTokenId}` endpoint to use the FrontendChargeResponse schema for the embedded charge object in the response, rather than ChargeEntity. Support both versions of the response temporarily by checking which fields are returned in the response. This will be updated again to remove support for the old response schema after the connector change is made. Alter the Pact test for this endpoint to not expect the response to contain the `charge.externalId` field to allow the connector to be updated. --- app/controllers/secure.controller.js | 28 ++++++++++----- test/controllers/secure.controller.test.js | 36 +++++++++++++------ .../integration/card/awaiting-auth.cy.test.js | 1 - .../billing-address-collection.cy.test.js | 3 -- .../integration/card/email-typo.cy.test.js | 1 - .../integration/card/payment.cy.test.js | 1 - test/fixtures/payment.fixtures.js | 4 +-- test/unit/charge.controller.test.js | 2 +- .../connector-client-token.pact.test.js | 2 +- 9 files changed, 48 insertions(+), 30 deletions(-) diff --git a/app/controllers/secure.controller.js b/app/controllers/secure.controller.js index 4abfea6c8..5e78154d2 100644 --- a/app/controllers/secure.controller.js +++ b/app/controllers/secure.controller.js @@ -25,21 +25,33 @@ const paths = require('../paths') exports.new = async function (req, res) { const chargeTokenId = req.params.chargeTokenId || req.body.chargeTokenId const correlationId = req.headers[CORRELATION_HEADER] || '' - var chargeId try { - const chargeData = await Charge(correlationId).findByToken(chargeTokenId, getLoggingFields(req)) - chargeId = chargeData.charge.externalId + const tokenResponse = await Charge(correlationId).findByToken(chargeTokenId, getLoggingFields(req)) + let chargeId, gatewayAccountId, gatewayAccountType + + // Temporarily support both the new and old response schemas from connector + if (tokenResponse.charge.externalId) { + chargeId = tokenResponse.charge.externalId + gatewayAccountId = tokenResponse.charge.gatewayAccount.gateway_account_id + gatewayAccountType = tokenResponse.charge.gatewayAccount.type + } else { + chargeId = tokenResponse.charge.charge_id + gatewayAccountId = tokenResponse.charge.gateway_account.gateway_account_id + gatewayAccountType = tokenResponse.charge.gateway_account.type + } + + const chargeStatus = tokenResponse.charge.status setLoggingField(req, PAYMENT_EXTERNAL_ID, chargeId) - setLoggingField(req, GATEWAY_ACCOUNT_ID, chargeData.charge.gatewayAccount.gateway_account_id) - setLoggingField(req, GATEWAY_ACCOUNT_TYPE, chargeData.charge.gatewayAccount.type) + setLoggingField(req, GATEWAY_ACCOUNT_ID, gatewayAccountId) + setLoggingField(req, GATEWAY_ACCOUNT_TYPE, gatewayAccountType) - if (chargeData.used === true) { + if (tokenResponse.used === true) { if (!getSessionVariable(req, createChargeIdSessionKey(chargeId))) { throw new Error('UNAUTHORISED') } logger.info('Payment token being reused', getLoggingFields(req)) - const stateName = chargeData.charge.status.toUpperCase().replace(/\s/g, '_') + const stateName = chargeStatus.toUpperCase().replace(/\s/g, '_') responseRouter.response(req, res, stateName, { chargeId: chargeId, returnUrl: paths.generateRoute('card.return', { chargeId }) @@ -48,7 +60,7 @@ exports.new = async function (req, res) { logger.info('Payment token used for the first time', getLoggingFields(req)) await Token.markTokenAsUsed(chargeTokenId, correlationId, getLoggingFields(req)) setSessionVariable(req, createChargeIdSessionKey(chargeId), { csrfSecret: csrf().secretSync() }) - res.redirect(303, generateRoute(resolveActionName(chargeData.charge.status, 'get'), { chargeId })) + res.redirect(303, generateRoute(resolveActionName(chargeStatus, 'get'), { chargeId })) } } catch (err) { if (err.message === 'UNAUTHORISED') { diff --git a/test/controllers/secure.controller.test.js b/test/controllers/secure.controller.test.js index 395e009cd..048f1054e 100644 --- a/test/controllers/secure.controller.test.js +++ b/test/controllers/secure.controller.test.js @@ -1,17 +1,17 @@ 'use strict' -// NPM dependencies const proxyquire = require('proxyquire') const sinon = require('sinon') const expect = require('chai').expect -// Local dependencies const paths = require('../../app/paths.js') const withAnalyticsError = require('../../app/utils/analytics').withAnalyticsError +const { validChargeDetails } = require('../fixtures/payment.fixtures') -// configure require('../test-helpers/html-assertions.js') +const chargeId = 'ch_dh6kpbb4k82oiibbe4b9haujjk' + const mockCharge = (function () { const mock = function (withSuccess, chargeObject) { return function () { @@ -134,13 +134,27 @@ describe('secure controller', function () { }) }) - describe('then mark as used successfully', function () { - it('should store the service name into the session and redirect', async function () { - await requireSecureController(mockCharge.withSuccess(chargeObject), mockToken.withSuccess(), responseRouter).new(request, response) - expect(response.redirect.calledWith(303, paths.generateRoute('card.new', { chargeId: chargeObject.charge.externalId }))).to.be.true // eslint-disable-line - expect(request.frontend_state).to.have.all.keys('ch_dh6kpbb4k82oiibbe4b9haujjk') - expect(request.frontend_state.ch_dh6kpbb4k82oiibbe4b9haujjk).to.eql({ - csrfSecret: 'foo' // pragma: allowlist secret + describe('when the token is used successfully', function () { + describe('old connector token response', () => { + it('should store the service name into the session and redirect', async function () { + await requireSecureController(mockCharge.withSuccess(chargeObject), mockToken.withSuccess(), responseRouter).new(request, response) + expect(response.redirect.calledWith(303, paths.generateRoute('card.new', {chargeId: chargeObject.charge.externalId}))).to.be.true // eslint-disable-line + expect(request.frontend_state).to.have.all.keys(chargeId) + expect(request.frontend_state.ch_dh6kpbb4k82oiibbe4b9haujjk).to.eql({ + csrfSecret: 'foo' // pragma: allowlist secret + }) + }) + }) + + describe('new connector token response', () => { + it('should store the service name into the session and redirect', async function () { + const newChargeObject = validChargeDetails({ chargeId }) + await requireSecureController(mockCharge.withSuccess(newChargeObject), mockToken.withSuccess(), responseRouter).new(request, response) + expect(response.redirect.calledWith(303, paths.generateRoute('card.new', {chargeId: chargeObject.charge.externalId}))).to.be.true // eslint-disable-line + expect(request.frontend_state).to.have.all.keys(chargeId) + expect(request.frontend_state.ch_dh6kpbb4k82oiibbe4b9haujjk).to.eql({ + csrfSecret: 'foo' // pragma: allowlist secret + }) }) }) }) @@ -253,7 +267,7 @@ describe('secure controller', function () { returnUrl: '/return/dh6kpbb4k82oiibbe4b9haujjk' } expect(responseRouter.response.calledWith(requestWithFrontendStateCookie, response, 'AUTHORISATION_SUCCESS', opts)).to.be.true // eslint-disable-line - expect(requestWithFrontendStateCookie.frontend_state).to.have.all.keys('ch_dh6kpbb4k82oiibbe4b9haujjk') + expect(requestWithFrontendStateCookie.frontend_state).to.have.all.keys(chargeId) expect(requestWithFrontendStateCookie.frontend_state.ch_dh6kpbb4k82oiibbe4b9haujjk).to.eql({ csrfSecret: 'foo' // pragma: allowlist secret }) diff --git a/test/cypress/integration/card/awaiting-auth.cy.test.js b/test/cypress/integration/card/awaiting-auth.cy.test.js index 4ddfc1126..cf65683b4 100644 --- a/test/cypress/integration/card/awaiting-auth.cy.test.js +++ b/test/cypress/integration/card/awaiting-auth.cy.test.js @@ -49,7 +49,6 @@ describe('Awaiting auth', () => { cy.get('#address-postcode').type(validPayment.postcode) cy.get('#email').type(validPayment.email) - const paymentDetails = { cardNumber: validPayment.cardNumber, expiryMonth: validPayment.expiryMonth, diff --git a/test/cypress/integration/card/billing-address-collection.cy.test.js b/test/cypress/integration/card/billing-address-collection.cy.test.js index 87d329653..96e3beeaf 100644 --- a/test/cypress/integration/card/billing-address-collection.cy.test.js +++ b/test/cypress/integration/card/billing-address-collection.cy.test.js @@ -28,7 +28,6 @@ describe('Billing address collection', () => { tokenId, chargeId, 'en', gatewayAccountId, serviceOpts, {}, {}, chargeOpts) it('Should show billing address section and populate with default billing address', () => { - cy.task('setupStubs', createPaymentChargeStubs) cy.visit(`/secure/${tokenId}`) @@ -82,7 +81,6 @@ describe('Billing address collection', () => { tokenId, chargeId, 'en', gatewayAccountId, serviceOpts, {}, {}, chargeOpts) it('Should not show the billing address section and allow valid card submission', () => { - cy.task('setupStubs', createPaymentChargeStubs) cy.visit(`/secure/${tokenId}`) @@ -95,7 +93,6 @@ describe('Billing address collection', () => { cy.get('#address-city').should('not.exist') cy.get('#address-postcode').should('not.exist') - cy.task('clearStubs') const checkCardDetailsStubs = cardPaymentStubs.checkCardDetailsStubs(chargeId) diff --git a/test/cypress/integration/card/email-typo.cy.test.js b/test/cypress/integration/card/email-typo.cy.test.js index f57e2ad15..e72a2cecc 100644 --- a/test/cypress/integration/card/email-typo.cy.test.js +++ b/test/cypress/integration/card/email-typo.cy.test.js @@ -127,7 +127,6 @@ describe('Standard card payment flow', () => { cy.get('#cardholder-name').should(($td) => expect($td).to.contain(validPayment.name)) cy.get('#email').should(($td) => expect($td).to.contain(validPayment.email)) - cy.task('clearStubs') cy.task('setupStubs', submitPaymentCaptureStubs) diff --git a/test/cypress/integration/card/payment.cy.test.js b/test/cypress/integration/card/payment.cy.test.js index 9f85c18f8..2c7a4f932 100644 --- a/test/cypress/integration/card/payment.cy.test.js +++ b/test/cypress/integration/card/payment.cy.test.js @@ -147,7 +147,6 @@ describe('Standard card payment flow', () => { cy.get('#address-postcode').type(validPayment.postcode) cy.get('#email').type(validPayment.email) - const lastFourCardDigits = validPayment.cardNumber.toString().slice(-4) cy.task('clearStubs') diff --git a/test/fixtures/payment.fixtures.js b/test/fixtures/payment.fixtures.js index 50ea1189d..b937143f7 100644 --- a/test/fixtures/payment.fixtures.js +++ b/test/fixtures/payment.fixtures.js @@ -258,9 +258,7 @@ const fixtures = { tokenResponse: (opts = {}) => { return { used: opts.used, - charge: { - externalId: opts.chargeExternalId - } + charge: {} } }, diff --git a/test/unit/charge.controller.test.js b/test/unit/charge.controller.test.js index 2e8618d0a..8e8549a3d 100644 --- a/test/unit/charge.controller.test.js +++ b/test/unit/charge.controller.test.js @@ -6,7 +6,7 @@ describe('walletEnabled', () => { { walletToggle: true, gatewayAccountId: '1', payTestGatewayAccounts: ['5', '10', '15', '20'], expected: true }, { walletToggle: false, gatewayAccountId: '1', payTestGatewayAccounts: ['5', '10', '15', '20'], expected: false }, { walletToggle: false, gatewayAccountId: '15', payTestGatewayAccounts: ['5', '10', '15', '20'], expected: true }, - { walletToggle: false, gatewayAccountId: '15', payTestGatewayAccounts: [], expected: false }, + { walletToggle: false, gatewayAccountId: '15', payTestGatewayAccounts: [], expected: false } ] testData.forEach(({ walletToggle, gatewayAccountId, payTestGatewayAccounts, expected }) => { diff --git a/test/unit/clients/connector-client-token.pact.test.js b/test/unit/clients/connector-client-token.pact.test.js index 76a2c7c07..d504f7081 100644 --- a/test/unit/clients/connector-client-token.pact.test.js +++ b/test/unit/clients/connector-client-token.pact.test.js @@ -50,7 +50,7 @@ describe('connector client - tokens', function () { tokenId: TOKEN }) expect(res.body.used).to.be.equal(false) - expect(res.body.charge.externalId).to.be.equal('chargeExternalId') + expect(res.body.charge).to.exist // eslint-disable-line }) }) })