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

Use validates_email_format_of gem in EmailValidator #1965

Closed
wants to merge 2 commits into from
Closed

Use validates_email_format_of gem in EmailValidator #1965

wants to merge 2 commits into from

Conversation

dividedharmony
Copy link
Contributor

This PR replaces a simplistic regular expression that was flagging invalid emails as valid. Instead, we will now use the validates_email_format_of gem to provide more robust email validation. This gem has a superior implementation and is more thoroughly tested.

This PR resolves #1794

Validating email addresses is deceptively complicated. Rather than do
it entirely ourselves, we can use this 'validates_email_format_of' gem
which closely follows widely-accepted standards and has a strong
test suite.
This commit replaces a simplistic regular expression that was flagging
invalid emails as valid. Instead, we will now use ValidateEmailFormatOf
to provide more robust email validation.

See issue #1794 for details
@tvdeyen tvdeyen self-assigned this May 23, 2017
@tvdeyen tvdeyen removed their assignment May 23, 2017
@mamhoff
Copy link
Contributor

mamhoff commented May 23, 2017

I don't think we should add another dependency to core for this task. Most authentication solutions come with email validation of sorts, and the needs people have for this vary wildly.

I specifically didn't add the Hack Day label to this one because it's much harder than it initally seems...

@mamhoff
Copy link
Contributor

mamhoff commented May 31, 2017

We discussed this in the core team, and we all think we should not try handling email validation in core. Closing this - thank you for the effort though!

@mamhoff mamhoff closed this May 31, 2017
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.

Replace E-mail Validator
3 participants