From dc72a8b30502dd28bf850c2dfdbf31b687fde5d3 Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Wed, 23 Dec 2015 15:04:46 -0200
Subject: [PATCH] Refactoring GithubImport::Importer
---
lib/gitlab/github_import/comment.rb | 59 +++++++
lib/gitlab/github_import/importer.rb | 73 ++-------
lib/gitlab/github_import/pull_request.rb | 103 ++++++++++++
spec/lib/gitlab/github_import/comment_spec.rb | 80 +++++++++
.../gitlab/github_import/pull_request_spec.rb | 153 ++++++++++++++++++
5 files changed, 407 insertions(+), 61 deletions(-)
create mode 100644 lib/gitlab/github_import/comment.rb
create mode 100644 lib/gitlab/github_import/pull_request.rb
create mode 100644 spec/lib/gitlab/github_import/comment_spec.rb
create mode 100644 spec/lib/gitlab/github_import/pull_request_spec.rb
diff --git a/lib/gitlab/github_import/comment.rb b/lib/gitlab/github_import/comment.rb
new file mode 100644
index 00000000000..55de78f889d
--- /dev/null
+++ b/lib/gitlab/github_import/comment.rb
@@ -0,0 +1,59 @@
+module Gitlab
+ module GithubImport
+ class Comment
+ attr_reader :project, :raw_data
+
+ def initialize(project, raw_data)
+ @project = project
+ @raw_data = raw_data
+ @formatter = Gitlab::ImportFormatter.new
+ end
+
+ def attributes
+ {
+ project: project,
+ note: note,
+ commit_id: raw_data.commit_id,
+ line_code: line_code,
+ author_id: author_id,
+ created_at: raw_data.created_at,
+ updated_at: raw_data.updated_at
+ }
+ end
+
+ private
+
+ def author
+ raw_data.user.login
+ end
+
+ def author_id
+ gl_user_id(raw_data.user.id) || project.creator_id
+ end
+
+ def body
+ raw_data.body || ""
+ end
+
+ def line_code
+ if on_diff?
+ Gitlab::Diff::LineCode.generate(raw_data.path, raw_data.position, 0)
+ end
+ end
+
+ def on_diff?
+ raw_data.path && raw_data.position
+ end
+
+ def note
+ @formatter.author_line(author) + body
+ end
+
+ def gl_user_id(github_id)
+ User.joins(:identities).
+ find_by("identities.extern_uid = ? AND identities.provider = 'github'", github_id.to_s).
+ try(:id)
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb
index 2c64f5cebc7..7c495655012 100644
--- a/lib/gitlab/github_import/importer.rb
+++ b/lib/gitlab/github_import/importer.rb
@@ -51,80 +51,31 @@ module Gitlab
def import_pull_requests
client.pull_requests(project.import_source, state: :all,
sort: :created,
- direction: :asc).each do |pull_request|
- source_branch = find_branch(pull_request.head.ref)
- target_branch = find_branch(pull_request.base.ref)
+ direction: :asc).each do |raw_data|
+ pull_request = PullRequest.new(project, raw_data)
- if source_branch && target_branch
- merge_request = MergeRequest.create!(
- title: pull_request.title,
- description: format_body(pull_request.user.login, pull_request.body),
- source_project: project,
- source_branch: source_branch.name,
- target_project: project,
- target_branch: target_branch.name,
- state: merge_request_state(pull_request),
- author_id: gl_author_id(project, pull_request.user.id),
- assignee_id: gl_user_id(pull_request.assignee.try(:id)),
- created_at: pull_request.created_at,
- updated_at: pull_request.updated_at
- )
-
- import_comments_on_pull_request(merge_request, pull_request)
- import_comments_on_pull_request_diff(merge_request, pull_request)
+ if pull_request.valid?
+ merge_request = MergeRequest.create!(pull_request.attributes)
+ import_comments_on_pull_request(merge_request, raw_data)
+ import_comments_on_pull_request_diff(merge_request, raw_data)
end
end
end
def import_comments_on_pull_request(merge_request, pull_request)
- client.issue_comments(project.import_source, pull_request.number).each do |c|
- merge_request.notes.create!(
- project: project,
- note: format_body(c.user.login, c.body),
- author_id: gl_author_id(project, c.user.id),
- created_at: c.created_at,
- updated_at: c.updated_at
- )
+ client.issue_comments(project.import_source, pull_request.number).each do |raw_data|
+ comment = Comment.new(project, raw_data)
+ merge_request.notes.create!(comment.attributes)
end
end
def import_comments_on_pull_request_diff(merge_request, pull_request)
- client.pull_request_comments(project.import_source, pull_request.number).each do |c|
- merge_request.notes.create!(
- project: project,
- note: format_body(c.user.login, c.body),
- commit_id: c.commit_id,
- line_code: generate_line_code(c.path, c.position),
- author_id: gl_author_id(project, c.user.id),
- created_at: c.created_at,
- updated_at: c.updated_at
- )
+ client.pull_request_comments(project.import_source, pull_request.number).each do |raw_data|
+ comment = Comment.new(project, raw_data)
+ merge_request.notes.create!(comment.attributes)
end
end
- def find_branch(name)
- project.repository.find_branch(name)
- end
-
- def format_body(author, body)
- @formatter.author_line(author) + (body || "")
- end
-
- def merge_request_state(pull_request)
- case true
- when pull_request.state == 'closed' && pull_request.merged_at.present?
- 'merged'
- when pull_request.state == 'closed'
- 'closed'
- else
- 'opened'
- end
- end
-
- def generate_line_code(file_path, position)
- Gitlab::Diff::LineCode.generate(file_path, position, 0)
- end
-
def gl_author_id(project, github_id)
gl_user_id(github_id) || project.creator_id
end
diff --git a/lib/gitlab/github_import/pull_request.rb b/lib/gitlab/github_import/pull_request.rb
new file mode 100644
index 00000000000..61e846472f2
--- /dev/null
+++ b/lib/gitlab/github_import/pull_request.rb
@@ -0,0 +1,103 @@
+module Gitlab
+ module GithubImport
+ class PullRequest
+ attr_reader :project, :raw_data
+
+ def initialize(project, raw_data)
+ @project = project
+ @raw_data = raw_data
+ @formatter = Gitlab::ImportFormatter.new
+ end
+
+ def attributes
+ {
+ title: raw_data.title,
+ description: description,
+ source_project: source_project,
+ source_branch: source_branch.name,
+ target_project: target_project,
+ target_branch: target_branch.name,
+ state: state,
+ author_id: author_id,
+ assignee_id: assignee_id,
+ created_at: raw_data.created_at,
+ updated_at: updated_at
+ }
+ end
+
+ def valid?
+ source_branch.present? && target_branch.present?
+ end
+
+ private
+
+ def assigned?
+ raw_data.assignee.present?
+ end
+
+ def assignee_id
+ if assigned?
+ gl_user_id(raw_data.assignee.id)
+ end
+ end
+
+ def author
+ raw_data.user.login
+ end
+
+ def author_id
+ gl_user_id(raw_data.user.id) || project.creator_id
+ end
+
+ def body
+ raw_data.body || ""
+ end
+
+ def description
+ @formatter.author_line(author) + body
+ end
+
+ def source_project
+ project
+ end
+
+ def source_branch
+ source_project.repository.find_branch(raw_data.head.ref)
+ end
+
+ def target_project
+ project
+ end
+
+ def target_branch
+ target_project.repository.find_branch(raw_data.base.ref)
+ end
+
+ def state
+ @state ||= case true
+ when raw_data.state == 'closed' && raw_data.merged_at.present?
+ 'merged'
+ when raw_data.state == 'closed'
+ 'closed'
+ else
+ 'opened'
+ end
+ end
+
+ def updated_at
+ case state
+ when 'merged' then raw_data.merged_at
+ when 'closed' then raw_data.closed_at
+ else
+ raw_data.updated_at
+ end
+ end
+
+ def gl_user_id(github_id)
+ User.joins(:identities).
+ find_by("identities.extern_uid = ? AND identities.provider = 'github'", github_id.to_s).
+ try(:id)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/github_import/comment_spec.rb b/spec/lib/gitlab/github_import/comment_spec.rb
new file mode 100644
index 00000000000..ff6b115574b
--- /dev/null
+++ b/spec/lib/gitlab/github_import/comment_spec.rb
@@ -0,0 +1,80 @@
+require 'spec_helper'
+
+describe Gitlab::GithubImport::Comment, lib: true do
+ let(:project) { create(:project) }
+ let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') }
+ let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') }
+ let(:updated_at) { DateTime.strptime('2014-03-03T18:58:10Z') }
+ let(:base_data) do
+ {
+ body: "I'm having a problem with this.",
+ user: octocat,
+ created_at: created_at,
+ updated_at: updated_at
+ }
+ end
+
+ subject(:comment) { described_class.new(project, raw_data)}
+
+ describe '#attributes' do
+ context 'when do not reference a portion of the diff' do
+ let(:raw_data) { OpenStruct.new(base_data) }
+
+ it 'returns formatted attributes' do
+ expected = {
+ project: project,
+ note: "*Created by: octocat*\n\nI'm having a problem with this.",
+ commit_id: nil,
+ line_code: nil,
+ author_id: project.creator_id,
+ created_at: created_at,
+ updated_at: updated_at
+ }
+
+ expect(comment.attributes).to eq(expected)
+ end
+ end
+
+ context 'when on a portion of the diff' do
+ let(:diff_data) do
+ {
+ body: 'Great stuff',
+ commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
+ diff_hunk: '@@ -16,33 +16,40 @@ public class Connection : IConnection...',
+ path: 'file1.txt',
+ position: 1
+ }
+ end
+
+ let(:raw_data) { OpenStruct.new(base_data.merge(diff_data)) }
+
+ it 'returns formatted attributes' do
+ expected = {
+ project: project,
+ note: "*Created by: octocat*\n\nGreat stuff",
+ commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e',
+ line_code: 'ce1be0ff4065a6e9415095c95f25f47a633cef2b_0_1',
+ author_id: project.creator_id,
+ created_at: created_at,
+ updated_at: updated_at
+ }
+
+ expect(comment.attributes).to eq(expected)
+ end
+ end
+
+ context 'when author is a GitLab user' do
+ let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) }
+
+ it 'returns project#creator_id as author_id when is not a GitLab user' do
+ expect(comment.attributes.fetch(:author_id)).to eq project.creator_id
+ end
+
+ it 'returns GitLab user id as author_id when is a GitLab user' do
+ gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github')
+
+ expect(comment.attributes.fetch(:author_id)).to eq gl_user.id
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/github_import/pull_request_spec.rb b/spec/lib/gitlab/github_import/pull_request_spec.rb
new file mode 100644
index 00000000000..6ac32a78955
--- /dev/null
+++ b/spec/lib/gitlab/github_import/pull_request_spec.rb
@@ -0,0 +1,153 @@
+require 'spec_helper'
+
+describe Gitlab::GithubImport::PullRequest, lib: true do
+ let(:project) { create(:project) }
+ let(:source_branch) { OpenStruct.new(ref: 'feature') }
+ let(:target_branch) { OpenStruct.new(ref: 'master') }
+ let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') }
+ let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') }
+ let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') }
+ let(:base_data) do
+ {
+ state: 'open',
+ title: 'New feature',
+ body: 'Please pull these awesome changes',
+ head: source_branch,
+ base: target_branch,
+ assignee: nil,
+ user: octocat,
+ created_at: created_at,
+ updated_at: updated_at,
+ closed_at: nil,
+ merged_at: nil
+ }
+ end
+
+ subject(:pull_request) { described_class.new(project, raw_data)}
+
+ describe '#attributes' do
+ context 'when pull request is open' do
+ let(:raw_data) { OpenStruct.new(base_data.merge(state: 'open')) }
+
+ it 'returns formatted attributes' do
+ expected = {
+ title: 'New feature',
+ description: "*Created by: octocat*\n\nPlease pull these awesome changes",
+ source_project: project,
+ source_branch: 'feature',
+ target_project: project,
+ target_branch: 'master',
+ state: 'opened',
+ author_id: project.creator_id,
+ assignee_id: nil,
+ created_at: created_at,
+ updated_at: updated_at
+ }
+
+ expect(pull_request.attributes).to eq(expected)
+ end
+ end
+
+ context 'when pull request is closed' do
+ let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') }
+ let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', closed_at: closed_at)) }
+
+ it 'returns formatted attributes' do
+ expected = {
+ title: 'New feature',
+ description: "*Created by: octocat*\n\nPlease pull these awesome changes",
+ source_project: project,
+ source_branch: 'feature',
+ target_project: project,
+ target_branch: 'master',
+ state: 'closed',
+ author_id: project.creator_id,
+ assignee_id: nil,
+ created_at: created_at,
+ updated_at: closed_at
+ }
+
+ expect(pull_request.attributes).to eq(expected)
+ end
+ end
+
+ context 'when pull request is merged' do
+ let(:merged_at) { DateTime.strptime('2011-01-28T13:01:12Z') }
+ let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', merged_at: merged_at)) }
+
+ it 'returns formatted attributes' do
+ expected = {
+ title: 'New feature',
+ description: "*Created by: octocat*\n\nPlease pull these awesome changes",
+ source_project: project,
+ source_branch: 'feature',
+ target_project: project,
+ target_branch: 'master',
+ state: 'merged',
+ author_id: project.creator_id,
+ assignee_id: nil,
+ created_at: created_at,
+ updated_at: merged_at
+ }
+
+ expect(pull_request.attributes).to eq(expected)
+ end
+ end
+
+ context 'when it is assigned to someone' do
+ let(:raw_data) { OpenStruct.new(base_data.merge(assignee: octocat)) }
+
+ it 'returns nil as assigned_id when is not a GitLab user' do
+ expect(pull_request.attributes.fetch(:assignee_id)).to be_nil
+ end
+
+ it 'returns GitLab user id as assigned_id when is a GitLab user' do
+ gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github')
+
+ expect(pull_request.attributes.fetch(:assignee_id)).to eq gl_user.id
+ end
+ end
+
+ context 'when author is a GitLab user' do
+ let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) }
+
+ it 'returns project#creator_id as author_id when is not a GitLab user' do
+ expect(pull_request.attributes.fetch(:author_id)).to eq project.creator_id
+ end
+
+ it 'returns GitLab user id as author_id when is a GitLab user' do
+ gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github')
+
+ expect(pull_request.attributes.fetch(:author_id)).to eq gl_user.id
+ end
+ end
+ end
+
+ describe '#valid?' do
+ let(:invalid_branch) { OpenStruct.new(ref: 'invalid-branch') }
+
+ context 'when source and target branches exists' do
+ let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: target_branch)) }
+
+ it 'returns true' do
+ expect(pull_request.valid?).to eq true
+ end
+ end
+
+ context 'when source branch doesn not exists' do
+ let(:raw_data) { OpenStruct.new(base_data.merge(head: invalid_branch, base: target_branch)) }
+
+ it 'returns false' do
+ expect(pull_request.valid?).to eq false
+ end
+ end
+
+ context 'when target branch doesn not exists' do
+ let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: invalid_branch)) }
+
+ it 'returns false' do
+ expect(pull_request.valid?).to eq false
+ end
+ end
+ end
+end
--
2.30.9