Commit 59588eba authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre Committed by Gabriel Mazetto

Prefixes source branch name with short SHA to avoid collision

parent a3e65ef0
...@@ -15,6 +15,10 @@ module Gitlab ...@@ -15,6 +15,10 @@ module Gitlab
raw_data.user.login raw_data.user.login
end end
def short_sha(length = 7)
sha.to_s[0..length]
end
private private
def branch_exists? def branch_exists?
......
module Gitlab module Gitlab
module GithubImport module GithubImport
class PullRequestFormatter < IssuableFormatter class PullRequestFormatter < IssuableFormatter
delegate :user, :exists?, :project, :ref, :repo, :sha, to: :source_branch, prefix: true delegate :user, :project, :ref, :repo, :sha, to: :source_branch, prefix: true
delegate :user, :exists?, :project, :ref, :repo, :sha, to: :target_branch, prefix: true delegate :user, :exists?, :project, :ref, :repo, :sha, :short_sha, to: :target_branch, prefix: true
def attributes def attributes
{ {
...@@ -39,17 +39,17 @@ module Gitlab ...@@ -39,17 +39,17 @@ module Gitlab
def source_branch_name def source_branch_name
@source_branch_name ||= begin @source_branch_name ||= begin
if cross_project? if cross_project?
if source_branch_repo source_branch_name_prefixed
"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 : source_branch_name_prefixed
end end
end end
end end
def source_branch_name_prefixed
"gh-#{target_branch_short_sha}/#{number}"
end
def source_branch_exists? def source_branch_exists?
return false if cross_project? return false if cross_project?
...@@ -62,10 +62,14 @@ module Gitlab ...@@ -62,10 +62,14 @@ module Gitlab
def target_branch_name def target_branch_name
@target_branch_name ||= begin @target_branch_name ||= begin
target_branch_exists? ? target_branch_ref : "pull/#{number}/#{target_branch_ref}" target_branch_exists? ? target_branch_ref : target_branch_name_prefixed
end end
end end
def target_branch_name_prefixed
"gl-#{target_branch_short_sha}/#{number}"
end
def cross_project? def cross_project?
return true if source_branch_repo.nil? return true if source_branch_repo.nil?
......
...@@ -4,7 +4,9 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -4,7 +4,9 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
let(:client) { double } let(:client) { double }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:source_sha) { create(:commit, project: project).id } let(:source_sha) { create(:commit, project: project).id }
let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } let(:target_commit) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit) }
let(:target_sha) { target_commit.id }
let(:target_short_sha) { target_commit.id.to_s[0..7] }
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) }
...@@ -204,24 +206,24 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -204,24 +206,24 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
context 'when source branch does not exist' do context 'when source branch does not exist' do
let(:raw_data) { double(base_data.merge(head: removed_branch)) } let(:raw_data) { double(base_data.merge(head: removed_branch)) }
it 'prefixes branch name with pull request number' do it 'prefixes branch name with to avoid collision' do
expect(pull_request.source_branch_name).to eq 'pull/1347/removed-branch' expect(pull_request.source_branch_name).to eq "gh-#{target_short_sha}/1347"
end end
end end
context 'when source branch is from a fork' do context 'when source branch is from a fork' do
let(:raw_data) { double(base_data.merge(head: forked_branch)) } 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 it 'prefixes branch name with to avoid collision' do
expect(pull_request.source_branch_name).to eq 'pull/1347/company/otherproject/master' expect(pull_request.source_branch_name).to eq "gh-#{target_short_sha}/1347"
end end
end end
context 'when source branch is from a deleted fork' do context 'when source branch is from a deleted fork' do
let(:raw_data) { double(base_data.merge(head: branch_deleted_repo)) } 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 it 'prefixes branch name with to avoid collision' do
expect(pull_request.source_branch_name).to eq 'pull/1347/octocat/master' expect(pull_request.source_branch_name).to eq "gh-#{target_short_sha}/1347"
end end
end end
end end
...@@ -238,8 +240,8 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -238,8 +240,8 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
context 'when target branch does not exist' do context 'when target branch does not exist' do
let(:raw_data) { double(base_data.merge(base: removed_branch)) } let(:raw_data) { double(base_data.merge(base: removed_branch)) }
it 'prefixes branch name with pull request number' do it 'prefixes branch name with to avoid collision' do
expect(pull_request.target_branch_name).to eq 'pull/1347/removed-branch' expect(pull_request.target_branch_name).to eq 'gl-2e5d3239/1347'
end end
end end
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