Commit 26fa716d authored by Gabriel Mazetto's avatar Gabriel Mazetto

Fix name colision when importing GitHub pull requests from forked repositories

parent 8707db80
...@@ -38,9 +38,13 @@ module Gitlab ...@@ -38,9 +38,13 @@ module Gitlab
def source_branch_name def source_branch_name
@source_branch_name ||= begin @source_branch_name ||= begin
if source_branch.repo.id != target_branch.repo.id
"pull/#{number}/#{source_branch.repo.full_name}/#{source_branch_ref}"
else
source_branch_exists? ? source_branch_ref : "pull/#{number}/#{source_branch_ref}" source_branch_exists? ? source_branch_ref : "pull/#{number}/#{source_branch_ref}"
end end
end end
end
def target_branch def target_branch
@target_branch ||= BranchFormatter.new(project, raw_data.base) @target_branch ||= BranchFormatter.new(project, raw_data.base)
......
...@@ -8,9 +8,11 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -8,9 +8,11 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
let(:repository) { double(id: 1, fork: false) } let(:repository) { double(id: 1, fork: false) }
let(:source_repo) { repository } let(:source_repo) { repository }
let(:source_branch) { double(ref: 'branch-merged', repo: source_repo, sha: source_sha) } let(:source_branch) { double(ref: 'branch-merged', repo: source_repo, sha: source_sha) }
let(:forked_source_repo) { double(id: 2, fork: true, name: 'otherproject', full_name: 'company/otherproject') }
let(:target_repo) { repository } let(:target_repo) { repository }
let(:target_branch) { double(ref: 'master', repo: target_repo, sha: target_sha) } let(:target_branch) { double(ref: 'master', repo: target_repo, sha: target_sha) }
let(:removed_branch) { double(ref: 'removed-branch', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') } let(:removed_branch) { double(ref: 'removed-branch', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') }
let(:forked_branch) { double(ref: 'master', repo: forked_source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') }
let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') }
let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') }
let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') }
...@@ -205,6 +207,14 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -205,6 +207,14 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
expect(pull_request.source_branch_name).to eq 'pull/1347/removed-branch' expect(pull_request.source_branch_name).to eq 'pull/1347/removed-branch'
end end
end end
context 'when source branch is from a fork' do
let(:raw_data) { double(base_data.merge(head: forked_branch)) }
it 'prefixes branch name with pull request number and project with namespace to avoid collision' do
expect(pull_request.source_branch_name).to eq 'pull/1347/company/otherproject/master'
end
end
end end
shared_examples 'Gitlab::GithubImport::PullRequestFormatter#target_branch_name' do shared_examples 'Gitlab::GithubImport::PullRequestFormatter#target_branch_name' 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