Skip to content

Commit

Permalink
PP-11605 Update billing address section
Browse files Browse the repository at this point in the history
- Making the billing address section more consistent like the guidelines from design system
  https://design-system.service.gov.uk/patterns/addresses/
  - Address line 2 now has its own label
  - This field is still optional.
  - Validation continues to only happen to this field when it contains a
    value.
- Remove code that would combine address line 1 & 2 together.  This code is no longer
  required because each line now has its own field label.
- Moved the country select to above the postcode field.
  - We can't make our page exacly like the design systems guidelines.  As we need
    the user to select the country before entering the post code.  So that we know
    which rules to apply to the post code validation.
  - Decision taken to move the country select just above the post code field
- Updated the translations
- There is any existing issue where the error summary does not display errors in the correct
  order.  A seperate story was created for this.
- Updated the tests and removed old tests that are not required.
  • Loading branch information
iqbalgds committed Dec 19, 2023
1 parent 8f5bf88 commit c5b897a
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 126 deletions.
6 changes: 3 additions & 3 deletions app/assets/javascripts/browsered/form-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var init = function () {
window.Card,
window.Charge
)
var required = chargeValidations.required
var formFields = chargeValidations.required.concat(chargeValidations.optional)

form.addEventListener('submit', function (e) {
checkFormSubmission(e)
Expand Down Expand Up @@ -289,7 +289,7 @@ var init = function () {

var allFields = function () {
var fields = {}
required.forEach(function (requiredField) {
formFields.forEach(function (requiredField) {
var getField = findInputByKey(requiredField)
if (getField) {
fields[requiredField] = getField
Expand All @@ -300,7 +300,7 @@ var init = function () {

var allFieldValues = function () {
var values = {}
required.forEach(function (requiredField) {
formFields.forEach(function (requiredField) {
var getField = findInputByKey(requiredField)
if (getField) {
values[requiredField] = getField.value.trim()
Expand Down
1 change: 0 additions & 1 deletion app/controllers/charge.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ module.exports = {
}
const validator = chargeValidator(i18n.__('fieldErrors'), logger, cardModel, chargeOptions, getLoggingFields(req))

normalise.addressLines(req.body)
normalise.whitespace(req.body)

const { worldpay3dsFlexDdcStatus } = req.body
Expand Down
9 changes: 1 addition & 8 deletions app/services/normalise-charge.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,7 @@ module.exports = (function () {
country: body.addressCountry
}
}
// body is passed by reference
const addressLines = function (body) {
if (!body.addressLine1 && body.addressLine2) {
body.addressLine1 = body.addressLine2
delete body.addressLine2
}
}

const whitespace = function (body) {
const toIgnore = [
'submitCardDetails',
Expand Down Expand Up @@ -195,7 +189,6 @@ module.exports = (function () {
return {
charge,
addressForApi,
addressLines,
whitespace,
creditCard,
expiryDate,
Expand Down
116 changes: 69 additions & 47 deletions app/views/charge.njk
Original file line number Diff line number Diff line change
Expand Up @@ -86,20 +86,20 @@
<div class="govuk-grid-row govuk-!-margin-bottom-9">
<main class="charge-new govuk-grid-column-two-thirds" id="main-content" role="main">
<div class="charge-new__content">
<div id="error-summary" data-cy="error-summary" class="govuk-error-summary {% if not hasError %}hidden{% endif %}" aria-labelledby="error-summary-title" role="alert" tabindex="-1" data-module="error-summary">
<h2 class="govuk-error-summary__title" id="error-summary-title">
{{ __p("fieldErrors.summary") }}
</h2>
<div class="govuk-error-summary__body">
<ul class="govuk-list govuk-error-summary__list">
{% for currentErrorField in errorFields %}
<li>
<a href="#{{ currentErrorField.cssKey }}-lbl" id="{{ currentErrorField.cssKey }}-error">{{ currentErrorField.value }}</a>
</li>
{% endfor %}
</ul>
<div id="error-summary" data-cy="error-summary" class="govuk-error-summary {% if not hasError %}hidden{% endif %}" aria-labelledby="error-summary-title" role="alert" tabindex="-1" data-module="error-summary">
<h2 class="govuk-error-summary__title" id="error-summary-title">
{{ __p("fieldErrors.summary") }}
</h2>
<div class="govuk-error-summary__body">
<ul class="govuk-list govuk-error-summary__list">
{% for currentErrorField in errorFields %}
<li>
<a href="#{{ currentErrorField.cssKey }}-lbl" id="{{ currentErrorField.cssKey }}-error">{{ currentErrorField.value }}</a>
</li>
{% endfor %}
</ul>
</div>
</div>
</div>
<div id="wallet-options">
{% if (allowApplePay or allowGooglePay) and not savePaymentInstrumentToAgreement %}
<div class="web-payment-button-section govuk-!-margin-bottom-7">
Expand Down Expand Up @@ -381,43 +381,15 @@
<h2 class="govuk-heading-m govuk-!-margin-bottom-1 non-web-payment-button-section">{{ __p("cardDetails.billingAddress") }}</h2>
</legend>

<div id="address-hint" class="govuk-hint govuk-!-margin-bottom-6">{{ __p("cardDetails.billingAddressHint") }}</div>

<div class="govuk-form-group {% if highlightErrorFields.addressCountry %} govuk-form-group--error{% endif %}" data-validation="addressCountry">
<label id="address-country-lbl" for="address-country" class="govuk-label">
<span
class="address-country-label"
data-label-replace="addressCountry"
data-original-label="{{ __p("cardDetails.country") }}">
{{ __p("cardDetails.country") }}
</span>
</label>

{% if highlightErrorFields.addressCountry %}
<p class="govuk-error-message" id="error-address-country">
{{ highlightErrorFields.addressCountry }}
</p>
{% endif %}
<div id="address-hint" class="govuk-body govuk-!-margin-bottom-6">{{ __p("cardDetails.billingAddressHint") }}</div>

<select name="addressCountry" class="govuk-select" id="address-country" autocomplete="billing country-name">
{% if not countryCode | length and service and service.defaultBillingAddressCountry %}
{% set countryCode = service.defaultBillingAddressCountry %}
{% endif %}
{% if not countryCode %}
<option disabled value="" selected>Pick a country or territory</option>
{% endif %}
{% for country in countries %}
<option value="{{ country[1].split(':')[1] }}"{% if country[1].split(':')[1] === countryCode %} selected="selected"{% endif %}>{{ country[0] }}</option>
{% endfor %}
</select>
</div>
<div class="govuk-form-group address{% if highlightErrorFields.addressLine1 %} govuk-form-group--error{% endif %}" data-validation="addressLine1">
<label id="address-line-1-lbl" for="address-line-1" class="govuk-label">
<span
class="address-line-1-label"
data-label-replace="addressLine1"
data-original-label="{{ __p("cardDetails.building") }}">
{{ __p("cardDetails.building") }}
data-original-label="{{ __p("cardDetails.addressLine1") }}">
{{ __p("cardDetails.addressLine1") }}
</span>
</label>

Expand All @@ -434,16 +406,36 @@
class="govuk-input"
value="{{ addressLine1 }}"
autocomplete="billing address-line1"/>
<input id="address-line-2"
</div>

<div class="govuk-form-group address{% if highlightErrorFields.addressLine2 %} govuk-form-group--error{% endif %}" data-validation="addressLine2">
<label id="address-line-2-lbl" for="address-line-2" class="govuk-label">
<span
class="address-line-2-label"
data-label-replace="addressLine2"
data-original-label="{{ __p("cardDetails.addressLine2") }}">
{{ __p("cardDetails.addressLine2") }}
</span>
</label>

{% if highlightErrorFields.addressLine2 %}
<p class="govuk-error-message" id="error-address-line-2">
{{ highlightErrorFields.addressLine2 }}
</p>
{% endif %}

<input id="address-line-2"
type="text"
name="addressLine2"
maxlength="100"
class="govuk-input govuk-!-margin-top-2"
class="govuk-input"
data-last-of-form-group
value="{{ addressLine2 }}"
aria-label="Enter address line 2"
autocomplete="billing address-line2"/>
autocomplete="billing address-line2"
data-cy="address-line-2"/>
</div>

<div class="govuk-form-group{% if highlightErrorFields.addressCity %} govuk-form-group--error{% endif %} govuk-!-width-three-quarters" data-validation="addressCity">
<label id="address-city-lbl" for="address-city" class="govuk-label">
<span
Expand All @@ -468,6 +460,36 @@
value="{{ addressCity }}"
autocomplete="billing address-level2"/>
</div>

<div class="govuk-form-group {% if highlightErrorFields.addressCountry %} govuk-form-group--error{% endif %}" data-validation="addressCountry">
<label id="address-country-lbl" for="address-country" class="govuk-label">
<span
class="address-country-label"
data-label-replace="addressCountry"
data-original-label="{{ __p("cardDetails.country") }}">
{{ __p("cardDetails.country") }}
</span>
</label>

{% if highlightErrorFields.addressCountry %}
<p class="govuk-error-message" id="error-address-country">
{{ highlightErrorFields.addressCountry }}
</p>
{% endif %}

<select name="addressCountry" class="govuk-select" id="address-country" autocomplete="billing country-name">
{% if not countryCode | length and service and service.defaultBillingAddressCountry %}
{% set countryCode = service.defaultBillingAddressCountry %}
{% endif %}
{% if not countryCode %}
<option disabled value="" selected>Pick a country or territory</option>
{% endif %}
{% for country in countries %}
<option value="{{ country[1].split(':')[1] }}"{% if country[1].split(':')[1] === countryCode %} selected="selected"{% endif %}>{{ country[0] }}</option>
{% endfor %}
</select>
</div>

<div class="govuk-form-group{% if highlightErrorFields.addressPostcode %} govuk-form-group--error{% endif %} {% if shouldShowEmail %} govuk-!-margin-bottom-0{% endif %}" data-validation="addressPostcode">
<label id="address-postcode-lbl" for="address-postcode" class="govuk-label">
<span
Expand Down
15 changes: 8 additions & 7 deletions locales/cy.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
"cardholderName": "Enw ar y cerdyn",
"billingAddress": "Cyfeiriad bilio",
"billingAddressHint": "Hwn yw’r cyfeiriad sy’n gysylltiedig â’r cerdyn",
"building": "Rhif neu enw’r adeilad a heol",
"addressLine1": "Llinell cyfeiriad 1",
"addressLine2": "Llinell cyfeiriad 2 (dewisol)",
"city": "Tref neu ddinas",
"postcode": "Cod post",
"country": "Gwlad neu diriogaeth",
Expand Down Expand Up @@ -143,14 +144,14 @@
"invalidYear": "Nid yw’r flwyddyn dod i ben hwn yn ddilys"
},
"addressLine1": {
"name": "enw/rhif yr adeilad a heol",
"message": "Rhowch gyfeiriad bilio dilys",
"containsTooManyDigits": "Rhowch gyfeiriad bilio dilys"
"name": "llinell cyfeiriad 1",
"message": "Rhowch linell cyfeiriad 1, fel arfer yr adeilad a'r stryd",
"containsTooManyDigits": "Rhowch linell cyfeiriad 1, fel arfer yr adeilad a'r stryd"
},
"addressLine2": {
"name": "llinell cyfeiriad ychwanegol",
"message": "Rhowch wybodaeth cyfeiriad dilys",
"containsTooManyDigits": "Rhowch wybodaeth cyfeiriad dilys"
"name": "llinell cyfeiriad 2",
"message": "Rhowch linell cyfeiriad 2",
"containsTooManyDigits": "Rhowch linell cyfeiriad 2"
},
"addressCity": {
"name": "dref/ddinas",
Expand Down
15 changes: 8 additions & 7 deletions locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
"cardholderName": "Name on card",
"billingAddress": "Billing address",
"billingAddressHint": "This is the address associated with the card",
"building": "Building number or name and street",
"addressLine1": "Address line 1",
"addressLine2": "Address line 2 (optional)",
"city": "Town or city",
"postcode": "Postcode",
"country": "Country or territory",
Expand Down Expand Up @@ -140,14 +141,14 @@
"invalidYear": "This expiry year is not a valid"
},
"addressLine1": {
"name": "building name/number and street",
"message": "Enter a valid billing address",
"containsTooManyDigits": "Enter a valid billing address"
"name": "address line 1",
"message": "Enter address line 1, typically the building and street",
"containsTooManyDigits": "Enter address line 1, typically the building and street"
},
"addressLine2": {
"name": "additional address line",
"message": "Enter valid address information",
"containsTooManyDigits": "Enter valid address information"
"name": "address line 2",
"message": "Enter address line 2",
"containsTooManyDigits": "Enter address line 2"
},
"addressCity": {
"name": "town/city",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ describe('Billing address collection', () => {
cy.log('should show the billing address section')

cy.get('h2').contains('Billing address').should('exist')
cy.get('#address-country-select').should('exist')
cy.get('#address-line-1').should('exist')
cy.get('#address-line-2').should('exist')
cy.get('#address-city').should('exist')
cy.get('#address-country-select').should('exist')
cy.get('#address-postcode').should('exist')

cy.log('Should populate default billing address from service')
Expand Down Expand Up @@ -87,10 +87,10 @@ describe('Billing address collection', () => {
cy.log('Should not show the billing address section')

cy.get('h2').contains('Billing address').should('not.exist')
cy.get('#address-country-select').should('not.exist')
cy.get('#address-line-1').should('not.exist')
cy.get('#address-line-2').should('not.exist')
cy.get('#address-city').should('not.exist')
cy.get('#address-country-select').should('not.exist')
cy.get('#address-postcode').should('not.exist')

cy.task('clearStubs')
Expand Down Expand Up @@ -139,10 +139,10 @@ describe('Billing address collection', () => {
cy.log('should not show the billing address section')

cy.get('h2').contains('Billing address').should('not.exist')
cy.get('#address-country-select').should('not.exist')
cy.get('#address-line-1').should('not.exist')
cy.get('#address-line-2').should('not.exist')
cy.get('#address-city').should('not.exist')
cy.get('#address-country-select').should('not.exist')
cy.get('#address-postcode').should('not.exist')

cy.log('should enter card details')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,37 @@ describe('Card details page validation', () => {
cy.get('.address-line-1-label').should('exist')
cy.get('.address-city-label').should('exist')
cy.get('.address-postcode-label').should('exist')

cy.log('No errors should be shown for the optional field address line 2')
cy.get('#address-line-2-error').should('not.exist')
cy.get('.address-line-2-label').should('exist')
})

it('Should perform validation of optional address line 2 correctly', () => {
cy.task('setupStubs', createPaymentChargeStubs)
cy.visit(`/secure/${tokenId}`)

cy.location('pathname').should('eq', `/card_details/${chargeId}`)
cy.window().its('chargeId').should('eq', `${chargeId}`)

cy.get('#card-details').submit()
cy.location('pathname').should('eq', `/card_details/${chargeId}`)

cy.log('No errors should be shown for the optional field address line 2 as the field is not being used')
cy.get('#address-line-2-error').should('not.exist')
cy.get('.address-line-2-label').should('exist')

cy.log('Should error when address line 2 contains an invalid data')
cy.get('[data-cy=address-line-2]').type('1111111111111111')
cy.get('#card-details').submit()

cy.get('#address-line-2-error').should('exist')

cy.log('Should not sure an error when the optional address line 2 is cleared')
cy.get('[data-cy=address-line-2]').clear()
cy.get('#card-details').submit()

cy.get('#address-line-2-error').should('not.exist')
})

it('Should perform inline validation of the expiry date correctly', () => {
Expand Down
4 changes: 2 additions & 2 deletions test/integration/card-details-errors.ft.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ describe('chargeTests', function () {
expect($('#error-cardholder-name').text()).to.contains('Enter the name as it appears on the card')
expect($('#cvc-error').text()).to.contains(errorMessages.cvc)
expect($('#error-cvc').text()).to.contains(errorMessages.cvc)
expect($('#address-line-1-error').text()).to.contains('Enter a valid building name/number and street')
expect($('#error-address-line-1').text()).to.contains('Enter a valid billing address')
expect($('#address-line-1-error').text()).to.contains('Enter a valid address line 1')
expect($('#error-address-line-1').text()).to.contains('Enter address line 1, typically the building and street')
expect($('#address-city-error').text()).to.contains(errorMessages.city)
expect($('#error-address-city').text()).to.contains(errorMessages.city)
expect($('#address-postcode-error').text()).to.contains(errorMessages.postcode)
Expand Down
Loading

0 comments on commit c5b897a

Please sign in to comment.