Nexmo / nexmo-cli

Nexmo CLI (Command Line Interface)
https://nexmo.com
MIT License
78 stars 52 forks source link

Long Dutch Numbers can't be used #84

Closed sammachin closed 7 years ago

sammachin commented 8 years ago

We have some 'longer than normal' dutch numbers in inventory which support SMS. for example +31 970 1024 0032

Unfortunalty the Number Insight API fails to validate them which means that you can't configure them using the CLI tool.

I've raised this as bug with number insight but putting here mainly for tracking

cbetta commented 8 years ago

@sammachin can't be used for what?

cbetta commented 8 years ago

So the following doesnt work:

➜  ~  nexmo ns NL --sms  
3197010240036
➜  ~  nexmo nb NL 3197010240036
Buying 3197010240036. This operation will charge your account.

Please type "confirm" to continue: confirm

Invalid request :: Not valid number format detected [ 3197010240036 ]
cbetta commented 8 years ago

So the problem with this is in the API, not the CLI. We have a ticket on Jira to look into this.

It would be nice if we could replace the country lookup the country code from a static file (libphonenumber?) if NI fails.

cbetta commented 7 years ago

Still an issue.

sammachin commented 7 years ago

The response I got from Eng. was that we just expose libphonenumber in NI basic and there's an on-going debate there about adding M2M numbers https://github.com/googlei18n/libphonenumber/issues/680

sammachin commented 7 years ago

I think we might have to put some sort of local workaround in the CLI to catch these dutch numbers so that the CLI can be used with them

cbetta commented 7 years ago

@sammachin so what would that look like?

sammachin commented 7 years ago

basically a regex thats checked before the NI call and locally returns the country. at the moment its just the dutch M2M numbers which are +31 970 XXXX XXXX so I think the regex would be ^31970\d{8}$

So some sort of function in the code that checks if the number matches the regex and returns NL first and if there's no match falls through to doing an NI check. Ideally structure it such that we can add more regex's for weird country scenarios in the future if we need to

cbetta commented 7 years ago

That sounds straightforward. I'll have a look at it.

sammachin commented 7 years ago

We have a similar issue thats just arisen with using the CLI to set up linking for shortcodes, (eg 61234) There isn't a pattern that we can use to determine the shortcode as potentially the same shortcode could exist in 2 countries as a different address.

As a workaround for thie and the NL issue along with any other 'invald' number formats could we look to see if the number starts with 2 alpha characters and if so skip the NI lookup and force the country code to whatever the letters are, so entering the number as NL31970XXXXXXXX would set to NL, similarly entering US61234 would set the country to US and the number to 61234

This needs to work on the link:sms as well as the number buy functions

sammachin commented 7 years ago

thinking a bit more about this I think just providing an optional --country_code flag on the number:buy and link:x functions would be the best option, we already have that flag on buy but it doesn't seem to skip the insight function at the moment

cbetta commented 7 years ago

@sammachin agreed, that seems to be the way to go.