diff options
author | Horatiu Eugen Vlad <horatiu@vlad.eu> | 2018-10-27 00:39:00 +0200 |
---|---|---|
committer | Horatiu Eugen Vlad <horatiu@vlad.eu> | 2019-03-05 19:56:01 +0000 |
commit | c8c0ea6c52d46ce63d838d1e739355d4deace434 (patch) | |
tree | a91cbe57b1cc4235f3be6b32f2829e200c73de09 | |
parent | 60876192f89e41494cb56e2dc0a90f5f0dbfa716 (diff) | |
download | gitlab-ce-c8c0ea6c52d46ce63d838d1e739355d4deace434.tar.gz |
Align EmailValidator to validate_email gem implementation.
Renamed EmailValidator to DeviseEmailValidator to avoid 'email:' naming collision with ActiveModel::Validations::EmailValidator in 'validates' statement.
Make use of the options attribute of the parent class ActiveModel::EachValidator.
Add more options: regex.
-rw-r--r-- | app/models/application_setting.rb | 2 | ||||
-rw-r--r-- | app/models/email.rb | 2 | ||||
-rw-r--r-- | app/models/member.rb | 2 | ||||
-rw-r--r-- | app/models/user.rb | 6 | ||||
-rw-r--r-- | app/validators/devise_email_validator.rb | 36 | ||||
-rw-r--r-- | app/validators/email_validator.rb | 7 | ||||
-rw-r--r-- | changelogs/unreleased/24971-align-emailvalidator-to-validate_email-gem-implementation.yml | 5 | ||||
-rw-r--r-- | spec/validators/devise_email_validator_spec.rb | 94 |
8 files changed, 141 insertions, 13 deletions
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 |