GraphQL: Remove runner.projects default sort order value

For guidance on the overall deprecations, removals and breaking changes workflow, please visit https://about.gitlab.com/handbook/product/gitlab-the-product/#breaking-changes-deprecations-and-removing-features

Deprecation Summary

The runner.projects's field default sort order will be removed, and therefore will change from id_asc to the fallback id_desc value.

Removal patch

Patch
diff --git a/app/assets/javascripts/ci/runner/components/runner_projects.vue b/app/assets/javascripts/ci/runner/components/runner_projects.vue
index 4cfc57340f5d..ee67196e8e2d 100644
--- a/app/assets/javascripts/ci/runner/components/runner_projects.vue
+++ b/app/assets/javascripts/ci/runner/components/runner_projects.vue
@@ -71,6 +71,7 @@ export default {
       return {
         id: runner.id,
         search: search.length >= SHORT_SEARCH_LENGTH ? search : '',
+        sort: 'ID_ASC',
         ...getPaginationVariables(this.pagination, RUNNER_DETAILS_PROJECTS_PAGE_SIZE),
       };
     },
diff --git a/app/assets/javascripts/ci/runner/graphql/show/runner_projects.query.graphql b/app/assets/javascripts/ci/runner/graphql/show/runner_projects.query.graphql
index e42648b30795..589a549c52e5 100644
--- a/app/assets/javascripts/ci/runner/graphql/show/runner_projects.query.graphql
+++ b/app/assets/javascripts/ci/runner/graphql/show/runner_projects.query.graphql
@@ -3,6 +3,7 @@
 query getRunnerProjects(
   $id: CiRunnerID!
   $search: String
+  $sort: String
   $first: Int
   $last: Int
   $before: String
@@ -14,7 +15,14 @@ query getRunnerProjects(
       id
     }
     projectCount
-    projects(search: $search, first: $first, last: $last, before: $before, after: $after) {
+    projects(
+      search: $search
+      sort: $sort
+      first: $first
+      last: $last
+      before: $before
+      after: $after
+    ) {
       nodes {
         id
         avatarUrl
diff --git a/app/graphql/resolvers/ci/runner_projects_resolver.rb b/app/graphql/resolvers/ci/runner_projects_resolver.rb
index 625efc615c83..a9c26acf7b29 100644
--- a/app/graphql/resolvers/ci/runner_projects_resolver.rb
+++ b/app/graphql/resolvers/ci/runner_projects_resolver.rb
@@ -13,17 +13,6 @@ class RunnerProjectsResolver < BaseResolver
 
       alias_method :runner, :object
 
-      argument :sort, GraphQL::Types::String,
-               required: false,
-               default_value: 'id_asc', # TODO: Remove in %16.0 and move :sort to ProjectSearchArguments, see https://gitlab.com/gitlab-org/gitlab/-/issues/372117
-               deprecated: {
-                 reason: 'Default sort order will change in 16.0. ' \
-                   'Specify `"id_asc"` if query results\' order is important',
-                 milestone: '15.4'
-               },
-               description: "Sort order of results. Format: `<field_name>_<sort_direction>`, " \
-                 "for example: `id_desc` or `name_asc`"
-
       def resolve_with_lookahead(**args)
         return unless runner.project_type?
 
diff --git a/app/graphql/resolvers/concerns/project_search_arguments.rb b/app/graphql/resolvers/concerns/project_search_arguments.rb
index faf3b85fc141..09cb6c2d0f31 100644
--- a/app/graphql/resolvers/concerns/project_search_arguments.rb
+++ b/app/graphql/resolvers/concerns/project_search_arguments.rb
@@ -19,6 +19,12 @@ module ProjectSearchArguments
     argument :topics, type: [GraphQL::Types::String],
                       required: false,
                       description: 'Filter projects by topics.'
+
+    argument :sort, GraphQL::Types::String,
+             required: false,
+             default_value: 'id_desc',
+             description: "Sort order of results. Format: `<field_name>_<sort_direction>`, " \
+                          "for example: `id_desc` or `name_asc`"
   end
 
   private
diff --git a/app/graphql/resolvers/projects_resolver.rb b/app/graphql/resolvers/projects_resolver.rb
index 08981f2c441b..04a7e05544a2 100644
--- a/app/graphql/resolvers/projects_resolver.rb
+++ b/app/graphql/resolvers/projects_resolver.rb
@@ -10,11 +10,6 @@ class ProjectsResolver < BaseResolver
              required: false,
              description: 'Filter projects by IDs.'
 
-    argument :sort, GraphQL::Types::String,
-             required: false,
-             description: "Sort order of results. Format: `<field_name>_<sort_direction>`, " \
-                 "for example: `id_desc` or `name_asc`"
-
     argument :with_issues_enabled, GraphQL::Types::Boolean,
              required: false,
              description: "Return only projects with issues enabled."
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index cb184b1e7a87..578684558039 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -12238,7 +12238,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="cirunnerprojectsmembership"></a>`membership` | [`Boolean`](#boolean) | Return only projects that the current user is a member of. |
 | <a id="cirunnerprojectssearch"></a>`search` | [`String`](#string) | Search query, which can be for the project name, a path, or a description. |
 | <a id="cirunnerprojectssearchnamespaces"></a>`searchNamespaces` | [`Boolean`](#boolean) | Include namespace in project search. |
-| <a id="cirunnerprojectssort"></a>`sort` **{warning-solid}** | [`String`](#string) | **Deprecated** in 15.4. Default sort order will change in 16.0. Specify `"id_asc"` if query results' order is important. |
+| <a id="cirunnerprojectssort"></a>`sort` | [`String`](#string) | Sort order of results. Format: `<field_name>_<sort_direction>`, for example: `id_desc` or `name_asc`. |
 | <a id="cirunnerprojectstopics"></a>`topics` | [`[String!]`](#string) | Filter projects by topics. |
 
 ##### `CiRunner.status`
diff --git a/spec/frontend/ci/runner/components/runner_projects_spec.js b/spec/frontend/ci/runner/components/runner_projects_spec.js
index afdc54d8ebc8..7d49874bfbc7 100644
--- a/spec/frontend/ci/runner/components/runner_projects_spec.js
+++ b/spec/frontend/ci/runner/components/runner_projects_spec.js
@@ -67,6 +67,7 @@ describe('RunnerProjects', () => {
     expect(mockRunnerProjectsQuery).toHaveBeenCalledWith({
       id: mockRunner.id,
       search: '',
+      sort: 'ID_ASC',
       first: RUNNER_DETAILS_PROJECTS_PAGE_SIZE,
     });
   });
@@ -108,7 +109,6 @@ describe('RunnerProjects', () => {
         name,
         fullName: nameWithNamespace,
         avatarUrl,
-        isOwner: true, // first project is always owner
       });
     });
 
@@ -124,6 +124,7 @@ describe('RunnerProjects', () => {
         expect(mockRunnerProjectsQuery).toHaveBeenLastCalledWith({
           id: mockRunner.id,
           search: '',
+          sort: 'ID_ASC',
           first: RUNNER_DETAILS_PROJECTS_PAGE_SIZE,
           after: 'AFTER_CURSOR',
         });
@@ -138,6 +139,7 @@ describe('RunnerProjects', () => {
         expect(mockRunnerProjectsQuery).toHaveBeenLastCalledWith({
           id: mockRunner.id,
           search: '',
+          sort: 'ID_ASC',
           last: RUNNER_DETAILS_PROJECTS_PAGE_SIZE,
           before: 'BEFORE_CURSOR',
         });
@@ -151,6 +153,7 @@ describe('RunnerProjects', () => {
         expect(mockRunnerProjectsQuery).toHaveBeenLastCalledWith({
           id: mockRunner.id,
           search: 'my search',
+          sort: 'ID_ASC',
           first: RUNNER_DETAILS_PROJECTS_PAGE_SIZE,
         });
       });
@@ -167,6 +170,7 @@ describe('RunnerProjects', () => {
         expect(mockRunnerProjectsQuery).toHaveBeenLastCalledWith({
           id: mockRunner.id,
           search: 'my search',
+          sort: 'ID_ASC',
           first: RUNNER_DETAILS_PROJECTS_PAGE_SIZE,
         });
       });
diff --git a/spec/graphql/resolvers/ci/runner_projects_resolver_spec.rb b/spec/graphql/resolvers/ci/runner_projects_resolver_spec.rb
index 44203fb29120..2fb5149c7f4a 100644
--- a/spec/graphql/resolvers/ci/runner_projects_resolver_spec.rb
+++ b/spec/graphql/resolvers/ci/runner_projects_resolver_spec.rb
@@ -16,7 +16,7 @@
 
   describe '#resolve' do
     context 'with authorized user', :enable_admin_mode do
-      let(:current_user) { create(:user, :admin) }
+      let_it_be(:current_user) { create(:user, :admin) }
 
       context 'with search argument' do
         let(:args) { { search: 'Project1.' } }
@@ -69,15 +69,15 @@
       end
 
       context 'without arguments' do
-        it 'returns a lazy value with all projects sorted by :id_asc' do
+        it 'returns a lazy value with all projects sorted by :id_desc' do
           expect(subject).to be_a(GraphQL::Execution::Lazy)
-          expect(subject.value.items).to eq([project1, project2, project3])
+          expect(subject.value.items).to eq([project3, project2, project1])
         end
       end
     end
 
     context 'with unauthorized user' do
-      let(:current_user) { create(:user) }
+      let_it_be(:current_user) { create(:user) }
 
       it { is_expected.to be_nil }
     end
diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb
index ed180522c980..68c722c5e9d4 100644
--- a/spec/requests/api/graphql/ci/runner_spec.rb
+++ b/spec/requests/api/graphql/ci/runner_spec.rb
@@ -614,8 +614,8 @@
           'projectCount' => 2,
           'projects' => {
             'nodes' => [
-              a_graphql_entity_for(project1),
-              a_graphql_entity_for(project2)
+              a_graphql_entity_for(project2),
+              a_graphql_entity_for(project1)
             ]
           })
         expect(runner2_data).to match a_hash_including(

Breaking Change

Users who rely on the order of the projects returned by runner.projects can specify the desired sort user in the projects field: projects(sort: "id_asc") {}.

Affected Topology

Both

Affected Tier

  • Free
  • Premium
  • Ultimate

Checklists

Labels

  • This issue is labeled deprecation, and with the relevant ~devops::, ~group::, and ~Category: labels.
  • This issue is labeled breaking change if the removal of the deprecated item will be a breaking change.

Timeline

Please add links to the relevant merge requests.

  • As soon as possible, but no later than the third milestone preceding the major release (for example, given the following release schedule: 14.8, 14.9, 14.10, 15.0 – 14.8 is the third milestone preceding the major release):
    • A deprecation announcement entry has been created so the deprecation will appear in release posts and on the general deprecation page. !119082 (diffs)
    • Documentation has been updated to mark the feature as deprecated.
  • On or before the major milestone: A removal entry has been created so the removal will appear on the removals by milestones page and be announced in the release post.
  • On the major milestone:
    • The deprecated item has been removed ( !117904 (merged)).
    • If the removal of the deprecated item is a breaking change, the merge request is labeled breaking change.

Mentions

  • Your stage's stable counterparts have been @mentioned on this issue. For example, Customer Support, Customer Success (Technical Account Manager), Product Marketing Manager.
    • To see who the stable counterparts are for a product team visit product categories
      • If there is no stable counterpart listed for Sales/CS please mention @timtams
      • If there is no stable counterpart listed for Support please mention @gitlab-com/support/managers
      • If there is no stable counterpart listed for Marketing please mention @cfoster3
  • Your GPM has been @mentioned so that they are aware of planned deprecations. The goal is to have reviews happen at least two releases before the final removal of the feature or introduction of a breaking change.

Deprecation Milestone

%16.0

Planned Removal Milestone

%17.0

Links

GraphQL: Add resolver to runner projects (!96386 - merged)

The runner.projects's field is sorted by id ASC. In order to harmonize the behavior with other projects resolvers, in %16.0 the default sort order will be removed, and the fallback id DESC sort order will be applied.

label GitLab Free

Edited Mar 28, 2024 by Pedro Pombeiro
Assignee Loading
Time tracking Loading