Merge pull request #102 from mena-devs/user-email-validation

User email validation improvements

Author: constantine-nikolaou

app/models/user.rb

@@ -42,10 +42,7 @@ class User < ApplicationRecord
          :recoverable, :rememberable, :trackable, :validatable,
          :confirmable, :omniauthable, omniauth_providers: [:slack]
 
-  validates :email, uniqueness: true
-  validates :email, format: {
-                      with: /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i,
-                      on: :create }
+  validates :email, uniqueness: true, email: true
 
   before_validation :generate_unique_user_id, on: :create
 
@@ -61,9 +58,9 @@ class User < ApplicationRecord
 
   scope :news_subscribers, -> { joins(:profile).where('profiles.receive_emails IS TRUE') }
   scope :job_alert_subscribers, -> { joins(:profile).where('profiles.receive_job_alerts IS TRUE') }
-  
+
   scope :verified, -> { where("confirmed_at IS NOT NULL")}
-  
+
   scope :admins, -> { where("admin is TRUE")}
 
   scope :today, -> { where("created_at >= ?", 1.day.ago)}
@@ -189,7 +186,7 @@ def self.update_user_from_slack(user_info)
 
       return user.save
     end
-    
+
     return false
   end
 

app/validators/email_validator.rb

@@ -1,11 +1,34 @@
 class EmailValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
-    record.errors[attribute] << (options[:message] || "must be a valid email address") unless email_valid?(value)
+    record.errors[attribute] << email_invalid_error_message unless email_valid?(value)\
+                                                           && handle_contains_number_of_dots(value)\
+                                                           && handle_last_character_is_not_dot(value)
+  end
+
+  private
+
+  def email_invalid_error_message
+    # options[:message] || I18n.t('errors.messages.email')
+    (options[:message] || 'must be a valid email address')
   end
 
   def email_valid?(email_address)
     return true if email_address.blank?
 
     email_address =~ /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i
   end
+
+  def handle_contains_number_of_dots(email_address)
+    return true if email_address.blank?
+
+    email_handle = email_address.split('@')
+    email_handle[0].count('.') < 3 if email_handle && !email_handle.blank?
+  end
+
+  def handle_last_character_is_not_dot(email_address)
+    return true if email_address.blank?
+
+    email_handle = email_address.split('@')
+    email_handle[0][-1] != '.' if email_handle && !email_handle.blank?
+  end
 end

spec/models/user_spec.rb

@@ -50,7 +50,7 @@
     end
 
     it { should validate_uniqueness_of :email }
-    
+
     describe '#email' do
       it { should_not allow_value("blah").for(:email) }
       it { should allow_value("[email protected]").for(:email) }

spec/requests/registrations_spec.rb

@@ -27,8 +27,12 @@
       end
     end
 
-    describe "Fails" do
-      it "should fail registration because email already exists" do
+    describe "Fails registration" do
+      before(:each) do
+        visit new_user_registration_path
+      end
+
+      it "email already exists" do
         fill_in 'Email', with: existing_user.email
         fill_in 'Password',  with: 'password'
         fill_in 'Password confirmation',  with: 'password'
@@ -39,10 +43,10 @@
         expect(page).to have_content("Email has already been taken")
       end
 
-      it "should fail registration because passwords do not match" do
+      it "passwords do not match" do
         fill_in 'Email', with: '[email protected]'
-        fill_in 'Password',  with: 'password'
-        fill_in 'Password confirmation',  with: 'passwordz'
+        fill_in 'Password', with: 'password'
+        fill_in 'Password confirmation', with: 'passwordz'
 
         click_on('Join us')
 
@@ -51,15 +55,37 @@
       end
 
 
-      it "should fail registration because email address is incorrect" do
+      it "email address is incorrect" do
         fill_in 'Email', with: 'mars-example.com'
