Allow querying a single MR within a project
This allows the user to get a single MR nested in a GraphQL project query. Since we need the full path and the iid anyway, this makes more sense than having a root query that needs the full path as well.
This commit is contained in:
parent
d85929d72a
commit
9403b1d951
|
@ -1,15 +1,14 @@
|
|||
module Resolvers
|
||||
class MergeRequestResolver < BaseResolver
|
||||
prepend FullPathResolver
|
||||
|
||||
type Types::MergeRequestType, null: true
|
||||
|
||||
argument :iid, GraphQL::ID_TYPE,
|
||||
required: true,
|
||||
description: 'The IID of the merge request, e.g., "1"'
|
||||
|
||||
def resolve(full_path:, iid:)
|
||||
project = model_by_full_path(Project, full_path)
|
||||
type Types::MergeRequestType, null: true
|
||||
|
||||
alias_method :project, :object
|
||||
|
||||
def resolve(iid:)
|
||||
return unless project.present?
|
||||
|
||||
BatchLoader.for(iid.to_s).batch(key: project.id) do |iids, loader|
|
||||
|
|
|
@ -61,5 +61,12 @@ module Types
|
|||
field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true
|
||||
field :only_allow_merge_if_all_discussions_are_resolved, GraphQL::BOOLEAN_TYPE, null: true
|
||||
field :printing_merge_request_link_enabled, GraphQL::BOOLEAN_TYPE, null: true
|
||||
|
||||
field :merge_request,
|
||||
Types::MergeRequestType,
|
||||
null: true,
|
||||
resolver: Resolvers::MergeRequestResolver do
|
||||
authorize :read_merge_request
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -9,13 +9,6 @@ module Types
|
|||
authorize :read_project
|
||||
end
|
||||
|
||||
field :merge_request, Types::MergeRequestType,
|
||||
null: true,
|
||||
resolver: Resolvers::MergeRequestResolver,
|
||||
description: "Find a merge request" do
|
||||
authorize :read_merge_request
|
||||
end
|
||||
|
||||
field :echo, GraphQL::STRING_TYPE, null: false, function: Functions::Echo.new
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Allow querying a single merge request within a project
|
||||
merge_request: 19853
|
||||
author:
|
||||
type: changed
|
|
@ -29,9 +29,7 @@ curl --data "value=100" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://g
|
|||
|
||||
## Available queries
|
||||
|
||||
A first iteration of a GraphQL API includes only 2 queries: `project` and
|
||||
`merge_request` and only returns scalar fields, or fields of the type `Project`
|
||||
or `MergeRequest`.
|
||||
A first iteration of a GraphQL API includes a query for: `project`. Within a project it is also possible to fetch a `mergeRequest` by IID.
|
||||
|
||||
## GraphiQL
|
||||
|
||||
|
|
|
@ -10,49 +10,36 @@ describe Resolvers::MergeRequestResolver do
|
|||
set(:other_project) { create(:project, :repository) }
|
||||
set(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) }
|
||||
|
||||
let(:full_path) { project.full_path }
|
||||
let(:iid_1) { merge_request_1.iid }
|
||||
let(:iid_2) { merge_request_2.iid }
|
||||
|
||||
let(:other_full_path) { other_project.full_path }
|
||||
let(:other_iid) { other_merge_request.iid }
|
||||
|
||||
describe '#resolve' do
|
||||
it 'batch-resolves merge requests by target project full path and IID' do
|
||||
path = full_path # avoid database query
|
||||
|
||||
result = batch(max_queries: 2) do
|
||||
[resolve_mr(path, iid_1), resolve_mr(path, iid_2)]
|
||||
[resolve_mr(project, iid_1), resolve_mr(project, iid_2)]
|
||||
end
|
||||
|
||||
expect(result).to contain_exactly(merge_request_1, merge_request_2)
|
||||
end
|
||||
|
||||
it 'can batch-resolve merge requests from different projects' do
|
||||
path = project.full_path # avoid database queries
|
||||
other_path = other_full_path
|
||||
|
||||
result = batch(max_queries: 3) do
|
||||
[resolve_mr(path, iid_1), resolve_mr(path, iid_2), resolve_mr(other_path, other_iid)]
|
||||
[resolve_mr(project, iid_1), resolve_mr(project, iid_2), resolve_mr(other_project, other_iid)]
|
||||
end
|
||||
|
||||
expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request)
|
||||
end
|
||||
|
||||
it 'resolves an unknown iid to nil' do
|
||||
result = batch { resolve_mr(full_path, -1) }
|
||||
|
||||
expect(result).to be_nil
|
||||
end
|
||||
|
||||
it 'resolves a known iid for an unknown full_path to nil' do
|
||||
result = batch { resolve_mr('unknown/project', iid_1) }
|
||||
result = batch { resolve_mr(project, -1) }
|
||||
|
||||
expect(result).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
def resolve_mr(full_path, iid)
|
||||
resolve(described_class, args: { full_path: full_path, iid: iid })
|
||||
def resolve_mr(project, iid)
|
||||
resolve(described_class, obj: project, args: { iid: iid })
|
||||
end
|
||||
end
|
||||
|
|
|
@ -2,4 +2,13 @@ require 'spec_helper'
|
|||
|
||||
describe GitlabSchema.types['Project'] do
|
||||
it { expect(described_class.graphql_name).to eq('Project') }
|
||||
|
||||
describe 'nested merge request' do
|
||||
it { expect(described_class).to have_graphql_field(:merge_request) }
|
||||
|
||||
it 'authorizes the merge request' do
|
||||
expect(described_class.fields['mergeRequest'])
|
||||
.to require_graphql_authorizations(:read_merge_request)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -5,7 +5,7 @@ describe GitlabSchema.types['Query'] do
|
|||
expect(described_class.graphql_name).to eq('Query')
|
||||
end
|
||||
|
||||
it { is_expected.to have_graphql_fields(:project, :merge_request, :echo) }
|
||||
it { is_expected.to have_graphql_fields(:project, :echo) }
|
||||
|
||||
describe 'project field' do
|
||||
subject { described_class.fields['project'] }
|
||||
|
@ -20,18 +20,4 @@ describe GitlabSchema.types['Query'] do
|
|||
is_expected.to require_graphql_authorizations(:read_project)
|
||||
end
|
||||
end
|
||||
|
||||
describe 'merge_request field' do
|
||||
subject { described_class.fields['mergeRequest'] }
|
||||
|
||||
it 'finds MRs by project and IID' do
|
||||
is_expected.to have_graphql_arguments(:full_path, :iid)
|
||||
is_expected.to have_graphql_type(Types::MergeRequestType)
|
||||
is_expected.to have_graphql_resolver(Resolvers::MergeRequestResolver)
|
||||
end
|
||||
|
||||
it 'authorizes with read_merge_request' do
|
||||
is_expected.to require_graphql_authorizations(:read_merge_request)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,49 +0,0 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe 'getting merge request information' do
|
||||
include GraphqlHelpers
|
||||
|
||||
let(:project) { create(:project, :repository) }
|
||||
let(:merge_request) { create(:merge_request, source_project: project) }
|
||||
let(:current_user) { create(:user) }
|
||||
|
||||
let(:query) do
|
||||
attributes = {
|
||||
'fullPath' => merge_request.project.full_path,
|
||||
'iid' => merge_request.iid
|
||||
}
|
||||
graphql_query_for('mergeRequest', attributes)
|
||||
end
|
||||
|
||||
context 'when the user has access to the merge request' do
|
||||
before do
|
||||
project.add_developer(current_user)
|
||||
post_graphql(query, current_user: current_user)
|
||||
end
|
||||
|
||||
it 'returns the merge request' do
|
||||
expect(graphql_data['mergeRequest']).not_to be_nil
|
||||
end
|
||||
|
||||
# This is a field coming from the `MergeRequestPresenter`
|
||||
it 'includes a web_url' do
|
||||
expect(graphql_data['mergeRequest']['webUrl']).to be_present
|
||||
end
|
||||
|
||||
it_behaves_like 'a working graphql query'
|
||||
end
|
||||
|
||||
context 'when the user does not have access to the merge request' do
|
||||
before do
|
||||
post_graphql(query, current_user: current_user)
|
||||
end
|
||||
|
||||
it 'returns an empty field' do
|
||||
post_graphql(query, current_user: current_user)
|
||||
|
||||
expect(graphql_data['mergeRequest']).to be_nil
|
||||
end
|
||||
|
||||
it_behaves_like 'a working graphql query'
|
||||
end
|
||||
end
|
|
@ -13,27 +13,76 @@ describe 'getting project information' do
|
|||
context 'when the user has access to the project' do
|
||||
before do
|
||||
project.add_developer(current_user)
|
||||
post_graphql(query, current_user: current_user)
|
||||
end
|
||||
|
||||
it 'includes the project' do
|
||||
post_graphql(query, current_user: current_user)
|
||||
|
||||
expect(graphql_data['project']).not_to be_nil
|
||||
end
|
||||
|
||||
it_behaves_like 'a working graphql query'
|
||||
end
|
||||
|
||||
context 'when the user does not have access to the project' do
|
||||
it_behaves_like 'a working graphql query' do
|
||||
before do
|
||||
post_graphql(query, current_user: current_user)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when requesting a nested merge request' do
|
||||
let(:merge_request) { create(:merge_request, source_project: project) }
|
||||
let(:merge_request_graphql_data) { graphql_data['project']['mergeRequest'] }
|
||||
|
||||
let(:query) do
|
||||
graphql_query_for(
|
||||
'project',
|
||||
{ 'fullPath' => project.full_path },
|
||||
query_graphql_field('mergeRequest', iid: merge_request.iid)
|
||||
)
|
||||
end
|
||||
|
||||
it_behaves_like 'a working graphql query' do
|
||||
before do
|
||||
post_graphql(query, current_user: current_user)
|
||||
end
|
||||
end
|
||||
|
||||
it 'contains merge request information' do
|
||||
post_graphql(query, current_user: current_user)
|
||||
|
||||
expect(merge_request_graphql_data).not_to be_nil
|
||||
end
|
||||
|
||||
# This is a field coming from the `MergeRequestPresenter`
|
||||
it 'includes a web_url' do
|
||||
post_graphql(query, current_user: current_user)
|
||||
|
||||
expect(merge_request_graphql_data['webUrl']).to be_present
|
||||
end
|
||||
|
||||
context 'when the user does not have access to the merge request' do
|
||||
let(:project) { create(:project, :public, :repository) }
|
||||
|
||||
it 'returns nil' do
|
||||
project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
|
||||
|
||||
post_graphql(query)
|
||||
|
||||
expect(merge_request_graphql_data).to be_nil
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the user does not have access to the project' do
|
||||
it 'returns an empty field' do
|
||||
post_graphql(query, current_user: current_user)
|
||||
|
||||
expect(graphql_data['project']).to be_nil
|
||||
end
|
||||
|
||||
it_behaves_like 'a working graphql query'
|
||||
it_behaves_like 'a working graphql query' do
|
||||
before do
|
||||
post_graphql(query, current_user: current_user)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -34,14 +34,20 @@ module GraphqlHelpers
|
|||
end
|
||||
|
||||
def graphql_query_for(name, attributes = {}, fields = nil)
|
||||
<<~QUERY
|
||||
{
|
||||
#{query_graphql_field(name, attributes, fields)}
|
||||
}
|
||||
QUERY
|
||||
end
|
||||
|
||||
def query_graphql_field(name, attributes = {}, fields = nil)
|
||||
fields ||= all_graphql_fields_for(name.classify)
|
||||
attributes = attributes_to_graphql(attributes)
|
||||
<<~QUERY
|
||||
{
|
||||
#{name}(#{attributes}) {
|
||||
#{fields}
|
||||
}
|
||||
}
|
||||
QUERY
|
||||
end
|
||||
|
||||
|
@ -50,12 +56,15 @@ module GraphqlHelpers
|
|||
return "" unless type
|
||||
|
||||
type.fields.map do |name, field|
|
||||
# We can't guess arguments, so skip fields that require them
|
||||
next if field.arguments.any?
|
||||
|
||||
if scalar?(field)
|
||||
name
|
||||
else
|
||||
"#{name} { #{all_graphql_fields_for(field_type(field))} }"
|
||||
end
|
||||
end.join("\n")
|
||||
end.compact.join("\n")
|
||||
end
|
||||
|
||||
def attributes_to_graphql(attributes)
|
||||
|
|
|
@ -13,6 +13,12 @@ RSpec::Matchers.define :have_graphql_fields do |*expected|
|
|||
end
|
||||
end
|
||||
|
||||
RSpec::Matchers.define :have_graphql_field do |field_name|
|
||||
match do |kls|
|
||||
expect(kls.fields.keys).to include(GraphqlHelpers.fieldnamerize(field_name))
|
||||
end
|
||||
end
|
||||
|
||||
RSpec::Matchers.define :have_graphql_arguments do |*expected|
|
||||
include GraphqlHelpers
|
||||
|
||||
|
|
Loading…
Reference in New Issue