Merge branch 'gh-pull-requests' into 'master'

Fix importer for GitHub Pull Requests when a branch was reused across Pull Requests

Closes #17766

## What does this MR do?

GitHub importer fails when a repository has two Pull Requests, with the same branch name and different SHA, one of which was closed without merging, and one which is open or was merged in. This happens because we only checks if the branch exists in the [Gitlab::GithubImport::BranchFormatter#exists?](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/github_import/branch_formatter.rb#L6-8), before creating the missing references on Github. With this MR we check if both branch, and SHA exists in the current project. If no, we create the missing reference on GitHub before import the PR.

## Are there points in the code the reviewer needs to double check?

Nope.

## Why was this MR needed?

Github importer will fail if branch was reused across Pull Requests.

## What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ce/issues/19439

/cc @royaldark @thomasjachmann

See merge request !5103

2 files changed, 23 insertions, 14 deletions

diff --git a/spec/lib/gitlab/github_import/branch_formatter_spec.rb b/spec/lib/gitlab/github_import/branch_formatter_spec.rb
index 3cb634ba010..fc9d5204148 100644
--- a/spec/lib/gitlab/github_import/branch_formatter_spec.rb
+++ b/spec/lib/gitlab/github_import/branch_formatter_spec.rb
@@ -2,17 +2,18 @@ require 'spec_helper'
 
 describe Gitlab::GithubImport::BranchFormatter, lib: true do
   let(:project) { create(:project) }
+  let(:commit) { create(:commit, project: project) }
   let(:repo) { double }
   let(:raw) do
     {
       ref: 'feature',
       repo: repo,
-      sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
+      sha: commit.id
     }
   end
 
   describe '#exists?' do
-    it 'returns true when branch exists' do
+    it 'returns true when both branch, and commit exists' do
       branch = described_class.new(project, double(raw))
 
       expect(branch.exists?).to eq true
@@ -23,6 +24,12 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do
 
       expect(branch.exists?).to eq false
     end
+
+    it 'returns false when commit does not exist' do
+      branch = described_class.new(project, double(raw.merge(sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b')))
+
+      expect(branch.exists?).to eq false
+    end
   end
 
   describe '#name' do
@@ -33,7 +40,7 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do
     end
 
     it 'returns formatted ref when branch does not exist' do
-      branch = described_class.new(project, double(raw.merge(ref: 'removed-branch')))
+      branch = described_class.new(project, double(raw.merge(ref: 'removed-branch', sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b')))
 
       expect(branch.name).to eq 'removed-branch-2e5d3239'
     end
@@ -51,18 +58,18 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do
     it 'returns raw sha' do
       branch = described_class.new(project, double(raw))
 
-      expect(branch.sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
+      expect(branch.sha).to eq commit.id
     end
   end
 
   describe '#valid?' do
-    it 'returns true when repository exists' do
+    it 'returns true when raw repo is present' do
       branch = described_class.new(project, double(raw))
 
       expect(branch.valid?).to eq true
     end
 
-    it 'returns false when repository does not exist' do
+    it 'returns false when raw repo is blank' do
       branch = described_class.new(project, double(raw.merge(repo: nil)))
 
       expect(branch.valid?).to eq false
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 120f59e6e71..9587252b990 100644
--- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb
+++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb
@@ -2,11 +2,13 @@ require 'spec_helper'
 
 describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
   let(:project) { create(:project) }
+  let(:source_sha) { create(:commit, project: project).id }
+  let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id }
   let(:repository) { double(id: 1, fork: false) }
   let(:source_repo) { repository }
-  let(:source_branch) { double(ref: 'feature', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') }
+  let(:source_branch) { double(ref: 'feature', repo: source_repo, sha: source_sha) }
   let(:target_repo) { repository }
-  let(:target_branch) { double(ref: 'master', repo: target_repo, sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7') }
+  let(:target_branch) { double(ref: 'master', repo: target_repo, sha: target_sha) }
   let(:octocat) { double(id: 123456, login: 'octocat') }
   let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') }
   let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') }
@@ -41,10 +43,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
           description: "*Created by: octocat*\n\nPlease pull these awesome changes",
           source_project: project,
           source_branch: 'feature',
-          head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b',
+          head_source_sha: source_sha,
           target_project: project,
           target_branch: 'master',
-          base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7',
+          base_target_sha: target_sha,
           state: 'opened',
           milestone: nil,
           author_id: project.creator_id,
@@ -68,10 +70,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
           description: "*Created by: octocat*\n\nPlease pull these awesome changes",
           source_project: project,
           source_branch: 'feature',
-          head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b',
+          head_source_sha: source_sha,
           target_project: project,
           target_branch: 'master',
-          base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7',
+          base_target_sha: target_sha,
           state: 'closed',
           milestone: nil,
           author_id: project.creator_id,
@@ -95,10 +97,10 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do
           description: "*Created by: octocat*\n\nPlease pull these awesome changes",
           source_project: project,
           source_branch: 'feature',
-          head_source_sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b',
+          head_source_sha: source_sha,
           target_project: project,
           target_branch: 'master',
-          base_target_sha: '8ffb3c15a5475e59ae909384297fede4badcb4c7',
+          base_target_sha: target_sha,
           state: 'merged',
           milestone: nil,
           author_id: project.creator_id,