Merge branch 'merge-when-build-succeeds-unchecked' into 'master'

Get "Merge when build succeeds" to work when commits were pushed to MR
target branch while builds were running

The Merge when build succeeds service only merges when the MR is
mergeable (open, not WIP, no conflicts).

When the target branch is updated, all affected MRs have their merge
status set to `unchecked`, and the conflicts check will only happen
when `check_if_can_be_merged` is called, which happens when the MR page
is viewed.

When someone enables the automatic merge, the target branch is updated,
no-one views the MR page again, and the build succeeds, the mergeability
check will fail and the MR will not in fact be merged.

This MR makes sure `check_if_can_be_merged` is always called when MR
mergeability is checked.

See merge request !2304

5 files changed, 15 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 1df2ff4e166..16866abe4ba 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.3.3 (unreleased)
+  - Get "Merge when build succeeds" to work when commits were pushed to MR target branch while builds were running
   - Fix project transfer e-mail sending incorrect paths in e-mail notification (Stan Hu)
   - Enable "Add key" button when user fills in a proper key
   - Better support for referencing and closing issues in Asana service (Mike Wyatt)
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index ab5c953189c..de948d271c8 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -153,7 +153,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
 
   def merge_check
-    @merge_request.check_if_can_be_merged if @merge_request.unchecked?
+    @merge_request.check_if_can_be_merged
 
     render partial: "projects/merge_requests/widget/show.html.haml", layout: false
   end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index ac25d38eb63..30d0c2b5961 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -229,6 +229,8 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def check_if_can_be_merged
+    return unless unchecked?
+
     can_be_merged =
       project.repository.can_be_merged?(source_sha, target_branch)
 
@@ -252,7 +254,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def mergeable?
-    open? && !work_in_progress? && can_be_merged?
+    return false unless open? && !work_in_progress?
+
+    check_if_can_be_merged
+
+    can_be_merged?
   end
 
   def gitlab_merge_status
@@ -452,6 +458,10 @@ class MergeRequest < ActiveRecord::Base
     !source_branch_exists? || !target_branch_exists?
   end
 
+  def broken?
+    self.commits.blank? || branch_missing? || cannot_be_merged?
+  end
+
   def can_be_merged_by?(user)
     ::Gitlab::GitAccess.new(user, project).can_push_to_branch?(target_branch)
   end
@@ -507,8 +517,4 @@ class MergeRequest < ActiveRecord::Base
   def ci_commit
     @ci_commit ||= source_project.ci_commit(last_commit.id) if last_commit && source_project
   end
-
-  def broken?
-    self.commits.blank? || branch_missing? || cannot_be_merged?
-  end
 end
diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml
index e9ffbd06be2..184f37894b3 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/app/views/projects/merge_requests/_show.html.haml
@@ -35,7 +35,7 @@
     = render "projects/merge_requests/show/how_to_merge"
     = render "projects/merge_requests/widget/show.html.haml"
 
-    - if @merge_request.open? && @merge_request.source_branch_exists? && @merge_request.can_be_merged? && @merge_request.can_be_merged_by?(current_user)
+    - if @merge_request.source_branch_exists? && @merge_request.mergeable? && @merge_request.can_be_merged_by?(current_user)
       .light.prepend-top-default
         You can also accept this merge request manually using the
         = succeed '.' do
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 3c1c6bda260..5c97fe1c88c 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -211,7 +211,7 @@ module API
         unauthorized! unless merge_request.can_be_merged_by?(current_user)
         not_allowed! if !merge_request.open? || merge_request.work_in_progress?
 
-        merge_request.check_if_can_be_merged if merge_request.unchecked?
+        merge_request.check_if_can_be_merged
 
         render_api_error!('Branch cannot be merged', 406) unless merge_request.can_be_merged?