Argentina(AR) number formatting and validation errors #24

byungjikroh · 2018-09-19T06:31:16Z

I've been using this library for a few years, but there are errors to format and validate Argentina numbers.

A formatted national number of AR by the library is invalid and not formatted again properly.
The only difference of result is 'italian_leading_zero: true' at second formatting.

Could you check it? Thanks. :)

Initial formatting

iex> {:ok, phone_number} = ExPhoneNumber.parse("+54 114321-1200", "AR")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}

iex> ExPhoneNumber.format(phone_number, :national)
"011 4321-1200"

iex> ExPhoneNumber.is_valid_number?(phone_number)
true

Second formatting

iex> {:ok, phone_number} = ExPhoneNumber.parse("011 4321-1200", "AR")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: true,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}

iex> ExPhoneNumber.format(phone_number, :national)
"01143211200"

iex> ExPhoneNumber.is_valid_number?(phone_number)
false

The text was updated successfully, but these errors were encountered:

szymon-jez · 2018-09-19T10:21:28Z

This library is based on Google's libphonenumber (see PhoneNumberMetadata.xml). Could you check if it also contains this issue?
Our PhoneNumberMetadata.xml was not updated for a long time - maybe we need an update?.

byungjikroh · 2018-09-19T16:17:56Z

Google's libphonenumber library has no problem.

Java version(linked on libphonenumber) result with same AR phone number above.
https://libphonenumber.appspot.com/phonenumberparser?number=011%204321-1200&country=AR

It returns 'italian_leading_zero' false but ex_phone_number does not (It returns true).
I suppose this difference might cause an error.

I have been using Ruby version (https://github.com/daddyz/phonelib) and Golang version (https://github.com/nyaruka/phonenumbers) but they works fine with AR phone numbers.

After comparing between two PhoneNumberMetadata.xml, there are quite some differences but I can't figure out which codes cause errors.

szymon-jez · 2018-09-20T07:34:55Z

Thanks for your research.
Does upgrading PhoneNumberMetadata.xml (replacing the current one with the one I linked from Google) fix the issues you are having? Could you then create a PR that would include tests that show if your issue is present and upgrade that XML file?

byungjikroh · 2018-09-20T08:50:15Z

I'm not sure I've done right but replacing current PhoneNumberMetadata.xml and PhoneNumberMetadataForTesting with Google's makes validation worse.

AR phone numbers are still returned invalid and other countries either.

iex(1)> {:ok, phone_number} = ExPhoneNumber.parse("+54 114321-1200", "AR")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(2)> ExPhoneNumber.format(phone_number, :national)
"011 4321-1200"
iex(3)> ExPhoneNumber.is_valid_number?(phone_number)
false

iex(4)> {:ok, phone_number} = ExPhoneNumber.parse("011 4321-1200", "AR")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: true,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(5)> ExPhoneNumber.format(phone_number, :national)
"01143211200"
iex(6)> ExPhoneNumber.is_valid_number?(phone_number)
false

iex(7)> {:ok, phone_number} = ExPhoneNumber.parse("555-555-5555", "US")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 1,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 5555555555,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(8)> ExPhoneNumber.format(phone_number, :national)
"(555) 555-5555"
iex(9)> ExPhoneNumber.is_valid_number?(phone_number)
false

Moreover, there is an exception error on a UK phone number like below.
The number is 01131688131 and it is a valid number.

 an exception was raised:
    ** (FunctionClauseError) no function clause matching in Regex.run/3
        (elixir) lib/regex.ex:279: Regex.run(nil, "1131688131", [return: :index])
        (ex_phone_number) lib/ex_phone_number/validation.ex:119: ExPhoneNumber.Validation.test_number_length_against_pattern/2
        (ex_phone_number) lib/ex_phone_number/validation.ex:87: ExPhoneNumber.Validation.is_shorter_than_possible_normal_number?/2
        (ex_phone_number) lib/ex_phone_number/parsing.ex:117: ExPhoneNumber.Parsing.parse_helper/4
...

I think that replacing raw Metadata can't solve the problem. It might require some refactoring work.

szymon-jez · 2018-09-20T08:58:37Z

I think that replacing raw Metadata can't solve the problem.

Yes, the data you provided above proves this.

It might require some refactoring work.

I think you mean re-engineering or fixing.
I also think that you seem well versed in this topic (you know how to test those matters) and because of this I think you are capable of doing the fix. Is that possible that you would find the time to design and implement a solution? I will be happy to review your work and help out.

byungjikroh · 2018-09-21T08:23:07Z

I thank you for suggesting but refactoring all country's code is out of my capability.

I'll try to fix AR phone numbers errors first and request to pull later. :)

szymon-jez · 2019-02-04T09:20:07Z

In #27 the XML Metadata was updated. Could you check if this resolves your issues?

szymon-jez · 2019-02-04T10:05:52Z

Now we have:

iex(3)> {:ok, phone_number} = ExPhoneNumber.parse("+54 114321-1200", "AR")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(4)> ExPhoneNumber.format(phone_number, :national)
"011 4321-1200"
iex(5)> ExPhoneNumber.is_valid_number?(phone_number) 
true
iex(6)> {:ok, phone_number} = ExPhoneNumber.parse("011 4321-1200", "AR")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: true,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(7)> ExPhoneNumber.format(phone_number, :national)
"01143211200"
iex(8)> ExPhoneNumber.is_valid_number?(phone_number)
false
iex(9)> {:ok, phone_number} = ExPhoneNumber.parse("555-555-5555", "US")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 1,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 5555555555,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(10)> ExPhoneNumber.format(phone_number, :national)
"(555) 555-5555"
iex(11)> ExPhoneNumber.is_valid_number?(phone_number)
false

iex(12)> {:ok, phone_number} = ExPhoneNumber.parse("01131688131", "GB")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 44,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 1131688131,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(13)> ExPhoneNumber.is_valid_number?(phone_number)                  
true

szymon-jez · 2019-02-04T10:08:12Z

And from your first post:

iex(4)> {:ok, phone_number} = ExPhoneNumber.parse("+54 114321-1200", "AR")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(5)> ExPhoneNumber.format(phone_number, :national)
"011 4321-1200"
iex(6)> ExPhoneNumber.is_valid_number?(phone_number)
true

iex(7)> {:ok, phone_number} = ExPhoneNumber.parse("011 4321-1200", "AR")  
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: true,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(8)> ExPhoneNumber.format(phone_number, :national)                   
"01143211200"
iex(9)> ExPhoneNumber.is_valid_number?(phone_number)                                                                                                                                                                  
false
``

byungjikroh · 2019-02-07T09:00:43Z

AR number still doesn't validate national formatted by itself like my original post.

iex> {:ok, phone_number} = ExPhoneNumber.parse("011 4321-1200", "AR")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: true,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}

