API support, incorporated feedback

10 files changed, 26 insertions, 33 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index d58dab2d666..931298df5d8 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -171,9 +171,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @merge_request.update(merge_error: nil)
 
     if params[:merge_when_build_succeeds] && @merge_request.ci_commit.active?
-      MergeRequests::MergeWhenBuildSucceedsService.new(@project,
-                                                      current_user,
-                                                      merge_params: merge_params)
+      MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user, merge_params)
                                                       .execute(@merge_request)
       @status = :merge_when_build_succeeds
     else
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index b1049fab788..acc86b1a8cd 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -75,7 +75,7 @@ class CommitStatus < ActiveRecord::Base
       build.update_attributes finished_at: Time.now
     end
 
-    after_transition running: :success do |build, transition|
+    after_transition [:pending, :running] => :success do |build, transition|
       MergeRequests::MergeWhenBuildSucceedsService.new(build.commit.gl_project, nil).trigger(build)
     end
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index c5f04dbcf4c..7b372399a3a 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -253,6 +253,10 @@ class MergeRequest < ActiveRecord::Base
     end
   end
 
+  def mergeable_by_or_author(user)
+    self.can_be_merged_by?(user) || self.author == user
+  end
+
   def mr_and_commit_notes
     # Fetch comments only from last 100 commits
     commits_for_notes_limit = 100
@@ -390,7 +394,7 @@ class MergeRequest < ActiveRecord::Base
 
   def reset_merge_when_build_succeeds
     return unless merge_when_build_succeeds?
-    
+
     self.merge_when_build_succeeds = false
     self.merge_user = nil
     self.merge_params = nil
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 7963af127e1..db8d18a7d84 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -6,15 +6,12 @@ module MergeRequests
   # Executed when you do merge via GitLab UI
   #
   class MergeService < MergeRequests::BaseService
-    attr_reader :merge_request, :commit_message
+    attr_reader :merge_request
 
-    def execute(merge_request, commit_message)
-      @commit_message = commit_message
+    def execute(merge_request)
       @merge_request = merge_request
 
-      unless @merge_request.mergeable?
-        return error('Merge request is not mergeable')
-      end
+      return error('Merge request is not mergeable') unless @merge_request.mergeable?
 
       merge_request.in_locked_state do
         if commit
@@ -32,7 +29,7 @@ module MergeRequests
       committer = repository.user_to_committer(current_user)
 
       options = {
-        message: commit_message,
+        message: params[:commit_message] || merge_request.merge_commit_message,
         author: committer,
         committer: committer
       }
@@ -46,6 +43,11 @@ module MergeRequests
 
     def after_merge
       MergeRequests::PostMergeService.new(project, current_user).execute(merge_request)
+
+      if params[:should_remove_source_branch]
+        DeleteBranchService.new(@merge_request.source_project, current_user).
+          execute(merge_request.source_branch)
+      end
     end
   end
 end
diff --git a/app/services/merge_requests/merge_when_build_succeeds_service.rb b/app/services/merge_requests/merge_when_build_succeeds_service.rb
index a4418360b8c..d5cae2f98f7 100644
--- a/app/services/merge_requests/merge_when_build_succeeds_service.rb
+++ b/app/services/merge_requests/merge_when_build_succeeds_service.rb
@@ -1,7 +1,7 @@
 module MergeRequests
   class MergeWhenBuildSucceedsService < MergeRequests::BaseService
     def execute(merge_request)
-      merge_request.merge_params.merge!(params[:merge_params])
+      merge_request.merge_params.merge!(params)
 
       # The service is also called when the merge params are updated.
       already_approved = merge_request.merge_when_build_succeeds?
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 335ef32abce..e5024857201 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -77,9 +77,7 @@ module MergeRequests
     end
 
     def reset_merge_when_build_succeeds
-      merge_requests_for_source_branch.each do |merge_request|
-        merge_request.reset_merge_when_build_succeeds
-      end
+      merge_requests_for_source_branch.each(&:reset_merge_when_build_succeeds)
     end
 
     def find_new_commits
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index c9846e9f26f..5e8281a3fd0 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -132,7 +132,7 @@ class SystemNoteService
 
   # Called when 'merge when build succeeds' is executed
   def self.merge_when_build_succeeds(noteable, project, author)
-    body = "Approved this request to be merged automatically when the build succeeds"
+    body = "This merge request will be automatically merged when the build for #{noteable.ci_commit.short_sha} succeeds"
 
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml
index 2afc5f81251..2a51241971f 100644
--- a/app/views/projects/merge_requests/widget/open/_accept.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml
@@ -29,5 +29,5 @@
     $('.accept_merge_request').on 'click', ->
       btn = $(this)
       btn.html("<i class='fa fa-spinner fa-spin'></i> Merge in progress")
-    $('.accept-mr-form').on 'ajax:sen', ->
+    $('.accept-mr-form').on 'ajax:send', ->
       $(".accept-mr-form :input").disable()
diff --git a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
index f3894334968..ddd1a7bd63d 100644
--- a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
@@ -1,9 +1,9 @@
 %h4
   Approved by #{link_to_member(@project, @merge_request.merge_user, avatar: true)}
   to be merged automatically when
-  #{link_to "the build", ci_status_path(@merge_request.ci_commit)} succeeds
+  #{link_to "the build", ci_status_path(@merge_request.ci_commit)} succeeds.
 %div
-  - if @merge_request.merge_params["should_remove_source_branch"]
+  - if @merge_request.merge_params["should_remove_source_branch"].present?
     = succeed '.' do
       The changes will be merged into
       %span.label-branch= @merge_request.target_branch
@@ -19,9 +19,9 @@
 - if remove_source_branch_button || @merge_request.can_be_merged_by?(current_user)
   .clearfix.prepend-top-10
     - if remove_source_branch_button
-      = link_to merge_namespace_project_merge_request_path(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
+      = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
         = icon('times')
         Remove Source Branch When Merged
     - if @merge_request.can_be_merged_by?(current_user) || @merge_request.author == current_user
-      = link_to merge_namespace_project_merge_request_path(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request), remote: true, method: :delete, class: "btn btn-grouped btn-warning btn-sm" do
+      = link_to cancel_merge_when_build_succeeds_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request), remote: true, method: :post, class: "btn btn-grouped btn-warning btn-sm" do
         Cancel Automatic Merge
diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb
index 5d1a8555b7d..c87c0a252b1 100644
--- a/app/workers/merge_worker.rb
+++ b/app/workers/merge_worker.rb
@@ -8,16 +8,7 @@ class MergeWorker
     current_user = User.find(current_user_id)
     merge_request = MergeRequest.find(merge_request_id)
 
-    result = MergeRequests::MergeService.new(merge_request.target_project, current_user).
-      execute(merge_request, params[:commit_message])
-
-    if result[:status] == :success && params[:should_remove_source_branch].present?
-      DeleteBranchService.new(merge_request.source_project, current_user).
-        execute(merge_request.source_branch)
-
-      merge_request.source_project.repository.expire_branch_names
-    end
-
-    result
+    MergeRequests::MergeService.new(merge_request.target_project, current_user, params).
+      execute(merge_request)
   end
 end