Commit a3872144 authored by Brett Walker's avatar Brett Walker Committed by Mark Chao

Enforce gid type for todo mutations

and slight refactor of restore_many to
use GitlabSchema.parse_gid
parent 90dd0053
...@@ -48,6 +48,10 @@ class GraphqlController < ApplicationController ...@@ -48,6 +48,10 @@ class GraphqlController < ApplicationController
render_error(exception.message, status: :unprocessable_entity) render_error(exception.message, status: :unprocessable_entity)
end end
rescue_from ::GraphQL::CoercionError do |exception|
render_error(exception.message, status: :unprocessable_entity)
end
private private
def set_user_last_activity def set_user_last_activity
......
...@@ -6,7 +6,7 @@ module Mutations ...@@ -6,7 +6,7 @@ module Mutations
authorize :award_emoji authorize :award_emoji
argument :awardable_id, argument :awardable_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Awardable],
required: true, required: true,
description: 'The global id of the awardable resource' description: 'The global id of the awardable resource'
...@@ -23,7 +23,10 @@ module Mutations ...@@ -23,7 +23,10 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Awardable].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end end
# Called by mutations methods after performing an authorization check # Called by mutations methods after performing an authorization check
......
...@@ -38,7 +38,7 @@ module Mutations ...@@ -38,7 +38,7 @@ module Mutations
# TODO: remove this line when the compatibility layer is removed # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883 # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = DesignID.coerce_isolated_input(id) id = DesignID.coerce_isolated_input(id)
GitlabSchema.object_from_id(id) GitlabSchema.find_by_gid(id)
end end
def not_found(gid) def not_found(gid)
......
...@@ -18,12 +18,12 @@ module Mutations ...@@ -18,12 +18,12 @@ module Mutations
description: 'The created annotation' description: 'The created annotation'
argument :environment_id, argument :environment_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Environment],
required: false, required: false,
description: 'The global id of the environment to add an annotation to' description: 'The global id of the environment to add an annotation to'
argument :cluster_id, argument :cluster_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Clusters::Cluster],
required: false, required: false,
description: 'The global id of the cluster to add an annotation to' description: 'The global id of the cluster to add an annotation to'
...@@ -84,7 +84,7 @@ module Mutations ...@@ -84,7 +84,7 @@ module Mutations
end end
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) GitlabSchema.find_by_gid(id)
end end
def annotation_create_params(args) def annotation_create_params(args)
...@@ -96,7 +96,16 @@ module Mutations ...@@ -96,7 +96,16 @@ module Mutations
end end
def annotation_source(args) def annotation_source(args)
annotation_source_id = args[:cluster_id] || args[:environment_id] # TODO: remove these lines when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
annotation_source_id = if args[:cluster_id]
::Types::GlobalIDType[::Clusters::Cluster].coerce_isolated_input(args[:cluster_id])
else
::Types::GlobalIDType[::Environment].coerce_isolated_input(args[:environment_id])
end
# TODO: uncomment following line once lines above are removed
# annotation_source_id = args[:cluster_id] || args[:environment_id]
authorized_find!(id: annotation_source_id) authorized_find!(id: annotation_source_id)
end end
end end
......
...@@ -11,7 +11,10 @@ module Mutations ...@@ -11,7 +11,10 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) # TODO: remove explicit coercion once compatibility layer has been removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Note].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end end
def check_object_is_noteable!(object) def check_object_is_noteable!(object)
......
...@@ -9,7 +9,7 @@ module Mutations ...@@ -9,7 +9,7 @@ module Mutations
authorize :create_note authorize :create_note
argument :noteable_id, argument :noteable_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Noteable],
required: true, required: true,
description: 'The global id of the resource to add a note to' description: 'The global id of the resource to add a note to'
...@@ -42,6 +42,13 @@ module Mutations ...@@ -42,6 +42,13 @@ module Mutations
private private
def find_object(id:)
# TODO: remove explicit coercion once compatibility layer has been removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Noteable].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end
def create_note_params(noteable, args) def create_note_params(noteable, args)
{ {
noteable: noteable, noteable: noteable,
......
...@@ -8,9 +8,9 @@ module Mutations ...@@ -8,9 +8,9 @@ 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 destroy' description: 'The global id of the note to destroy'
def resolve(id:) def resolve(id:)
note = authorized_find!(id: id) note = authorized_find!(id: id)
......
...@@ -6,7 +6,10 @@ module Mutations ...@@ -6,7 +6,10 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Todo].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end end
def map_to_global_ids(ids) def map_to_global_ids(ids)
...@@ -16,7 +19,7 @@ module Mutations ...@@ -16,7 +19,7 @@ module Mutations
end end
def to_global_id(id) def to_global_id(id)
::URI::GID.build(app: GlobalID.app, model_name: Todo.name, model_id: id, params: nil).to_s Gitlab::GlobalId.as_global_id(id, model_name: Todo.name).to_s
end end
end end
end end
......
...@@ -8,7 +8,7 @@ module Mutations ...@@ -8,7 +8,7 @@ module Mutations
MAX_UPDATE_AMOUNT = 50 MAX_UPDATE_AMOUNT = 50
argument :ids, argument :ids,
[GraphQL::ID_TYPE], [::Types::GlobalIDType[::Todo]],
required: true, required: true,
description: 'The global ids of the todos to restore (a maximum of 50 is supported at once)' description: 'The global ids of the todos to restore (a maximum of 50 is supported at once)'
...@@ -37,24 +37,18 @@ module Mutations ...@@ -37,24 +37,18 @@ module Mutations
private private
def gids_of(ids) def gids_of(ids)
ids.map { |id| ::URI::GID.build(app: GlobalID.app, model_name: Todo.name, model_id: id, params: nil).to_s } ids.map { |id| Gitlab::GlobalId.as_global_id(id, model_name: Todo.name).to_s }
end end
def model_ids_of(ids) def model_ids_of(ids)
ids.map do |gid| ids.map do |gid|
parsed_gid = ::URI::GID.parse(gid) # TODO: remove this line when the compatibility layer is removed
parsed_gid.model_id.to_i if accessible_todo?(parsed_gid) # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
gid = ::Types::GlobalIDType[::Todo].coerce_isolated_input(gid)
gid.model_id.to_i
end.compact end.compact
end end
def accessible_todo?(gid)
gid.app == GlobalID.app && todo?(gid)
end
def todo?(gid)
GlobalID.parse(gid)&.model_class&.ancestors&.include?(Todo)
end
def raise_too_many_todos_requested_error def raise_too_many_todos_requested_error
raise Gitlab::Graphql::Errors::ArgumentError, 'Too many todos requested.' raise Gitlab::Graphql::Errors::ArgumentError, 'Too many todos requested.'
end end
......
---
title: Updated GraphQL mutation input ids to be more type specific
merge_request: 44073
author:
type: changed
...@@ -32,7 +32,7 @@ input AddAwardEmojiInput { ...@@ -32,7 +32,7 @@ input AddAwardEmojiInput {
""" """
The global id of the awardable resource The global id of the awardable resource
""" """
awardableId: ID! awardableId: AwardableID!
""" """
A unique identifier for the client performing the mutation. A unique identifier for the client performing the mutation.
...@@ -77,7 +77,7 @@ input AddProjectToSecurityDashboardInput { ...@@ -77,7 +77,7 @@ input AddProjectToSecurityDashboardInput {
""" """
ID of the project to be added to Instance Security Dashboard ID of the project to be added to Instance Security Dashboard
""" """
id: ID! id: ProjectID!
} }
""" """
...@@ -792,7 +792,7 @@ input AwardEmojiAddInput { ...@@ -792,7 +792,7 @@ input AwardEmojiAddInput {
""" """
The global id of the awardable resource The global id of the awardable resource
""" """
awardableId: ID! awardableId: AwardableID!
""" """
A unique identifier for the client performing the mutation. A unique identifier for the client performing the mutation.
...@@ -832,7 +832,7 @@ input AwardEmojiRemoveInput { ...@@ -832,7 +832,7 @@ input AwardEmojiRemoveInput {
""" """
The global id of the awardable resource The global id of the awardable resource
""" """
awardableId: ID! awardableId: AwardableID!
""" """
A unique identifier for the client performing the mutation. A unique identifier for the client performing the mutation.
...@@ -872,7 +872,7 @@ input AwardEmojiToggleInput { ...@@ -872,7 +872,7 @@ input AwardEmojiToggleInput {
""" """
The global id of the awardable resource The global id of the awardable resource
""" """
awardableId: ID! awardableId: AwardableID!
""" """
A unique identifier for the client performing the mutation. A unique identifier for the client performing the mutation.
...@@ -910,6 +910,11 @@ type AwardEmojiTogglePayload { ...@@ -910,6 +910,11 @@ type AwardEmojiTogglePayload {
toggledOn: Boolean! toggledOn: Boolean!
} }
"""
Identifier of Awardable
"""
scalar AwardableID
type BaseService implements Service { type BaseService implements Service {
""" """
Indicates if the service is active Indicates if the service is active
...@@ -2459,6 +2464,11 @@ Identifier of Clusters::AgentToken ...@@ -2459,6 +2464,11 @@ Identifier of Clusters::AgentToken
""" """
scalar ClustersAgentTokenID scalar ClustersAgentTokenID
"""
Identifier of Clusters::Cluster
"""
scalar ClustersClusterID
type Commit { type Commit {
""" """
Author of the commit Author of the commit
...@@ -3003,7 +3013,7 @@ input CreateAnnotationInput { ...@@ -3003,7 +3013,7 @@ input CreateAnnotationInput {
""" """
The global id of the cluster to add an annotation to The global id of the cluster to add an annotation to
""" """
clusterId: ID clusterId: ClustersClusterID
""" """
The path to a file defining the dashboard on which the annotation should be added The path to a file defining the dashboard on which the annotation should be added
...@@ -3023,7 +3033,7 @@ input CreateAnnotationInput { ...@@ -3023,7 +3033,7 @@ input CreateAnnotationInput {
""" """
The global id of the environment to add an annotation to The global id of the environment to add an annotation to
""" """
environmentId: ID environmentId: EnvironmentID
""" """
Timestamp indicating starting moment to which the annotation relates Timestamp indicating starting moment to which the annotation relates
...@@ -3158,7 +3168,7 @@ input CreateDiffNoteInput { ...@@ -3158,7 +3168,7 @@ input CreateDiffNoteInput {
""" """
The global id of the resource to add a note to The global id of the resource to add a note to
""" """
noteableId: ID! noteableId: NoteableID!
""" """
The position of this note on a diff The position of this note on a diff
...@@ -3288,7 +3298,7 @@ input CreateImageDiffNoteInput { ...@@ -3288,7 +3298,7 @@ input CreateImageDiffNoteInput {
""" """
The global id of the resource to add a note to The global id of the resource to add a note to
""" """
noteableId: ID! noteableId: NoteableID!
""" """
The position of this note on a diff The position of this note on a diff
...@@ -3403,7 +3413,7 @@ input CreateNoteInput { ...@@ -3403,7 +3413,7 @@ input CreateNoteInput {
""" """
The global id of the resource to add a note to The global id of the resource to add a note to
""" """
noteableId: ID! noteableId: NoteableID!
} }
""" """
...@@ -5150,7 +5160,7 @@ input DestroyNoteInput { ...@@ -5150,7 +5160,7 @@ input DestroyNoteInput {
""" """
The global id of the note to destroy The global id of the note to destroy
""" """
id: ID! id: NoteID!
} }
""" """
...@@ -5624,7 +5634,7 @@ input DismissVulnerabilityInput { ...@@ -5624,7 +5634,7 @@ input DismissVulnerabilityInput {
""" """
ID of the vulnerability to be dismissed ID of the vulnerability to be dismissed
""" """
id: ID! id: VulnerabilityID!
} }
""" """
...@@ -5758,6 +5768,11 @@ type EnvironmentEdge { ...@@ -5758,6 +5768,11 @@ type EnvironmentEdge {
node: Environment node: Environment
} }
"""
Identifier of Environment
"""
scalar EnvironmentID
""" """
Represents an epic Represents an epic
""" """
...@@ -6886,17 +6901,17 @@ input EpicTreeNodeFieldsInputType { ...@@ -6886,17 +6901,17 @@ input EpicTreeNodeFieldsInputType {
""" """
The id of the epic_issue or issue that the actual epic or issue is switched with The id of the epic_issue or issue that the actual epic or issue is switched with
""" """
adjacentReferenceId: ID adjacentReferenceId: EpicTreeSortingID
""" """
The id of the epic_issue or epic that is being moved The id of the epic_issue or epic that is being moved
""" """
id: ID! id: EpicTreeSortingID!
""" """
ID of the new parent epic ID of the new parent epic
""" """
newParentId: ID newParentId: EpicID
""" """
The type of the switch, after or before allowed The type of the switch, after or before allowed
...@@ -6911,7 +6926,7 @@ input EpicTreeReorderInput { ...@@ -6911,7 +6926,7 @@ input EpicTreeReorderInput {
""" """
The id of the base epic of the tree The id of the base epic of the tree
""" """
baseEpicId: ID! baseEpicId: EpicID!
""" """
A unique identifier for the client performing the mutation. A unique identifier for the client performing the mutation.
...@@ -6939,6 +6954,11 @@ type EpicTreeReorderPayload { ...@@ -6939,6 +6954,11 @@ type EpicTreeReorderPayload {
errors: [String!]! errors: [String!]!
} }
"""
Identifier of EpicTreeSorting
"""
scalar EpicTreeSortingID
""" """
Epic ID wildcard values Epic ID wildcard values
""" """
...@@ -12019,6 +12039,11 @@ type NamespaceEdge { ...@@ -12019,6 +12039,11 @@ type NamespaceEdge {
node: Namespace node: Namespace
} }
"""
Identifier of Namespace
"""
scalar NamespaceID
""" """
Autogenerated input type of NamespaceIncreaseStorageTemporarily Autogenerated input type of NamespaceIncreaseStorageTemporarily
""" """
...@@ -12031,7 +12056,7 @@ input NamespaceIncreaseStorageTemporarilyInput { ...@@ -12031,7 +12056,7 @@ input NamespaceIncreaseStorageTemporarilyInput {
""" """
The global id of the namespace to mutate The global id of the namespace to mutate
""" """
id: ID! id: NamespaceID!
} }
""" """
...@@ -12228,6 +12253,11 @@ type NoteEdge { ...@@ -12228,6 +12253,11 @@ type NoteEdge {
node: Note node: Note
} }
"""
Identifier of Note
"""
scalar NoteID
type NotePermissions { type NotePermissions {
""" """
Indicates the user can perform `admin_note` on this resource Indicates the user can perform `admin_note` on this resource
...@@ -12307,6 +12337,11 @@ interface Noteable { ...@@ -12307,6 +12337,11 @@ interface Noteable {
): NoteConnection! ): NoteConnection!
} }
"""
Identifier of Noteable
"""
scalar NoteableID
""" """
Represents a package Represents a package
""" """
...@@ -14501,6 +14536,11 @@ type ProjectEdge { ...@@ -14501,6 +14536,11 @@ type ProjectEdge {
node: Project node: Project
} }
"""
Identifier of Project
"""
scalar ProjectID
""" """
Represents a Project Membership Represents a Project Membership
""" """
...@@ -15779,7 +15819,7 @@ input RemoveAwardEmojiInput { ...@@ -15779,7 +15819,7 @@ input RemoveAwardEmojiInput {
""" """
The global id of the awardable resource The global id of the awardable resource
""" """
awardableId: ID! awardableId: AwardableID!
""" """
A unique identifier for the client performing the mutation. A unique identifier for the client performing the mutation.
...@@ -18260,6 +18300,11 @@ type TodoEdge { ...@@ -18260,6 +18300,11 @@ type TodoEdge {
node: Todo node: Todo
} }
"""
Identifier of Todo
"""
scalar TodoID
""" """
Autogenerated input type of TodoMarkDone Autogenerated input type of TodoMarkDone
""" """
...@@ -18322,7 +18367,7 @@ input TodoRestoreManyInput { ...@@ -18322,7 +18367,7 @@ input TodoRestoreManyInput {
""" """
The global ids of the todos to restore (a maximum of 50 is supported at once) The global ids of the todos to restore (a maximum of 50 is supported at once)
""" """
ids: [ID!]! ids: [TodoID!]!
} }
""" """
...@@ -18449,7 +18494,7 @@ input ToggleAwardEmojiInput { ...@@ -18449,7 +18494,7 @@ input ToggleAwardEmojiInput {
""" """
The global id of the awardable resource The global id of the awardable resource
""" """
awardableId: ID! awardableId: AwardableID!
""" """
A unique identifier for the client performing the mutation. A unique identifier for the client performing the mutation.
...@@ -18766,7 +18811,7 @@ input UpdateBoardInput { ...@@ -18766,7 +18811,7 @@ input UpdateBoardInput {
""" """
The id of user to be assigned to the board. The id of user to be assigned to the board.
""" """
assigneeId: ID assigneeId: UserID
""" """
A unique identifier for the client performing the mutation. A unique identifier for the client performing the mutation.
...@@ -18786,7 +18831,7 @@ input UpdateBoardInput { ...@@ -18786,7 +18831,7 @@ input UpdateBoardInput {
""" """
The board global id. The board global id.
""" """
id: ID! id: BoardID!
""" """
The IDs of labels to be added to the board. The IDs of labels to be added to the board.
...@@ -18801,7 +18846,7 @@ input UpdateBoardInput { ...@@ -18801,7 +18846,7 @@ input UpdateBoardInput {
""" """
The id of milestone to be assigned to the board. The id of milestone to be assigned to the board.
""" """
milestoneId: ID milestoneId: MilestoneID
""" """
Name of the board Name of the board
...@@ -20290,7 +20335,7 @@ input VulnerabilityDismissInput { ...@@ -20290,7 +20335,7 @@ input VulnerabilityDismissInput {
""" """
ID of the vulnerability to be dismissed ID of the vulnerability to be dismissed
""" """
id: ID! id: VulnerabilityID!
} }
""" """
......
...@@ -6,7 +6,7 @@ module Mutations ...@@ -6,7 +6,7 @@ module Mutations
graphql_name 'UpdateBoard' graphql_name 'UpdateBoard'
argument :id, argument :id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Board],
required: true, required: true,
description: 'The board global id.' description: 'The board global id.'
...@@ -26,13 +26,13 @@ module Mutations ...@@ -26,13 +26,13 @@ module Mutations
description: copy_field_description(Types::BoardType, :hide_closed_list) description: copy_field_description(Types::BoardType, :hide_closed_list)
argument :assignee_id, argument :assignee_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::User],
required: false, required: false,
loads: ::Types::UserType, loads: ::Types::UserType,
description: 'The id of user to be assigned to the board.' description: 'The id of user to be assigned to the board.'
argument :milestone_id, argument :milestone_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Milestone],
required: false, required: false,
description: 'The id of milestone to be assigned to the board.' description: 'The id of milestone to be assigned to the board.'
...@@ -81,16 +81,25 @@ module Mutations ...@@ -81,16 +81,25 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Board].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end end
def parse_arguments(args = {}) def parse_arguments(args = {})
if args[:assignee_id] if args[:assignee_id]
args[:assignee_id] = GitlabSchema.parse_gid(args[:assignee_id], expected_type: ::User).model_id # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
args[:assignee_id] = ::Types::GlobalIDType[::User].coerce_isolated_input(args[:assignee_id])
args[:assignee_id] = args[:assignee_id].model_id
end end
if args[:milestone_id] if args[:milestone_id]
args[:milestone_id] = GitlabSchema.parse_gid(args[:milestone_id], expected_type: ::Milestone).model_id # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
args[:milestone_id] = ::Types::GlobalIDType[::Milestone].coerce_isolated_input(args[:milestone_id])
args[:milestone_id] = args[:milestone_id].model_id
end end
args[:label_ids] &&= args[:label_ids].map do |label_id| args[:label_ids] &&= args[:label_ids].map do |label_id|
......
...@@ -8,7 +8,7 @@ module Mutations ...@@ -8,7 +8,7 @@ module Mutations
authorize :admin_epic authorize :admin_epic
argument :base_epic_id, argument :base_epic_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Epic],
required: true, required: true,
description: 'The id of the base epic of the tree' description: 'The id of the base epic of the tree'
...@@ -18,14 +18,32 @@ module Mutations ...@@ -18,14 +18,32 @@ module Mutations
description: 'Parameters for updating the tree positions' description: 'Parameters for updating the tree positions'
def resolve(args) def resolve(args)
params = args[:moved] moving_object_id = args[:moved][:id]
moving_params = params.to_hash.slice(:adjacent_reference_id, :relative_position, :new_parent_id).merge(base_epic_id: args[:base_epic_id]) moving_params = args[:moved].to_hash.slice(:adjacent_reference_id, :relative_position, :new_parent_id).merge(base_epic_id: args[:base_epic_id])
moving_object_id, moving_params = coerce_input(moving_object_id, moving_params)
result = ::Epics::TreeReorderService.new(current_user, params[:id], moving_params).execute result = ::Epics::TreeReorderService.new(current_user, moving_object_id, moving_params).execute
errors = result[:status] == :error ? [result[:message]] : [] errors = result[:status] == :error ? [result[:message]] : []
{ errors: errors } { errors: errors }
end end
# TODO: remove explicit coercion once compatibility layer has been removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
def coerce_input(moving_object_id, moving_params)
moving_object_id = ::Types::GlobalIDType[::EpicTreeSorting].coerce_isolated_input(moving_object_id)
moving_params[:base_epic_id] = ::Types::GlobalIDType[::Epic].coerce_isolated_input(moving_params[:base_epic_id])
if moving_params[:adjacent_reference_id]
moving_params[:adjacent_reference_id] = ::Types::GlobalIDType[::EpicTreeSorting].coerce_isolated_input(moving_params[:adjacent_reference_id])
end
if moving_params[:new_parent_id]
moving_params[:new_parent_id] = ::Types::GlobalIDType[::Epic].coerce_isolated_input(moving_params[:new_parent_id])
end
[moving_object_id, moving_params]
end
end end
end end
end end
...@@ -11,7 +11,7 @@ module Mutations ...@@ -11,7 +11,7 @@ module Mutations
null: true, null: true,
description: 'Project that was added to the Instance Security Dashboard' description: 'Project that was added to the Instance Security Dashboard'
argument :id, GraphQL::ID_TYPE, argument :id, ::Types::GlobalIDType[::Project],
required: true, required: true,
description: 'ID of the project to be added to Instance Security Dashboard' description: 'ID of the project to be added to Instance Security Dashboard'
...@@ -29,7 +29,10 @@ module Mutations ...@@ -29,7 +29,10 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) # TODO: remove explicit coercion once compatibility layer has been removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Project].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end end
def add_project(project) def add_project(project)
......
...@@ -3,9 +3,9 @@ ...@@ -3,9 +3,9 @@
module Mutations module Mutations
module Namespaces module Namespaces
class Base < ::Mutations::BaseMutation class Base < ::Mutations::BaseMutation
argument :id, GraphQL::ID_TYPE, argument :id, ::Types::GlobalIDType[::Namespace],
required: true, required: true,
description: "The global id of the namespace to mutate" description: 'The global id of the namespace to mutate'
field :namespace, field :namespace,
Types::NamespaceType, Types::NamespaceType,
...@@ -15,7 +15,10 @@ module Mutations ...@@ -15,7 +15,10 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = Types::GlobalIDType[::Namespace].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end end
end end
end end
......
...@@ -12,7 +12,7 @@ module Mutations ...@@ -12,7 +12,7 @@ module Mutations
description: 'The vulnerability after dismissal' description: 'The vulnerability after dismissal'
argument :id, argument :id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Vulnerability],
required: true, required: true,
description: 'ID of the vulnerability to be dismissed' description: 'ID of the vulnerability to be dismissed'
...@@ -38,7 +38,10 @@ module Mutations ...@@ -38,7 +38,10 @@ module Mutations
end end
def find_object(id:) def find_object(id:)
GitlabSchema.object_from_id(id) # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Vulnerability].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id)
end end
end end
end end
......
...@@ -8,12 +8,12 @@ module Types ...@@ -8,12 +8,12 @@ module Types
description 'A node of an epic tree.' description 'A node of an epic tree.'
argument :id, argument :id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::EpicTreeSorting],
required: true, required: true,
description: 'The id of the epic_issue or epic that is being moved' description: 'The id of the epic_issue or epic that is being moved'
argument :adjacent_reference_id, argument :adjacent_reference_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::EpicTreeSorting],
required: false, required: false,
description: 'The id of the epic_issue or issue that the actual epic or issue is switched with' description: 'The id of the epic_issue or issue that the actual epic or issue is switched with'
...@@ -23,7 +23,7 @@ module Types ...@@ -23,7 +23,7 @@ module Types
description: 'The type of the switch, after or before allowed' description: 'The type of the switch, after or before allowed'
argument :new_parent_id, argument :new_parent_id,
GraphQL::ID_TYPE, ::Types::GlobalIDType[::Epic],
required: false, required: false,
description: 'ID of the new parent epic' description: 'ID of the new parent epic'
end end
......
...@@ -154,7 +154,7 @@ module Epics ...@@ -154,7 +154,7 @@ module Epics
end end
def find_object(id) def find_object(id)
GitlabSchema.object_from_id(id) GitlabSchema.find_by_gid(id)
end end
end end
end end
...@@ -38,6 +38,14 @@ RSpec.describe Mutations::Boards::Update do ...@@ -38,6 +38,14 @@ RSpec.describe Mutations::Boards::Update do
end end
end end
context 'with invalid params' do
it 'raises an error' do
mutation_params[:id] = project.to_global_id
expect { subject }.to raise_error(::GraphQL::CoercionError)
end
end
context 'when user can update board' do context 'when user can update board' do
before do before do
board.resource_parent.add_reporter(user) board.resource_parent.add_reporter(user)
......
...@@ -81,6 +81,14 @@ RSpec.describe Mutations::InstanceSecurityDashboard::AddProject do ...@@ -81,6 +81,14 @@ RSpec.describe Mutations::InstanceSecurityDashboard::AddProject do
expect(user.security_dashboard_projects).to include(already_added_project) expect(user.security_dashboard_projects).to include(already_added_project)
end end
end end
context 'with invalid params' do
let(:selected_project) { user }
it 'raises an error' do
expect { subject }.to raise_error(::GraphQL::CoercionError)
end
end
end end
end end
end end
......
...@@ -9,9 +9,7 @@ RSpec.describe Mutations::Namespaces::IncreaseStorageTemporarily do ...@@ -9,9 +9,7 @@ RSpec.describe Mutations::Namespaces::IncreaseStorageTemporarily do
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
describe '#resolve' do describe '#resolve' do
subject do subject { mutation.resolve(id: namespace.to_global_id.to_s) }
mutation.resolve(id: namespace.to_global_id.to_s)
end
before do before do
allow_next_instance_of(EE::Namespace::RootStorageSize, namespace) do |root_storage| allow_next_instance_of(EE::Namespace::RootStorageSize, namespace) do |root_storage|
...@@ -38,5 +36,13 @@ RSpec.describe Mutations::Namespaces::IncreaseStorageTemporarily do ...@@ -38,5 +36,13 @@ RSpec.describe Mutations::Namespaces::IncreaseStorageTemporarily do
expect(namespace.reload.temporary_storage_increase_ends_on).to be_present expect(namespace.reload.temporary_storage_increase_ends_on).to be_present
end end
end end
context 'with invalid params' do
let_it_be(:namespace) { user }
it 'raises an error' do
expect { subject }.to raise_error(::GraphQL::CoercionError)
end
end
end end
end end
...@@ -2,16 +2,19 @@ ...@@ -2,16 +2,19 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Mutations::Vulnerabilities::Dismiss do RSpec.describe Mutations::Vulnerabilities::Dismiss do
include GraphqlHelpers
let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) } let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
describe '#resolve' do describe '#resolve' do
let_it_be(:vulnerability) { create(:vulnerability, :with_findings) } let_it_be(:vulnerability) { create(:vulnerability, :with_findings) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:vulnerability_id) { GitlabSchema.id_from_object(vulnerability).to_s }
let(:comment) { 'Dismissal Feedbacl' } let(:comment) { 'Dismissal Feedbacl' }
let(:mutated_vulnerability) { subject[:vulnerability] } let(:mutated_vulnerability) { subject[:vulnerability] }
subject { mutation.resolve(id: GitlabSchema.id_from_object(vulnerability).to_s, comment: comment) } subject { mutation.resolve(id: vulnerability_id, comment: comment) }
context 'when the user can dismiss the vulnerability' do context 'when the user can dismiss the vulnerability' do
before do before do
...@@ -24,6 +27,14 @@ RSpec.describe Mutations::Vulnerabilities::Dismiss do ...@@ -24,6 +27,14 @@ RSpec.describe Mutations::Vulnerabilities::Dismiss do
end end
end end
context 'with invalid params' do
let(:vulnerability_id) { global_id_of(user) }
it 'raises an error' do
expect { subject }.to raise_error(::GraphQL::CoercionError)
end
end
context 'when user has access to the project' do context 'when user has access to the project' do
before do before do
vulnerability.project.add_developer(user) vulnerability.project.add_developer(user)
......
...@@ -126,7 +126,8 @@ RSpec.describe 'Updating an epic tree' do ...@@ -126,7 +126,8 @@ RSpec.describe 'Updating an epic tree' do
it 'returns the error message' do it 'returns the error message' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['errors']).to eq(['Only epics and epic_issues are supported.']) expect(graphql_errors.first['message']).to include("\"#{variables[:moved][:id]}\" does not represent an instance of EpicTreeSorting")
expect(graphql_errors.first['message']).to include("\"#{variables[:moved][:adjacent_reference_id]}\" does not represent an instance of EpicTreeSorting")
end end
end end
......
...@@ -52,7 +52,8 @@ RSpec.describe Mutations::Todos::MarkDone do ...@@ -52,7 +52,8 @@ RSpec.describe Mutations::Todos::MarkDone do
end end
it 'ignores invalid GIDs' do it 'ignores invalid GIDs' do
expect { mutation.resolve(id: 'invalid_gid') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) expect { mutation.resolve(id: author.to_global_id.to_s) }
.to raise_error(::GraphQL::CoercionError)
expect(todo1.reload.state).to eq('pending') expect(todo1.reload.state).to eq('pending')
expect(todo2.reload.state).to eq('done') expect(todo2.reload.state).to eq('done')
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Mutations::Todos::RestoreMany do RSpec.describe Mutations::Todos::RestoreMany do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:author) { create(:user) } let_it_be(:author) { create(:user) }
let_it_be(:other_user) { create(:user) } let_it_be(:other_user) { create(:user) }
...@@ -44,8 +46,9 @@ RSpec.describe Mutations::Todos::RestoreMany do ...@@ -44,8 +46,9 @@ RSpec.describe Mutations::Todos::RestoreMany do
expect_states_were_not_changed expect_states_were_not_changed
end end
it 'ignores invalid GIDs' do it 'raises an error with invalid or non-Todo GIDs' do
expect { mutation.resolve(ids: ['invalid_gid']) }.to raise_error(URI::BadURIError) expect { mutation.resolve(ids: [author.to_global_id.to_s]) }
.to raise_error(GraphQL::CoercionError)
expect_states_were_not_changed expect_states_were_not_changed
end end
...@@ -78,38 +81,12 @@ RSpec.describe Mutations::Todos::RestoreMany do ...@@ -78,38 +81,12 @@ RSpec.describe Mutations::Todos::RestoreMany do
it 'fails if too many todos are requested for update' do it 'fails if too many todos are requested for update' do
expect { restore_mutation([todo1] * 51) }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) expect { restore_mutation([todo1] * 51) }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
end end
it 'does not update todos from another app' do
todo4 = create(:todo)
todo4_gid = ::URI::GID.parse("gid://otherapp/Todo/#{todo4.id}")
result = mutation.resolve(ids: [todo4_gid.to_s])
expect(result[:updated_ids]).to be_empty
expect_states_were_not_changed
end
it 'does not update todos from another model' do
todo4 = create(:todo)
todo4_gid = ::URI::GID.parse("gid://#{GlobalID.app}/Project/#{todo4.id}")
result = mutation.resolve(ids: [todo4_gid.to_s])
expect(result[:updated_ids]).to be_empty
expect_states_were_not_changed
end
end end
def restore_mutation(todos) def restore_mutation(todos)
mutation.resolve(ids: todos.map { |todo| global_id_of(todo) } ) mutation.resolve(ids: todos.map { |todo| global_id_of(todo) } )
end end
def global_id_of(todo)
todo.to_global_id.to_s
end
def expect_states_were_not_changed def expect_states_were_not_changed
expect(todo1.reload.state).to eq('done') expect(todo1.reload.state).to eq('done')
expect(todo2.reload.state).to eq('pending') expect(todo2.reload.state).to eq('pending')
......
...@@ -3,6 +3,8 @@ ...@@ -3,6 +3,8 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Mutations::Todos::Restore do RSpec.describe Mutations::Todos::Restore do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:author) { create(:user) } let_it_be(:author) { create(:user) }
let_it_be(:other_user) { create(:user) } let_it_be(:other_user) { create(:user) }
...@@ -49,8 +51,9 @@ RSpec.describe Mutations::Todos::Restore do ...@@ -49,8 +51,9 @@ RSpec.describe Mutations::Todos::Restore do
expect(other_user_todo.reload.state).to eq('done') expect(other_user_todo.reload.state).to eq('done')
end end
it 'ignores invalid GIDs' do it 'raises error for invalid GID' do
expect { mutation.resolve(id: 'invalid_gid') }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) expect { mutation.resolve(id: author.to_global_id.to_s) }
.to raise_error(::GraphQL::CoercionError)
expect(todo1.reload.state).to eq('done') expect(todo1.reload.state).to eq('done')
expect(todo2.reload.state).to eq('pending') expect(todo2.reload.state).to eq('pending')
...@@ -61,8 +64,4 @@ RSpec.describe Mutations::Todos::Restore do ...@@ -61,8 +64,4 @@ RSpec.describe Mutations::Todos::Restore do
def restore_mutation(todo) def restore_mutation(todo)
mutation.resolve(id: global_id_of(todo)) mutation.resolve(id: global_id_of(todo))
end end
def global_id_of(todo)
todo.to_global_id.to_s
end
end end
...@@ -45,8 +45,9 @@ RSpec.describe 'Adding an AwardEmoji' do ...@@ -45,8 +45,9 @@ RSpec.describe 'Adding an AwardEmoji' do
it_behaves_like 'a mutation that does not create an AwardEmoji' it_behaves_like 'a mutation that does not create an AwardEmoji'
it_behaves_like 'a mutation that returns top-level errors', it_behaves_like 'a mutation that returns top-level errors' do
errors: ['Cannot award emoji to this resource'] let(:match_errors) { include(/was provided invalid value for awardableId/) }
end
end end
context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do
......
...@@ -50,8 +50,9 @@ RSpec.describe 'Removing an AwardEmoji' do ...@@ -50,8 +50,9 @@ RSpec.describe 'Removing an AwardEmoji' do
it_behaves_like 'a mutation that does not destroy an AwardEmoji' it_behaves_like 'a mutation that does not destroy an AwardEmoji'
it_behaves_like 'a mutation that returns top-level errors', it_behaves_like 'a mutation that returns top-level errors' do
errors: ['Cannot award emoji to this resource'] let(:match_errors) { include(/was provided invalid value for awardableId/) }
end
end end
context 'when the given awardable is an Awardable' do context 'when the given awardable is an Awardable' do
......
...@@ -44,8 +44,9 @@ RSpec.describe 'Toggling an AwardEmoji' do ...@@ -44,8 +44,9 @@ RSpec.describe 'Toggling an AwardEmoji' do
it_behaves_like 'a mutation that does not create or destroy an AwardEmoji' it_behaves_like 'a mutation that does not create or destroy an AwardEmoji'
it_behaves_like 'a mutation that returns top-level errors', it_behaves_like 'a mutation that returns top-level errors' do
errors: ['Cannot award emoji to this resource'] let(:match_errors) { include(/was provided invalid value for awardableId/) }
end
end end
context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do context 'when the given awardable is an Awardable but still cannot be awarded an emoji' do
......
...@@ -101,7 +101,9 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do ...@@ -101,7 +101,9 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do
graphql_mutation(:create_annotation, variables) graphql_mutation(:create_annotation, variables)
end end
it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab ID.'] it_behaves_like 'a mutation that returns top-level errors' do
let(:match_errors) { include(/is not a valid Global ID/) }
end
end end
end end
end end
...@@ -109,7 +111,7 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do ...@@ -109,7 +111,7 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do
context 'when annotation source is cluster' do context 'when annotation source is cluster' do
let(:mutation) do let(:mutation) do
variables = { variables = {
cluster_id: GitlabSchema.id_from_object(cluster).to_s, cluster_id: cluster.to_global_id.to_s,
starting_at: starting_at, starting_at: starting_at,
ending_at: ending_at, ending_at: ending_at,
dashboard_path: dashboard_path, dashboard_path: dashboard_path,
...@@ -188,15 +190,17 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do ...@@ -188,15 +190,17 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do
graphql_mutation(:create_annotation, variables) graphql_mutation(:create_annotation, variables)
end end
it_behaves_like 'a mutation that returns top-level errors', errors: ['invalid_id is not a valid GitLab ID.'] it_behaves_like 'a mutation that returns top-level errors' do
let(:match_errors) { include(/is not a valid Global ID/) }
end
end end
end end
context 'when both environment_id and cluster_id are provided' do context 'when both environment_id and cluster_id are provided' do
let(:mutation) do let(:mutation) do
variables = { variables = {
environment_id: GitlabSchema.id_from_object(environment).to_s, environment_id: environment.to_global_id.to_s,
cluster_id: GitlabSchema.id_from_object(cluster).to_s, cluster_id: cluster.to_global_id.to_s,
starting_at: starting_at, starting_at: starting_at,
ending_at: ending_at, ending_at: ending_at,
dashboard_path: dashboard_path, dashboard_path: dashboard_path,
...@@ -210,14 +214,14 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do ...@@ -210,14 +214,14 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do
end end
context 'when a non-cluster or environment id is provided' do context 'when a non-cluster or environment id is provided' do
let(:gid) { { environment_id: project.to_global_id.to_s } }
let(:mutation) do let(:mutation) do
variables = { variables = {
environment_id: GitlabSchema.id_from_object(project).to_s,
starting_at: starting_at, starting_at: starting_at,
ending_at: ending_at, ending_at: ending_at,
dashboard_path: dashboard_path, dashboard_path: dashboard_path,
description: description description: description
} }.merge!(gid)
graphql_mutation(:create_annotation, variables) graphql_mutation(:create_annotation, variables)
end end
...@@ -226,6 +230,18 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do ...@@ -226,6 +230,18 @@ RSpec.describe Mutations::Metrics::Dashboard::Annotations::Create do
project.add_developer(current_user) project.add_developer(current_user)
end end
it_behaves_like 'a mutation that returns top-level errors', errors: [described_class::INVALID_ANNOTATION_SOURCE_ERROR] describe 'non-environment id' do
it_behaves_like 'a mutation that returns top-level errors' do
let(:match_errors) { include(/does not represent an instance of Environment/) }
end
end
describe 'non-cluster id' do
let(:gid) { { cluster_id: project.to_global_id.to_s } }
it_behaves_like 'a mutation that returns top-level errors' do
let(:match_errors) { include(/does not represent an instance of Clusters::Cluster/) }
end
end
end end
end end
...@@ -76,8 +76,8 @@ RSpec.describe 'Marking todos done' do ...@@ -76,8 +76,8 @@ RSpec.describe 'Marking todos done' do
end end
context 'when using an invalid gid' do context 'when using an invalid gid' do
let(:input) { { id: 'invalid_gid' } } let(:input) { { id: GitlabSchema.id_from_object(author).to_s } }
let(:invalid_gid_error) { 'invalid_gid is not a valid GitLab ID.' } let(:invalid_gid_error) { "\"#{input[:id]}\" does not represent an instance of #{todo1.class}" }
it 'contains the expected error' do it 'contains the expected error' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
......
...@@ -76,8 +76,8 @@ RSpec.describe 'Restoring Todos' do ...@@ -76,8 +76,8 @@ RSpec.describe 'Restoring Todos' do
end end
context 'when using an invalid gid' do context 'when using an invalid gid' do
let(:input) { { id: 'invalid_gid' } } let(:input) { { id: GitlabSchema.id_from_object(author).to_s } }
let(:invalid_gid_error) { 'invalid_gid is not a valid GitLab ID.' } let(:invalid_gid_error) { "\"#{input[:id]}\" does not represent an instance of #{todo1.class}" }
it 'contains the expected error' do it 'contains the expected error' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
......
...@@ -52,11 +52,15 @@ RSpec.shared_examples 'a Note mutation when the given resource id is not for a N ...@@ -52,11 +52,15 @@ RSpec.shared_examples 'a Note mutation when the given resource id is not for a N
it_behaves_like 'a Note mutation that does not create a Note' it_behaves_like 'a Note mutation that does not create a Note'
it_behaves_like 'a mutation that returns top-level errors', errors: ['Cannot add notes to this resource'] it_behaves_like 'a mutation that returns top-level errors' do
let(:match_errors) { include(/ does not represent an instance of Noteable/) }
end
end end
RSpec.shared_examples 'a Note mutation when the given resource id is not for a Note' do RSpec.shared_examples 'a Note mutation when the given resource id is not for a Note' do
let(:note) { create(:issue) } let(:note) { create(:issue) }
it_behaves_like 'a mutation that returns top-level errors', errors: ['Resource is not a note'] it_behaves_like 'a mutation that returns top-level errors' do
let(:match_errors) { include(/does not represent an instance of Note/) }
end
end end
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