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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate :email in User model using email_validator gem #831

Closed
wants to merge 3 commits into from

Conversation

pdostal
Copy link

@pdostal pdostal commented Dec 21, 2014

#747: I planned to validate :email using some regex expression, but as you can see here, it's not so easy - so I used email_validator gem - Is it OK? 馃槃 馃摟 馃巹

@colindean
Copy link
Contributor

Can you run the tests locally? There are several failures that look like they are related to emails not validating when maybe they should. Also possible that those tests need to be updated with a valid email.

@pdostal
Copy link
Author

pdostal commented Dec 22, 2014

Sorry, these tests are too complicated for me. Can you point me what I have to exactly do?
It only validatites :email in User model, so it should be accessible even in tests.
In fabricators are valid addresses so I really don't know where is problem :/

@wilkie
Copy link
Contributor

wilkie commented Dec 22, 2014

We don't require an email address. So it fails to validate for any test that doesn't provide one. You have to apply a validation that allows email to be blank:

validates :email, :email => true, :allow_blank => true

After this, one test now errors because the validator finds "new_email@new_email.com" to be invalid (probably due to domain rules about underscores). So, we just need to fix the fake email address in that test to just "new_email@newemail.com"

After that, they should all pass! :)

@pdostal
Copy link
Author

pdostal commented Dec 22, 2014

馃槷 now it makes sense! Thanks you @wilkie 馃憤

I'll watch and try to solve something by myself :)

@pdostal pdostal closed this May 11, 2018
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.

None yet

3 participants