Merge branch '290302-sort-container-repositories' into 'master'

Add sort argument to container repos graphql

See merge request gitlab-org/gitlab!53404

app/finders/container_repositories_finder.rb

@@ -14,7 +14,8 @@ class ContainerRepositoriesFinder
     return unless authorized?
 
     repositories = @subject.is_a?(Project) ? project_repositories : group_repositories
-    filter_by_image_name(repositories)
+    repositories = filter_by_image_name(repositories)
+    sort(repositories)
   end
 
   private
@@ -39,6 +40,12 @@ class ContainerRepositoriesFinder
     repositories.search_by_name(@params[:name])
   end
 
+  def sort(repositories)
+    return repositories unless @params[:sort]
+
+    repositories.order_by(@params[:sort])
+  end
+
   def authorized?
     Ability.allowed?(@user, :read_container_image, @subject)
   end

app/graphql/resolvers/container_repositories_resolver.rb

@@ -10,8 +10,13 @@ module Resolvers
               required: false,
               description: 'Filter the container repositories by their name.'
 
-    def resolve(name: nil)
-      ContainerRepositoriesFinder.new(user: current_user, subject: object, params: { name: name })
+    argument :sort, Types::ContainerRepositorySortEnum,
+             description: 'Sort container repositories by this criteria.',
+             required: false,
+             default_value: :created_desc
+
+    def resolve(name: nil, sort: nil)
+      ContainerRepositoriesFinder.new(user: current_user, subject: object, params: { name: name, sort: sort })
                                  .execute
                                  .tap { track_event(:list_repositories, :container) }
     end

app/graphql/types/container_repository_sort_enum.rb

+# frozen_string_literal: true
+
+module Types
+  class ContainerRepositorySortEnum < SortEnum
+    graphql_name 'ContainerRepositorySort'
+    description 'Values for sorting container repositories'
+
+    value 'NAME_ASC', 'Name by ascending order', value: :name_asc
+    value 'NAME_DESC', 'Name by descending order', value: :name_desc
+  end
+end

app/models/container_repository.rb

@@ -4,6 +4,7 @@ class ContainerRepository < ApplicationRecord
   include Gitlab::Utils::StrongMemoize
   include Gitlab::SQL::Pattern
   include EachBatch
+  include Sortable
 
   WAITING_CLEANUP_STATUSES = %i[cleanup_scheduled cleanup_unfinished].freeze
 

changelogs/unreleased/290302-sort-container-repositories.yml

+---
+title: Add sort argument to container_repositories graphql resolver
+merge_request: 53404
+author:
+type: changed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -4427,6 +4427,61 @@ Identifier of ContainerRepository.
 """
 scalar ContainerRepositoryID
 
+"""
+Values for sorting container repositories
+"""
+enum ContainerRepositorySort {
+  """
+  Created at ascending order
+  """
+  CREATED_ASC
+
+  """
+  Created at descending order
+  """
+  CREATED_DESC
+
+  """
+  Name by ascending order
+  """
+  NAME_ASC
+
+  """
+  Name by descending order
+  """
+  NAME_DESC
+
+  """
+  Updated at ascending order
+  """
+  UPDATED_ASC
+
+  """
+  Updated at descending order
+  """
+  UPDATED_DESC
+
+  """
+  Created at ascending order
+  """
+  created_asc @deprecated(reason: "Use CREATED_ASC. Deprecated in 13.5.")
+
+  """
+  Created at descending order
+  """
+  created_desc @deprecated(reason: "Use CREATED_DESC. Deprecated in 13.5.")
+
+  """
+  Updated at ascending order
+  """
+  updated_asc @deprecated(reason: "Use UPDATED_ASC. Deprecated in 13.5.")
+
+  """
+  Updated at descending order
+  """
+  updated_desc @deprecated(reason: "Use UPDATED_DESC. Deprecated in 13.5.")
+}
+
 """
 Status of a container repository
 """
@@ -10650,6 +10705,11 @@ type Group {
     Filter the container repositories by their name.
     """
     name: String
+
+    """
+    Sort container repositories by this criteria.
+    """
+    sort: ContainerRepositorySort = created_desc
   ): ContainerRepositoryConnection
 
   """
@@ -18690,6 +18750,11 @@ type Project {
     Filter the container repositories by their name.
     """
     name: String
