Fix duplicate 'Email has already been taken' message when creating a user

2 files changed, 24 insertions, 1 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 6ac287203b1..25b46da2aaf 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -137,7 +137,9 @@ class User < ActiveRecord::Base
   # Validations
   #
   validates :name, presence: true
-  validates :email, presence: true, email: { strict_mode: true }, uniqueness: true
+  # Note that a 'uniqueness' and presence check is provided by devise :validatable for email. We do not need to
+  # duplicate that here as the validation framework will have duplicate errors in the event of a failure
+  validates :email, presence: true, email: { strict_mode: true }
   validates :notification_email, presence: true, email: { strict_mode: true }
   validates :public_email, presence: true, email: { strict_mode: true }, allow_blank: true, uniqueness: true
   validates :bio, length: { maximum: 255 }, allow_blank: true
diff --git a/spec/features/users_spec.rb b/spec/features/users_spec.rb
index 93d2b18b5fc..a4c3dfe9205 100644
--- a/spec/features/users_spec.rb
+++ b/spec/features/users_spec.rb
@@ -27,4 +27,25 @@ feature 'Users' do
     user.reload
     expect(user.reset_password_token).to be_nil
   end
+
+  let!(:user) { create(:user, username: 'user1', name: 'User 1', email: 'user1@gitlab.com') }
+  scenario 'Should show one error if email is already taken' do
+    visit new_user_session_path
+    fill_in 'user_name', with: 'Another user name'
+    fill_in 'user_username', with: 'anotheruser'
+    fill_in 'user_email', with: user.email
+    fill_in 'user_password_sign_up', with: '12341234'
+    expect { click_button 'Sign up' }.to change { User.count }.by(0)
+    expect(page).to have_text('Email has already been taken')
+    expect(number_of_errors_on_page(page)).to be(1), 'errors on page:\n #{errors_on_page page}'
+  end
+
+  def errors_on_page(page)
+    page.find('#error_explanation').find('ul').all('li').map{ |item| item.text }.join("\n")
+  end
+
+  def number_of_errors_on_page(page)
+    page.find('#error_explanation').find('ul').all('li').count
+  end
+
 end