Send EmailsOnPush email when branch or tag is created or deleted.

7 files changed, 262 insertions, 127 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 09b60e8e54a..7216676d5e8 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 7.10.0 (unreleased)
   - Add a service to support external wikis (Hannes Rosenögger)
+  - Send EmailsOnPush email when branch or tag is created or deleted.
 
 v 7.9.0 (unreleased)
   - Add HipChat integration documentation (Stan Hu)
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index b55129de292..d2165c9f764 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -16,31 +16,59 @@ module Emails
            subject: subject("Project was moved"))
     end
 
-    def repository_push_email(project_id, recipient, author_id, branch, compare, reverse_compare = false, send_from_committer_email = false, disable_diffs = false)
+    def repository_push_email(project_id, recipient, author_id, ref, action, compare, reverse_compare = false, send_from_committer_email = false, disable_diffs = false)
       @project = Project.find(project_id)
       @author  = User.find(author_id)
       @reverse_compare = reverse_compare
       @compare = compare
-      @commits = Commit.decorate(compare.commits)
-      @diffs   = compare.diffs
-      @branch  = Gitlab::Git.ref_name(branch)
+      @ref_name  = Gitlab::Git.ref_name(ref)
+      @ref_type  = Gitlab::Git.tag_ref?(ref) ? "tag" : "branch"
+      @action  = action
       @disable_diffs = disable_diffs
 
-      @subject = "[#{@project.path_with_namespace}][#{@branch}] "
+      if @compare
+        @commits = Commit.decorate(compare.commits)
+        @diffs   = compare.diffs
+      end
+
+      @action_name = 
+        case action
+        when :create
+          "pushed new"
+        when :delete
+          "deleted"
+        else
+          "pushed to"
+        end
+
+      @subject = "[#{@project.path_with_namespace}]"
+      @subject << "[#{@ref_name}]" if action == :push
+      @subject << " "
+
+      if action == :push
+        if @commits.length > 1
+          @target_url = namespace_project_compare_url(@project.namespace,
+                                                      @project,
+                                                      from: Commit.new(@compare.base),
+                                                      to:   Commit.new(@compare.head))
+          @subject << "Deleted " if @reverse_compare
+          @subject << "#{@commits.length} commits: #{@commits.first.title}"
+        else
+          @target_url = namespace_project_commit_url(@project.namespace,
+                                                     @project, @commits.first)
 
-      if @commits.length > 1
-        @target_url = namespace_project_compare_url(@project.namespace,
-                                                    @project,
-                                                    from: Commit.new(@compare.base),
-                                                    to:   Commit.new(@compare.head))
-        @subject << "Deleted " if @reverse_compare
-        @subject << "#{@commits.length} commits: #{@commits.first.title}"
+          @subject << "Deleted 1 commit: " if @reverse_compare
+          @subject << @commits.first.title
+        end
       else
-        @target_url = namespace_project_commit_url(@project.namespace,
-                                                   @project, @commits.first)
+        unless action == :delete
+          @target_url = namespace_project_tree_url(@project.namespace,
+                                                   @project, @ref_name)
+        end
 
-        @subject << "Deleted 1 commit: " if @reverse_compare
-        @subject << @commits.first.title
+        subject_action = @action_name.dup
+        subject_action[0] = subject_action[0].capitalize
+        @subject << "#{subject_action} #{@ref_type} #{@ref_name}"
       end
 
       @disable_footer = true
diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb
index acb5e7f1af5..3e465ab1a38 100644
--- a/app/models/project_services/emails_on_push_service.rb
+++ b/app/models/project_services/emails_on_push_service.rb
@@ -36,7 +36,7 @@ class EmailsOnPushService < Service
   end
 
   def supported_events
-    %w(push)
+    %w(push tag_push)
   end
 
   def execute(push_data)
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index 039b92df2be..bbf7004c906 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -1,66 +1,67 @@
-%h3 #{@author.name} pushed to #{@branch} at #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)}
+%h3 #{@author.name} #{@action_name} #{@ref_type} #{@ref_name} at #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)}
 
