Commit c45bb62c authored by Douwe Maan's avatar Douwe Maan

Merge branch 'osw-merge-to-ref-changes-for-ci-team' into 'master'

Make merge to refs/merge-requests/:iid/merge not raise when FF-only enabled

Closes #58393

See merge request gitlab-org/gitlab-ce!25653
parents e571cbaa 2cb45dd0
...@@ -20,7 +20,12 @@ module MergeRequests ...@@ -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) 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
...@@ -38,12 +43,8 @@ module MergeRequests ...@@ -38,12 +43,8 @@ module MergeRequests
error = error =
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?
"#{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)
elsif @merge_request.should_be_rebased?
'Fast-forward merge is not possible. Please update your source branch.'
elsif !@merge_request.mergeable_to_ref? elsif !@merge_request.mergeable_to_ref?
"Merge request is not mergeable to #{target_ref}" "Merge request is not mergeable to #{target_ref}"
elsif !source elsif !source
...@@ -68,9 +69,5 @@ module MergeRequests ...@@ -68,9 +69,5 @@ module MergeRequests
rescue Gitlab::Git::PreReceiveError => error rescue Gitlab::Git::PreReceiveError => error
raise MergeError, error.message raise MergeError, error.message
end end
def merge_method_supported?
[:merge, :rebase_merge].include?(project.merge_method)
end
end end
end end
---
title: Make merge to refs/merge-requests/:iid/merge not raise when FF-only enabled
merge_request: 25653
author:
type: fixed
...@@ -1132,18 +1132,6 @@ describe API::MergeRequests do ...@@ -1132,18 +1132,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 ...@@ -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
...@@ -52,9 +32,31 @@ describe MergeRequests::MergeToRefService do ...@@ -52,9 +32,31 @@ 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[: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 }
...@@ -73,25 +75,31 @@ describe MergeRequests::MergeToRefService do ...@@ -73,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, {})
...@@ -122,29 +130,15 @@ describe MergeRequests::MergeToRefService do ...@@ -122,29 +130,15 @@ describe MergeRequests::MergeToRefService do
context 'when semi-linear merge method' do context 'when semi-linear merge method' do
let(:merge_method) { :rebase_merge } let(:merge_method) { :rebase_merge }
it 'return error when MR should be able to fast-forward' do it_behaves_like 'successfully merges to ref with merge method'
allow(merge_request).to receive(:should_be_rebased?) { true } it_behaves_like 'successfully evaluates pre-condition checks'
error_message = 'Fast-forward merge is not possible. Please update your source branch.'
result = service.execute(merge_request)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq(error_message)
end
end end
context 'when fast-forward merge method' do context 'when fast-forward merge method' do
let(:merge_method) { :ff } let(:merge_method) { :ff }
it 'returns error' do it_behaves_like 'successfully merges to ref with merge method'
error_message = "Fast-forward to #{merge_request.merge_ref_path} is currently not supported." it_behaves_like 'successfully evaluates pre-condition checks'
result = service.execute(merge_request)
expect(result[:status]).to eq(:error)
expect(result[:message]).to eq(error_message)
end
end end
context 'when MR is not mergeable to ref' do context 'when MR is not mergeable to ref' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment