Merge branch '249245-add-sorting-vulnerabilities-by-state' into 'master'

Add ability to sort vulnerabilities by state

See merge request gitlab-org/gitlab!42973

db/migrate/20200928164807_add_index_on_vulnerabilities_state_case.rb

+# frozen_string_literal: true
+
+class AddIndexOnVulnerabilitiesStateCase < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_vulnerabilities_on_state_case_id'
+  STATE_ORDER_ARRAY_POSITION = 'ARRAY_POSITION(ARRAY[1, 4, 3, 2]::smallint[], state)'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :vulnerabilities, "#{STATE_ORDER_ARRAY_POSITION}, id DESC", name: INDEX_NAME
+    add_concurrent_index :vulnerabilities, "#{STATE_ORDER_ARRAY_POSITION} DESC, id DESC", name: "#{INDEX_NAME}_desc"
+  end
+
+  def down
+    remove_concurrent_index_by_name :vulnerabilities, "#{INDEX_NAME}_desc"
+    remove_concurrent_index_by_name :vulnerabilities, INDEX_NAME
+  end
+end

db/schema_migrations/20200928164807

+346d0e913212d6e84528d47228ba7e6d0cf4a396e7fc921f7c684acfaaeeedb8
\ No newline at end of file

db/structure.sql

@@ -21621,6 +21621,10 @@ CREATE INDEX index_vulnerabilities_on_resolved_by_id ON vulnerabilities USING bt
 
 CREATE INDEX index_vulnerabilities_on_start_date_sourcing_milestone_id ON vulnerabilities USING btree (start_date_sourcing_milestone_id);
 
+CREATE INDEX index_vulnerabilities_on_state_case_id ON vulnerabilities USING btree (array_position(ARRAY[(1)::smallint, (4)::smallint, (3)::smallint, (2)::smallint], state), id DESC);
+
+CREATE INDEX index_vulnerabilities_on_state_case_id_desc ON vulnerabilities USING btree (array_position(ARRAY[(1)::smallint, (4)::smallint, (3)::smallint, (2)::smallint], state) DESC, id DESC);
+
 CREATE INDEX index_vulnerabilities_on_updated_by_id ON vulnerabilities USING btree (updated_by_id);
 
 CREATE INDEX index_vulnerability_exports_on_author_id ON vulnerability_exports USING btree (author_id);

doc/api/graphql/reference/gitlab_schema.graphql

@@ -20158,7 +20158,7 @@ type Vulnerability implements Noteable {
   severity: VulnerabilitySeverity
 
   """
-  State of the vulnerability (DETECTED, DISMISSED, RESOLVED, CONFIRMED)
+  State of the vulnerability (DETECTED, CONFIRMED, RESOLVED, DISMISSED)
   """
   state: VulnerabilityState
 
@@ -20820,6 +20820,16 @@ enum VulnerabilitySort {
   """
   severity_desc
 
+  """
+  State in ascending order
+  """
+  state_asc
+
+  """
+  State in descending order
+  """
+  state_desc
+
   """
   Title in ascending order
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -58503,7 +58503,7 @@
             },
             {
               "name": "state",
-              "description": "State of the vulnerability (DETECTED, DISMISSED, RESOLVED, CONFIRMED)",
+              "description": "State of the vulnerability (DETECTED, CONFIRMED, RESOLVED, DISMISSED)",
               "args": [
 
               ],
@@ -60482,6 +60482,18 @@
               "description": "Report Type in ascending order",
               "isDeprecated": false,
               "deprecationReason": null
+            },
+            {
+              "name": "state_desc",
+              "description": "State in descending order",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "state_asc",
+              "description": "State in ascending order",
+              "isDeprecated": false,
+              "deprecationReason": null
             }
           ],
           "possibleTypes": null