-- if @reverse_compare
-  %p
-    %strong WARNING:
-    The push did not contain any new commits, but force pushed to delete the commits and changes below.
+- if @compare
+  - if @reverse_compare
+    %p
+      %strong WARNING:
+      The push did not contain any new commits, but force pushed to delete the commits and changes below.
 
-%h4
-  = @reverse_compare ? "Deleted commits:" : "Commits:"
+  %h4
+    = @reverse_compare ? "Deleted commits:" : "Commits:"
 
-%ul
-  - @commits.each do |commit|
-    %li
-      %strong #{link_to commit.short_id, namespace_project_commit_url(@project.namespace, @project, commit)}
-      %div
-        %span by #{commit.author_name}
-        %i at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")}
-      %pre.commit-message 
-        = commit.safe_message
+  %ul
+    - @commits.each do |commit|
+      %li
+        %strong #{link_to commit.short_id, namespace_project_commit_url(@project.namespace, @project, commit)}
+        %div
+          %span by #{commit.author_name}
+          %i at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")}
+        %pre.commit-message 
+          = commit.safe_message
 
-%h4 #{pluralize @diffs.count, "changed file"}:
+  %h4 #{pluralize @diffs.count, "changed file"}:
 
-%ul
-  - @diffs.each_with_index do |diff, i|
-    %li.file-stats
-      %a{href: "#{@target_url if @disable_diffs}#diff-#{i}" }
-        - if diff.deleted_file
-          %span.deleted-file
-            &minus;
+  %ul
+    - @diffs.each_with_index do |diff, i|
+      %li.file-stats
+        %a{href: "#{@target_url if @disable_diffs}#diff-#{i}" }
+          - if diff.deleted_file
+            %span.deleted-file
+              &minus;
+              = diff.old_path
+          - elsif diff.renamed_file
             = diff.old_path
-        - elsif diff.renamed_file
-          = diff.old_path
-          &rarr;
-          = diff.new_path
-        - elsif diff.new_file
-          %span.new-file
-            &plus;
+            &rarr;
             = diff.new_path
-        - else
-          = diff.new_path
-
-- unless @disable_diffs
-  %h4 Changes:
-  - @diffs.each_with_index do |diff, i|
-    %li{id: "diff-#{i}"}
-      %a{href: @target_url + "#diff-#{i}"}
-        - if diff.deleted_file
-          %strong
-            = diff.old_path
-          deleted
-        - elsif diff.renamed_file
-          %strong
-            = diff.old_path
-          &rarr;
-          %strong
+          - elsif diff.new_file
+            %span.new-file
+              &plus;
+              = diff.new_path
+          - else
             = diff.new_path
-        - else
-          %strong
-            = diff.new_path
-      %hr
-      %pre
-        = color_email_diff(diff.diff)
-      %br
 
-- if @compare.timeout
-  %h5 Huge diff. To prevent performance issues changes are hidden
+  - unless @disable_diffs
+    %h4 Changes:
+    - @diffs.each_with_index do |diff, i|
+      %li{id: "diff-#{i}"}
+        %a{href: @target_url + "#diff-#{i}"}
+          - if diff.deleted_file
+            %strong
+              = diff.old_path
+            deleted
+          - elsif diff.renamed_file
+            %strong
+              = diff.old_path
+            &rarr;
+            %strong
+              = diff.new_path
+          - else
+            %strong
+              = diff.new_path
+        %hr
+        %pre
+          = color_email_diff(diff.diff)
+        %br
+
+  - if @compare.timeout
+    %h5 Huge diff. To prevent performance issues changes are hidden
diff --git a/app/views/notify/repository_push_email.text.haml b/app/views/notify/repository_push_email.text.haml
index 8d67a42234e..97a176ed2a3 100644
--- a/app/views/notify/repository_push_email.text.haml
+++ b/app/views/notify/repository_push_email.text.haml
@@ -1,47 +1,49 @@
-#{@author.name} pushed to #{@branch} at #{@project.name_with_namespace}
-\
-\
-- if @reverse_compare
-  WARNING: The push did not contain any new commits, but force pushed to delete the commits and changes below.
+#{@author.name} #{@action_name} #{@ref_type} #{@ref_name} at #{@project.name_with_namespace}
+- if @compare
   \
   \
