Merge branch 'merge-system-note' into 'master'

Refactor merge request merge service

See merge request !1374

6 files changed, 19 insertions, 24 deletions

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 2cc427d35c2..de0ee0e2c5a 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -189,7 +189,9 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def automerge!(current_user, commit_message = nil)
-    MergeRequests::AutoMergeService.new.execute(self, current_user, commit_message)
+    MergeRequests::AutoMergeService.
+      new(target_project, current_user).
+      execute(self, commit_message)
   end
 
   def open?
diff --git a/app/services/merge_requests/auto_merge_service.rb b/app/services/merge_requests/auto_merge_service.rb
index 20b88d1510c..b5d90a74e15 100644
--- a/app/services/merge_requests/auto_merge_service.rb
+++ b/app/services/merge_requests/auto_merge_service.rb
@@ -5,15 +5,16 @@ module MergeRequests
   # mark merge request as merged and execute all hooks and notifications
   # Called when you do merge via GitLab UI
   class AutoMergeService < BaseMergeService
-    def execute(merge_request, current_user, commit_message)
+    def execute(merge_request, commit_message)
       merge_request.lock_mr
 
       if Gitlab::Satellite::MergeAction.new(current_user, merge_request).merge!(commit_message)
         merge_request.merge
 
-        notification.merge_mr(merge_request, current_user)
+        notification_service.merge_mr(merge_request, current_user)
         create_merge_event(merge_request, current_user)
-        execute_project_hooks(merge_request)
+        create_note(merge_request)
+        execute_hooks(merge_request)
 
         true
       else
diff --git a/app/services/merge_requests/base_merge_service.rb b/app/services/merge_requests/base_merge_service.rb
index 700a21ca011..9579573adf9 100644
--- a/app/services/merge_requests/base_merge_service.rb
+++ b/app/services/merge_requests/base_merge_service.rb
@@ -1,21 +1,10 @@
 module MergeRequests
-  class BaseMergeService
+  class BaseMergeService < MergeRequests::BaseService
 
     private
 
-    def notification
-      NotificationService.new
-    end
-
     def create_merge_event(merge_request, current_user)
       EventCreateService.new.merge_mr(merge_request, current_user)
     end
-
-    def execute_project_hooks(merge_request)
-      if merge_request.project
-        hook_data = merge_request.to_hook_data(current_user)
-        merge_request.project.execute_hooks(hook_data, :merge_request_hooks)
-      end
-    end
   end
 end
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 680766140bd..5de7247d617 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -6,12 +6,13 @@ module MergeRequests
   # Called when you do merge via command line and push code
   # to target branch
   class MergeService < BaseMergeService
-    def execute(merge_request, current_user, commit_message)
+    def execute(merge_request, commit_message)
       merge_request.merge
 
-      notification.merge_mr(merge_request, current_user)
+      notification_service.merge_mr(merge_request, current_user)
       create_merge_event(merge_request, current_user)
-      execute_project_hooks(merge_request)
+      create_note(merge_request)
+      execute_hooks(merge_request)
 
       true
     rescue
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index baf0936cc3d..a6705de61f2 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -32,7 +32,9 @@ module MergeRequests
 
 
       merge_requests.uniq.select(&:source_project).each do |merge_request|
-        MergeRequests::MergeService.new.execute(merge_request, @current_user, nil)
+        MergeRequests::MergeService.
+          new(merge_request.target_project, @current_user).
+          execute(merge_request, nil)
       end
     end
 
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 9f294152053..35c7aac94df 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -47,10 +47,10 @@ describe MergeRequests::RefreshService do
         reload_mrs
       end
 
-      it { @merge_request.notes.should be_empty }
+      it { @merge_request.notes.last.note.should include('changed to merged') }
       it { @merge_request.should be_merged }
       it { @fork_merge_request.should be_merged }
-      it { @fork_merge_request.notes.should be_empty }
+      it { @fork_merge_request.notes.last.note.should include('changed to merged') }
     end
 
     context 'push to fork repo source branch' do
@@ -61,7 +61,7 @@ describe MergeRequests::RefreshService do
 
       it { @merge_request.notes.should be_empty }
       it { @merge_request.should be_open }
-      it { @fork_merge_request.notes.should_not be_empty }
+      it { @fork_merge_request.notes.last.note.should include('new commit') }
       it { @fork_merge_request.should be_open }
     end
 
@@ -84,7 +84,7 @@ describe MergeRequests::RefreshService do
         reload_mrs
       end
 
-      it { @merge_request.notes.should be_empty }
+      it { @merge_request.notes.last.note.should include('changed to merged') }
       it { @merge_request.should be_merged }
       it { @fork_merge_request.should be_open }
       it { @fork_merge_request.notes.should be_empty }