Add option to send EmailsOnPush from committer email if domain matches.

See #1809.

7 files changed, 51 insertions, 10 deletions

diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb
index e80cabd6e18..88106b2418a 100644
--- a/app/controllers/admin/services_controller.rb
+++ b/app/controllers/admin/services_controller.rb
@@ -45,7 +45,8 @@ class Admin::ServicesController < Admin::ApplicationController
         :room, :recipients, :project_url, :webhook,
         :user_key, :device, :priority, :sound, :bamboo_url, :username, :password,
         :build_key, :server, :teamcity_url, :build_type,
-        :description, :issues_url, :new_issue_url, :restrict_to_branch
+        :description, :issues_url, :new_issue_url, :restrict_to_branch,
+        :send_from_committer_email
     ])
   end
 end
diff --git a/app/controllers/projects/services_controller.rb b/app/controllers/projects/services_controller.rb
index 5c29a6550f5..dd3987605e3 100644
--- a/app/controllers/projects/services_controller.rb
+++ b/app/controllers/projects/services_controller.rb
@@ -50,7 +50,8 @@ class Projects::ServicesController < Projects::ApplicationController
       :room, :recipients, :project_url, :webhook,
       :user_key, :device, :priority, :sound, :bamboo_url, :username, :password,
       :build_key, :server, :teamcity_url, :build_type,
-      :description, :issues_url, :new_issue_url, :restrict_to_branch
+      :description, :issues_url, :new_issue_url, :restrict_to_branch,
+      :send_from_committer_email
     )
   end
 end
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index f3a2ae14d35..3b60aed6f9e 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -16,13 +16,13 @@ module Emails
            subject: subject("Project was moved"))
     end
 
-    def repository_push_email(project_id, recipient, author_id, branch, compare)
+    def repository_push_email(project_id, recipient, author_id, branch, compare, send_from_committer_email = false)
       @project = Project.find(project_id)
       @author  = User.find(author_id)
       @compare = compare
       @commits = Commit.decorate(compare.commits)
       @diffs   = compare.diffs
-      @branch  = branch
+      @branch  = branch.gsub("refs/heads/", "")
 
       @subject = "[#{@project.path_with_namespace}][#{@branch}] "
 
@@ -40,7 +40,7 @@ module Emails
 
       @disable_footer = true
 
-      mail(from: sender(author_id),
+      mail(from: sender(author_id, send_from_committer_email),
            to: recipient,
            subject: @subject)
     end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 46ead62f75f..00d609cd93c 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -45,10 +45,15 @@ class Notify < ActionMailer::Base
 
   # 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)
+  def sender(sender_id, send_from_user_email = false)
     if sender = User.find(sender_id)
       address = default_sender_address
       address.display_name = sender.name
+
+      if send_from_user_email && sender.email.end_with?("@#{Gitlab.config.gitlab.host}")
+        address.address = sender.email
+      end
+
       address.format
     end
   end
diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb
index 86693ad0c7e..a5653665bfb 100644
--- a/app/models/project_services/emails_on_push_service.rb
+++ b/app/models/project_services/emails_on_push_service.rb
@@ -14,6 +14,7 @@
 #
 
 class EmailsOnPushService < Service
+  prop_accessor :send_from_committer_email
   prop_accessor :recipients
   validates :recipients, presence: true, if: :activated?
 
@@ -29,12 +30,17 @@ class EmailsOnPushService < Service
     'emails_on_push'
   end
 
+  def send_from_committer_email?
+    self.send_from_committer_email == "1"
+  end
+
   def execute(push_data)
-    EmailsOnPushWorker.perform_async(project_id, recipients, push_data)
+    EmailsOnPushWorker.perform_async(project_id, recipients, push_data, self.send_from_committer_email?)
   end
 
   def fields
     [
+      { type: 'checkbox', name: 'send_from_committer_email', title: "Send from committer email if domain matches" },
       { type: 'textarea', name: 'recipients', placeholder: 'Emails separated by whitespace' },
     ]
   end
diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb
index e3f6f3a6aef..3814b17a8a2 100644
--- a/app/workers/emails_on_push_worker.rb
+++ b/app/workers/emails_on_push_worker.rb
@@ -1,7 +1,7 @@
 class EmailsOnPushWorker
   include Sidekiq::Worker
 
-  def perform(project_id, recipients, push_data)
+  def perform(project_id, recipients, push_data, send_from_committer_email = false)
     project = Project.find(project_id)
     before_sha = push_data["before"]
     after_sha = push_data["after"]
@@ -19,7 +19,7 @@ class EmailsOnPushWorker
     return false unless compare && compare.commits.present?
 
     recipients.split(" ").each do |recipient|
-      Notify.repository_push_email(project_id, recipient, author_id, branch, compare).deliver
+      Notify.repository_push_email(project_id, recipient, author_id, branch, compare, send_from_committer_email).deliver
     end
   ensure
     compare = nil
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 41b0daacded..ad2b7c11f84 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -569,8 +569,9 @@ describe Notify do
     let(:compare) { Gitlab::Git::Compare.new(project.repository.raw_repository, sample_image_commit.id, sample_commit.id) }
     let(:commits) { Commit.decorate(compare.commits) }
     let(:diff_path) { namespace_project_compare_path(project.namespace, project, from: commits.first, to: commits.last) }
+    let(:send_from_committer_email) { false }
 
-    subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare) }
+    subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare, send_from_committer_email) }
 
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]
@@ -601,6 +602,33 @@ describe Notify do
     it 'doesn not contain the misleading footer' do
       is_expected.not_to have_body_text /you are a member of/
     end
+
+    context "when set to send from committer email if domain matches" do
+
+      let(:send_from_committer_email) { true }
+
+      context "when the committer email domain matches" do
+
+        before do
+          allow(Gitlab.config.gitlab).to receive(:host).and_return("gitlab.dev")
+          user.update_attribute(:email, "user@#{Gitlab.config.gitlab.host}")
+          user.confirm!
+        end
+
+        it "is sent from the committer email" do
+          sender = subject.header[:from].addrs[0]
+          expect(sender.address).to eq(user.email)
+        end
+      end
+
+      context "when the committer email doesn't match" do
+
+        it "is sent from the default email" do
+          sender = subject.header[:from].addrs[0]
+          expect(sender.address).to eq(gitlab_sender)
+        end
+      end
+    end
   end
 
   describe 'email on push with a single commit' do