From 4259334fb64ef49580e58d434bc63d3c4738a77c Mon Sep 17 00:00:00 2001
From: Ahmad Sherif <me@ahmadsherif.com>
Date: Thu, 27 Oct 2016 15:07:18 +0200
Subject: [PATCH] Fix applying labels for GitHub-imported MRs
---
CHANGELOG.md | 1 +
lib/gitlab/github_import/client.rb | 12 +++++++-----
lib/gitlab/github_import/importer.rb | 11 +++++++++--
spec/lib/gitlab/github_import/importer_spec.rb | 1 +
4 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ff8e03e7d58..6bf63582cbd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens
- Show full status link on MR & commit pipelines
- Fix documents and comments on Build API `scope`
+ - Fix applying labels for GitHub-imported MRs
- Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov)
## 8.13.1 (2016-10-25)
diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb
index 7f424b74efb..348005d5659 100644
--- a/lib/gitlab/github_import/client.rb
+++ b/lib/gitlab/github_import/client.rb
@@ -105,18 +105,20 @@ module Gitlab
data = api.send(method, *args)
return data unless data.is_a?(Array)
+ last_response = api.last_response
+
if block_given?
yield data
- each_response_page(&block)
+ # api.last_response could change while we're yielding (e.g. fetching labels for each PR)
+ # so we cache our own last request
+ each_response_page(last_response, &block)
else
- each_response_page { |page| data.concat(page) }
+ each_response_page(last_response) { |page| data.concat(page) }
data
end
end
- def each_response_page
- last_response = api.last_response
-
+ def each_response_page(last_response)
while last_response.rels[:next]
sleep rate_limit_sleep_time if rate_limit_exceed?
last_response = last_response.rels[:next].get
diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb
index 4b70f33a851..4ac932dc213 100644
--- a/lib/gitlab/github_import/importer.rb
+++ b/lib/gitlab/github_import/importer.rb
@@ -132,8 +132,15 @@ module Gitlab
end
def apply_labels(issuable, raw_issuable)
- if raw_issuable.labels.count > 0
- label_ids = raw_issuable.labels
+ # GH returns labels for issues but not for pull requests!
+ labels = if issuable.is_a?(MergeRequest)
+ client.labels_for_issue(repo, raw_issuable.number)
+ else
+ raw_issuable.labels
+ end
+
+ if labels.count > 0
+ label_ids = labels
.map { |attrs| @labels[attrs.name] }
.compact
diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb
index 1af553f8f03..84f280ceaae 100644
--- a/spec/lib/gitlab/github_import/importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer_spec.rb
@@ -154,6 +154,7 @@ describe Gitlab::GithubImport::Importer, lib: true do
{ type: :label, url: "https://api.github.com/repos/octocat/Hello-World/labels/bug", errors: "Validation failed: Title can't be blank, Title is invalid" },
{ type: :milestone, url: "https://api.github.com/repos/octocat/Hello-World/milestones/1", errors: "Validation failed: Title has already been taken" },
{ type: :issue, url: "https://api.github.com/repos/octocat/Hello-World/issues/1348", errors: "Validation failed: Title can't be blank, Title is too short (minimum is 0 characters)" },
+ { type: :pull_request, url: "https://api.github.com/repos/octocat/Hello-World/pulls/1347", errors: "Invalid Repository. Use user/repo format." },
{ type: :pull_request, url: "https://api.github.com/repos/octocat/Hello-World/pulls/1347", errors: "Validation failed: Validate branches Cannot Create: This merge request already exists: [\"New feature\"]" },
{ type: :wiki, errors: "Gitlab::Shell::Error" },
{ type: :release, url: 'https://api.github.com/repos/octocat/Hello-World/releases/2', errors: "Validation failed: Description can't be blank" }
--
GitLab