Merge branch 'sh-import-source-branch-github-forks' into 'master'

Create the source branch for a GitHub import

Closes #57370

See merge request gitlab-org/gitlab-ce!25064

(cherry picked from commit 6dfce6e786f4828f3d2b0ff63d2a2dea4c82471c)

e34a3213 Create the source branch for a GitHub import

5 files changed, 74 insertions, 5 deletions

diff --git a/changelogs/unreleased/sh-import-source-branch-github-forks.yml b/changelogs/unreleased/sh-import-source-branch-github-forks.yml
new file mode 100644
index 00000000000..b5ea60202c0
--- /dev/null
+++ b/changelogs/unreleased/sh-import-source-branch-github-forks.yml
@@ -0,0 +1,5 @@
+---
+title: Create the source branch for a GitHub import
+merge_request: 25064
+author:
+type: fixed
diff --git a/lib/gitlab/github_import/importer/pull_request_importer.rb b/lib/gitlab/github_import/importer/pull_request_importer.rb
index ae7c4cf1b38..e294173f992 100644
--- a/lib/gitlab/github_import/importer/pull_request_importer.rb
+++ b/lib/gitlab/github_import/importer/pull_request_importer.rb
@@ -67,6 +67,36 @@ module Gitlab
 
         def insert_git_data(merge_request, already_exists)
           insert_or_replace_git_data(merge_request, pull_request.source_branch_sha, pull_request.target_branch_sha, already_exists)
+          # We need to create the branch after the merge request is
+          # populated to ensure the merge request is in the right state
+          # when the branch is created.
+          create_source_branch_if_not_exists(merge_request)
+        end
+
+        # An imported merge request will not be mergeable unless the
+        # source branch exists. For pull requests from forks, the source
+        # branch will be in the form of
+        # "github/fork/{project-name}/{source_branch}". This branch will never
+        # exist, so we create it here.
+        #
+        # Note that we only create the branch if the merge request is still open.
+        # For projects that have many pull requests, we assume that if it's closed
+        # the branch has already been deleted.
+        def create_source_branch_if_not_exists(merge_request)
+          return unless merge_request.open?
+
+          source_branch = pull_request.formatted_source_branch
+
+          return if project.repository.branch_exists?(source_branch)
+
+          project.repository.add_branch(merge_request.author, source_branch, pull_request.source_branch_sha)
+        rescue Gitlab::Git::CommandError => e
+          Gitlab::Sentry.track_acceptable_exception(e,
+                                                    extra: {
+                                                      source_branch: source_branch,
+                                                      project_id: merge_request.project.id,
+                                                      merge_request_id: merge_request.id
+                                                    })
         end
       end
     end
diff --git a/lib/gitlab/github_import/representation/pull_request.rb b/lib/gitlab/github_import/representation/pull_request.rb
index 593b491a837..0ccc4bfaed3 100644
--- a/lib/gitlab/github_import/representation/pull_request.rb
+++ b/lib/gitlab/github_import/representation/pull_request.rb
@@ -76,10 +76,10 @@ module Gitlab
         # Returns a formatted source branch.
         #
         # For cross-project pull requests the branch name will be in the format
-        # `owner-name:branch-name`.
+        # `github/fork/owner-name/branch-name`.
         def formatted_source_branch
           if cross_project? && source_repository_owner
-            "#{source_repository_owner}:#{source_branch}"
+            "github/fork/#{source_repository_owner}/#{source_branch}"
           elsif source_branch == target_branch
             # Sometimes the source and target branch are the same, but GitLab
             # doesn't support this. This can happen when both the user and
diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
index 0f21b8843b6..15e59718dce 100644
--- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
@@ -89,7 +89,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
               description: 'This is my pull request',
               source_project_id: project.id,
               target_project_id: project.id,
-              source_branch: 'alice:feature',
+              source_branch: 'github/fork/alice/feature',
               target_branch: 'master',
               state: :merged,
               milestone_id: milestone.id,
@@ -134,7 +134,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
               description: "*Created by: alice*\n\nThis is my pull request",
               source_project_id: project.id,
               target_project_id: project.id,
-              source_branch: 'alice:feature',
+              source_branch: 'github/fork/alice/feature',
               target_branch: 'master',
               state: :merged,
               milestone_id: milestone.id,
@@ -259,6 +259,40 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
         .and_return(user.id)
     end
 
+    it 'does not create the source branch if merge request is merged' do
+      mr, exists = importer.create_merge_request
+
+      importer.insert_git_data(mr, exists)
+
+      expect(project.repository.branch_exists?(mr.source_branch)).to be_falsey
+      expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy
+    end
+
+    it 'creates the source branch if merge request is open' do
+      mr, exists = importer.create_merge_request
+      mr.state = 'opened'
+      mr.save
+
+      importer.insert_git_data(mr, exists)
+
+      expect(project.repository.branch_exists?(mr.source_branch)).to be_truthy
+      expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy
+    end
+
+    it 'ignores Git errors when creating a branch' do
+      mr, exists = importer.create_merge_request
+      mr.state = 'opened'
+      mr.save
+
+      expect(project.repository).to receive(:add_branch).and_raise(Gitlab::Git::CommandError)
+      expect(Gitlab::Sentry).to receive(:track_acceptable_exception).and_call_original
+
+      importer.insert_git_data(mr, exists)
+
+      expect(project.repository.branch_exists?(mr.source_branch)).to be_falsey
+      expect(project.repository.branch_exists?(mr.target_branch)).to be_truthy
+    end
+
     it 'creates the merge request diffs' do
       mr, exists = importer.create_merge_request
 
diff --git a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb
index 33f6ff0ae6a..d478e5ae899 100644
--- a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb
+++ b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb
@@ -238,7 +238,7 @@ describe Gitlab::GithubImport::Representation::PullRequest do
           target_repository_id: 2
         )
 
-        expect(pr.formatted_source_branch).to eq('foo:branch')
+        expect(pr.formatted_source_branch).to eq('github/fork/foo/branch')
       end
     end