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

5 files changed, 15 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index d841b149615..240fa43d6cb 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -20,6 +20,7 @@ v 8.4.0 (unreleased)
   - Fix API project lookups when querying with a namespace with dots (Stan Hu)
 
 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)
 
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 ba7c2c01e93..095876450a0 100644
--- a/app/views/projects/merge_requests/_show.html.haml
+++ b/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
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?