iex> ExPhoneNumber.format(phone_number, :national)
"01143211200"

iex> ExPhoneNumber.is_valid_number?(phone_number)
false

But invalid GB number like 01131688131 is valid now.

szymon-jez · 2019-02-07T10:00:12Z

Thanks for checking. I now have also read it all and I confirm that we have still this bug here.

I thank you for suggesting but refactoring all country's code is out of my capability.

As we have recently updated the metadata (XML).

I'll try to fix AR phone numbers errors first and request to pull later. :)

Now it should be much easier for you to try fixing the part of the code that is causing this issues.

byungjikroh · 2019-02-07T11:25:55Z

Okay, I'll try checking AR phone number format code.
Thanks for updating metadata (XML). 😃

ideaMarcos · 2019-06-17T21:24:07Z

Was trying to see what was going on here so I was able to reproduce the original issue with iex. Then I added a test to validation_test.exs to investigate

    context "test AR number" do
      it "returns true" do
        assert {:ok, phone_number} =
                 ExPhoneNumber.parse("011 4321-1200", "AR")
                 |> IO.inspect(label: "phone_number")

        assert is_valid_number?(phone_number)
               |> IO.inspect(label: "is_valid_number?")
      end
    end

This test passes which is weird. I get a different result from parse() than in iex

phone_number: {:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 54,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 1143211200,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
is_valid_number?: true

Only difference is italian_leading_zero: nil. Not sure if I'm doing something wrong here.

szymon-jez · 2022-04-29T07:42:53Z

Closing for now as #43 may fix those issues. If after doing #43 those issues will still prevail we can reopen.

szymon-jez added the bug label Feb 4, 2019

byungjikroh closed this as completed Feb 7, 2019

byungjikroh reopened this Feb 7, 2019

szymon-jez closed this as completed Apr 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Argentina(AR) number formatting and validation errors #24

Argentina(AR) number formatting and validation errors #24

byungjikroh commented Sep 19, 2018 •

edited

Loading

szymon-jez commented Sep 19, 2018

byungjikroh commented Sep 19, 2018 •

edited

Loading

szymon-jez commented Sep 20, 2018 •

edited

Loading

byungjikroh commented Sep 20, 2018

szymon-jez commented Sep 20, 2018

byungjikroh commented Sep 21, 2018

szymon-jez commented Feb 4, 2019

szymon-jez commented Feb 4, 2019

szymon-jez commented Feb 4, 2019 •

edited

Loading

byungjikroh commented Feb 7, 2019 •

edited

Loading

szymon-jez commented Feb 7, 2019 •

edited

Loading

byungjikroh commented Feb 7, 2019

ideaMarcos commented Jun 17, 2019

szymon-jez commented Apr 29, 2022

Argentina(AR) number formatting and validation errors #24

Argentina(AR) number formatting and validation errors #24

Comments

byungjikroh commented Sep 19, 2018 • edited Loading

szymon-jez commented Sep 19, 2018

byungjikroh commented Sep 19, 2018 • edited Loading

szymon-jez commented Sep 20, 2018 • edited Loading

byungjikroh commented Sep 20, 2018

szymon-jez commented Sep 20, 2018

byungjikroh commented Sep 21, 2018

szymon-jez commented Feb 4, 2019

szymon-jez commented Feb 4, 2019

szymon-jez commented Feb 4, 2019 • edited Loading

byungjikroh commented Feb 7, 2019 • edited Loading

szymon-jez commented Feb 7, 2019 • edited Loading

byungjikroh commented Feb 7, 2019

ideaMarcos commented Jun 17, 2019

szymon-jez commented Apr 29, 2022

byungjikroh commented Sep 19, 2018 •

edited

Loading

byungjikroh commented Sep 19, 2018 •

edited

Loading

szymon-jez commented Sep 20, 2018 •

edited

Loading

szymon-jez commented Feb 4, 2019 •

edited

Loading

byungjikroh commented Feb 7, 2019 •

edited

Loading

szymon-jez commented Feb 7, 2019 •

edited

Loading