Code cleanup from review

Includes spacing fix in description generation

ee/app/services/issues/build_from_vulnerability_service.rb

@@ -5,11 +5,11 @@ module Issues
     def execute
       vulnerability = params[:vulnerability]
 
-      params.merge!({
-        title: "Investigate vulnerability: #{vulnerability.title}",
+      params.merge!(
+        title: _("Investigate vulnerability: %{title}") % { title: vulnerability.title },
         description: render_description(vulnerability),
         confidential: true
-      })
+      )
 
       super
     end

ee/app/views/vulnerabilities/issue_description.md.erb

@@ -15,14 +15,14 @@
 <% if vulnerability.try(:file) %>
 * <%= _("Location") %>: [<%= vulnerability.location_text %>](<%= vulnerability.location_link %>)
 <% end %>
-
 <% if vulnerability.solution.present? %>
+
 ### <%= _("Solution") %>:
 
 <%= vulnerability.solution %>
 <% end %>
-
 <% if vulnerability.identifiers.present? %>
+
 ### <%= _("Identifiers") %>:
 
 <% vulnerability.identifiers.each do |identifier| %>
@@ -33,8 +33,8 @@
 <% end %>
 <% end %>
 <% end %>
-
 <% if vulnerability.links.present? %>
+
 ### <%= _("Links") %>:
 
 <% vulnerability.links.each do |link| %>
@@ -45,8 +45,8 @@
 <% end %>
 <% end %>
 <% end %>
-
 <% if vulnerability.remediations.present? && vulnerability.remediations.any? %>
+
 ### <%= _("Remediations") %>:
 
 <% vulnerability.remediations.each do |remediation| %>
@@ -60,8 +60,8 @@
   </details>
 <% end %>
 <% end %>
-
 <% if vulnerability.scanner.present? || vulnerability.scan.present? %>
+
 ### <%= _("Scanner") %>:
 
 <% if vulnerability.scanner.present? %>

ee/spec/services/ee/issues/build_from_vulnerability_service_spec.rb

@@ -2,94 +2,110 @@
 
 require 'spec_helper'
 
