Issue #46 (Wrong result from local number) fix

[aazhevsky]

Hello,

These changes will fix the Wrong result obtained from local number #46 issue.

After applying the changes, the following local US phone numbers are parsed successfully:

(888) 555-5512
(408) 555-5270
(707) 555-1854

All the interfaces are kept, as well as all tests passed. The major problem is that the code of the fix is a bit clumsy. At best, CountryCodeParser.extractCountryCode should return an optional, as PhoneParser._findDestinationMetadata itself.

I consciously stopped on as it is, not to spread the changes over the whole project.

[cedvdb]

Hey, I will take a look this weekend. I agree it is a bit clumsy.

[aazhevsky]

Thanks a lot!

Thank you for your efforts!

Could you also add an entry to the changelog?

Please clarify what I have to do.

[cedvdb]

Hello again, sorry for the back and forth, I somewhat misunderstood your issue at first. I believe the part of the code that should be fixed for your actual issue is not exactly the part that you fixed. Correct me if I'm wrong:

This was your issue:

        expect(
            PhoneNumber.parse('(707) 555-1854', callerCountry: IsoCode.US)
              .international,
            equals('+17075551854'));

The actual culprit is this line https://github.com/cedvdb/phone_numbers_parser/blob/dev/lib/src/parsers/phone_parser.dart#L102

    if (callerMetadata != null &&
        callerNationalPrefix != null &&
        phoneWithoutExitCode.startsWith(callerNationalPrefix)) {   // will be false because it is **local** (same area in us)
      return callerMetadata;
    }

where you'd want to enter this condition. You do not enter this because the phone number is in local form. Therefor the condition: phoneWithoutExitCode.startsWith(callerNationalPrefix)) is wrong or incomplete. It should probably be phoneWithoutExitCode.startsWith(callerNationalPrefix)) || checkThatPhoneNumberIsValidForCallerMetadata())

Am I wrong that this would be the only change required to fix your issue ?

[aazhevsky]

Hello again, sorry for the back and forth, I somewhat misunderstood your issue at first. I believe the part of the code that should be fixed for your actual issue is not exactly the part that you fixed. Correct me if I'm wrong:

This was your issue:

        expect(
            PhoneNumber.parse('(707) 555-1854', callerCountry: IsoCode.US)
              .international,
            equals('+17075551854'));

The actual culprit is this line https://github.com/cedvdb/phone_numbers_parser/blob/dev/lib/src/parsers/phone_parser.dart#L102

    if (callerMetadata != null &&
        callerNationalPrefix != null &&
        phoneWithoutExitCode.startsWith(callerNationalPrefix)) {   // will be false because it is **local** (same area in us)
      return callerMetadata;
    }

where you'd want to enter this condition. You do not enter this because the phone number is in local form. Therefor the condition: phoneWithoutExitCode.startsWith(callerNationalPrefix)) is wrong or incomplete. It should probably be phoneWithoutExitCode.startsWith(callerNationalPrefix)) || checkThatPhoneNumberIsValidForCallerMetadata())

Am I wrong that this would be the only change required to fix your issue ?

Potentially, yes, but when I replace the block

    if (callerMetadata != null &&
        callerNationalPrefix != null &&
        phoneWithoutExitCode.startsWith(callerNationalPrefix)) {   // will be false because it is **local** (same area in us)
      return callerMetadata;
    }

with

    if (callerMetadata != null &&        
        ((callerNationalPrefix != null && phoneWithoutExitCode.startsWith(callerNationalPrefix)) || 
        Validator.validateWithPattern(PhoneNumber(nsn: phoneWithoutExitCode, isoCode: callerMetadata.isoCode)))) {
      return callerMetadata;
    }

in the initial version of the library, the tests fail on

expect(
            PhoneNumber.parse('+33 655 5705 76', callerCountry: IsoCode.DE)
                .international,
            equals('+33655570576'));

However, the tests I added perform well. I think an exit code check should be mandatory in this method.

[cedvdb]

I've been away the past few days. I'll have a look into this tomorrow to see what I can do.

[cedvdb]

I refactored the code with some of your input in PR https://github.com/cedvdb/phone_numbers_parser/pull/50

I fixed the issue in PR https://github.com/cedvdb/phone_numbers_parser/pull/51

Thanks a lot for the help in investigating this

[aazhevsky]

I see it was a complex solution. Many thanks for your efforts!

[cedvdb]

@aazhevsky the solution itself was 3 lines (there is a function rename that should not be part of the PR), the refactor took a bit longer