Add API support for refreshing merge ref path

Add a merge_requests/:iid/merge_to_ref API which make
use of the groundwork to write merge results into
refs/merge-requests/:iid/merge.
This commit is contained in:
Oswaldo Ferreira 2019-02-08 19:25:26 -02:00
parent d133bd7fb4
commit e8fbc070e0
7 changed files with 125 additions and 52 deletions

View file

@ -15,10 +15,7 @@ module MergeRequests
# Overridden in EE. # Overridden in EE.
def hooks_validation_error(_merge_request) def hooks_validation_error(_merge_request)
<<<<<<< HEAD
# No-op # No-op
=======
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
end end
def source def source
@ -31,7 +28,6 @@ module MergeRequests
private private
<<<<<<< HEAD
# Overridden in EE. # Overridden in EE.
def error_check! def error_check!
# No-op # No-op
@ -41,8 +37,6 @@ module MergeRequests
raise MergeError, message raise MergeError, message
end end
=======
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
def handle_merge_error(*args) def handle_merge_error(*args)
# No-op # No-op
end end

View file

@ -14,19 +14,11 @@ module MergeRequests
def execute(merge_request) def execute(merge_request)
@merge_request = merge_request @merge_request = merge_request
<<<<<<< HEAD
validate! validate!
commit_id = commit commit_id = commit
raise_error('Conflicts detected during merge') unless commit_id raise_error('Conflicts detected during merge') unless commit_id
=======
error_check!
commit_id = commit
raise MergeError, 'Conflicts detected during merge' unless commit_id
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
success(commit_id: commit_id) success(commit_id: commit_id)
rescue MergeError => error rescue MergeError => error
@ -35,7 +27,6 @@ module MergeRequests
private private
<<<<<<< HEAD
def validate! def validate!
authorization_check! authorization_check!
error_check! error_check!
@ -48,11 +39,6 @@ module MergeRequests
if Feature.disabled?(:merge_to_tmp_merge_ref_path, project) if Feature.disabled?(:merge_to_tmp_merge_ref_path, project)
'Feature is not enabled' 'Feature is not enabled'
elsif !merge_method_supported? elsif !merge_method_supported?
=======
def error_check!
error =
if !merge_method_supported?
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
"#{project.human_merge_method} to #{target_ref} is currently not supported." "#{project.human_merge_method} to #{target_ref} is currently not supported."
elsif !hooks_validation_pass?(merge_request) elsif !hooks_validation_pass?(merge_request)
hooks_validation_error(merge_request) hooks_validation_error(merge_request)
@ -64,7 +50,6 @@ module MergeRequests
'No source for merge' 'No source for merge'
end end
<<<<<<< HEAD
raise_error(error) if error raise_error(error) if error
end end
@ -72,9 +57,6 @@ module MergeRequests
unless Ability.allowed?(current_user, :admin_merge_request, project) unless Ability.allowed?(current_user, :admin_merge_request, project)
raise_error("You are not allowed to merge to this ref") raise_error("You are not allowed to merge to this ref")
end end
=======
raise MergeError, error if error
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
end end
def target_ref def target_ref

View file

@ -0,0 +1,5 @@
---
title: API support for MR merge to temporary merge ref path
merge_request: 24918
author:
type: added

View file

@ -1101,6 +1101,40 @@ Parameters:
} }
``` ```
## Merge to default merge ref path
Merge the changes between the merge request source and target branches into `refs/merge-requests/:iid/merge`
ref, of the target project repository. This ref will have the state the target branch would have if
a regular merge action was taken.
This is not a regular merge action given it doesn't change the merge request state in any manner.
This ref (`refs/merge-requests/:iid/merge`) is **always** overwritten when submitting
requests to this API, so none of its state is kept or used in the process.
If the merge request has conflicts, is empty or already merged,
you'll get a `400` and a descriptive error message. If you don't have permissions to do so,
you'll get a `403`.
It returns the HEAD commit of `refs/merge-requests/:iid/merge` in the response body in
case of `200`.
```
PUT /projects/:id/merge_requests/:merge_request_iid/merge_to_ref
```
Parameters:
- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user
- `merge_request_iid` (required) - Internal ID of MR
- `merge_commit_message` (optional) - Custom merge commit message
```json
{
"commit_id": "854a3a7a17acbcc0bbbea170986df1eb60435f34"
}
```
## Cancel Merge When Pipeline Succeeds ## Cancel Merge When Pipeline Succeeds
If you don't have permissions to accept this merge request - you'll get a `401` If you don't have permissions to accept this merge request - you'll get a `401`

View file

@ -388,6 +388,31 @@ module API
present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project present merge_request, with: Entities::MergeRequest, current_user: current_user, project: user_project
end end
desc 'Merge a merge request to its default temporary merge ref path'
params do
optional :merge_commit_message, type: String, desc: 'Custom merge commit message'
end
put ':id/merge_requests/:merge_request_iid/merge_to_ref' do
merge_request = find_project_merge_request(params[:merge_request_iid])
authorize! :admin_merge_request, user_project
merge_params = {
commit_message: params[:merge_commit_message]
}
result = ::MergeRequests::MergeToRefService
.new(merge_request.target_project, current_user, merge_params)
.execute(merge_request)
if result[:status] == :success
present result.slice(:commit_id), 200
else
http_status = result[:http_status] || 400
render_api_error!(result[:message], http_status)
end
end
desc 'Cancel merge if "Merge When Pipeline Succeeds" is enabled' do desc 'Cancel merge if "Merge When Pipeline Succeeds" is enabled' do
success Entities::MergeRequest success Entities::MergeRequest
end end

