From 98909dd12cd27b85921962326bcaf651c092dcd5 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 23 Dec 2015 20:02:59 -0200 Subject: [PATCH] Generate separate comments when importing GitHub Issues into GitLab --- lib/gitlab/github_import/base_formatter.rb | 21 +++ .../{comment.rb => comment_formatter.rb} | 18 +-- lib/gitlab/github_import/importer.rb | 59 +++----- lib/gitlab/github_import/issue_formatter.rb | 66 +++++++++ ...l_request.rb => pull_request_formatter.rb} | 22 +-- ...ment_spec.rb => comment_formatter_spec.rb} | 2 +- .../github_import/issue_formatter_spec.rb | 139 ++++++++++++++++++ ...spec.rb => pull_request_formatter_spec.rb} | 15 +- 8 files changed, 266 insertions(+), 76 deletions(-) create mode 100644 lib/gitlab/github_import/base_formatter.rb rename lib/gitlab/github_import/{comment.rb => comment_formatter.rb} (63%) create mode 100644 lib/gitlab/github_import/issue_formatter.rb rename lib/gitlab/github_import/{pull_request.rb => pull_request_formatter.rb} (80%) rename spec/lib/gitlab/github_import/{comment_spec.rb => comment_formatter_spec.rb} (97%) create mode 100644 spec/lib/gitlab/github_import/issue_formatter_spec.rb rename spec/lib/gitlab/github_import/{pull_request_spec.rb => pull_request_formatter_spec.rb} (92%) diff --git a/lib/gitlab/github_import/base_formatter.rb b/lib/gitlab/github_import/base_formatter.rb new file mode 100644 index 000000000..202263c67 --- /dev/null +++ b/lib/gitlab/github_import/base_formatter.rb @@ -0,0 +1,21 @@ +module Gitlab + module GithubImport + class BaseFormatter + attr_reader :formatter, :project, :raw_data + + def initialize(project, raw_data) + @project = project + @raw_data = raw_data + @formatter = Gitlab::ImportFormatter.new + end + + private + + 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/comment.rb b/lib/gitlab/github_import/comment_formatter.rb similarity index 63% rename from lib/gitlab/github_import/comment.rb rename to lib/gitlab/github_import/comment_formatter.rb index 55de78f88..7d58e5399 100644 --- a/lib/gitlab/github_import/comment.rb +++ b/lib/gitlab/github_import/comment_formatter.rb @@ -1,14 +1,6 @@ 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 - + class CommentFormatter < BaseFormatter def attributes { project: project, @@ -46,13 +38,7 @@ module Gitlab 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) + formatter.author_line(author) + body end end end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 7c4956550..38ca73722 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -21,29 +21,17 @@ module Gitlab private def import_issues - # Issues && Comments client.list_issues(project.import_source, state: :all, sort: :created, - direction: :asc).each do |issue| - if issue.pull_request.nil? + direction: :asc).each do |raw_data| + gh_issue = IssueFormatter.new(project, raw_data) - body = @formatter.author_line(issue.user.login) - body += issue.body || "" + if gh_issue.valid? + issue = Issue.create!(gh_issue.attributes) - if issue.comments > 0 - body += @formatter.comments_header - - client.issue_comments(project.import_source, issue.number).each do |c| - body += @formatter.comment(c.user.login, c.created_at, c.body) - end + if gh_issue.has_comments? + import_comments(gh_issue.number, issue) end - - project.issues.create!( - description: body, - title: issue.title, - state: issue.state == 'closed' ? 'closed' : 'opened', - author_id: gl_author_id(project, issue.user.id) - ) end end end @@ -52,39 +40,30 @@ module Gitlab client.pull_requests(project.import_source, state: :all, sort: :created, direction: :asc).each do |raw_data| - pull_request = PullRequest.new(project, raw_data) + pull_request = PullRequestFormatter.new(project, raw_data) 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) + import_comments(pull_request.number, merge_request) + import_comments_on_diff(pull_request.number, merge_request) end end end - def import_comments_on_pull_request(merge_request, pull_request) - 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 |raw_data| - comment = Comment.new(project, raw_data) - merge_request.notes.create!(comment.attributes) - end + def import_comments(issue_number, noteable) + comments = client.issue_comments(project.import_source, issue_number) + create_comments(comments, noteable) end - def gl_author_id(project, github_id) - gl_user_id(github_id) || project.creator_id + def import_comments_on_diff(pull_request_number, merge_request) + comments = client.pull_request_comments(project.import_source, pull_request_number) + create_comments(comments, merge_request) end - def gl_user_id(github_id) - if github_id - User.joins(:identities). - find_by("identities.extern_uid = ? AND identities.provider = 'github'", github_id.to_s). - try(:id) + def create_comments(comments, noteable) + comments.each do |raw_data| + comment = CommentFormatter.new(project, raw_data) + noteable.notes.create!(comment.attributes) end end end diff --git a/lib/gitlab/github_import/issue_formatter.rb b/lib/gitlab/github_import/issue_formatter.rb new file mode 100644 index 000000000..1e3ba44f2 --- /dev/null +++ b/lib/gitlab/github_import/issue_formatter.rb @@ -0,0 +1,66 @@ +module Gitlab + module GithubImport + class IssueFormatter < BaseFormatter + def attributes + { + project: project, + title: raw_data.title, + description: description, + state: state, + author_id: author_id, + assignee_id: assignee_id, + created_at: raw_data.created_at, + updated_at: updated_at + } + end + + def has_comments? + raw_data.comments > 0 + end + + def number + raw_data.number + end + + def valid? + raw_data.pull_request.nil? + 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 state + raw_data.state == 'closed' ? 'closed' : 'opened' + end + + def updated_at + state == 'closed' ? raw_data.closed_at : raw_data.updated_at + end + end + end +end diff --git a/lib/gitlab/github_import/pull_request.rb b/lib/gitlab/github_import/pull_request_formatter.rb similarity index 80% rename from lib/gitlab/github_import/pull_request.rb rename to lib/gitlab/github_import/pull_request_formatter.rb index 61e846472..42dc09c2a 100644 --- a/lib/gitlab/github_import/pull_request.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -1,14 +1,6 @@ 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 - + class PullRequestFormatter < BaseFormatter def attributes { title: raw_data.title, @@ -25,6 +17,10 @@ module Gitlab } end + def number + raw_data.number + end + def valid? source_branch.present? && target_branch.present? end @@ -54,7 +50,7 @@ module Gitlab end def description - @formatter.author_line(author) + body + formatter.author_line(author) + body end def source_project @@ -92,12 +88,6 @@ module Gitlab 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_formatter_spec.rb similarity index 97% rename from spec/lib/gitlab/github_import/comment_spec.rb rename to spec/lib/gitlab/github_import/comment_formatter_spec.rb index ff6b11557..a324a82e6 100644 --- a/spec/lib/gitlab/github_import/comment_spec.rb +++ b/spec/lib/gitlab/github_import/comment_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::Comment, lib: true do +describe Gitlab::GithubImport::CommentFormatter, 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') } diff --git a/spec/lib/gitlab/github_import/issue_formatter_spec.rb b/spec/lib/gitlab/github_import/issue_formatter_spec.rb new file mode 100644 index 000000000..fd05428b3 --- /dev/null +++ b/spec/lib/gitlab/github_import/issue_formatter_spec.rb @@ -0,0 +1,139 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::IssueFormatter, lib: true do + let!(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } + 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 + { + number: 1347, + state: 'open', + title: 'Found a bug', + body: "I'm having a problem with this.", + assignee: nil, + user: octocat, + comments: 0, + pull_request: nil, + created_at: created_at, + updated_at: updated_at, + closed_at: nil + } + end + + subject(:issue) { described_class.new(project, raw_data)} + + describe '#attributes' do + context 'when issue is open' do + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'open')) } + + it 'returns formatted attributes' do + expected = { + project: project, + title: 'Found a bug', + description: "*Created by: octocat*\n\nI'm having a problem with this.", + state: 'opened', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: updated_at + } + + expect(issue.attributes).to eq(expected) + end + end + + context 'when issue 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 = { + project: project, + title: 'Found a bug', + description: "*Created by: octocat*\n\nI'm having a problem with this.", + state: 'closed', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: closed_at + } + + expect(issue.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 assignee_id when is not a GitLab user' do + expect(issue.attributes.fetch(:assignee_id)).to be_nil + end + + it 'returns GitLab user id as assignee_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(issue.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(issue.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(issue.attributes.fetch(:author_id)).to eq gl_user.id + end + end + end + + describe '#has_comments?' do + context 'when number of comments is greater than zero' do + let(:raw_data) { OpenStruct.new(base_data.merge(comments: 1)) } + + it 'returns true' do + expect(issue.has_comments?).to eq true + end + end + + context 'when number of comments is equal to zero' do + let(:raw_data) { OpenStruct.new(base_data.merge(comments: 0)) } + + it 'returns false' do + expect(issue.has_comments?).to eq false + end + end + end + + describe '#number' do + let(:raw_data) { OpenStruct.new(base_data.merge(number: 1347)) } + + it 'returns pull request number' do + expect(issue.number).to eq 1347 + end + end + + describe '#valid?' do + context 'when mention a pull request' do + let(:raw_data) { OpenStruct.new(base_data.merge(pull_request: OpenStruct.new)) } + + it 'returns false' do + expect(issue.valid?).to eq false + end + end + + context 'when does not mention a pull request' do + let(:raw_data) { OpenStruct.new(base_data.merge(pull_request: nil)) } + + it 'returns true' do + expect(issue.valid?).to eq true + end + end + end +end diff --git a/spec/lib/gitlab/github_import/pull_request_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb similarity index 92% rename from spec/lib/gitlab/github_import/pull_request_spec.rb rename to spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index 6ac32a789..b4465ef37 100644 --- a/spec/lib/gitlab/github_import/pull_request_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::GithubImport::PullRequest, lib: true do +describe Gitlab::GithubImport::PullRequestFormatter, lib: true do let(:project) { create(:project) } let(:source_branch) { OpenStruct.new(ref: 'feature') } let(:target_branch) { OpenStruct.new(ref: 'master') } @@ -9,6 +9,7 @@ describe Gitlab::GithubImport::PullRequest, lib: true do let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } let(:base_data) do { + number: 1347, state: 'open', title: 'New feature', body: 'Please pull these awesome changes', @@ -97,11 +98,11 @@ describe Gitlab::GithubImport::PullRequest, lib: true do 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 + it 'returns nil as assignee_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 + it 'returns GitLab user id as assignee_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 @@ -123,6 +124,14 @@ describe Gitlab::GithubImport::PullRequest, lib: true do end end + describe '#number' do + let(:raw_data) { OpenStruct.new(base_data.merge(number: 1347)) } + + it 'returns pull request number' do + expect(pull_request.number).to eq 1347 + end + end + describe '#valid?' do let(:invalid_branch) { OpenStruct.new(ref: 'invalid-branch') } -- GitLab