Merge branch '6240-filtering-for-history-chart' into 'master'

Add filtering for history and summary on security dashboard

See merge request gitlab-org/gitlab-ee!8972

ee/app/controllers/groups/security/vulnerabilities_controller.rb

@@ -4,41 +4,46 @@ class Groups::Security::VulnerabilitiesController < Groups::Security::Applicatio
   HISTORY_RANGE = 3.months
 
   def index
-    @vulnerabilities = ::Security::VulnerabilitiesFinder.new(group: group, params: finder_params)
-      .execute
-      .ordered
-      .page(params[:page])
+    vulnerabilities = found_vulnerabilities.ordered.page(params[:page])
 
     respond_to do |format|
       format.json do
         render json: Vulnerabilities::OccurrenceSerializer
           .new(current_user: @current_user)
           .with_pagination(request, response)
-          .represent(@vulnerabilities, preload: true)
+          .represent(vulnerabilities, preload: true)
       end
     end
   end
 
   def summary
+    vulnerabilities_summary = found_vulnerabilities.counted_by_severity
+
     respond_to do |format|
       format.json do
-        render json: VulnerabilitySummarySerializer.new.represent(group)
+        render json: VulnerabilitySummarySerializer.new.represent(vulnerabilities_summary)
       end
     end
   end
 
   def history
+    vulnerabilities_counter = found_vulnerabilities(:all).count_by_day_and_severity(HISTORY_RANGE)
+
     respond_to do |format|
       format.json do
-        render json: Vulnerabilities::HistorySerializer.new.represent(group.all_vulnerabilities.count_by_day_and_severity(HISTORY_RANGE))
+        render json: Vulnerabilities::HistorySerializer.new.represent(vulnerabilities_counter)
       end
     end
   end
 
   private
 
-  def finder_params
+  def filter_params
     params.permit(report_type: [], project_id: [], severity: [])
       .merge(hide_dismissed: Gitlab::Utils.to_boolean(params[:hide_dismissed]))
   end
+
+  def found_vulnerabilities(collection = :latest)
+    ::Security::VulnerabilitiesFinder.new(group: group, params: filter_params).execute(collection)
+  end
 end

ee/app/finders/security/vulnerabilities_finder.rb

@@ -21,8 +21,8 @@ module Security
       @params = params
     end
 
-    def execute
-      collection = group.latest_vulnerabilities
+    def execute(scope = :latest)
+      collection = init_collection(scope)
       collection = by_report_type(collection)
       collection = by_project(collection)
       collection = by_severity(collection)
@@ -52,5 +52,9 @@ module Security
         Vulnerabilities::Occurrence::LEVELS.values_at(
           *params[:severity]).compact)
     end
+
+    def init_collection(scope)
+      scope == :all ? group.all_vulnerabilities : group.latest_vulnerabilities
+    end
   end
 end

ee/app/models/vulnerabilities/occurrence.rb

@@ -62,7 +62,6 @@ module Vulnerabilities
 
     scope :report_type, -> (type) { where(report_type: self.report_types[type]) }
     scope :ordered, -> { order("severity desc", :id) }
-    scope :counted_by_report_and_severity, -> { group(:report_type, :severity).count }
 
     scope :by_report_types, -> (values) { where(report_type: values) }
     scope :by_projects, -> (values) { where(project_id: values) }
@@ -85,6 +84,10 @@ module Vulnerabilities
         .order('day')
     end
 
