Commit a3e65ef0 authored by Gabriel Mazetto's avatar Gabriel Mazetto

Fix GitHub importer for PRs of deleted forked repositories

parent fee8dcde
...@@ -11,6 +11,10 @@ module Gitlab ...@@ -11,6 +11,10 @@ module Gitlab
sha.present? && ref.present? sha.present? && ref.present?
end end
def user
raw_data.user.login
end
private private
def branch_exists? def branch_exists?
......
module Gitlab module Gitlab
module GithubImport module GithubImport
class PullRequestFormatter < IssuableFormatter class PullRequestFormatter < IssuableFormatter
delegate :exists?, :project, :ref, :repo, :sha, to: :source_branch, prefix: true delegate :user, :exists?, :project, :ref, :repo, :sha, to: :source_branch, prefix: true
delegate :exists?, :project, :ref, :repo, :sha, to: :target_branch, prefix: true delegate :user, :exists?, :project, :ref, :repo, :sha, to: :target_branch, prefix: true
def attributes def attributes
{ {
...@@ -39,13 +39,23 @@ module Gitlab ...@@ -39,13 +39,23 @@ module Gitlab
def source_branch_name def source_branch_name
@source_branch_name ||= begin @source_branch_name ||= begin
if cross_project? if cross_project?
"pull/#{number}/#{source_branch_repo.full_name}/#{source_branch_ref}" if source_branch_repo
"pull/#{number}/#{source_branch_repo.full_name}/#{source_branch_ref}"
else
"pull/#{number}/#{source_branch_user}/#{source_branch_ref}"
end
else 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 end
def source_branch_exists?
return false if cross_project?
source_branch.exists?
end
def target_branch def target_branch
@target_branch ||= BranchFormatter.new(project, raw_data.base) @target_branch ||= BranchFormatter.new(project, raw_data.base)
end end
...@@ -57,7 +67,9 @@ module Gitlab ...@@ -57,7 +67,9 @@ module Gitlab
end end
def cross_project? def cross_project?
source_branch.repo.id != target_branch.repo.id return true if source_branch_repo.nil?
source_branch_repo.id != target_branch_repo.id
end end
def opened? def opened?
......
...@@ -13,6 +13,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -13,6 +13,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
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(:forked_branch) { double(ref: 'master', repo: forked_source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') }
let(:branch_deleted_repo) { double(ref: 'master', repo: nil, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b', user: octocat) }
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') }
...@@ -215,6 +216,14 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -215,6 +216,14 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
expect(pull_request.source_branch_name).to eq 'pull/1347/company/otherproject/master' expect(pull_request.source_branch_name).to eq 'pull/1347/company/otherproject/master'
end end
end end
context 'when source branch is from a deleted fork' do
let(:raw_data) { double(base_data.merge(head: branch_deleted_repo)) }
it 'prefixes branch name with pull request number and user login to avoid collision' do
expect(pull_request.source_branch_name).to eq 'pull/1347/octocat/master'
end
end
end end
shared_examples 'Gitlab::GithubImport::PullRequestFormatter#target_branch_name' do shared_examples 'Gitlab::GithubImport::PullRequestFormatter#target_branch_name' do
...@@ -290,6 +299,14 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -290,6 +299,14 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
end end
end end
context 'when source repository does not exist anymore' do
let(:raw_data) { double(base_data.merge(head: branch_deleted_repo)) }
it 'returns true' do
expect(pull_request.cross_project?).to eq true
end
end
context 'when source and target repositories are the same' do context 'when source and target repositories are the same' do
let(:raw_data) { double(base_data.merge(head: source_branch)) } let(:raw_data) { double(base_data.merge(head: source_branch)) }
...@@ -299,6 +316,14 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -299,6 +316,14 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
end end
end end
describe '#source_branch_exists?' do
let(:raw_data) { double(base_data.merge(head: forked_branch)) }
it 'returns false when is a cross_project' do
expect(pull_request.source_branch_exists?).to eq false
end
end
describe '#url' do describe '#url' do
let(:raw_data) { double(base_data) } let(:raw_data) { double(base_data) }
......
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