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

CHANGELOG

@@ -23,6 +23,7 @@ v 8.4.0 (unreleased)
   - Enable Microsoft Azure OAuth2 support (Janis Meybohm)
 
 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 (Stan Hu)
 

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

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

app/views/projects/merge_requests/_show.html.haml

@@ -38,7 +38,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

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?