Allow notification email to be set separately from primary email.

Closes #1932.

CHANGELOG

@@ -60,6 +60,7 @@ v 7.8.0
   - API: Access groups with their path (Julien Bianchi)
   - Added link to milestone and keeping resource context on smaller viewports for issues and merge requests (Jason Blanchard)
   - 
+  - Allow notification email to be set separately from primary email.
   - 
   - API: Add support for editing an existing project (Mika Mäenpää and Hannes Rosenögger) 
   - 

app/controllers/admin/users_controller.rb

@@ -102,6 +102,9 @@ 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?
+
     respond_to do |format|
       format.html { redirect_to :back, notice: "Successfully removed email." }
       format.js { render nothing: true }

app/controllers/profiles/emails_controller.rb

@@ -18,6 +18,9 @@ class Profiles::EmailsController < ApplicationController
     @email = current_user.emails.find(params[:id])
     @email.destroy
 
+    current_user.set_notification_email
+    current_user.save if current_user.notification_email_changed?
+
     respond_to do |format|
       format.html { redirect_to profile_emails_url }
       format.js { render nothing: true }

app/controllers/profiles/notifications_controller.rb

@@ -2,6 +2,7 @@ class Profiles::NotificationsController < ApplicationController
   layout 'profile'
 
   def show
+    @user = current_user
     @notification = current_user.notification
     @project_members = current_user.project_members
     @group_members = current_user.group_members
@@ -11,8 +12,7 @@ class Profiles::NotificationsController < ApplicationController
     type = params[:notification_type]
 
     @saved = if type == 'global'
-               current_user.notification_level = params[:notification_level]
-               current_user.save
+               current_user.update_attributes(user_params)
              elsif type == 'group'
                users_group = current_user.group_members.find(params[:notification_id])
                users_group.notification_level = params[:notification_level]
@@ -22,5 +22,23 @@ class Profiles::NotificationsController < ApplicationController
                project_member.notification_level = params[:notification_level]
                project_member.save
              end
+
+    respond_to do |format|
+      format.html do
+        if @saved
+          flash[:notice] = "Notification settings saved"
+        else
+          flash[:alert] = "Failed to save new settings"
+        end
+
+        redirect_to :back
+      end
+
+      format.js
+    end
+  end
+
+  def user_params
+    params.require(:user).permit(:notification_email, :notification_level)
   end
 end

app/mailers/emails/profile.rb

@@ -4,20 +4,20 @@ module Emails
       @user = User.find(user_id)
       @target_url = user_url(@user)
       @token = token
-      mail(to: @user.email, subject: subject("Account was created for you"))
+      mail(to: @user.notification_email, subject: subject("Account was created for you"))
     end
 
     def new_email_email(email_id)
       @email = Email.find(email_id)
       @user = @email.user
-      mail(to: @user.email, subject: subject("Email was added to your account"))
+      mail(to: @user.notification_email, subject: subject("Email was added to your account"))
     end
 
     def new_ssh_key_email(key_id)
       @key = Key.find(key_id)
       @user = @key.user
       @target_url = user_url(@user)
-      mail(to: @user.email, subject: subject("SSH key was added to your account"))
+      mail(to: @user.notification_email, subject: subject("SSH key was added to your account"))
     end
   end
 end

app/mailers/emails/projects.rb

@@ -12,7 +12,7 @@ module Emails
       @user = User.find user_id
       @project = Project.find project_id
       @target_url = project_url(@project)
-      mail(to: @user.email,
+      mail(to: @user.notification_email,
            subject: subject("Project was moved"))
     end
 

app/mailers/notify.rb

@@ -60,7 +60,7 @@ class Notify < ActionMailer::Base
   # Returns a String containing the User's email address.
   def recipient(recipient_id)
     if recipient = User.find(recipient_id)
-      recipient.email
+      recipient.notification_email
     end
   end
 

app/models/user.rb

@@ -43,6 +43,7 @@
 #  website_url              :string(255)      default(""), not null
 #  last_credential_check_at :datetime
 #  github_access_token      :string(255)
+#  notification_email       :string(255)
 #
 
 require 'carrierwave/orm/activerecord'
@@ -114,6 +115,7 @@ class User < ActiveRecord::Base
   #
   validates :name, presence: true
   validates :email, presence: true, email: { strict_mode: true }, uniqueness: true
+  validates :notification_email, presence: true, email: { strict_mode: true }
   validates :bio, length: { maximum: 255 }, allow_blank: true
   validates :projects_limit, presence: true, numericality: { greater_than_or_equal_to: 0 }
   validates :username,
@@ -127,10 +129,12 @@ class User < ActiveRecord::Base
   validate :namespace_uniq, if: ->(user) { user.username_changed? }
   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? }
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
   before_validation :generate_password, on: :create
   before_validation :sanitize_attrs
+  before_validation :set_notification_email, if: ->(user) { user.email_changed? }
 
   before_save :ensure_authentication_token
   after_save :ensure_namespace_correct
@@ -286,6 +290,10 @@ class User < ActiveRecord::Base
     self.errors.add(:email, 'has already been taken') if Email.exists?(email: self.email)
   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
+
   # Groups user has access to
   def authorized_groups
     @authorized_groups ||= begin
@@ -431,6 +439,12 @@ class User < ActiveRecord::Base
     end
   end
 
+  def set_notification_email
+    if self.notification_email.blank? || !self.all_emails.include?(self.notification_email)
+      self.notification_email = self.email 
+    end
+  end
+
   def requires_ldap_check?
     if !Gitlab.config.ldap.enabled
       false
