diff --git a/CHANGELOG b/CHANGELOG
index 77e50e799c4cab9cd3339b1bdf2ac59e72fad223..25936eb1e1ddb1ac98e7cd90d65b87b98c935119 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -33,6 +33,9 @@ v 7.10.0 (unreleased)
- Don't show commit comment button when user is not signed in.
v 7.9.0
+ - Send EmailsOnPush email when branch or tag is created or deleted.
+
+v 7.9.0 (unreleased)
- Add HipChat integration documentation (Stan Hu)
- Update documentation for object_kind field in Webhook push and tag push Webhooks (Stan Hu)
- Fix broken email images (Hannes Rosenögger)
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index b55129de2921e96fdb2cdc510d3fb25b3f7d14dc..48458baa674f27971662134b2b24b4cc22eb3a2b 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -16,31 +16,65 @@ 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: nil,
+ 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 acb5e7f1af5f68c279526bfa7681a39806c511d6..6f6e5950aab3575b00cc53f9030e4946003dd46b 100644
--- a/app/models/project_services/emails_on_push_service.rb
+++ b/app/models/project_services/emails_on_push_service.rb
@@ -36,13 +36,19 @@ class EmailsOnPushService < Service
end
def supported_events
- %w(push)
+ %w(push tag_push)
end
def execute(push_data)
return unless supported_events.include?(push_data[:object_kind])
- EmailsOnPushWorker.perform_async(project_id, recipients, push_data, send_from_committer_email?, disable_diffs?)
+ EmailsOnPushWorker.perform_async(
+ project_id,
+ recipients,
+ push_data,
+ send_from_committer_email: send_from_committer_email?,
+ disable_diffs: disable_diffs?
+ )
end
def send_from_committer_email?
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index 039b92df2be043ef54f6d22fd2f7abbf076cb94c..bbf7004c9063464be57d900f1f0b190ba1bac660 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
- −
+ %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
+ −
+ = diff.old_path
+ - elsif diff.renamed_file
= diff.old_path
- - elsif diff.renamed_file
- = diff.old_path
- →
- = diff.new_path
- - elsif diff.new_file
- %span.new-file
- +
+ →
= 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
- →
- %strong
+ - elsif diff.new_file
+ %span.new-file
+ +
+ = 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
+ →
+ %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 8d67a42234e33f01b7233a707bc411a57e6a8ca4..97a176ed2a3800577ce0ec8340c0c6d7428c41dc 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 e59ca81defe36a532853f699a3187b0a5a6282f1..89fa2117dd2bc1db944ecd02bb39e0b5d4f0f3d5 100644
--- a/app/workers/emails_on_push_worker.rb
+++ b/app/workers/emails_on_push_worker.rb
@@ -1,40 +1,49 @@
class EmailsOnPushWorker
include Sidekiq::Worker
- def perform(project_id, recipients, push_data, send_from_committer_email = false, disable_diffs = false)
+ def perform(project_id, recipients, push_data, send_from_committer_email: false, disable_diffs: false)
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|
Notify.repository_push_email(
project_id,
recipient,
- author_id,
- branch,
- compare,
- reverse_compare,
- send_from_committer_email,
- disable_diffs
+ author_id: author_id,
+ ref: ref,
+ action: action,
+ compare: compare,
+ reverse_compare: reverse_compare,
+ send_from_committer_email: send_from_committer_email,
+ disable_diffs: disable_diffs
).deliver
end
ensure
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index e3a3b542358e238de5fb649045c9a7193a867287..ba42f9e5c706589e144dac1f0f1b69dc96665ed0 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', author_id: user.id, ref: 'refs/heads/master', action: :create) }
+
+ 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', author_id: user.id, ref: 'refs/tags/v1.0', action: :create) }
+
+ 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', author_id: user.id, ref: 'refs/heads/master', action: :delete) }
+
+ 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', author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) }
+
+ 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', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, send_from_committer_email: 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', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare) }
it 'is sent as the author' do
sender = subject.header[:from].addrs[0]