Merge branch '201897-group-packages-list-api-sorting-is-not-working' into 'master'

Adds sorting to group packages api

Closes #201897

See merge request gitlab-org/gitlab!24432

ee/app/finders/packages/group_packages_finder.rb

@@ -4,7 +4,7 @@ module Packages
   class GroupPackagesFinder
     attr_reader :current_user, :group, :params
 
-    def initialize(current_user, group, params = { exclude_subgroups: false })
+    def initialize(current_user, group, params = { exclude_subgroups: false, order_by: 'created_at', sort: 'asc' })
       @current_user = current_user
       @group = group
       @params = params
@@ -19,7 +19,9 @@ module Packages
     private
 
     def packages_for_group_projects
-      packages = ::Packages::Package.for_projects(group_projects_visible_to_current_user)
+      packages = ::Packages::Package
+        .for_projects(group_projects_visible_to_current_user)
+        .sort_by_attribute("#{params[:order_by]}_#{params[:sort]}")
 
       return packages unless package_type
 

ee/changelogs/unreleased/201897-group-packages-list-api-sorting-is-not-working.yml

+---
+title: Adds sorting to group packages api
+merge_request: 24432
+author:
+type: added

ee/lib/api/group_packages.rb

@@ -21,12 +21,18 @@ module API
       end
       params do
         use :pagination
+        optional :order_by, type: String, values: %w[created_at name version type], default: 'created_at',
+                            desc: 'Return packages ordered by `created_at`, `name`, `version` or `type` fields.'
+        optional :sort, type: String, values: %w[asc desc], default: 'asc',
+                        desc: 'Return packages sorted in `asc` or `desc` order.'
       end
       get ':id/packages' do
         packages = Packages::GroupPackagesFinder.new(
           current_user,
           user_group,
-          exclude_subgroups: params[:exclude_subgroups]
+          exclude_subgroups: params[:exclude_subgroups],
+          order_by: params[:order_by],
+          sort: params[:sort]
         ).execute
 
         present paginate(packages), with: EE::API::Entities::Package, user: current_user, group: true

ee/spec/requests/api/group_packages_spec.rb

@@ -5,8 +5,8 @@ require 'spec_helper'
 describe API::GroupPackages do
   let(:group) { create(:group, :public) }
   let(:project) { create(:project, :public, namespace: group) }
-  let!(:package1) { create(:npm_package, project: project) }
-  let!(:package2) { create(:npm_package, project: project) }
+  let!(:package1) { create(:npm_package, project: project, version: '3.1.0', name: "@#{project.root_namespace.path}/foo1") }
+  let!(:package2) { create(:nuget_package, project: project, version: '2.0.4') }
   let(:user) { create(:user) }
 
   subject { get api(url) }
@@ -20,6 +20,42 @@ describe API::GroupPackages do
         stub_licensed_features(packages: true)
       end
 
+      context 'with sorting' do
+        let(:package3) { create(:maven_package, project: project, version: '1.1.1', name: 'zzz') }
+
+        before do
+          travel_to(1.day.ago) do
+            package3
+          end
+        end
+
+        context 'without sorting params' do
+          let(:packages) { [package3, package1, package2] }
+
+          it 'sorts by created_at asc' do
+            subject
+
+            expect(json_response.map { |package| package['id'] }).to eq(packages.map(&:id))
+          end
+        end
+
+        it_behaves_like 'package sorting', 'name' do
+          let(:packages) { [package1, package2, package3] }
+        end
+
+        it_behaves_like 'package sorting', 'created_at' do
+          let(:packages) { [package3, package1, package2] }
+        end
+
+        it_behaves_like 'package sorting', 'version' do
+          let(:packages) { [package3, package2, package1] }
+        end
+
+        it_behaves_like 'package sorting', 'type' do
+          let(:packages) { [package3, package1, package2] }
+        end
+      end
+
       context 'with private group' do
         let(:group) { create(:group, :private) }
         let(:subgroup) { create(:group, :private, parent: group) }

ee/spec/requests/api/project_packages_spec.rb

@@ -70,32 +70,6 @@ describe API::ProjectPackages do
       end
 
       context 'with sorting' do
-        shared_examples 'package sorting' do |order_by|
-          subject { get api(url), params: { sort: sort, order_by: order_by } }
-
-          context "sorting by #{order_by}" do
-            context 'ascending order' do
-              let(:sort) { 'asc' }
-
-              it 'returns the sorted packages' do
-                subject
-
-                expect(json_response.map { |package| package['id'] }).to eq(packages.map(&:id))
-              end
-            end
-
-            context 'descending order' do
-              let(:sort) { 'desc' }
-
-              it 'returns the sorted packages' do
-                subject
-
-                expect(json_response.map { |package| package['id'] }).to eq(packages.reverse.map(&:id))
-              end
-            end
-          end
-        end
-
         let(:package3) { create(:maven_package, project: project, version: '1.1.1', name: 'zzz') }
 
         before do

ee/spec/support/shared_examples/services/packages_shared_examples.rb

@@ -68,6 +68,32 @@ RSpec.shared_examples 'returns packages with subgroups' do |container_type, user
   end
 end
 
+RSpec.shared_examples 'package sorting' do |order_by|
+  subject { get api(url), params: { sort: sort, order_by: order_by } }
+
+  context "sorting by #{order_by}" do
+    context 'ascending order' do
+      let(:sort) { 'asc' }
+
+      it 'returns the sorted packages' do
+        subject
+
+        expect(json_response.map { |package| package['id'] }).to eq(packages.map(&:id))
+      end
+    end
+
+    context 'descending order' do
+      let(:sort) { 'desc' }
+
+      it 'returns the sorted packages' do
+        subject
+
+        expect(json_response.map { |package| package['id'] }).to eq(packages.reverse.map(&:id))
+      end
+    end
+  end
+end
+
 RSpec.shared_examples 'rejects packages access' do |container_type, user_type, status|
   context "for #{user_type}" do
     before do