From 69a53210d3d3ba53b19dc2cfd1a1de61fab65249 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 20 Mar 2018 10:22:06 +0800 Subject: [PATCH] Improve `UserEmail#email` validation to use the index. --- app/models/user_email.rb | 13 +++++++++---- spec/models/user_spec.rb | 13 +++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/models/user_email.rb b/app/models/user_email.rb index 1b533410319..101eddcd5c5 100644 --- a/app/models/user_email.rb +++ b/app/models/user_email.rb @@ -7,14 +7,13 @@ class UserEmail < ActiveRecord::Base before_validation :strip_downcase_email - validates :email, presence: true, uniqueness: true - + validates :email, presence: true validates :email, email: true, format: { with: EmailValidator.email_regex }, if: :validate_email? + validates :primary, uniqueness: { scope: [:user_id] }, if: :user_id validate :user_id_not_changed, if: :primary - - validates :primary, uniqueness: { scope: [:user_id] } + validate :unique_email private @@ -30,6 +29,12 @@ class UserEmail < ActiveRecord::Base email_changed? end + def unique_email + if self.will_save_change_to_email? && self.class.where("lower(email) = ?", email).exists? + self.errors.add(:email, :taken) + end + end + def user_id_not_changed if self.will_save_change_to_user_id? && self.persisted? self.errors.add(:user_id, I18n.t( diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 7d1e2e67a51..dc7ffc18434 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -28,6 +28,19 @@ describe User do end end + describe 'when record has an email that as already been taken' do + it 'should not be valid' do + user2 = Fabricate(:user) + user.email = user2.email.upcase + + expect(user).to_not be_valid + + expect(user.errors.messages[:primary_email]).to include(I18n.t( + 'activerecord.errors.messages.taken' + )) + end + end + describe 'when user is staged' do it 'should still validate presence of primary_email' do user.staged = true