Commit ff62e206 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fix-gh-pr-import' into 'master'

Fix importing PR's from GitHub when the source repo was removed

Closes #13847 
Closes gitlab-com/support-forum#584

See merge request !3172
parents 8ee1c0ff 37b00b16
...@@ -45,12 +45,15 @@ module Gitlab ...@@ -45,12 +45,15 @@ module Gitlab
direction: :asc).each do |raw_data| direction: :asc).each do |raw_data|
pull_request = PullRequestFormatter.new(project, raw_data) pull_request = PullRequestFormatter.new(project, raw_data)
if !pull_request.cross_project? && pull_request.valid? if pull_request.valid?
merge_request = MergeRequest.create!(pull_request.attributes) merge_request = MergeRequest.new(pull_request.attributes)
if merge_request.save
import_comments(pull_request.number, merge_request) import_comments(pull_request.number, merge_request)
import_comments_on_diff(pull_request.number, merge_request) import_comments_on_diff(pull_request.number, merge_request)
end end
end end
end
true true
rescue ActiveRecord::RecordInvalid => e rescue ActiveRecord::RecordInvalid => e
......
...@@ -17,16 +17,12 @@ module Gitlab ...@@ -17,16 +17,12 @@ module Gitlab
} }
end end
def cross_project?
source_repo.id != target_repo.id
end
def number def number
raw_data.number raw_data.number
end end
def valid? def valid?
source_branch.present? && target_branch.present? !cross_project? && source_branch.present? && target_branch.present?
end end
private private
...@@ -53,6 +49,10 @@ module Gitlab ...@@ -53,6 +49,10 @@ module Gitlab
raw_data.body || "" raw_data.body || ""
end end
def cross_project?
source_repo.present? && target_repo.present? && source_repo.id != target_repo.id
end
def description def description
formatter.author_line(author) + body formatter.author_line(author) + body
end end
......
...@@ -127,34 +127,6 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -127,34 +127,6 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
end end
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 describe '#number' do
let(:raw_data) { OpenStruct.new(base_data.merge(number: 1347)) } let(:raw_data) { OpenStruct.new(base_data.merge(number: 1347)) }
...@@ -166,7 +138,8 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -166,7 +138,8 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
describe '#valid?' do describe '#valid?' do
let(:invalid_branch) { OpenStruct.new(ref: 'invalid-branch') } let(:invalid_branch) { OpenStruct.new(ref: 'invalid-branch') }
context 'when source and target branches exists' do 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)) } let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: target_branch)) }
it 'returns true' do it 'returns true' do
...@@ -174,7 +147,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -174,7 +147,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
end end
end end
context 'when source branch doesn not exists' do context 'and source branch doesn not exists' do
let(:raw_data) { OpenStruct.new(base_data.merge(head: invalid_branch, base: target_branch)) } let(:raw_data) { OpenStruct.new(base_data.merge(head: invalid_branch, base: target_branch)) }
it 'returns false' do it 'returns false' do
...@@ -182,7 +155,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -182,7 +155,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
end end
end end
context 'when target branch doesn not exists' do context 'and target branch doesn not exists' do
let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: invalid_branch)) } let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: invalid_branch)) }
it 'returns false' do it 'returns false' do
...@@ -190,4 +163,23 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do ...@@ -190,4 +163,23 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
end end
end end
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 false' do
expect(pull_request.valid?).to eq false
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 false' do
expect(pull_request.valid?).to eq false
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