Merge branch 'ajk-globalid-snippets' into 'master'

GraphQL Snippets: use Global-ID scalar

See merge request gitlab-org/gitlab!36117

app/graphql/mutations/snippets/destroy.rb

@@ -7,8 +7,7 @@ module Mutations
 
       ERROR_MSG = 'Error deleting the snippet'
 
-      argument :id,
-               GraphQL::ID_TYPE,
+      argument :id, ::Types::GlobalIDType[::Snippet],
                required: true,
                description: 'The global id of the snippet to destroy'
 

app/graphql/mutations/snippets/mark_as_spam.rb

@@ -5,8 +5,7 @@ module Mutations
     class MarkAsSpam < Base
       graphql_name 'MarkAsSpamSnippet'
 
-      argument :id,
-               GraphQL::ID_TYPE,
+      argument :id, ::Types::GlobalIDType[::Snippet],
                required: true,
                description: 'The global id of the snippet to update'
 

app/graphql/mutations/snippets/update.rb

@@ -7,8 +7,7 @@ module Mutations
 
       graphql_name 'UpdateSnippet'
 
-      argument :id,
-               GraphQL::ID_TYPE,
+      argument :id, ::Types::GlobalIDType[::Snippet],
                required: true,
                description: 'The global id of the snippet to update'
 

app/graphql/queries/snippet/snippet.query.graphql

