Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CRO-29] Add more country info #2

Merged
merged 4 commits into from
Dec 14, 2021
Merged

[CRO-29] Add more country info #2

merged 4 commits into from
Dec 14, 2021

Conversation

jamszh
Copy link
Contributor

@jamszh jamszh commented Dec 7, 2021

https://aussiecommerce.atlassian.net/browse/CRO-25

For CRO-25, we want to be able to do some basic front-end validation on phone numbers.

True validation of phone numbers is very complex (see libphonenumber), sowe'll simply be checking the number of digits
We've confirmed this is how Expedia does it. "As long as it looks like a number, it is good".

I intend for this lib to be used in svc-traveller when generating the details form.

We should be able to remove the country-telephone-data lib from the customer portal once all these changes are done.

Other Misc changes:

  • Official country name (unicode) if it exists
  • Structure change
string[]
to

<K,V> where K is country iso code and V is a country object

The existing interface has not changed, so there are no breaking changes client side.

/*
* Return a list of country names
*/
export function getCountries(): string[] {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to change the function name, but this will result in breaking changes. So I have left it is as is.

@mynameisrufus
Copy link
Contributor

I'm just wondering if the reason this lib has no recent commits is because discovery service has moved to using https://www.npmjs.com/package/countries-list and https://github.com/lux-group/svc-discovery/blob/master/package.json#L18

Would it be good if this data was surfaced for everyone through svc-discovery?

@jamszh
Copy link
Contributor Author

jamszh commented Dec 9, 2021

I'm just wondering if the reason this lib has no recent commits is because discovery service has moved to using https://www.npmjs.com/package/countries-list and https://github.com/lux-group/svc-discovery/blob/master/package.json#L18

It appears there is some overlap. Especially with the country names data.
This lib is still used in both customer and admin.


Would it be good if this data was surfaced for everyone through svc-discovery?

The only "new" data I've added is the sizeOfNSN field which I can't imagine being useful anywhere except for the phone number validation (in the traveler form). Do you see a use for it in other places?

If we were to move it, I think it would make more sense to have it in the countries-list lib rather than the service itself, but that lib isn't ours.


Would it be good if this data was surfaced for everyone through svc-discovery?

For the rest of the data.
I guess it makes sense if we wanted to completely remove libraries like lib-countries and country-telephone-data from our customer/admin clients and have API calls to fetch this data.

Perhaps this is something that can be done on ENGX?

@jamszh
Copy link
Contributor Author

jamszh commented Dec 9, 2021

This lib is also a public one.
I wasn't able to find a library that had this sizeOfNSN data (that wasn't enormous), so it might also be useful to others who want country data and some light phone number validation.

Copy link

@shawn-cx-li shawn-cx-li left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lib is also a public one.
I wasn't able to find a library that had this sizeOfNSN data (that wasn't enormous), so it might also be useful to others who want country data and some light phone number validation.

Yeah, seems lib-countries doesn't have the sizeOfNSN you need, also I think it's not worthy to make request to svc-calendar for enquiry mobile number format, I'd rather to have it set in this lib

Just out of curious, where do you get the sizeOfNSN data from?

@jamszh
Copy link
Contributor Author

jamszh commented Dec 14, 2021

Just out of curious, where do you get the sizeOfNSN data from?

I was able to get most of this info from Wikipedia (I think it is a good source).

/*
 * Table of data extracted from:
 * https://en.wikipedia.org/wiki/List_of_mobile_telephone_prefixes_by_country
 *
 * See "Size of NSN" column.
 */

For the countries that didn't have size of NSN information in the wikipedia article, I looked at other sources which I've left in-line comments.
e.g.

  CW: {
    countryName: 'Curaçao',
    iso2: 'CW',
    phonePrefix: '599',
    sizeOfNSN: { min: 6, max: 7 } // http://www.wtng.info/wtng-599-an.html
  },

@jamszh jamszh merged commit 94926bf into master Dec 14, 2021
@jamszh jamszh deleted the CRO-25 branch December 16, 2021 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants