Skip to content

Commit

Permalink
PP-10104 payment page validation bugs (#3893)
Browse files Browse the repository at this point in the history
Payment page validation UI bugs
  • Loading branch information
hjvoid authored and iqbalgds committed Sep 16, 2024
1 parent 21cdf9e commit 9ffb54d
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 28 deletions.
17 changes: 9 additions & 8 deletions app/assets/javascripts/browsered/form-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,16 @@ var init = function () {
var addHighlightError = function (addType, error) {
var listElement = document.createElement('li')
var errorAnchor = document.createElement('a')
errorAnchor.setAttribute('href', '#' + error.cssKey + '-lbl')
errorAnchor.setAttribute('href', '#' + error.cssKey)
errorAnchor.id = error.cssKey + '-error'
errorAnchor.innerText = error.value
listElement.appendChild(errorAnchor)

if (addType === 'append') {
document.getElementsByClassName('govuk-error-summary__list')[0].appendChild(listElement)
} else {
document.getElementsByClassName('govuk-error-summary__list')[0].insertBefore(listElement, parent.firstChild)
if(!document.getElementById(errorAnchor.id)){
errorAnchor.innerText = error.value
listElement.appendChild(errorAnchor)
if (addType === 'append') {
document.getElementsByClassName('govuk-error-summary__list')[0].appendChild(listElement)
} else {
document.getElementsByClassName('govuk-error-summary__list')[0].insertBefore(listElement, parent.firstChild)
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion app/utils/charge-validation-fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ function containsSuspectedCVV (input) {

// Must be bound prior to use
function cardNo (cardNo) {
if (!cardNo) return 'message' // default message
const numbersAndSpacesOnly = /^[\d ]+$/;
if (!cardNo || !numbersAndSpacesOnly.test(cardNo)) return 'message' // default message
cardNo = cardNo.replace(/\D/g, '')
const cardType = creditCardType(cardNo)
if (!cardNo || cardNo.length < 12 || cardNo.length > 19) return 'numberIncorrectLength'
Expand Down
11 changes: 1 addition & 10 deletions app/utils/charge-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,6 @@ const changeCase = require('change-case')
// Local dependencies
const chargeValidationFields = require('./charge-validation-fields')

// Constants
const CUSTOM_ERRORS = {
expiryYear: {
skip: true
},
expiryMonth: {
cssKey: 'expiry-date'
}
}

module.exports = (translations, logger, Card, chargeOptions = { collect_billing_address: true }) => {
const validations = chargeValidationFields(Card, chargeOptions)
Expand Down Expand Up @@ -48,7 +39,7 @@ module.exports = (translations, logger, Card, chargeOptions = { collect_billing_
const messageTemplate = translations.generic
const problem = value || isOptional ? translation[customValidation] : messageTemplate.replace('%s', translation.name)
// Push to Error Fields
const customError = CUSTOM_ERRORS[name] || {}
const customError = name === "expiryYear" ? {skip: true} : {}
const cssKey = customError.cssKey || changeCase.paramCase(name)
if (!customError.skip) checkResult.errorFields.push({ cssKey, key: name, value: problem })
// Push to Highlight Fields
Expand Down
21 changes: 16 additions & 5 deletions app/views/charge.njk
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,13 @@
<label id="card-no-lbl" for="card-no" class="govuk-label govuk-label--s govuk-!-width-three-quarters">
<span
class="card-no-label"
data-label-replace="cardNo"
data-original-label="{{ __p("cardDetails.cardNo") }}">
{{ __p("cardDetails.cardNo") }}
</span>
</label>

{% if highlightErrorFields.cardNo %}
<p class="govuk-error-message" id="error-card-no">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.cardNo }}
</p>
{% endif %}
Expand All @@ -204,12 +203,15 @@
<p class="govuk-body accepted-cards-hint withdrawal-text govuk-!-margin-bottom-0">
{% if withdrawalText === 'debit_credit' %}
{{ __p("cardDetails.withdrawalText").debit_credit }}
<span data-label-replace="cardNo"></span>
{% endif %}
{% if withdrawalText === 'debit' %}
{{ __p("cardDetails.withdrawalText").debit }}
<span data-label-replace="cardNo"></span>
{% endif %}
{% if withdrawalText === 'credit' %}
{{ __p("cardDetails.withdrawalText").credit }}
<span data-label-replace="cardNo"></span>
{% endif %}
</p>

Expand Down Expand Up @@ -247,17 +249,18 @@
<legend>
<div class="govuk-label govuk-label--s" id="expiry-date-lbl" for="expiry-month">
<span class="expiry-date-label"
data-label-replace="expiryMonth"
data-original-label="{{ __p("cardDetails.expiry") }}">
{{ __p("cardDetails.expiry") }}
</span>
</div>
</legend>
<div class="govuk-hint govuk-!-margin-bottom-2" id="expiry-date-hint">
<div class="govuk-hint govuk-!-margin-bottom-0" id="expiry-date-hint">
{{ __p("cardDetails.expiryHint", exampleCardExpiryDateYear) }}
<span data-label-replace="expiryMonth"></span>
</div>
{% if highlightErrorFields.expiryMonth %}
<p class="govuk-error-message" id="error-expiry-date">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.expiryMonth }}
</p>
{% endif %}
Expand Down Expand Up @@ -307,6 +310,7 @@

{% if highlightErrorFields.cardholderName %}
<p class="govuk-error-message" id="error-cardholder-name">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.cardholderName }}
</p>
{% endif %}
Expand All @@ -323,14 +327,15 @@

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

<div class="govuk-hint govuk-!-margin-bottom-2">
<span class="generic-cvc">
{{ __p("cardDetails.cvcTip") }}
<span data-label-replace="cvc"></span>
</span>
<span class="amex-cvc hidden">
<span class="hidden">
Expand All @@ -344,6 +349,7 @@

{% if highlightErrorFields.cvc %}
<p class="govuk-error-message" id="error-cvc">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.cvc }}
</p>
{% endif %}
Expand Down Expand Up @@ -395,6 +401,7 @@

{% if highlightErrorFields.addressLine1 %}
<p class="govuk-error-message" id="error-address-line-1">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.addressLine1 }}
</p>
{% endif %}
Expand Down Expand Up @@ -448,6 +455,7 @@

{% if highlightErrorFields.addressCity %}
<p class="govuk-error-message" id="error-address-city">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.addressCity }}
</p>
{% endif %}
Expand All @@ -473,6 +481,7 @@

