summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2018-06-14 15:06:53 +0200
committerBob Van Landuyt <bob@vanlanduyt.co>2018-06-15 14:38:32 +0200
commit9403b1d951c47aca67d2bac1369b86c125c5119d (patch)
treeb87c3d85588b258dceb5c22ff1e855f3c7d48cdf
parentd85929d72aa3ffc2e96d12d7b6308d3bf541676c (diff)
downloadgitlab-ce-9403b1d951c47aca67d2bac1369b86c125c5119d.tar.gz
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.
-rw-r--r--app/graphql/resolvers/merge_request_resolver.rb11
-rw-r--r--app/graphql/types/project_type.rb7
-rw-r--r--app/graphql/types/query_type.rb7
-rw-r--r--changelogs/unreleased/bvl-graphql-nested-merge-request.yml5
-rw-r--r--doc/api/graphql/index.md4
-rw-r--r--spec/graphql/resolvers/merge_request_resolver_spec.rb23
-rw-r--r--spec/graphql/types/project_type_spec.rb9
-rw-r--r--spec/graphql/types/query_type_spec.rb16
-rw-r--r--spec/requests/api/graphql/merge_request_query_spec.rb49
-rw-r--r--spec/requests/api/graphql/project_query_spec.rb63
-rw-r--r--spec/support/helpers/graphql_helpers.rb15
-rw-r--r--spec/support/matchers/graphql_matchers.rb6
12 files changed, 107 insertions, 108 deletions
diff --git a/app/graphql/resolvers/merge_request_resolver.rb b/app/graphql/resolvers/merge_request_resolver.rb
index 93d48e2645c..9f2d348e95f 100644
--- a/app/graphql/resolvers/merge_request_resolver.rb
+++ b/app/graphql/resolvers/merge_request_resolver.rb
@@ -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|
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb
index 9e885d5845a..d9058ae7431 100644
--- a/app/graphql/types/project_type.rb
+++ b/app/graphql/types/project_type.rb
@@ -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
diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb
index be79c78bf67..010ec2d7942 100644
--- a/app/graphql/types/query_type.rb
+++ b/app/graphql/types/query_type.rb
@@ -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
diff --git a/changelogs/unreleased/bvl-graphql-nested-merge-request.yml b/changelogs/unreleased/bvl-graphql-nested-merge-request.yml
new file mode 100644
index 00000000000..f0f0488d31a
--- /dev/null
+++ b/changelogs/unreleased/bvl-graphql-nested-merge-request.yml
@@ -0,0 +1,5 @@
+---
+title: Allow querying a single merge request within a project
+merge_request: 19853
+author:
+type: changed
diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md
index dcd5377284c..59e27922f64 100644
--- a/doc/api/graphql/index.md
+++ b/doc/api/graphql/index.md
@@ -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
diff --git a/spec/graphql/resolvers/merge_request_resolver_spec.rb b/spec/graphql/resolvers/merge_request_resolver_spec.rb
index af015533209..73993b3a039 100644
--- a/spec/graphql/resolvers/merge_request_resolver_spec.rb
+++ b/spec/graphql/resolvers/merge_request_resolver_spec.rb
@@ -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
diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb
index e0f89105b86..b4eeca2e3f1 100644
--- a/spec/graphql/types/project_type_spec.rb
+++ b/spec/graphql/types/project_type_spec.rb
@@ -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
diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb
index 8488252fd59..e1df6f9811d 100644
--- a/spec/graphql/types/query_type_spec.rb
+++ b/spec/graphql/types/query_type_spec.rb
@@ -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
diff --git a/spec/requests/api/graphql/merge_request_query_spec.rb b/spec/requests/api/graphql/merge_request_query_spec.rb
deleted file mode 100644
index 12b1d5d18a2..00000000000
--- a/spec/requests/api/graphql/merge_request_query_spec.rb
+++ /dev/null
@@ -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
diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb
index 8196bcfa87c..796ffc9d569 100644
--- a/spec/requests/api/graphql/project_query_spec.rb
+++ b/spec/requests/api/graphql/project_query_spec.rb
@@ -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
+ it_behaves_like 'a working graphql query' do
+ before do
+ post_graphql(query, current_user: current_user)
+ end
+ end
- context 'when the user does not have access to the project' do
- before do
- post_graphql(query, current_user: current_user)
+ 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
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index 30ff9a1196a..0930b9da368 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/spec/support/helpers/graphql_helpers.rb
@@ -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)
diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb
index ba7a1c8cde0..d23cbaf4beb 100644
--- a/spec/support/matchers/graphql_matchers.rb
+++ b/spec/support/matchers/graphql_matchers.rb
@@ -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