Merge branch '213592-sort-by-classifications' into 'master'

License Compliance - Add `order_by` filter

Closes #213592

See merge request gitlab-org/gitlab!28970

ee/app/controllers/projects/licenses_controller.rb

@@ -70,14 +70,15 @@ module Projects
     end
 
     def matching_policies_params
-      params.permit(:detected, classification: [])
+      params.permit(:detected, :sort_by, :sort_direction, classification: [])
     end
 
     def matching_policies_from(license_compliance)
       filters = matching_policies_params
       license_compliance.find_policies(
         detected_only: truthy?(filters[:detected]),
-        classification: filters[:classification]
+        classification: filters[:classification],
+        sort: { by: filters[:sort_by], direction: filters[:sort_direction] }
       )
     end
 

ee/app/models/sca/license_compliance.rb

@@ -4,6 +4,11 @@ module SCA
   class LicenseCompliance
     include ::Gitlab::Utils::StrongMemoize
 
+    SORT_DIRECTION = {
+      asc: -> (items) { items },
+      desc: -> (items) { items.reverse }
+    }.with_indifferent_access
+
     def initialize(project)
       @project = project
     end
@@ -14,12 +19,13 @@ module SCA
       end
     end
 
-    def find_policies(detected_only: false, classification: [])
+    def find_policies(detected_only: false, classification: [], sort: { by: :name, direction: :asc })
       classifications = Array(classification || [])
-      policies.reject do |policy|
+      matching_policies = policies.reject do |policy|
         (detected_only && policy.dependencies.none?) ||
           (classifications.present? && !policy.classification.in?(classifications))
       end
+      sort_items(items: matching_policies, by: sort&.dig(:by), direction: sort&.dig(:direction))
     end
 
     def latest_build_for_default_branch
@@ -84,5 +90,11 @@ module SCA
     def build_policy(reported_license, software_license_policy)
       ::SCA::LicensePolicy.new(reported_license, software_license_policy)
     end
+
+    def sort_items(items:, by:, direction:, available_attributes: ::SCA::LicensePolicy::ATTRIBUTES)
+      attribute = available_attributes[by] || available_attributes[:name]
+      direction = SORT_DIRECTION[direction] || SORT_DIRECTION[:asc]
+      direction.call(items.sort_by { |item| attribute.call(item) })
+    end
   end
 end

ee/app/models/sca/license_policy.rb

@@ -2,6 +2,12 @@
 
 module SCA
   class LicensePolicy
+    CLASSIFICATION_RANKING = { 'allowed' => 0, 'unclassified' => 1, 'denied' => 2 }.freeze
+    ATTRIBUTES = {
+      classification: ->(policy) { CLASSIFICATION_RANKING[policy.classification] },
+      name: ->(policy) { policy.name }
+    }.with_indifferent_access
+
     attr_reader :id, :name, :url, :dependencies, :spdx_identifier, :classification
 
     def initialize(reported_license, software_policy)

ee/changelogs/unreleased/213592-sort-by-classifications.yml

+---
+title: License Compliance - Add `order_by` filter
+merge_request: 28970
+author:
+type: added

ee/spec/controllers/projects/licenses_controller_spec.rb

@@ -101,6 +101,12 @@ describe Projects::LicensesController do
               it { expect(response).to have_gitlab_http_status(:ok) }
               it { expect(json_response["licenses"].count).to be(4) }
 
+              it 'sorts by name by default' do
+                names = json_response['licenses'].map { |x| x['name'] }
+
+                expect(names).to eql(['BSD 3-Clause "New" or "Revised" License', 'MIT', other_license.name, 'unknown'])
+              end
+
               it 'includes a policy for an unclassified and known license that was detected in the scan report' do
                 expect(json_response.dig("licenses", 0)).to include({
                   "id" => nil,
@@ -233,6 +239,24 @@ describe Projects::LicensesController do
                 })
               end
             end
+
+            context "when loading policies ordered by `classification` in `ascending` order" do
+              before do
+                get :index, params: { namespace_id: project.namespace, project_id: project, sort_by: :classification, sort_direction: :asc }, format: :json
+              end
+
+              specify { expect(response).to have_gitlab_http_status(:ok) }
+              specify { expect(json_response['licenses'].map { |x| x['classification'] }).to eq(%w[allowed unclassified unclassified denied]) }
+            end
+
+            context "when loading policies ordered by `classification` in `descending` order" do
+              before do
+                get :index, params: { namespace_id: project.namespace, project_id: project, sort_by: :classification, sort_direction: :desc }, format: :json
+              end
+
+              specify { expect(response).to have_gitlab_http_status(:ok) }
+              specify { expect(json_response['licenses'].map { |x| x['classification'] }).to eq(%w[denied unclassified unclassified allowed]) }
+            end
           end
 
           context 'without existing report' do

ee/spec/models/sca/license_compliance_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe SCA::LicenseCompliance do
 
   let(:project) { create(:project, :repository, :private) }
   let(:mit) { create(:software_license, :mit) }
-  let(:other_license) { create(:software_license, spdx_identifier: "Other-Id") }
+  let(:other_license) { create(:software_license, name: "SOFTWARE-LICENSE", spdx_identifier: "Other-Id") }
 
   before do
     stub_licensed_features(license_scanning: true)
@@ -255,6 +255,39 @@ RSpec.describe SCA::LicenseCompliance do
         )
       end
     end
+
+    context 'when sorting policies' do
+      let(:sorted_by_name_asc) { ['BSD 3-Clause "New" or "Revised" License', 'MIT', 'SOFTWARE-LICENSE', 'unknown'] }
+
+      where(:attribute, :direction, :expected) do
+        sorted_by_name_asc = ['BSD 3-Clause "New" or "Revised" License', 'MIT', 'SOFTWARE-LICENSE', 'unknown']
+        sorted_by_classification_asc = ['SOFTWARE-LICENSE', 'BSD 3-Clause "New" or "Revised" License', 'unknown', 'MIT']
+        [
+          [:classification, :asc, sorted_by_classification_asc],
+          [:classification, :desc, sorted_by_classification_asc.reverse],
+          [:name, :desc, sorted_by_name_asc.reverse],
+          [:invalid, :asc, sorted_by_name_asc],
+          [:name, :invalid, sorted_by_name_asc],
+          [:name, nil, sorted_by_name_asc],
+          [nil, :asc, sorted_by_name_asc],
+          [nil, nil, sorted_by_name_asc]
+        ]
+      end
+
+      with_them do
+        let(:results) { subject.find_policies(sort: { by: attribute, direction: direction }) }
+
+        it { expect(results.map(&:name)).to eq(expected) }
+      end
+
+      context 'when using the default sort options' do
+        it { expect(subject.find_policies.map(&:name)).to eq(sorted_by_name_asc) }
+      end
+
+      context 'when `nil` sort options are provided' do
+        it { expect(subject.find_policies(sort: nil).map(&:name)).to eq(sorted_by_name_asc) }
+      end
+    end
   end
 
   describe "#latest_build_for_default_branch" do