panvol / pandemic-volunteers

❤️ Pandemic Volunteers | ⚠️ Help Wanted
https://pandemicvolunteers.org
MIT License
29 stars 7 forks source link

Feature #23 Remove regex phone nr #29

Closed ewynx closed 4 years ago

ewynx commented 4 years ago

Issue #23

house92 commented 4 years ago

We wanted to remove the regex requirement for the phones since the regex I wrote didn't cover some generated phone numbers during testing.

We could discuss phone requirements on the issue: #23

On the other hand, this PR also remove phone number requirements from the form validation. But phone is still needed in the DB as I think it should be.

@hantuzun The form validation may be faulty. There can be local numbers which are shorter than 7 digits, so all we can really do is assert that any number has been entered, which is as good as pointless if a user can simply enter "0" to bypass the validation. I'd suggest that this form validation be removed, as has been done in this PR, and we look at a solution like https://numverify.com/documentation if we really want to verify numbers properly

kopchickm commented 4 years ago

I agree with @house92 about dropping the validation entirely. I'd rather be overly permissive.

hantuzun commented 4 years ago

Hi @house92.

I agree with you, let's drop the validation. Do you think that should we "require" phone number? It's not required in the PR on the frontend.

I think if somebody doesn't dare to share their number, they'd be less likely to actually do volunteering.

BTW, thanks for sharing https://numverify.com/documentation -- good to know.

hantuzun commented 4 years ago

This has been handled in another PR instead: https://github.com/panvol/pandemic-volunteers/pull/39