From f076e4ebeb5cba94233f9795db6c60a60a553dff Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Wed, 6 Jan 2016 18:58:10 +0000 Subject: 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 | 1 + app/controllers/projects/merge_requests_controller.rb | 2 +- app/models/merge_request.rb | 16 +++++++++++----- app/views/projects/merge_requests/_show.html.haml | 2 +- lib/api/merge_requests.rb | 2 +- 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? -- cgit v1.2.3