-
Notifications
You must be signed in to change notification settings - Fork 574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for international phone numbers in volunteer model #846
base: master
Are you sure you want to change the base?
Conversation
Just made changes for the volunteer model for now. Will change rest on furthur discussion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Please verify this will NOT affect some of the dependencies on phone field which is already in production - once you can confirm on that, we can merge this to upstream.
One thing that can break with this commit is + infront of international numbers . We need to verify wether it breaks the calling or sms feature. @atmb4u |
As + was present in the previous regex. I dont think it will break the app. This should be good @atmb4u |
@Gotham13121997 I think this is a good idea in principle. Can you confirm what kinds of numbers are now valid/invalid? We had a nasty bug once that prevented data entry because of a wrong regex, just trying to understand what the change is. |
I have tested on indian numbers with 0 without 0 and with 91. and on international number that was mentioned in the issue. It seems legit. I have not done furthur checking on international numbers though. This was ported from libphonenumber by google so I trusted it should work |
It's allowing alphabets d+420602123456 is allowing successful registration |
Test against this list http://www.iaug.org/p/fo/et/thread=3486 |
@KManiKumarReddy value going into database is a number though. I think it removes the alphabets by itself |
Issue Reference
This PR addresses the Issue : Fixes #841
Summarize
Please, describe what the PR does, the changes you have made.