From 582540889a15f5abcfcf9ff946a676500ea828bf Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <pierre@capitainetrain.com>
Date: Mon, 24 Mar 2014 15:11:35 +0100
Subject: [PATCH] Fix the merge notification email not being sent

The 'author_id_of_changes' attribute is not persisted in the database.
As we retrieve the merge request from the DB just before sending the
email, this attribute was always nil.

Also there was no tests for the merge notification code - tests have
been added.

Fix #6605
---
 app/mailers/emails/merge_requests.rb       |  4 ++--
 app/services/notification_service.rb       |  2 +-
 spec/mailers/notify_spec.rb                | 24 ++++++++++++++++++++++
 spec/services/notification_service_spec.rb |  6 +++---
 4 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb
index 5e1b8faf13..a97d55f1b5 100644
--- a/app/mailers/emails/merge_requests.rb
+++ b/app/mailers/emails/merge_requests.rb
@@ -29,11 +29,11 @@ module Emails
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
 
-    def merged_merge_request_email(recipient_id, merge_request_id)
+    def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
       @merge_request = MergeRequest.find(merge_request_id)
       @project = @merge_request.project
       @target_url = project_merge_request_url(@project, @merge_request)
-      mail(from: sender(@merge_request.author_id_of_changes),
+      mail(from: sender(updated_by_user_id),
            to: recipient(recipient_id),
            subject: subject("#{@merge_request.title} (!#{@merge_request.iid})"))
     end
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 5daf573630..44fe9893e7 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -91,7 +91,7 @@ class NotificationService
     recipients = recipients.concat(project_watchers(merge_request.target_project)).uniq
 
     recipients.each do |recipient|
-      mailer.merged_merge_request_email(recipient.id, merge_request.id)
+      mailer.merged_merge_request_email(recipient.id, merge_request.id, merge_request.author_id_of_changes)
     end
   end
 
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index f990ed659b..22d60429cc 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -229,6 +229,7 @@ describe Notify do
       end
 
       context 'for merge requests' 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: Faker::Lorem.sentence) }
 
@@ -288,7 +289,30 @@ describe Notify do
           it 'contains a link to the merge request' do
             should have_body_text /#{project_merge_request_path project, merge_request}/
           end
+        end
 
+        describe 'that are merged' do
+          subject { Notify.merged_merge_request_email(recipient.id, merge_request.id, merge_author.id) }
+
+          it_behaves_like 'a multiple recipients email'
+
+          it 'is sent as the merge author' do
+            sender = subject.header[:from].addrs[0]
+            sender.display_name.should eq(merge_author.name)
+            sender.address.should eq(gitlab_sender)
+          end
+
+          it 'has the correct subject' do
+            should have_subject /#{merge_request.title} \(!#{merge_request.iid}\)/
+          end
+
+          it 'contains the new status' do
+            should have_body_text /merged/i
+          end
+
+          it 'contains a link to the merge request' do
+            should have_body_text /#{project_merge_request_path project, merge_request}/
+          end
         end
       end
     end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 077ad8b6e1..59c17d6e4d 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -233,15 +233,15 @@ describe NotificationService do
         should_email(@u_watcher.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
-        notification.merge_mr(merge_request)
+        notification.merge_mr(merge_request, @u_disabled)
       end
 
       def should_email(user_id)
-        Notify.should_receive(:merged_merge_request_email).with(user_id, merge_request.id)
+        Notify.should_receive(:merged_merge_request_email).with(user_id, merge_request.id, @u_disabled.id)
       end
 
       def should_not_email(user_id)
-        Notify.should_not_receive(:merged_merge_request_email).with(user_id, merge_request.id)
+        Notify.should_not_receive(:merged_merge_request_email).with(user_id, merge_request.id, @u_disabled.id)
       end
     end
   end