Merge branch 'fix-default-dependency-list-sort-order' into 'master'

Fix default dependency list sort order icon

See merge request gitlab-org/gitlab!65238

ee/app/assets/javascripts/dependencies/store/modules/list/state.js

@@ -16,5 +16,5 @@ export default () => ({
   },
   filter: FILTER.all,
   sortField: 'severity',
-  sortOrder: SORT_ORDER.ascending,
+  sortOrder: SORT_ORDER.descending,
 });

ee/app/services/security/dependency_list_service.rb

@@ -51,25 +51,33 @@ module Security
     end
 
     def sort(collection)
+      default_sort_order = 'asc'
+
       case params[:sort_by]
       when 'packager'
         collection.sort_by! { |a| a[:packager] }
       when 'severity'
+        default_sort_order = 'desc'
         sort_dependency_vulnerabilities_by_severity!(collection)
         sort_dependencies_by_severity!(collection)
       else
         collection.sort_by! { |a| a[:name] }
       end
 
-      collection.reverse! if params[:sort] == 'desc'
+      if params[:sort] && params[:sort] != default_sort_order
+        collection.reverse!
+      end
 
       collection
     end
 
     def compare_severity_levels(level1, level2)
+      # level2 appears before level1 because we want the default sort order to be in descending
+      # order of severity level, for example "critical, high, medium, low"
       ::Enums::Vulnerability.severity_levels[level2] <=> ::Enums::Vulnerability.severity_levels[level1]
     end
 
+    # sort dependency vulnerabilities in descending order by severity level
     def sort_dependency_vulnerabilities_by_severity!(collection)
       collection.each do |dependency|
         dependency[:vulnerabilities].sort! do |vulnerability1, vulnerability2|
@@ -78,8 +86,8 @@ module Security
       end
     end
 
-    # vulnerabilities are already sorted by severity level so we can assume that first vulnerability in
-    # vulnerabilities array will have highest severity
+    # vulnerabilities are already sorted in descending order by severity level so we can assume that
+    # first vulnerability in the vulnerabilities array will have the highest severity
     def sort_dependencies_by_severity!(collection)
       collection.sort! do |dep_i, dep_j|
         level_i = dep_i.dig(:vulnerabilities, 0, :severity) || :info

ee/spec/frontend/dependencies/components/__snapshots__/dependencies_actions_spec.js.snap

@@ -93,7 +93,7 @@ exports[`DependenciesActions component matches the snapshot 1`] = `
       variant="default"
     >
       <gl-icon-stub
-        name="sort-lowest"
+        name="sort-highest"
         size="16"
       />
     </gl-button-stub>

ee/spec/services/security/dependency_list_service_spec.rb

@@ -97,38 +97,61 @@ RSpec.describe Security::DependencyListService do
         end
       end
 
-      # this test ensures the dependency list severity sort order is `info, unknown, low, medium, high, critical`
-      # which is asending severity order, however, the UI label for this sort order is currently `desc`.
-      # TODO: change the UI label to use `asc` for this sort order and use `desc` for the default sort order
-      # of `critical, high, medium, low, unknown, info`
-      # See https://gitlab.com/gitlab-org/gitlab/-/issues/332653
-      context 'sorted by asc severity' do
+      context 'sorted by severity' do
         let(:params) do
           {
-            sort: 'desc',
             sort_by: 'severity'
           }
         end
 
-        it 'returns array of data sorted by package severity level in ascending order' do
-          dependencies = subject.last(2).map do |dependency|
-            {
-              name: dependency[:name],
-              vulnerabilities: dependency[:vulnerabilities].map do |vulnerability|
-                vulnerability[:severity]
-              end
-            }
+        context 'in descending order' do
+          before do
+            params[:sort] = 'desc'
+          end
+
+          it 'returns array of data sorted by package severity level in descending order' do
+            dependencies = subject.first(2).map do |dependency|
+              {
+                name: dependency[:name],
+                vulnerabilities: dependency[:vulnerabilities].pluck(:severity)
+              }
+            end
+
+            expect(dependencies).to eq([{ name: "saml2-js", vulnerabilities: %w(critical medium unknown) },
+                                        { name: "nokogiri", vulnerabilities: ["high"] }])
           end
 
-          expect(dependencies).to eq([{ name: "nokogiri", vulnerabilities: ["high"] },
-                                      { name: "saml2-js", vulnerabilities: %w(critical medium unknown) }])
+          it 'returns array of data with package vulnerabilities sorted in descending order' do
+            saml2js_dependency = subject.find { |dep| dep[:name] == 'saml2-js' }
+            saml2js_severities = saml2js_dependency[:vulnerabilities].map {|v| v[:severity] }
+
+            expect(saml2js_severities).to eq(%w(critical medium unknown))
+          end
         end
 
-        it 'returns array of data with package vulnerabilities sorted in descending order' do
-          saml2js_dependency = subject.find { |dep| dep[:name] == 'saml2-js' }
-          saml2js_severities = saml2js_dependency[:vulnerabilities].map {|v| v[:severity] }
+        context 'in ascending order' do
+          before do
+            params[:sort] = 'asc'
+          end
+
+          it 'returns array of data sorted by package severity level in ascending order' do
+            dependencies = subject.last(2).map do |dependency|
+              {
+                name: dependency[:name],
+                vulnerabilities: dependency[:vulnerabilities].pluck(:severity)
+              }
+            end
+
+            expect(dependencies).to eq([{ name: "nokogiri", vulnerabilities: ["high"] },
+                                        { name: "saml2-js", vulnerabilities: %w(critical medium unknown) }])
+          end
+
+          it 'returns array of data with package vulnerabilities sorted in descending order' do
+            saml2js_dependency = subject.find { |dep| dep[:name] == 'saml2-js' }
+            saml2js_severities = saml2js_dependency[:vulnerabilities].map {|v| v[:severity] }
 
-          expect(saml2js_severities).to eq(%w(critical medium unknown))
+            expect(saml2js_severities).to eq(%w(critical medium unknown))
+          end
         end
       end
     end