Merge branch 'metadata-2' into 'master'

Extract metadata collection to a separate class

See merge request gitlab-org/gitlab!29693

lib/gitlab/jira_import/issue_serializer.rb

@@ -4,14 +4,12 @@ module Gitlab
   module JiraImport
     class IssueSerializer
       attr_reader :jira_issue, :project, :params, :formatter
-      attr_accessor :metadata
 
       def initialize(project, jira_issue, params = {})
         @jira_issue = jira_issue
         @project = project
         @params = params
         @formatter = Gitlab::ImportFormatter.new
-        @metadata = []
       end
 
       def execute
@@ -38,7 +36,7 @@ module Gitlab
         body << formatter.author_line(jira_issue.reporter.displayName)
         body << formatter.assignee_line(jira_issue.assignee.displayName) if jira_issue.assignee
         body << jira_issue.description
-        body << add_metadata
+        body << MetadataCollector.new(jira_issue).execute
 
         body.join
       end
@@ -51,50 +49,6 @@ module Gitlab
           Issuable::STATE_ID_MAP[:opened]
         end
       end
-
-      def add_metadata
-        add_field(%w(issuetype name), 'Issue type')
-        add_field(%w(priority name), 'Priority')
-        add_labels
-        add_field('environment', 'Environment')
-        add_field('duedate', 'Due date')
-        add_parent
-        add_versions
-
-        return if metadata.empty?
-
-        metadata.join("\n").prepend("\n\n---\n\n**Issue metadata**\n\n")
-      end
-
-      def add_field(keys, field_label)
-        value = fields.dig(*keys)
-        return if value.blank?
-
-        metadata << "- #{field_label}: #{value}"
-      end
-
-      def add_labels
-        return if fields['labels'].blank?
-
-        metadata << "- Labels: #{fields['labels'].join(', ')}"
-      end
-
-      def add_parent
-        parent_issue_key = fields.dig('parent', 'key')
-        return if parent_issue_key.blank?
-
-        metadata << "- Parent issue: [#{parent_issue_key}] #{fields['parent']['fields']['summary']}"
-      end
-
-      def add_versions
-        return if fields['fixVersions'].blank?
-
-        metadata << "- Fix versions: #{fields['fixVersions'].map { |version| version['name'] }.join(', ')}"
-      end
-
-      def fields
-        jira_issue.fields
-      end
     end
   end
 end

lib/gitlab/jira_import/metadata_collector.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module JiraImport
+    class MetadataCollector
+      attr_accessor :jira_issue, :metadata
+
+      def initialize(jira_issue)
+        @jira_issue = jira_issue
+        @metadata = []
+      end
+
+      def execute
+        add_field(%w(issuetype name), 'Issue type')
+        add_field(%w(priority name), 'Priority')
+        add_labels
+        add_field('environment', 'Environment')
+        add_field('duedate', 'Due date')
+        add_parent
+        add_versions
+
+        return if metadata.empty?
+
+        metadata.join("\n").prepend("\n\n---\n\n**Issue metadata**\n\n")
+      end
+
+      private
+
+      def add_field(keys, field_label)
+        value = fields.dig(*keys)
+        return if value.blank?
+
+        metadata << "- #{field_label}: #{value}"
+      end
+
+      def add_labels
+        return if fields['labels'].blank?
+
+        metadata << "- Labels: #{fields['labels'].join(', ')}"
+      end
+
+      def add_parent
+        parent_issue_key = fields.dig('parent', 'key')
+        return if parent_issue_key.blank?
+
+        metadata << "- Parent issue: [#{parent_issue_key}] #{fields['parent']['fields']['summary']}"
+      end
+
+      def add_versions
+        return if fields['fixVersions'].blank?
+
+        metadata << "- Fix versions: #{fields['fixVersions'].map { |version| version['name'] }.join(', ')}"
+      end
+
+      def fields
+        jira_issue.fields
+      end
+    end
+  end
+end

spec/lib/gitlab/jira_import/issue_serializer_spec.rb

@@ -18,22 +18,12 @@ describe Gitlab::JiraImport::IssueSerializer do
     let(:parent_field) do
       { 'key' => 'FOO-2', 'id' => '1050', 'fields' => { 'summary' => 'parent issue FOO' } }
     end
-    let(:issue_type_field) { { 'name' => 'Task' } }
-    let(:fix_versions_field) { [{ 'name' => '1.0' }, { 'name' => '1.1' }] }
     let(:priority_field) { { 'name' => 'Medium' } }
-    let(:labels_field) { %w(bug backend) }
-    let(:environment_field) { 'staging' }
-    let(:duedate_field) { '2020-03-01' }
 
     let(:fields) do
       {
         'parent' => parent_field,
-        'issuetype' => issue_type_field,
-        'fixVersions' => fix_versions_field,
-        'priority' => priority_field,
-        'labels' => labels_field,
-        'environment' => environment_field,
-        'duedate' => duedate_field
+        'priority' => priority_field
       }
     end
 
@@ -68,13 +58,8 @@ describe Gitlab::JiraImport::IssueSerializer do
 
         **Issue metadata**
 
