diff --git a/app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql b/app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql
index 8a5613c75d20d6d5d5d45d0866fb3c97fd0864fd..e0de6d12b13e5d564cbded353fa41c5141b13c77 100644
--- a/app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql
+++ b/app/assets/javascripts/releases/graphql/fragments/release.fragment.graphql
@@ -1,5 +1,6 @@
fragment Release on Release {
__typename
+ id
name
tagName
tagPath
diff --git a/app/assets/javascripts/releases/graphql/fragments/release_for_editing.fragment.graphql b/app/assets/javascripts/releases/graphql/fragments/release_for_editing.fragment.graphql
index 1823a32735058273cd6ef07499d89769da3cdc06..236d266a40a8b6aaa158069b5ce83b6ee3c6fe15 100644
--- a/app/assets/javascripts/releases/graphql/fragments/release_for_editing.fragment.graphql
+++ b/app/assets/javascripts/releases/graphql/fragments/release_for_editing.fragment.graphql
@@ -1,4 +1,5 @@
fragment ReleaseForEditing on Release {
+ id
name
tagName
description
diff --git a/app/assets/javascripts/releases/graphql/mutations/create_release.mutation.graphql b/app/assets/javascripts/releases/graphql/mutations/create_release.mutation.graphql
index 56bfe7c23d6eff2b5606b0a3c72e8a28687a63e8..7344772adb98dbbe8a86c2b1b4419a53a8a24bbb 100644
--- a/app/assets/javascripts/releases/graphql/mutations/create_release.mutation.graphql
+++ b/app/assets/javascripts/releases/graphql/mutations/create_release.mutation.graphql
@@ -1,6 +1,7 @@
mutation createRelease($input: ReleaseCreateInput!) {
releaseCreate(input: $input) {
release {
+ id
links {
selfUrl
}
diff --git a/app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql b/app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql
index bda7ac52a478f958d243f2757f9c12aee1d1996b..61a06f268bd4ca12773408e8d5eb7cdd6751f93c 100644
--- a/app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql
+++ b/app/assets/javascripts/releases/graphql/queries/all_releases.query.graphql
@@ -13,6 +13,7 @@ query allReleases(
__typename
nodes {
__typename
+ id
name
tagName
tagPath
diff --git a/app/graphql/resolvers/milestones_resolver.rb b/app/graphql/resolvers/milestones_resolver.rb
index dc6d781f584be3f93cc0cfcc4e3d5bec47875928..25ff783b408bab1243d16737851ddc4bd6df7552 100644
--- a/app/graphql/resolvers/milestones_resolver.rb
+++ b/app/graphql/resolvers/milestones_resolver.rb
@@ -4,6 +4,11 @@ module Resolvers
class MilestonesResolver < BaseResolver
include Gitlab::Graphql::Authorize::AuthorizeResource
include TimeFrameArguments
+ include LooksAhead
+
+ # authorize before resolution
+ authorize :read_milestone
+ authorizes_object!
argument :ids, [GraphQL::Types::ID],
required: false,
@@ -34,12 +39,10 @@ class MilestonesResolver < BaseResolver
NON_STABLE_CURSOR_SORTS = %i[expired_last_due_date_asc expired_last_due_date_desc].freeze
- def resolve(**args)
+ def resolve_with_lookahead(**args)
validate_timeframe_params!(args)
- authorize!
-
- milestones = MilestonesFinder.new(milestones_finder_params(args)).execute
+ milestones = apply_lookahead(MilestonesFinder.new(milestones_finder_params(args)).execute)
if non_stable_cursor_sort?(args[:sort])
offset_pagination(milestones)
@@ -50,6 +53,12 @@ def resolve(**args)
private
+ def preloads
+ {
+ releases: :releases
+ }
+ end
+
def milestones_finder_params(args)
{
ids: parse_gids(args[:ids]),
@@ -69,12 +78,6 @@ def parent_id_parameters(args)
raise NotImplementedError
end
- # MilestonesFinder does not check for current_user permissions,
- # so for now we need to keep it here.
- def authorize!
- Ability.allowed?(context[:current_user], :read_milestone, parent) || raise_resource_not_available_error!
- end
-
def parse_gids(gids)
gids&.map { |gid| GitlabSchema.parse_gid(gid, expected_type: Milestone).model_id }
end
diff --git a/app/graphql/types/milestone_type.rb b/app/graphql/types/milestone_type.rb
index 18e4a5d33e3942e8e0636646486e2bb1a3d9f651..7741fd723f0e93a6f8b6a67ccf473fcf7130f4c0 100644
--- a/app/graphql/types/milestone_type.rb
+++ b/app/graphql/types/milestone_type.rb
@@ -59,6 +59,10 @@ class MilestoneType < BaseObject
field :stats, Types::MilestoneStatsType, null: true,
description: 'Milestone statistics.'
+ field :releases, ::Types::ReleaseType.connection_type,
+ null: true,
+ description: 'Releases associated with this milestone.'
+
def stats
milestone
end
diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb
index 01b1a71896a7514fbdeb387212c830c0984078d4..bc3774bcc1d5fb9c112f87e763d683a573deea1e 100644
--- a/app/graphql/types/query_type.rb
+++ b/app/graphql/types/query_type.rb
@@ -52,6 +52,7 @@ class QueryType < ::Types::BaseObject
field :milestone, ::Types::MilestoneType,
null: true,
+ extras: [:lookahead],
description: 'Find a milestone.' do
argument :id, ::Types::GlobalIDType[Milestone], required: true, description: 'Find a milestone by its ID.'
end
@@ -156,8 +157,9 @@ def merge_request(id:)
GitlabSchema.find_by_gid(id)
end
- def milestone(id:)
- GitlabSchema.find_by_gid(id)
+ def milestone(id:, lookahead:)
+ preloads = [:releases] if lookahead.selects?(:releases)
+ Gitlab::Graphql::Loaders::BatchModelLoader.new(id.model_class, id.model_id, preloads).find
end
def container_repository(id:)
diff --git a/app/graphql/types/release_type.rb b/app/graphql/types/release_type.rb
index 95b6b43bb46cdef6d08958d265a68837037a659a..43dc0c4ce85c2ea6acaefe9dac9afd9f335be6ea 100644
--- a/app/graphql/types/release_type.rb
+++ b/app/graphql/types/release_type.rb
@@ -13,6 +13,9 @@ class ReleaseType < BaseObject
present_using ReleasePresenter
+ field :id, ::Types::GlobalIDType[Release],
+ null: false,
+ description: 'Global ID of the release.'
field :assets, Types::ReleaseAssetsType, null: true, method: :itself,
description: 'Assets of the release.'
field :created_at, Types::TimeType, null: true,
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 742383e411a18b80ac0a3493fcc9678d58f4f80a..9860abcb8e6d3b9c889814d0271032100b6d28a1 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -13854,6 +13854,7 @@ Represents a milestone.
| <a id="milestoneid"></a>`id` | [`ID!`](#id) | ID of the milestone. |
| <a id="milestoneiid"></a>`iid` | [`ID!`](#id) | Internal ID of the milestone. |
| <a id="milestoneprojectmilestone"></a>`projectMilestone` | [`Boolean!`](#boolean) | Indicates if milestone is at project level. |
+| <a id="milestonereleases"></a>`releases` | [`ReleaseConnection`](#releaseconnection) | Releases associated with this milestone. (see [Connections](#connections)) |
| <a id="milestonestartdate"></a>`startDate` | [`Time`](#time) | Timestamp of the milestone start date. |
| <a id="milestonestate"></a>`state` | [`MilestoneStateEnum!`](#milestonestateenum) | State of the milestone. |
| <a id="milestonestats"></a>`stats` | [`MilestoneStats`](#milestonestats) | Milestone statistics. |
@@ -15851,6 +15852,7 @@ Represents a release.
| <a id="releasedescription"></a>`description` | [`String`](#string) | Description (also known as "release notes") of the release. |
| <a id="releasedescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. |
| <a id="releaseevidences"></a>`evidences` | [`ReleaseEvidenceConnection`](#releaseevidenceconnection) | Evidence for the release. (see [Connections](#connections)) |
+| <a id="releaseid"></a>`id` | [`ReleaseID!`](#releaseid) | Global ID of the release. |
| <a id="releaselinks"></a>`links` | [`ReleaseLinks`](#releaselinks) | Links of the release. |
| <a id="releasemilestones"></a>`milestones` | [`MilestoneConnection`](#milestoneconnection) | Milestones associated to the release. (see [Connections](#connections)) |
| <a id="releasename"></a>`name` | [`String`](#string) | Name of the release. |
@@ -20133,6 +20135,12 @@ A `ProjectID` is a global ID. It is encoded as a string.
An example `ProjectID` is: `"gid://gitlab/Project/1"`.
+### `ReleaseID`
+
+A `ReleaseID` is a global ID. It is encoded as a string.
+
+An example `ReleaseID` is: `"gid://gitlab/Release/1"`.
+
### `ReleasesLinkID`
A `ReleasesLinkID` is a global ID. It is encoded as a string.
diff --git a/ee/app/graphql/resolvers/timebox_report_resolver.rb b/ee/app/graphql/resolvers/timebox_report_resolver.rb
index c610f29bebf81741ff9a4b7c6a2937a0ae545fce..ce4d0afc6cfb331118f0ceb96314ed9404a26afc 100644
--- a/ee/app/graphql/resolvers/timebox_report_resolver.rb
+++ b/ee/app/graphql/resolvers/timebox_report_resolver.rb
@@ -18,9 +18,7 @@ def resolve(**args)
response = TimeboxReportService.new(timebox, project_scopes).execute
- raise GraphQL::ExecutionError, response.message if response.error?
-
- response.payload
+ response.payload if response.success?
end
private
diff --git a/ee/spec/graphql/resolvers/timebox_report_resolver_spec.rb b/ee/spec/graphql/resolvers/timebox_report_resolver_spec.rb
index 2589e7ab3e7f63aafdb773222e11fcc488d268da..1cbf859e0ef66143cfc046630ec5e68276db9a9a 100644
--- a/ee/spec/graphql/resolvers/timebox_report_resolver_spec.rb
+++ b/ee/spec/graphql/resolvers/timebox_report_resolver_spec.rb
@@ -70,11 +70,7 @@
stub_const('TimeboxReportService::EVENT_COUNT_LIMIT', 1)
end
- it 'generates a GraphQL error' do
- expect_graphql_error_to_be_created(GraphQL::ExecutionError, 'Burnup chart could not be generated due to too many events') do
- subject
- end
- end
+ it { is_expected.to be_nil }
end
end
diff --git a/ee/spec/requests/api/graphql/milestone_spec.rb b/ee/spec/requests/api/graphql/milestone_spec.rb
index 34f69331e122842f2df8d93c255023ba7b50ea0f..0e1ffb693ae39abbf2a4646d9994db3cf7007489 100644
--- a/ee/spec/requests/api/graphql/milestone_spec.rb
+++ b/ee/spec/requests/api/graphql/milestone_spec.rb
@@ -46,10 +46,10 @@
stub_licensed_features(milestone_charts: false)
end
- it 'returns an error' do
+ it 'returns nil' do
post_graphql(query, current_user: current_user)
- expect(graphql_errors).to include(a_hash_including('message' => 'Milestone does not support burnup charts'))
+ expect(graphql_data_at(:milestone, :report)).to be_nil
end
end
diff --git a/lib/gitlab/graphql/authorize/authorize_resource.rb b/lib/gitlab/graphql/authorize/authorize_resource.rb
index dc49c806398a8a9b9fcd69bdda268b25d81411a5..884fc85c4ecb341e63b9e7157fb579c9ebf68f3a 100644
--- a/lib/gitlab/graphql/authorize/authorize_resource.rb
+++ b/lib/gitlab/graphql/authorize/authorize_resource.rb
@@ -15,11 +15,7 @@ def required_permissions
# If the `#authorize` call is used on multiple classes, we add the
# permissions specified on a subclass, to the ones that were specified
# on its superclass.
- @required_permissions ||= if respond_to?(:superclass) && superclass.respond_to?(:required_permissions)
- superclass.required_permissions.dup
- else
- []
- end
+ @required_permissions ||= call_superclass_method(:required_permissions, []).dup
end
def authorize(*permissions)
@@ -27,6 +23,8 @@ def authorize(*permissions)
end
def authorizes_object?
+ return true if call_superclass_method(:authorizes_object?, false)
+
defined?(@authorizes_object) ? @authorizes_object : false
end
@@ -37,6 +35,14 @@ def authorizes_object!
def raise_resource_not_available_error!(msg = RESOURCE_ACCESS_ERROR)
raise ::Gitlab::Graphql::Errors::ResourceNotAvailable, msg
end
+
+ private
+
+ def call_superclass_method(method_name, or_else)
+ return or_else unless respond_to?(:superclass) && superclass.respond_to?(method_name)
+
+ superclass.send(method_name) # rubocop: disable GitlabSecurity/PublicSend
+ end
end
def find_object(*args)
diff --git a/lib/gitlab/graphql/loaders/batch_model_loader.rb b/lib/gitlab/graphql/loaders/batch_model_loader.rb
index 805864cdd4c65f9556e64e449a8052a99fe22792..41c3af339092b5120505c622a6a52f615e595f08 100644
--- a/lib/gitlab/graphql/loaders/batch_model_loader.rb
+++ b/lib/gitlab/graphql/loaders/batch_model_loader.rb
@@ -4,20 +4,27 @@ module Gitlab
module Graphql
module Loaders
class BatchModelLoader
- attr_reader :model_class, :model_id
+ attr_reader :model_class, :model_id, :preloads
- def initialize(model_class, model_id)
+ def initialize(model_class, model_id, preloads = nil)
@model_class = model_class
@model_id = model_id
+ @preloads = preloads || []
end
# rubocop: disable CodeReuse/ActiveRecord
def find
- BatchLoader::GraphQL.for(model_id.to_i).batch(key: model_class) do |ids, loader, args|
+ BatchLoader::GraphQL.for([model_id.to_i, preloads]).batch(key: model_class) do |for_params, loader, args|
model = args[:key]
+ keys_by_id = for_params.group_by(&:first)
+ ids = for_params.map(&:first)
+ preloads = for_params.flat_map(&:second).uniq
results = model.where(id: ids)
+ results = results.preload(*preloads) unless preloads.empty?
- results.each { |record| loader.call(record.id, record) }
+ results.each do |record|
+ keys_by_id.fetch(record.id, []).each { |k| loader.call(k, record) }
+ end
end
end
# rubocop: enable CodeReuse/ActiveRecord
diff --git a/spec/graphql/features/authorization_spec.rb b/spec/graphql/features/authorization_spec.rb
index 1d518e20da7a45eb6bc98d08ba38a042b2009326..5ae497f9d37a3c5031182a7954f0bb366f621214 100644
--- a/spec/graphql/features/authorization_spec.rb
+++ b/spec/graphql/features/authorization_spec.rb
@@ -179,7 +179,7 @@
describe 'type and field authorizations together' do
let(:authorizing_object) { anything }
let(:permission_1) { permission_collection.first }
- let(:permission_2) { permission_collection.last }
+ let(:permission_2) { permission_collection.second }
let(:type) do
type_factory do |type|
@@ -224,6 +224,55 @@
include_examples 'authorization with a collection of permissions'
end
+ context 'when the resolver is a subclass of one that authorizes the object' do
+ let(:permission_object_one) { be_nil }
+ let(:permission_object_two) { be_nil }
+ let(:parent) do
+ parent = Class.new(Resolvers::BaseResolver)
+ parent.include(::Gitlab::Graphql::Authorize::AuthorizeResource)
+ parent.authorizes_object!
+ parent.authorize permission_1
+ parent
+ end
+
+ let(:resolver) do
+ simple_resolver(test_object, base_class: parent)
+ end
+
+ include_examples 'authorization with a collection of permissions'
+ end
+
+ context 'when the resolver is a subclass of one that authorizes the object, extra permission' do
+ let(:permission_object_one) { be_nil }
+ let(:permission_object_two) { be_nil }
+ let(:parent) do
+ parent = Class.new(Resolvers::BaseResolver)
+ parent.include(::Gitlab::Graphql::Authorize::AuthorizeResource)
+ parent.authorizes_object!
+ parent.authorize permission_1
+ parent
+ end
+
+ let(:resolver) do
+ resolver = simple_resolver(test_object, base_class: parent)
+ resolver.include(::Gitlab::Graphql::Authorize::AuthorizeResource)
+ resolver.authorize permission_2
+ resolver
+ end
+
+ context 'when the field does not define any permissions' do
+ let(:query_type) do
+ query_factory do |query|
+ query.field :item, type,
+ null: true,
+ resolver: resolver
+ end
+ end
+
+ include_examples 'authorization with a collection of permissions'
+ end
+ end
+
context 'when the resolver does not authorize the object, but instead calls authorized_find!' do
let(:permission_object_one) { test_object }
let(:permission_object_two) { be_nil }
diff --git a/spec/graphql/resolvers/group_milestones_resolver_spec.rb b/spec/graphql/resolvers/group_milestones_resolver_spec.rb
index 7abc779a63c0d247971ea598cf1d7ccd05b0daea..3d0c4a9d7cb9ed15effd859121c1e81a217f55b8 100644
--- a/spec/graphql/resolvers/group_milestones_resolver_spec.rb
+++ b/spec/graphql/resolvers/group_milestones_resolver_spec.rb
@@ -126,16 +126,6 @@ def args(**arguments)
end
end
- context 'when user cannot read milestones' do
- it 'generates an error' do
- unauthorized_user = create(:user)
-
- expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do
- resolve_group_milestones({}, { current_user: unauthorized_user })
- end
- end
- end
-
context 'when including descendant milestones in a public group' do
let_it_be(:group) { create(:group, :public) }
diff --git a/spec/graphql/resolvers/project_milestones_resolver_spec.rb b/spec/graphql/resolvers/project_milestones_resolver_spec.rb
index 2cf490c2b6a2a83c5dd3a8ae42ceb9c79932abc4..d99a3a40a6c2a502df632277f6a2022403c8266b 100644
--- a/spec/graphql/resolvers/project_milestones_resolver_spec.rb
+++ b/spec/graphql/resolvers/project_milestones_resolver_spec.rb
@@ -172,15 +172,5 @@ def resolve_project_milestones(args = {}, context = { current_user: current_user
resolve_project_milestones(containing_date: t)
end
end
-
- context 'when user cannot read milestones' do
- it 'generates an error' do
- unauthorized_user = create(:user)
-
- expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do
- resolve_project_milestones({}, { current_user: unauthorized_user })
- end
- end
- end
end
end
diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb
index 9d02f061435ba0ffc000d2616d7213235a819af9..e701ec483a33c16b64b580150ce4a45e9f57959b 100644
--- a/spec/graphql/types/base_field_spec.rb
+++ b/spec/graphql/types/base_field_spec.rb
@@ -3,6 +3,91 @@
require 'spec_helper'
RSpec.describe Types::BaseField do
+ describe 'authorized?' do
+ let(:object) { double }
+ let(:current_user) { nil }
+ let(:ctx) { { current_user: current_user } }
+
+ it 'defaults to true' do
+ field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true)
+
+ expect(field).to be_authorized(object, nil, ctx)
+ end
+
+ it 'tests the field authorization, if provided' do
+ field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, authorize: :foo)
+
+ expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false)
+
+ expect(field).not_to be_authorized(object, nil, ctx)
+ end
+
+ it 'tests the field authorization, if provided, when it succeeds' do
+ field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, authorize: :foo)
+
+ expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true)
+
+ expect(field).to be_authorized(object, nil, ctx)
+ end
+
+ it 'only tests the resolver authorization if it authorizes_object?' do
+ resolver = Class.new
+
+ field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
+ resolver_class: resolver)
+
+ expect(field).to be_authorized(object, nil, ctx)
+ end
+
+ it 'tests the resolver authorization, if provided' do
+ resolver = Class.new do
+ include Gitlab::Graphql::Authorize::AuthorizeResource
+
+ authorizes_object!
+ end
+
+ field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
+ resolver_class: resolver)
+
+ expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false)
+
+ expect(field).not_to be_authorized(object, nil, ctx)
+ end
+
+ it 'tests field authorization before resolver authorization, when field auth fails' do
+ resolver = Class.new do
+ include Gitlab::Graphql::Authorize::AuthorizeResource
+
+ authorizes_object!
+ end
+
+ field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
+ authorize: :foo,
+ resolver_class: resolver)
+
+ expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false)
+
+ expect(field).not_to be_authorized(object, nil, ctx)
+ end
+
+ it 'tests field authorization before resolver authorization, when field auth succeeds' do
+ resolver = Class.new do
+ include Gitlab::Graphql::Authorize::AuthorizeResource
+
+ authorizes_object!
+ end
+
+ field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
+ authorize: :foo,
+ resolver_class: resolver)
+
+ expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true)
+ expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false)
+
+ expect(field).not_to be_authorized(object, nil, ctx)
+ end
+ end
+
context 'when considering complexity' do
let(:resolver) do
Class.new(described_class) do
diff --git a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb
index 0c548e1ce325cfcd2ad124d91d72f924daaa4405..ac512e28e7bf5dc34b10923306bfc7c79a478e5b 100644
--- a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb
+++ b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb
@@ -103,4 +103,36 @@ def self.authorization
.to contain_exactly(:base_authorization, :sub_authorization)
end
end
+
+ describe 'authorizes_object?' do
+ it 'is false by default' do
+ a_class = Class.new do
+ include Gitlab::Graphql::Authorize::AuthorizeResource
+ end
+
+ expect(a_class).not_to be_authorizes_object
+ end
+
+ it 'is true after calling authorizes_object!' do
+ a_class = Class.new do
+ include Gitlab::Graphql::Authorize::AuthorizeResource
+
+ authorizes_object!
+ end
+
+ expect(a_class).to be_authorizes_object
+ end
+
+ it 'is true if a parent authorizes_object' do
+ parent = Class.new do
+ include Gitlab::Graphql::Authorize::AuthorizeResource
+
+ authorizes_object!
+ end
+
+ child = Class.new(parent)
+
+ expect(child).to be_authorizes_object
+ end
+ end
end
diff --git a/spec/requests/api/graphql/milestone_spec.rb b/spec/requests/api/graphql/milestone_spec.rb
index 59de116fa2b940f4d51baa9c76bfa6b291417d4e..f6835936418bbaa48c9e928be00c38189cc27011 100644
--- a/spec/requests/api/graphql/milestone_spec.rb
+++ b/spec/requests/api/graphql/milestone_spec.rb
@@ -5,43 +5,125 @@
RSpec.describe 'Querying a Milestone' do
include GraphqlHelpers
- let_it_be(:current_user) { create(:user) }
+ let_it_be(:guest) { create(:user) }
let_it_be(:project) { create(:project) }
let_it_be(:milestone) { create(:milestone, project: project) }
+ let_it_be(:release_a) { create(:release, project: project) }
+ let_it_be(:release_b) { create(:release, project: project) }
- let(:query) do
- graphql_query_for('milestone', { id: milestone.to_global_id.to_s }, 'title')
+ before_all do
+ milestone.releases << [release_a, release_b]
+ project.add_guest(guest)
end
- subject { graphql_data['milestone'] }
-
- before do
- post_graphql(query, current_user: current_user)
+ let(:expected_release_nodes) do
+ contain_exactly(a_graphql_entity_for(release_a), a_graphql_entity_for(release_b))
end
- context 'when the user has access to the milestone' do
- before_all do
- project.add_guest(current_user)
+ context 'when we post the query' do
+ let(:current_user) { nil }
+ let(:query) do
+ graphql_query_for('milestone', { id: milestone.to_global_id.to_s }, all_graphql_fields_for('Milestone'))
end
- it_behaves_like 'a working graphql query'
+ subject { graphql_data['milestone'] }
- it { is_expected.to include('title' => milestone.name) }
- end
+ before do
+ post_graphql(query, current_user: current_user)
+ end
- context 'when the user does not have access to the milestone' do
- it_behaves_like 'a working graphql query'
+ context 'when the user has access to the milestone' do
+ let(:current_user) { guest }
- it { is_expected.to be_nil }
+ it_behaves_like 'a working graphql query'
+
+ it { is_expected.to include('title' => milestone.name) }
+
+ it 'contains release information' do
+ is_expected.to include('releases' => include('nodes' => expected_release_nodes))
+ end
+ end
+
+ context 'when the user does not have access to the milestone' do
+ it_behaves_like 'a working graphql query'
+
+ it { is_expected.to be_nil }
+ end
+
+ context 'when ID argument is missing' do
+ let(:query) do
+ graphql_query_for('milestone', {}, 'title')
+ end
+
+ it 'raises an exception' do
+ expect(graphql_errors).to include(a_hash_including('message' => "Field 'milestone' is missing required arguments: id"))
+ end
+ end
end
- context 'when ID argument is missing' do
- let(:query) do
- graphql_query_for('milestone', {}, 'title')
+ context 'when there are two milestones' do
+ let_it_be(:milestone_b) { create(:milestone, project: project) }
+
+ let(:current_user) { guest }
+ let(:milestone_fields) do
+ <<~GQL
+ fragment milestoneFields on Milestone {
+ #{all_graphql_fields_for('Milestone', max_depth: 1)}
+ releases { nodes { #{all_graphql_fields_for('Release', max_depth: 1)} } }
+ }
+ GQL
+ end
+
+ let(:single_query) do
+ <<~GQL
+ query ($id_a: MilestoneID!) {
+ a: milestone(id: $id_a) { ...milestoneFields }
+ }
+
+ #{milestone_fields}
+ GQL
+ end
+
+ let(:multi_query) do
+ <<~GQL
+ query ($id_a: MilestoneID!, $id_b: MilestoneID!) {
+ a: milestone(id: $id_a) { ...milestoneFields }
+ b: milestone(id: $id_b) { ...milestoneFields }
+ }
+ #{milestone_fields}
+ GQL
+ end
+
+ it 'produces correct results' do
+ r = run_with_clean_state(multi_query,
+ context: { current_user: current_user },
+ variables: {
+ id_a: global_id_of(milestone).to_s,
+ id_b: milestone_b.to_global_id.to_s
+ })
+
+ expect(r.to_h['errors']).to be_blank
+ expect(graphql_dig_at(r.to_h, :data, :a, :releases, :nodes)).to match expected_release_nodes
+ expect(graphql_dig_at(r.to_h, :data, :b, :releases, :nodes)).to be_empty
end
- it 'raises an exception' do
- expect(graphql_errors).to include(a_hash_including('message' => "Field 'milestone' is missing required arguments: id"))
+ it 'does not suffer from N+1 performance issues' do
+ baseline = ActiveRecord::QueryRecorder.new do
+ run_with_clean_state(single_query,
+ context: { current_user: current_user },
+ variables: { id_a: milestone.to_global_id.to_s })
+ end
+
+ multi = ActiveRecord::QueryRecorder.new do
+ run_with_clean_state(multi_query,
+ context: { current_user: current_user },
+ variables: {
+ id_a: milestone.to_global_id.to_s,
+ id_b: milestone_b.to_global_id.to_s
+ })
+ end
+
+ expect(multi).not_to exceed_query_limit(baseline)
end
end
end
diff --git a/spec/requests/api/graphql/project/milestones_spec.rb b/spec/requests/api/graphql/project/milestones_spec.rb
index 3e8948d83b18b1cda3960492ae50722e061e1dbb..d1ee157fc74e87e093022ac06100fca5ffe1c5b0 100644
--- a/spec/requests/api/graphql/project/milestones_spec.rb
+++ b/spec/requests/api/graphql/project/milestones_spec.rb
@@ -59,6 +59,27 @@ def result_list(expected)
end
end
+ context 'the user does not have access' do
+ let_it_be(:project) { create(:project) }
+ let_it_be(:milestones) { create_list(:milestone, 2, project: project) }
+
+ it 'is nil' do
+ post_graphql(query, current_user: current_user)
+
+ expect(graphql_data_at(:project)).to be_nil
+ end
+
+ context 'the user has access' do
+ let(:expected) { milestones }
+
+ before do
+ project.add_guest(current_user)
+ end
+
+ it_behaves_like 'searching with parameters'
+ end
+ end
+
context 'there are no search params' do
let(:search_params) { nil }
let(:expected) { all_milestones }
diff --git a/spec/support/graphql/resolver_factories.rb b/spec/support/graphql/resolver_factories.rb
index 8188f17cc43c52e388e956208f8af3474bfbbf77..3c5aad34e8ba948cb817eed2af3f9e7779ceaf2b 100644
--- a/spec/support/graphql/resolver_factories.rb
+++ b/spec/support/graphql/resolver_factories.rb
@@ -15,8 +15,8 @@ def new_resolver(resolved_value = 'Resolved value', method: :resolve)
private
- def simple_resolver(resolved_value = 'Resolved value')
- Class.new(Resolvers::BaseResolver) do
+ def simple_resolver(resolved_value = 'Resolved value', base_class: Resolvers::BaseResolver)
+ Class.new(base_class) do
define_method :resolve do |**_args|
resolved_value
end
diff --git a/spec/support/matchers/exceed_query_limit.rb b/spec/support/matchers/exceed_query_limit.rb
index b48c7f905b24e6517002d66dc6ea09e261d93bda..23f43e05a10a08db235f7f2889633a4dbf6aac85 100644
--- a/spec/support/matchers/exceed_query_limit.rb
+++ b/spec/support/matchers/exceed_query_limit.rb
@@ -323,7 +323,12 @@ def skip_cached
include ExceedQueryLimitHelpers
match do |block|
- verify_count(&block)
+ if block.is_a?(ActiveRecord::QueryRecorder)
+ @recorder = block
+ verify_count
+ else
+ verify_count(&block)
+ end
end
failure_message_when_negated do |actual|