PP-6079 Don't collect billing address for moto payments

Currently, we don't show fields to collect the billing address on the card details page if billing address collection is disabled for the service. Now, we also won't show these fields if a payment is a MOTO payment even if billing address collection is enabled for the service. Determine whether we should collect the address in the resolve_service middleware and add a field to res.locals so that we can access it in all templates and controllers that need this value. Add Cypress tests to test this and remove tests from charge_ui_tests which are now better covered by Cypress tests.
alphagov · Feb 4, 2020 · 1c1fecf · 1c1fecf
1 parent 615c562
commit 1c1fecf
Show file tree

Hide file tree

Showing 10 changed files with 252 additions and 104 deletions.
diff --git a/app/controllers/charge_controller.js b/app/controllers/charge_controller.js
@@ -122,7 +122,7 @@ module.exports = {
     const cardModel = Card(req.chargeData.gateway_account.card_types, req.chargeData.gateway_account.block_prepaid_cards, req.headers[CORRELATION_HEADER])
     const chargeOptions = {
       email_collection_mode: charge.gatewayAccount.emailCollectionMode,
-      collect_billing_address: res.locals.service.collectBillingAddress
+      collect_billing_address: res.locals.collectBillingAddress
     }
     const validator = chargeValidator(i18n.__('fieldErrors'), logger, cardModel, chargeOptions, getLoggingFields(req))
 
@@ -185,7 +185,7 @@ module.exports = {
 
     const correlationId = req.headers[CORRELATION_HEADER] || ''
     const payload = normalise.apiPayload(req, card)
-    if (res.locals.service.collectBillingAddress === false) {
+    if (res.locals.collectBillingAddress === false) {
       delete payload.address
     }
     try {

diff --git a/app/middleware/resolve_service.js b/app/middleware/resolve_service.js
@@ -25,6 +25,7 @@ module.exports = function resolveServiceMiddleware (req, res, next) {
   const cachedService = serviceCache.get(gatewayAccountId)
   if (cachedService) {
     res.locals.service = cachedService
+    res.locals.collectBillingAddress = res.locals.service.collectBillingAddress && !req.chargeData.moto
     next()
   } else {
     // @FIXME(sfount) tests shouldn't rely on middleware returning a value if
@@ -34,6 +35,7 @@ module.exports = function resolveServiceMiddleware (req, res, next) {
       .then(service => {
         serviceCache.put(gatewayAccountId, service, SERVICE_CACHE_MAX_AGE)
         res.locals.service = service
+        res.locals.collectBillingAddress = res.locals.service.collectBillingAddress && !req.chargeData.moto
         next()
       })
       .catch((err) => {

diff --git a/app/views/charge.njk b/app/views/charge.njk
@@ -272,7 +272,7 @@
           </span>
           <img src="/images/amex-security-code.png" class="amex-cvc hidden" alt="{{ __p("cardDetails.amexcvcTip") }}"/>
         </div>
-        {% if service.collectBillingAddress %}
+        {% if collectBillingAddress %}
           <div class="govuk-form-group govuk-!-width-three-quarters govuk-!-padding-top-4 govuk-!-margin-top-8 pay-!-border-top {% if highlightErrorFields.addressCountry %} govuk-form-group--error{% endif %}" data-validation="addressCountry">
             <legend for="address-country" class="govuk-!-margin-bottom-6">
               <h2 class="govuk-heading-m govuk-!-margin-bottom-1">{{ __p("cardDetails.billingAddress") }}</h2>

diff --git a/app/views/includes/scripts.njk b/app/views/includes/scripts.njk
@@ -30,7 +30,7 @@
   };
   window.Charge = {
     email_collection_mode: '{{ gatewayAccount.emailCollectionMode | safe }}',
-    collect_billing_address: {{ "true" if service.collectBillingAddress else "false" }},
+    collect_billing_address: {{ "true" if collectBillingAddress else "false" }},
     worldpay_3ds_flex_ddc_jwt: '{{ worldpay3dsFlexDdcJwt }}',
     worldpay_3ds_flex_ddc_url: '{{ worldpay3dsFlexDdcUrl }}'
   }
@@ -41,7 +41,7 @@
     if (mainWrap.classList.contains('charge-new')) {
       window.payScripts.helpers.setGlobalChargeId();
       showCardType().init();
-      {% if service.collectBillingAddress %}
+      {% if collectBillingAddress %}
         window.payScripts.helpers.initialiseAddressCountryAutocomplete();
       {% endif %}
       window.payScripts.inputConfirm.init();

diff --git a/test/charge_ui_tests.js b/test/charge_ui_tests.js
@@ -41,7 +41,7 @@ const generateConfirmViewTemplateData = (templateData = {}) => {
 describe('The charge view', function () {
   it('should render the amount', function () {
     const templateData = {
-      'amount': '50.00'
+      amount: '50.00'
     }
 
     const body = renderTemplate('charge', templateData)
@@ -51,7 +51,7 @@ describe('The charge view', function () {
   it('should have a submit form.', function () {
     const postAction = '/post_card_path'
     const templateData = {
-      'post_card_action': postAction
+      post_card_action: postAction
     }
 
     const body = renderTemplate('charge', templateData)
@@ -72,9 +72,7 @@ describe('The charge view', function () {
   it('should show all input fields.', function () {
     const body = renderTemplate('charge', {
       id: '1234',
-      service: {
-        collectBillingAddress: true
-      }
+      collectBillingAddress: true
     })
     body.should.containInputWithIdAndName('csrf', 'csrfToken', 'hidden')
     body.should.containInputWithIdAndName('card-no', 'cardNo', 'text').withAttribute('maxlength', '26').withLabel('card-no-lbl', 'Card number')
@@ -91,30 +89,8 @@ describe('The charge view', function () {
     body.should.not.containSelector('.custom-branding-image')
   })
 
-  it('should not show billing address for services not wanting to capture it', function () {
-    const body = renderTemplate('charge', {
-      id: '1234',
-      service: {
-        collectBillingAddress: false
-      }
-    })
-    body.should.containInputWithIdAndName('csrf', 'csrfToken', 'hidden')
-    body.should.containInputWithIdAndName('card-no', 'cardNo', 'text').withAttribute('maxlength', '26').withLabel('card-no-lbl', 'Card number')
-    body.should.containInputWithIdAndName('cvc', 'cvc', 'text').withLabel('cvc-lbl', 'Card security code')
-    body.should.containInputWithIdAndName('expiry-month', 'expiryMonth', 'text')
-    body.should.containInputWithIdAndName('expiry-year', 'expiryYear', 'text')
-    body.should.containInputWithIdAndName('cardholder-name', 'cardholderName', 'text').withAttribute('maxlength', '200').withLabel('cardholder-name-lbl', 'Name on card')
-    body.should.not.containSelector('#address-country')
-    body.should.not.containSelector('#address-line-1')
-    body.should.not.containSelector('#address-line-2')
-    body.should.not.containSelector('#address-city')
-    body.should.not.containSelector('#address-postcode')
-    body.should.containInputWithIdAndName('charge-id', 'chargeId', 'hidden').withAttribute('value', '1234')
-    body.should.not.containSelector('.custom-branding-image')
-  })
-
   it('should display custom branding', () => {
-    const templateData = lodash.merge('charge', { 'id': '1234' }, customBrandingData)
+    const templateData = lodash.merge('charge', { id: '1234' }, customBrandingData)
     const body = renderTemplate('charge', templateData)
     body.should.containSelector('.custom-branding-image')
 
@@ -133,9 +109,7 @@ describe('The charge view', function () {
       addressLine2: 'blah blah',
       addressCity: 'Leicester City',
       addressPostcode: 'CT16 1FB',
-      service: {
-        collectBillingAddress: true
-      }
+      collectBillingAddress: true
     }
     const body = renderTemplate('charge', responseData)
 
@@ -164,28 +138,6 @@ describe('The confirm view', function () {
     body.should.containSelector('#address').withText('1 street lane, avenue city, AB1 3DF')
   })
 
-  it('should not show billing address for services not wanting to capture it', function () {
-    const body = renderTemplate('confirm', generateConfirmViewTemplateData({
-      service: {
-        collectBillingAddress: false
-      },
-      charge: {
-        cardDetails: {
-          billingAddress: null
-        }
-      }
-    }))
-    const $ = cheerio.load(body)
-    $('#payment-description').html().should.contain('Payment Description &amp; &lt;xss attack&gt; assessment')
-    body.should.containInputWithIdAndName('csrf', 'csrfToken', 'hidden')
-    body.should.containSelector('#card-number').withText('●●●●●●●●●●●●5100')
-    body.should.containSelector('#expiry-date').withText('11/99')
-    body.should.containSelector('#cardholder-name').withText('Francisco Blaya-Gonzalvez')
-    body.should.not.containSelector('#address').withText('1 street lane, avenue city, AB1 3DF')
-    body.should.containSelector('#payment-description').withText('Payment Description')
-    body.should.containSelector('#amount').withText('£10.00')
-  })
-
   it('should display custom branding', () => {
     const templateData = lodash.merge(successTemplateDataWithCollectBillingAddress, customBrandingData)
     const body = renderTemplate('confirm', templateData)
@@ -199,7 +151,7 @@ describe('The confirm view', function () {
   })
 
   it('should render a confirm button', function () {
-    const body = renderTemplate('confirm', { confirmPath: '/card_details/123/confirm', 'charge': { id: 1234, amount: 50 } })
+    const body = renderTemplate('confirm', { confirmPath: '/card_details/123/confirm', charge: { id: 1234, amount: 50 } })
     const $ = cheerio.load(body)
     body.should.containSelector('form#confirmation').withAttributes(
       {
@@ -213,7 +165,7 @@ describe('The confirm view', function () {
   it('should have a cancel form.', function () {
     const postAction = '/post_cancel_path'
     const templateData = {
-      'post_cancel_action': postAction
+      post_cancel_action: postAction
     }
 
     const body = renderTemplate('charge', templateData)

diff --git a/test/controllers/charge_controller_create_test.js b/test/controllers/charge_controller_create_test.js
@@ -15,6 +15,19 @@ const chargeId = '42mdrsshtsk4chpeoifhlgf4lk'
 const card = paymentFixtures.validCardDetails()
 const chargeData = paymentFixtures.validChargeDetails({ emailCollectionMode: 'OFF' }).getPlain()
 
+const paymentDetails = {
+  chargeId: chargeId,
+  cardNo: '4242424242424242',
+  expiryMonth: '01',
+  expiryYear: '20',
+  cardholderName: 'Joe Bloggs',
+  cvc: '111',
+  addressCountry: 'GB',
+  addressLine1: '1 Horse Guards',
+  addressCity: 'London',
+  addressPostcode: 'E1 8QS'
+}
+
 const mockedChargeValidationBackend = function () {
   const validation = {
     hasError: false
@@ -38,7 +51,6 @@ describe('POST /card_details/{chargeId} endpoint', function () {
   let response
   let chargeAuthStub
   let mockedConnectorClient
-  let paymentDetails
 
   beforeEach(() => {
     chargeAuthStub = sinon.stub().resolves(
@@ -56,28 +68,13 @@ describe('POST /card_details/{chargeId} endpoint', function () {
     response = {
       redirect: sinon.spy(),
       locals: {
-        service: {
-          collectBillingAddress: true
-        }
-
+        collectBillingAddress: true
       }
     }
-    paymentDetails = {
-      'chargeId': chargeId,
-      'cardNo': '4242424242424242',
-      'expiryMonth': '01',
-      'expiryYear': '20',
-      'cardholderName': 'Joe Bloggs',
-      'cvc': '111',
-      'addressCountry': 'GB',
-      'addressLine1': '1 Horse Guards',
-      'addressCity': 'London',
-      'addressPostcode': 'E1 8QS'
-    }
   })
 
   it('should send worldpay_3ds_flex_ddc_result to connector when the request includes a worldpay3dsFlexDdcResult parameter', async function () {
-    paymentDetails['worldpay3dsFlexDdcResult'] = 'a-worldpay-3ds-flex-ddc-result'
+    paymentDetails.worldpay3dsFlexDdcResult = 'a-worldpay-3ds-flex-ddc-result'
     const request = {
       chargeData: chargeData,
       body: paymentDetails,
@@ -112,8 +109,8 @@ describe('POST /card_details/{chargeId} endpoint', function () {
 
     expect(chargeAuthStub.calledWith(sinon.match( // eslint-disable-line
       {
-        'chargeId': chargeId,
-        'payload': payload
+        chargeId: chargeId,
+        payload: payload
       }
     ))).to.be.true // eslint-disable-line
   })
@@ -148,12 +145,51 @@ describe('POST /card_details/{chargeId} endpoint', function () {
 
     delete payload.accept_header
     delete payload.user_agent_header
-    delete payload.worldpay_3ds_flex_ddc_result
 
     expect(chargeAuthStub.calledWith(sinon.match( // eslint-disable-line
       {
-        'chargeId': chargeId,
-        'payload': payload
+        chargeId: chargeId,
+        payload: payload
+      }
+    ))).to.be.true // eslint-disable-line
+  })
+
+  it('should not include billing address in authorisation request when billing address collection disabled', async function () {
+    response.locals.collectBillingAddress = false
+
+    const request = {
+      chargeData: chargeData,
+      body: paymentDetails,
+      chargeId: chargeId,
+      header: sinon.spy(),
+      headers: {
+        'x-request-id': 'unique-id',
+        'x-forwarded-for': '127.0.0.1'
+      }
+    }
+    await requireChargeController(mockedConnectorClient).create(request, response)
+
+    const payload = paymentFixtures.validAuthorisationRequest({
+      cardNumber: paymentDetails.cardNo,
+      cvc: paymentDetails.cvc,
+      cardBrand: card.brand,
+      expiryDate: `${paymentDetails.expiryMonth}/${paymentDetails.expiryYear}`,
+      cardholderName: paymentDetails.cardholderName,
+      cardType: card.type,
+      corporateCard: card.corporate,
+      prepaid: card.prepaid,
+      noBillingAddress: true
+    }).getPlain()
+
+    delete payload.accept_header
+    delete payload.user_agent_header
+
+    console.log('PAYLOAD ' + JSON.stringify(payload))
+
+    expect(chargeAuthStub.calledWith(sinon.match( // eslint-disable-line
+      {
+        chargeId: chargeId,
+        payload: payload
       }
     ))).to.be.true // eslint-disable-line
   })