Fixes Email Validation #2506
Fixes Email Validation #2506
Conversation
Generated by 🚫 Danger |
|
@SidharthBansal Please merge it and thanks for all the help. |
|
@namanjain1812 can you please update regex to /\A[-[:alnum:]+.]+@[[:alnum:]-.]+[.][[:alpha:]]+\z/ as we discussed in #2169 so that it will include the non-latin characters? |
There was a problem hiding this comment.
Please change the regex as per discussion in #2169 to /\A[-[:alnum:]+.]+@[[:alnum:]-.]+[.][[:alpha:]]+\z/
Refer rubular for regex if needed
|
@SidharthBansal please review it.I have included 2 tests but i also tried adding a test including non-latin(chinese) words but Travis seems to not recognize them and hence gave checks failed message |
|
@namanjain1812 It looks great. |
test/unit/user_test.rb
Outdated
| user = User.new(username: 'zen', | ||
| password: 'nez', | ||
| password_confirmation: 'nez', | ||
| email: 'abc@xyz.com') |
There was a problem hiding this comment.
This email is valid and the user should be saved to the database.
What do you think @SidharthBansal?
There was a problem hiding this comment.
Sorry I missed, you need to write it should not save to the database.
@seafr thanks
|
@SidharthBansal I agree with you regarding the tests for emails with non-latin characters. This is a FTO issue, and we can leave such tests for a subsequent PR. |
test/unit/user_test.rb
Outdated
| @@ -182,7 +182,7 @@ class UserTest < ActiveSupport::TestCase | |||
| user = User.new(username: 'zen', | |||
| password: 'nez', | |||
| password_confirmation: 'nez', | |||
| email: 'abc@xyz.com') | |||
| email: 'abc@.com') | |||
|
Awesome -- is this good to go, then? Thanks, everybody!!! 👍 👍 |
|
Yes, I think so. The 'ready' label was removed due to that one test, which has since been fixed. |
|
Done, then!!! 🐯 👍 💯 |
|
Thanks everybody!!!! |
|
@namanjain1812 congrats on your first PR and welcome to the community! |
* Update user.rb * Update user.rb * Update user.rb * Update user.rb * Update user_test.rb * Update user_test.rb * Update user_test.rb * Update user_test.rb * Update user_test.rb * Update user_test.rb * Update user_test.rb * Update user_test.rb
Closes #1996
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake testfixes #0000-style reference to original issue #@publiclab/reviewersfor help, in a comment belowIf tests do fail, click on the red
Xto learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
We have a loose schedule of reviewing and pulling in changes every Tuesday and Friday, and publishing changes on Fridays.
Thanks!