+    def self.counted_by_severity
+      group(:severity).count
+    end
+
     def feedback(feedback_type:)
       params = {
         project_id: project_id,

ee/app/serializers/vulnerability_summary_entity.rb

 # frozen_string_literal: true
 
 class VulnerabilitySummaryEntity < Grape::Entity
-  Vulnerabilities::Occurrence::REPORT_TYPES.each do |report_type_name, report_type|
-    expose report_type_name do
   Vulnerabilities::Occurrence::LEVELS.each do |severity_name, severity|
-        expose severity_name do |group|
-          grouped_vulnerabilities[[report_type_name, severity]] || 0
+    expose severity_name do |param|
+      object[severity] || 0
     end
   end
-    end
-  end
-
-  private
-
-  def grouped_vulnerabilities
-    @grouped_by_report_and_severity ||= object.latest_vulnerabilities.counted_by_report_and_severity
-  end
 end

ee/changelogs/unreleased/6240-filtering-for-history-chart.yml

+---
+title: Add filtering for summary and history on security dashboard
+merge_request: 8972
+author:
+type: added

ee/spec/controllers/groups/security/vulnerabilities_controller_spec.rb

@@ -223,11 +223,24 @@ describe Groups::Security::VulnerabilitiesController do
 
           expect(response).to have_gitlab_http_status(200)
           expect(json_response).to be_an(Hash)
-          expect(json_response.dig('sast', 'high')).to eq(3)
-          expect(json_response.dig('dependency_scanning', 'low')).to eq(3)
-          expect(json_response.dig('dast', 'medium')).to eq(1)
+          expect(json_response['high']).to eq(3)
+          expect(json_response['low']).to eq(4)
+          expect(json_response['medium']).to eq(1)
           expect(response).to match_response_schema('vulnerabilities/summary', dir: 'ee')
         end
+
+        context 'with enabled filters' do
+          it 'returns counts for filtered vulnerabilities' do
+            get :summary, group_id: group, report_type: %w[sast dast], severity: %[high low], format: :json
+
+            expect(response).to have_gitlab_http_status(200)
+            expect(json_response).to be_an(Hash)
+            expect(json_response['high']).to eq(3)
+            expect(json_response['low']).to eq(1)
+            expect(json_response['medium']).to eq(1)
+            expect(response).to match_response_schema('vulnerabilities/summary', dir: 'ee')
+          end
+        end
       end
     end
   end
@@ -334,6 +347,21 @@ describe Groups::Security::VulnerabilitiesController do
           })
           expect(response).to match_response_schema('vulnerabilities/history', dir: 'ee')
         end
+
+        it 'returns filtered history if filters are enabled' do
+          travel_to(Time.zone.parse('2019-02-10')) do
+            get :history,  params: { group_id: group, report_type: %w[dependency_scanning sast] }, format: :json
+          end
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to be_an(Hash)
+          expect(json_response['total']).to eq({ '2018-11-10' => 5, '2018-11-12' => 2 })
+          expect(json_response['critical']).to eq({ '2018-11-10' => 1 })
+          expect(json_response['high']).to eq({ '2018-11-10' => 2 })
+          expect(json_response['medium']).to eq({})
+          expect(json_response['low']).to eq({ '2018-11-10' => 2, '2018-11-12' => 2 })
+          expect(response).to match_response_schema('vulnerabilities/history', dir: 'ee')
+        end
       end
     end
   end

ee/spec/fixtures/api/schemas/vulnerabilities/summary.json

 {
   "type" : "object",
-  "required" : [
-    "dast",
-    "sast",
-    "container_scanning",
-    "dependency_scanning"
-  ],
   "properties" : {
-    "dast" : { "$ref": "summary_for_report_type.json" },
-    "sast" : { "$ref": "summary_for_report_type.json" },
-    "container_scanning" : { "$ref": "summary_for_report_type.json" },
-    "dependency_scanning" : { "$ref": "summary_for_report_type.json" }
+    "undefined" : { "type": "integer" },
+    "ignore" : { "type": "integer" },
+    "unknown" : { "type": "integer" },
+    "experimental" : { "type": "integer" },
+    "low" : { "type": "integer" },
+    "medium" : { "type": "integer" },
+    "high" : { "type": "integer" },
+    "critical" : { "type": "integer" }
   },
   "additional_properties" : false
 }

ee/spec/fixtures/api/schemas/vulnerabilities/summary_for_report_type.json

-{
-  "type" : "object",
-  "properties" : {
-    "undefined" : { "type": "integer" },
-    "ignore" : { "type": "integer" },
-    "unknown" : { "type": "integer" },
-    "experimental" : { "type": "integer" },
-    "low" : { "type": "integer" },
-    "medium" : { "type": "integer" },
-    "high" : { "type": "integer" },
-    "critical" : { "type": "integer" }
-  },
-  "additional_properties" : false
-}

ee/spec/models/vulnerabilities/occurrence_spec.rb

@@ -195,4 +195,16 @@ describe Vulnerabilities::Occurrence do
       end
     end
   end
+
+  describe '.count_by_severity' do
+    let!(:high_vulnerabilities) { create_list(:vulnerabilities_occurrence, 3, severity: :high) }
+    let!(:medium_vulnerabilities) { create_list(:vulnerabilities_occurrence, 2, severity: :medium) }
+    let!(:low_vulnerabilities) { create_list(:vulnerabilities_occurrence, 1, severity: :low) }
+
+    subject { described_class.counted_by_severity }
+
+    it 'returns counts' do
+      is_expected.to eq({ 4 => 1, 5 => 2, 6 => 3 })
+    end
+  end
 end