diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 88746375c67..f83f9691194 100644 --- a/app/models/application_setting.rb +++ b/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, diff --git a/app/models/email.rb b/app/models/email.rb index 3ce6e792fa8..7c33c5c7e64 100644 --- a/app/models/email.rb +++ b/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) } diff --git a/app/models/member.rb b/app/models/member.rb index 8e071a8ff21..5dbc0c2eec9 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -28,7 +28,7 @@ class Member < ActiveRecord::Base presence: { if: :invite? }, - email: { + devise_email: { allow_nil: true }, uniqueness: { diff --git a/app/models/user.rb b/app/models/user.rb index 9c091ac366c..14cedd9c4d1 100644 --- a/app/models/user.rb +++ b/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, diff --git a/app/validators/devise_email_validator.rb b/app/validators/devise_email_validator.rb new file mode 100644 index 00000000000..6ca921ca7fa --- /dev/null +++ b/app/validators/devise_email_validator.rb @@ -0,0 +1,36 @@ +# 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 diff --git a/app/validators/email_validator.rb b/app/validators/email_validator.rb deleted file mode 100644 index 9459edb7515..00000000000 --- a/app/validators/email_validator.rb +++ /dev/null @@ -1,7 +0,0 @@ -# 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 diff --git a/changelogs/unreleased/24971-align-emailvalidator-to-validate_email-gem-implementation.yml b/changelogs/unreleased/24971-align-emailvalidator-to-validate_email-gem-implementation.yml new file mode 100644 index 00000000000..04dbc3a1d5a --- /dev/null +++ b/changelogs/unreleased/24971-align-emailvalidator-to-validate_email-gem-implementation.yml @@ -0,0 +1,5 @@ +--- +title: Align EmailValidator to validate_email gem implementation +merge_request: 24971 +author: Horatiu Eugen Vlad +type: fixed diff --git a/spec/validators/devise_email_validator_spec.rb b/spec/validators/devise_email_validator_spec.rb new file mode 100644 index 00000000000..7860b659bd3 --- /dev/null +++ b/spec/validators/devise_email_validator_spec.rb @@ -0,0 +1,94 @@ +# 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