Merge branch 'ld-graphql-use-new-global_id-for-note-mutations' into 'master'

Update GraphQL note mutations to use new GlobalID types

See merge request gitlab-org/gitlab!45341

.rubocop_todo.yml

@@ -1261,8 +1261,6 @@ Graphql/IDType:
     - 'app/graphql/mutations/discussions/toggle_resolve.rb'
     - 'app/graphql/mutations/issues/update.rb'
     - 'app/graphql/mutations/metrics/dashboard/annotations/delete.rb'
-    - 'app/graphql/mutations/notes/create/note.rb'
-    - 'app/graphql/mutations/notes/update/base.rb'
     - 'app/graphql/mutations/snippets/destroy.rb'
     - 'app/graphql/mutations/snippets/mark_as_spam.rb'
     - 'app/graphql/mutations/snippets/update.rb'

app/graphql/mutations/notes/base.rb

@@ -16,20 +16,6 @@ module Mutations
         id = ::Types::GlobalIDType[::Note].coerce_isolated_input(id)
         GitlabSchema.find_by_gid(id)
       end
-
-      def check_object_is_noteable!(object)
-        unless object.is_a?(Noteable)
-          raise Gitlab::Graphql::Errors::ResourceNotAvailable,
-                'Cannot add notes to this resource'
-        end
-      end
-
-      def check_object_is_note!(object)
-        unless object.is_a?(Note)
-          raise Gitlab::Graphql::Errors::ResourceNotAvailable,
-                'Resource is not a note'
-        end
-      end
     end
   end
 end

app/graphql/mutations/notes/create/base.rb

@@ -26,8 +26,6 @@ module Mutations
         def resolve(args)
           noteable = authorized_find!(id: args[:noteable_id])
 
-          check_object_is_noteable!(noteable)
-
           note = ::Notes::CreateService.new(
             noteable.project,
             current_user,

app/graphql/mutations/notes/create/note.rb

@@ -7,7 +7,7 @@ module Mutations
         graphql_name 'CreateNote'
 
         argument :discussion_id,
-                  GraphQL::ID_TYPE,
+                  ::Types::GlobalIDType[::Discussion],
                   required: false,
                   description: 'The global id of the discussion this note is in reply to'
 
@@ -17,7 +17,11 @@ module Mutations
           discussion_id = nil
 
           if args[:discussion_id]
-            discussion = GitlabSchema.object_from_id(args[:discussion_id], expected_type: ::Discussion)
+            # TODO: remove this line when the compatibility layer is removed
+            # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+            discussion_gid = ::Types::GlobalIDType[::Discussion].coerce_isolated_input(args[:discussion_id])
+            discussion = GitlabSchema.find_by_gid(discussion_gid)
+
             authorize_discussion!(discussion)
 
             discussion_id = discussion.id

app/graphql/mutations/notes/destroy.rb

@@ -15,8 +15,6 @@ module Mutations
       def resolve(id:)
         note = authorized_find!(id: id)
 
-        check_object_is_note!(note)
-
         ::Notes::DestroyService.new(note.project, current_user).execute(note)
 
         {

app/graphql/mutations/notes/update/base.rb

@@ -9,7 +9,7 @@ module Mutations
         authorize :admin_note
 
         argument :id,
-                  GraphQL::ID_TYPE,
+                  ::Types::GlobalIDType[::Note],
                   required: true,
                   description: 'The global id of the note to update'
 

app/graphql/mutations/notes/update/image_diff_note.rb

@@ -33,7 +33,7 @@ module Mutations
 
         private
 
-        def pre_update_checks!(note, args)
+        def pre_update_checks!(note, _args)
           unless note.is_a?(DiffNote) && note.position.on_image?
             raise Gitlab::Graphql::Errors::ResourceNotAvailable,
                   'Resource is not an ImageDiffNote'

app/graphql/mutations/notes/update/note.rb

@@ -18,8 +18,8 @@ module Mutations
 
         private
 
-        def pre_update_checks!(note, _args)
-          check_object_is_note!(note)
+        def pre_update_checks!(_note, _args)
+          # no-op
         end
       end
     end

changelogs/unreleased/ld-graphql-use-new-global_id-for-note-mutations.yml

+---
+title: Updated GraphQL note mutation input ids to be more type-specific
+merge_request: 45341
+author:
+type: changed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -3614,7 +3614,7 @@ input CreateNoteInput {
   """
   The global id of the discussion this note is in reply to
   """
-  discussionId: ID
+  discussionId: DiscussionID
 
   """
   The global id of the resource to add a note to
@@ -5843,6 +5843,11 @@ type DiscussionEdge {
   node: Discussion
 }
 
+"""
+Identifier of Discussion
+"""
+scalar DiscussionID
+
 """
 Autogenerated input type of DiscussionToggleResolve
 """
@@ -19590,7 +19595,7 @@ input UpdateImageDiffNoteInput {
   """
   The global id of the note to update
   """
-  id: ID!
+  id: NoteID!
 
   """
   The position of this note on a diff
@@ -19790,7 +19795,7 @@ input UpdateNoteInput {
   """
   The global id of the note to update
   """
-  id: ID!
+  id: NoteID!
 }
 
 """

doc/api/graphql/reference/gitlab_schema.json

@@ -9665,7 +9665,7 @@
               "description": "The global id of the discussion this note is in reply to",
               "type": {
                 "kind": "SCALAR",
-                "name": "ID",
+                "name": "DiscussionID",
                 "ofType": null
               },
               "defaultValue": null
@@ -16061,6 +16061,16 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "SCALAR",
+          "name": "DiscussionID",
+          "description": "Identifier of Discussion",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "INPUT_OBJECT",
           "name": "DiscussionToggleResolveInput",
@@ -56847,7 +56857,7 @@
                 "name": null,
                 "ofType": {
                   "kind": "SCALAR",
-                  "name": "ID",
+                  "name": "NoteID",
                   "ofType": null
                 }
               },
@@ -57357,7 +57367,7 @@
                 "name": null,
                 "ofType": {
                   "kind": "SCALAR",
-                  "name": "ID",
+                  "name": "NoteID",
                   "ofType": null
                 }
               },

spec/requests/api/graphql/mutations/notes/create/note_spec.rb

@@ -60,6 +60,14 @@ RSpec.describe 'Adding a Note' do
 
           expect(mutation_response['note']['discussion']['id']).to eq(discussion.to_global_id.to_s)
         end
+
+        context 'when the discussion_id is not for a Discussion' do
+          let(:discussion) { create(:issue) }
+
+          it_behaves_like 'a mutation that returns top-level errors' do
+            let(:match_errors) { include(/ does not represent an instance of Discussion/) }
+          end
+        end
       end
     end
   end

spec/requests/api/graphql/mutations/notes/update/image_diff_note_spec.rb

@@ -178,6 +178,12 @@ RSpec.describe 'Updating an image DiffNote' do
       it_behaves_like 'a mutation that returns top-level errors', errors: ['body or position arguments are required']
     end
 
+    context 'when the resource is not a Note' do
+      let(:diff_note) { note }
+
+      it_behaves_like 'a Note mutation when the given resource id is not for a Note'
+    end
+
     context 'when resource is not a DiffNote on an image' do
       let!(:diff_note) { create(:diff_note_on_merge_request, note: original_body) }