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

Do not validate the length of email parts if ignore_max_length is true #1435

Merged
merged 2 commits into from
Sep 18, 2020

Conversation

evantahler
Copy link
Contributor

This PR skips the special validation of the domain and user if ignore_max_length is set to true. The README already implies this behavior is happening:

If ignore_max_length is set to true, the validator will not check for the standard max length of an email.

Closes #1434

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)

@codecov
Copy link

codecov bot commented Sep 13, 2020

Codecov Report

Merging #1435 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1435   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           95        95           
  Lines         1254      1254           
=========================================
  Hits          1254      1254           
Impacted Files Coverage Δ
src/lib/isEmail.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3f9d2b...1f97a1f. Read the comment docs.

Copy link
Member

@profnandaa profnandaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for your contrib! 🎉

/cc. @tux-tn @ezkemboi

Copy link
Member

@tux-tn tux-tn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉
EDIT: it's only skipping the validation for the user part, since we are using isFQDN internally and it's validating that the fqdn is less than 64 characters.

@evantahler
Copy link
Contributor Author

evantahler commented Sep 17, 2020

Thanks @tux-tn - Is there something you would like me to do/change? I don't quite follow you comment.

@profnandaa
Copy link
Member

@tux-tn -- are there any changes that need to be made as per your comment?

@tux-tn
Copy link
Member

tux-tn commented Sep 18, 2020

@profnandaa it depends, Should ignore_max_length also ignore domain length in email address or is it only used for the user part?

@profnandaa
Copy link
Member

@profnandaa -- maybe we just go with the userpart only, what do you think? @evantahler

@evantahler
Copy link
Contributor Author

I think so too - the test case I provided is the use case I have... so this meets my needs!

@profnandaa profnandaa merged commit 2a80c34 into validatorjs:master Sep 18, 2020
@evantahler
Copy link
Contributor Author

Thanks! Please let me know when the next version is released!

@profnandaa
Copy link
Member

Cool; and sorry you just missed the September release train by minutes, so this will be queued up for our next release 20/10.

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

Successfully merging this pull request may close these issues.

There should be a way to disable the "length" checks for isEmail
3 participants