-        - Issue type: Task
         - Priority: Medium
-        - Labels: bug, backend
-        - Environment: staging
-        - Due date: 2020-03-01
         - Parent issue: [FOO-2] parent issue FOO
-        - Fix versions: 1.0, 1.1
       MD
     end
 
@@ -91,54 +76,6 @@ describe Gitlab::JiraImport::IssueSerializer do
           author_id: project.creator_id
         )
       end
-
-      context 'when some metadata fields are missing' do
-        let(:assignee) { nil }
-        let(:parent_field) { nil }
-        let(:fix_versions_field) { [] }
-        let(:labels_field) { [] }
-        let(:environment_field) { nil }
-        let(:duedate_field) { '2020-03-01' }
-
-        it 'skips the missing fields' do
-          expected_description = <<~MD
-            *Created by: Reporter*
-
-            basic description
-
-            ---
-
-            **Issue metadata**
-
-            - Issue type: Task
-            - Priority: Medium
-            - Due date: 2020-03-01
-          MD
-
-          expect(subject[:description]).to eq(expected_description.strip)
-        end
-      end
-
-      context 'when all metadata fields are missing' do
-        let(:assignee) { nil }
-        let(:parent_field) { nil }
-        let(:issue_type_field) { nil }
-        let(:fix_versions_field) { [] }
-        let(:priority_field) { nil }
-        let(:labels_field) { [] }
-        let(:environment_field) { nil }
-        let(:duedate_field) { nil }
-
-        it 'skips the whole metadata secction' do
-          expected_description = <<~MD
-            *Created by: Reporter*
-
-            basic description
-          MD
-
-          expect(subject[:description]).to eq(expected_description.strip)
-        end
-      end
     end
 
     context 'with done status' do

spec/lib/gitlab/jira_import/metadata_collector_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::JiraImport::MetadataCollector do
+  describe '#execute' do
+    let(:key) { 'PROJECT-5' }
+    let(:summary) { 'some title' }
+    let(:description) { 'basic description' }
+    let(:created_at) { '2020-01-01 20:00:00' }
+    let(:updated_at) { '2020-01-10 20:00:00' }
+    let(:assignee) { double(displayName: 'Solver') }
+    let(:jira_status) { 'new' }
+
+    let(:parent_field) do
+      { 'key' => 'FOO-2', 'id' => '1050', 'fields' => { 'summary' => 'parent issue FOO' } }
+    end
+    let(:issue_type_field) { { 'name' => 'Task' } }
+    let(:fix_versions_field) { [{ 'name' => '1.0' }, { 'name' => '1.1' }] }
+    let(:priority_field) { { 'name' => 'Medium' } }
+    let(:labels_field) { %w(bug backend) }
+    let(:environment_field) { 'staging' }
+    let(:duedate_field) { '2020-03-01' }
+
+    let(:fields) do
+      {
+        'parent' => parent_field,
+        'issuetype' => issue_type_field,
+        'fixVersions' => fix_versions_field,
+        'priority' => priority_field,
+        'labels' => labels_field,
+        'environment' => environment_field,
+        'duedate' => duedate_field
+      }
+    end
+    let(:jira_issue) do
+      double(
+        id: '1234',
+        key: key,
+        summary: summary,
+        description: description,
+        created: created_at,
+        updated: updated_at,
+        assignee: assignee,
+        reporter: double(displayName: 'Reporter'),
+        status: double(statusCategory: { 'key' => jira_status }),
+        fields: fields
+      )
+    end
+
+    subject { described_class.new(jira_issue).execute }
+
+    context 'when all metadata fields are present' do
+      it 'skips writes all fields' do
+        expected_result = <<~MD
+          ---
+
+          **Issue metadata**
+
+          - Issue type: Task
+          - Priority: Medium
+          - Labels: bug, backend
+          - Environment: staging
+          - Due date: 2020-03-01
+          - Parent issue: [FOO-2] parent issue FOO
+          - Fix versions: 1.0, 1.1
+        MD
+
+        expect(subject.strip).to eq(expected_result.strip)
+      end
+    end
+
+    context 'when some metadata fields are missing' do
+      let(:assignee) { nil }
+      let(:parent_field) { nil }
+      let(:fix_versions_field) { [] }
+      let(:labels_field) { [] }
+      let(:environment_field) { nil }
+
+      it 'skips the missing fields' do
+        expected_result = <<~MD
+          ---
+
+          **Issue metadata**
+
+          - Issue type: Task
+          - Priority: Medium
+          - Due date: 2020-03-01
+        MD
+
+        expect(subject.strip).to eq(expected_result.strip)
+      end
+    end
+
+    context 'when all metadata fields are missing' do
+      let(:assignee) { nil }
+      let(:parent_field) { nil }
+      let(:issue_type_field) { nil }
+      let(:fix_versions_field) { [] }
+      let(:priority_field) { nil }
+      let(:labels_field) { [] }
+      let(:environment_field) { nil }
+      let(:duedate_field) { nil }
+
+      it 'returns nil' do
+        expect(subject).to be_nil
+      end
+    end
+  end
+end