{% if highlightErrorFields.addressCountry %}
<p class="govuk-error-message" id="error-address-country">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.addressCountry }}
</p>
{% endif %}
Expand Down Expand Up @@ -502,6 +511,7 @@

{% if highlightErrorFields.addressPostcode %}
<p class="govuk-error-message" id="error-address-postcode">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.addressPostcode }}
</p>
{% endif %}
Expand Down Expand Up @@ -541,6 +551,7 @@

{% if highlightErrorFields.email %}
<p class="govuk-error-message" id="error-email">
<span class="govuk-visually-hidden">{{ __p("fieldErrors.visuallyHiddenError") }}</span>
{{ highlightErrorFields.email }}
</p>
{% endif %}
Expand Down
1 change: 1 addition & 0 deletions locales/cy.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
"fieldErrors": {
"summary": "Mae’r meysydd canlynol yn wag neu yn cynnwys camgymeriadau",
"generic": "Rhowch %s dilys",
"visuallyHiddenError": "Gwall:",
"fields": {
"cardholderName": {
"name": "enw",
Expand Down
1 change: 1 addition & 0 deletions locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
"fieldErrors": {
"summary": "The following fields are missing or contain errors",
"generic": "Enter a valid %s",
"visuallyHiddenError": "Error:",
"fields": {
"cardholderName": {
"name": "name",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('Card details page validation', () => {
cy.get('#error-summary-title').should(($td) => expect($td).to.contain('The following fields are missing or contain errors'))
cy.get('#cardholder-name-error').should('exist')
cy.get('#cvc-error').should('exist')
cy.get('#expiry-date-error').should('exist')
cy.get('#expiry-month-error').should('exist')
cy.get('#address-line-1-error').should('exist')
cy.get('#address-city-error').should('exist')
cy.get('#address-postcode-error').should('exist')
Expand Down
2 changes: 1 addition & 1 deletion test/integration/card-details-errors.ft.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ describe('chargeTests', function () {
}
expect($('#card-no-error').text()).to.contains(errorMessages.cardNo)
expect($('#error-card-no').text()).to.contains(errorMessages.cardNo)
expect($('#expiry-date-error').text()).to.contains(errorMessages.expiryDate)
expect($('#expiry-month-error').text()).to.contains(errorMessages.expiryDate)
expect($('#error-expiry-date').text()).to.contains(errorMessages.expiryDate)
expect($('#cardholder-name-error').text()).to.contains('Enter a valid name')
expect($('#error-cardholder-name').text()).to.contains('Enter the name as it appears on the card')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ describe('card validation: card number', function () {
expect(result).to.equal(true)
})

it('should strip non numbers', function () {
it('should return invalid if cardNo contains non-numbers', function () {
const result = fields.fieldValidations.cardNo('42424242424242dsakljl42')
expect(result).to.equal(true)
expect(result).to.equal('message')
})

it('should return incorrect length', function () {
Expand Down

0 comments on commit 9ffb54d

Please sign in to comment.