-RSpec.describe Issues::BuildFromVulnerabilityService, '#execute' do
+RSpec.describe Issues::BuildFromVulnerabilityService do
   let_it_be(:group)   { create(:group) }
   let_it_be(:project) { create(:project, :public, :repository, namespace: group) }
   let_it_be(:user)    { create(:user) }
   let(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
   let(:params) { { vulnerability: vulnerability } }
-  let(:result) { described_class.new(project, user, params).execute }
+  let(:issue) { described_class.new(project, user, params).execute }
 
-  before do
-    stub_licensed_features(security_dashboard: true)
-    group.add_developer(user)
-  end
+  describe '#execute' do
+    before_all do
+      group.add_developer(user)
+    end
 
-  context 'when a vulnerability has remediations' do
-    let(:vulnerability) { create(:vulnerability, :with_remediation, project: project) }
+    before do
+      stub_licensed_features(security_dashboard: true)
+    end
 
-    context 'when raw_metadata has no remediations' do
-      let(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
+    context 'when a vulnerability has remediations' do
+      let(:vulnerability) { create(:vulnerability, :with_remediation, project: project) }
 
-      it 'does not display Remediations section' do
-        expect(vulnerability.remediations).to eq(nil)
-        expect(result.description).not_to match(/Remediations/)
-      end
-    end
+      context 'when raw_metadata has no remediations' do
+        let(:vulnerability) { create(:vulnerability, :with_finding, project: project) }
 
-    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!
+        it 'does not display Remediations section' do
+          expect(vulnerability.remediations).to eq(nil)
+          expect(issue.description).not_to match(/Remediations/)
+        end
       end
 
-      it 'does not display Remediations section' do
-        expect(vulnerability.remediations).to eq([nil])
-        expect(result.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(issue.description).not_to match(/Remediations/)
+        end
       end
-    end
 
-    context 'when raw_metadata has a remediation' do
-      it 'displays Remediations section' do
-        expect(vulnerability.remediations.length).to eq(1)
-        expect(result.description).to match(/Remediations/)
-      end
+      context 'when raw_metadata has a remediation' do
+        it 'displays Remediations section' do
+          expect(vulnerability.remediations.length).to eq(1)
+          expect(issue.description).to match(/Remediations/)
+        end
 
-      it 'attaches the diff' do
-        expect(result.description).to match(/This is a diff/)
+        it 'attaches the diff' do
+          expect(issue.description).to match(/This is a diff/)
+        end
       end
     end
-  end
 
-  context 'when an issue is built' do
-    let(:expected_title) { "Investigate vulnerability: #{vulnerability.title}" }
-    let(:expected_description) do
-      <<~DESC.chomp
-        Issue created from vulnerability <a href="http://localhost/#{group.name}/#{project.name}/-/security/vulnerabilities/#{vulnerability.id}">#{vulnerability.id}</a>
+    context 'when an issue is built' do
+      let(:expected_title) { "Investigate vulnerability: #{vulnerability.title}" }
+      let(:expected_description) do
+        <<~DESC
+          Issue created from vulnerability <a href="http://localhost/#{group.name}/#{project.name}/-/security/vulnerabilities/#{vulnerability.id}">#{vulnerability.id}</a>
 
-        ### Description:
+          ### Description:
 
-        Description of #{vulnerability.title}
+          Description of #{vulnerability.title}
 
-        * Severity: #{vulnerability.severity}
-        * Confidence: #{vulnerability.confidence}
-        * Location: [maven/src/main/java/com/gitlab/security_products/tests/App.java:29](http://localhost/#{project.full_path}/-/blob/master/maven/src/main/java/com/gitlab/security_products/tests/App.java#L29)
+          * Severity: #{vulnerability.severity}
+          * Confidence: #{vulnerability.confidence}
+          * Location: [maven/src/main/java/com/gitlab/security_products/tests/App.java:29](http://localhost/#{project.full_path}/-/blob/master/maven/src/main/java/com/gitlab/security_products/tests/App.java#L29)
 
-        ### Solution:
+          ### Solution:
 
-        #{vulnerability.solution}
+          #{vulnerability.solution}
 
-        ### Identifiers:
+          ### Identifiers:
 
-        * [CVE-2018-1234](http://cve.mitre.org/cgi-bin/cvename.cgi?name=2018-1234)
+          * [CVE-2018-1234](http://cve.mitre.org/cgi-bin/cvename.cgi?name=2018-1234)
 
-        ### Links:
+          ### Links:
 
-        * [Cipher does not check for integrity first?](https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first)
-      DESC
-    end
+          * [Cipher does not check for integrity first?](https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first)
+
+          ### Scanner:
 
-    it 'builds the issue with the given params' do
-      issue = result
-      expect(issue).not_to be_persisted
-      expect(issue.project).to eq(project)
-      expect(issue.author).to eq(user)
-      expect(issue.title).to eq(expected_title)
-      expect(issue.description.strip).to eq(expected_description)
-      expect(issue).to be_confidential
+          * Name: Find Security Bugs
+        DESC
+      end
+
+      it 'builds the issue with the given params' do
+        expected_attributes = {
+          project: project,
+          author: user,
+          title: expected_title,
+          description: expected_description
+        }
+
+        expect(issue).not_to be_persisted
+        expect(issue).to have_attributes(expected_attributes)
+        # expect(issue.project).to eq(project)
+        # expect(issue.author).to eq(user)
+        # expect(issue.title).to eq(expected_title)
+        # expect(issue.description.strip).to eq(expected_description)
+        # expect(issue).to be_confidential
+      end
     end
   end
 end

ee/spec/services/ee/issues/create_from_vulnerability_data_service_spec.rb

@@ -118,7 +118,6 @@ RSpec.describe Issues::CreateFromVulnerabilityDataService, '#execute' do
             * [Awesome-security blog post](https;//example.com/blog-post)
             * https://example.com/another-link
 
-
             ### Scanner:
 
             * Name: Gemnasium
@@ -164,9 +163,6 @@ RSpec.describe Issues::CreateFromVulnerabilityDataService, '#execute' do
 
             Please do something!
 
-
-
-
             ### Scanner:
 
             * Name: Gemnasium

ee/spec/services/ee/issues/create_from_vulnerability_service_spec.rb

@@ -119,7 +119,6 @@ RSpec.describe Issues::CreateFromVulnerabilityService, '#execute' do
 
           * [Cipher does not check for integrity first?](https://crypto.stackexchange.com/questions/31428/pbewithmd5anddes-cipher-does-not-check-for-integrity-first)
 
-
           ### Scanner:
 
           * Name: Find Security Bugs