summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2018-05-21 09:52:24 +0200
committerBob Van Landuyt <bob@vanlanduyt.co>2018-06-05 09:41:25 +0200
commit1cf9c420ea763a359735b15f90a55eac84c73624 (patch)
tree9072ce3f8cb735444b1099e1036addd7aa208386
parent6e842915d7c9a4df4ae93c2d3c707ff631628e27 (diff)
downloadgitlab-ce-1cf9c420ea763a359735b15f90a55eac84c73624.tar.gz
Add `present_using` to types
By specifying a presenter for the object type, we can keep the logic out of `GitlabSchema`. The presenter gets initialized using the object being presented, and the context (including the `current_user`).
-rw-r--r--app/graphql/gitlab_schema.rb3
-rw-r--r--app/graphql/types/merge_request_type.rb56
-rw-r--r--app/graphql/types/project_type.rb1
-rw-r--r--app/presenters/merge_request_presenter.rb19
-rw-r--r--changelogs/unreleased/bvl-graphql-start-34754.yml5
-rw-r--r--doc/api/graphql/index.md34
-rw-r--r--doc/development/README.md2
-rw-r--r--doc/development/api_graphql_styleguide.md66
-rw-r--r--lib/gitlab/graphql/present.rb34
-rw-r--r--spec/graphql/gitlab_schema_spec.rb7
-rw-r--r--spec/requests/graphql/merge_request_query_spec.rb24
-rw-r--r--spec/requests/graphql/project_query_spec.rb23
-rw-r--r--spec/support/helpers/graphql_helpers.rb33
-rw-r--r--spec/support/shared_examples/requests/graphql_shared_examples.rb18
14 files changed, 292 insertions, 33 deletions
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index 143ccf92c85..ac75c4bf2e3 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/app/graphql/gitlab_schema.rb
@@ -1,11 +1,12 @@
Gitlab::Graphql::Authorize.register!
+Gitlab::Graphql::Present.register!
GitlabSchema = GraphQL::Schema.define do
use BatchLoader::GraphQL
enable_preloading
enable_authorization
+ enable_presenting
- mutation(Types::MutationType)
query(Types::QueryType)
end
diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb
index 9b12f6f2bf3..fc430ca03d5 100644
--- a/app/graphql/types/merge_request_type.rb
+++ b/app/graphql/types/merge_request_type.rb
@@ -1,50 +1,46 @@
Types::MergeRequestType = GraphQL::ObjectType.define do
+ present_using MergeRequestPresenter
+
name 'MergeRequest'
field :id, !types.ID
field :iid, !types.ID
- field :title, types.String
+ field :title, !types.String
field :description, types.String
field :state, types.String
-
- field :created_at, Types::TimeType
- field :updated_at, Types::TimeType
-
- field :source_project, -> { Types::ProjectType }
- field :target_project, -> { Types::ProjectType }
-
+ field :created_at, !Types::TimeType
+ field :updated_at, !Types::TimeType
+ field :source_project, Types::ProjectType
+ field :target_project, !Types::ProjectType
# Alias for target_project
- field :project, -> { Types::ProjectType }
-
+ field :project, !Types::ProjectType
+ field :project_id, !types.Int, property: :target_project_id
field :source_project_id, types.Int
- field :target_project_id, types.Int
- field :project_id, types.Int
-
- field :source_branch, types.String
- field :target_branch, types.String
-
+ field :target_project_id, !types.Int
+ field :source_branch, !types.String
+ field :target_branch, !types.String
field :work_in_progress, types.Boolean, property: :work_in_progress?
field :merge_when_pipeline_succeeds, types.Boolean
-
field :sha, types.String, property: :diff_head_sha
field :merge_commit_sha, types.String
-
field :user_notes_count, types.Int
field :should_remove_source_branch, types.Boolean, property: :should_remove_source_branch?
field :force_remove_source_branch, types.Boolean, property: :force_remove_source_branch?
-
field :merge_status, types.String
-
- field :web_url, types.String do
- resolve ->(merge_request, args, ctx) { Gitlab::UrlBuilder.build(merge_request) }
- end
-
+ field :in_progress_merge_commit_sha, types.String
+ field :merge_error, types.String
+ field :allow_maintainer_to_push, types.Boolean
+ field :should_be_rebased, types.Boolean, property: :should_be_rebased?
+ field :rebase_commit_sha, types.String
+ field :rebase_in_progress, types.Boolean, property: :rebase_in_progress?
+ field :diff_head_sha, types.String
+ field :merge_commit_message, types.String
+ field :merge_ongoing, types.Boolean, property: :merge_ongoing?
+ field :work_in_progress, types.Boolean, property: :work_in_progress?
+ field :source_branch_exists, types.Boolean, property: :source_branch_exists?
+ field :mergeable_discussions_state, types.Boolean
+ field :web_url, types.String, property: :web_url
field :upvotes, types.Int
field :downvotes, types.Int
-
- field :subscribed, types.Boolean do
- resolve ->(merge_request, args, ctx) do
- merge_request.subscribed?(ctx[:current_user], merge_request.target_project)
- end
- end
+ field :subscribed, types.Boolean, property: :subscribed?
end
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb
index bfefc594896..593c1b67830 100644
--- a/app/graphql/types/project_type.rb
+++ b/app/graphql/types/project_type.rb
@@ -31,6 +31,7 @@ Types::ProjectType = GraphQL::ObjectType.define do
field :container_registry_enabled, types.Boolean
field :shared_runners_enabled, types.Boolean
field :lfs_enabled, types.Boolean
+ field :ff_only_enabled, types.Boolean, property: :merge_requests_ff_only_enabled
field :avatar_url, types.String do
resolve ->(project, args, ctx) { project.avatar_url(only_path: false) }
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index ad839d9840a..8d466c33510 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -179,6 +179,25 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
.can_push_to_branch?(source_branch)
end
+ def mergeable_discussions_state
+ # This avoids calling MergeRequest#mergeable_discussions_state without
+ # considering the state of the MR first. If a MR isn't mergeable, we can
+ # safely short-circuit it.
+ if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true)
+ merge_request.mergeable_discussions_state?
+ else
+ false
+ end
+ end
+
+ def web_url
+ Gitlab::UrlBuilder.build(merge_request)
+ end
+
+ def subscribed?
+ merge_request.subscribed?(current_user, merge_request.target_project)
+ end
+
private
def cached_can_be_reverted?
diff --git a/changelogs/unreleased/bvl-graphql-start-34754.yml b/changelogs/unreleased/bvl-graphql-start-34754.yml
new file mode 100644
index 00000000000..a31f46d3a61
--- /dev/null
+++ b/changelogs/unreleased/bvl-graphql-start-34754.yml
@@ -0,0 +1,5 @@
+---
+title: Setup graphql with initial project & merge request query
+merge_request: 19008
+author:
+type: added
diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md
new file mode 100644
index 00000000000..78e634df347
--- /dev/null
+++ b/doc/api/graphql/index.md
@@ -0,0 +1,34 @@
+# GraphQL API (Beta)
+
+> [Introduced][ce-19008] in GitLab 11.0.
+
+## Enabling the GraphQL feature
+
+The GraphQL API itself is currently in Beta, and therefore hidden behind a
+feature flag. To enable it on your selfhosted instance, run
+`Feature.enable(:graphql)`.
+
+Start the console by running
+
+```bash
+sudo gitlab-rails console
+```
+
+Then enable the feature by running
+
+```ruby
+Feature.enable(:graphql)
+```
+
+## Available queries
+
+A first iteration of a GraphQL API inlcudes only 2 queries: `project` and
+`merge_request` and only returns scalar fields, or fields of the type `Project`
+or `MergeRequest`.
+
+## GraphiQL
+
+The API can be explored by using the GraphiQL IDE, it is available on your
+instance on `gitlab.example.com/api/graphiql`.
+
+[ce-19008]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19008
diff --git a/doc/development/README.md b/doc/development/README.md
index 898c60e96c0..78c1b6bc6e3 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -32,6 +32,8 @@ description: 'Learn how to contribute to GitLab.'
- [GitLab utilities](utilities.md)
- [API styleguide](api_styleguide.md) Use this styleguide if you are
contributing to the API.
+- [GrapQL API styleguide](api_graphql_styleguide.md) Use this
+ styleguide if you are contribution to the [GraphQL API](../api/graphql/index.md)
- [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers
- [Working with Gitaly](gitaly.md)
- [Manage feature flags](feature_flags.md)
diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md
new file mode 100644
index 00000000000..4d73efa3f71
--- /dev/null
+++ b/doc/development/api_graphql_styleguide.md
@@ -0,0 +1,66 @@
+# GraphQL API
+
+## Authentication
+
+Authentication happens through the `GrapqlController`, right now this
+uses the same authentication as the rails application. So the session
+can be shared.
+
+It is also possible to add a `private_token` to the querystring, or
+add a `HTTP_PRIVATE_TOKEN` header.
+
+### Authorization
+
+Fields can be authorized using the same abilities used in the rails
+app. This can be done using the `authorize` helper:
+
+```ruby
+Types::QueryType = GraphQL::ObjectType.define do
+ name 'Query'
+
+ field :project, Types::ProjectType do
+ argument :full_path, !types.ID do
+ description 'The full path of the project, e.g., "gitlab-org/gitlab-ce"'
+ end
+
+ authorize :read_project
+
+ resolve Loaders::FullPathLoader[:project]
+ end
+end
+```
+
+The object found by the resolve call is used for authorization.
+
+
+## Types
+
+When exposing a model through the GraphQL API, we do so by creating a
+new type in `app/graphql/types`.
+
+When exposing properties in a type, make sure to keep the logic inside
+the definition as minimal as possible. Instead, consider moving any
+logic into a presenter:
+
+```ruby
+Types::MergeRequestType = GraphQL::ObjectType.define do
+ present_using MergeRequestPresenter
+
+ name 'MergeRequest'
+end
+```
+
+An existing presenter could be used, but it is also possible to create
+a new presenter specifically for GraphQL.
+
+The presenter is initialized using the object resolved by a field, and
+the context.
+
+## Testing
+
+_full stack_ tests for a graphql query or mutation live in
+`spec/requests/graphql`.
+
+When adding a query, the `a working graphql query` shared example can
+be used to test the query, it expects a valid `query` to be available
+in the spec.
diff --git a/lib/gitlab/graphql/present.rb b/lib/gitlab/graphql/present.rb
new file mode 100644
index 00000000000..b060692b334
--- /dev/null
+++ b/lib/gitlab/graphql/present.rb
@@ -0,0 +1,34 @@
+module Gitlab
+ module Graphql
+ class Present
+ PRESENT_USING = -> (type, presenter_class, *args) do
+ type.metadata[:presenter_class] = presenter_class
+ end
+
+ INSTRUMENT_PROC = -> (schema) do
+ schema.instrument(:field, new)
+ end
+
+ def self.register!
+ GraphQL::Schema.accepts_definitions(enable_presenting: INSTRUMENT_PROC)
+ GraphQL::ObjectType.accepts_definitions(present_using: PRESENT_USING)
+ end
+
+ def instrument(type, field)
+ return field unless type.metadata[:presenter_class]
+
+ old_resolver = field.resolve_proc
+
+ resolve_with_presenter = -> (obj, args, context) do
+ presenter = type.metadata[:presenter_class].new(obj, **context.to_h)
+
+ old_resolver.call(presenter, args, context)
+ end
+
+ field.redefine do
+ resolve(resolve_with_presenter)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb
index 070b851b109..f25cc2fd6c9 100644
--- a/spec/graphql/gitlab_schema_spec.rb
+++ b/spec/graphql/gitlab_schema_spec.rb
@@ -13,7 +13,14 @@ describe GitlabSchema do
expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize))
end
+ it 'enables using presenters' do
+ expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Present))
+ end
+
it 'has the base mutation' do
+ pending <<~REASON
+ Having empty mutations breaks the automatic documentation in Graphiql, so removed for now."
+ REASON
expect(described_class.mutation).to eq(::Types::MutationType)
end
diff --git a/spec/requests/graphql/merge_request_query_spec.rb b/spec/requests/graphql/merge_request_query_spec.rb
new file mode 100644
index 00000000000..cbc19782e2f
--- /dev/null
+++ b/spec/requests/graphql/merge_request_query_spec.rb
@@ -0,0 +1,24 @@
+require 'spec_helper'
+
+describe 'getting merge request information' do
+ include GraphqlHelpers
+
+ let(:project) { create(:project, :repository, :public) }
+ let(:merge_request) { create(:merge_request, source_project: project) }
+
+ let(:query) do
+ <<~QUERY
+ {
+ merge_request(project: "#{merge_request.project.full_path}", iid: "#{merge_request.iid}") {
+ #{all_graphql_fields_for(MergeRequest)}
+ }
+ }
+ QUERY
+ end
+
+ it_behaves_like 'a working graphql query' do
+ it 'renders a merge request with all fields' do
+ expect(response_data['merge_request']).not_to be_nil
+ end
+ end
+end
diff --git a/spec/requests/graphql/project_query_spec.rb b/spec/requests/graphql/project_query_spec.rb
new file mode 100644
index 00000000000..110ed433e03
--- /dev/null
+++ b/spec/requests/graphql/project_query_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe 'getting project information' do
+ include GraphqlHelpers
+
+ let(:project) { create(:project, :repository, :public) }
+
+ let(:query) do
+ <<~QUERY
+ {
+ project(full_path: "#{project.full_path}") {
+ #{all_graphql_fields_for(Project)}
+ }
+ }
+ QUERY
+ end
+
+ it_behaves_like 'a working graphql query' do
+ it 'renders a project with all fields' do
+ expect(response_data['project']).not_to be_nil
+ end
+ end
+end
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index 1eaa7603af0..ae29b16e32c 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/spec/support/helpers/graphql_helpers.rb
@@ -9,12 +9,12 @@ module GraphqlHelpers
wrapper = proc do
begin
BatchLoader::Executor.ensure_current
- blk.call
+ yield
ensure
BatchLoader::Executor.clear_current
end
end
-
+
if max_queries
result = nil
expect { result = wrapper.call }.not_to exceed_query_limit(max_queries)
@@ -23,4 +23,33 @@ module GraphqlHelpers
wrapper.call
end
end
+
+ def all_graphql_fields_for(klass)
+ type = GitlabSchema.types[klass.name]
+ return "" unless type
+
+ type.fields.map do |name, field|
+ if scalar?(field)
+ name
+ else
+ "#{name} { #{all_graphql_fields_for(field_type(field))} }"
+ end
+ end.join("\n")
+ end
+
+ def post_graphql(query)
+ post '/api/graphql', query: query
+ end
+
+ def scalar?(field)
+ field_type(field).kind.scalar?
+ end
+
+ def field_type(field)
+ if field.type.respond_to?(:of_type)
+ field.type.of_type
+ else
+ field.type
+ end
+ end
end
diff --git a/spec/support/shared_examples/requests/graphql_shared_examples.rb b/spec/support/shared_examples/requests/graphql_shared_examples.rb
new file mode 100644
index 00000000000..c143b83696e
--- /dev/null
+++ b/spec/support/shared_examples/requests/graphql_shared_examples.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+shared_examples 'a working graphql query' do
+ include GraphqlHelpers
+
+ let(:parsed_response) { JSON.parse(response.body) }
+ let(:response_data) { parsed_response['data'] }
+
+ before do
+ post_graphql(query)
+ end
+
+ it 'is returns a successfull response', :aggregate_failures do
+ expect(response).to be_success
+ expect(parsed_response['errors']).to be_nil
+ expect(response_data).not_to be_empty
+ end
+end