@@ -504,6 +518,10 @@ class User < ActiveRecord::Base
     end
   end
 
+  def all_emails
+    [self.email, *self.emails.map(&:email)]
+  end
+
   def hook_attrs
     {
       name: name,

app/views/profiles/emails/index.html.haml

@@ -3,7 +3,11 @@
 %p.light
   Your
   %b Primary Email
-  will be used for account notifications, avatar detection and web based operations, such as edits and merges.
+  will be used for avatar detection and web based operations, such as edits and merges.
+  %br
+  Your
+  %b Notification Email
+  will be used for account notifications.
   %br
   All email addresses will be used to identify your commits.
 

app/views/profiles/notifications/show.html.haml

@@ -1,40 +1,57 @@
 %h3.page-title
   Notifications settings
 %p.light
-  GitLab uses the email specified in your profile for notifications
+  These are your global notification settings.
 %hr
-= form_tag profile_notifications_path, method: :put, remote: true, class: 'update-notifications form-horizontal global-notifications-form' do
+
+
+= form_for @user, url: profile_notifications_path, method: :put, html: { class: 'update-notifications form-horizontal global-notifications-form' } do |f|
+  -if @user.errors.any?
+    %div.alert.alert-danger
+      %ul
+        - @user.errors.full_messages.each do |msg|
+          %li= msg
+
   = hidden_field_tag :notification_type, 'global'
 
-  = label_tag :notification_level, 'Notification level', class: 'control-label'
-  .col-sm-10
-    .radio
-      = label_tag nil, class: '' do
-        = radio_button_tag :notification_level, Notification::N_DISABLED, @notification.disabled?, class: 'trigger-submit'
-        .level-title
-          Disabled
-        %p You will not get any notifications via email
+  .form-group
+    = f.label :notification_email, class: "control-label"
+    .col-sm-10
+      = f.select :notification_email, @user.all_emails, { include_blank: false }, class: "form-control"
 
-    .radio
-      = label_tag nil, class: '' do
-        = radio_button_tag :notification_level, Notification::N_MENTION, @notification.mention?, class: 'trigger-submit'
-        .level-title
-          Mention
-        %p You will receive notifications only for comments in which you were @mentioned
+  .form-group
+    = f.label :notification_level, class: 'control-label'
+    .col-sm-10
+      .radio
+        = f.label :notification_level, value: Notification::N_DISABLED do
+          = f.radio_button :notification_level, Notification::N_DISABLED
+          .level-title
+            Disabled
+          %p You will not get any notifications via email
 
-    .radio
-      = label_tag nil, class: '' do
-        = radio_button_tag :notification_level, Notification::N_PARTICIPATING, @notification.participating?, class: 'trigger-submit'
-        .level-title
-          Participating
-        %p You will only receive notifications from related resources (e.g. from your commits or assigned issues)
+      .radio
+        = f.label :notification_level, value: Notification::N_MENTION do
+          = f.radio_button :notification_level, Notification::N_MENTION
+          .level-title
+            Mention
+          %p You will receive notifications only for comments in which you were @mentioned
 
-    .radio
-      = label_tag nil, class: '' do
-        = radio_button_tag :notification_level, Notification::N_WATCH, @notification.watch?, class: 'trigger-submit'
-        .level-title
-          Watch
-        %p You will receive all notifications from projects in which you participate
+      .radio
+        = f.label :notification_level, value: Notification::N_PARTICIPATING do
+          = f.radio_button :notification_level, Notification::N_PARTICIPATING
+          .level-title
+            Participating
+          %p You will only receive notifications from related resources (e.g. from your commits or assigned issues)
+
+      .radio
+        = f.label :notification_level, value: Notification::N_WATCH do
+          = f.radio_button :notification_level, Notification::N_WATCH
+          .level-title
+            Watch
+          %p You will receive all notifications from projects in which you participate
+
+  .form-actions
+    = f.submit 'Save changes', class: "btn btn-save"
 
 .clearfix
   %hr

db/migrate/20150206222854_add_notification_email_to_user.rb

@@ -0,0 +1,11 @@
+class AddNotificationEmailToUser < ActiveRecord::Migration
+  def up
+    add_column :users, :notification_email, :string
+
+    execute "UPDATE users SET notification_email = email"
+  end
+
+  def down
+    remove_column :users, :notification_email
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150125163100) do
+ActiveRecord::Schema.define(version: 20150206222854) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -436,6 +436,7 @@ ActiveRecord::Schema.define(version: 20150125163100) do
     t.string   "website_url",              default: "",    null: false
     t.string   "github_access_token"
     t.string   "gitlab_access_token"
+    t.string   "notification_email"
   end
 
   add_index "users", ["admin"], name: "index_users_on_admin", using: :btree

spec/mailers/notify_spec.rb

@@ -9,9 +9,14 @@ describe Notify do
   let(:recipient) { create(:user, email: 'recipient@example.com') }
   let(:project) { create(:project) }
 
+  before(:each) do
+    email = recipient.emails.create(email: "notifications@example.com")
+    recipient.update_attribute(:notification_email, email.email)
+  end
+
   shared_examples 'a multiple recipients email' do
     it 'is sent to the given recipient' do
-      should deliver_to recipient.email
+      should deliver_to recipient.notification_email
     end
   end
 
@@ -441,7 +446,7 @@ describe Notify do
         end
 
         it 'is sent to the given recipient' do
-          should deliver_to recipient.email
+          should deliver_to recipient.notification_email
         end
 
         it 'contains the message from the note' do