-query GetSnippetQuery($ids: [ID!]) {
+query GetSnippetQuery($ids: [SnippetID!]) {
   snippets(ids: $ids) {
     __typename
     nodes {

app/graphql/resolvers/concerns/resolves_snippets.rb

@@ -6,7 +6,7 @@ module ResolvesSnippets
   included do
     type Types::SnippetType, null: false
 
-    argument :ids, [GraphQL::ID_TYPE],
+    argument :ids, [::Types::GlobalIDType[::Snippet]],
              required: false,
              description: 'Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"'
 
@@ -32,16 +32,15 @@ module ResolvesSnippets
     }.merge(options_by_type(args[:type]))
   end
 
-  def resolve_ids(ids)
-    Array.wrap(ids).map { |id| resolve_gid(id, :id) }
-  end
-
-  def resolve_gid(gid, argument)
-    return unless gid.present?
+  def resolve_ids(ids, type = ::Types::GlobalIDType[::Snippet])
+    Array.wrap(ids).map do |id|
+      next unless id.present?
 
-    GlobalID.parse(gid)&.model_id.tap do |id|
-      raise Gitlab::Graphql::Errors::ArgumentError, "Invalid global id format for param #{argument}" if id.nil?
-    end
+      # TODO: remove this line when the compatibility layer is removed
+      # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+      id = type.coerce_isolated_input(id)
+      id.model_id
+    end.compact
   end
 
   def options_by_type(type)

app/graphql/resolvers/snippets_resolver.rb

@@ -8,11 +8,11 @@ module Resolvers
 
     alias_method :user, :object
 
-    argument :author_id, GraphQL::ID_TYPE,
+    argument :author_id, ::Types::GlobalIDType[::User],
               required: false,
               description: 'The ID of an author'
 
-    argument :project_id, GraphQL::ID_TYPE,
+    argument :project_id, ::Types::GlobalIDType[::Project],
               required: false,
               description: 'The ID of a project'
 
@@ -36,9 +36,11 @@ module Resolvers
     private
 
     def snippet_finder_params(args)
+      # TODO: remove the type arguments when the compatibility layer is removed
+      # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
       super
-        .merge(author: resolve_gid(args[:author_id], :author),
-               project: resolve_gid(args[:project_id], :project),
+        .merge(author: resolve_ids(args[:author_id], ::Types::GlobalIDType[::User]),
+               project: resolve_ids(args[:project_id], ::Types::GlobalIDType[::Project]),
                explore: args[:explore])
     end
   end

app/graphql/types/snippet_type.rb

@@ -13,7 +13,7 @@ module Types
 
     expose_permissions Types::PermissionTypes::Snippet
 
-    field :id, GraphQL::ID_TYPE,
+    field :id, Types::GlobalIDType[::Snippet],
           description: 'ID of the snippet',
           null: false
 

changelogs/unreleased/ajk-globalid-snippets.yml

+---
+title: 'GraphQL Snippets: use Global-ID scalar'
+merge_request: 36117
+author:
+type: changed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -5955,7 +5955,7 @@ input DestroySnippetInput {
   """
   The global id of the snippet to destroy
   """
-  id: ID!
+  id: SnippetID!
 }
 
 """
@@ -11576,7 +11576,7 @@ input MarkAsSpamSnippetInput {
   """
   The global id of the snippet to update
   """
-  id: ID!
+  id: SnippetID!
 }
 
 """
@@ -15990,7 +15990,7 @@ type Project {
     """
     Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"
     """
-    ids: [ID!]
+    ids: [SnippetID!]
 
     """
     Returns the last _n_ elements from the list.
@@ -17036,7 +17036,7 @@ type Query {
     """
     The ID of an author
     """
-    authorId: ID
+    authorId: UserID
 
     """
     Returns the elements in the list that come before the specified cursor.
@@ -17056,7 +17056,7 @@ type Query {
     """
     Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"
     """
-    ids: [ID!]
+    ids: [SnippetID!]
 
     """
     Returns the last _n_ elements from the list.
@@ -17066,7 +17066,7 @@ type Query {
     """
     The ID of a project
     """
-    projectId: ID
+    projectId: ProjectID
 
     """
     The type of snippet
@@ -19638,7 +19638,7 @@ type Snippet implements Noteable {
   """
   ID of the snippet
   """
-  id: ID!
+  id: SnippetID!
 
   """
   All notes on this noteable
@@ -19916,6 +19916,11 @@ type SnippetEdge {
   node: Snippet
 }
 
+"""
+Identifier of Snippet
+"""
+scalar SnippetID
+
 type SnippetPermissions {
   """
   Indicates the user can perform `admin_snippet` on this resource
@@ -21831,7 +21836,7 @@ input UpdateSnippetInput {
   """
   The global id of the snippet to update
   """
-  id: ID!
+  id: SnippetID!
 
   """
   Title of the snippet
@@ -22139,7 +22144,7 @@ type User {
     """
     Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"
     """
-    ids: [ID!]
+    ids: [SnippetID!]
 
     """
     Returns the last _n_ elements from the list.

doc/api/graphql/reference/gitlab_schema.json

@@ -16300,7 +16300,7 @@
                 "name": null,
                 "ofType": {
                   "kind": "SCALAR",
-                  "name": "ID",
+                  "name": "SnippetID",
                   "ofType": null
                 }
               },
@@ -31712,7 +31712,7 @@
                 "name": null,
                 "ofType": {
                   "kind": "SCALAR",
-                  "name": "ID",
+                  "name": "SnippetID",
                   "ofType": null
                 }
               },
@@ -46307,7 +46307,7 @@
                       "name": null,
                       "ofType": {
                         "kind": "SCALAR",
-                        "name": "ID",
+                        "name": "SnippetID",
                         "ofType": null
                       }
                     }
@@ -49373,7 +49373,7 @@
                       "name": null,
                       "ofType": {
                         "kind": "SCALAR",
-                        "name": "ID",
+                        "name": "SnippetID",
                         "ofType": null
                       }
                     }
@@ -49395,7 +49395,7 @@
                   "description": "The ID of an author",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "UserID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -49405,7 +49405,7 @@
                   "description": "The ID of a project",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "ProjectID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -56879,7 +56879,7 @@
                 "name": null,
                 "ofType": {
                   "kind": "SCALAR",
-                  "name": "ID",
+                  "name": "SnippetID",
                   "ofType": null
                 }
               },
@@ -57745,6 +57745,16 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "SCALAR",
+          "name": "SnippetID",
+          "description": "Identifier of Snippet",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "OBJECT",
           "name": "SnippetPermissions",
@@ -63283,7 +63293,7 @@
                 "name": null,
                 "ofType": {
                   "kind": "SCALAR",
-                  "name": "ID",
+                  "name": "SnippetID",
                   "ofType": null
                 }
               },
@@ -64057,7 +64067,7 @@
                       "name": null,
                       "ofType": {
                         "kind": "SCALAR",
-                        "name": "ID",
+                        "name": "SnippetID",
                         "ofType": null
                       }
                     }

doc/api/graphql/reference/index.md

@@ -2841,7 +2841,7 @@ Represents a snippet entry.
 | `discussions` | DiscussionConnection! | All discussions on this noteable |
 | `fileName` | String | File Name of the snippet |
 | `httpUrlToRepo` | String | HTTP URL to the snippet repository |
-| `id` | ID! | ID of the snippet |
+| `id` | SnippetID! | ID of the snippet |
 | `notes` | NoteConnection! | All notes on this noteable |
 | `project` | Project | The project the snippet is associated with |
 | `rawUrl` | String! | Raw URL of the snippet |

spec/graphql/resolvers/projects/snippets_resolver_spec.rb

@@ -56,12 +56,6 @@ RSpec.describe Resolvers::Projects::SnippetsResolver do
 
         expect(snippets).to contain_exactly(project_snippet, other_project_snippet)
       end
-
-      it 'returns an error if the gid is invalid' do
-        expect do
-          resolve_snippets(args: { ids: 'foo' })
-        end.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
-      end
     end
 
     context 'when no project is provided' do

spec/graphql/resolvers/snippets_resolver_spec.rb

@@ -36,7 +36,7 @@ RSpec.describe Resolvers::SnippetsResolver do
     context 'when using filters' do
       context 'by author id' do
         it 'returns the snippets' do
-          snippets = resolve_snippets(args: { author_id: current_user.to_global_id })
+          snippets = resolve_snippets(args: { author_id: global_id_of(current_user) })
 
           expect(snippets).to contain_exactly(personal_snippet, project_snippet)
         end
@@ -44,7 +44,7 @@ RSpec.describe Resolvers::SnippetsResolver do
         it 'returns an error if the param id is invalid' do
           expect do
             resolve_snippets(args: { author_id: 'foo' })
-          end.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
+          end.to raise_error(GraphQL::CoercionError)
         end
       end
 
@@ -65,7 +65,7 @@ RSpec.describe Resolvers::SnippetsResolver do
         it 'returns an error if the param id is invalid' do
           expect do
             resolve_snippets(args: { project_id: 'foo' })
-          end.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
+          end.to raise_error(GraphQL::CoercionError)
         end
       end
 
@@ -99,14 +99,14 @@ RSpec.describe Resolvers::SnippetsResolver do
         expect(snippets).to contain_exactly(personal_snippet, project_snippet)
       end
 
-      it 'returns an error if the gid is invalid' do
+      it 'returns an error if the id cannot be coerced' do
         args = {
           ids: [personal_snippet.to_global_id, 'foo']
         }
 
         expect do
           resolve_snippets(args: args)
-        end.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
+        end.to raise_error(GraphQL::CoercionError, '"foo" is not a valid Global ID')
       end
 
       it 'returns an error if both project and author are provided' do

spec/graphql/resolvers/users/snippets_resolver_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe Resolvers::Users::SnippetsResolver do
 
         expect do
           resolve_snippets(args: args)
-        end.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
+        end.to raise_error(GraphQL::CoercionError)
       end
     end
   end

spec/requests/api/graphql/mutations/snippets/destroy_spec.rb

@@ -53,10 +53,11 @@ RSpec.describe 'Destroying a Snippet' do
         let!(:snippet_gid) { project.to_gid.to_s }
 
         it 'returns an error' do
+          err_message = %Q["#{snippet_gid}" does not represent an instance of Snippet]
+
           post_graphql_mutation(mutation, current_user: current_user)
 
-          expect(graphql_errors)
-            .to include(a_hash_including('message' => "#{snippet_gid} is not a valid ID for Snippet."))
+          expect(graphql_errors).to include(a_hash_including('message' => a_string_including(err_message)))
         end
 
         it 'does not destroy the Snippet' do