Commit a00a23ca authored by Luke Duncalfe's avatar Luke Duncalfe

GraphQL mutations for managing Notes

https://gitlab.com/gitlab-org/gitlab-ce/issues/62826
parent 42e876b7
# frozen_string_literal: true
module Mutations
module Notes
class Base < BaseMutation
include Gitlab::Graphql::Authorize::AuthorizeResource
field :note,
Types::Notes::NoteType,
null: true,
description: 'The note after mutation'
private
def find_object(id:)
GitlabSchema.object_from_id(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
# frozen_string_literal: true
module Mutations
module Notes
module Create
# This is a Base class for the Note creation Mutations and is not
# mounted as a GraphQL mutation itself.
class Base < Mutations::Notes::Base
authorize :create_note
argument :noteable_id,
GraphQL::ID_TYPE,
required: true,
description: 'The global id of the resource to add a note to'
argument :body,
GraphQL::STRING_TYPE,
required: true,
description: copy_field_description(Types::Notes::NoteType, :body)
private
def resolve(args)
noteable = authorized_find!(id: args[:noteable_id])
check_object_is_noteable!(noteable)
note = ::Notes::CreateService.new(
noteable.project,
current_user,
create_note_params(noteable, args)
).execute
{
note: (note if note.persisted?),
errors: errors_on_object(note)
}
end
def create_note_params(noteable, args)
{
noteable: noteable,
note: args[:body]
}
end
end
end
end
end
# frozen_string_literal: true
module Mutations
module Notes
module Create
class DiffNote < Base
graphql_name 'CreateDiffNote'
argument :position,
Types::Notes::DiffPositionInputType,
required: true,
description: copy_field_description(Types::Notes::NoteType, :position)
private
def create_note_params(noteable, args)
super(noteable, args).merge({
type: 'DiffNote',
position: position(noteable, args)
})
end
def position(noteable, args)
position = args[:position].to_h
position[:position_type] = 'text'
position.merge!(position[:paths].to_h)
Gitlab::Diff::Position.new(position)
end
end
end
end
end
# frozen_string_literal: true
module Mutations
module Notes
module Create
class ImageDiffNote < Base
graphql_name 'CreateImageDiffNote'
argument :position,
Types::Notes::DiffImagePositionInputType,
required: true,
description: copy_field_description(Types::Notes::NoteType, :position)
private
def create_note_params(noteable, args)
super(noteable, args).merge({
type: 'DiffNote',
position: position(noteable, args)
})
end
def position(noteable, args)
position = args[:position].to_h
position[:position_type] = 'image'
position.merge!(position[:paths].to_h)
Gitlab::Diff::Position.new(position)
end
end
end
end
end
# frozen_string_literal: true
module Mutations
module Notes
module Create
class Note < Base
graphql_name 'CreateNote'
end
end
end
end
# frozen_string_literal: true
module Mutations
module Notes
class Destroy < Base
graphql_name 'DestroyNote'
authorize :admin_note
argument :id,
GraphQL::ID_TYPE,
required: true,
description: 'The global id of the note to destroy'
def resolve(id:)
note = authorized_find!(id: id)
check_object_is_note!(note)
::Notes::DestroyService.new(note.project, current_user).execute(note)
{
errors: []
}
end
end
end
end
# frozen_string_literal: true
module Mutations
module Notes
class Update < Base
graphql_name 'UpdateNote'
authorize :admin_note
argument :id,
GraphQL::ID_TYPE,
required: true,
description: 'The global id of the note to update'
argument :body,
GraphQL::STRING_TYPE,
required: true,
description: copy_field_description(Types::Notes::NoteType, :body)
def resolve(args)
note = authorized_find!(id: args[:id])
check_object_is_note!(note)
note = ::Notes::UpdateService.new(
note.project,
current_user,
{ note: args[:body] }
).execute(note)
{
note: note.reset,
errors: errors_on_object(note)
}
end
end
end
end
......@@ -2,5 +2,6 @@
module Types
class BaseInputObject < GraphQL::Schema::InputObject
prepend Gitlab::Graphql::CopyFieldDescription
end
end
# frozen_string_literal: true
module Types
# rubocop: disable Graphql/AuthorizeTypes
class DiffPathsInputType < BaseInputObject
argument :old_path, GraphQL::STRING_TYPE, required: false,
description: 'The path of the file on the start sha'
argument :new_path, GraphQL::STRING_TYPE, required: false,
description: 'The path of the file on the head sha'
end
# rubocop: enable Graphql/AuthorizeTypes
end
......@@ -10,5 +10,10 @@ module Types
mount_mutation Mutations::AwardEmojis::Remove
mount_mutation Mutations::AwardEmojis::Toggle
mount_mutation Mutations::MergeRequests::SetWip, calls_gitaly: true
mount_mutation Mutations::Notes::Create::Note, calls_gitaly: true
mount_mutation Mutations::Notes::Create::DiffNote, calls_gitaly: true
mount_mutation Mutations::Notes::Create::ImageDiffNote, calls_gitaly: true
mount_mutation Mutations::Notes::Update
mount_mutation Mutations::Notes::Destroy
end
end
# frozen_string_literal: true
module Types
module Notes
# rubocop: disable Graphql/AuthorizeTypes
class DiffImagePositionInputType < DiffPositionBaseInputType
graphql_name 'DiffImagePositionInput'
argument :x, GraphQL::INT_TYPE, required: true,
description: copy_field_description(Types::Notes::DiffPositionType, :x)
argument :y, GraphQL::INT_TYPE, required: true,
description: copy_field_description(Types::Notes::DiffPositionType, :y)
argument :width, GraphQL::INT_TYPE, required: true,
description: copy_field_description(Types::Notes::DiffPositionType, :width)
argument :height, GraphQL::INT_TYPE, required: true,
description: copy_field_description(Types::Notes::DiffPositionType, :height)
end
# rubocop: enable Graphql/AuthorizeTypes
end
end
# frozen_string_literal: true
module Types
module Notes
# rubocop: disable Graphql/AuthorizeTypes
class DiffPositionBaseInputType < BaseInputObject
argument :head_sha, GraphQL::STRING_TYPE, required: true,
description: copy_field_description(Types::Notes::DiffPositionType, :head_sha)
argument :base_sha, GraphQL::STRING_TYPE, required: false,
description: copy_field_description(Types::Notes::DiffPositionType, :base_sha)
argument :start_sha, GraphQL::STRING_TYPE, required: true,
description: copy_field_description(Types::Notes::DiffPositionType, :start_sha)
argument :paths,
Types::DiffPathsInputType,
required: true,
description: 'The paths of the file that was changed. ' \
'Both of the properties of this input are optional, but at least one of them is required'
end
# rubocop: enable Graphql/AuthorizeTypes
end
end
# frozen_string_literal: true
module Types
module Notes
# rubocop: disable Graphql/AuthorizeTypes
class DiffPositionInputType < DiffPositionBaseInputType
graphql_name 'DiffPositionInput'
argument :old_line, GraphQL::INT_TYPE, required: false,
description: copy_field_description(Types::Notes::DiffPositionType, :old_line)
argument :new_line, GraphQL::INT_TYPE, required: true,
description: copy_field_description(Types::Notes::DiffPositionType, :new_line)
end
# rubocop: enable Graphql/AuthorizeTypes
end
end
---
title: GraphQL mutations for managing Notes
merge_request: 30210
author:
type: added
# frozen_string_literal: true
require 'spec_helper'
describe 'Adding a DiffNote' do
include GraphqlHelpers
set(:current_user) { create(:user) }
let(:noteable) { create(:merge_request, source_project: project, target_project: project) }
let(:project) { create(:project, :repository) }
let(:diff_refs) { noteable.diff_refs }
let(:mutation) do
variables = {
noteable_id: GitlabSchema.id_from_object(noteable).to_s,
body: 'Body text',
position: {
paths: {
old_path: 'files/ruby/popen.rb',
new_path: 'files/ruby/popen2.rb'
},
new_line: 14,
base_sha: diff_refs.base_sha,
head_sha: diff_refs.head_sha,
start_sha: diff_refs.start_sha
}
}
graphql_mutation(:create_diff_note, variables)
end
def mutation_response
graphql_mutation_response(:create_diff_note)
end
it_behaves_like 'a Note mutation when the user does not have permission'
context 'when the user has permission' do
before do
project.add_developer(current_user)
end
it_behaves_like 'a Note mutation that creates a Note'
it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote
context do
let(:diff_refs) { build(:merge_request).diff_refs } # Allow fake diff refs so arguments are valid
it_behaves_like 'a Note mutation when the given resource id is not for a Noteable'
end
it 'returns the note with the correct position' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['note']['body']).to eq('Body text')
mutation_position_response = mutation_response['note']['position']
expect(mutation_position_response['positionType']).to eq('text')
expect(mutation_position_response['filePath']).to eq('files/ruby/popen2.rb')
expect(mutation_position_response['oldPath']).to eq('files/ruby/popen.rb')
expect(mutation_position_response['newPath']).to eq('files/ruby/popen2.rb')
expect(mutation_position_response['newLine']).to eq(14)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Adding an image DiffNote' do
include GraphqlHelpers
set(:current_user) { create(:user) }
let(:noteable) { create(:merge_request, source_project: project, target_project: project) }
let(:project) { create(:project, :repository) }
let(:diff_refs) { noteable.diff_refs }
let(:mutation) do
variables = {
noteable_id: GitlabSchema.id_from_object(noteable).to_s,
body: 'Body text',
position: {
paths: {
old_path: 'files/images/any_image.png',
new_path: 'files/images/any_image2.png'
},
width: 100,
height: 200,
x: 1,
y: 2,
base_sha: diff_refs.base_sha,
head_sha: diff_refs.head_sha,
start_sha: diff_refs.start_sha
}
}
graphql_mutation(:create_image_diff_note, variables)
end
def mutation_response
graphql_mutation_response(:create_image_diff_note)
end
it_behaves_like 'a Note mutation when the user does not have permission'
context 'when the user has permission' do
before do
project.add_developer(current_user)
end
it_behaves_like 'a Note mutation that creates a Note'
it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote
context do
let(:diff_refs) { build(:merge_request).diff_refs } # Allow fake diff refs so arguments are valid
it_behaves_like 'a Note mutation when the given resource id is not for a Noteable'
end
it 'returns the note with the correct position' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['note']['body']).to eq('Body text')
mutation_position_response = mutation_response['note']['position']
expect(mutation_position_response['filePath']).to eq('files/images/any_image2.png')
expect(mutation_position_response['oldPath']).to eq('files/images/any_image.png')
expect(mutation_position_response['newPath']).to eq('files/images/any_image2.png')
expect(mutation_position_response['positionType']).to eq('image')
expect(mutation_position_response['width']).to eq(100)
expect(mutation_position_response['height']).to eq(200)
expect(mutation_position_response['x']).to eq(1)
expect(mutation_position_response['y']).to eq(2)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Adding a Note' do
include GraphqlHelpers
set(:current_user) { create(:user) }
let(:noteable) { create(:merge_request, source_project: project, target_project: project) }
let(:project) { create(:project, :repository) }
let(:mutation) do
variables = {
noteable_id: GitlabSchema.id_from_object(noteable).to_s,
body: 'Body text'
}
graphql_mutation(:create_note, variables)
end
def mutation_response
graphql_mutation_response(:create_note)
end
it_behaves_like 'a Note mutation when the user does not have permission'
context 'when the user has permission' do
before do
project.add_developer(current_user)
end
it_behaves_like 'a Note mutation that creates a Note'
it_behaves_like 'a Note mutation when there are active record validation errors'
it_behaves_like 'a Note mutation when the given resource id is not for a Noteable'
it 'returns the note' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['note']['body']).to eq('Body text')
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Destroying a Note' do
include GraphqlHelpers
let!(:note) { create(:note) }
let(:mutation) do
variables = {
id: GitlabSchema.id_from_object(note).to_s
}
graphql_mutation(:destroy_note, variables)
end
def mutation_response
graphql_mutation_response(:destroy_note)
end
context 'when the user does not have permission' do
let(:current_user) { create(:user) }
it_behaves_like 'a mutation that returns top-level errors',
errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action']
it 'does not destroy the Note' do
expect do
post_graphql_mutation(mutation, current_user: current_user)
end.not_to change { Note.count }
end
end
context 'when the user has permission' do
let(:current_user) { note.author }
it_behaves_like 'a Note mutation when the given resource id is not for a Note'
it 'destroys the Note' do
expect do
post_graphql_mutation(mutation, current_user: current_user)
end.to change { Note.count }.by(-1)
end
it 'returns an empty Note' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response).to have_key('note')
expect(mutation_response['note']).to be_nil
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Updating a Note' do
include GraphqlHelpers
let!(:note) { create(:note, note: original_body) }
let(:original_body) { 'Initial body text' }
let(:updated_body) { 'Updated body text' }
let(:mutation) do
variables = {
id: GitlabSchema.id_from_object(note).to_s,
body: updated_body
}
graphql_mutation(:update_note, variables)
end
def mutation_response
graphql_mutation_response(:update_note)
end
context 'when the user does not have permission' do
let(:current_user) { create(:user) }
it_behaves_like 'a mutation that returns top-level errors',
errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action']
it 'does not update the Note' do
post_graphql_mutation(mutation, current_user: current_user)
expect(note.reload.note).to eq(original_body)
end
end
context 'when the user has permission' do
let(:current_user) { note.author }
it_behaves_like 'a Note mutation when the given resource id is not for a Note'
it 'updates the Note' do
post_graphql_mutation(mutation, current_user: current_user)
expect(note.reload.note).to eq(updated_body)
end
it 'returns the updated Note' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['note']['body']).to eq(updated_body)
end
context 'when there are ActiveRecord validation errors' do
let(:updated_body) { '' }
it_behaves_like 'a mutation that returns errors in the response', errors: ["Note can't be blank"]
it 'does not update the Note' do
post_graphql_mutation(mutation, current_user: current_user)
expect(note.reload.note).to eq(original_body)
end
it 'returns the Note with its original body' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['note']['body']).to eq(original_body)
end
end
end
end
......@@ -57,7 +57,8 @@ module GraphqlHelpers
end
def variables_for_mutation(name, input)
graphql_input = input.map { |name, value| [GraphqlHelpers.fieldnamerize(name), value] }.to_h
graphql_input = prepare_input_for_mutation(input)
result = { input_variable_name_for_mutation(name) => graphql_input }
# Avoid trying to serialize multipart data into JSON
......@@ -68,6 +69,18 @@ module GraphqlHelpers
end
end
# Recursively convert a Hash with Ruby-style keys to GraphQL fieldname-style keys
#
# prepare_input_for_mutation({ 'my_key' => 1 })
# => { 'myKey' => 1}
def prepare_input_for_mutation(input)
input.map do |name, value|
value = prepare_input_for_mutation(value) if value.is_a?(Hash)
[GraphqlHelpers.fieldnamerize(name), value]
end.to_h
end
def input_variable_name_for_mutation(mutation_name)
mutation_name = GraphqlHelpers.fieldnamerize(mutation_name)
mutation_field = GitlabSchema.mutation.fields[mutation_name]
......
# frozen_string_literal: true
RSpec.shared_examples 'a Note mutation that does not create a Note' do
it do
expect do
post_graphql_mutation(mutation, current_user: current_user)
end.not_to change { Note.count }
end
end
RSpec.shared_examples 'a Note mutation that creates a Note' do
it do
expect do
post_graphql_mutation(mutation, current_user: current_user)
end.to change { Note.count }.by(1)
end
end
RSpec.shared_examples 'a Note mutation when the user does not have permission' do
it_behaves_like 'a Note mutation that does not create a Note'
it_behaves_like 'a mutation that returns top-level errors',
errors: ['The resource that you are attempting to access does not exist or you don\'t have permission to perform this action']
end
RSpec.shared_examples 'a Note mutation when there are active record validation errors' do |model: Note|
before do
expect_next_instance_of(model) do |note|
expect(note).to receive(:valid?).at_least(:once).and_return(false)
expect(note).to receive_message_chain(