Provide reliable source and target IDs
Returns the source and target IDs used to create the merge commit on Gitaly.
This commit is contained in:
parent
959ad992b6
commit
2cb45dd0d5
4 changed files with 71 additions and 46 deletions
|
@ -20,7 +20,12 @@ module MergeRequests
|
||||||
|
|
||||||
raise_error('Conflicts detected during merge') unless commit_id
|
raise_error('Conflicts detected during merge') unless commit_id
|
||||||
|
|
||||||
success(commit_id: commit_id, source_id: source)
|
commit = project.commit(commit_id)
|
||||||
|
target_id, source_id = commit.parent_ids
|
||||||
|
|
||||||
|
success(commit_id: commit.id,
|
||||||
|
target_id: target_id,
|
||||||
|
source_id: source_id)
|
||||||
rescue MergeError => error
|
rescue MergeError => error
|
||||||
error(error.message)
|
error(error.message)
|
||||||
end
|
end
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Make merge to refs/merge-requests/:iid/merge not raise when FF-only enabled
|
||||||
|
merge_request: 25653
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -1060,18 +1060,6 @@ describe API::MergeRequests do
|
||||||
|
|
||||||
expect(response).to have_gitlab_http_status(404)
|
expect(response).to have_gitlab_http_status(404)
|
||||||
end
|
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
|
end
|
||||||
|
|
||||||
describe "PUT /projects/:id/merge_requests/:merge_request_iid" do
|
describe "PUT /projects/:id/merge_requests/:merge_request_iid" do
|
||||||
|
|
|
@ -19,27 +19,7 @@ describe MergeRequests::MergeToRefService do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
set(:user) { create(:user) }
|
shared_examples_for 'successfully merges to ref with merge method' do
|
||||||
let(:merge_request) { create(:merge_request, :simple) }
|
|
||||||
let(:project) { merge_request.project }
|
|
||||||
|
|
||||||
before do
|
|
||||||
project.add_maintainer(user)
|
|
||||||
end
|
|
||||||
|
|
||||||
describe '#execute' do
|
|
||||||
let(:service) do
|
|
||||||
described_class.new(project, user,
|
|
||||||
commit_message: 'Awesome message',
|
|
||||||
'should_remove_source_branch' => true)
|
|
||||||
end
|
|
||||||
|
|
||||||
def process_merge_to_ref
|
|
||||||
perform_enqueued_jobs do
|
|
||||||
service.execute(merge_request)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'writes commit to merge ref' do
|
it 'writes commit to merge ref' do
|
||||||
repository = project.repository
|
repository = project.repository
|
||||||
target_ref = merge_request.merge_ref_path
|
target_ref = merge_request.merge_ref_path
|
||||||
|
@ -53,9 +33,30 @@ describe MergeRequests::MergeToRefService do
|
||||||
expect(result[:status]).to eq(:success)
|
expect(result[:status]).to eq(:success)
|
||||||
expect(result[:commit_id]).to be_present
|
expect(result[:commit_id]).to be_present
|
||||||
expect(result[:source_id]).to eq(merge_request.source_branch_sha)
|
expect(result[:source_id]).to eq(merge_request.source_branch_sha)
|
||||||
|
expect(result[:target_id]).to eq(merge_request.target_branch_sha)
|
||||||
expect(repository.ref_exists?(target_ref)).to be(true)
|
expect(repository.ref_exists?(target_ref)).to be(true)
|
||||||
expect(ref_head.id).to eq(result[:commit_id])
|
expect(ref_head.id).to eq(result[:commit_id])
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
shared_examples_for 'successfully evaluates pre-condition checks' do
|
||||||
|
it 'returns error when feature is disabled' do
|
||||||
|
stub_feature_flags(merge_to_tmp_merge_ref_path: false)
|
||||||
|
|
||||||
|
result = service.execute(merge_request)
|
||||||
|
|
||||||
|
expect(result[:status]).to eq(:error)
|
||||||
|
expect(result[:message]).to eq('Feature is not enabled')
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns an error when the failing to process the merge' do
|
||||||
|
allow(project.repository).to receive(:merge_to_ref).and_return(nil)
|
||||||
|
|
||||||
|
result = service.execute(merge_request)
|
||||||
|
|
||||||
|
expect(result[:status]).to eq(:error)
|
||||||
|
expect(result[:message]).to eq('Conflicts detected during merge')
|
||||||
|
end
|
||||||
|
|
||||||
it 'does not send any mail' do
|
it 'does not send any mail' do
|
||||||
expect { process_merge_to_ref }.not_to change { ActionMailer::Base.deliveries.count }
|
expect { process_merge_to_ref }.not_to change { ActionMailer::Base.deliveries.count }
|
||||||
|
@ -74,25 +75,31 @@ describe MergeRequests::MergeToRefService do
|
||||||
|
|
||||||
process_merge_to_ref
|
process_merge_to_ref
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
it 'returns error when feature is disabled' do
|
set(:user) { create(:user) }
|
||||||
stub_feature_flags(merge_to_tmp_merge_ref_path: false)
|
let(:merge_request) { create(:merge_request, :simple) }
|
||||||
|
let(:project) { merge_request.project }
|
||||||
|
|
||||||
result = service.execute(merge_request)
|
before do
|
||||||
|
project.add_maintainer(user)
|
||||||
|
end
|
||||||
|
|
||||||
expect(result[:status]).to eq(:error)
|
describe '#execute' do
|
||||||
expect(result[:message]).to eq('Feature is not enabled')
|
let(:service) do
|
||||||
|
described_class.new(project, user, commit_message: 'Awesome message',
|
||||||
|
should_remove_source_branch: true)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'returns an error when the failing to process the merge' do
|
def process_merge_to_ref
|
||||||
allow(project.repository).to receive(:merge_to_ref).and_return(nil)
|
perform_enqueued_jobs do
|
||||||
|
service.execute(merge_request)
|
||||||
result = service.execute(merge_request)
|
end
|
||||||
|
|
||||||
expect(result[:status]).to eq(:error)
|
|
||||||
expect(result[:message]).to eq('Conflicts detected during merge')
|
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it_behaves_like 'successfully merges to ref with merge method'
|
||||||
|
it_behaves_like 'successfully evaluates pre-condition checks'
|
||||||
|
|
||||||
context 'commit history comparison with regular MergeService' do
|
context 'commit history comparison with regular MergeService' do
|
||||||
let(:merge_ref_service) do
|
let(:merge_ref_service) do
|
||||||
described_class.new(project, user, {})
|
described_class.new(project, user, {})
|
||||||
|
@ -116,7 +123,27 @@ describe MergeRequests::MergeToRefService do
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'merge pre-condition checks' do
|
context 'merge pre-condition checks' do
|
||||||
|
before do
|
||||||
|
merge_request.project.update!(merge_method: merge_method)
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when semi-linear merge method' do
|
||||||
|
let(:merge_method) { :rebase_merge }
|
||||||
|
|
||||||
|
it_behaves_like 'successfully merges to ref with merge method'
|
||||||
|
it_behaves_like 'successfully evaluates pre-condition checks'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when fast-forward merge method' do
|
||||||
|
let(:merge_method) { :ff }
|
||||||
|
|
||||||
|
it_behaves_like 'successfully merges to ref with merge method'
|
||||||
|
it_behaves_like 'successfully evaluates pre-condition checks'
|
||||||
|
end
|
||||||
|
|
||||||
context 'when MR is not mergeable to ref' do
|
context 'when MR is not mergeable to ref' do
|
||||||
|
let(:merge_method) { :merge }
|
||||||
|
|
||||||
it 'returns error' do
|
it 'returns error' do
|
||||||
allow(merge_request).to receive(:mergeable_to_ref?) { false }
|
allow(merge_request).to receive(:mergeable_to_ref?) { false }
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue