From 1a0d1ad0ebd769f643a56683afc76a729d367646 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Zaj=C4=85c?= <mzajac@gitlab.com>
Date: Mon, 21 Sep 2020 04:00:03 +0200
Subject: [PATCH] Handle different remediations values

The `remediations` key in metadata can be:

1. not present
2. be null
3. be [null]
4. array of Hashes with "summary" and "diff" keys
---
 ee/app/presenters/vulnerability_presenter.rb  |  4 ++
 ee/spec/factories/vulnerabilities.rb          | 15 ++++++
 ee/spec/factories/vulnerabilities/findings.rb |  6 ++-
 .../create_from_vulnerability_service_spec.rb | 52 +++++++++++++++----
 4 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/ee/app/presenters/vulnerability_presenter.rb b/ee/app/presenters/vulnerability_presenter.rb
index 55e68566bff..f04b3f6f87f 100644
--- a/ee/app/presenters/vulnerability_presenter.rb
+++ b/ee/app/presenters/vulnerability_presenter.rb
@@ -7,6 +7,10 @@ class VulnerabilityPresenter < Gitlab::View::Presenter::Delegated
     vulnerability.links.map(&:with_indifferent_access)
   end
 
+  def remediations
+    vulnerability.remediations.to_a.compact.map(&:with_indifferent_access)
+  end
+
   def location_text
     return file unless line
 
diff --git a/ee/spec/factories/vulnerabilities.rb b/ee/spec/factories/vulnerabilities.rb
index 98bc729a329..8a9cf96e38a 100644
--- a/ee/spec/factories/vulnerabilities.rb
+++ b/ee/spec/factories/vulnerabilities.rb
@@ -86,6 +86,21 @@ FactoryBot.define do
       end
     end
 
+    trait :with_remediation do
+      after(:build) do |vulnerability|
+        finding = build(
+          :vulnerabilities_finding,
+          :identifier,
+          :with_remediation,
+          vulnerability: vulnerability,
+          report_type: vulnerability.report_type,
+          project: vulnerability.project
+        )
+
+        vulnerability.findings = [finding]
+      end
+    end
+
     trait :with_findings do
       after(:build) do |vulnerability|
         findings_with_solution = build_list(
diff --git a/ee/spec/factories/vulnerabilities/findings.rb b/ee/spec/factories/vulnerabilities/findings.rb
index b743b4fde96..3c81ed09863 100644
--- a/ee/spec/factories/vulnerabilities/findings.rb
+++ b/ee/spec/factories/vulnerabilities/findings.rb
@@ -16,7 +16,8 @@ FactoryBot.define do
         raw_metadata.delete('solution')
         raw_metadata['remediations'] = [
           {
-            summary: evaluator.summary
+            summary: evaluator.summary,
+            diff: Base64.encode64("This ain't a diff")
           }
         ]
         finding.raw_metadata = raw_metadata.to_json
@@ -114,7 +115,8 @@ FactoryBot.define do
         raw_metadata.delete(:solution)
         raw_metadata[:remediations] = [
           {
-            summary: 'Use GCM mode which includes HMAC in the resulting encrypted data, providing integrity of the result.'
+            summary: 'Use GCM mode which includes HMAC in the resulting encrypted data, providing integrity of the result.',
+            diff: Base64.encode64("This is a diff")
           }
         ]
         finding.raw_metadata = raw_metadata.to_json
diff --git a/ee/spec/services/ee/issues/create_from_vulnerability_service_spec.rb b/ee/spec/services/ee/issues/create_from_vulnerability_service_spec.rb
index 646c6f359b5..eb79c67d5d8 100644
--- a/ee/spec/services/ee/issues/create_from_vulnerability_service_spec.rb
+++ b/ee/spec/services/ee/issues/create_from_vulnerability_service_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Issues::CreateFromVulnerabilityService, '#execute' do
   let_it_be(:group)   { create(:group) }
   let_it_be(:project) { create(:project, :public, :repository, namespace: group) }
   let_it_be(:user)    { create(:user) }
-  let_it_be(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
+  let(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
   let(:params) { { vulnerability: vulnerability, link_type: Vulnerabilities::IssueLink.link_types[:created] } }
 
   before do
@@ -32,18 +32,48 @@ RSpec.describe Issues::CreateFromVulnerabilityService, '#execute' do
   context 'when a vulnerability exists' do
     let(:result) { described_class.new(container: project, current_user: user, params: params).execute }
 
-    context 'when raw_metadata has no remediations' do
-      before do
-        finding = vulnerability.finding
-        metadata = Gitlab::Json.parse(finding.raw_metadata)
-        metadata["remediations"] = [nil]
-        finding.raw_metadata = metadata.to_json
-        finding.save!
+    context 'is a vulnerability with remediations' do
+      let(:vulnerability) { create(:vulnerability, :with_remediation, project: project) }
+
+      context 'when raw_metadata has no remediations' do
+        before do
+          finding = vulnerability.finding
+          metadata = Gitlab::Json.parse(finding.raw_metadata)
+          metadata.delete("remediations")
+          finding.raw_metadata = metadata.to_json
+          finding.save!
+        end
+
+        it 'does not display Remediations section' do
+          expect(vulnerability.remediations).to eq(nil)
+          expect(result[:issue].description).not_to match(/Remediations/)
+        end
       end
 
-      it 'does not display Remediations section' do
-        expect(vulnerability.remediations).to eq([nil])
-        expect(result[:issue].description).not_to match(/Remediations/)
+      context 'when raw_metadata has empty remediations key' do
+        before do
+          finding = vulnerability.finding
+          metadata = Gitlab::Json.parse(finding.raw_metadata)
+          metadata["remediations"] = [nil]
+          finding.raw_metadata = metadata.to_json
+          finding.save!
+        end
+
+        it 'does not display Remediations section' do
+          expect(vulnerability.remediations).to eq([nil])
+          expect(result[:issue].description).not_to match(/Remediations/)
+        end
+      end
+
+      context 'when raw_metadata has a remediation' do
+        it 'displays Remediations section' do
+          expect(vulnerability.remediations.length).to eq(1)
+          expect(result[:issue].description).to match(/Remediations/)
+        end
+
+        it 'attaches the diff' do
+          expect(result[:issue].description).to match(/This is a diff/)
+        end
       end
     end
 
-- 
2.30.9