feat: standardise phone number format while importing vCard

[Dagolin]

Fixes #981

Enhaced by https://github.com/giggsey/libphonenumber-for-php, while importing vCard check and format the phone number as international phone number format eg +44 117 496 0123 if the country of the contact can be recognized, the format of the number is open for discussion.

Since we need the country to tell which format the number should be, I left the phone number input field of contact (On CONTACT INFORMATION area) untouched because we have no country field for phone number, and it's awkward to add it.

[djaiss]

Thanks a lot for your effort. I've added a huge comment to refactor your code - I hope it's not too harsh. Thanks in advance for your help!

[djaiss]

• This whole new section should be split, otherwise it'll be very hard to unit test this method, considering the number of if involved.
• Ideally we should not have more than one if - here we have 3 nested ifs which is not good

You need to refactor this:

• create a new method in an helper (in LocaleHelper for instance), something like getCountryForVCard or something, that will return the country ISO based on the vcard you pass as parameter
• then create a new method in an helper (in LocaleHelper for instance), called formatTelephoneNumber or something, that will take both the tel number and the iso country and returns the formatted phone number (or null).
• Then, the entire new code becomes (pseudo code and might contain errors):

$tel = $this->currentEntry->TEL;

if (! is_null($this->formatValue($tel)) {
    $tel = (string) $tel;

    $isoCode = LocaleHelper::getCountryForVCard($this->currentEntry);

    $tel = LocaleHelper::formatTelephoneNumber($tel, $isoCode);

   // and that's it, you have your phone number 
}

Then, write the unit tests for the new methods inside the Helpers, and that's it.

The idea is that a method should ideally do just one thing, not multiple things as you've done, and it will be way easier to check if something is wrong.

[Dagolin]

@djaiss Thanks for the reminding, no prob!

[Dagolin]

@djaiss Thank you for all the pro detail tips and the great pseudo code, that's something I should be ready and have it done before I submit a PR, a lesson learned indeed.

I hope this did not bother you too much, thanks alot.

[djaiss]

Thanks a lot! It never bothers me when someone take his precious personal time to help on the project 😀

I'm going to review the PR very soon, thanks!

[sonarcloud]

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

[sonarcloud]

SonarQube analysis found issues:
Bugs: 0
Vulnerabilities: 0
Code Smells: 1

See all issues in SonarCloud

[djaiss]

Thanks again for your help @Dagolin

After 6 months (sigh), we've finally merged this PR.

Really sorry for the time it took to make it happen.

[github-actions]

This pull request has been automatically locked since there
has not been any recent activity after it was closed.
Please open a new issue for related bugs.