diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index d58dab2d6668176e9a3352589026e85a6a762b28..931298df5d825d5d254a629a8fd00b6b5737c885 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 b1049fab788513af0d3eb37a6254cc7dd56447a9..acc86b1a8cd0f2da2859b634838b93de75788b14 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 c5f04dbcf4c53722a54c484c78d39c8e23a88475..7b372399a3ab3e8607de2e5eebfb9cdc90b17342 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 7963af127e1c61c92fc5361518fef0e4dd7c43bb..db8d18a7d840e6d10ae618cf494d73ba7140ba2f 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 a4418360b8c336be7966840c4559c49d7eef9f13..d5cae2f98f798a4e563517426adb9b602f3e1b87 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 335ef32abced5aa67354b0dd46e2773cfca35464..e5024857201f97f99458b748d6ff259c1bd50f14 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 c9846e9f26f7e134becb0fd8049c48913af2bda9..5e8281a3fd008f295ec3f74e0a9391df62b64bb5 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 2afc5f81251c0ca4edf7496dae9c978c1d612a42..2a51241971f6a6e2e8601f648f0b18e432048722 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 f38943349687138efca4f395c3ab4b3bedc4b353..ddd1a7bd63dd211440824ff9e16760a6622385fe 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 5d1a8555b7d69e08b7071f0c163de4e7189bdd8d..c87c0a252b1f56f6489c4cf505ccd5e91f86e2c2 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
diff --git a/config/routes.rb b/config/routes.rb
index 917c3d3f1edae0f29dc16e4d6331698e2d7d1adb..993687665ccf0eb0dac4eb98a4220e74d8896bd8 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -556,8 +556,7 @@ Gitlab::Application.routes.draw do
get :diffs
get :commits
post :merge
- delete :merge, action: :cancel_merge_when_build_succeeds
- get :merge_check
+ post :cancel_merge_when_build_succeeds
get :ci_status
post :toggle_subscription
end
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index ffa7f2cdf14d388d6c1bfaff2e3d73863168c73a..1b2ad1caf494d47fe0fe4c4b7608b7d85fa88dec 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -2,8 +2,8 @@
## List merge requests
-Get all merge requests for this project.
-The `state` parameter can be used to get only merge requests with a given state (`opened`, `closed`, or `merged`) or all of them (`all`).
+Get all merge requests for this project.
+The `state` parameter can be used to get only merge requests with a given state (`opened`, `closed`, or `merged`) or all of them (`all`).
The pagination parameters `page` and `per_page` can be used to restrict the list of merge requests.
```
@@ -292,9 +292,59 @@ PUT /projects/:id/merge_request/:merge_request_id/merge
Parameters:
-- `id` (required) - The ID of a project
-- `merge_request_id` (required) - ID of MR
-- `merge_commit_message` (optional) - Custom merge commit message
+- `id` (required) - The ID of a project
+- `merge_request_id` (required) - ID of MR
+- `merge_commit_message` (optional) - Custom merge commit message
+- `should_remove_source_branch` (optional) - if `true` removes the source branch
+- `merge_when_build_succeeds` (optional) - if `true` the MR is merge when the build succeeds
+
+```json
+{
+ "id": 1,
+ "target_branch": "master",
+ "source_branch": "test1",
+ "project_id": 3,
+ "title": "test1",
+ "state": "merged",
+ "upvotes": 0,
+ "downvotes": 0,
+ "author": {
+ "id": 1,
+ "username": "admin",
+ "email": "admin@example.com",
+ "name": "Administrator",
+ "state": "active",
+ "created_at": "2012-04-29T08:46:00Z"
+ },
+ "assignee": {
+ "id": 1,
+ "username": "admin",
+ "email": "admin@example.com",
+ "name": "Administrator",
+ "state": "active",
+ "created_at": "2012-04-29T08:46:00Z"
+ }
+}
+```
+
+## Cancel Merge When Build Succeeds
+
+Cancels the merge when build succeeds and reset the merge parameters
+
+If successfull you'll get `200 OK`.
+
+If you don't have permissions to accept this merge request - you'll get a 401
+
+If the merge request is already merged or closed - you get 405 and error message 'Method Not Allowed'
+
+In case the merge request is not set to be merged when the build succeeds, you'll also get a 405 with the error message 'Method Not Allowed'
+```
+PUT /projects/:id/merge_request/:merge_request_id/cancel_merge_when_build_succeeds
+```
+Parameters:
+
+- `id` (required) - The ID of a project
+- `merge_request_id` (required) - ID of MR
```json
{
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 6eb84baf9cb0aab677a820a2f4b05ad861e832bd..f981432db3685957e9c8d0711ef3e220c07980de 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -179,9 +179,10 @@ module API
# Merge MR
#
# Parameters:
- # id (required) - The ID of a project
- # merge_request_id (required) - ID of MR
- # merge_commit_message (optional) - Custom merge commit message
+ # id (required) - The ID of a project
+ # merge_request_id (required) - ID of MR
+ # merge_commit_message (optional) - Custom merge commit message
+ # merge_when_build_succeeds (optional) - truethy when this MR should be merged when the build is succesfull
# Example:
# PUT /projects/:id/merge_request/:merge_request_id/merge
#
@@ -191,34 +192,57 @@ module API
allowed = ::Gitlab::GitAccess.new(current_user, user_project).
can_push_to_branch?(merge_request.target_branch)
- if allowed
- if merge_request.unchecked?
- merge_request.check_if_can_be_merged
- end
+ # Merge request can not be merged
+ # because user dont have permissions to push into target branch
+ unauthorized! unless allowed
+
+ not_allowed! unless merge_request.open?
- if merge_request.open? && !merge_request.work_in_progress?
- if merge_request.can_be_merged?
- commit_message = params[:merge_commit_message] || merge_request.merge_commit_message
+ merge_request.check_if_can_be_merged if merge_request.unchecked?
- ::MergeRequests::MergeService.new(merge_request.target_project, current_user).
- execute(merge_request, commit_message)
+ merge_params = {
+ commit_message: params[:merge_commit_message],
+ should_remove_source_branch: params[:should_remove_source_branch]
+ }
- present merge_request, with: Entities::MergeRequest
- else
- render_api_error!('Branch cannot be merged', 405)
- end
+ if !merge_request.work_in_progress?
+ if parse_boolean(params[:merge_when_build_succeeds])
+ ::MergeRequest::MergeWhenBuildSucceedsService.new(merge_request.target_project, current_user, merge_params).
+ execute(merge_request)
else
- # Merge request can not be merged
- # because it is already closed/merged or marked as WIP
- not_allowed!
+ ::MergeRequests::MergeService.new(merge_request.target_project, current_user, merge_params).
+ execute(merge_request, merge_params)
end
else
- # Merge request can not be merged
- # because user dont have permissions to push into target branch
- unauthorized!
+ render_api_error!('Branch cannot be merged', 405)
end
+
+ present merge_request, with: Entities::MergeRequest
end
+ # Cancel Merge if Merge When build succeeds is enabled
+ # Parameters:
+ # id (required) - The ID of a project
+ # merge_request_id (required) - ID of MR
+ #
+ post ":id/merge_request/:merge_request_id/cancel_merge_when_build_succeeds" do
+ merge_request = user_project.merge_requests.find(params[:merge_request_id])
+
+ allowed = ::Gitlab::GitAccess.new(current_user, user_project).
+ can_push_to_branch?(merge_request.target_branch)
+
+ # Merge request can not be merged
+ # because user dont have permissions to push into target branch
+ unauthorized! unless allowed
+
+ if merge_request.merged? || !merge_request.open? || !merge_request.merge_when_build_succeeds?
+ not_allowed!
+ end
+
+ merge_request.reset_merge_when_build_succeeds
+
+ present merge_request, with: Entities::MergeRequest
+ end
# Get a merge request's comments
#
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index eed2cbc54128104814664888ca94702bdbb07a73..e42534a8491a448ebb38f55294add2588e124af3 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -30,7 +30,7 @@ describe MergeRequest do
describe 'associations' do
it { is_expected.to belong_to(:target_project).with_foreign_key(:target_project_id).class_name('Project') }
it { is_expected.to belong_to(:source_project).with_foreign_key(:source_project_id).class_name('Project') }
-
+ it { is_expected.to belong_to(:merge_user).class_name("User") }
it { is_expected.to have_one(:merge_request_diff).dependent(:destroy) }
end
@@ -53,6 +53,8 @@ describe MergeRequest do
it { is_expected.to respond_to(:unchecked?) }
it { is_expected.to respond_to(:can_be_merged?) }
it { is_expected.to respond_to(:cannot_be_merged?) }
+ it { is_expected.to respond_to(:merge_params) }
+ it { is_expected.to respond_to(:merge_when_build_succeeds) }
end
describe '#to_reference' do
@@ -171,6 +173,16 @@ describe MergeRequest do
end
end
+ describe "#reset_merge_when_build_succeeds" do
+ let(:merge_if_green) { create :merge_request, merge_when_build_succeeds: true }
+ it "sets the item to false" do
+ merge_if_green.reset_merge_when_build_succeeds
+ merge_if_green.reload
+
+ expect(merge_if_green.merge_when_build_succeeds).to be_falsey
+ end
+ end
+
describe "#hook_attrs" do
it "has all the required keys" do
attrs = subject.hook_attrs
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index a45130bd473232cf9de5f426d15315b34d17481c..35912ece6441506a25aaa10f8fd9dad4f67b5da1 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -207,6 +207,22 @@ describe SystemNoteService do
end
end
+ describe '.merge_when_build_succeeds' do
+ let(:ci_commit) { create :ci_commit, gl_project: project }
+ let(:merge_request) { create :merge_request, project: project }
+
+ subject { described_class.merge_when_build_succeeds(merge_request, project, author) }
+
+ it_behaves_like 'a system note'
+
+ it "posts the Merge When Build Succeeds system note" do
+ allow(merge_request).to receive(:ci_commit).and_return(ci_commit)
+ allow(ci_commit).to receive(:short_sha).and_return('12345678')
+
+ expect(subject.note).to eq "This merge request will be automatically merged when the build for 12345678 succeeds"
+ end
+ end
+
describe '.change_title' do
subject { described_class.change_title(noteable, project, author, 'Old title') }