Skip to content
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 phone number regex validator. #249

Conversation

AlecsGherghel
Copy link
Collaborator

What does it fix?

Closes #218

Added a new validator for phone field from NGO model.

@AlecsGherghel
Copy link
Collaborator Author

@catileptic Do we want to use the same validation logic for all phone fields?

I see that we also have:

  • PaymentOrder.phone
  • NGOHelper.phone
  • PersonalRequest.phone
  • RegisterNGORequest.phone

@catileptic
Copy link
Member

Let's not merge this yet. The regexp doesn't support numbers that start with anything other than +4 (or doesn't have the country code).

@aramboi
Copy link
Member

aramboi commented May 24, 2020

Rather than adding our custom regex in the models, maybe it would be better to use a Django app like https://github.com/stefanfoulis/django-phonenumber-field that lets you choose from one of the standardized formats. Underneath it relies on a python port of the Google phone number library used in Android and it has support for national and international number formatting. Example from the readme:

>>> phonenumbers.format_number(x, phonenumbers.PhoneNumberFormat.NATIONAL)
'020 8366 1177'
>>> phonenumbers.format_number(x, phonenumbers.PhoneNumberFormat.INTERNATIONAL)
'+44 20 8366 1177'
>>> phonenumbers.format_number(x, phonenumbers.PhoneNumberFormat.E164)
'+442083661177'

@AlecsGherghel
Copy link
Collaborator Author

@aramboi That looks good! At the time of creating this PR I was looking for a quick turnaround without adding a new project dependency.

But now that the project is doing well live, I think that relying on existing libraries sounds like a great idea :)

I'll be closing this in the meantime and when there's a decision around the path going forward I'll either re-open a different one or maybe someone else will do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Phone field accepts invalid phone number formats
3 participants