+
+    """
+    Sort container repositories by this criteria.
+    """
+    sort: ContainerRepositorySort = created_desc
   ): ContainerRepositoryConnection
 
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -12061,6 +12061,77 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "ENUM",
+          "name": "ContainerRepositorySort",
+          "description": "Values for sorting container repositories",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": [
+            {
+              "name": "updated_desc",
+              "description": "Updated at descending order",
+              "isDeprecated": true,
+              "deprecationReason": "Use UPDATED_DESC. Deprecated in 13.5."
+            },
+            {
+              "name": "updated_asc",
+              "description": "Updated at ascending order",
+              "isDeprecated": true,
+              "deprecationReason": "Use UPDATED_ASC. Deprecated in 13.5."
+            },
+            {
+              "name": "created_desc",
+              "description": "Created at descending order",
+              "isDeprecated": true,
+              "deprecationReason": "Use CREATED_DESC. Deprecated in 13.5."
+            },
+            {
+              "name": "created_asc",
+              "description": "Created at ascending order",
+              "isDeprecated": true,
+              "deprecationReason": "Use CREATED_ASC. Deprecated in 13.5."
+            },
+            {
+              "name": "UPDATED_DESC",
+              "description": "Updated at descending order",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "UPDATED_ASC",
+              "description": "Updated at ascending order",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "CREATED_DESC",
+              "description": "Created at descending order",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "CREATED_ASC",
+              "description": "Created at ascending order",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "NAME_ASC",
+              "description": "Name by ascending order",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "NAME_DESC",
+              "description": "Name by descending order",
+              "isDeprecated": false,
+              "deprecationReason": null
+            }
+          ],
+          "possibleTypes": null
+        },
         {
           "kind": "ENUM",
           "name": "ContainerRepositoryStatus",
@@ -29330,6 +29401,16 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "sort",
+                  "description": "Sort container repositories by this criteria.",
+                  "type": {
+                    "kind": "ENUM",
+                    "name": "ContainerRepositorySort",
+                    "ofType": null
+                  },
+                  "defaultValue": "created_desc"
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",
@@ -54963,6 +55044,16 @@
                   },
                   "defaultValue": null
                 },
+                {
+                  "name": "sort",
+                  "description": "Sort container repositories by this criteria.",
+                  "type": {
+                    "kind": "ENUM",
+                    "name": "ContainerRepositorySort",
+                    "ofType": null
+                  },
+                  "defaultValue": "created_desc"
+                },
                 {
                   "name": "after",
                   "description": "Returns the elements in the list that come after the specified cursor.",

doc/api/graphql/reference/index.md

@@ -4698,6 +4698,23 @@ Status of the tags cleanup of a container repository.
 | `UNFINISHED` | The tags cleanup has been partially executed. There are still remaining tags to delete. |
 | `UNSCHEDULED` | The tags cleanup is not scheduled. This is the default state. |
 
+### ContainerRepositorySort
+
+Values for sorting container repositories.
+
+| Value | Description |
+| ----- | ----------- |
+| `CREATED_ASC` | Created at ascending order |
+| `CREATED_DESC` | Created at descending order |
+| `NAME_ASC` | Name by ascending order |
+| `NAME_DESC` | Name by descending order |
+| `UPDATED_ASC` | Updated at ascending order |
+| `UPDATED_DESC` | Updated at descending order |
+| `created_asc` **{warning-solid}** | **Deprecated:** Use CREATED_ASC. Deprecated in 13.5. |
+| `created_desc` **{warning-solid}** | **Deprecated:** Use CREATED_DESC. Deprecated in 13.5. |
+| `updated_asc` **{warning-solid}** | **Deprecated:** Use UPDATED_ASC. Deprecated in 13.5. |
+| `updated_desc` **{warning-solid}** | **Deprecated:** Use UPDATED_DESC. Deprecated in 13.5. |
+
 ### ContainerRepositoryStatus
 
 Status of a container repository.

spec/finders/container_repositories_finder_spec.rb

@@ -32,6 +32,34 @@ RSpec.describe ContainerRepositoriesFinder do
     end
   end
 
+  shared_examples 'with sorting' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, group: group) }
+    let_it_be(:sort_repository) do
+      create(:container_repository, name: 'bar', project: project, created_at: 1.day.ago)
+    end
+
+    let_it_be(:sort_repository2) do
+      create(:container_repository, name: 'foo', project: project, created_at: 1.hour.ago, updated_at: 1.hour.ago)
+    end
+
+    [:created_desc, :updated_asc, :name_desc].each do |order|
+      context "with sort set to #{order}" do
+        let(:params) { { sort: order } }
+
+        it { is_expected.to eq([sort_repository2, sort_repository])}
+      end
+    end
+
+    [:created_asc, :updated_desc, :name_asc].each do |order|
+      context "with sort set to #{order}" do
+        let(:params) { { sort: order } }
+
+        it { is_expected.to eq([sort_repository, sort_repository2])}
+      end
+    end
+  end
+
   describe '#execute' do
     context 'with authorized user' do
       subject { described_class.new(user: reporter, subject: subject_object, params: params).execute }
@@ -47,6 +75,7 @@ RSpec.describe ContainerRepositoriesFinder do
         it { is_expected.to match_array([project_repository, other_repository]) }
 
         it_behaves_like 'with name search'
