Commit 3c3e8801 authored by Luke Duncalfe's avatar Luke Duncalfe Committed by charlie ablett

Update GraphQL note mutations to use new GlobalID

This changes the note mutations to use the new `GlobalID` types for
arguments, with the temporary compatibility layer added in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36209.

https://gitlab.com/gitlab-org/gitlab/-/issues/268042
parent dd4c115b
...@@ -1264,8 +1264,6 @@ Graphql/IDType: ...@@ -1264,8 +1264,6 @@ Graphql/IDType:
- 'app/graphql/mutations/issues/update.rb' - 'app/graphql/mutations/issues/update.rb'
- 'app/graphql/mutations/merge_requests/set_milestone.rb' - 'app/graphql/mutations/merge_requests/set_milestone.rb'
- 'app/graphql/mutations/metrics/dashboard/annotations/delete.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/destroy.rb'
- 'app/graphql/mutations/snippets/mark_as_spam.rb' - 'app/graphql/mutations/snippets/mark_as_spam.rb'
- 'app/graphql/mutations/snippets/update.rb' - 'app/graphql/mutations/snippets/update.rb'
......
...@@ -16,20 +16,6 @@ module Mutations ...@@ -16,20 +16,6 @@ module Mutations
id = ::Types::GlobalIDType[::Note].coerce_isolated_input(id) id = ::Types::GlobalIDType[::Note].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end 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 end
end end
...@@ -26,8 +26,6 @@ module Mutations ...@@ -26,8 +26,6 @@ module Mutations
def resolve(args) def resolve(args)
noteable = authorized_find!(id: args[:noteable_id]) noteable = authorized_find!(id: args[:noteable_id])
check_object_is_noteable!(noteable)
note = ::Notes::CreateService.new( note = ::Notes::CreateService.new(
noteable.project, noteable.project,
current_user, current_user,
......
...@@ -7,7 +7,7 @@ module Mutations ...@@ -7,7 +7,7 @@ module Mutations
graphql_name 'CreateNote' graphql_name 'CreateNote'
argument :discussion_id, argument :discussion_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Discussion],
required: false, required: false,
description: 'The global id of the discussion this note is in reply to' description: 'The global id of the discussion this note is in reply to'
...@@ -17,7 +17,11 @@ module Mutations ...@@ -17,7 +17,11 @@ module Mutations
discussion_id = nil discussion_id = nil
if args[:discussion_id] 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) authorize_discussion!(discussion)
discussion_id = discussion.id discussion_id = discussion.id
......
...@@ -15,8 +15,6 @@ module Mutations ...@@ -15,8 +15,6 @@ module Mutations
def resolve(id:) def resolve(id:)
note = authorized_find!(id: id) note = authorized_find!(id: id)
check_object_is_note!(note)
::Notes::DestroyService.new(note.project, current_user).execute(note) ::Notes::DestroyService.new(note.project, current_user).execute(note)
{ {
......
...@@ -9,7 +9,7 @@ module Mutations ...@@ -9,7 +9,7 @@ module Mutations
authorize :admin_note authorize :admin_note
argument :id, argument :id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Note],
required: true, required: true,
description: 'The global id of the note to update' description: 'The global id of the note to update'
......
...@@ -33,7 +33,7 @@ module Mutations ...@@ -33,7 +33,7 @@ module Mutations
private private
def pre_update_checks!(note, args) def pre_update_checks!(note, _args)
unless note.is_a?(DiffNote) && note.position.on_image? unless note.is_a?(DiffNote) && note.position.on_image?
raise Gitlab::Graphql::Errors::ResourceNotAvailable, raise Gitlab::Graphql::Errors::ResourceNotAvailable,
'Resource is not an ImageDiffNote' 'Resource is not an ImageDiffNote'
......
...@@ -18,8 +18,8 @@ module Mutations ...@@ -18,8 +18,8 @@ module Mutations
private private
def pre_update_checks!(note, _args) def pre_update_checks!(_note, _args)
check_object_is_note!(note) # no-op
end end
end end
end end
......
---
title: Updated GraphQL note mutation input ids to be more type-specific
merge_request: 45341
author:
type: changed
...@@ -3614,7 +3614,7 @@ input CreateNoteInput { ...@@ -3614,7 +3614,7 @@ input CreateNoteInput {
""" """
The global id of the discussion this note is in reply to 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 The global id of the resource to add a note to
...@@ -5843,6 +5843,11 @@ type DiscussionEdge { ...@@ -5843,6 +5843,11 @@ type DiscussionEdge {
node: Discussion node: Discussion
} }
"""
Identifier of Discussion
"""
scalar DiscussionID
""" """
Autogenerated input type of DiscussionToggleResolve Autogenerated input type of DiscussionToggleResolve
""" """
...@@ -19591,7 +19596,7 @@ input UpdateImageDiffNoteInput { ...@@ -19591,7 +19596,7 @@ input UpdateImageDiffNoteInput {
""" """
The global id of the note to update The global id of the note to update
""" """
id: ID! id: NoteID!
""" """
The position of this note on a diff The position of this note on a diff
...@@ -19791,7 +19796,7 @@ input UpdateNoteInput { ...@@ -19791,7 +19796,7 @@ input UpdateNoteInput {
""" """
The global id of the note to update The global id of the note to update
""" """
id: ID! id: NoteID!
} }
""" """
......
...@@ -9665,7 +9665,7 @@ ...@@ -9665,7 +9665,7 @@
"description": "The global id of the discussion this note is in reply to", "description": "The global id of the discussion this note is in reply to",
"type": { "type": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "DiscussionID",
"ofType": null "ofType": null
}, },
"defaultValue": null "defaultValue": null
...@@ -16061,6 +16061,16 @@ ...@@ -16061,6 +16061,16 @@
"enumValues": null, "enumValues": null,
"possibleTypes": null "possibleTypes": null
}, },
{
"kind": "SCALAR",
"name": "DiscussionID",
"description": "Identifier of Discussion",
"fields": null,
"inputFields": null,
"interfaces": null,
"enumValues": null,
"possibleTypes": null
},
{ {
"kind": "INPUT_OBJECT", "kind": "INPUT_OBJECT",
"name": "DiscussionToggleResolveInput", "name": "DiscussionToggleResolveInput",
...@@ -56847,7 +56857,7 @@ ...@@ -56847,7 +56857,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "NoteID",
"ofType": null "ofType": null
} }
}, },
...@@ -57357,7 +57367,7 @@ ...@@ -57357,7 +57367,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "NoteID",
"ofType": null "ofType": null
} }
}, },
...@@ -60,6 +60,14 @@ RSpec.describe 'Adding a Note' do ...@@ -60,6 +60,14 @@ RSpec.describe 'Adding a Note' do
expect(mutation_response['note']['discussion']['id']).to eq(discussion.to_global_id.to_s) expect(mutation_response['note']['discussion']['id']).to eq(discussion.to_global_id.to_s)
end 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 end
end end
......
...@@ -178,6 +178,12 @@ RSpec.describe 'Updating an image DiffNote' do ...@@ -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'] it_behaves_like 'a mutation that returns top-level errors', errors: ['body or position arguments are required']
end 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 context 'when resource is not a DiffNote on an image' do
let!(:diff_note) { create(:diff_note_on_merge_request, note: original_body) } let!(:diff_note) { create(:diff_note_on_merge_request, note: original_body) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment