Cht-core: Phone number validation is too permissive

[estellecomment]

Raised by Fred as priority for LG : https://github.com/medic/medic-projects/issues/146

The change to isPossibleNumber in https://github.com/medic/medic-webapp/commit/f8dd9f28517cce471d14b6de70feeacca286a2d5 is too permissive, it allows [phone].

But reverting to isValidNumber gets back to the original problem in https://github.com/medic/medic-webapp/issues/2105, it doesn't allow [phone] for instance.

[alxndrsn]

Likely caused by f8dd9f28517cce471d14b6de70feeacca286a2d5, which was a fix for #2105

[garethbowen]

As Alex notes in #2105 "Maybe I chose a bad example".

@estellecomment It would be good to check isValidNumber() against known numbers now that we've updated our libphonenumber. If the update fixed it we should switch back to using isValidNumber() instead of isPossibleNumber().

[estellecomment]

I just pulled the most recent develop and switched back to isValidNumber(), and [phone] is still invalid.

[alxndrsn]

We should probably be fixing for Uganda upstream, but it'll be a pain if this happens regularly.

[estellecomment]

@SCdF is right, let's just count digits. If we outsource phone validation to Google, but Google doesn't do it right, then let's do a simple "prefix + X digits" homemade thing, and that'll work. It won't be as precise but that's ok.

[alxndrsn]

How will you implement that? Needs to work in all countries we currently have projects, and kept up-to-date when we roll out in new countries. And potentially support any country when DIY is released. Strongly recommend fixing new Ugandan number formats upstream (and, short-term, with a monkey patch).

[estellecomment]

We can add phone format in project config, and we stick to that one format in the project.

[fredatthehub]

I may not understand the full implementation background for this but my 2-cents is that "prefix + X digits" should work. Most partners work with about 4 operators in a country, the key difference for each country would be the country code, but most prefixes may be repetitive. Since each partner will have there own instance setup, I believe this can be made part of the configuration.

[garethbowen]

My understanding is that libphonenumber is what Google uses on Android. It's updated frequently so as long as we keep up to date it should provide very accurate and strict validation without the need for configuration. To progress this issue I need a list of half a dozen valid phone numbers to test.

@fredatthehub Can you please PM me on Slack some phone numbers that should be valid and I'll get right on this.

[abbyad]

It seems as though the validation was never wrong (https://github.com/medic/medic-projects/issues/146#issuecomment-211373832), but that they wanted to future proof the app by accepting numbers that have yet to be assigned to a MNO.

I think we should return to using isValidNumber if it is acceptable to push an updated app whenever there are relevant libphonenumber updates.

[garethbowen]

Fixed and merged into testing to be released in 2.6.1

[sglangevin]

@garethbowen I PM-ed you some phone numbers for testing that were shared by LG. (cc @alxndrsn)

[garethbowen]

@sglangevin All the numbers you PMed me pass so we should be good to go!