+        it_behaves_like 'with sorting'
       end
 
       context 'when subject_type is project' do
@@ -55,6 +84,7 @@ RSpec.describe ContainerRepositoriesFinder do
         it { is_expected.to match_array([project_repository]) }
 
         it_behaves_like 'with name search'
+        it_behaves_like 'with sorting'
       end
 
       context 'with invalid subject_type' do

spec/graphql/resolvers/container_repositories_resolver_spec.rb

@@ -27,6 +27,34 @@ RSpec.describe Resolvers::ContainerRepositoriesResolver do
 
         it { is_expected.to contain_exactly(named_container_repository) }
       end
+
+      context 'with a sort argument' do
+        let_it_be(:group) { create(:group) }
+        let_it_be(:project) { create(:project, group: group) }
+        let_it_be(:sort_repository) do
+          create(:container_repository, name: 'bar', project: project, created_at: 1.day.ago)
+        end
+
+        let_it_be(:sort_repository2) do
+          create(:container_repository, name: 'foo', project: project, created_at: 1.hour.ago, updated_at: 1.hour.ago)
+        end
+
+        [:created_desc, :updated_asc, :name_desc].each do |order|
+          context "#{order}" do
+            let(:args) { { sort: order } }
+
+            it { is_expected.to eq([sort_repository2, sort_repository]) }
+          end
+        end
+
+        [:created_asc, :updated_desc, :name_asc].each do |order|
+          context "#{order}" do
+            let(:args) { { sort: order } }
+
+            it { is_expected.to eq([sort_repository, sort_repository2]) }
+          end
+        end
+      end
     end
 
     context 'with authorized user' do

spec/graphql/types/container_repository_sort_enum_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabSchema.types['ContainerRepositorySort'] do
+  specify { expect(described_class.graphql_name).to eq('ContainerRepositorySort') }
+
+  it_behaves_like 'common sort values'
+
+  it 'exposes all the existing issue sort values' do
+    expect(described_class.values.keys).to include(
+      *%w[NAME_ASC NAME_DESC]
+    )
+  end
+end

spec/requests/api/graphql/project/container_repositories_spec.rb

@@ -156,4 +156,51 @@ RSpec.describe 'getting container repositories in a project' do
 
     expect(container_repositories_count_response).to eq(container_repositories.size)
   end
+
+  describe 'sorting and pagination' do
+    let_it_be(:data_path) { [:project, :container_repositories] }
+    let_it_be(:sort_project) { create(:project, :public) }
+    let_it_be(:current_user) { create(:user) }
+    let_it_be(:container_repository1) { create(:container_repository, name: 'b', project: sort_project) }
+    let_it_be(:container_repository2) { create(:container_repository, name: 'a', project: sort_project) }
+    let_it_be(:container_repository3) { create(:container_repository, name: 'd', project: sort_project) }
+    let_it_be(:container_repository4) { create(:container_repository, name: 'c', project: sort_project) }
+    let_it_be(:container_repository5) { create(:container_repository, name: 'e', project: sort_project) }
+
+    before do
+      stub_container_registry_tags(repository: container_repository1.path, tags: %w(tag1 tag1 tag3), with_manifest: false)
+      stub_container_registry_tags(repository: container_repository2.path, tags: %w(tag4 tag5 tag6), with_manifest: false)
+      stub_container_registry_tags(repository: container_repository3.path, tags: %w(tag7 tag8), with_manifest: false)
+      stub_container_registry_tags(repository: container_repository4.path, tags: %w(tag9), with_manifest: false)
+      stub_container_registry_tags(repository: container_repository5.path, tags: %w(tag10 tag11), with_manifest: false)
+    end
+
+    def pagination_query(params)
+      graphql_query_for(:project, { full_path: sort_project.full_path },
+        query_nodes(:container_repositories, :name, include_pagination_info: true, args: params)
+      )
+    end
+
+    def pagination_results_data(data)
+      data.map { |container_repository| container_repository.dig('name') }
+    end
+
+    context 'when sorting by name' do
+      context 'when ascending' do
+        it_behaves_like 'sorted paginated query' do
+          let(:sort_param) { :NAME_ASC }
+          let(:first_param) { 2 }
+          let(:expected_results) { [container_repository2.name, container_repository1.name, container_repository4.name, container_repository3.name, container_repository5.name] }
+        end
+      end
+
+      context 'when descending' do
+        it_behaves_like 'sorted paginated query' do
+          let(:sort_param) { :NAME_DESC }
+          let(:first_param) { 2 }
+          let(:expected_results) { [container_repository5.name, container_repository3.name, container_repository4.name, container_repository1.name, container_repository2.name] }
+        end
+      end
+    end
+  end
 end