Automatic merge of gitlab-org/gitlab-ce master

app/models/application_setting.rb

@@ -69,7 +69,7 @@ class ApplicationSetting < ActiveRecord::Base
             url: true
 
   validates :admin_notification_email,
-            email: true,
+            devise_email: true,
             allow_blank: true
 
   validates :two_factor_grace_period,

app/models/email.rb

@@ -7,7 +7,7 @@ class Email < ActiveRecord::Base
   belongs_to :user
 
   validates :user_id, presence: true
-  validates :email, presence: true, uniqueness: true, email: true
+  validates :email, presence: true, uniqueness: true, devise_email: true
   validate :unique_email, if: ->(email) { email.email_changed? }
 
   scope :confirmed, -> { where.not(confirmed_at: nil) }

app/models/member.rb

@@ -28,7 +28,7 @@ class Member < ActiveRecord::Base
     presence: {
       if: :invite?
     },
-    email: {
+    devise_email: {
       allow_nil: true
     },
     uniqueness: {

app/models/user.rb

@@ -162,9 +162,9 @@ class User < ApplicationRecord
   validates :name, presence: true
   validates :email, confirmation: true
   validates :notification_email, presence: true
-  validates :notification_email, email: true, if: ->(user) { user.notification_email != user.email }
-  validates :public_email, presence: true, uniqueness: true, email: true, allow_blank: true
-  validates :commit_email, email: true, allow_nil: true, if: ->(user) { user.commit_email != user.email }
+  validates :notification_email, devise_email: true, if: ->(user) { user.notification_email != user.email }
+  validates :public_email, presence: true, uniqueness: true, devise_email: true, allow_blank: true
+  validates :commit_email, devise_email: true, allow_nil: true, if: ->(user) { user.commit_email != user.email }
   validates :bio, length: { maximum: 255 }, allow_blank: true
   validates :projects_limit,
     presence: true,

app/validators/devise_email_validator.rb

+# frozen_string_literal: true
+
+# DeviseEmailValidator
+#
+# Custom validator for email formats. It asserts that there are no
+# @ symbols or whitespaces in either the localpart or the domain, and that
+# there is a single @ symbol separating the localpart and the domain.
+#
+# The available options are:
+# - regexp: Email regular expression used to validate email formats as instance of Regexp class.
+#           If provided value has different type then a new Rexexp class instance is created using the value.
+#           Default: +Devise.email_regexp+
+#
+# Example:
+#   class User < ActiveRecord::Base
+#     validates :personal_email, devise_email: true
+#
+#     validates :public_email, devise_email: { regexp: Devise.email_regexp }
+#   end
+class DeviseEmailValidator < ActiveModel::EachValidator
+  DEFAULT_OPTIONS = {
+    regexp: Devise.email_regexp
+  }.freeze
+
+  def initialize(options)
+    options.reverse_merge!(DEFAULT_OPTIONS)
+
+    raise ArgumentError, "Expected 'regexp' argument of type class Regexp" unless options[:regexp].is_a?(Regexp)
+
+    super(options)
+  end
+
+  def validate_each(record, attribute, value)
+    record.errors.add(attribute, :invalid) unless value =~ options[:regexp]
+  end
+end

app/validators/email_validator.rb

-# frozen_string_literal: true
-
-class EmailValidator < ActiveModel::EachValidator
-  def validate_each(record, attribute, value)
-    record.errors.add(attribute, :invalid) unless value =~ Devise.email_regexp
-  end
-end

changelogs/unreleased/24971-align-emailvalidator-to-validate_email-gem-implementation.yml

+---
+title: Align EmailValidator to validate_email gem implementation
+merge_request: 24971
+author: Horatiu Eugen Vlad
+type: fixed

spec/validators/devise_email_validator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DeviseEmailValidator do
+  let!(:user) { build(:user, public_email: 'test@example.com') }
+  subject { validator.validate(user) }
+
+  describe 'validations' do
+    context 'by default' do
+      let(:validator) { described_class.new(attributes: [:public_email]) }
+
+      it 'allows when email is valid' do
+        subject
+
+        expect(user.errors).to be_empty
+      end
+
+      it 'returns error when email is invalid' do
+        user.public_email = 'invalid'
+
+        subject
+
+        expect(user.errors).to be_present
+        expect(user.errors.first[1]).to eq 'is invalid'
+      end
+
+      it 'returns error when email is nil' do
+        user.public_email = nil
+
+        subject
+
+        expect(user.errors).to be_present
+      end
+
+      it 'returns error when email is blank' do
+        user.public_email = ''
+
+        subject
+
+        expect(user.errors).to be_present
+        expect(user.errors.first[1]).to eq 'is invalid'
+      end
+    end
+  end
+
+  context 'when regexp is set as Regexp' do
+    let(:validator) { described_class.new(attributes: [:public_email], regexp: /[0-9]/) }
+
+    it 'allows when value match' do
+      user.public_email = '1'
+
+      subject
+
+      expect(user.errors).to be_empty
+    end
+
+    it 'returns error when value does not match' do
+      subject
+
+      expect(user.errors).to be_present
+    end
+  end
+
+  context 'when regexp is set as String' do
+    it 'raise argument error' do
+      expect { described_class.new( { regexp: 'something' } ) }.to raise_error ArgumentError
+    end
+  end
+
+  context 'when allow_nil is set to true' do
+    let(:validator) { described_class.new(attributes: [:public_email], allow_nil: true) }
+
+    it 'allows when email is nil' do
+      user.public_email = nil
+
+      subject
+
+      expect(user.errors).to be_empty
+    end
+  end
+
+  context 'when allow_blank is set to true' do
+    let(:validator) { described_class.new(attributes: [:public_email], allow_blank: true) }
+
+    it 'allows when email is blank' do
+      user.public_email = ''
+
+      subject
+
+      expect(user.errors).to be_empty
+    end
+  end
+end