View file

@ -1001,6 +1001,67 @@ describe API::MergeRequests do
end end
end end
describe "PUT /projects/:id/merge_requests/:merge_request_iid/merge_to_ref" do
let(:pipeline) { create(:ci_pipeline_without_jobs) }
let(:url) do
"/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge_to_ref"
end
it 'returns the generated ID from the merge service in case of success' do
put api(url, user), params: { merge_commit_message: 'Custom message' }
commit = project.commit(json_response['commit_id'])
expect(response).to have_gitlab_http_status(200)
expect(json_response['commit_id']).to be_present
expect(commit.message).to eq('Custom message')
end
it "returns 400 if branch can't be merged" do
merge_request.update!(state: 'merged')
put api(url, user)
expect(response).to have_gitlab_http_status(400)
expect(json_response['message'])
.to eq("Merge request is not mergeable to #{merge_request.merge_ref_path}")
end
it 'returns 403 if user has no permissions to merge to the ref' do
user2 = create(:user)
project.add_reporter(user2)
put api(url, user2)
expect(response).to have_gitlab_http_status(403)
expect(json_response['message']).to eq('403 Forbidden')
end
it 'returns 404 for an invalid merge request IID' do
put api("/projects/#{project.id}/merge_requests/12345/merge_to_ref", user)
expect(response).to have_gitlab_http_status(404)
end
it "returns 404 if the merge request id is used instead of iid" do
put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user)
expect(response).to have_gitlab_http_status(404)
end
it "returns 400 when merge method is not supported" do
merge_request.project.update!(merge_method: 'ff')
put api(url, user)
expected_error =
'Fast-forward to refs/merge-requests/1/merge is currently not supported.'
expect(response).to have_gitlab_http_status(400)
expect(json_response['message']).to eq(expected_error)
end
end
describe "PUT /projects/:id/merge_requests/:merge_request_iid" do describe "PUT /projects/:id/merge_requests/:merge_request_iid" do
context "to close a MR" do context "to close a MR" do
it "returns merge_request" do it "returns merge_request" do

View file

@ -3,7 +3,6 @@
require 'spec_helper' require 'spec_helper'
describe MergeRequests::MergeToRefService do describe MergeRequests::MergeToRefService do
<<<<<<< HEAD
shared_examples_for 'MergeService for target ref' do shared_examples_for 'MergeService for target ref' do
it 'target_ref has the same state of target branch' do it 'target_ref has the same state of target branch' do
repo = merge_request.target_project.repository repo = merge_request.target_project.repository
@ -20,8 +19,6 @@ describe MergeRequests::MergeToRefService do
end end
end end
=======
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
set(:user) { create(:user) } set(:user) { create(:user) }
let(:merge_request) { create(:merge_request, :simple) } let(:merge_request) { create(:merge_request, :simple) }
let(:project) { merge_request.project } let(:project) { merge_request.project }
@ -77,7 +74,6 @@ describe MergeRequests::MergeToRefService do
process_merge_to_ref process_merge_to_ref
end end
<<<<<<< HEAD
it 'returns error when feature is disabled' do it 'returns error when feature is disabled' do
stub_feature_flags(merge_to_tmp_merge_ref_path: false) stub_feature_flags(merge_to_tmp_merge_ref_path: false)
@ -87,8 +83,6 @@ describe MergeRequests::MergeToRefService do
expect(result[:message]).to eq('Feature is not enabled') expect(result[:message]).to eq('Feature is not enabled')
end end
=======
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
it 'returns an error when the failing to process the merge' do it 'returns an error when the failing to process the merge' do
allow(project.repository).to receive(:merge_to_ref).and_return(nil) allow(project.repository).to receive(:merge_to_ref).and_return(nil)
@ -107,25 +101,6 @@ describe MergeRequests::MergeToRefService do
MergeRequests::MergeService.new(project, user, {}) MergeRequests::MergeService.new(project, user, {})
end end
<<<<<<< HEAD
=======
shared_examples_for 'MergeService for target ref' do
it 'target_ref has the same state of target branch' do
repo = merge_request.target_project.repository
process_merge_to_ref
merge_service.execute(merge_request)
ref_commits = repo.commits(merge_request.merge_ref_path, limit: 3)
target_branch_commits = repo.commits(merge_request.target_branch, limit: 3)
ref_commits.zip(target_branch_commits).each do |ref_commit, target_branch_commit|
expect(ref_commit.parents).to eq(target_branch_commit.parents)
end
end
end
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
context 'when merge commit' do context 'when merge commit' do
it_behaves_like 'MergeService for target ref' it_behaves_like 'MergeService for target ref'
end end
@ -210,7 +185,6 @@ describe MergeRequests::MergeToRefService do
it { expect(todo).not_to be_done } it { expect(todo).not_to be_done }
end end
<<<<<<< HEAD
it 'returns error when user has no authorization to admin the merge request' do it 'returns error when user has no authorization to admin the merge request' do
unauthorized_user = create(:user) unauthorized_user = create(:user)
@ -223,7 +197,5 @@ describe MergeRequests::MergeToRefService do
expect(result[:status]).to eq(:error) expect(result[:status]).to eq(:error)
expect(result[:message]).to eq('You are not allowed to merge to this ref') expect(result[:message]).to eq('You are not allowed to merge to this ref')
end end
=======
>>>>>>> 89c57ca2673... Support merge to ref for merge-commit and squash
end end
end end