diff --git a/CHANGELOG-EE b/CHANGELOG-EE index 918c4082f3..8c8b579643 100644 --- a/CHANGELOG-EE +++ b/CHANGELOG-EE @@ -1,6 +1,7 @@ v 8.2.0 - Invalidate stored jira password if the endpoint URL is changed - Fix: Page is not reloaded periodically to check if rebase is finished + - When someone as marked as a required approver for a merge request, an email should be sent v 8.1.0 (unreleased) - added an issues template (Hannes Rosenögger) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index d5e342000d..b1179c1140 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -135,6 +135,8 @@ class MergeRequest < ActiveRecord::Base scope :closed, -> { with_state(:closed) } scope :closed_and_merged, -> { with_states(:closed, :merged) } + participant :approvers_left + def self.reference_prefix '!' end diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 90ebdfc3fe..09ed6740d5 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -5,5 +5,9 @@ %p Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} +- if @merge_request.approvers.any? + %p + Approvers: #{render_items_list(@merge_request.approvers_left.map(&:name))} + -if @merge_request.description = markdown(@merge_request.description, pipeline: :email) diff --git a/app/views/notify/new_merge_request_email.text.erb b/app/views/notify/new_merge_request_email.text.erb index bdcca6e4ab..79b27390b3 100644 --- a/app/views/notify/new_merge_request_email.text.erb +++ b/app/views/notify/new_merge_request_email.text.erb @@ -5,4 +5,7 @@ New Merge Request #<%= @merge_request.iid %> <%= merge_path_description(@merge_request, 'to') %> Author: <%= @merge_request.author_name %> Assignee: <%= @merge_request.assignee_name %> +<% if @merge_request.approvers.any? %> +Approvers: <%= render_items_list(@merge_request.approvers_left.map(&:name)) %> +<% end %> diff --git a/spec/factories/approvers.rb b/spec/factories/approvers.rb new file mode 100644 index 0000000000..6d7d61d35a --- /dev/null +++ b/spec/factories/approvers.rb @@ -0,0 +1,8 @@ +# Read about factories at https://github.com/thoughtbot/factory_girl + +FactoryGirl.define do + factory :approver do + target factory: :merge_request + user + end +end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 6080d0ccde..79180d4214 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -64,8 +64,15 @@ FactoryGirl.define do target_branch "master" end + trait :with_approver do + after :create do |merge_request| + create :approver, target: merge_request + end + end + factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:reopened] factory :merge_request_with_diffs, traits: [:with_diffs] + factory :merge_request_with_approver, traits: [:with_approver] end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 9f030c07e6..3761732c37 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -276,6 +276,7 @@ describe Notify do let(:merge_author) { create(:user) } let(:merge_request) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project) } let(:merge_request_with_description) { create(:merge_request, author: current_user, assignee: assignee, source_project: project, target_project: project, description: FFaker::Lorem.sentence) } + let(:merge_request_with_approver) { create(:merge_request_with_approver, author: current_user, assignee: assignee, source_project: project, target_project: project) } describe 'that are new' do subject { Notify.new_merge_request_email(merge_request.assignee_id, merge_request.id) } @@ -304,8 +305,26 @@ describe Notify do end end + describe "that are new with approver" do + subject do + Notify.new_merge_request_email( + merge_request_with_approver.assignee_id, + merge_request_with_approver.id + ) + end + + it "contains the approvers list" do + is_expected.to have_body_text /#{merge_request_with_approver.approvers.first.user.name}/ + end + end + describe 'that are new with a description' do - subject { Notify.new_merge_request_email(merge_request_with_description.assignee_id, merge_request_with_description.id) } + subject do + Notify.new_merge_request_email( + merge_request_with_description.assignee_id, + merge_request_with_description.id + ) + end it 'contains the description' do is_expected.to have_body_text /#{merge_request_with_description.description}/