Commit 2c7cf0c5 authored by Mikołaj Wawrzyniak's avatar Mikołaj Wawrzyniak

Merge branch 'ph/282476/fixGraphQLDiffComments' into 'master'

Fixes createDiffNote GraphQL mutation

See merge request gitlab-org/gitlab!54801
parents 70a9bd19 f29ff46a
......@@ -11,6 +11,22 @@ module Mutations
required: true,
description: copy_field_description(Types::Notes::NoteType, :position)
def ready?(**args)
# As both arguments are optional, validate here that one of the
# arguments are present.
#
# This may be able to be done using InputUnions in the future
# if this RFC is merged:
# https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md
if args[:position].to_hash.values_at(:old_line, :new_line).compact.blank?
raise Gitlab::Graphql::Errors::ArgumentError,
'position oldLine or newLine arguments are required'
end
super(**args)
end
private
def create_note_params(noteable, args)
......
......@@ -8,7 +8,7 @@ module Types
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,
argument :new_line, GraphQL::INT_TYPE, required: false,
description: copy_field_description(Types::Notes::DiffPositionType, :new_line)
end
# rubocop: enable Graphql/AuthorizeTypes
......
---
title: Fixed diff notes GraphQL mutation not allowing comments on deleted lines
merge_request: 54801
author:
type: fixed
......@@ -9,8 +9,9 @@ RSpec.describe 'Adding a DiffNote' do
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 = {
let(:base_variables) do
{
noteable_id: GitlabSchema.id_from_object(noteable).to_s,
body: 'Body text',
position: {
......@@ -18,16 +19,16 @@ RSpec.describe 'Adding a DiffNote' do
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
let(:variables) { base_variables.deep_merge({ position: { new_line: 14 } }) }
let(:mutation) { graphql_mutation(:create_diff_note, variables) }
def mutation_response
graphql_mutation_response(:create_diff_note)
end
......@@ -41,6 +42,18 @@ RSpec.describe 'Adding a DiffNote' do
it_behaves_like 'a Note mutation that creates a Note'
context 'add comment to old line' do
let(:variables) { base_variables.deep_merge({ position: { old_line: 14 } }) }
it_behaves_like 'a Note mutation that creates a Note'
end
context 'add a comment with a position without lines' do
let(:variables) { base_variables }
it_behaves_like 'a Note mutation that does not create a Note'
end
it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote
it_behaves_like 'a Note mutation when there are rate limit validation errors'
......
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