diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 768595ceeb4..45cef123c34 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -13,7 +13,7 @@ class Projects::PipelinesController < Projects::ApplicationController def index @scope = params[:scope] @pipelines = PipelinesFinder - .new(project, scope: @scope) + .new(project, current_user, scope: @scope) .execute .page(params[:page]) .per(30) @@ -178,7 +178,7 @@ class Projects::PipelinesController < Projects::ApplicationController end def limited_pipelines_count(project, scope = nil) - finder = PipelinesFinder.new(project, scope: scope) + finder = PipelinesFinder.new(project, current_user, scope: scope) view_context.limited_counter_with_delimiter(finder.execute) end diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb index 0a487839aff..a99a889a7e9 100644 --- a/app/finders/pipelines_finder.rb +++ b/app/finders/pipelines_finder.rb @@ -1,15 +1,20 @@ class PipelinesFinder - attr_reader :project, :pipelines, :params + attr_reader :project, :pipelines, :params, :current_user ALLOWED_INDEXED_COLUMNS = %w[id status ref user_id].freeze - def initialize(project, params = {}) + def initialize(project, current_user, params = {}) @project = project + @current_user = current_user @pipelines = project.pipelines @params = params end def execute + unless Ability.allowed?(current_user, :read_pipeline, project) + return Ci::Pipeline.none + end + items = pipelines items = by_scope(items) items = by_status(items) diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index de4fc1d8e32..d9f9129d08a 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -2,7 +2,10 @@ class GitlabSchema < GraphQL::Schema use BatchLoader::GraphQL use Gitlab::Graphql::Authorize use Gitlab::Graphql::Present + use Gitlab::Graphql::Connections query(Types::QueryType) + + default_max_page_size 100 # mutation(Types::MutationType) end diff --git a/app/graphql/resolvers/concerns/resolves_pipelines.rb b/app/graphql/resolvers/concerns/resolves_pipelines.rb new file mode 100644 index 00000000000..9ec45378d8e --- /dev/null +++ b/app/graphql/resolvers/concerns/resolves_pipelines.rb @@ -0,0 +1,23 @@ +module ResolvesPipelines + extend ActiveSupport::Concern + + included do + type [Types::Ci::PipelineType], null: false + argument :status, + Types::Ci::PipelineStatusEnum, + required: false, + description: "Filter pipelines by their status" + argument :ref, + GraphQL::STRING_TYPE, + required: false, + description: "Filter pipelines by the ref they are run for" + argument :sha, + GraphQL::STRING_TYPE, + required: false, + description: "Filter pipelines by the sha of the commit they are run for" + end + + def resolve_pipelines(project, params = {}) + PipelinesFinder.new(project, context[:current_user], params).execute + end +end diff --git a/app/graphql/resolvers/merge_request_pipelines_resolver.rb b/app/graphql/resolvers/merge_request_pipelines_resolver.rb new file mode 100644 index 00000000000..00b51ee1381 --- /dev/null +++ b/app/graphql/resolvers/merge_request_pipelines_resolver.rb @@ -0,0 +1,16 @@ +module Resolvers + class MergeRequestPipelinesResolver < BaseResolver + include ::ResolvesPipelines + + alias_method :merge_request, :object + + def resolve(**args) + resolve_pipelines(project, args) + .merge(merge_request.all_pipelines) + end + + def project + merge_request.source_project + end + end +end diff --git a/app/graphql/resolvers/project_pipelines_resolver.rb b/app/graphql/resolvers/project_pipelines_resolver.rb new file mode 100644 index 00000000000..7f175a3b26c --- /dev/null +++ b/app/graphql/resolvers/project_pipelines_resolver.rb @@ -0,0 +1,11 @@ +module Resolvers + class ProjectPipelinesResolver < BaseResolver + include ResolvesPipelines + + alias_method :project, :object + + def resolve(**args) + resolve_pipelines(project, args) + end + end +end diff --git a/app/graphql/types/ci/pipeline_status_enum.rb b/app/graphql/types/ci/pipeline_status_enum.rb new file mode 100644 index 00000000000..2c12e5001d8 --- /dev/null +++ b/app/graphql/types/ci/pipeline_status_enum.rb @@ -0,0 +1,9 @@ +module Types + module Ci + class PipelineStatusEnum < BaseEnum + ::Ci::Pipeline.all_state_names.each do |state_symbol| + value state_symbol.to_s.upcase, value: state_symbol.to_s + end + end + end +end diff --git a/app/graphql/types/ci/pipeline_type.rb b/app/graphql/types/ci/pipeline_type.rb new file mode 100644 index 00000000000..bbb7d9354d0 --- /dev/null +++ b/app/graphql/types/ci/pipeline_type.rb @@ -0,0 +1,31 @@ +module Types + module Ci + class PipelineType < BaseObject + expose_permissions Types::PermissionTypes::Ci::Pipeline + + graphql_name 'Pipeline' + + field :id, GraphQL::ID_TYPE, null: false + field :iid, GraphQL::ID_TYPE, null: false + + field :sha, GraphQL::STRING_TYPE, null: false + field :before_sha, GraphQL::STRING_TYPE, null: true + field :status, PipelineStatusEnum, null: false + field :duration, + GraphQL::INT_TYPE, + null: true, + description: "Duration of the pipeline in seconds" + field :coverage, + GraphQL::FLOAT_TYPE, + null: true, + description: "Coverage percentage" + field :created_at, Types::TimeType, null: false + field :updated_at, Types::TimeType, null: false + field :started_at, Types::TimeType, null: true + field :finished_at, Types::TimeType, null: true + field :committed_at, Types::TimeType, null: true + + # TODO: Add triggering user as a type + end + end +end diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index a1f3c0dd8c0..88cd2adc6dc 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -45,5 +45,11 @@ module Types field :upvotes, GraphQL::INT_TYPE, null: false field :downvotes, GraphQL::INT_TYPE, null: false field :subscribed, GraphQL::BOOLEAN_TYPE, method: :subscribed?, null: false + + field :head_pipeline, Types::Ci::PipelineType, null: true, method: :actual_head_pipeline do + authorize :read_pipeline + end + field :pipelines, Types::Ci::PipelineType.connection_type, + resolver: Resolvers::MergeRequestPipelinesResolver end end diff --git a/app/graphql/types/permission_types/ci/pipeline.rb b/app/graphql/types/permission_types/ci/pipeline.rb new file mode 100644 index 00000000000..942539c7cf7 --- /dev/null +++ b/app/graphql/types/permission_types/ci/pipeline.rb @@ -0,0 +1,11 @@ +module Types + module PermissionTypes + module Ci + class Pipeline < BasePermissionType + graphql_name 'PipelinePermissions' + + abilities :update_pipeline, :admin_pipeline, :destroy_pipeline + end + end + end +end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index a832e8b4bde..97707215b4e 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -70,5 +70,10 @@ module Types resolver: Resolvers::MergeRequestResolver do authorize :read_merge_request end + + field :pipelines, + Types::Ci::PipelineType.connection_type, + null: false, + resolver: Resolvers::ProjectPipelinesResolver end end diff --git a/changelogs/unreleased/bvl-graphql-pipeline-lists.yml b/changelogs/unreleased/bvl-graphql-pipeline-lists.yml new file mode 100644 index 00000000000..be258dc12ad --- /dev/null +++ b/changelogs/unreleased/bvl-graphql-pipeline-lists.yml @@ -0,0 +1,5 @@ +--- +title: Add pipeline lists to GraphQL +merge_request: 20249 +author: +type: added diff --git a/config/application.rb b/config/application.rb index d9483cd806d..97bc86b3e3a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -45,7 +45,8 @@ module Gitlab #{config.root}/app/workers/concerns #{config.root}/app/services/concerns #{config.root}/app/serializers/concerns - #{config.root}/app/finders/concerns]) + #{config.root}/app/finders/concerns + #{config.root}/app/graphql/resolvers/concerns]) config.generators.templates.push("#{config.root}/generator_templates") diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 33f078b0a63..b4a2349844b 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -54,6 +54,94 @@ a new presenter specifically for GraphQL. The presenter is initialized using the object resolved by a field, and the context. +### Connection Types + +GraphQL uses [cursor based +pagination](https://graphql.org/learn/pagination/#pagination-and-edges) +to expose collections of items. This provides the clients with a lot +of flexibility while also allowing the backend to use different +pagination models. + +To expose a collection of resources we can use a connection type. This wraps the array with default pagination fields. For example a query for project-pipelines could look like this: + +``` +query($project_path: ID!) { + project(fullPath: $project_path) { + pipelines(first: 2) { + pageInfo { + hasNextPage + hasPreviousPage + } + edges { + cursor + node { + id + status + } + } + } + } +} +``` + +This would return the first 2 pipelines of a project and related +pagination info., ordered by descending ID. The returned data would +look like this: + +```json +{ + "data": { + "project": { + "pipelines": { + "pageInfo": { + "hasNextPage": true, + "hasPreviousPage": false + }, + "edges": [ + { + "cursor": "Nzc=", + "node": { + "id": "77", + "status": "FAILED" + } + }, + { + "cursor": "Njc=", + "node": { + "id": "67", + "status": "FAILED" + } + } + ] + } + } + } +} +``` + +To get the next page, the cursor of the last known element could be +passed: + +``` +query($project_path: ID!) { + project(fullPath: $project_path) { + pipelines(first: 2, after: "Njc=") { + pageInfo { + hasNextPage + hasPreviousPage + } + edges { + cursor + node { + id + status + } + } + } + } +} +``` + ### Exposing permissions for a type To expose permissions the current user has on a resource, you can call diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index 8374a57edfa..5d33a13d035 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -31,7 +31,7 @@ module API get ':id/pipelines' do authorize! :read_pipeline, user_project - pipelines = PipelinesFinder.new(user_project, params).execute + pipelines = PipelinesFinder.new(user_project, current_user, params).execute present paginate(pipelines), with: Entities::PipelineBasic end diff --git a/lib/gitlab/graphql/connections.rb b/lib/gitlab/graphql/connections.rb new file mode 100644 index 00000000000..2582ffeb2a8 --- /dev/null +++ b/lib/gitlab/graphql/connections.rb @@ -0,0 +1,12 @@ +module Gitlab + module Graphql + module Connections + def self.use(_schema) + GraphQL::Relay::BaseConnection.register_connection_implementation( + ActiveRecord::Relation, + Gitlab::Graphql::Connections::KeysetConnection + ) + end + end + end +end diff --git a/lib/gitlab/graphql/connections/keyset_connection.rb b/lib/gitlab/graphql/connections/keyset_connection.rb new file mode 100644 index 00000000000..abee2afe144 --- /dev/null +++ b/lib/gitlab/graphql/connections/keyset_connection.rb @@ -0,0 +1,73 @@ +module Gitlab + module Graphql + module Connections + class KeysetConnection < GraphQL::Relay::BaseConnection + def cursor_from_node(node) + encode(node[order_field].to_s) + end + + def sliced_nodes + @sliced_nodes ||= + begin + sliced = nodes + + sliced = sliced.where(before_slice) if before.present? + sliced = sliced.where(after_slice) if after.present? + + sliced + end + end + + def paged_nodes + if first && last + raise Gitlab::Graphql::Errors::ArgumentError.new("Can only provide either `first` or `last`, not both") + end + + if last + sliced_nodes.last(limit_value) + else + sliced_nodes.limit(limit_value) + end + end + + private + + def before_slice + if sort_direction == :asc + table[order_field].lt(decode(before)) + else + table[order_field].gt(decode(before)) + end + end + + def after_slice + if sort_direction == :asc + table[order_field].gt(decode(after)) + else + table[order_field].lt(decode(after)) + end + end + + def limit_value + @limit_value ||= [first, last, max_page_size].compact.min + end + + def table + nodes.arel_table + end + + def order_info + @order_info ||= nodes.order_values.first + end + + def order_field + @order_field ||= order_info&.expr&.name || nodes.primary_key + end + + def sort_direction + @order_direction ||= order_info&.direction || :desc + end + end + end + end +end diff --git a/lib/gitlab/graphql/errors.rb b/lib/gitlab/graphql/errors.rb new file mode 100644 index 00000000000..1d8e8307ab9 --- /dev/null +++ b/lib/gitlab/graphql/errors.rb @@ -0,0 +1,8 @@ +module Gitlab + module Graphql + module Errors + BaseError = Class.new(GraphQL::ExecutionError) + ArgumentError = Class.new(BaseError) + end + end +end diff --git a/lib/gitlab/graphql/present/instrumentation.rb b/lib/gitlab/graphql/present/instrumentation.rb index 6f2b26c9676..f87fd147b15 100644 --- a/lib/gitlab/graphql/present/instrumentation.rb +++ b/lib/gitlab/graphql/present/instrumentation.rb @@ -3,6 +3,8 @@ module Gitlab module Present class Instrumentation def instrument(type, field) + return field unless field.metadata[:type_class] + presented_in = field.metadata[:type_class].owner return field unless presented_in.respond_to?(:presenter_class) return field unless presented_in.presenter_class diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 9618a8417ec..1cc7f33b57a 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -4,7 +4,7 @@ describe Projects::PipelinesController do include ApiHelpers set(:user) { create(:user) } - set(:project) { create(:project, :public, :repository) } + let(:project) { create(:project, :public, :repository) } let(:feature) { ProjectFeature::DISABLED } before do @@ -91,6 +91,24 @@ describe Projects::PipelinesController do end end + context 'when the project is private' do + let(:project) { create(:project, :private, :repository) } + + it 'returns `not_found` when the user does not have access' do + sign_in(create(:user)) + + get_pipelines_index_json + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns the pipelines when the user has access' do + get_pipelines_index_json + + expect(json_response['pipelines'].size).to eq(5) + end + end + def get_pipelines_index_json get :index, namespace_id: project.namespace, project_id: project, diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb index d6253b605b9..c6e832ad69b 100644 --- a/spec/finders/pipelines_finder_spec.rb +++ b/spec/finders/pipelines_finder_spec.rb @@ -1,9 +1,10 @@ require 'spec_helper' describe PipelinesFinder do - let(:project) { create(:project, :repository) } - - subject { described_class.new(project, params).execute } + let(:project) { create(:project, :public, :repository) } + let(:current_user) { nil } + let(:params) { {} } + subject { described_class.new(project, current_user, params).execute } describe "#execute" do context 'when params is empty' do @@ -223,5 +224,27 @@ describe PipelinesFinder do end end end + + context 'when the project has limited access to piplines' do + let(:project) { create(:project, :private, :repository) } + let(:current_user) { create(:user) } + let!(:pipelines) { create_list(:ci_pipeline, 2, project: project) } + + context 'when the user has access' do + before do + project.add_developer(current_user) + end + + it 'is expected to return pipelines' do + is_expected.to contain_exactly(*pipelines) + end + end + + context 'the user is not allowed to read pipelines' do + it 'returns empty' do + is_expected.to be_empty + end + end + end end end diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index b892f6b44ed..515bbe78cb7 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -27,6 +27,12 @@ describe GitlabSchema do expect(described_class.query).to eq(::Types::QueryType.to_graphql) end + it 'paginates active record relations using `Gitlab::Graphql::Connections::KeysetConnection`' do + connection = GraphQL::Relay::BaseConnection::CONNECTION_IMPLEMENTATIONS[ActiveRecord::Relation.name] + + expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection) + end + def field_instrumenters described_class.instrumenters[:field] end diff --git a/spec/graphql/resolvers/concerns/resolves_pipelines_spec.rb b/spec/graphql/resolvers/concerns/resolves_pipelines_spec.rb new file mode 100644 index 00000000000..ea7159eacf9 --- /dev/null +++ b/spec/graphql/resolvers/concerns/resolves_pipelines_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe ResolvesPipelines do + include GraphqlHelpers + + subject(:resolver) do + Class.new(Resolvers::BaseResolver) do + include ResolvesPipelines + + def resolve(**args) + resolve_pipelines(object, args) + end + end + end + + let(:current_user) { create(:user) } + set(:project) { create(:project, :private) } + set(:pipeline) { create(:ci_pipeline, project: project) } + set(:failed_pipeline) { create(:ci_pipeline, :failed, project: project) } + set(:ref_pipeline) { create(:ci_pipeline, project: project, ref: 'awesome-feature') } + set(:sha_pipeline) { create(:ci_pipeline, project: project, sha: 'deadbeef') } + + before do + project.add_developer(current_user) + end + + it { is_expected.to have_graphql_arguments(:status, :ref, :sha) } + + it 'finds all pipelines' do + expect(resolve_pipelines).to contain_exactly(pipeline, failed_pipeline, ref_pipeline, sha_pipeline) + end + + it 'allows filtering by status' do + expect(resolve_pipelines(status: 'failed')).to contain_exactly(failed_pipeline) + end + + it 'allows filtering by ref' do + expect(resolve_pipelines(ref: 'awesome-feature')).to contain_exactly(ref_pipeline) + end + + it 'allows filtering by sha' do + expect(resolve_pipelines(sha: 'deadbeef')).to contain_exactly(sha_pipeline) + end + + it 'does not return any pipelines if the user does not have access' do + expect(resolve_pipelines({}, {})).to be_empty + end + + def resolve_pipelines(args = {}, context = { current_user: current_user }) + resolve(resolver, obj: project, args: args, ctx: context) + end +end diff --git a/spec/graphql/resolvers/merge_request_pipelines_resolver_spec.rb b/spec/graphql/resolvers/merge_request_pipelines_resolver_spec.rb new file mode 100644 index 00000000000..09b17bf6fc9 --- /dev/null +++ b/spec/graphql/resolvers/merge_request_pipelines_resolver_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Resolvers::MergeRequestPipelinesResolver do + include GraphqlHelpers + + set(:merge_request) { create(:merge_request) } + set(:pipeline) do + create( + :ci_pipeline, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha + ) + end + set(:other_project_pipeline) { create(:ci_pipeline, project: merge_request.source_project) } + set(:other_pipeline) { create(:ci_pipeline) } + let(:current_user) { create(:user) } + + before do + merge_request.project.add_developer(current_user) + end + + def resolve_pipelines + resolve(described_class, obj: merge_request, ctx: { current_user: current_user }) + end + + it 'resolves only MRs for the passed merge request' do + expect(resolve_pipelines).to contain_exactly(pipeline) + end +end diff --git a/spec/graphql/resolvers/project_pipelines_resolver_spec.rb b/spec/graphql/resolvers/project_pipelines_resolver_spec.rb new file mode 100644 index 00000000000..407ca2f9d78 --- /dev/null +++ b/spec/graphql/resolvers/project_pipelines_resolver_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Resolvers::ProjectPipelinesResolver do + include GraphqlHelpers + + set(:project) { create(:project) } + set(:pipeline) { create(:ci_pipeline, project: project) } + set(:other_pipeline) { create(:ci_pipeline) } + let(:current_user) { create(:user) } + + before do + project.add_developer(current_user) + end + + def resolve_pipelines + resolve(described_class, obj: project, ctx: { current_user: current_user }) + end + + it 'resolves only MRs for the passed merge request' do + expect(resolve_pipelines).to contain_exactly(pipeline) + end +end diff --git a/spec/graphql/types/ci/pipeline_type_spec.rb b/spec/graphql/types/ci/pipeline_type_spec.rb new file mode 100644 index 00000000000..ec1c689a4be --- /dev/null +++ b/spec/graphql/types/ci/pipeline_type_spec.rb @@ -0,0 +1,7 @@ +require 'spec_helper' + +describe Types::Ci::PipelineType do + it { expect(described_class.graphql_name).to eq('Pipeline') } + + it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::Ci::Pipeline) } +end diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb index 6e57122867a..c369953e3ea 100644 --- a/spec/graphql/types/merge_request_type_spec.rb +++ b/spec/graphql/types/merge_request_type_spec.rb @@ -1,5 +1,16 @@ require 'spec_helper' -describe Types::MergeRequestType do +describe GitlabSchema.types['MergeRequest'] do it { expect(described_class).to expose_permissions_using(Types::PermissionTypes::MergeRequest) } + + describe 'head pipeline' do + it 'has a head pipeline field' do + expect(described_class).to have_graphql_field(:head_pipeline) + end + + it 'authorizes the field' do + expect(described_class.fields['headPipeline']) + .to require_graphql_authorizations(:read_pipeline) + end + end end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 7b5bc335511..49606c397b9 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -13,4 +13,6 @@ describe GitlabSchema.types['Project'] do .to require_graphql_authorizations(:read_merge_request) end end + + it { is_expected.to have_graphql_field(:pipelines) } end diff --git a/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb b/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb new file mode 100644 index 00000000000..96615ae80de --- /dev/null +++ b/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb @@ -0,0 +1,112 @@ +require 'spec_helper' + +describe Gitlab::Graphql::Connections::KeysetConnection do + let(:nodes) { Project.all.order(id: :asc) } + let(:arguments) { {} } + subject(:connection) do + described_class.new(nodes, arguments, max_page_size: 3) + end + + def encoded_property(value) + Base64.strict_encode64(value.to_s) + end + + describe '#cursor_from_nodes' do + let(:project) { create(:project) } + + it 'returns an encoded ID' do + expect(connection.cursor_from_node(project)) + .to eq(encoded_property(project.id)) + end + + context 'when an order was specified' do + let(:nodes) { Project.order(:updated_at) } + + it 'returns the encoded value of the order' do + expect(connection.cursor_from_node(project)) + .to eq(encoded_property(project.updated_at)) + end + end + end + + describe '#sliced_nodes' do + let(:projects) { create_list(:project, 4) } + + context 'when before is passed' do + let(:arguments) { { before: encoded_property(projects[1].id) } } + + it 'only returns the project before the selected one' do + expect(subject.sliced_nodes).to contain_exactly(projects.first) + end + + context 'when the sort order is descending' do + let(:nodes) { Project.all.order(id: :desc) } + + it 'returns the correct nodes' do + expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1]) + end + end + end + + context 'when after is passed' do + let(:arguments) { { after: encoded_property(projects[1].id) } } + + it 'only returns the project before the selected one' do + expect(subject.sliced_nodes).to contain_exactly(*projects[2..-1]) + end + + context 'when the sort order is descending' do + let(:nodes) { Project.all.order(id: :desc) } + + it 'returns the correct nodes' do + expect(subject.sliced_nodes).to contain_exactly(projects.first) + end + end + end + + context 'when both before and after are passed' do + let(:arguments) do + { + after: encoded_property(projects[1].id), + before: encoded_property(projects[3].id) + } + end + + it 'returns the expected set' do + expect(subject.sliced_nodes).to contain_exactly(projects[2]) + end + end + end + + describe '#paged_nodes' do + let!(:projects) { create_list(:project, 5) } + + it 'returns the collection limited to max page size' do + expect(subject.paged_nodes.size).to eq(3) + end + + context 'when `first` is passed' do + let(:arguments) { { first: 2 } } + + it 'returns only the first elements' do + expect(subject.paged_nodes).to contain_exactly(projects.first, projects.second) + end + end + + context 'when `last` is passed' do + let(:arguments) { { last: 2 } } + + it 'returns only the last elements' do + expect(subject.paged_nodes).to contain_exactly(projects[3], projects[4]) + end + end + + context 'when both are passed' do + let(:arguments) { { first: 2, last: 2 } } + + it 'raises an error' do + expect { subject.paged_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) + end + end + end +end diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index ad57c43bc87..deb6abbc026 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -67,4 +67,28 @@ describe 'getting merge request information nested in a project' do expect(merge_request_graphql_data).to be_nil end end + + context 'when there are pipelines' do + before do + pipeline = create( + :ci_pipeline, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha + ) + merge_request.update!(head_pipeline: pipeline) + end + + it 'has a head pipeline' do + post_graphql(query, current_user: current_user) + + expect(merge_request_graphql_data['headPipeline']).to be_present + end + + it 'has pipeline connections' do + post_graphql(query, current_user: current_user) + + expect(merge_request_graphql_data['pipelines']['edges'].size).to eq(1) + end + end end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index a2b3dc5d121..0727ada4691 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -26,6 +26,18 @@ describe 'getting project information' do post_graphql(query, current_user: current_user) end end + + context 'when there are pipelines present' do + before do + create(:ci_pipeline, project: project) + end + + it 'is included in the pipelines connection' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['pipelines']['edges'].size).to eq(1) + end + end end context 'when the user does not have access to the project' do diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 0930b9da368..b9322975b5a 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -57,12 +57,12 @@ module GraphqlHelpers type.fields.map do |name, field| # We can't guess arguments, so skip fields that require them - next if field.arguments.any? + next if required_arguments?(field) - if scalar?(field) - name - else + if nested_fields?(field) "#{name} { #{all_graphql_fields_for(field_type(field))} }" + else + name end end.compact.join("\n") end @@ -85,10 +85,22 @@ module GraphqlHelpers json_response['data'] end + def nested_fields?(field) + !scalar?(field) && !enum?(field) + end + def scalar?(field) field_type(field).kind.scalar? end + def enum?(field) + field_type(field).kind.enum? + end + + def required_arguments?(field) + field.arguments.values.any? { |argument| argument.type.non_null? } + end + def field_type(field) if field.type.respond_to?(:of_type) field.type.of_type