@@ -60501,7 +60513,7 @@
               "deprecationReason": null
             },
             {
-              "name": "DISMISSED",
+              "name": "CONFIRMED",
               "description": null,
               "isDeprecated": false,
               "deprecationReason": null
@@ -60513,7 +60525,7 @@
               "deprecationReason": null
             },
             {
-              "name": "CONFIRMED",
+              "name": "DISMISSED",
               "description": null,
               "isDeprecated": false,
               "deprecationReason": null

doc/api/graphql/reference/index.md

@@ -2821,7 +2821,7 @@ Represents a vulnerability.
 | `resolvedOnDefaultBranch` | Boolean! | Indicates whether the vulnerability is fixed on the default branch or not |
 | `scanner` | VulnerabilityScanner | Scanner metadata for the vulnerability. |
 | `severity` | VulnerabilitySeverity | Severity of the vulnerability (INFO, UNKNOWN, LOW, MEDIUM, HIGH, CRITICAL) |
-| `state` | VulnerabilityState | State of the vulnerability (DETECTED, DISMISSED, RESOLVED, CONFIRMED) |
+| `state` | VulnerabilityState | State of the vulnerability (DETECTED, CONFIRMED, RESOLVED, DISMISSED) |
 | `title` | String | Title of the vulnerability |
 | `userNotesCount` | Int! | Number of user notes attached to the vulnerability |
 | `userPermissions` | VulnerabilityPermissions! | Permissions for the current user on the resource |
@@ -3767,6 +3767,8 @@ Vulnerability sort values.
 | `report_type_desc` | Report Type in descending order |
 | `severity_asc` | Severity in ascending order |
 | `severity_desc` | Severity in descending order |
+| `state_asc` | State in ascending order |
+| `state_desc` | State in descending order |
 | `title_asc` | Title in ascending order |
 | `title_desc` | Title in descending order |
 

ee/app/graphql/types/vulnerability_sort_enum.rb

@@ -13,5 +13,7 @@ module Types
     value 'detected_asc', 'Detection timestamp in ascending order'
     value 'report_type_desc', 'Report Type in descending order'
     value 'report_type_asc', 'Report Type in ascending order'
+    value 'state_desc', 'State in descending order'
+    value 'state_asc', 'State in ascending order'
   end
 end

ee/app/models/ee/vulnerability.rb

@@ -54,7 +54,9 @@ module EE
       has_many :notes, as: :noteable, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
       has_many :user_mentions, class_name: 'VulnerabilityUserMention'
 
-      enum state: { detected: 1, dismissed: 2, resolved: 3, confirmed: 4 }
+      # keep the order of the values in the state enum, it is used in state_order method to properly order vulnerabilities based on state
+      # remember to recreate index_vulnerabilities_on_state_case_id index when you update or extend this enum
+      enum state: { detected: 1, confirmed: 4, resolved: 3, dismissed: 2 }
       enum severity: ::Vulnerabilities::Finding::SEVERITY_LEVELS, _prefix: :severity
       enum confidence: ::Vulnerabilities::Finding::CONFIDENCE_LEVELS, _prefix: :confidence
       enum report_type: ::Vulnerabilities::Finding::REPORT_TYPES
@@ -105,6 +107,8 @@ module EE
       scope :order_created_at_desc, -> { reorder(created_at: :desc, id: :desc) }
       scope :order_report_type_asc, -> { select(*arel.projections, report_type_order.as('case_order_value')).reorder(report_type_order.asc, id: :desc) }
       scope :order_report_type_desc, -> { select(*arel.projections, report_type_order.as('case_order_value')).reorder(report_type_order.desc, id: :desc) }
+      scope :order_state_asc, -> { select(*arel.projections, state_order.as('array_position')).reorder(state_order.asc, id: :desc) }
+      scope :order_state_desc, -> { select(*arel.projections, state_order.as('array_position')).reorder(state_order.desc, id: :desc) }
       scope :order_id_desc, -> { reorder(id: :desc) }
 
       scope :with_limit, -> (maximum) { limit(maximum) }
@@ -208,6 +212,16 @@ module EE
           end
       end
 
+      def state_order
+        Arel::Nodes::NamedFunction.new(
+          'ARRAY_POSITION',
+          [
+            Arel.sql("ARRAY#{states.values}::smallint[]"),
+            arel_table[:state]
+          ]
+        )
+      end
+
       def active_states
         ACTIVE_STATES
       end
@@ -230,6 +244,8 @@ module EE
         when 'detected_asc' then order_created_at_asc
         when 'report_type_desc' then order_report_type_desc
         when 'report_type_asc' then order_report_type_asc
+        when 'state_desc' then order_state_desc
+        when 'state_asc' then order_state_asc
         else
           order_severity_desc
         end

ee/changelogs/unreleased/249245-add-sorting-vulnerabilities-by-state.yml

+---
+title: Add ability to sort vulnerabilities by state
+merge_request: 42973
+author:
+type: added

ee/spec/graphql/types/vulnerability_sort_enum_spec.rb

@@ -6,6 +6,6 @@ RSpec.describe GitlabSchema.types['VulnerabilitySort'] do
   it { expect(described_class.graphql_name).to eq('VulnerabilitySort') }
 
   it 'exposes all the existing Vulnerability sort orders' do
-    expect(described_class.values.keys).to include(*%w[severity_desc severity_asc title_desc title_asc detected_desc detected_asc report_type_desc report_type_asc])
+    expect(described_class.values.keys).to include(*%w[severity_desc severity_asc title_desc title_asc detected_desc detected_asc report_type_desc report_type_asc state_desc state_asc])
   end
 end

ee/spec/models/ee/vulnerability_spec.rb

@@ -301,7 +301,7 @@ RSpec.describe Vulnerability do
     end
   end
 
-  describe '.order_report_type' do
+  describe '.order_report_type_' do
     let_it_be(:vulnerability_dast) { create(:vulnerability, :dast) }
     let_it_be(:vulnerability_secret_detection) { create(:vulnerability, :secret_detection) }
     let_it_be(:vulnerability_sast) { create(:vulnerability, :sast) }
@@ -324,6 +324,29 @@ RSpec.describe Vulnerability do
     end
   end
 
+  describe '.order_state_' do
+    let_it_be(:vulnerability_confirmed) { create(:vulnerability, :confirmed) }
+    let_it_be(:vulnerability_detected) { create(:vulnerability, :detected) }
+    let_it_be(:vulnerability_dismissed) { create(:vulnerability, :dismissed) }
+    let_it_be(:vulnerability_resolved) { create(:vulnerability, :resolved) }
+
+    describe 'asc' do
+      subject { described_class.order_state_asc }
+
+      it 'returns vulnerabilities ordered by state' do
+        is_expected.to eq([vulnerability_detected, vulnerability_confirmed, vulnerability_resolved, vulnerability_dismissed])
+      end
+    end
+
+    describe 'desc' do
+      subject { described_class.order_state_desc }
+
+      it 'returns vulnerabilities ordered by state' do
+        is_expected.to eq([vulnerability_dismissed, vulnerability_resolved, vulnerability_confirmed, vulnerability_detected])
+      end
+    end
+  end
+
   describe '.with_resolution' do
     let_it_be(:vulnerability_with_resolution) { create(:vulnerability, resolved_on_default_branch: true) }
     let_it_be(:vulnerability_without_resolution) { create(:vulnerability, resolved_on_default_branch: false) }

lib/gitlab/graphql/pagination/keyset/order_info.rb

@@ -96,6 +96,8 @@ module Gitlab
               ['similarity', order_value.direction, order_value.expr]
             elsif ordering_by_case?(order_value)
               ['case_order_value', order_value.direction, order_value.expr]
+            elsif ordering_by_array_position?(order_value)
+              ['array_position', order_value.direction, order_value.expr]
             else
               [order_value.expr.name, order_value.direction, nil]
             end
@@ -106,6 +108,11 @@ module Gitlab
             order_value.expr.is_a?(Arel::Nodes::NamedFunction) && order_value.expr&.name&.downcase == 'lower'
           end
 
+          # determine if ordering using ARRAY_POSITION, eg. "ORDER BY ARRAY_POSITION(Array[4,3,1,2]::smallint, state)"
+          def ordering_by_array_position?(order_value)
+            order_value.expr.is_a?(Arel::Nodes::NamedFunction) && order_value.expr&.name&.downcase == 'array_position'
+          end
+
           # determine if ordering using SIMILARITY scoring based on Gitlab::Database::SimilarityScore
           def ordering_by_similarity?(order_value)
             Gitlab::Database::SimilarityScore.order_by_similarity?(order_value)

spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb

@@ -74,6 +74,18 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::OrderInfo do
         expect(order_list.first.sort_direction).to eq :asc
       end
     end
+
+    context 'when ordering by ARRAY_POSITION', :aggregate_failuers do
+      let(:array_position) { Arel::Nodes::NamedFunction.new('ARRAY_POSITION', [Arel.sql("ARRAY[1,0]::smallint[]"), Project.arel_table[:auto_cancel_pending_pipelines]]) }
+      let(:relation) { Project.order(array_position.asc) }
+
+      it 'assigns the right attribute name, named function, and direction' do
+        expect(order_list.count).to eq 1
+        expect(order_list.first.attribute_name).to eq 'array_position'
+        expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::NamedFunction)
+        expect(order_list.first.sort_direction).to eq :asc
+      end
+    end
   end
 
   describe '#validate_ordering' do