Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.4k
fix(isEmail): replace all dots in gmail length validation#1718
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
fix(isEmail): replace all dots in gmail length validation#1718
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedAug 30, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #1718 +/- ##========================================= Coverage 100.00% 100.00% ========================================= Files 101 101 Lines 2005 2005 Branches 452 452 ========================================= Hits 2005 2005
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.
Could you add test cases?
d0aa34a to89f02c2Compare
modified existing test case. |
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 🎉
Nice catch!
| 'test|123@m端ller.com', | ||
| 'test123+ext@gmail.com', | ||
| 'some.name.midd.leNa.me+extension@GoogleMail.com', | ||
| 'some.name.midd.leNa.me.and.locality+extension@GoogleMail.com', |
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.
Why it was passing the test before? Is the GoogleMail domain part of gmail?
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.
Max length is supposed to be 30 characters without the dots and the email alias (any string after the+ character) . Even after removing only one dot the old string length was less than 30
03-65 commentedSep 1, 2021
I have no idea how to code |
03-65 commentedSep 1, 2021
I don't know what you mean and I'm unable to Upload files it tells me |
03-65 commentedSep 9, 2021
Thank you |
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.
Good catch, thanks! 🎉
Uh oh!
There was an error while loading.Please reload this page.
The gmail domain specific validator checks the length of the username part and replaces dots as they don't count against the limit. But the replace function only replaced the first dot. Fixed by using regex with g switch.
Checklist