Commit 3f47ffdf authored by Stan Hu's avatar Stan Hu

Merge branch 'ajk-gql-mr-resolvers-split' into 'master'

[GQL] Add merge request filters

See merge request gitlab-org/gitlab!32328
parents 4d17ad3e 58e1c348
...@@ -7,8 +7,15 @@ module Mutations ...@@ -7,8 +7,15 @@ module Mutations
def resolve_issuable(type:, parent_path:, iid:) def resolve_issuable(type:, parent_path:, iid:)
parent = resolve_issuable_parent(type, parent_path) parent = resolve_issuable_parent(type, parent_path)
key = type == :merge_request ? :iids : :iid
args = { key => iid.to_s }
issuable_resolver(type, parent, context).resolve(iid: iid.to_s) resolver = issuable_resolver(type, parent, context)
ready, early_return = resolver.ready?(**args)
return early_return unless ready
resolver.resolve(**args)
end end
private private
......
# frozen_string_literal: true
# Mixin for resolving merge requests. All arguments must be in forms
# that `MergeRequestsFinder` can handle, so you may need to use aliasing.
module ResolvesMergeRequests
extend ActiveSupport::Concern
included do
type Types::MergeRequestType, null: true
end
def resolve(**args)
args[:iids] = Array.wrap(args[:iids]) if args[:iids]
args.compact!
if args.keys == [:iids]
batch_load_merge_requests(args[:iids])
else
args[:project_id] = project.id
MergeRequestsFinder.new(current_user, args).execute
end.then(&(single? ? :first : :itself))
end
def ready?(**args)
return early_return if no_results_possible?(args)
super
end
def early_return
[false, single? ? nil : MergeRequest.none]
end
private
def batch_load_merge_requests(iids)
iids.map { |iid| batch_load(iid) }.select(&:itself) # .compact doesn't work on BatchLoader
end
# rubocop: disable CodeReuse/ActiveRecord
def batch_load(iid)
BatchLoader::GraphQL.for(iid.to_s).batch(key: project) do |iids, loader, args|
args[:key].merge_requests.where(iid: iids).each do |mr|
loader.call(mr.iid.to_s, mr)
end
end
end
# rubocop: enable CodeReuse/ActiveRecord
end
# frozen_string_literal: true
module Resolvers
class MergeRequestResolver < BaseResolver.single
include ResolvesMergeRequests
alias_method :project, :synchronized_object
argument :iid, GraphQL::STRING_TYPE,
required: true,
as: :iids,
description: 'IID of the merge request, for example `1`'
def no_results_possible?(args)
project.nil?
end
end
end
...@@ -2,47 +2,39 @@ ...@@ -2,47 +2,39 @@
module Resolvers module Resolvers
class MergeRequestsResolver < BaseResolver class MergeRequestsResolver < BaseResolver
argument :iid, GraphQL::STRING_TYPE, include ResolvesMergeRequests
required: false,
description: 'IID of the merge request, for example `1`' alias_method :project, :synchronized_object
argument :iids, [GraphQL::STRING_TYPE], argument :iids, [GraphQL::STRING_TYPE],
required: false, required: false,
description: 'Array of IIDs of merge requests, for example `[1, 2]`' description: 'Array of IIDs of merge requests, for example `[1, 2]`'
type Types::MergeRequestType, null: true argument :source_branches, [GraphQL::STRING_TYPE],
required: false,
alias_method :project, :object as: :source_branch,
description: 'Array of source branch names. All resolved merge requests will have one of these branches as their source.'
def resolve(**args)
project = object.respond_to?(:sync) ? object.sync : object
return MergeRequest.none if project.nil?
args[:iids] ||= [args[:iid]].compact argument :target_branches, [GraphQL::STRING_TYPE],
required: false,
as: :target_branch,
description: 'Array of target branch names. All resolved merge requests will have one of these branches as their target.'
if args[:iids].any? argument :state, ::Types::MergeRequestStateEnum,
batch_load_merge_requests(args[:iids]) required: false,
else description: 'A merge request state. If provided, all resolved merge requests will have this state.'
args[:project_id] = project.id
MergeRequestsFinder.new(context[:current_user], args).execute argument :labels, [GraphQL::STRING_TYPE],
end required: false,
end as: :label_name,
description: 'Array of label names. All resolved merge requests will have all of these labels.'
def batch_load_merge_requests(iids) def self.single
iids.map { |iid| batch_load(iid) }.select(&:itself) # .compact doesn't work on BatchLoader ::Resolvers::MergeRequestResolver
end end
# rubocop: disable CodeReuse/ActiveRecord def no_results_possible?(args)
def batch_load(iid) project.nil? || args.values.any? { |v| v.is_a?(Array) && v.empty? }
BatchLoader::GraphQL.for(iid.to_s).batch(key: project) do |iids, loader, args|
arg_key = args[:key].respond_to?(:sync) ? args[:key].sync : args[:key]
arg_key.merge_requests.where(iid: iids).each do |mr|
loader.call(mr.iid.to_s, mr)
end
end
end end
# rubocop: enable CodeReuse/ActiveRecord
end end
end end
...@@ -81,8 +81,14 @@ module Types ...@@ -81,8 +81,14 @@ module Types
description: 'Default merge commit message of the merge request' description: 'Default merge commit message of the merge request'
field :merge_ongoing, GraphQL::BOOLEAN_TYPE, method: :merge_ongoing?, null: false, field :merge_ongoing, GraphQL::BOOLEAN_TYPE, method: :merge_ongoing?, null: false,
description: 'Indicates if a merge is currently occurring' description: 'Indicates if a merge is currently occurring'
field :source_branch_exists, GraphQL::BOOLEAN_TYPE, method: :source_branch_exists?, null: false, field :source_branch_exists, GraphQL::BOOLEAN_TYPE,
null: false, calls_gitaly: true,
method: :source_branch_exists?,
description: 'Indicates if the source branch of the merge request exists' description: 'Indicates if the source branch of the merge request exists'
field :target_branch_exists, GraphQL::BOOLEAN_TYPE,
null: false, calls_gitaly: true,
method: :target_branch_exists?,
description: 'Indicates if the target branch of the merge request exists'
field :mergeable_discussions_state, GraphQL::BOOLEAN_TYPE, null: true, field :mergeable_discussions_state, GraphQL::BOOLEAN_TYPE, null: true,
description: 'Indicates if all discussions in the merge request have been resolved, allowing the merge request to be merged' description: 'Indicates if all discussions in the merge request have been resolved, allowing the merge request to be merged'
field :web_url, GraphQL::STRING_TYPE, null: true, field :web_url, GraphQL::STRING_TYPE, null: true,
......
...@@ -3,6 +3,11 @@ ...@@ -3,6 +3,11 @@
module Types module Types
module PermissionTypes module PermissionTypes
class MergeRequest < BasePermissionType class MergeRequest < BasePermissionType
PERMISSION_FIELDS = %i[push_to_source_branch
remove_source_branch
cherry_pick_on_current_merge_request
revert_on_current_merge_request].freeze
present_using MergeRequestPresenter present_using MergeRequestPresenter
description 'Check permissions for the current user on a merge request' description 'Check permissions for the current user on a merge request'
graphql_name 'MergeRequestPermissions' graphql_name 'MergeRequestPermissions'
...@@ -10,10 +15,9 @@ module Types ...@@ -10,10 +15,9 @@ module Types
abilities :read_merge_request, :admin_merge_request, abilities :read_merge_request, :admin_merge_request,
:update_merge_request, :create_note :update_merge_request, :create_note
permission_field :push_to_source_branch, method: :can_push_to_source_branch?, calls_gitaly: true PERMISSION_FIELDS.each do |field_name|
permission_field :remove_source_branch, method: :can_remove_source_branch?, calls_gitaly: true permission_field field_name, method: :"can_#{field_name}?", calls_gitaly: true
permission_field :cherry_pick_on_current_merge_request, method: :can_cherry_pick_on_current_merge_request? end
permission_field :revert_on_current_merge_request, method: :can_revert_on_current_merge_request?
end end
end end
end end
---
title: Add filters to merge request fields
merge_request: 32328
author:
type: added
...@@ -6005,6 +6005,11 @@ type MergeRequest implements Noteable { ...@@ -6005,6 +6005,11 @@ type MergeRequest implements Noteable {
""" """
targetBranch: String! targetBranch: String!
"""
Indicates if the target branch of the merge request exists
"""
targetBranchExists: Boolean!
""" """
Target project of the merge request Target project of the merge request
""" """
...@@ -7871,12 +7876,7 @@ type Project { ...@@ -7871,12 +7876,7 @@ type Project {
""" """
IID of the merge request, for example `1` IID of the merge request, for example `1`
""" """
iid: String iid: String!
"""
Array of IIDs of merge requests, for example `[1, 2]`
"""
iids: [String!]
): MergeRequest ): MergeRequest
""" """
...@@ -7899,19 +7899,34 @@ type Project { ...@@ -7899,19 +7899,34 @@ type Project {
first: Int first: Int
""" """
IID of the merge request, for example `1` Array of IIDs of merge requests, for example `[1, 2]`
""" """
iid: String iids: [String!]
""" """
Array of IIDs of merge requests, for example `[1, 2]` Array of label names. All resolved merge requests will have all of these labels.
""" """
iids: [String!] labels: [String!]
""" """
Returns the last _n_ elements from the list. Returns the last _n_ elements from the list.
""" """
last: Int last: Int
"""
Array of source branch names. All resolved merge requests will have one of these branches as their source.
"""
sourceBranches: [String!]
"""
A merge request state. If provided, all resolved merge requests will have this state.
"""
state: MergeRequestState
"""
Array of target branch names. All resolved merge requests will have one of these branches as their target.
"""
targetBranches: [String!]
): MergeRequestConnection ): MergeRequestConnection
""" """
......
...@@ -16717,6 +16717,24 @@ ...@@ -16717,6 +16717,24 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
}, },
{
"name": "targetBranchExists",
"description": "Indicates if the target branch of the merge request exists",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "Boolean",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
},
{ {
"name": "targetProject", "name": "targetProject",
"description": "Target project of the merge request", "description": "Target project of the merge request",
...@@ -23270,12 +23288,29 @@ ...@@ -23270,12 +23288,29 @@
"name": "iid", "name": "iid",
"description": "IID of the merge request, for example `1`", "description": "IID of the merge request, for example `1`",
"type": { "type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "String", "name": "String",
"ofType": null "ofType": null
}
}, },
"defaultValue": null "defaultValue": null
}
],
"type": {
"kind": "OBJECT",
"name": "MergeRequest",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}, },
{
"name": "mergeRequests",
"description": "Merge requests of the project",
"args": [
{ {
"name": "iids", "name": "iids",
"description": "Array of IIDs of merge requests, for example `[1, 2]`", "description": "Array of IIDs of merge requests, for example `[1, 2]`",
...@@ -23293,33 +23328,56 @@ ...@@ -23293,33 +23328,56 @@
} }
}, },
"defaultValue": null "defaultValue": null
} },
], {
"name": "sourceBranches",
"description": "Array of source branch names. All resolved merge requests will have one of these branches as their source.",
"type": { "type": {
"kind": "OBJECT", "kind": "LIST",
"name": "MergeRequest", "name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null "ofType": null
}
}
}, },
"isDeprecated": false, "defaultValue": null
"deprecationReason": null
}, },
{ {
"name": "mergeRequests", "name": "targetBranches",
"description": "Merge requests of the project", "description": "Array of target branch names. All resolved merge requests will have one of these branches as their target.",
"args": [
{
"name": "iid",
"description": "IID of the merge request, for example `1`",
"type": { "type": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "String", "name": "String",
"ofType": null "ofType": null
}
}
}, },
"defaultValue": null "defaultValue": null
}, },
{ {
"name": "iids", "name": "state",
"description": "Array of IIDs of merge requests, for example `[1, 2]`", "description": "A merge request state. If provided, all resolved merge requests will have this state.",
"type": {
"kind": "ENUM",
"name": "MergeRequestState",
"ofType": null
},
"defaultValue": null
},
{
"name": "labels",
"description": "Array of label names. All resolved merge requests will have all of these labels.",
"type": { "type": {
"kind": "LIST", "kind": "LIST",
"name": null, "name": null,
...@@ -886,6 +886,7 @@ Autogenerated return type of MarkAsSpamSnippet ...@@ -886,6 +886,7 @@ Autogenerated return type of MarkAsSpamSnippet
| `state` | MergeRequestState! | State of the merge request | | `state` | MergeRequestState! | State of the merge request |
| `subscribed` | Boolean! | Indicates if the currently logged in user is subscribed to this merge request | | `subscribed` | Boolean! | Indicates if the currently logged in user is subscribed to this merge request |
| `targetBranch` | String! | Target branch of the merge request | | `targetBranch` | String! | Target branch of the merge request |
| `targetBranchExists` | Boolean! | Indicates if the target branch of the merge request exists |
| `targetProject` | Project! | Target project of the merge request | | `targetProject` | Project! | Target project of the merge request |
| `targetProjectId` | Int! | ID of the merge request target project | | `targetProjectId` | Int! | ID of the merge request target project |
| `taskCompletionStatus` | TaskCompletionStatus! | Completion status of tasks | | `taskCompletionStatus` | TaskCompletionStatus! | Completion status of tasks |
......
...@@ -84,7 +84,7 @@ module Gitlab ...@@ -84,7 +84,7 @@ module Gitlab
elsif resolved_type.is_a? Array elsif resolved_type.is_a? Array
# A simple list of rendered types each object being an object to authorize # A simple list of rendered types each object being an object to authorize
resolved_type.select do |single_object_type| resolved_type.select do |single_object_type|
allowed_access?(current_user, single_object_type.object) allowed_access?(current_user, unpromise(single_object_type).object)
end end
else else
raise "Can't authorize #{@field}" raise "Can't authorize #{@field}"
...@@ -113,6 +113,17 @@ module Gitlab ...@@ -113,6 +113,17 @@ module Gitlab
def scalar_type? def scalar_type?
node_type_for_basic_connection(@field.type).kind.scalar? node_type_for_basic_connection(@field.type).kind.scalar?
end end
# Sometimes we get promises, and have to resolve them. The dedicated way
# of doing this (GitlabSchema.after_lazy) is a private framework method,
# and so we use duck-typing interface inference here instead.
def unpromise(maybe_promise)
if maybe_promise.respond_to?(:value) && !maybe_promise.respond_to?(:object)
maybe_promise.value
else
maybe_promise
end
end
end end
end end
end end
......
...@@ -133,6 +133,11 @@ FactoryBot.define do ...@@ -133,6 +133,11 @@ FactoryBot.define do
end end
end end
trait :unique_branches do
source_branch { generate(:branch) }
target_branch { generate(:branch) }
end
trait :with_coverage_reports do trait :with_coverage_reports do
after(:build) do |merge_request| after(:build) do |merge_request|
merge_request.head_pipeline = build( merge_request.head_pipeline = build(
......
...@@ -6,6 +6,24 @@ describe MergeRequestsFinder do ...@@ -6,6 +6,24 @@ describe MergeRequestsFinder do
context "multiple projects with merge requests" do context "multiple projects with merge requests" do
include_context 'MergeRequestsFinder multiple projects with merge requests context' include_context 'MergeRequestsFinder multiple projects with merge requests context'
shared_examples 'scalar or array parameter' do
let(:values) { merge_requests.pluck(attribute) }
let(:params) { {} }
let(:key) { attribute }
it 'takes scalar values' do
found = described_class.new(user, params.merge(key => values.first)).execute
expect(found).to contain_exactly(merge_requests.first)
end
it 'takes array values' do
found = described_class.new(user, params.merge(key => values)).execute
expect(found).to match_array(merge_requests)
end
end
describe '#execute' do describe '#execute' do
it 'filters by scope' do it 'filters by scope' do
params = { scope: 'authored', state: 'opened' } params = { scope: 'authored', state: 'opened' }
...@@ -91,28 +109,56 @@ describe MergeRequestsFinder do ...@@ -91,28 +109,56 @@ describe MergeRequestsFinder do
expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3, merge_request5) expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3, merge_request5)
end end
it 'filters by iid' do describe ':iid parameter' do
params = { project_id: project1.id, iids: merge_request1.iid } it_behaves_like 'scalar or array parameter' do
let(:params) { { project_id: project1.id } }
let(:merge_requests) { [merge_request1, merge_request2] }
let(:key) { :iids }
let(:attribute) { :iid }
end
end
merge_requests = described_class.new(user, params).execute [:source_branch, :target_branch].each do |param|
describe "#{param} parameter" do
let(:merge_requests) { create_list(:merge_request, 2, :unique_branches, source_project: project4, target_project: project4, author: user) }
let(:attribute) { param }
expect(merge_requests).to contain_exactly(merge_request1) it_behaves_like 'scalar or array parameter'
end
end end
it 'filters by source branch' do describe ':label_name parameter' do
params = { source_branch: merge_request2.source_branch } let(:common_labels) { create_list(:label, 3) }
let(:distinct_labels) { create_list(:label, 3) }
merge_requests = described_class.new(user, params).execute let(:merge_requests) do
common_attrs = {
source_project: project1, target_project: project1, author: user
}
distinct_labels.map do |label|
labels = [label, *common_labels]
create(:labeled_merge_request, :closed, labels: labels, **common_attrs)
end
end
expect(merge_requests).to contain_exactly(merge_request2) def find(label_name)
described_class.new(user, label_name: label_name).execute
end end
it 'filters by target branch' do it 'accepts a single label' do
params = { target_branch: merge_request2.target_branch } found = find(distinct_labels.first.title)
common = find(common_labels.first.title)
merge_requests = described_class.new(user, params).execute expect(found).to contain_exactly(merge_requests.first)
expect(common).to match_array(merge_requests)
end
expect(merge_requests).to contain_exactly(merge_request2) it 'accepts an array of labels, all of which must match' do
all_distinct = find(distinct_labels.pluck(:title))
all_common = find(common_labels.pluck(:title))
expect(all_distinct).to be_empty
expect(all_common).to match_array(merge_requests)
end
end end
it 'filters by source project id' do it 'filters by source project id' do
...@@ -158,7 +204,10 @@ describe MergeRequestsFinder do ...@@ -158,7 +204,10 @@ describe MergeRequestsFinder do
merge_requests = described_class.new(user, params).execute merge_requests = described_class.new(user, params).execute
expect(merge_requests).to contain_exactly(merge_request1, merge_request2, merge_request3, merge_request4, merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3, wip_merge_request4) expect(merge_requests).to contain_exactly(
merge_request1, merge_request2, merge_request3, merge_request4,
merge_request5, wip_merge_request1, wip_merge_request2, wip_merge_request3,
wip_merge_request4)
end end
it 'adds wip to scalar params' do it 'adds wip to scalar params' do
......
...@@ -6,18 +6,43 @@ describe Resolvers::MergeRequestsResolver do ...@@ -6,18 +6,43 @@ describe Resolvers::MergeRequestsResolver do
include GraphqlHelpers include GraphqlHelpers
let_it_be(:project) { create(:project, :repository) } let_it_be(:project) { create(:project, :repository) }
let_it_be(:merge_request_1) { create(:merge_request, :simple, source_project: project, target_project: project) } let_it_be(:current_user) { create(:user) }
let_it_be(:merge_request_2) { create(:merge_request, :rebased, source_project: project, target_project: project) } let_it_be(:common_attrs) { { author: current_user, source_project: project, target_project: project } }
let_it_be(:merge_request_1) { create(:merge_request, :simple, **common_attrs) }
let_it_be(:merge_request_2) { create(:merge_request, :rebased, **common_attrs) }
let_it_be(:merge_request_3) { create(:merge_request, :unique_branches, **common_attrs) }
let_it_be(:merge_request_4) { create(:merge_request, :unique_branches, :locked, **common_attrs) }
let_it_be(:merge_request_5) { create(:merge_request, :simple, :locked, **common_attrs) }
let_it_be(:merge_request_6) { create(:labeled_merge_request, :unique_branches, labels: create_list(:label, 2), **common_attrs) }
let_it_be(:other_project) { create(:project, :repository) } let_it_be(:other_project) { create(:project, :repository) }
let_it_be(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) } let_it_be(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) }
let(:iid_1) { merge_request_1.iid } let(:iid_1) { merge_request_1.iid }
let(:iid_2) { merge_request_2.iid } let(:iid_2) { merge_request_2.iid }
let(:other_iid) { other_merge_request.iid } let(:other_iid) { other_merge_request.iid }
before do
project.add_developer(current_user)
end
describe '#resolve' do describe '#resolve' do
context 'no arguments' do
it 'returns all merge requests' do
result = resolve_mr(project, {})
expect(result).to contain_exactly(merge_request_1, merge_request_2, merge_request_3, merge_request_4, merge_request_5, merge_request_6)
end
it 'returns only merge requests that the current user can see' do
result = resolve_mr(project, {}, user: build(:user))
expect(result).to be_empty
end
end
context 'by iid alone' do
it 'batch-resolves by target project full path and individual IID' do it 'batch-resolves by target project full path and individual IID' do
result = batch_sync(max_queries: 2) do result = batch_sync(max_queries: 2) do
resolve_mr(project, iid: iid_1) + resolve_mr(project, iid: iid_2) [iid_1, iid_2].map { |iid| resolve_mr_single(project, iid) }
end end
expect(result).to contain_exactly(merge_request_1, merge_request_2) expect(result).to contain_exactly(merge_request_1, merge_request_2)
...@@ -33,18 +58,18 @@ describe Resolvers::MergeRequestsResolver do ...@@ -33,18 +58,18 @@ describe Resolvers::MergeRequestsResolver do
it 'can batch-resolve merge requests from different projects' do it 'can batch-resolve merge requests from different projects' do
result = batch_sync(max_queries: 3) do result = batch_sync(max_queries: 3) do
resolve_mr(project, iid: iid_1) + resolve_mr(project, iids: iid_1) +
resolve_mr(project, iid: iid_2) + resolve_mr(project, iids: iid_2) +
resolve_mr(other_project, iid: other_iid) resolve_mr(other_project, iids: 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 be empty' do it 'resolves an unknown iid to be empty' do
result = batch_sync { resolve_mr(project, iid: -1) } result = batch_sync { resolve_mr_single(project, -1) }
expect(result.compact).to be_empty expect(result).to be_nil
end end
it 'resolves empty iids to be empty' do it 'resolves empty iids to be empty' do
...@@ -53,14 +78,92 @@ describe Resolvers::MergeRequestsResolver do ...@@ -53,14 +78,92 @@ describe Resolvers::MergeRequestsResolver do
expect(result).to be_empty expect(result).to be_empty
end end
it 'resolves an unknown project to be nil when single' do
result = batch_sync { resolve_mr_single(nil, iid_1) }
expect(result).to be_nil
end
it 'resolves an unknown project to be empty' do it 'resolves an unknown project to be empty' do
result = batch_sync { resolve_mr(nil, iid: iid_1) } result = batch_sync { resolve_mr(nil, iids: [iid_1]) }
expect(result).to be_empty
end
end
context 'by source branches' do
it 'takes one argument' do
result = resolve_mr(project, source_branch: [merge_request_3.source_branch])
expect(result).to contain_exactly(merge_request_3)
end
it 'takes more than one argument' do
mrs = [merge_request_3, merge_request_4]
branches = mrs.map(&:source_branch)
result = resolve_mr(project, source_branch: branches )
expect(result).to match_array(mrs)
end
end
expect(result.compact).to be_empty context 'by target branches' do
it 'takes one argument' do
result = resolve_mr(project, target_branch: [merge_request_3.target_branch])
expect(result).to contain_exactly(merge_request_3)
end
it 'takes more than one argument' do
mrs = [merge_request_3, merge_request_4]
branches = mrs.map(&:target_branch)
result = resolve_mr(project, target_branch: branches )
expect(result.compact).to match_array(mrs)
end
end
context 'by state' do
it 'takes one argument' do
result = resolve_mr(project, state: 'locked')
expect(result).to contain_exactly(merge_request_4, merge_request_5)
end
end
context 'by label' do
let_it_be(:label) { merge_request_6.labels.first }
let_it_be(:with_label) { create(:labeled_merge_request, :closed, labels: [label], **common_attrs) }
it 'takes one argument' do
result = resolve_mr(project, label_name: [label.title])
expect(result).to contain_exactly(merge_request_6, with_label)
end
it 'takes multiple arguments, with semantics of ALL MUST MATCH' do
result = resolve_mr(project, label_name: merge_request_6.labels.map(&:title))
expect(result).to contain_exactly(merge_request_6)
end
end
describe 'combinations' do
it 'requires all filters' do
create(:merge_request, :closed, source_project: project, target_project: project, source_branch: merge_request_4.source_branch)
result = resolve_mr(project, source_branch: [merge_request_4.source_branch], state: 'locked')
expect(result.compact).to contain_exactly(merge_request_4)
end end
end end
end
def resolve_mr_single(project, iid)
resolve_mr(project, { iids: iid }, resolver: described_class.single)
end
def resolve_mr(project, args) def resolve_mr(project, args, resolver: described_class, user: current_user)
resolve(described_class, obj: project, args: args) resolve(resolver, obj: project, args: args, ctx: { current_user: user })
end end
end end
...@@ -19,7 +19,8 @@ describe GitlabSchema.types['MergeRequest'] do ...@@ -19,7 +19,8 @@ describe GitlabSchema.types['MergeRequest'] do
force_remove_source_branch merge_status in_progress_merge_commit_sha force_remove_source_branch merge_status in_progress_merge_commit_sha
merge_error allow_collaboration should_be_rebased rebase_commit_sha merge_error allow_collaboration should_be_rebased rebase_commit_sha
rebase_in_progress merge_commit_message default_merge_commit_message rebase_in_progress merge_commit_message default_merge_commit_message
merge_ongoing source_branch_exists mergeable_discussions_state web_url merge_ongoing mergeable_discussions_state web_url
source_branch_exists target_branch_exists
upvotes downvotes head_pipeline pipelines task_completion_status upvotes downvotes head_pipeline pipelines task_completion_status
milestone assignees participants subscribed labels discussion_locked time_estimate milestone assignees participants subscribed labels discussion_locked time_estimate
total_time_spent reference total_time_spent reference
......
...@@ -45,18 +45,32 @@ describe GitlabSchema.types['Project'] do ...@@ -45,18 +45,32 @@ describe GitlabSchema.types['Project'] do
it { is_expected.to have_graphql_resolver(Resolvers::IssuesResolver) } it { is_expected.to have_graphql_resolver(Resolvers::IssuesResolver) }
end end
describe 'merge_requests field' do describe 'merge_request field' do
subject { described_class.fields['mergeRequest'] } subject { described_class.fields['mergeRequest'] }
it { is_expected.to have_graphql_type(Types::MergeRequestType) } it { is_expected.to have_graphql_type(Types::MergeRequestType) }
it { is_expected.to have_graphql_resolver(Resolvers::MergeRequestsResolver.single) } it { is_expected.to have_graphql_resolver(Resolvers::MergeRequestsResolver.single) }
it { is_expected.to have_graphql_arguments(:iid) }
end end
describe 'merge_request field' do describe 'merge_requests field' do
subject { described_class.fields['mergeRequests'] } subject { described_class.fields['mergeRequests'] }
it { is_expected.to have_graphql_type(Types::MergeRequestType.connection_type) } it { is_expected.to have_graphql_type(Types::MergeRequestType.connection_type) }
it { is_expected.to have_graphql_resolver(Resolvers::MergeRequestsResolver) } it { is_expected.to have_graphql_resolver(Resolvers::MergeRequestsResolver) }
it do
is_expected.to have_graphql_arguments(:iids,
:source_branches,
:target_branches,
:state,
:labels,
:before,
:after,
:first,
:last
)
end
end end
describe 'snippets field' do describe 'snippets field' do
......
# frozen_string_literal: true
require 'spec_helper'
describe 'getting merge request listings nested in a project' do
include GraphqlHelpers
let_it_be(:project) { create(:project, :repository, :public) }
let_it_be(:current_user) { create(:user) }
let_it_be(:label) { create(:label) }
let_it_be(:merge_request_a) { create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) }
let_it_be(:merge_request_b) { create(:merge_request, :closed, :unique_branches, source_project: project) }
let_it_be(:merge_request_c) { create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) }
let_it_be(:merge_request_d) { create(:merge_request, :locked, :unique_branches, source_project: project) }
let(:results) { graphql_data.dig('project', 'mergeRequests', 'nodes') }
let(:search_params) { nil }
def query_merge_requests(fields)
graphql_query_for(
:project,
{ full_path: project.full_path },
query_graphql_field(:merge_requests, search_params, [
query_graphql_field(:nodes, nil, fields)
])
)
end
let(:query) do
query_merge_requests(all_graphql_fields_for('MergeRequest', max_depth: 1))
end
it_behaves_like 'a working graphql query' do
before do
post_graphql(query, current_user: current_user)
end
end
# The following tests are needed to guarantee that we have correctly annotated
# all the gitaly calls. Selecting combinations of fields may mask this due to
# memoization.
context 'requesting a single field' do
let(:fresh_mr) { create(:merge_request, :unique_branches, source_project: project) }
let(:search_params) { { iids: [fresh_mr.iid.to_s] } }
before do
project.repository.expire_branches_cache
end
context 'selecting any single scalar field' do
where(:field) do
scalar_fields_of('MergeRequest').map { |name| [name] }
end
with_them do
it_behaves_like 'a working graphql query' do
let(:query) do
query_merge_requests([:iid, field].uniq)
end
before do
post_graphql(query, current_user: current_user)
end
it 'selects the correct MR' do
expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s))
end
end
end
end
context 'selecting any single nested field' do
where(:field, :subfield, :is_connection) do
nested_fields_of('MergeRequest').flat_map do |name, field|
type = field_type(field)
is_connection = type.name.ends_with?('Connection')
type = field_type(type.fields['nodes']) if is_connection
type.fields
.select { |_, field| !nested_fields?(field) && !required_arguments?(field) }
.map(&:first)
.map { |subfield| [name, subfield, is_connection] }
end
end
with_them do
it_behaves_like 'a working graphql query' do
let(:query) do
fld = is_connection ? query_graphql_field(:nodes, nil, [subfield]) : subfield
query_merge_requests([:iid, query_graphql_field(field, nil, [fld])])
end
before do
post_graphql(query, current_user: current_user)
end
it 'selects the correct MR' do
expect(results).to contain_exactly(a_hash_including('iid' => fresh_mr.iid.to_s))
end
end
end
end
end
shared_examples 'searching with parameters' do
let(:expected) do
mrs.map { |mr| a_hash_including('iid' => mr.iid.to_s, 'title' => mr.title) }
end
it 'finds the right mrs' do
post_graphql(query, current_user: current_user)
expect(results).to match_array(expected)
end
end
context 'there are no search params' do
let(:search_params) { nil }
let(:mrs) { [merge_request_a, merge_request_b, merge_request_c, merge_request_d] }
it_behaves_like 'searching with parameters'
end
context 'the search params do not match anything' do
let(:search_params) { { iids: %w(foo bar baz) } }
let(:mrs) { [] }
it_behaves_like 'searching with parameters'
end
context 'searching by iids' do
let(:search_params) { { iids: mrs.map(&:iid).map(&:to_s) } }
let(:mrs) { [merge_request_a, merge_request_c] }
it_behaves_like 'searching with parameters'
end
context 'searching by state' do
let(:search_params) { { state: :closed } }
let(:mrs) { [merge_request_b, merge_request_c] }
it_behaves_like 'searching with parameters'
end
context 'searching by source_branch' do
let(:search_params) { { source_branches: mrs.map(&:source_branch) } }
let(:mrs) { [merge_request_b, merge_request_c] }
it_behaves_like 'searching with parameters'
end
context 'searching by target_branch' do
let(:search_params) { { target_branches: mrs.map(&:target_branch) } }
let(:mrs) { [merge_request_a, merge_request_d] }
it_behaves_like 'searching with parameters'
end
context 'searching by label' do
let(:search_params) { { labels: [label.title] } }
let(:mrs) { [merge_request_a, merge_request_c] }
it_behaves_like 'searching with parameters'
end
context 'searching by combination' do
let(:search_params) { { state: :closed, labels: [label.title] } }
let(:mrs) { [merge_request_c] }
it_behaves_like 'searching with parameters'
end
end
...@@ -304,6 +304,22 @@ module GraphqlHelpers ...@@ -304,6 +304,22 @@ module GraphqlHelpers
graphql_data.fetch(GraphqlHelpers.fieldnamerize(mutation_name)) graphql_data.fetch(GraphqlHelpers.fieldnamerize(mutation_name))
end end
def scalar_fields_of(type_name)
GitlabSchema.types[type_name].fields.map do |name, field|
next if nested_fields?(field) || required_arguments?(field)
name
end.compact
end
def nested_fields_of(type_name)
GitlabSchema.types[type_name].fields.map do |name, field|
next if !nested_fields?(field) || required_arguments?(field)
[name, field]
end.compact
end
def nested_fields?(field) def nested_fields?(field)
!scalar?(field) && !enum?(field) !scalar?(field) && !enum?(field)
end end
......
...@@ -45,11 +45,32 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests ...@@ -45,11 +45,32 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests
allow_gitaly_n_plus_1 { create(:project, group: subgroup) } allow_gitaly_n_plus_1 { create(:project, group: subgroup) }
end end
let!(:merge_request1) { create(:merge_request, assignees: [user], author: user, source_project: project2, target_project: project1, target_branch: 'merged-target') } let!(:merge_request1) do
let!(:merge_request2) { create(:merge_request, :conflict, assignees: [user], author: user, source_project: project2, target_project: project1, state: 'closed') } create(:merge_request, assignees: [user], author: user,
let!(:merge_request3) { create(:merge_request, :simple, author: user, assignees: [user2], source_project: project2, target_project: project2, state: 'locked', title: 'thing WIP thing') } source_project: project2, target_project: project1,
let!(:merge_request4) { create(:merge_request, :simple, author: user, source_project: project3, target_project: project3, title: 'WIP thing') } target_branch: 'merged-target')
let!(:merge_request5) { create(:merge_request, :simple, author: user, source_project: project4, target_project: project4, title: '[WIP]') } end
let!(:merge_request2) do
create(:merge_request, :conflict, assignees: [user], author: user,
source_project: project2, target_project: project1,
state: 'closed')
end
let!(:merge_request3) do
create(:merge_request, :simple, author: user, assignees: [user2],
source_project: project2, target_project: project2,
state: 'locked',
title: 'thing WIP thing')
end
let!(:merge_request4) do
create(:merge_request, :simple, author: user,
source_project: project3, target_project: project3,
title: 'WIP thing')
end
let_it_be(:merge_request5) do
create(:merge_request, :simple, author: user,
source_project: project4, target_project: project4,
title: '[WIP]')
end
before do before do
project1.add_maintainer(user) project1.add_maintainer(user)
......
...@@ -22,7 +22,7 @@ RSpec.shared_examples 'resolving an issuable in GraphQL' do |type| ...@@ -22,7 +22,7 @@ RSpec.shared_examples 'resolving an issuable in GraphQL' do |type|
.with(full_path: parent.full_path) .with(full_path: parent.full_path)
.and_return(resolved_parent) .and_return(resolved_parent)
expect(resolver_class).to receive(:new) expect(resolver_class.single).to receive(:new)
.with(object: resolved_parent, context: context, field: nil) .with(object: resolved_parent, context: context, field: nil)
.and_call_original .and_call_original
...@@ -41,7 +41,7 @@ RSpec.shared_examples 'resolving an issuable in GraphQL' do |type| ...@@ -41,7 +41,7 @@ RSpec.shared_examples 'resolving an issuable in GraphQL' do |type|
it 'returns nil if issuable is not found' do it 'returns nil if issuable is not found' do
result = mutation.resolve_issuable(type: type, parent_path: parent.full_path, iid: "100") result = mutation.resolve_issuable(type: type, parent_path: parent.full_path, iid: "100")
result = type == :merge_request ? result.sync : result result = result.respond_to?(:sync) ? result.sync : result
expect(result).to be_nil expect(result).to be_nil
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