-= @reverse_compare ? "Deleted commits:" : "Commits:"
-- @commits.each do |commit|
-  #{commit.short_id} by #{commit.author_name} at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")}
-  #{commit.safe_message}
-  \- - - - -
-\
-\
-#{pluralize @diffs.count, "changed file"}:
-\
-- @diffs.each do |diff|
-  - if diff.deleted_file
-    \- − #{diff.old_path}
-  - elsif diff.renamed_file
-    \- #{diff.old_path} → #{diff.new_path}
-  - elsif diff.new_file
-    \- + #{diff.new_path}
-  - else
-    \- #{diff.new_path}
-- unless @disable_diffs
+  - if @reverse_compare
+    WARNING: The push did not contain any new commits, but force pushed to delete the commits and changes below.
+    \
+    \
+  = @reverse_compare ? "Deleted commits:" : "Commits:"
+  - @commits.each do |commit|
+    #{commit.short_id} by #{commit.author_name} at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")}
+    #{commit.safe_message}
+    \- - - - -
+  \
   \
+  #{pluralize @diffs.count, "changed file"}:
   \
-  Changes:
   - @diffs.each do |diff|
-    \
-    \=====================================
     - if diff.deleted_file
-      #{diff.old_path} deleted
+      \- − #{diff.old_path}
     - elsif diff.renamed_file
-      #{diff.old_path} → #{diff.new_path}
+      \- #{diff.old_path} → #{diff.new_path}
+    - elsif diff.new_file
+      \- + #{diff.new_path}
     - else
-      = diff.new_path
-    \=====================================
-    != diff.diff
-- if @compare.timeout
-  \
-  \
-  Huge diff. To prevent performance issues it was hidden
-\
-\
-View it on GitLab: #{@target_url}
+      \- #{diff.new_path}
+  - unless @disable_diffs
+    \
+    \
+    Changes:
+    - @diffs.each do |diff|
+      \
+      \=====================================
+      - if diff.deleted_file
+        #{diff.old_path} deleted
+      - elsif diff.renamed_file
+        #{diff.old_path} → #{diff.new_path}
+      - else
+        = diff.new_path
+      \=====================================
+      != diff.diff
+  - if @compare.timeout
+    \
+    \
+    Huge diff. To prevent performance issues it was hidden
+  - if @target_url
+    \
+    \
+    View it on GitLab: #{@target_url}
diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb
index e59ca81defe..429b29525dc 100644
--- a/app/workers/emails_on_push_worker.rb
+++ b/app/workers/emails_on_push_worker.rb
@@ -5,24 +5,32 @@ class EmailsOnPushWorker
     project = Project.find(project_id)
     before_sha = push_data["before"]
     after_sha = push_data["after"]
-    branch = push_data["ref"]
+    ref = push_data["ref"]
     author_id = push_data["user_id"]
 
-    if Gitlab::Git.blank_ref?(before_sha) || Gitlab::Git.blank_ref?(after_sha)
-      # skip if new branch was pushed or branch was removed
-      return true
-    end
+    action = 
+      if Gitlab::Git.blank_ref?(before_sha)
+        :create 
+      elsif Gitlab::Git.blank_ref?(after_sha)
+        :delete
+      else
+        :push
+      end
 
-    compare = Gitlab::Git::Compare.new(project.repository.raw_repository, before_sha, after_sha)
+    compare = nil
+    reverse_compare = false
+    if action == :push
+      compare = Gitlab::Git::Compare.new(project.repository.raw_repository, before_sha, after_sha)
 
-    return false if compare.same
+      return false if compare.same
 
-    if compare.commits.empty?
-      compare = Gitlab::Git::Compare.new(project.repository.raw_repository, after_sha, before_sha)
+      if compare.commits.empty?
+        compare = Gitlab::Git::Compare.new(project.repository.raw_repository, after_sha, before_sha)
 
-      reverse_compare = true
+        reverse_compare = true
 
-      return false if compare.commits.empty?
+        return false if compare.commits.empty?
+      end
     end
 
     recipients.split(" ").each do |recipient|
@@ -30,7 +38,8 @@ class EmailsOnPushWorker
         project_id, 
         recipient, 
         author_id, 
