Skip to content

Commit

Permalink
fix(EMS-379): fix issue where buyer country would not update when JS …
Browse files Browse the repository at this point in the history
…is disabled (#141)

* fix(EMS-379): fix issue where buyer country would not update in a 'change answer' scenario if JS is disabled

* fix(EMS-379): update e2e tests to check autocomplete value instead of hidden input value
  • Loading branch information
ttbarnes authored Oct 27, 2022
1 parent c2b7306 commit 397378b
Show file tree
Hide file tree
Showing 16 changed files with 36 additions and 103 deletions.
1 change: 0 additions & 1 deletion e2e-tests/constants/field-ids.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const FIELD_IDS = {
VALID_BUYER_BODY: 'validBuyerBody',
VALID_COMPANY_BASE: 'validCompanyBase',
BUYER_COUNTRY: 'buyerCountry',
COUNTRY: 'country',
HAS_MINIMUM_UK_GOODS_OR_SERVICES: 'hasMinimumUkGoodsOrServices',
AMOUNT_CURRENCY: 'amountAndCurrency',
CURRENCY: 'currency',
Expand Down
1 change: 0 additions & 1 deletion e2e-tests/content-strings/error-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const FIELD_IDS = require('../constants/field-ids');

const ERROR_MESSAGES = {
[FIELD_IDS.BUYER_COUNTRY]: 'Select where your buyer is based',
[FIELD_IDS.COUNTRY]: 'Select where your buyer is based',
[FIELD_IDS.VALID_BUYER_BODY]: 'Select if your buyer is a government or public sector body',
[FIELD_IDS.VALID_COMPANY_BASE]: 'Select if your company is based in the UK, Channel Islands, Isle of Man or not',
[FIELD_IDS.HAS_MINIMUM_UK_GOODS_OR_SERVICES]: {
Expand Down
10 changes: 4 additions & 6 deletions e2e-tests/content-strings/fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,15 @@ const FIELD_VALUES = require('../constants/field-values');
const LINKS = require('./links');

const FIELDS = {
[FIELD_IDS.COUNTRY]: {
[FIELD_IDS.BUYER_COUNTRY]: {
HINT: 'Cover is based on the country your buyer is located in, not the destination of your goods or services.',
},
[FIELD_IDS.VALID_COMPANY_BASE]: {
SUMMARY: {
TITLE: 'Your company',
TITLE: 'Buyer is based in',
},
},
[FIELD_IDS.BUYER_COUNTRY]: {
[FIELD_IDS.VALID_COMPANY_BASE]: {
SUMMARY: {
TITLE: 'Buyer is based in',
TITLE: 'Your company',
},
},
[FIELD_IDS.HAS_MINIMUM_UK_GOODS_OR_SERVICES]: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ context('Buyer country page - as an exporter, I want to check if UKEF issue expo

it('renders a hint', () => {
buyerCountryPage.hint().invoke('text').then((text) => {
expect(text.trim()).equal(FIELDS[FIELD_IDS.COUNTRY].HINT);
expect(text.trim()).equal(FIELDS[FIELD_IDS.BUYER_COUNTRY].HINT);
});
});

Expand Down Expand Up @@ -97,22 +97,6 @@ context('Buyer country page - as an exporter, I want to check if UKEF issue expo
results.should('have.length.greaterThan', 1);
});

it('adds the country name to a hidden input value after searching', () => {
buyerCountryPage.searchInput().type('Algeria');

const noResults = buyerCountryPage.noResults();
noResults.should('not.exist');

const results = buyerCountryPage.results();

// select the first result (Algeria)
results.first().click();

// check hidden input value
const expectedValue = 'Algeria';
buyerCountryPage.hiddenInput().should('have.attr', 'value', expectedValue);
});

it('allows user to remove a selected country and search again', () => {
buyerCountryPage.searchInput().type('Algeria');
const results = buyerCountryPage.results();
Expand All @@ -126,9 +110,12 @@ context('Buyer country page - as an exporter, I want to check if UKEF issue expo
// search for a different country, submit with enter key
buyerCountryPage.searchInput().type('Brazil{enter}');

// check hidden input value
// check selected value in autocomplete
const expectedValue = 'Brazil';
buyerCountryPage.hiddenInput().should('have.attr', 'value', expectedValue);

buyerCountryPage.results().invoke('text').then((text) => {
expect(text.trim()).equal(expectedValue);
});
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ context('Change your answers (export fields) - as an exporter, I want to change
it('has originally submitted answer selected', () => {
const expectedValue = submissionData[BUYER_COUNTRY];

buyerCountryPage.hiddenInput().should('have.attr', 'value', expectedValue);
buyerCountryPage.results().invoke('text').then((text) => {
expect(text.trim()).equal(expectedValue);
});
});

it('has a hash tag and heading/label ID in the URL so that the element gains focus and user has context of what they want to change', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,9 @@ context('Get a quote/your quote page (single policy) - as an exporter, I want to
});

it('clears the session', () => {
buyerCountryPage.hiddenInput().should('have.attr', 'value', '');
// buyer country auto complete stores the selected value in the first list item of the 'results' list.
// Therefore, if it's not defined, nothing has been selected/submitted.
buyerCountryPage.results().should('not.exist');
});
});
});
Expand Down
9 changes: 4 additions & 5 deletions e2e-tests/cypress/e2e/pages/quote/buyerCountry.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ import { FIELD_IDS } from '../../../../constants';

const buyerCountryPage = {
heading: () => cy.get('[data-cy="heading"]'),
hint: () => cy.get(`[data-cy="${FIELD_IDS.COUNTRY}-hint"]`),
searchInput: () => cy.get(`#${FIELD_IDS.COUNTRY}`),
hiddenInput: () => cy.get(`#${FIELD_IDS.BUYER_COUNTRY}`),
results: () => cy.get(`#${FIELD_IDS.COUNTRY} + ul li`),
hint: () => cy.get(`[data-cy="${FIELD_IDS.BUYER_COUNTRY}-hint"]`),
searchInput: () => cy.get(`#${FIELD_IDS.BUYER_COUNTRY}`),
results: () => cy.get(`#${FIELD_IDS.BUYER_COUNTRY} + ul li`),
noResults: () => cy.get('.autocomplete__option--no-results'),
errorMessage: () => cy.get(`[data-cy="${FIELD_IDS.COUNTRY}-error-message"]`),
errorMessage: () => cy.get(`[data-cy="${FIELD_IDS.BUYER_COUNTRY}-error-message"]`),
submitButton: () => cy.get('[data-cy="submit-button"]'),
};

Expand Down
1 change: 0 additions & 1 deletion src/ui/server/constants/field-ids.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ export const FIELD_IDS = {
VALID_BUYER_BODY: 'validBuyerBody',
VALID_COMPANY_BASE: 'validCompanyBase',
BUYER_COUNTRY: 'buyerCountry',
COUNTRY: 'country',
HAS_MINIMUM_UK_GOODS_OR_SERVICES: 'hasMinimumUkGoodsOrServices',
AMOUNT_CURRENCY: 'amountAndCurrency',
CURRENCY: 'currency',
Expand Down
1 change: 0 additions & 1 deletion src/ui/server/content-strings/error-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ type ErrorMessage = {

export const ERROR_MESSAGES = {
[FIELD_IDS.BUYER_COUNTRY]: 'Select where your buyer is based',
[FIELD_IDS.COUNTRY]: 'Select where your buyer is based',
[FIELD_IDS.VALID_BUYER_BODY]: 'Select if your buyer is a government or public sector body',
[FIELD_IDS.VALID_COMPANY_BASE]: 'Select if your company is based in the UK, Channel Islands, Isle of Man or not',
[FIELD_IDS.HAS_MINIMUM_UK_GOODS_OR_SERVICES]: {
Expand Down
10 changes: 4 additions & 6 deletions src/ui/server/content-strings/fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,15 @@ import { FIELD_IDS, FIELD_VALUES } from '../constants';
import { LINKS } from './links';

export const FIELDS = {
[FIELD_IDS.COUNTRY]: {
[FIELD_IDS.BUYER_COUNTRY]: {
HINT: 'Cover is based on the country your buyer is located in, not the destination of your goods or services.',
},
[FIELD_IDS.VALID_COMPANY_BASE]: {
SUMMARY: {
TITLE: 'Your company',
TITLE: 'Buyer is based in',
},
},
[FIELD_IDS.BUYER_COUNTRY]: {
[FIELD_IDS.VALID_COMPANY_BASE]: {
SUMMARY: {
TITLE: 'Buyer is based in',
TITLE: 'Your company',
},
},
[FIELD_IDS.HAS_MINIMUM_UK_GOODS_OR_SERVICES]: {
Expand Down
10 changes: 3 additions & 7 deletions src/ui/server/controllers/quote/buyer-country/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('controllers/quote/buyer-country', () => {
describe('PAGE_VARIABLES', () => {
it('should have correct properties', () => {
const expected = {
FIELD_ID: FIELD_IDS.COUNTRY,
FIELD_ID: FIELD_IDS.BUYER_COUNTRY,
PAGE_CONTENT_STRINGS: PAGES.BUYER_COUNTRY_PAGE,
};

Expand Down Expand Up @@ -131,7 +131,6 @@ describe('controllers/quote/buyer-country', () => {
const expectedVariables = {
...singleInputPageVariables(PAGE_VARIABLES),
BACK_LINK: getBackLink(req.headers.referer),
HIDDEN_FIELD_ID: FIELD_IDS.BUYER_COUNTRY,
countries: mapCountries(mockCountriesResponse),
submittedValues: req.session.submittedData,
isChangeRoute: isChangeRoute(req.originalUrl),
Expand All @@ -151,7 +150,6 @@ describe('controllers/quote/buyer-country', () => {
const expectedVariables = {
...singleInputPageVariables(PAGE_VARIABLES),
BACK_LINK: getBackLink(req.headers.referer),
HIDDEN_FIELD_ID: FIELD_IDS.BUYER_COUNTRY,
countries: expectedCountries,
submittedValues: req.session.submittedData,
isChangeRoute: isChangeRoute(req.originalUrl),
Expand Down Expand Up @@ -214,7 +212,6 @@ describe('controllers/quote/buyer-country', () => {
expect(res.render).toHaveBeenCalledWith(TEMPLATES.QUOTE.BUYER_COUNTRY, {
...singleInputPageVariables(PAGE_VARIABLES),
BACK_LINK: getBackLink(req.headers.referer),
HIDDEN_FIELD_ID: FIELD_IDS.BUYER_COUNTRY,
countries: mapCountries(mockCountriesResponse),
validationErrors: generateValidationErrors(req.body),
isChangeRoute: isChangeRoute(req.originalUrl),
Expand Down Expand Up @@ -335,15 +332,14 @@ describe('controllers/quote/buyer-country', () => {
});
});

describe(`when the country is supported for an online quote, submitted with ${FIELD_IDS.COUNTRY} (no JS) and there are no validation errors`, () => {
describe(`when the country is supported for an online quote, submitted with ${FIELD_IDS.BUYER_COUNTRY} (no JS) and there are no validation errors`, () => {
const selectedCountryName = mockAnswers[FIELD_IDS.BUYER_COUNTRY];
const mappedCountries = mapCountries(mockCountriesResponse);

const selectedCountry = getCountryByName(mappedCountries, selectedCountryName);

const validBody = {
[FIELD_IDS.BUYER_COUNTRY]: '',
[FIELD_IDS.COUNTRY]: selectedCountryName,
[FIELD_IDS.BUYER_COUNTRY]: selectedCountryName,
};

beforeEach(() => {
Expand Down
6 changes: 2 additions & 4 deletions src/ui/server/controllers/quote/buyer-country/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { updateSubmittedData } from '../../../helpers/update-submitted-data';
import { Request, Response } from '../../../../types';

export const PAGE_VARIABLES = {
FIELD_ID: FIELD_IDS.COUNTRY,
FIELD_ID: FIELD_IDS.BUYER_COUNTRY,
PAGE_CONTENT_STRINGS: PAGES.BUYER_COUNTRY_PAGE,
};

Expand Down Expand Up @@ -71,7 +71,6 @@ export const get = async (req: Request, res: Response) => {
return res.render(TEMPLATES.QUOTE.BUYER_COUNTRY, {
...singleInputPageVariables(PAGE_VARIABLES),
BACK_LINK: getBackLink(req.headers.referer),
HIDDEN_FIELD_ID: FIELD_IDS.BUYER_COUNTRY,
countries: mappedCountries,
submittedValues: req.session.submittedData,
isChangeRoute: isChangeRoute(req.originalUrl),
Expand All @@ -93,14 +92,13 @@ export const post = async (req: Request, res: Response) => {
return res.render(TEMPLATES.QUOTE.BUYER_COUNTRY, {
...singleInputPageVariables(PAGE_VARIABLES),
BACK_LINK: getBackLink(req.headers.referer),
HIDDEN_FIELD_ID: FIELD_IDS.BUYER_COUNTRY,
countries: mappedCountries,
validationErrors,
isChangeRoute: isChangeRoute(req.originalUrl),
});
}

const submittedCountryName = req.body[FIELD_IDS.BUYER_COUNTRY] || req.body[FIELD_IDS.COUNTRY];
const submittedCountryName = req.body[FIELD_IDS.BUYER_COUNTRY];

const country = getCountryByName(mappedCountries, submittedCountryName);

Expand Down
34 changes: 4 additions & 30 deletions src/ui/server/controllers/quote/buyer-country/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,44 +13,18 @@ describe('controllers/quote/buyer-country/validation', () => {
});
});

describe(`when both ${FIELD_IDS.BUYER_COUNTRY} and ${FIELD_IDS.COUNTRY} are provided`, () => {
describe(`when ${FIELD_IDS.COUNTRY} does NOT have a value`, () => {
describe(`when ${FIELD_IDS.BUYER_COUNTRY} is provided`, () => {
describe(`when ${FIELD_IDS.BUYER_COUNTRY} does NOT have a value`, () => {
it('should return true', () => {
const mockBody = {
[FIELD_IDS.BUYER_COUNTRY]: '',
[FIELD_IDS.COUNTRY]: '',
};

const result = hasErrors(mockBody);

expect(result).toEqual(true);
});
});

describe(`when ${FIELD_IDS.COUNTRY} has a value`, () => {
it('should return false', () => {
const mockBody = {
[FIELD_IDS.BUYER_COUNTRY]: '',
[FIELD_IDS.COUNTRY]: 'Australia',
};

const result = hasErrors(mockBody);

expect(result).toEqual(false);
});
});
});

describe(`when only ${FIELD_IDS.BUYER_COUNTRY} is provided and there is no value`, () => {
it('should return true', () => {
const mockBody = {
[FIELD_IDS.BUYER_COUNTRY]: '',
};

const result = hasErrors(mockBody);

expect(result).toEqual(true);
});
});

it('should return false', () => {
Expand All @@ -69,7 +43,7 @@ describe('controllers/quote/buyer-country/validation', () => {
it('should return validation errors', () => {
const result = validation({});

const expected = generateValidationErrors(FIELD_IDS.COUNTRY, CONTENT_STRINGS.ERROR_MESSAGES[FIELD_IDS.COUNTRY]);
const expected = generateValidationErrors(FIELD_IDS.BUYER_COUNTRY, CONTENT_STRINGS.ERROR_MESSAGES[FIELD_IDS.BUYER_COUNTRY]);

expect(result).toEqual(expected);
});
Expand All @@ -79,7 +53,7 @@ describe('controllers/quote/buyer-country/validation', () => {
it('should return validation errors', () => {
const result = validation({});

const expected = generateValidationErrors(FIELD_IDS.COUNTRY, CONTENT_STRINGS.ERROR_MESSAGES[FIELD_IDS.COUNTRY]);
const expected = generateValidationErrors(FIELD_IDS.BUYER_COUNTRY, CONTENT_STRINGS.ERROR_MESSAGES[FIELD_IDS.BUYER_COUNTRY]);

expect(result).toEqual(expected);
});
Expand Down
14 changes: 1 addition & 13 deletions src/ui/server/controllers/quote/buyer-country/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,7 @@ const hasErrors = (formBody: RequestBody) => {
return true;
}

const keys = Object.keys(formBody);
if (keys.includes(FIELD_IDS.BUYER_COUNTRY) && keys.includes(FIELD_IDS.COUNTRY)) {
// form submitted without client side JS

if (!objectHasProperty(formBody, FIELD_IDS.COUNTRY)) {
return true;
}

return false;
}

if (!objectHasProperty(formBody, FIELD_IDS.BUYER_COUNTRY)) {
// form submitted with client side JS
return true;
}

Expand All @@ -32,7 +20,7 @@ const validation = (formBody: RequestBody) => {
let errors;

if (hasErrors(formBody)) {
errors = generateValidationErrors(FIELD_IDS.COUNTRY, ERROR_MESSAGES[FIELD_IDS.COUNTRY]);
errors = generateValidationErrors(FIELD_IDS.BUYER_COUNTRY, ERROR_MESSAGES[FIELD_IDS.BUYER_COUNTRY]);

return errors;
}
Expand Down
2 changes: 1 addition & 1 deletion src/ui/server/helpers/single-input-page-variables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('server/helpers/single-input-page-variables', () => {

describe('when a FIELD_ID exists in content string fields', () => {
it('should also return FIELD_HINT', () => {
mock.FIELD_ID = FIELD_IDS.COUNTRY;
mock.FIELD_ID = FIELD_IDS.BUYER_COUNTRY;
const result = singleInputPageVariables(mock);

const expected = FIELDS[mock.FIELD_ID].HINT;
Expand Down
7 changes: 1 addition & 6 deletions src/ui/templates/quote/buyer-country.njk
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,7 @@
var element = document.getElementById('{{ FIELD_ID }}');
accessibleAutocomplete.enhanceSelectElement({
selectElement: element,
defaultValue: '',
onConfirm: function(event) {
if (event) {
document.getElementById('{{ HIDDEN_FIELD_ID }}').value = event;
}
}
defaultValue: ''
});
</script>

Expand Down

0 comments on commit 397378b

Please sign in to comment.