Allow primary email to be set to an email that you've already added.change-primary-email

6 files changed, 46 insertions, 18 deletions

diff --git a/CHANGELOG b/CHANGELOG
index ecffcb5262c..72b0d697e7d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.11.0 (unreleased)
+  - Allow primary email to be set to an email that you've already added.
   - Get Gitorious importer to work again.
   - Fix clone URL field and X11 Primary selection (Dmitry Medvinsky)
   - Ignore invalid lines in .gitmodules
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index adb83996f8b..d36e359934c 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -102,8 +102,7 @@ class Admin::UsersController < Admin::ApplicationController
     email = user.emails.find(params[:email_id])
     email.destroy
 
-    user.set_notification_email
-    user.save if user.notification_email_changed?
+    user.update_secondary_emails!
 
     respond_to do |format|
       format.html { redirect_to :back, notice: "Successfully removed email." }
diff --git a/app/controllers/profiles/emails_controller.rb b/app/controllers/profiles/emails_controller.rb
index 954c98c0d9f..dddca05ce24 100644
--- a/app/controllers/profiles/emails_controller.rb
+++ b/app/controllers/profiles/emails_controller.rb
@@ -3,14 +3,17 @@ class Profiles::EmailsController < ApplicationController
 
   def index
     @primary = current_user.email
-    @public_email = current_user.public_email
     @emails = current_user.emails
   end
 
   def create
     @email = current_user.emails.new(email_params)
 
-    flash[:alert] = @email.errors.full_messages.first unless @email.save
+    if @email.save
+      NotificationService.new.new_email(@email)
+    else
+      flash[:alert] = @email.errors.full_messages.first
+    end
 
     redirect_to profile_emails_url
   end
@@ -19,9 +22,7 @@ class Profiles::EmailsController < ApplicationController
     @email = current_user.emails.find(params[:id])
     @email.destroy
 
-    current_user.set_notification_email
-    current_user.set_public_email
-    current_user.save if current_user.notification_email_changed? or current_user.public_email_changed?
+    current_user.update_secondary_emails!
 
     respond_to do |format|
       format.html { redirect_to profile_emails_url }
diff --git a/app/models/email.rb b/app/models/email.rb
index 556b0e9586e..935705e2ed4 100644
--- a/app/models/email.rb
+++ b/app/models/email.rb
@@ -18,7 +18,6 @@ class Email < ActiveRecord::Base
   validates :email, presence: true, email: { strict_mode: true }, uniqueness: true
   validate :unique_email, if: ->(email) { email.email_changed? }
 
-  after_create :notify
   before_validation :cleanup_email
 
   def cleanup_email
@@ -28,8 +27,4 @@ class Email < ActiveRecord::Base
   def unique_email
     self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email)
   end
-
-  def notify
-    NotificationService.new.new_email(self)
-  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index d6b93afe739..847100de282 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -139,6 +139,7 @@ class User < ActiveRecord::Base
   validate :avatar_type, if: ->(user) { user.avatar_changed? }
   validate :unique_email, if: ->(user) { user.email_changed? }
   validate :owns_notification_email, if: ->(user) { user.notification_email_changed? }
+  validate :owns_public_email, if: ->(user) { user.public_email_changed? }
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
   before_validation :generate_password, on: :create
@@ -146,6 +147,7 @@ class User < ActiveRecord::Base
   before_validation :set_notification_email, if: ->(user) { user.email_changed? }
   before_validation :set_public_email, if: ->(user) { user.public_email_changed? }
 
+  after_update :update_emails_with_primary_email, if: ->(user) { user.email_changed? }
   before_save :ensure_authentication_token
   after_save :ensure_namespace_correct
   after_initialize :set_projects_limit
@@ -276,13 +278,29 @@ class User < ActiveRecord::Base
   end
 
   def unique_email
-    self.errors.add(:email, 'has already been taken') if Email.exists?(email: self.email)
+    if !self.emails.exists?(email: self.email) && Email.exists?(email: self.email)
+      self.errors.add(:email, 'has already been taken')
+    end
   end
 
   def owns_notification_email
     self.errors.add(:notification_email, "is not an email you own") unless self.all_emails.include?(self.notification_email)
   end
 
+  def owns_public_email
+    self.errors.add(:public_email, "is not an email you own") unless self.all_emails.include?(self.public_email)
+  end
+
+  def update_emails_with_primary_email
+    primary_email_record = self.emails.find_by(email: self.email)
+    if primary_email_record
+      primary_email_record.destroy
+      self.emails.create(email: self.email_was)
+      
+      self.update_secondary_emails!
+    end
+  end
+
   # Groups user has access to
   def authorized_groups
     @authorized_groups ||= begin
@@ -448,10 +466,16 @@ class User < ActiveRecord::Base
 
   def set_public_email
     if self.public_email.blank? || !self.all_emails.include?(self.public_email)
-      self.public_email = ''
+      self.public_email = nil
     end
   end
 
+  def update_secondary_emails!
+    self.set_notification_email
+    self.set_public_email
+    self.save if self.notification_email_changed? || self.public_email_changed?
+  end
+
   def set_projects_limit
     connection_default_value_defined = new_record? && !projects_limit_changed?
     return unless self.projects_limit.nil? || connection_default_value_defined
diff --git a/app/views/profiles/emails/index.html.haml b/app/views/profiles/emails/index.html.haml
index 09f290429ea..9ef49ef9ec9 100644
--- a/app/views/profiles/emails/index.html.haml
+++ b/app/views/profiles/emails/index.html.haml
@@ -4,11 +4,15 @@
   Your
   %b Primary Email
   will be used for avatar detection and web based operations, such as edits and merges.
-  %br
+%p.light
   Your
   %b Notification Email
   will be used for account notifications.
-  %br
+%p.light
+  Your
+  %b Public Email
+  will be displayed on your public profile.
+%p.light
   All email addresses will be used to identify your commits.
 
 %hr
@@ -20,13 +24,17 @@
     %li
       %strong= @primary
       %span.label.label-success Primary Email
-      - if @primary === @public_email
+      - if @primary === current_user.public_email
         %span.label.label-info Public Email
+      - if @primary === current_user.notification_email
+        %span.label.label-info Notification Email
     - @emails.each do |email|
       %li
         %strong= email.email
-        - if email.email === @public_email
+        - if email.email === current_user.public_email
           %span.label.label-info Public Email
+        - if email.email === current_user.notification_email
+          %span.label.label-info Notification Email
         %span.cgray
           added #{time_ago_with_tooltip(email.created_at)}
         = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-sm btn-remove pull-right'