-        branch, 
+        ref, 
+        action,
         compare, 
         reverse_compare,
         send_from_committer_email,
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index e3a3b542358..aeb0e14d836 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -640,6 +640,100 @@ describe Notify do
     end
   end
 
+  describe 'email on push for a created branch' do
+    let(:example_site_path) { root_path }
+    let(:user) { create(:user) }
+    let(:tree_path) { namespace_project_tree_path(project.namespace, project, "master") }
+
+    subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'refs/heads/master', :create, nil) }
+
+    it 'is sent as the author' do
+      sender = subject.header[:from].addrs[0]
+      expect(sender.display_name).to eq(user.name)
+      expect(sender.address).to eq(gitlab_sender)
+    end
+
+    it 'is sent to recipient' do
+      is_expected.to deliver_to 'devs@company.name'
+    end
+
+    it 'has the correct subject' do
+      is_expected.to have_subject /Pushed new branch master/
+    end
+
+    it 'contains a link to the branch' do
+      is_expected.to have_body_text /#{tree_path}/
+    end
+  end
+
+  describe 'email on push for a created tag' do
+    let(:example_site_path) { root_path }
+    let(:user) { create(:user) }
+    let(:tree_path) { namespace_project_tree_path(project.namespace, project, "v1.0") }
+
+    subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'refs/tags/v1.0', :create, nil) }
+
+    it 'is sent as the author' do
+      sender = subject.header[:from].addrs[0]
+      expect(sender.display_name).to eq(user.name)
+      expect(sender.address).to eq(gitlab_sender)
+    end
+
+    it 'is sent to recipient' do
+      is_expected.to deliver_to 'devs@company.name'
+    end
+
+    it 'has the correct subject' do
+      is_expected.to have_subject /Pushed new tag v1\.0/
+    end
+
+    it 'contains a link to the tag' do
+      is_expected.to have_body_text /#{tree_path}/
+    end
+  end
+
+  describe 'email on push for a deleted branch' do
+    let(:example_site_path) { root_path }
+    let(:user) { create(:user) }
+
+    subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'refs/heads/master', :delete, nil) }
+
+    it 'is sent as the author' do
+      sender = subject.header[:from].addrs[0]
+      expect(sender.display_name).to eq(user.name)
+      expect(sender.address).to eq(gitlab_sender)
+    end
+
+    it 'is sent to recipient' do
+      is_expected.to deliver_to 'devs@company.name'
+    end
+
+    it 'has the correct subject' do
+      is_expected.to have_subject /Deleted branch master/
+    end
+  end
+
+  describe 'email on push for a deleted tag' do
+    let(:example_site_path) { root_path }
+    let(:user) { create(:user) }
+
+    subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'refs/tags/v1.0', :delete, nil) }
+
+    it 'is sent as the author' do
+      sender = subject.header[:from].addrs[0]
+      expect(sender.display_name).to eq(user.name)
+      expect(sender.address).to eq(gitlab_sender)
+    end
+
+    it 'is sent to recipient' do
+      is_expected.to deliver_to 'devs@company.name'
+    end
+
+    it 'has the correct subject' do
+      is_expected.to have_subject /Deleted tag v1\.0/
+    end
+  end
+
   describe 'email on push with multiple commits' do
     let(:example_site_path) { root_path }
     let(:user) { create(:user) }
@@ -648,7 +742,7 @@ describe Notify do
     let(:diff_path) { namespace_project_compare_path(project.namespace, project, from: Commit.new(compare.base), to: Commit.new(compare.head)) }
     let(:send_from_committer_email) { false }
 
-    subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'master', compare, false, send_from_committer_email) }
+    subject { Notify.repository_push_email(project.id, 'devs@company.name', user.id, 'refs/heads/master', :push, compare, false, send_from_committer_email) }
 
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]
@@ -736,7 +830,7 @@ describe Notify do
     let(:commits) { Commit.decorate(compare.commits) }
     let(:diff_path) { namespace_project_commit_path(project.namespace, project, commits.first) }
 
-    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, 'refs/heads/master', :push, compare) }
 
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]