-        fill_in 'Password',  with: 'password'
-        fill_in 'Password confirmation',  with: 'password'
+        fill_in 'Password', with: 'password'
+        fill_in 'Password confirmation', with: 'password'
+
+        click_on('Join us')
+
+        expect(page).to have_content("2 errors prohibited this user from being saved")
+        expect(page).to have_content("must be a valid email address")
+      end
+
+      it "email address contains uncommon number of characters" do
+        fill_in 'Email', with: '[email protected]'
+        fill_in 'Password', with: 'password'
+        fill_in 'Password confirmation', with: 'password'
+
+        click_on('Join us')
+
+        expect(page).to have_content("1 error prohibited this user from being saved")
+        expect(page).to have_content("must be a valid email address")
+      end
+
+      it "email address handle ends with a dot" do
+        fill_in 'Email', with: '[email protected]'
+        fill_in 'Password', with: 'password'
+        fill_in 'Password confirmation', with: 'password'
 
         click_on('Join us')
 
         expect(page).to have_content("1 error prohibited this user from being saved")
-        expect(page).to have_content("Email is invalid")
+        expect(page).to have_content("must be a valid email address")
       end
     end
   end

spec/requests/users_spec.rb

@@ -37,7 +37,7 @@
       expect(page).to have_content('Password')
     end
   end
-  
+
   # /users/password/new
   describe "GET /users/password/new" do
     before do
@@ -70,7 +70,7 @@
   describe "GET /users/password/edit" do
     it "should not allow reset of password if the password is not matching" do
       visit edit_user_password_path(reset_password_token: user.reset_password_token)
-      
+
       expect(page).to have_content('Change your password')
       expect(page).to have_content('Type your new password')
       expect(page).to have_content('Password')
@@ -84,7 +84,7 @@
 
     it "should not allow reset password if the token is not valid" do
       visit edit_user_password_path(reset_password_token: user.reset_password_token)
-      
+
       expect(page).to have_content('Change your password')
       expect(page).to have_content('Type your new password')
       expect(page).to have_content('Password')
@@ -105,7 +105,7 @@
       user.save
 
       visit edit_user_password_path(reset_password_token: reset_password_token)
-      
+
       expect(page).to have_content('Change your password')
       expect(page).to have_content('Type your new password')
       expect(page).to have_content('Password')
@@ -115,42 +115,12 @@
       fill_in 'Password confirmation', with: 'OpenSource'
 
       click_on('Change my password')
-      
+
       expect(page).to have_content("We are MENA Devs")
       expect(page).to have_content("one of the largest active online communities in the MENA region")
     end
   end
 
-  describe "GET /users/sign_in" do
-    before do
-      visit new_user_session_path
-
-      expect(page).to have_content('Login with your email and password')
-      expect(page).to have_content('Email')
-      expect(page).to have_content('Password')
-    end
-
-    it "should login successfully using email and password" do
-      fill_in 'Email', with: user.email
-      fill_in 'Password',  with: 'password'
-
-      click_on('Log in')
-
-      expect(page).to have_content("My Account")
-      expect(page).to have_content("Logout")
-    end
-
-    it "should not allow login if email does not exist" do
-      fill_in 'Email', with: '[email protected]'
-      fill_in 'Password',  with: 'password'
-
-      click_on('Log in')
-
-      expect(page).to have_content('Email')
-      expect(page).to have_content('Password')
-    end
-  end
-
   describe "GET /directory/users" do
     before do
       create(:profile, user: open_profile_member, privacy_level: 2)

spec/validators/email_validator_spec.rb

@@ -0,0 +1,25 @@
+require 'rails_helper'
+
+RSpec.describe EmailValidator, type: :model do
+  describe "Email validations" do
+    let(:user) { create(:user) }
+
+    it 'email_valid?' do
+      user.email = 'random-email'
+
+      expect(user.valid?).to be_falsey
+    end
+
+    it 'handle_contains_number_of_dots' do
+      user.email = '[email protected]'
+
+      expect(user.valid?).to be_falsey
+    end
+
+    it 'handle_last_character_is_not_dot' do
+      user.email = '[email protected]'
+
+      expect(user.valid?).to be_falsey
+    end
+  end
+end