Merge branch 'emailsonpush-replyto' into 'master'

Set EmailsOnPush reply-to address to committer email when enabled.

Addresses private issue https://dev.gitlab.org/gitlab/gitlabhq/issues/2227.

See merge request !520

CHANGELOG

@@ -5,6 +5,7 @@ v 7.10.0 (unreleased)
   - Don't crash when project repository doesn't exist.
   - Add config var to block auto-created LDAP users.
   - Don't use HTML ellipsis in EmailsOnPush subject truncated commit message.
+  - Set EmailsOnPush reply-to address to committer email when enabled.
   - Fix broken file browsing with a submodule that contains a relative link (Stan Hu)
   - Fix persistent XSS vulnerability around profile website URLs.
   - Fix project import URL regex to prevent arbitary local repos from being imported.

app/mailers/emails/projects.rb

@@ -125,9 +125,17 @@ module Emails
 
       @disable_footer = true
 
-      mail(from: sender(author_id, send_from_committer_email),
-           to: recipient,
-           subject: @subject)
+      reply_to = 
+        if send_from_committer_email && can_send_from_user_email?(@author)
+          @author.email
+        else
+          Gitlab.config.gitlab.email_reply_to
+        end
+
+      mail(from:      sender(author_id, send_from_committer_email),
+           reply_to:  reply_to,
+           to:        recipient,
+           subject:   @subject)
     end
   end
 end

app/mailers/notify.rb

@@ -60,20 +60,24 @@ class Notify < ActionMailer::Base
     address
   end
 
+  def can_send_from_user_email?(sender)
+    sender_domain = sender.email.split("@").last
+    self.class.allowed_email_domains.include?(sender_domain)
+  end
+
   # Return an email address that displays the name of the sender.
   # Only the displayed name changes; the actual email address is always the same.
   def sender(sender_id, send_from_user_email = false)
-    if sender = User.find(sender_id)
-      address = default_sender_address
-      address.display_name = sender.name
+    return unless sender = User.find(sender_id)
+    
+    address = default_sender_address
+    address.display_name = sender.name
 
-      sender_domain = sender.email.split("@").last
-      if send_from_user_email && self.class.allowed_email_domains.include?(sender_domain)
-        address.address = sender.email
-      end
-
-      address.format
+    if send_from_user_email && can_send_from_user_email?(sender)
+      address.address = sender.email
     end
+
+    address.format
   end
 
   # Look up a User by their ID and return their email address

spec/mailers/notify_spec.rb

@@ -725,6 +725,11 @@ describe Notify do
           sender = subject.header[:from].addrs[0]
           expect(sender.address).to eq(user.email)
         end
+
+        it "is set to reply to the committer email" do
+          sender = subject.header[:reply_to].addrs[0]
+          expect(sender.address).to eq(user.email)
+        end
       end
 
       context "when the committer email domain is not completely within the GitLab domain" do
@@ -738,6 +743,11 @@ describe Notify do
           sender = subject.header[:from].addrs[0]
           expect(sender.address).to eq(gitlab_sender)
         end
+
+        it "is set to reply to the default email" do
+          sender = subject.header[:reply_to].addrs[0]
+          expect(sender.address).to eq(gitlab_sender_reply_to)
+        end
       end
 
       context "when the committer email domain is outside the GitLab domain" do
@@ -751,6 +761,11 @@ describe Notify do
           sender = subject.header[:from].addrs[0]
           expect(sender.address).to eq(gitlab_sender)
         end
+
+        it "is set to reply to the default email" do
+          sender = subject.header[:reply_to].addrs[0]
+          expect(sender.address).to eq(gitlab_sender_reply_to)
+        end
       end
     end
   end