summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@gitlab.com>2019-06-20 08:02:33 +0000
committerDouwe Maan <douwe@gitlab.com>2019-06-20 08:02:33 +0000
commit406808583c2392a0b57f68f98b2418e9d23b23ab (patch)
tree98613723a6caf6dc152f9ff431e75906ebe2c5ca
parentadeccba13676b831335e2f12f779f77602298b31 (diff)
downloadgitlab-ce-406808583c2392a0b57f68f98b2418e9d23b23ab.tar.gz
Render GFM html in GraphQL
This adds a `markdown_field` to our types. Using this helper will render a model's markdown field using the existing `MarkupHelper` with the context of the GraphQL query available to the helper. Having the context available to the helper is needed for redacting links to resources that the current user is not allowed to see. Because rendering the HTML can cause queries, the complexity of a these fields is raised by 5 above the default. The markdown field helper can be used as follows: ``` markdown_field :note_html, null: false ``` This would generate a field that will render the markdown field `note` of the model. This could be overridden by adding the `method:` argument. Passing a symbol for the method name: ``` markdown_field :body_html, null: false, method: :note ``` It will have this description by default: > The GitLab Flavored Markdown rendering of `note` This could be overridden by passing a `description:` argument. The type of a `markdown_field` is always `GraphQL::STRING_TYPE`.
-rw-r--r--app/graphql/types/base_object.rb1
-rw-r--r--app/graphql/types/issue_type.rb2
-rw-r--r--app/graphql/types/label_type.rb1
-rw-r--r--app/graphql/types/merge_request_type.rb2
-rw-r--r--app/graphql/types/namespace_type.rb1
-rw-r--r--app/graphql/types/notes/note_type.rb2
-rw-r--r--app/graphql/types/project_type.rb1
-rw-r--r--app/helpers/markup_helper.rb2
-rw-r--r--changelogs/unreleased/bvl-markdown-graphql.yml5
-rw-r--r--lib/gitlab/graphql/markdown_field.rb26
-rw-r--r--lib/gitlab/graphql/markdown_field/resolver.rb22
-rw-r--r--spec/graphql/types/issue_type_spec.rb5
-rw-r--r--spec/graphql/types/label_type_spec.rb10
-rw-r--r--spec/graphql/types/merge_request_type_spec.rb17
-rw-r--r--spec/graphql/types/namespace_type_spec.rb9
-rw-r--r--spec/graphql/types/notes/note_type_spec.rb2
-rw-r--r--spec/graphql/types/project_type_spec.rb30
-rw-r--r--spec/lib/gitlab/graphql/markdown_field/resolver_spec.rb33
-rw-r--r--spec/lib/gitlab/graphql/markdown_field_spec.rb55
19 files changed, 207 insertions, 19 deletions
diff --git a/app/graphql/types/base_object.rb b/app/graphql/types/base_object.rb
index e40059c46bb..dad16898ba6 100644
--- a/app/graphql/types/base_object.rb
+++ b/app/graphql/types/base_object.rb
@@ -4,6 +4,7 @@ module Types
class BaseObject < GraphQL::Schema::Object
prepend Gitlab::Graphql::Present
prepend Gitlab::Graphql::ExposePermissions
+ prepend Gitlab::Graphql::MarkdownField
field_class Types::BaseField
diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb
index f2365499eee..8b208cab1df 100644
--- a/app/graphql/types/issue_type.rb
+++ b/app/graphql/types/issue_type.rb
@@ -14,7 +14,9 @@ module Types
field :iid, GraphQL::ID_TYPE, null: false
field :title, GraphQL::STRING_TYPE, null: false
+ markdown_field :title_html, null: true
field :description, GraphQL::STRING_TYPE, null: true
+ markdown_field :description_html, null: true
field :state, IssueStateEnum, null: false
field :reference, GraphQL::STRING_TYPE, null: false, method: :to_reference do
diff --git a/app/graphql/types/label_type.rb b/app/graphql/types/label_type.rb
index ccd466edc1a..50eb1b89c61 100644
--- a/app/graphql/types/label_type.rb
+++ b/app/graphql/types/label_type.rb
@@ -5,6 +5,7 @@ module Types
graphql_name 'Label'
field :description, GraphQL::STRING_TYPE, null: true
+ markdown_field :description_html, null: true
field :title, GraphQL::STRING_TYPE, null: false
field :color, GraphQL::STRING_TYPE, null: false
field :text_color, GraphQL::STRING_TYPE, null: false
diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb
index dac4c24cf10..577ccd48ef8 100644
--- a/app/graphql/types/merge_request_type.rb
+++ b/app/graphql/types/merge_request_type.rb
@@ -15,7 +15,9 @@ module Types
field :id, GraphQL::ID_TYPE, null: false
field :iid, GraphQL::STRING_TYPE, null: false
field :title, GraphQL::STRING_TYPE, null: false
+ markdown_field :title_html, null: true
field :description, GraphQL::STRING_TYPE, null: true
+ markdown_field :description_html, null: true
field :state, MergeRequestStateEnum, null: false
field :created_at, Types::TimeType, null: false
field :updated_at, Types::TimeType, null: false
diff --git a/app/graphql/types/namespace_type.rb b/app/graphql/types/namespace_type.rb
index f6d91320e50..62feccaa660 100644
--- a/app/graphql/types/namespace_type.rb
+++ b/app/graphql/types/namespace_type.rb
@@ -12,6 +12,7 @@ module Types
field :full_path, GraphQL::ID_TYPE, null: false
field :description, GraphQL::STRING_TYPE, null: true
+ markdown_field :description_html, null: true
field :visibility, GraphQL::STRING_TYPE, null: true
field :lfs_enabled, GraphQL::BOOLEAN_TYPE, null: true, method: :lfs_enabled?
field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true
diff --git a/app/graphql/types/notes/note_type.rb b/app/graphql/types/notes/note_type.rb
index 85c55d16ac2..fe54a45c7dc 100644
--- a/app/graphql/types/notes/note_type.rb
+++ b/app/graphql/types/notes/note_type.rb
@@ -35,6 +35,8 @@ module Types
method: :note,
description: "The content note itself"
+ markdown_field :body_html, null: true, method: :note
+
field :created_at, Types::TimeType, null: false
field :updated_at, Types::TimeType, null: false
field :discussion, Types::Notes::DiscussionType, null: true, description: "The discussion this note is a part of"
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb
index 81914b70c7f..ac957eafafc 100644
--- a/app/graphql/types/project_type.rb
+++ b/app/graphql/types/project_type.rb
@@ -17,6 +17,7 @@ module Types
field :name, GraphQL::STRING_TYPE, null: false
field :description, GraphQL::STRING_TYPE, null: true
+ markdown_field :description_html, null: true
field :tag_list, GraphQL::STRING_TYPE, null: true
diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb
index bf894360a2e..8ccb39f8444 100644
--- a/app/helpers/markup_helper.rb
+++ b/app/helpers/markup_helper.rb
@@ -278,7 +278,7 @@ module MarkupHelper
def prepare_for_rendering(html, context = {})
return '' unless html.present?
- context.merge!(
+ context.reverse_merge!(
current_user: (current_user if defined?(current_user)),
# RelativeLinkFilter
diff --git a/changelogs/unreleased/bvl-markdown-graphql.yml b/changelogs/unreleased/bvl-markdown-graphql.yml
new file mode 100644
index 00000000000..c2432b3ae81
--- /dev/null
+++ b/changelogs/unreleased/bvl-markdown-graphql.yml
@@ -0,0 +1,5 @@
+---
+title: Render GFM in GraphQL
+merge_request: 29700
+author:
+type: added
diff --git a/lib/gitlab/graphql/markdown_field.rb b/lib/gitlab/graphql/markdown_field.rb
new file mode 100644
index 00000000000..7be6810f7ba
--- /dev/null
+++ b/lib/gitlab/graphql/markdown_field.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Graphql
+ module MarkdownField
+ extend ActiveSupport::Concern
+
+ prepended do
+ def self.markdown_field(name, **kwargs)
+ if kwargs[:resolver].present? || kwargs[:resolve].present?
+ raise ArgumentError, 'Only `method` is allowed to specify the markdown field'
+ end
+
+ method_name = kwargs.delete(:method) || name.to_s.sub(/_html$/, '')
+ kwargs[:resolve] = Gitlab::Graphql::MarkdownField::Resolver.new(method_name.to_sym).proc
+
+ kwargs[:description] ||= "The GitLab Flavored Markdown rendering of `#{method_name}`"
+ # Adding complexity to rendered notes since that could cause queries.
+ kwargs[:complexity] ||= 5
+
+ field name, GraphQL::STRING_TYPE, **kwargs
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/graphql/markdown_field/resolver.rb b/lib/gitlab/graphql/markdown_field/resolver.rb
new file mode 100644
index 00000000000..11a01b95ad1
--- /dev/null
+++ b/lib/gitlab/graphql/markdown_field/resolver.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Graphql
+ module MarkdownField
+ class Resolver
+ attr_reader :method_name
+
+ def initialize(method_name)
+ @method_name = method_name
+ end
+
+ def proc
+ -> (object, _args, ctx) do
+ # We need to `dup` the context so the MarkdownHelper doesn't modify it
+ ::MarkupHelper.markdown_field(object, method_name, ctx.to_h.dup)
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/graphql/types/issue_type_spec.rb b/spec/graphql/types/issue_type_spec.rb
index 210932f8488..799a8662b94 100644
--- a/spec/graphql/types/issue_type_spec.rb
+++ b/spec/graphql/types/issue_type_spec.rb
@@ -10,7 +10,10 @@ describe GitlabSchema.types['Issue'] do
it { expect(described_class.interfaces).to include(Types::Notes::NoteableType.to_graphql) }
it 'has specific fields' do
- %i[relative_position web_path web_url reference].each do |field_name|
+ fields = %i[title_html description_html relative_position web_path web_url
+ reference]
+
+ fields.each do |field_name|
expect(described_class).to have_graphql_field(field_name)
end
end
diff --git a/spec/graphql/types/label_type_spec.rb b/spec/graphql/types/label_type_spec.rb
new file mode 100644
index 00000000000..f498b32f9ed
--- /dev/null
+++ b/spec/graphql/types/label_type_spec.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe GitlabSchema.types['Label'] do
+ it 'has the correct fields' do
+ expected_fields = [:description, :description_html, :title, :color, :text_color]
+
+ is_expected.to have_graphql_fields(*expected_fields)
+ end
+end
diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb
index fd1c782bcc5..f73bd062369 100644
--- a/spec/graphql/types/merge_request_type_spec.rb
+++ b/spec/graphql/types/merge_request_type_spec.rb
@@ -7,7 +7,20 @@ describe GitlabSchema.types['MergeRequest'] do
it { expect(described_class.interfaces).to include(Types::Notes::NoteableType.to_graphql) }
- describe 'nested head pipeline' do
- it { expect(described_class).to have_graphql_field(:head_pipeline) }
+ it 'has the expected fields' do
+ expected_fields = %w[
+ notes discussions user_permissions id iid title title_html description
+ description_html state created_at updated_at source_project target_project
+ project project_id source_project_id target_project_id source_branch
+ target_branch work_in_progress merge_when_pipeline_succeeds diff_head_sha
+ merge_commit_sha user_notes_count should_remove_source_branch
+ force_remove_source_branch merge_status in_progress_merge_commit_sha
+ merge_error allow_collaboration should_be_rebased rebase_commit_sha
+ rebase_in_progress merge_commit_message default_merge_commit_message
+ merge_ongoing source_branch_exists mergeable_discussions_state web_url
+ upvotes downvotes subscribed head_pipeline pipelines task_completion_status
+ ]
+
+ is_expected.to have_graphql_fields(*expected_fields)
end
end
diff --git a/spec/graphql/types/namespace_type_spec.rb b/spec/graphql/types/namespace_type_spec.rb
index b4144cc4121..77fd590586e 100644
--- a/spec/graphql/types/namespace_type_spec.rb
+++ b/spec/graphql/types/namespace_type_spec.rb
@@ -5,5 +5,12 @@ require 'spec_helper'
describe GitlabSchema.types['Namespace'] do
it { expect(described_class.graphql_name).to eq('Namespace') }
- it { expect(described_class).to have_graphql_field(:projects) }
+ it 'has the expected fields' do
+ expected_fields = %w[
+ id name path full_name full_path description description_html visibility
+ lfs_enabled request_access_enabled projects
+ ]
+
+ is_expected.to have_graphql_fields(*expected_fields)
+ end
end
diff --git a/spec/graphql/types/notes/note_type_spec.rb b/spec/graphql/types/notes/note_type_spec.rb
index 8022b20f9dd..e8a58da4b17 100644
--- a/spec/graphql/types/notes/note_type_spec.rb
+++ b/spec/graphql/types/notes/note_type_spec.rb
@@ -5,7 +5,7 @@ describe GitlabSchema.types['Note'] do
it 'exposes the expected fields' do
expected_fields = [:id, :project, :author, :body, :created_at,
:updated_at, :discussion, :resolvable, :position, :user_permissions,
- :resolved_by, :resolved_at, :system]
+ :resolved_by, :resolved_at, :system, :body_html]
is_expected.to have_graphql_fields(*expected_fields)
end
diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb
index cb5ac2e3cb1..69fbc72bdf5 100644
--- a/spec/graphql/types/project_type_spec.rb
+++ b/spec/graphql/types/project_type_spec.rb
@@ -7,18 +7,22 @@ describe GitlabSchema.types['Project'] do
it { expect(described_class).to require_graphql_authorizations(:read_project) }
- describe 'nested merge request' do
- it { expect(described_class).to have_graphql_field(:merge_requests) }
- it { expect(described_class).to have_graphql_field(:merge_request) }
+ it 'has the expected fields' do
+ expected_fields = %w[
+ user_permissions id full_path path name_with_namespace
+ name description description_html tag_list ssh_url_to_repo
+ http_url_to_repo web_url star_count forks_count
+ created_at last_activity_at archived visibility
+ container_registry_enabled shared_runners_enabled
+ lfs_enabled merge_requests_ff_only_enabled avatar_url
+ issues_enabled merge_requests_enabled wiki_enabled
+ snippets_enabled jobs_enabled public_jobs open_issues_count import_status
+ only_allow_merge_if_pipeline_succeeds request_access_enabled
+ only_allow_merge_if_all_discussions_are_resolved printing_merge_request_link_enabled
+ namespace group statistics repository merge_requests merge_request issues
+ issue pipelines
+ ]
+
+ is_expected.to have_graphql_fields(*expected_fields)
end
-
- describe 'nested issues' do
- it { expect(described_class).to have_graphql_field(:issues) }
- end
-
- it { is_expected.to have_graphql_field(:pipelines) }
-
- it { is_expected.to have_graphql_field(:repository) }
-
- it { is_expected.to have_graphql_field(:statistics) }
end
diff --git a/spec/lib/gitlab/graphql/markdown_field/resolver_spec.rb b/spec/lib/gitlab/graphql/markdown_field/resolver_spec.rb
new file mode 100644
index 00000000000..b95bcdef188
--- /dev/null
+++ b/spec/lib/gitlab/graphql/markdown_field/resolver_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::Graphql::MarkdownField::Resolver do
+ include Gitlab::Routing
+ let(:resolver) { described_class.new(:note) }
+
+ describe '#proc' do
+ let(:project) { create(:project, :public) }
+ let(:issue) { create(:issue, project: project) }
+ let(:note) do
+ create(:note,
+ note: "Referencing #{issue.to_reference(full: true)}")
+ end
+
+ it 'renders markdown correctly' do
+ expect(resolver.proc.call(note, {}, {})).to include(issue_path(issue))
+ end
+
+ context 'when the issue is not publicly accessible' do
+ let(:project) { create(:project, :private) }
+
+ it 'hides the references from users that are not allowed to see the reference' do
+ expect(resolver.proc.call(note, {}, {})).not_to include(issue_path(issue))
+ end
+
+ it 'shows the reference to users that are allowed to see it' do
+ expect(resolver.proc.call(note, {}, { current_user: project.owner }))
+ .to include(issue_path(issue))
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/graphql/markdown_field_spec.rb b/spec/lib/gitlab/graphql/markdown_field_spec.rb
new file mode 100644
index 00000000000..a8566aa8e1c
--- /dev/null
+++ b/spec/lib/gitlab/graphql/markdown_field_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::Graphql::MarkdownField do
+ describe '.markdown_field' do
+ it 'creates the field with some default attributes' do
+ field = class_with_markdown_field(:test_html, null: true, method: :hello).fields['testHtml']
+
+ expect(field.name).to eq('testHtml')
+ expect(field.description).to eq('The GitLab Flavored Markdown rendering of `hello`')
+ expect(field.type).to eq(GraphQL::STRING_TYPE)
+ expect(field.to_graphql.complexity).to eq(5)
+ end
+
+ context 'developer warnings' do
+ let(:expected_error) { /Only `method` is allowed to specify the markdown field/ }
+
+ it 'raises when passing a resolver' do
+ expect { class_with_markdown_field(:test_html, null: true, resolver: 'not really') }
+ .to raise_error(expected_error)
+ end
+
+ it 'raises when passing a resolve block' do
+ expect { class_with_markdown_field(:test_html, null: true, resolve: -> (_, _, _) { 'not really' } ) }
+ .to raise_error(expected_error)
+ end
+ end
+
+ context 'resolving markdown' do
+ let(:note) { build(:note, note: '# Markdown!') }
+ let(:thing_with_markdown) { double('markdown thing', object: note) }
+ let(:expected_markdown) { '<h1 data-sourcepos="1:1-1:11" dir="auto">Markdown!</h1>' }
+
+ it 'renders markdown from the same property as the field name without the `_html` suffix' do
+ field = class_with_markdown_field(:note_html, null: false).fields['noteHtml']
+
+ expect(field.to_graphql.resolve(thing_with_markdown, {}, {})).to eq(expected_markdown)
+ end
+
+ it 'renders markdown from a specific property when a `method` argument is passed' do
+ field = class_with_markdown_field(:test_html, null: false, method: :note).fields['testHtml']
+
+ expect(field.to_graphql.resolve(thing_with_markdown, {}, {})).to eq(expected_markdown)
+ end
+ end
+ end
+
+ def class_with_markdown_field(name, **args)
+ Class.new(GraphQL::Schema::Object) do
+ prepend Gitlab::Graphql::MarkdownField
+
+ markdown_field name, **args
+ end
+ end
+end