diff options
author | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2016-03-10 18:55:18 -0300 |
---|---|---|
committer | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2016-03-10 19:29:54 -0300 |
commit | 37b00b16a568655343a96976bf2b5252e0894c9c (patch) | |
tree | f517e25d1f517a94f446bca4ab19e3bc7d691ec8 | |
parent | 8ecdc0a7de05d81247f2892c7d7e2850ea5f811d (diff) | |
download | gitlab-ce-37b00b16a568655343a96976bf2b5252e0894c9c.tar.gz |
Fix importing PR's from GitHub when the source repo was removed
-rw-r--r-- | lib/gitlab/github_import/importer.rb | 11 | ||||
-rw-r--r-- | lib/gitlab/github_import/pull_request_formatter.rb | 10 | ||||
-rw-r--r-- | spec/lib/gitlab/github_import/pull_request_formatter_spec.rb | 64 |
3 files changed, 40 insertions, 45 deletions
diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index e2a85f29825..172c5441e36 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -45,10 +45,13 @@ module Gitlab direction: :asc).each do |raw_data| pull_request = PullRequestFormatter.new(project, raw_data) - if !pull_request.cross_project? && pull_request.valid? - merge_request = MergeRequest.create!(pull_request.attributes) - import_comments(pull_request.number, merge_request) - import_comments_on_diff(pull_request.number, merge_request) + if pull_request.valid? + merge_request = MergeRequest.new(pull_request.attributes) + + if merge_request.save + import_comments(pull_request.number, merge_request) + import_comments_on_diff(pull_request.number, merge_request) + end end end diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb index f96fed0f5cf..4e507b090e8 100644 --- a/lib/gitlab/github_import/pull_request_formatter.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -17,16 +17,12 @@ module Gitlab } end - def cross_project? - source_repo.id != target_repo.id - end - def number raw_data.number end def valid? - source_branch.present? && target_branch.present? + !cross_project? && source_branch.present? && target_branch.present? end private @@ -53,6 +49,10 @@ module Gitlab raw_data.body || "" end + def cross_project? + source_repo.present? && target_repo.present? && source_repo.id != target_repo.id + end + def description formatter.author_line(author) + body end diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index 6cebcb5009a..e49dcb42342 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -127,34 +127,6 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end end - describe '#cross_project?' do - context 'when source, and target repositories are the same' do - let(:raw_data) { OpenStruct.new(base_data) } - - it 'returns false' do - expect(pull_request.cross_project?).to eq false - end - end - - context 'when source repo is a fork' do - let(:source_repo) { OpenStruct.new(id: 2, fork: true) } - let(:raw_data) { OpenStruct.new(base_data) } - - it 'returns true' do - expect(pull_request.cross_project?).to eq true - end - end - - context 'when target repo is a fork' do - let(:target_repo) { OpenStruct.new(id: 2, fork: true) } - let(:raw_data) { OpenStruct.new(base_data) } - - it 'returns true' do - expect(pull_request.cross_project?).to eq true - end - end - end - describe '#number' do let(:raw_data) { OpenStruct.new(base_data.merge(number: 1347)) } @@ -166,24 +138,44 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do describe '#valid?' do let(:invalid_branch) { OpenStruct.new(ref: 'invalid-branch') } - context 'when source and target branches exists' do - let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: target_branch)) } + context 'when source, and target repositories are the same' do + context 'and source and target branches exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: target_branch)) } - it 'returns true' do - expect(pull_request.valid?).to eq true + it 'returns true' do + expect(pull_request.valid?).to eq true + end + end + + context 'and source branch doesn not exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: invalid_branch, base: target_branch)) } + + it 'returns false' do + expect(pull_request.valid?).to eq false + end + end + + context 'and target branch doesn not exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: invalid_branch)) } + + it 'returns false' do + expect(pull_request.valid?).to eq false + end end end - context 'when source branch doesn not exists' do - let(:raw_data) { OpenStruct.new(base_data.merge(head: invalid_branch, base: target_branch)) } + context 'when source repo is a fork' do + let(:source_repo) { OpenStruct.new(id: 2, fork: true) } + let(:raw_data) { OpenStruct.new(base_data) } it 'returns false' do expect(pull_request.valid?).to eq false end end - context 'when target branch doesn not exists' do - let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: invalid_branch)) } + context 'when target repo is a fork' do + let(:target_repo) { OpenStruct.new(id: 2, fork: true) } + let(:raw_data) { OpenStruct.new(base_data) } it 'returns false' do expect(pull_request.valid?).to eq false |