From 67b7637e5d7d3cf3e3f5cde6e7f984ece368c48c Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 7 Dec 2016 11:33:32 +0200 Subject: Apply review comments. Iteration 1 --- lib/bitbucket/client.rb | 29 +++++++++++----------- lib/bitbucket/connection.rb | 8 +++--- lib/bitbucket/paginator.rb | 2 +- lib/bitbucket/representation/issue.rb | 6 +---- .../representation/pull_request_comment.rb | 2 +- lib/bitbucket/representation/repo.rb | 6 +++-- lib/gitlab/bitbucket_import/importer.rb | 2 +- 7 files changed, 26 insertions(+), 29 deletions(-) diff --git a/lib/bitbucket/client.rb b/lib/bitbucket/client.rb index 33e977d655d..9fa44506374 100644 --- a/lib/bitbucket/client.rb +++ b/lib/bitbucket/client.rb @@ -5,40 +5,39 @@ module Bitbucket end def issues(repo) - relative_path = "/repositories/#{repo}/issues" - paginator = Paginator.new(connection, relative_path, :issue) + path = "/repositories/#{repo}/issues" + paginator = Paginator.new(connection, path, :issue) Collection.new(paginator) end - def issue_comments(repo, number) - relative_path = "/repositories/#{repo}/issues/#{number}/comments" - paginator = Paginator.new(connection, relative_path, :url) + def issue_comments(repo, issue_id) + path = "/repositories/#{repo}/issues/#{issue_id}/comments" + paginator = Paginator.new(connection, path, :url) Collection.new(paginator).map do |comment_url| - parsed_response = connection.get(comment_url.to_s) - Representation::Comment.new(parsed_response) + Representation::Comment.new(connection.get(comment_url.to_s)) end end def pull_requests(repo) - relative_path = "/repositories/#{repo}/pullrequests?state=ALL" - paginator = Paginator.new(connection, relative_path, :pull_request) + path = "/repositories/#{repo}/pullrequests?state=ALL" + paginator = Paginator.new(connection, path, :pull_request) Collection.new(paginator) end def pull_request_comments(repo, pull_request) - relative_path = "/repositories/#{repo}/pullrequests/#{pull_request}/comments" - paginator = Paginator.new(connection, relative_path, :pull_request_comment) + path = "/repositories/#{repo}/pullrequests/#{pull_request}/comments" + paginator = Paginator.new(connection, path, :pull_request_comment) Collection.new(paginator) end def pull_request_diff(repo, pull_request) - relative_path = "/repositories/#{repo}/pullrequests/#{pull_request}/diff" + path = "/repositories/#{repo}/pullrequests/#{pull_request}/diff" - connection.get(relative_path) + connection.get(path) end def repo(name) @@ -47,8 +46,8 @@ module Bitbucket end def repos - relative_path = "/repositories/#{user.username}" - paginator = Paginator.new(connection, relative_path, :repo) + path = "/repositories/#{user.username}" + paginator = Paginator.new(connection, path, :repo) Collection.new(paginator) end diff --git a/lib/bitbucket/connection.rb b/lib/bitbucket/connection.rb index e28285f119c..692a596c057 100644 --- a/lib/bitbucket/connection.rb +++ b/lib/bitbucket/connection.rb @@ -5,8 +5,8 @@ module Bitbucket DEFAULT_QUERY = {} def initialize(options = {}) - @api_version = options.fetch(:api_version, DEFAULT_API_VERSION) - @base_uri = options.fetch(:base_uri, DEFAULT_BASE_URI) + @api_version = options.fetch(:api_version, DEFAULT_API_VERSION) + @base_uri = options.fetch(:base_uri, DEFAULT_BASE_URI) @default_query = options.fetch(:query, DEFAULT_QUERY) @token = options[:token] @@ -23,7 +23,7 @@ module Bitbucket @connection ||= OAuth2::AccessToken.new(client, @token, refresh_token: @refresh_token, expires_at: @expires_at, expires_in: @expires_in) end - def query(params = {}) + def set_default_query_parameters(params = {}) @default_query.merge!(params) end @@ -63,7 +63,7 @@ module Bitbucket end def provider - Gitlab.config.omniauth.providers.find { |provider| provider.name == 'bitbucket' } + Gitlab::OAuth::Provider.config_for('bitbucket') end def options diff --git a/lib/bitbucket/paginator.rb b/lib/bitbucket/paginator.rb index a1672d9eaa1..d0e23007ff8 100644 --- a/lib/bitbucket/paginator.rb +++ b/lib/bitbucket/paginator.rb @@ -8,7 +8,7 @@ module Bitbucket @url = url @page = nil - connection.query(pagelen: PAGE_LENGTH, sort: :created_on) + connection.set_default_query_parameters(pagelen: PAGE_LENGTH, sort: :created_on) end def next diff --git a/lib/bitbucket/representation/issue.rb b/lib/bitbucket/representation/issue.rb index 48647ad51f6..dc034c19750 100644 --- a/lib/bitbucket/representation/issue.rb +++ b/lib/bitbucket/representation/issue.rb @@ -8,7 +8,7 @@ module Bitbucket end def author - reporter.fetch('username', 'Anonymous') + raw.dig('reporter', 'username') || 'Anonymous' end def description @@ -40,10 +40,6 @@ module Bitbucket def closed? CLOSED_STATUS.include?(raw['state']) end - - def reporter - raw.fetch('reporter', {}) - end end end end diff --git a/lib/bitbucket/representation/pull_request_comment.rb b/lib/bitbucket/representation/pull_request_comment.rb index c63d749cba7..ae2b069d6a2 100644 --- a/lib/bitbucket/representation/pull_request_comment.rb +++ b/lib/bitbucket/representation/pull_request_comment.rb @@ -18,7 +18,7 @@ module Bitbucket end def parent_id - raw.fetch('parent', {}).fetch('id', nil) + raw.dig('parent', 'id') end def inline? diff --git a/lib/bitbucket/representation/repo.rb b/lib/bitbucket/representation/repo.rb index b291dfe0441..8969ecd1c19 100644 --- a/lib/bitbucket/representation/repo.rb +++ b/lib/bitbucket/representation/repo.rb @@ -23,7 +23,9 @@ module Bitbucket url = raw['links']['clone'].find { |link| link['name'] == 'https' }.fetch('href') if token.present? - url.sub(/^[^\@]*/, "https://x-token-auth:#{token}") + clone_url = URI::parse(url) + clone_url.user = "x-token-auth:#{token}" + clone_url.to_s else url end @@ -37,7 +39,7 @@ module Bitbucket raw['full_name'] end - def has_issues? + def issues_enabled? raw['has_issues'] end diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 34d93542955..0f583b07e93 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -32,7 +32,7 @@ module Gitlab end def import_issues - return unless repo.has_issues? + return unless repo.issues_enabled? client.issues(repo).each do |issue| description = @formatter.author_line(issue.author) -- cgit v1.2.1