Generate separate comments when importing GitHub Issues into GitLab

8 files changed, 266 insertions, 76 deletions

diff --git a/lib/gitlab/github_import/base_formatter.rb b/lib/gitlab/github_import/base_formatter.rb
new file mode 100644
index 00000000000..202263c6742
--- /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
index 55de78f889d..7d58e53991a 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 7c495655012..38ca7372202 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 00000000000..1e3ba44f27c
--- /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
index 61e846472f2..42dc09c2ac5 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
index ff6b115574b..a324a82e69f 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 00000000000..fd05428b322
--- /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
index 6ac32a78955..b4465ef3743 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') }