Commit 8c66149b authored by Mark Chao's avatar Mark Chao

Merge branch 'bw_update_global_id_checks' into 'master'

Use stricter typed Global IDs

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