diff options
author | Douwe Maan <douwe@gitlab.com> | 2019-03-07 14:27:52 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2019-03-07 14:27:52 +0300 |
commit | c45bb62c0ae36018891a343c7c820fc1a901e33e (patch) | |
tree | f5a9cb739ab4ed509d66947bbf9581906310e271 | |
parent | e571cbaa2f0b5096e514bc96fbc8f26909d8e52b (diff) | |
parent | 2cb45dd0d56156ace389239088822c428d87585c (diff) |
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
4 files changed, 56 insertions, 72 deletions
diff --git a/app/services/merge_requests/merge_to_ref_service.rb b/app/services/merge_requests/merge_to_ref_service.rb index 586652ae44e..69cc441f1bb 100644 --- a/app/services/merge_requests/merge_to_ref_service.rb +++ b/app/services/merge_requests/merge_to_ref_service.rb @@ -20,7 +20,12 @@ module MergeRequests 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 error(error.message) end @@ -38,12 +43,8 @@ module MergeRequests error = if Feature.disabled?(:merge_to_tmp_merge_ref_path, project) '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) 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? "Merge request is not mergeable to #{target_ref}" elsif !source @@ -68,9 +69,5 @@ module MergeRequests rescue Gitlab::Git::PreReceiveError => error raise MergeError, error.message end - - def merge_method_supported? - [:merge, :rebase_merge].include?(project.merge_method) - end end end diff --git a/changelogs/unreleased/osw-merge-to-ref-changes-for-ci-team.yml b/changelogs/unreleased/osw-merge-to-ref-changes-for-ci-team.yml new file mode 100644 index 00000000000..dfccd6194d4 --- /dev/null +++ b/changelogs/unreleased/osw-merge-to-ref-changes-for-ci-team.yml @@ -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 diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 6752a1c36bf..fee6312a9c7 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1132,18 +1132,6 @@ describe API::MergeRequests do 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 diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index 96f2fde7117..fabca8f6b4a 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -19,27 +19,7 @@ describe MergeRequests::MergeToRefService do end end - set(:user) { create(:user) } - 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 - + shared_examples_for 'successfully merges to ref with merge method' do it 'writes commit to merge ref' do repository = project.repository target_ref = merge_request.merge_ref_path @@ -52,9 +32,31 @@ describe MergeRequests::MergeToRefService do expect(result[:status]).to eq(:success) 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(ref_head.id).to eq(result[:commit_id]) 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 expect { process_merge_to_ref }.not_to change { ActionMailer::Base.deliveries.count } @@ -73,25 +75,31 @@ describe MergeRequests::MergeToRefService do process_merge_to_ref end + end - it 'returns error when feature is disabled' do - stub_feature_flags(merge_to_tmp_merge_ref_path: false) + set(:user) { create(:user) } + 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) - expect(result[:message]).to eq('Feature is not enabled') + describe '#execute' do + let(:service) do + described_class.new(project, user, commit_message: 'Awesome message', + should_remove_source_branch: true) 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') + def process_merge_to_ref + perform_enqueued_jobs do + service.execute(merge_request) + 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 let(:merge_ref_service) do described_class.new(project, user, {}) @@ -122,29 +130,15 @@ describe MergeRequests::MergeToRefService do context 'when semi-linear merge method' do let(:merge_method) { :rebase_merge } - it 'return error when MR should be able to fast-forward' do - allow(merge_request).to receive(:should_be_rebased?) { true } - - 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 + 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 'returns error' do - error_message = "Fast-forward to #{merge_request.merge_ref_path} is currently not supported." - - result = service.execute(merge_request) - - expect(result[:status]).to eq(:error) - expect(result[:message]).to eq(error_message) - end + 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 |