From 2182853dc6ecf890ac45edd3c47b66798f08bcd6 Mon Sep 17 00:00:00 2001 From: Benjamin Wheeler Date: Tue, 5 Nov 2019 18:07:31 -0500 Subject: [PATCH 1/5] Revert "Revert "fixed country options to use full country name string as option value"" --- src/lib/country-data.js | 22 ++++++++++++---------- test/unit/lib/country-data.test.js | 17 +++++++++++++---- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/lib/country-data.js b/src/lib/country-data.js index 3d2f32884..8c80c1e2f 100644 --- a/src/lib/country-data.js +++ b/src/lib/country-data.js @@ -1060,23 +1060,25 @@ const dupeCommonCountries = module.exports.dupeCommonCountries = (startingCountr /* * registrationCountryOptions is the result of taking the standard countryInfo, - * and duplicating 'United States of America' and 'United Kingdom' at the top of the list. + * setting a 'value' key and a 'label' key both to the country data's 'name' value, + * but using the 'display' value for 'label' instead of 'name' if 'display' exists; + * then duplicating 'United States of America' and 'United Kingdom' at the top of the list. * The result is an array like: * [ - * {code: 'us', name: 'United States', display: 'United States of America'}, - * {code: 'gb', name: 'United Kingdom'}, - * {code: 'af', name: 'Afghanistan'}, + * {value: 'United States', label: 'United States of America'}, + * {value: 'United Kingdom', label: 'United Kingdom'}, + * {value: 'Afghanistan', label: 'Afghanistan'}, * ... - * {code: 'ae', name: 'United Arab Emirates'}, - * {code: 'us', name: 'United States', display: 'United States of America'}, - * {code: 'gb', name: 'United Kingdom'}, - * {code: 'uz', name: 'Uzbekistan'}, + * {value: 'United Arab Emirates', label: 'United Arab Emirates'}, + * {value: 'United States', label: 'United States of America'}, + * {value: 'United Kingdom', label: 'United Kingdom'}, + * {value: 'Uzbekistan', label: 'Uzbekistan'}, * ... - * {code: 'zm', name: 'Zimbabwe'} + * {value: 'Zimbabwe', label: 'Zimbabwe'} * ] */ module.exports.registrationCountryOptions = - countryOptions(dupeCommonCountries(countryInfo, ['us', 'gb']), 'code'); + countryOptions(dupeCommonCountries(countryInfo, ['us', 'gb']), 'name'); /* subdivisionOptions uses iso-3166 data to produce an array like: * [ diff --git a/test/unit/lib/country-data.test.js b/test/unit/lib/country-data.test.js index 5c8122604..1321e2f96 100644 --- a/test/unit/lib/country-data.test.js +++ b/test/unit/lib/country-data.test.js @@ -74,6 +74,15 @@ describe('unit test lib/country-data.js', () => { expect(ukItems.length).toEqual(2); }); + test('registrationCountryOptions object uses country names for both option label and option value', () => { + expect(typeof registrationCountryOptions).toBe('object'); + // test that there is one option with label and value === 'Brazil' + const brazilOptions = registrationCountryOptions.reduce((acc, thisCountry) => ( + (thisCountry.value === 'Brazil' && thisCountry.label === 'Brazil') ? [...acc, thisCountry] : acc + ), []); + expect(brazilOptions.length).toEqual(1); + }); + test('registrationCountryOptions object places USA and UK at start, with display name versions', () => { expect(typeof registrationCountryOptions).toBe('object'); const numCountries = countryInfo.length; @@ -81,17 +90,17 @@ describe('unit test lib/country-data.js', () => { // test that the two entries have been added to the start of the array, and that // the name of the USA includes "America" expect(registrationCountryOptions.length).toEqual(numCountries + 2); - expect(registrationCountryOptions[0]).toEqual({value: 'us', label: 'United States of America'}); - expect(registrationCountryOptions[1]).toEqual({value: 'gb', label: 'United Kingdom'}); + expect(registrationCountryOptions[0]).toEqual({value: 'United States', label: 'United States of America'}); + expect(registrationCountryOptions[1]).toEqual({value: 'United Kingdom', label: 'United Kingdom'}); // test that there are now two entries for USA const usaOptions = registrationCountryOptions.reduce((acc, thisCountry) => ( - thisCountry.value === 'us' ? [...acc, thisCountry] : acc + thisCountry.value === 'United States' ? [...acc, thisCountry] : acc ), []); expect(usaOptions.length).toEqual(2); // test that there are now two entries for UK const ukOptions = registrationCountryOptions.reduce((acc, thisCountry) => ( - thisCountry.value === 'gb' ? [...acc, thisCountry] : acc + thisCountry.value === 'United Kingdom' ? [...acc, thisCountry] : acc ), []); expect(ukOptions.length).toEqual(2); }); From 70d81b85fe0ca2313b7afea76d3f866ebd38f5c9 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 5 Nov 2019 18:30:20 -0500 Subject: [PATCH 2/5] made separate functions for country options with name, code --- src/lib/country-data.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lib/country-data.js b/src/lib/country-data.js index 8c80c1e2f..046fcaae7 100644 --- a/src/lib/country-data.js +++ b/src/lib/country-data.js @@ -1059,7 +1059,7 @@ const dupeCommonCountries = module.exports.dupeCommonCountries = (startingCountr }; /* - * registrationCountryOptions is the result of taking the standard countryInfo, + * registrationCountryNameOptions is the result of taking the standard countryInfo, * setting a 'value' key and a 'label' key both to the country data's 'name' value, * but using the 'display' value for 'label' instead of 'name' if 'display' exists; * then duplicating 'United States of America' and 'United Kingdom' at the top of the list. @@ -1077,8 +1077,11 @@ const dupeCommonCountries = module.exports.dupeCommonCountries = (startingCountr * {value: 'Zimbabwe', label: 'Zimbabwe'} * ] */ -module.exports.registrationCountryOptions = +module.exports.registrationCountryNameOptions = countryOptions(dupeCommonCountries(countryInfo, ['us', 'gb']), 'name'); +// use country code for value, instead of country name: +module.exports.registrationCountryCodeOptions = + countryOptions(dupeCommonCountries(countryInfo, ['us', 'gb']), 'code'); /* subdivisionOptions uses iso-3166 data to produce an array like: * [ From 3e7753691f71b8351abdb332a93494b21b774597 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 5 Nov 2019 18:30:49 -0500 Subject: [PATCH 3/5] use country code options in regular registration --- src/components/registration/steps.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/registration/steps.jsx b/src/components/registration/steps.jsx index 415f38ac9..686dd40e6 100644 --- a/src/components/registration/steps.jsx +++ b/src/components/registration/steps.jsx @@ -44,7 +44,7 @@ const getCountryOptions = reactIntl => ( disabled: true, value: '' }, - ...countryData.registrationCountryOptions + ...countryData.registrationCountryCodeOptions ] ); From 7b69935d50275124a7cdc1a9a299f2f398258bca Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 5 Nov 2019 18:31:05 -0500 Subject: [PATCH 4/5] in join flow, use country name options --- src/components/join-flow/country-step.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/join-flow/country-step.jsx b/src/components/join-flow/country-step.jsx index d3cdc30b0..2164a99cd 100644 --- a/src/components/join-flow/country-step.jsx +++ b/src/components/join-flow/country-step.jsx @@ -27,7 +27,7 @@ class CountryStep extends React.Component { } setCountryOptions () { if (this.countryOptions.length === 0) { - this.countryOptions = [...countryData.registrationCountryOptions]; + this.countryOptions = [...countryData.registrationCountryNameOptions]; this.countryOptions.unshift({ // add placeholder as first option disabled: true, label: this.props.intl.formatMessage({id: 'registration.selectCountry'}), From dd3e1c9f28646b1fa6d43f7992a44e0fe12b78a6 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 5 Nov 2019 18:31:13 -0500 Subject: [PATCH 5/5] update country data test --- test/unit/lib/country-data.test.js | 32 ++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/test/unit/lib/country-data.test.js b/test/unit/lib/country-data.test.js index 1321e2f96..5768e62dd 100644 --- a/test/unit/lib/country-data.test.js +++ b/test/unit/lib/country-data.test.js @@ -3,7 +3,8 @@ const { countryOptions, lookupCountryInfo, dupeCommonCountries, - registrationCountryOptions, + registrationCountryCodeOptions, + registrationCountryNameOptions, subdivisionOptions } = require('../../../src/lib/country-data'); @@ -74,32 +75,41 @@ describe('unit test lib/country-data.js', () => { expect(ukItems.length).toEqual(2); }); - test('registrationCountryOptions object uses country names for both option label and option value', () => { - expect(typeof registrationCountryOptions).toBe('object'); + test('registrationCountryNameOptions object uses country names for both option label and option value', () => { + expect(typeof registrationCountryNameOptions).toBe('object'); // test that there is one option with label and value === 'Brazil' - const brazilOptions = registrationCountryOptions.reduce((acc, thisCountry) => ( + const brazilOptions = registrationCountryNameOptions.reduce((acc, thisCountry) => ( (thisCountry.value === 'Brazil' && thisCountry.label === 'Brazil') ? [...acc, thisCountry] : acc ), []); expect(brazilOptions.length).toEqual(1); }); - test('registrationCountryOptions object places USA and UK at start, with display name versions', () => { - expect(typeof registrationCountryOptions).toBe('object'); + test('registrationCountryCodeOptions object uses country codes for option value', () => { + expect(typeof registrationCountryCodeOptions).toBe('object'); + // test that there is one option with label and value === 'Brazil' + const brazilOptions = registrationCountryCodeOptions.reduce((acc, thisCountry) => ( + (thisCountry.value === 'br' && thisCountry.label === 'Brazil') ? [...acc, thisCountry] : acc + ), []); + expect(brazilOptions.length).toEqual(1); + }); + + test('registrationCountryNameOptions object places USA and UK at start, with display name versions', () => { + expect(typeof registrationCountryNameOptions).toBe('object'); const numCountries = countryInfo.length; // test that the two entries have been added to the start of the array, and that // the name of the USA includes "America" - expect(registrationCountryOptions.length).toEqual(numCountries + 2); - expect(registrationCountryOptions[0]).toEqual({value: 'United States', label: 'United States of America'}); - expect(registrationCountryOptions[1]).toEqual({value: 'United Kingdom', label: 'United Kingdom'}); + expect(registrationCountryNameOptions.length).toEqual(numCountries + 2); + expect(registrationCountryNameOptions[0]).toEqual({value: 'United States', label: 'United States of America'}); + expect(registrationCountryNameOptions[1]).toEqual({value: 'United Kingdom', label: 'United Kingdom'}); // test that there are now two entries for USA - const usaOptions = registrationCountryOptions.reduce((acc, thisCountry) => ( + const usaOptions = registrationCountryNameOptions.reduce((acc, thisCountry) => ( thisCountry.value === 'United States' ? [...acc, thisCountry] : acc ), []); expect(usaOptions.length).toEqual(2); // test that there are now two entries for UK - const ukOptions = registrationCountryOptions.reduce((acc, thisCountry) => ( + const ukOptions = registrationCountryNameOptions.reduce((acc, thisCountry) => ( thisCountry.value === 'United Kingdom' ? [...acc, thisCountry] : acc ), []); expect(ukOptions.length).toEqual(2);