Commit 66870960 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'fix/gh-import-bugs' into 'master'

Fix couple of GitHub importing bugs

Fix a bug in GH comment importing and label applying for imported MRs.

See merge request !7139
parents 20a7db44 ce38ae8c
...@@ -27,6 +27,8 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -27,6 +27,8 @@ 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 - Fix Sign in page 'Forgot your password?' link overlaps on medium-large screens
- Show full status link on MR & commit pipelines - Show full status link on MR & commit pipelines
- Fix documents and comments on Build API `scope` - Fix documents and comments on Build API `scope`
- Fix applying labels for GitHub-imported MRs
- Fix importing MR comments from GitHub
- Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov) - Refactor email, use setter method instead AR callbacks for email attribute (Semyon Pupkov)
- Shortened merge request modal to let clipboard button not overlap - Shortened merge request modal to let clipboard button not overlap
......
...@@ -105,18 +105,20 @@ module Gitlab ...@@ -105,18 +105,20 @@ module Gitlab
data = api.send(method, *args) data = api.send(method, *args)
return data unless data.is_a?(Array) return data unless data.is_a?(Array)
last_response = api.last_response
if block_given? if block_given?
yield data 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 else
each_response_page { |page| data.concat(page) } each_response_page(last_response) { |page| data.concat(page) }
data data
end end
end end
def each_response_page def each_response_page(last_response)
last_response = api.last_response
while last_response.rels[:next] while last_response.rels[:next]
sleep rate_limit_sleep_time if rate_limit_exceed? sleep rate_limit_sleep_time if rate_limit_exceed?
last_response = last_response.rels[:next].get last_response = last_response.rels[:next].get
......
...@@ -132,8 +132,15 @@ module Gitlab ...@@ -132,8 +132,15 @@ module Gitlab
end end
def apply_labels(issuable, raw_issuable) def apply_labels(issuable, raw_issuable)
if raw_issuable.labels.count > 0 # GH returns labels for issues but not for pull requests!
label_ids = raw_issuable.labels 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] } .map { |attrs| @labels[attrs.name] }
.compact .compact
...@@ -143,21 +150,22 @@ module Gitlab ...@@ -143,21 +150,22 @@ module Gitlab
def import_comments def import_comments
client.issues_comments(repo, per_page: 100) do |comments| client.issues_comments(repo, per_page: 100) do |comments|
create_comments(comments, :issue) create_comments(comments)
end end
client.pull_requests_comments(repo, per_page: 100) do |comments| client.pull_requests_comments(repo, per_page: 100) do |comments|
create_comments(comments, :pull_request) create_comments(comments)
end end
end end
def create_comments(comments, issuable_type) def create_comments(comments)
ActiveRecord::Base.no_touching do ActiveRecord::Base.no_touching do
comments.each do |raw| comments.each do |raw|
begin begin
comment = CommentFormatter.new(project, raw) comment = CommentFormatter.new(project, raw)
issuable_class = issuable_type == :issue ? Issue : MergeRequest # GH does not return info about comment's parent, so we guess it by checking its URL!
iid = raw.send("#{issuable_type}_url").split('/').last # GH doesn't return parent ID directly *_, parent, iid = URI(raw.html_url).path.split('/')
issuable_class = parent == 'issues' ? Issue : MergeRequest
issuable = issuable_class.find_by_iid(iid) issuable = issuable_class.find_by_iid(iid)
next unless issuable next unless issuable
......
...@@ -154,6 +154,7 @@ describe Gitlab::GithubImport::Importer, lib: true do ...@@ -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: :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: :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: :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: :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: :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" } { type: :release, url: 'https://api.github.com/repos/octocat/Hello-World/releases/2', errors: "Validation failed: Description can't be blank" }
......
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