Commit 30f5b9a5 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'push-mr-url-guards' into 'master'

Don't show new MR URL after push when it doesn't make sense



See merge request !5786
parents 0b6caf45 8779da45
...@@ -30,10 +30,21 @@ module MergeRequests ...@@ -30,10 +30,21 @@ module MergeRequests
end end
def get_branches(changes) def get_branches(changes)
return [] if project.empty_repo?
return [] unless project.merge_requests_enabled
changes_list = Gitlab::ChangesList.new(changes) changes_list = Gitlab::ChangesList.new(changes)
changes_list.map do |change| changes_list.map do |change|
next unless Gitlab::Git.branch_ref?(change[:ref]) next unless Gitlab::Git.branch_ref?(change[:ref])
Gitlab::Git.branch_name(change[:ref])
# Deleted branch
next if Gitlab::Git.blank_ref?(change[:newrev])
# Default branch
branch_name = Gitlab::Git.branch_name(change[:ref])
next if branch_name == project.default_branch
branch_name
end.compact end.compact
end end
......
...@@ -7,7 +7,9 @@ describe MergeRequests::GetUrlsService do ...@@ -7,7 +7,9 @@ describe MergeRequests::GetUrlsService do
let(:new_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } let(:new_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" }
let(:show_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" } let(:show_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" }
let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" }
let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
let(:default_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master" }
describe "#execute" do describe "#execute" do
shared_examples 'new_merge_request_link' do shared_examples 'new_merge_request_link' do
...@@ -32,6 +34,28 @@ describe MergeRequests::GetUrlsService do ...@@ -32,6 +34,28 @@ describe MergeRequests::GetUrlsService do
end end
end end
shared_examples 'no_merge_request_url' do
it 'returns no URL' do
result = service.execute(changes)
expect(result).to be_empty
end
end
context 'pushing to default branch' do
let(:changes) { default_branch_changes }
it_behaves_like 'no_merge_request_url'
end
context 'pushing to project with MRs disabled' do
let(:changes) { new_branch_changes }
before do
project.merge_requests_enabled = false
end
it_behaves_like 'no_merge_request_url'
end
context 'pushing one completely new branch' do context 'pushing one completely new branch' do
let(:changes) { new_branch_changes } let(:changes) { new_branch_changes }
it_behaves_like 'new_merge_request_link' it_behaves_like 'new_merge_request_link'
...@@ -42,6 +66,11 @@ describe MergeRequests::GetUrlsService do ...@@ -42,6 +66,11 @@ describe MergeRequests::GetUrlsService do
it_behaves_like 'new_merge_request_link' it_behaves_like 'new_merge_request_link'
end end
context 'pushing to deleted branch' do
let(:changes) { deleted_branch_changes }
it_behaves_like 'no_merge_request_url'
end
context 'pushing to existing branch and merge request opened' do context 'pushing to existing branch and merge request opened' do
let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch) } let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch) }
let(:changes) { existing_branch_changes } let(:changes) { existing_branch_changes }
...@@ -61,6 +90,11 @@ describe MergeRequests::GetUrlsService do ...@@ -61,6 +90,11 @@ describe MergeRequests::GetUrlsService do
let(:changes) { existing_branch_changes } let(:changes) { existing_branch_changes }
# Source project is now the forked one # Source project is now the forked one
let(:service) { MergeRequests::GetUrlsService.new(forked_project) } let(:service) { MergeRequests::GetUrlsService.new(forked_project) }
before do
allow(forked_project).to receive(:empty_repo?).and_return(false)
end
it_behaves_like 'show_merge_request_url' it_behaves_like 'show_merge_request_url'
end end
......
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