Merge branch 'approvers_email' into 'master'

When someone as marked as a required approver for a merge request, an email should be sent

https://gitlab.com/gitlab-org/gitlab-ee/issues/16

See merge request !44

CHANGELOG-EE

 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)

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

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)

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 %>
 

spec/factories/approvers.rb

+# Read about factories at https://github.com/thoughtbot/factory_girl
+
+FactoryGirl.define do
+  factory :approver do
+    target factory: :merge_request
+    user
+  end
+end

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

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}/