daddyz / phonelib

Ruby gem for phone validation and formatting using google libphonenumber library data
MIT License
1.08k stars 131 forks source link

`Phonelib.parse('651 123 4567x1234', 'US')` gives `nil` as the country code #295

Closed h3xx closed 8 months ago

h3xx commented 10 months ago

Before you mention it, yes I know about the configuration option that lets you use ; as the extension separator. That's not what's causing the issue in this case.

I also know I can work around this by pre-formatting the number I give phonelib. But the point is I shouldn't have to. There's enough information already given to make a reasonable assumption about the phone number's country code and Do What I Mean.

Issue 0 - country_code is nil

libphonenumber reports the correct country code:

{"country_code":1,"national_number":6511234567,"extension":"1234","raw_input":"651 123 4567x1234","country_code_source":20}

Whereas this library:

Phonelib.parse('651 123 4567x1234', 'US').country_code
=> nil

In case you point out that that libphonenumber detected the extension because it detected the x (different issue), even when I change x to , libphonenumber still gets the country code right:

{"country_code":1,"national_number":65112345671234,"raw_input":"651 123 4567 1234","country_code_source":20}

Phonelib.parse('651 123 4567 1234', 'US').country_code
=> nil

Issue 1 - full_e164 gives garbage data, setting the country code to +6.

Phonelib.parse('651 123 4567x1234', 'US').full_e164
=> "+65112345671234"

And without x:

Phonelib.parse('651 123 4567 1234', 'US').full_e164
=> "+65112345671234"
libphonenumber parsing report (~24 lines) >****Parsing Result:**** >{"country_code":1,"national_number":6511234567,"extension":"1234","raw_input":"651 123 4567x1234","country_code_source":20} > >****Validation Results:**** >Result from isPossibleNumber(): true >Result from isValidNumber(): false >Phone Number region: null >Result from getNumberType(): UNKNOWN > >****ShortNumberInfo Results:**** >Result from isPossibleShortNumber: false >Result from isValidShortNumber: false >Result from isPossibleShortNumberForRegion: false >Result from isValidShortNumberForRegion: false > >****Formatting Results:**** >E164 format: invalid >Original format: (651) 123-4567 ext. 1234 >National format: (651) 123-4567 ext. 1234 >International format: invalid >Out-of-country format from US: invalid >Out-of-country format from Switzerland: invalid >Format for mobile dialing (calling from US): invalid >Format for national dialing with preferred carrier code and empty fallback carrier code: invalid

Phonelib version: 0.8.7

daddyz commented 8 months ago

@h3xx the point is that gem will return country only when number is valid or possible. So when you specify extension separator properly and provide a valid number, you will get the country code. The idea behind this is to provide a country code only when gem is sure about the country. But when you specify '651 123 4567 1234' as a number, it is not possible, so no country code

Phonelib.extension_separate_symbols = 'x'
Phonelib.parse('651 123 4567x1234', 'US').country_code # => "1"
Phonelib.parse('651 123 4567x1234', 'US').possible? # => true
Phonelib.parse('651 123 4567x1234', 'US').valid? # => false

Phonelib.parse('651 123 4567 1234', 'US').country_code # => nil
Phonelib.parse('651 123 4567 1234', 'US').possible? # => false
Phonelib.parse('651 123 4567 1234', 'US').valid? # => false
h3xx commented 8 months ago

It's a bit disappointing that there were TWO issues in my initial comment, only one of which was addressed before it was closed.

It's a bit disappointing that this gem could be made more user-friendly but isn't.

It also could be made to more closely follow what phonelib does. The path of least surprise would be to document why the behavior differers from phonelib.