Commit 0328d4fa authored by James Lopez's avatar James Lopez

Merge branch '56485-implement-graphql-mergerequestsresolver' into 'master'

Resolve "Implement GraphQL MergeRequestsResolver"

Closes #56485

See merge request gitlab-org/gitlab-ce!24805
parents b95cf314 f80f6bbc
...@@ -25,7 +25,8 @@ module Mutations ...@@ -25,7 +25,8 @@ module Mutations
def find_object(project_path:, iid:) def find_object(project_path:, iid:)
project = resolve_project(full_path: project_path) project = resolve_project(full_path: project_path)
resolver = Resolvers::MergeRequestResolver.new(object: project, context: context) resolver = Resolvers::MergeRequestsResolver
.single.new(object: project, context: context)
resolver.resolve(iid: iid) resolver.resolve(iid: iid)
end end
......
...@@ -2,5 +2,12 @@ ...@@ -2,5 +2,12 @@
module Resolvers module Resolvers
class BaseResolver < GraphQL::Schema::Resolver class BaseResolver < GraphQL::Schema::Resolver
def self.single
@single ||= Class.new(self) do
def resolve(**args)
super.first
end
end
end
end end
end end
...@@ -2,7 +2,9 @@ ...@@ -2,7 +2,9 @@
module Resolvers module Resolvers
class IssuesResolver < BaseResolver class IssuesResolver < BaseResolver
extend ActiveSupport::Concern argument :iid, GraphQL::ID_TYPE,
required: false,
description: 'The IID of the issue, e.g., "1"'
argument :iids, [GraphQL::ID_TYPE], argument :iids, [GraphQL::ID_TYPE],
required: false, required: false,
...@@ -22,6 +24,7 @@ module Resolvers ...@@ -22,6 +24,7 @@ module Resolvers
# Will need to be be made group & namespace aware with # Will need to be be made group & namespace aware with
# https://gitlab.com/gitlab-org/gitlab-ce/issues/54520 # https://gitlab.com/gitlab-org/gitlab-ce/issues/54520
args[:project_id] = project.id args[:project_id] = project.id
args[:iids] ||= [args[:iid]].compact
IssuesFinder.new(context[:current_user], args).execute IssuesFinder.new(context[:current_user], args).execute
end end
......
# frozen_string_literal: true # frozen_string_literal: true
module Resolvers module Resolvers
class MergeRequestResolver < BaseResolver class MergeRequestsResolver < BaseResolver
argument :iid, GraphQL::ID_TYPE, argument :iid, GraphQL::ID_TYPE,
required: true, required: false,
description: 'The IID of the merge request, e.g., "1"' description: 'The IID of the merge request, e.g., "1"'
argument :iids, [GraphQL::ID_TYPE],
required: false,
description: 'The list of IIDs of issues, e.g., [1, 2]'
type Types::MergeRequestType, null: true type Types::MergeRequestType, null: true
alias_method :project, :object alias_method :project, :object
# rubocop: disable CodeReuse/ActiveRecord def resolve(**args)
def resolve(iid:)
return unless project.present? return unless project.present?
args[:iids] ||= [args[:iid]].compact
args[:iids].map { |iid| batch_load(iid) }
.select(&:itself) # .compact doesn't work on BatchLoader
end
# rubocop: disable CodeReuse/ActiveRecord
def batch_load(iid)
BatchLoader.for(iid.to_s).batch(key: project) do |iids, loader, args| BatchLoader.for(iid.to_s).batch(key: project) do |iids, loader, args|
args[:key].merge_requests.where(iid: iids).each do |mr| args[:key].merge_requests.where(iid: iids).each do |mr|
loader.call(mr.iid.to_s, mr) loader.call(mr.iid.to_s, mr)
......
...@@ -66,10 +66,17 @@ module Types ...@@ -66,10 +66,17 @@ module Types
field :only_allow_merge_if_all_discussions_are_resolved, 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 :printing_merge_request_link_enabled, GraphQL::BOOLEAN_TYPE, null: true
field :merge_requests,
Types::MergeRequestType.connection_type,
null: true,
resolver: Resolvers::MergeRequestsResolver do
authorize :read_merge_request
end
field :merge_request, field :merge_request,
Types::MergeRequestType, Types::MergeRequestType,
null: true, null: true,
resolver: Resolvers::MergeRequestResolver do resolver: Resolvers::MergeRequestsResolver.single do
authorize :read_merge_request authorize :read_merge_request
end end
...@@ -78,6 +85,11 @@ module Types ...@@ -78,6 +85,11 @@ module Types
null: true, null: true,
resolver: Resolvers::IssuesResolver resolver: Resolvers::IssuesResolver
field :issue,
Types::IssueType,
null: true,
resolver: Resolvers::IssuesResolver.single
field :pipelines, field :pipelines,
Types::Ci::PipelineType.connection_type, Types::Ci::PipelineType.connection_type,
null: false, null: false,
......
---
title: Add field mergeRequests for project in GraphQL
merge_request: 24805
author:
type: added
...@@ -35,10 +35,22 @@ module Gitlab ...@@ -35,10 +35,22 @@ module Gitlab
private private
def build_checker(current_user, abilities) def build_checker(current_user, abilities)
proc do |obj| lambda do |value|
# Load the elements if they weren't loaded by BatchLoader yet # Load the elements if they weren't loaded by BatchLoader yet
obj = obj.sync if obj.respond_to?(:sync) value = value.sync if value.respond_to?(:sync)
obj if abilities.all? { |ability| Ability.allowed?(current_user, ability, obj) }
check = lambda do |object|
abilities.all? do |ability|
Ability.allowed?(current_user, ability, object)
end
end
case value
when Array
value.select(&check)
else
value if check.call(value)
end
end end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe Resolvers::BaseResolver do
include GraphqlHelpers
let(:resolver) do
Class.new(described_class) do
def resolve(**args)
[args, args]
end
end
end
describe '.single' do
it 'returns a subclass from the resolver' do
expect(resolver.single.superclass).to eq(resolver)
end
it 'returns the same subclass every time' do
expect(resolver.single.object_id).to eq(resolver.single.object_id)
end
it 'returns a resolver that gives the first result from the original resolver' do
result = resolve(resolver.single, args: { test: 1 })
expect(result).to eq(test: 1)
end
end
end
...@@ -33,6 +33,10 @@ describe Resolvers::IssuesResolver do ...@@ -33,6 +33,10 @@ describe Resolvers::IssuesResolver do
expect(resolve_issues).to contain_exactly(issue, issue2) expect(resolve_issues).to contain_exactly(issue, issue2)
end end
it 'finds a specific issue with iid' do
expect(resolve_issues(iid: issue.iid)).to contain_exactly(issue)
end
it 'finds a specific issue with iids' do it 'finds a specific issue with iids' do
expect(resolve_issues(iids: issue.iid)).to contain_exactly(issue) expect(resolve_issues(iids: issue.iid)).to contain_exactly(issue)
end end
......
require 'spec_helper' require 'spec_helper'
describe Resolvers::MergeRequestResolver do describe Resolvers::MergeRequestsResolver do
include GraphqlHelpers include GraphqlHelpers
set(:project) { create(:project, :repository) } set(:project) { create(:project, :repository) }
...@@ -16,9 +16,17 @@ describe Resolvers::MergeRequestResolver do ...@@ -16,9 +16,17 @@ describe Resolvers::MergeRequestResolver do
let(:other_iid) { other_merge_request.iid } let(:other_iid) { other_merge_request.iid }
describe '#resolve' do describe '#resolve' do
it 'batch-resolves merge requests by target project full path and IID' do it 'batch-resolves by target project full path and individual IID' do
result = batch(max_queries: 2) do result = batch(max_queries: 2) do
[resolve_mr(project, iid_1), resolve_mr(project, iid_2)] resolve_mr(project, iid: iid_1) + resolve_mr(project, iid: iid_2)
end
expect(result).to contain_exactly(merge_request_1, merge_request_2)
end
it 'batch-resolves by target project full path and IIDS' do
result = batch(max_queries: 2) do
resolve_mr(project, iids: [iid_1, iid_2])
end end
expect(result).to contain_exactly(merge_request_1, merge_request_2) expect(result).to contain_exactly(merge_request_1, merge_request_2)
...@@ -26,20 +34,28 @@ describe Resolvers::MergeRequestResolver do ...@@ -26,20 +34,28 @@ describe Resolvers::MergeRequestResolver do
it 'can batch-resolve merge requests from different projects' do it 'can batch-resolve merge requests from different projects' do
result = batch(max_queries: 3) do result = batch(max_queries: 3) do
[resolve_mr(project, iid_1), resolve_mr(project, iid_2), resolve_mr(other_project, other_iid)] resolve_mr(project, iid: iid_1) +
resolve_mr(project, iid: iid_2) +
resolve_mr(other_project, iid: other_iid)
end end
expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request) expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request)
end end
it 'resolves an unknown iid to nil' do it 'resolves an unknown iid to be empty' do
result = batch { resolve_mr(project, -1) } result = batch { resolve_mr(project, iid: -1) }
expect(result).to be_empty
end
it 'resolves empty iids to be empty' do
result = batch { resolve_mr(project, iids: []) }
expect(result).to be_nil expect(result).to be_empty
end end
end end
def resolve_mr(project, iid) def resolve_mr(project, args)
resolve(described_class, obj: project, args: { iid: iid }) resolve(described_class, obj: project, args: args)
end end
end end
...@@ -6,12 +6,18 @@ describe GitlabSchema.types['Project'] do ...@@ -6,12 +6,18 @@ describe GitlabSchema.types['Project'] do
it { expect(described_class.graphql_name).to eq('Project') } it { expect(described_class.graphql_name).to eq('Project') }
describe 'nested merge request' do describe 'nested merge request' do
it { expect(described_class).to have_graphql_field(:merge_requests) }
it { expect(described_class).to have_graphql_field(:merge_request) } it { expect(described_class).to have_graphql_field(:merge_request) }
it 'authorizes the merge request' do it 'authorizes the merge request' do
expect(described_class.fields['mergeRequest']) expect(described_class.fields['mergeRequest'])
.to require_graphql_authorizations(:read_merge_request) .to require_graphql_authorizations(:read_merge_request)
end end
it 'authorizes the merge requests' do
expect(described_class.fields['mergeRequests'])
.to require_graphql_authorizations(:read_merge_request)
end
end end
describe 'nested issues' do describe 'nested issues' do
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Graphql::Authorize::Instrumentation do
describe '#build_checker' do
let(:current_user) { double(:current_user) }
let(:abilities) { [double(:first_ability), double(:last_ability)] }
let(:checker) do
described_class.new.__send__(:build_checker, current_user, abilities)
end
it 'returns a checker which checks for a single object' do
object = double(:object)
abilities.each do |ability|
spy_ability_check_for(ability, object, passed: true)
end
expect(checker.call(object)).to eq(object)
end
it 'returns a checker which checks for all objects' do
objects = [double(:first), double(:last)]
abilities.each do |ability|
objects.each do |object|
spy_ability_check_for(ability, object, passed: true)
end
end
expect(checker.call(objects)).to eq(objects)
end
context 'when some objects would not pass the check' do
it 'returns nil when it is single object' do
disallowed = double(:object)
spy_ability_check_for(abilities.first, disallowed, passed: false)
expect(checker.call(disallowed)).to be_nil
end
it 'returns only objects which passed when there are more than one' do
allowed = double(:allowed)
disallowed = double(:disallowed)
spy_ability_check_for(abilities.first, disallowed, passed: false)
abilities.each do |ability|
spy_ability_check_for(ability, allowed, passed: true)
end
expect(checker.call([disallowed, allowed]))
.to contain_exactly(allowed)
end
end
def spy_ability_check_for(ability, object, passed: true)
expect(Ability)
.to receive(:allowed?)
.with(current_user, ability, object)
.and_return(passed)
end
end
end
...@@ -56,4 +56,38 @@ describe 'getting an issue list for a project' do ...@@ -56,4 +56,38 @@ describe 'getting an issue list for a project' do
expect(issues_data).to eq [] expect(issues_data).to eq []
end end
end end
context 'when there is a confidential issue' do
let!(:confidential_issue) do
create(:issue, :confidential, project: project)
end
context 'when the user cannot see confidential issues' do
it 'returns issues without confidential issues' do
post_graphql(query, current_user: current_user)
expect(issues_data.size).to eq(2)
issues_data.each do |issue|
expect(issue.dig('node', 'confidential')).to eq(false)
end
end
end
context 'when the user can see confidential issues' do
it 'returns issues with confidential issues' do
project.add_developer(current_user)
post_graphql(query, current_user: current_user)
expect(issues_data.size).to eq(3)
confidentials = issues_data.map do |issue|
issue.dig('node', 'confidential')
end
expect(confidentials).to eq([true, false, false])
end
end
end
end end
...@@ -84,7 +84,7 @@ module GraphqlHelpers ...@@ -84,7 +84,7 @@ module GraphqlHelpers
QUERY QUERY
end end
def all_graphql_fields_for(class_name) def all_graphql_fields_for(class_name, parent_types = Set.new)
type = GitlabSchema.types[class_name.to_s] type = GitlabSchema.types[class_name.to_s]
return "" unless type return "" unless type
...@@ -92,8 +92,17 @@ module GraphqlHelpers ...@@ -92,8 +92,17 @@ module GraphqlHelpers
# We can't guess arguments, so skip fields that require them # We can't guess arguments, so skip fields that require them
next if required_arguments?(field) next if required_arguments?(field)
singular_field_type = field_type(field)
# If field type is the same as parent type, then we're hitting into
# mutual dependency. Break it from infinite recursion
next if parent_types.include?(singular_field_type)
if nested_fields?(field) if nested_fields?(field)
"#{name} { #{all_graphql_fields_for(field_type(field))} }" fields =
all_graphql_fields_for(singular_field_type, parent_types | [type])
"#{name} { #{fields} }"
else else
name name
end end
......
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