Merge branch 'metadata-format-checks' into 'master'

Add format checks for complex fields

See merge request gitlab-org/gitlab!30260

lib/gitlab/jira_import/metadata_collector.rb

@@ -34,22 +34,26 @@ module Gitlab
       end
 
       def add_labels
-        return if fields['labels'].blank?
+        return if fields['labels'].blank? || !fields['labels'].is_a?(Array)
 
         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']}"
+        parent_summary_key = fields.dig('parent', 'fields', 'summary')
+
+        metadata << "- Parent issue: [#{parent_issue_key}] #{parent_summary_key}".strip
       end
 
       def add_versions
-        return if fields['fixVersions'].blank?
+        return if fields['fixVersions'].blank? || !fields['fixVersions'].is_a?(Array)
 
-        metadata << "- Fix versions: #{fields['fixVersions'].map { |version| version['name'] }.join(', ')}"
+        versions = fields['fixVersions'].map { |version| version['name'] }.compact.join(', ')
+        metadata << "- Fix versions: #{versions}"
       end
 
       def fields

spec/lib/gitlab/jira_import/metadata_collector_spec.rb

@@ -51,7 +51,7 @@ describe Gitlab::JiraImport::MetadataCollector do
     subject { described_class.new(jira_issue).execute }
 
     context 'when all metadata fields are present' do
-      it 'skips writes all fields' do
+      it 'writes all fields' do
         expected_result = <<~MD
           ---
 
@@ -70,6 +70,102 @@ describe Gitlab::JiraImport::MetadataCollector do
       end
     end
 
+    context 'when some fields are in incorrect format' do
+      let(:assignee) { nil }
+      let(:parent_field) { nil }
+      let(:fix_versions_field) { [] }
+      let(:priority_field) { nil }
+      let(:labels_field) { [] }
+      let(:environment_field) { nil }
+      let(:duedate_field) { nil }
+
+      context 'when fixVersions field is not an array' do
+        let(:fix_versions_field) { { 'title' => '1.0', 'name' => '1.1' } }
+
+        it 'skips these fields' do
+          expected_result = <<~MD
+            ---
+
+            **Issue metadata**
+
+            - Issue type: Task
+          MD
+
+          expect(subject.strip).to eq(expected_result.strip)
+        end
+      end
+
+      context 'when a fixVersions element is in incorrect format' do
+        let(:fix_versions_field) { [{ 'title' => '1.0' }, { 'name' => '1.1' }] }
+
+        it 'skips the element' do
+          expected_result = <<~MD
+            ---
+
+            **Issue metadata**
+
+            - Issue type: Task
+            - Fix versions: 1.1
+          MD
+
+          expect(subject.strip).to eq(expected_result.strip)
+        end
+      end
+
+      context 'when a labels field is not an array' do
+        let(:labels_field) { { 'first' => 'bug' } }
+
+        it 'skips the labels' do
+          expected_result = <<~MD
+            ---
+
+            **Issue metadata**
+
+            - Issue type: Task
+          MD
+
+          expect(subject.strip).to eq(expected_result.strip)
+        end
+      end
+
+      context 'when a parent field has incorrectly formatted summary' do
+        let(:parent_field) do
+          { 'key' => 'FOO-2', 'id' => '1050', 'other_field' => { 'summary' => 'parent issue FOO' } }
+        end
+
+        it 'skips the summary' do
+          expected_result = <<~MD
+            ---
+
+            **Issue metadata**
+
+            - Issue type: Task
+            - Parent issue: [FOO-2]
+          MD
+
+          expect(subject.strip).to eq(expected_result.strip)
+        end
+      end
+
+      context 'when a parent field is missing the key' do
+        let(:parent_field) do
+          { 'not_key' => 'FOO-2', 'id' => '1050', 'other_field' => { 'summary' => 'parent issue FOO' } }
+        end
+
+        it 'skips the field' do
+          expected_result = <<~MD
+            ---
+
+            **Issue metadata**
+
+            - Issue type: Task
+          MD
+
+          expect(subject.strip).to eq(expected_result.strip)
+        end
+      end
+    end
+
     context 'when some metadata fields are missing' do
       let(:assignee) { nil }
       let(:parent_field) { nil }