-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Prevent isMobilePhone from allowing landline numbers in es-CO #1623
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1623 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 100 100
Lines 1807 1807
=========================================
Hits 1807 1807
Continue to review full report at Codecov.
|
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.
Solves main problem completely (no landlines),
though still passing some inexisting carriers/operators (353 ... 359),
below is an ugly, but efficient regex i modified based on yours,
/^(\+?57)?(300|301|302|304|305|310|311|312|313|314|315|316|317|318|319|320|321|322|323|324|350|351)\d{7}$/
notice i removed the '03' from the first capture group as it's only the way people dials from a landline to mobile (works in colombia only, not really part of the mobile number,
+57 country code is ok though, as it works worldwide)
i approve it since the main issue is solved,
yet if we want to be strict and not allow inexisting operators, that's my ugly proposal as i'm not that good with regex (dont know if it has some kind of conditionals)
Thank you very much.
@jhoalx Thank you for the explanation and the feedback, i changed the regex to reflect the edge cases you mentioned |
Fixes #1622
The regex for es-Co mobile phone validation updated in #1541 is allowing landline numbers. The solo purpose of this PR is to allow only mobile phone numbers to be validated
Checklist