Merge branch 'bvl-graphql-nested-merge-request' into 'master'

Expose single merge request within a project GraphQL query

See merge request gitlab-org/gitlab-ce!19853

7 files changed, 89 insertions, 92 deletions

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