Update google-libphonenumber to support new Nepali numbers

medic / cht-core

The CHT Core Framework makes it faster to build responsive, offline-first digital health apps that equip health workers to provide better care in their communities. It is a central resource of the Community Health Toolkit.

https://communityhealthtoolkit.org

GNU Affero General Public License v3.0

468 stars 217 forks source link

Update google-libphonenumber to support new Nepali numbers #7621

Closed mrjones-plip closed 2 years ago

mrjones-plip commented 2 years ago

Describe the issue Some deployments in Nepal need to accept newer numbers:

Nepal telecom has started to convert the CDMA SIM into GSM SIM, the range of the newly changed SIM starts from the +977976XXXXXXX but the web app has not accepted this range of the mobile numbers

Describe the improvement you'd like Update the ruimarinho/google-libphonenumber dependency once it has pulled in the upstream change (v8.12.48) from google/libphonenumber which has the support we need.

Describe alternatives you've considered Manually pushing in the numbers with cht-conf

mrjones-plip commented 2 years ago

Note - we bump dependencies every release, so this maybe super easy and done by default, but adding a ticket to track for visibility to other teammates. If this lib gets bumped in 4.0.0, but it doesn't include the upstream fix from google/libphonenumber, please push this out to 4.1.0

mrjones-plip commented 2 years ago

Ah - great news! There's already a PR to bring in the change from upstream! Once this closes and they cut a release, we can in turn bump our dependencies to pull in the change.

binokaryg commented 2 years ago

The previous PR has been superseded by ruimarinho/google-libphonenumber#334 which has already been merged.

mrjones-plip commented 2 years ago

Thanks Binod! Now we just need to do the dependency update on our side.

mrjones-plip commented 2 years ago

@dianabarsan - I see package.json hasn't been updated for 4.0.0. Do you know if we could expect this ticket's dependency (google-libphonenumber) would get updated in 4.0.0 or if would be for 4.1.0's dependency update?

dianabarsan commented 2 years ago

It will be updated for 4.0.

mrjones-plip commented 2 years ago

Wonderful - thanks for the update!

ngaruko commented 2 years ago

@mrjones-plip @binokaryg . If I have understood, this issue has been fixed and the ticket could be closed?

mrjones-plip commented 2 years ago

I don't think so? I was very much hoping to get this fixed in 3.16, but I don't see a PR attached to this ticket.

I'll defer to @njogz , release manager for 3.16: we should either fix and release in 3.16 or move to 4.0

njogz commented 2 years ago

@mrjones-plip @ngaruko this never got picked up and I had filed it under nice to have in 3.16 and since we are not updating other packges I think it's best if we move this to 4.0.

binokaryg commented 2 years ago

We are using a workaround (only checking phone number length) so that we can add the new phone numbers. It can take a long time to take the multiple instances to 4.0. It would be great if we could support it earlier.

mrjones-plip commented 2 years ago

@binokaryg - @njogz confirmed:

reviewing if the update google-libphonenumber ticket should be in 3.16

And I see a draft PR was just opened. Thanks @njogz !

binokaryg commented 2 years ago

AT passed:

mrjones-plip commented 2 years ago

Thanks so much @binokaryg! Really great to see it working with in a dev deployment with the specific config and a Nepali mHealth validated number :heart_eyes:

tatilepizs commented 2 years ago

Config: Default Environment: Local with docker helper script Platform: WebApp  Browser: Chrome

Test scenario: Same test that @binokaryg did but using Default config. Everything looks good.

Reproducible on Master

Fixed on 7621-update-google-libphonenumber

mrjones-plip commented 2 years ago

Closing this assuming the PR being merged and 3.16 just passed RT. Please re-open if this isn't correct!