From 80a785ab5160e3a0ab75ef695b4dc4b2e6fe6735 Mon Sep 17 00:00:00 2001 From: Tony Barnes Date: Thu, 28 Dec 2023 10:25:52 +0000 Subject: [PATCH] fix(EMS-2492): Quote - Policy type form validation (#1602) * fix(EMS-2492): quote - policy type form validation * chore(EMS-2492): quote - policy type - simplify single/multiple field references * fix(EMS-2492): typo * fix(EMS-2492): fix/update unit test --- e2e-tests/pages/quote/policyType.js | 1 + .../exporter-location.spec.js | 3 +- .../policy-type-validation.spec.js | 18 +++++++-- .../validation/rules/policy-type.test.ts | 18 +++++++-- .../validation/rules/policy-type.ts | 6 +-- src/ui/templates/cookies.njk | 2 +- src/ui/templates/quote/policy-type.njk | 40 ++++++++++--------- 7 files changed, 58 insertions(+), 30 deletions(-) diff --git a/e2e-tests/pages/quote/policyType.js b/e2e-tests/pages/quote/policyType.js index eef7e03e90..e94a28911f 100644 --- a/e2e-tests/pages/quote/policyType.js +++ b/e2e-tests/pages/quote/policyType.js @@ -9,6 +9,7 @@ const { const policyTypePage = { [POLICY_TYPE]: { + ...field(POLICY_TYPE), single: { ...field(SINGLE_POLICY_TYPE), hintListItem: (index) => cy.get(`[data-cy="${SINGLE_POLICY_TYPE}-hint-list-item-${index}"]`), diff --git a/e2e-tests/quote/cypress/e2e/journeys/quote/exporter-location/exporter-location.spec.js b/e2e-tests/quote/cypress/e2e/journeys/quote/exporter-location/exporter-location.spec.js index ec0bb68616..6f892003c2 100644 --- a/e2e-tests/quote/cypress/e2e/journeys/quote/exporter-location/exporter-location.spec.js +++ b/e2e-tests/quote/cypress/e2e/journeys/quote/exporter-location/exporter-location.spec.js @@ -60,12 +60,13 @@ context('Exporter location page - as an exporter, I want to check if my company describe('form submission', () => { describe('when submitting an empty form', () => { it('should render validation errors', () => { + const errorIndex = 0; const expectedErrorsCount = 1; const expectedErrorMessage = ERROR_MESSAGES.ELIGIBILITY[FIELD_ID]; cy.submitAndAssertRadioErrors( yesRadio(FIELD_ID), - 0, + errorIndex, expectedErrorsCount, expectedErrorMessage, ); diff --git a/e2e-tests/quote/cypress/e2e/journeys/quote/policy-type/policy-type-validation.spec.js b/e2e-tests/quote/cypress/e2e/journeys/quote/policy-type/policy-type-validation.spec.js index 5382347b4a..974f64f073 100644 --- a/e2e-tests/quote/cypress/e2e/journeys/quote/policy-type/policy-type-validation.spec.js +++ b/e2e-tests/quote/cypress/e2e/journeys/quote/policy-type/policy-type-validation.spec.js @@ -1,4 +1,5 @@ import { submitButton } from '../../../../../../pages/shared'; +import { policyTypePage } from '../../../../../../pages/quote'; import partials from '../../../../../../partials'; import { ERROR_MESSAGES } from '../../../../../../content-strings'; import { ROUTES, FIELD_IDS } from '../../../../../../constants'; @@ -40,12 +41,23 @@ context('Policy type page - policy type & length validation - single policy type submitButton().click(); cy.checkErrorSummaryListHeading(); - partials.errorSummaryListItems().should('have.length', 1); - const expectedMessage = ERROR_MESSAGES.ELIGIBILITY[POLICY_TYPE]; + const expectedErrorMessage = ERROR_MESSAGES.ELIGIBILITY[POLICY_TYPE]; + + cy.checkText( + partials.errorSummaryListItems().first(), + expectedErrorMessage, + ); + + partials.errorSummaryListItemLinks().first().click(); + + const singlePolicyTypeField = policyTypePage[POLICY_TYPE].single; + singlePolicyTypeField.input().should('have.focus'); + + const policyTypeField = policyTypePage[POLICY_TYPE]; - cy.checkText(partials.errorSummaryListItems().first(), expectedMessage); + cy.checkText(policyTypeField.errorMessage().first(), `Error: ${expectedErrorMessage}`); }); }); }); diff --git a/src/ui/server/controllers/quote/policy-type/validation/rules/policy-type.test.ts b/src/ui/server/controllers/quote/policy-type/validation/rules/policy-type.test.ts index 918e80c572..9ddc822406 100644 --- a/src/ui/server/controllers/quote/policy-type/validation/rules/policy-type.test.ts +++ b/src/ui/server/controllers/quote/policy-type/validation/rules/policy-type.test.ts @@ -1,9 +1,9 @@ import rule from './policy-type'; -import { FIELD_IDS } from '../../../../../constants'; +import { FIELD_IDS, FIELD_VALUES } from '../../../../../constants'; import { ERROR_MESSAGES } from '../../../../../content-strings'; import emptyFieldValidation from '../../../../../shared-validation/empty-field'; -const { POLICY_TYPE: FIELD_ID } = FIELD_IDS; +const { POLICY_TYPE: FIELD_ID, SINGLE_POLICY_TYPE } = FIELD_IDS; const ERROR_MESSAGE = ERROR_MESSAGES.ELIGIBILITY[FIELD_ID]; describe('controllers/quote/policy-type/validation/rules/policy-type', () => { @@ -22,7 +22,7 @@ describe('controllers/quote/policy-type/validation/rules/policy-type', () => { const result = rule(mockBody, mockErrors); - const expected = emptyFieldValidation({}, FIELD_ID, ERROR_MESSAGE, mockErrors); + const expected = emptyFieldValidation({}, SINGLE_POLICY_TYPE, ERROR_MESSAGE, mockErrors); expect(result).toEqual(expected); }); @@ -34,9 +34,19 @@ describe('controllers/quote/policy-type/validation/rules/policy-type', () => { const result = rule(mockBody, mockErrors); - const expected = emptyFieldValidation(mockBody, FIELD_ID, ERROR_MESSAGE, mockErrors); + const expected = emptyFieldValidation({}, SINGLE_POLICY_TYPE, ERROR_MESSAGE, mockErrors); expect(result).toEqual(expected); }); }); + + describe('when a valid policy type provided', () => { + it('should return the provided errors', () => { + mockBody[FIELD_ID] = FIELD_VALUES.POLICY_TYPE.SINGLE; + + const result = rule(mockBody, mockErrors); + + expect(result).toEqual(mockErrors); + }); + }); }); diff --git a/src/ui/server/controllers/quote/policy-type/validation/rules/policy-type.ts b/src/ui/server/controllers/quote/policy-type/validation/rules/policy-type.ts index 0263ba74bf..4da04592e3 100644 --- a/src/ui/server/controllers/quote/policy-type/validation/rules/policy-type.ts +++ b/src/ui/server/controllers/quote/policy-type/validation/rules/policy-type.ts @@ -4,7 +4,7 @@ import { isValidPolicyType } from '../../../../../helpers/policy-type'; import emptyFieldValidation from '../../../../../shared-validation/empty-field'; import { RequestBody } from '../../../../../../types'; -const { POLICY_TYPE: FIELD_ID } = FIELD_IDS; +const { POLICY_TYPE: FIELD_ID, SINGLE_POLICY_TYPE } = FIELD_IDS; const ERROR_MESSAGE = ERROR_MESSAGES.ELIGIBILITY[FIELD_ID]; /** @@ -18,10 +18,10 @@ const policyTypeRules = (formBody: RequestBody, errors: object) => { const value = formBody[FIELD_ID]; if (!isValidPolicyType(value)) { - return emptyFieldValidation({}, FIELD_ID, ERROR_MESSAGE, errors); + return emptyFieldValidation({}, SINGLE_POLICY_TYPE, ERROR_MESSAGE, errors); } - return emptyFieldValidation(formBody, FIELD_ID, ERROR_MESSAGE, errors); + return errors; }; export default policyTypeRules; diff --git a/src/ui/templates/cookies.njk b/src/ui/templates/cookies.njk index cb8f4e956c..862b29c673 100644 --- a/src/ui/templates/cookies.njk +++ b/src/ui/templates/cookies.njk @@ -157,7 +157,7 @@ checked: submittedValue === 'false' } ], - errorMessage: validationErrors.errorList[FIELD_ID]and { + errorMessage: validationErrors.errorList[FIELD_ID] and { text: validationErrors.errorList[FIELD_ID].text, attributes: { "data-cy": FIELD_ID + "-error-message" diff --git a/src/ui/templates/quote/policy-type.njk b/src/ui/templates/quote/policy-type.njk index 3c18c8ea05..01da2aed53 100644 --- a/src/ui/templates/quote/policy-type.njk +++ b/src/ui/templates/quote/policy-type.njk @@ -26,6 +26,10 @@ }) }} {% endif %} + {% set SINGLE_OPTION = FIELDS.POLICY_TYPE.OPTIONS.SINGLE %} + {% set MULTIPLE_OPTION = FIELDS.POLICY_TYPE.OPTIONS.MULTIPLE %} + +
@@ -35,8 +39,8 @@ {% set singlePolicyHintHtml %}
    - {% for item in FIELDS.POLICY_TYPE.OPTIONS.SINGLE.HINT %} -
  • {{ item }}
    • + {% for item in SINGLE_OPTION.HINT %} +
  • {{ item }}
    • {% endfor %}
{% endset %} @@ -51,8 +55,8 @@ {% set multiplePolicyHintHtml %}
    - {% for item in FIELDS.POLICY_TYPE.OPTIONS.MULTIPLE.HINT %} -
  • {{ item }}
    • + {% for item in MULTIPLE_OPTION.HINT %} +
  • {{ item }}
    • {% endfor %}
{% endset %} @@ -68,13 +72,13 @@ classes: "govuk-!-margin-bottom-4", items: [ { - id: FIELDS.POLICY_TYPE.OPTIONS.SINGLE.ID, - value: FIELDS.POLICY_TYPE.OPTIONS.SINGLE.VALUE, - text: FIELDS.POLICY_TYPE.OPTIONS.SINGLE.TEXT, + id: SINGLE_OPTION.ID, + value: SINGLE_OPTION.VALUE, + text: SINGLE_OPTION.TEXT, label: { classes: 'govuk-heading-m govuk-!-font-weight-bold govuk-!-font-size-24', attributes: { - 'data-cy': FIELDS.POLICY_TYPE.OPTIONS.SINGLE.ID + '-label' + 'data-cy': SINGLE_OPTION.ID + '-label' } }, hint: { @@ -82,18 +86,18 @@ classes: 'govuk-!-padding-left-0 govuk-!-margin-bottom-8' }, attributes: { - 'data-cy': FIELDS.POLICY_TYPE.OPTIONS.SINGLE.ID + '-input' + 'data-cy': SINGLE_OPTION.ID + '-input' }, - checked: submittedValues[FIELDS.POLICY_TYPE.ID] === FIELDS.POLICY_TYPE.OPTIONS.SINGLE.VALUE + checked: submittedValues[FIELDS.POLICY_TYPE.ID] === SINGLE_OPTION.VALUE }, { - id: FIELDS.POLICY_TYPE.OPTIONS.MULTIPLE.ID, - value: FIELDS.POLICY_TYPE.OPTIONS.MULTIPLE.VALUE, - text: FIELDS.POLICY_TYPE.OPTIONS.MULTIPLE.TEXT, + id: MULTIPLE_OPTION.ID, + value: MULTIPLE_OPTION.VALUE, + text: MULTIPLE_OPTION.TEXT, label: { classes: 'govuk-heading-m govuk-!-font-weight-bold govuk-!-font-size-24', attributes: { - 'data-cy': FIELDS.POLICY_TYPE.OPTIONS.MULTIPLE.ID + '-label' + 'data-cy': MULTIPLE_OPTION.ID + '-label' } }, hint: { @@ -101,16 +105,16 @@ classes: 'govuk-!-padding-left-0' }, attributes: { - 'data-cy': FIELDS.POLICY_TYPE.OPTIONS.MULTIPLE.ID + '-input' + 'data-cy': MULTIPLE_OPTION.ID + '-input' }, - checked: submittedValues[FIELDS.POLICY_TYPE.ID] === FIELDS.POLICY_TYPE.OPTIONS.MULTIPLE.VALUE, + checked: submittedValues[FIELDS.POLICY_TYPE.ID] === MULTIPLE_OPTION.VALUE, conditional: { html: multiplePolicyTypeInsetHtml } } ], - errorMessage: validationError and { - text: validationError.text, + errorMessage: validationErrors.errorList[SINGLE_OPTION.ID] and { + text: validationErrors.errorList[SINGLE_OPTION.ID].text, attributes: { "data-cy": FIELDS.POLICY_TYPE.ID + "-error-message" }