API support, incorporated feedback
This commit is contained in:
parent
63b234706d
commit
2f048df4a4
|
@ -171,9 +171,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
|
||||||
@merge_request.update(merge_error: nil)
|
@merge_request.update(merge_error: nil)
|
||||||
|
|
||||||
if params[:merge_when_build_succeeds] && @merge_request.ci_commit.active?
|
if params[:merge_when_build_succeeds] && @merge_request.ci_commit.active?
|
||||||
MergeRequests::MergeWhenBuildSucceedsService.new(@project,
|
MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user, merge_params)
|
||||||
current_user,
|
|
||||||
merge_params: merge_params)
|
|
||||||
.execute(@merge_request)
|
.execute(@merge_request)
|
||||||
@status = :merge_when_build_succeeds
|
@status = :merge_when_build_succeeds
|
||||||
else
|
else
|
||||||
|
|
|
@ -75,7 +75,7 @@ class CommitStatus < ActiveRecord::Base
|
||||||
build.update_attributes finished_at: Time.now
|
build.update_attributes finished_at: Time.now
|
||||||
end
|
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)
|
MergeRequests::MergeWhenBuildSucceedsService.new(build.commit.gl_project, nil).trigger(build)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -253,6 +253,10 @@ class MergeRequest < ActiveRecord::Base
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def mergeable_by_or_author(user)
|
||||||
|
self.can_be_merged_by?(user) || self.author == user
|
||||||
|
end
|
||||||
|
|
||||||
def mr_and_commit_notes
|
def mr_and_commit_notes
|
||||||
# Fetch comments only from last 100 commits
|
# Fetch comments only from last 100 commits
|
||||||
commits_for_notes_limit = 100
|
commits_for_notes_limit = 100
|
||||||
|
|
|
@ -6,15 +6,12 @@ module MergeRequests
|
||||||
# Executed when you do merge via GitLab UI
|
# Executed when you do merge via GitLab UI
|
||||||
#
|
#
|
||||||
class MergeService < MergeRequests::BaseService
|
class MergeService < MergeRequests::BaseService
|
||||||
attr_reader :merge_request, :commit_message
|
attr_reader :merge_request
|
||||||
|
|
||||||
def execute(merge_request, commit_message)
|
def execute(merge_request)
|
||||||
@commit_message = commit_message
|
|
||||||
@merge_request = merge_request
|
@merge_request = merge_request
|
||||||
|
|
||||||
unless @merge_request.mergeable?
|
return error('Merge request is not mergeable') unless @merge_request.mergeable?
|
||||||
return error('Merge request is not mergeable')
|
|
||||||
end
|
|
||||||
|
|
||||||
merge_request.in_locked_state do
|
merge_request.in_locked_state do
|
||||||
if commit
|
if commit
|
||||||
|
@ -32,7 +29,7 @@ module MergeRequests
|
||||||
committer = repository.user_to_committer(current_user)
|
committer = repository.user_to_committer(current_user)
|
||||||
|
|
||||||
options = {
|
options = {
|
||||||
message: commit_message,
|
message: params[:commit_message] || merge_request.merge_commit_message,
|
||||||
author: committer,
|
author: committer,
|
||||||
committer: committer
|
committer: committer
|
||||||
}
|
}
|
||||||
|
@ -46,6 +43,11 @@ module MergeRequests
|
||||||
|
|
||||||
def after_merge
|
def after_merge
|
||||||
MergeRequests::PostMergeService.new(project, current_user).execute(merge_request)
|
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
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
module MergeRequests
|
module MergeRequests
|
||||||
class MergeWhenBuildSucceedsService < MergeRequests::BaseService
|
class MergeWhenBuildSucceedsService < MergeRequests::BaseService
|
||||||
def execute(merge_request)
|
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.
|
# The service is also called when the merge params are updated.
|
||||||
already_approved = merge_request.merge_when_build_succeeds?
|
already_approved = merge_request.merge_when_build_succeeds?
|
||||||
|
|
|
@ -77,9 +77,7 @@ module MergeRequests
|
||||||
end
|
end
|
||||||
|
|
||||||
def reset_merge_when_build_succeeds
|
def reset_merge_when_build_succeeds
|
||||||
merge_requests_for_source_branch.each do |merge_request|
|
merge_requests_for_source_branch.each(&:reset_merge_when_build_succeeds)
|
||||||
merge_request.reset_merge_when_build_succeeds
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def find_new_commits
|
def find_new_commits
|
||||||
|
|
|
@ -132,7 +132,7 @@ class SystemNoteService
|
||||||
|
|
||||||
# Called when 'merge when build succeeds' is executed
|
# Called when 'merge when build succeeds' is executed
|
||||||
def self.merge_when_build_succeeds(noteable, project, author)
|
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)
|
create_note(noteable: noteable, project: project, author: author, note: body)
|
||||||
end
|
end
|
||||||
|
|
|
@ -29,5 +29,5 @@
|
||||||
$('.accept_merge_request').on 'click', ->
|
$('.accept_merge_request').on 'click', ->
|
||||||
btn = $(this)
|
btn = $(this)
|
||||||
btn.html("<i class='fa fa-spinner fa-spin'></i> Merge in progress")
|
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()
|
$(".accept-mr-form :input").disable()
|
||||||
|
|
|
@ -1,9 +1,9 @@
|
||||||
%h4
|
%h4
|
||||||
Approved by #{link_to_member(@project, @merge_request.merge_user, avatar: true)}
|
Approved by #{link_to_member(@project, @merge_request.merge_user, avatar: true)}
|
||||||
to be merged automatically when
|
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
|
%div
|
||||||
- if @merge_request.merge_params["should_remove_source_branch"]
|
- if @merge_request.merge_params["should_remove_source_branch"].present?
|
||||||
= succeed '.' do
|
= succeed '.' do
|
||||||
The changes will be merged into
|
The changes will be merged into
|
||||||
%span.label-branch= @merge_request.target_branch
|
%span.label-branch= @merge_request.target_branch
|
||||||
|
@ -19,9 +19,9 @@
|
||||||
- if remove_source_branch_button || @merge_request.can_be_merged_by?(current_user)
|
- if remove_source_branch_button || @merge_request.can_be_merged_by?(current_user)
|
||||||
.clearfix.prepend-top-10
|
.clearfix.prepend-top-10
|
||||||
- if remove_source_branch_button
|
- 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')
|
= icon('times')
|
||||||
Remove Source Branch When Merged
|
Remove Source Branch When Merged
|
||||||
- if @merge_request.can_be_merged_by?(current_user) || @merge_request.author == current_user
|
- 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
|
Cancel Automatic Merge
|
||||||
|
|
|
@ -8,16 +8,7 @@ class MergeWorker
|
||||||
current_user = User.find(current_user_id)
|
current_user = User.find(current_user_id)
|
||||||
merge_request = MergeRequest.find(merge_request_id)
|
merge_request = MergeRequest.find(merge_request_id)
|
||||||
|
|
||||||
result = MergeRequests::MergeService.new(merge_request.target_project, current_user).
|
MergeRequests::MergeService.new(merge_request.target_project, current_user, params).
|
||||||
execute(merge_request, params[:commit_message])
|
execute(merge_request)
|
||||||
|
|
||||||
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
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -556,8 +556,7 @@ Gitlab::Application.routes.draw do
|
||||||
get :diffs
|
get :diffs
|
||||||
get :commits
|
get :commits
|
||||||
post :merge
|
post :merge
|
||||||
delete :merge, action: :cancel_merge_when_build_succeeds
|
post :cancel_merge_when_build_succeeds
|
||||||
get :merge_check
|
|
||||||
get :ci_status
|
get :ci_status
|
||||||
post :toggle_subscription
|
post :toggle_subscription
|
||||||
end
|
end
|
||||||
|
|
|
@ -295,6 +295,56 @@ Parameters:
|
||||||
- `id` (required) - The ID of a project
|
- `id` (required) - The ID of a project
|
||||||
- `merge_request_id` (required) - ID of MR
|
- `merge_request_id` (required) - ID of MR
|
||||||
- `merge_commit_message` (optional) - Custom merge commit message
|
- `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
|
```json
|
||||||
{
|
{
|
||||||
|
|
|
@ -182,6 +182,7 @@ module API
|
||||||
# id (required) - The ID of a project
|
# id (required) - The ID of a project
|
||||||
# merge_request_id (required) - ID of MR
|
# merge_request_id (required) - ID of MR
|
||||||
# merge_commit_message (optional) - Custom merge commit message
|
# 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:
|
# Example:
|
||||||
# PUT /projects/:id/merge_request/:merge_request_id/merge
|
# PUT /projects/:id/merge_request/:merge_request_id/merge
|
||||||
#
|
#
|
||||||
|
@ -191,34 +192,57 @@ module API
|
||||||
allowed = ::Gitlab::GitAccess.new(current_user, user_project).
|
allowed = ::Gitlab::GitAccess.new(current_user, user_project).
|
||||||
can_push_to_branch?(merge_request.target_branch)
|
can_push_to_branch?(merge_request.target_branch)
|
||||||
|
|
||||||
if allowed
|
# Merge request can not be merged
|
||||||
if merge_request.unchecked?
|
# because user dont have permissions to push into target branch
|
||||||
merge_request.check_if_can_be_merged
|
unauthorized! unless allowed
|
||||||
|
|
||||||
|
not_allowed! unless merge_request.open?
|
||||||
|
|
||||||
|
merge_request.check_if_can_be_merged if merge_request.unchecked?
|
||||||
|
|
||||||
|
merge_params = {
|
||||||
|
commit_message: params[:merge_commit_message],
|
||||||
|
should_remove_source_branch: params[:should_remove_source_branch]
|
||||||
|
}
|
||||||
|
|
||||||
|
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
|
||||||
|
::MergeRequests::MergeService.new(merge_request.target_project, current_user, merge_params).
|
||||||
|
execute(merge_request, merge_params)
|
||||||
end
|
end
|
||||||
|
|
||||||
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
|
|
||||||
|
|
||||||
::MergeRequests::MergeService.new(merge_request.target_project, current_user).
|
|
||||||
execute(merge_request, commit_message)
|
|
||||||
|
|
||||||
present merge_request, with: Entities::MergeRequest
|
|
||||||
else
|
else
|
||||||
render_api_error!('Branch cannot be merged', 405)
|
render_api_error!('Branch cannot be merged', 405)
|
||||||
end
|
end
|
||||||
else
|
|
||||||
# Merge request can not be merged
|
present merge_request, with: Entities::MergeRequest
|
||||||
# because it is already closed/merged or marked as WIP
|
|
||||||
not_allowed!
|
|
||||||
end
|
|
||||||
else
|
|
||||||
# Merge request can not be merged
|
|
||||||
# because user dont have permissions to push into target branch
|
|
||||||
unauthorized!
|
|
||||||
end
|
|
||||||
end
|
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
|
# Get a merge request's comments
|
||||||
#
|
#
|
||||||
|
|
|
@ -30,7 +30,7 @@ describe MergeRequest do
|
||||||
describe 'associations' 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(: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(: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) }
|
it { is_expected.to have_one(:merge_request_diff).dependent(:destroy) }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -53,6 +53,8 @@ describe MergeRequest do
|
||||||
it { is_expected.to respond_to(:unchecked?) }
|
it { is_expected.to respond_to(:unchecked?) }
|
||||||
it { is_expected.to respond_to(:can_be_merged?) }
|
it { is_expected.to respond_to(:can_be_merged?) }
|
||||||
it { is_expected.to respond_to(:cannot_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
|
end
|
||||||
|
|
||||||
describe '#to_reference' do
|
describe '#to_reference' do
|
||||||
|
@ -171,6 +173,16 @@ describe MergeRequest do
|
||||||
end
|
end
|
||||||
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
|
describe "#hook_attrs" do
|
||||||
it "has all the required keys" do
|
it "has all the required keys" do
|
||||||
attrs = subject.hook_attrs
|
attrs = subject.hook_attrs
|
||||||
|
|
|
@ -207,6 +207,22 @@ describe SystemNoteService do
|
||||||
end
|
end
|
||||||
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
|
describe '.change_title' do
|
||||||
subject { described_class.change_title(noteable, project, author, 'Old title') }
|
subject { described_class.change_title(noteable, project, author, 'Old title') }
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue