Merge branch '247930-remove-resolve-procs-from-types-1' into 'master'

GraphQL: Remove use of resolve procs from type definition (Part 1)

See merge request gitlab-org/gitlab!42111

app/graphql/types/issue_type.rb

@@ -36,8 +36,7 @@ module Types
     end
 
     field :author, Types::UserType, null: false,
-          description: 'User that created the issue',
-          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, obj.author_id).find }
+          description: 'User that created the issue'
 
     field :assignees, Types::UserType.connection_type, null: true,
           description: 'Assignees of the issue'
@@ -45,16 +44,14 @@ module Types
     field :labels, Types::LabelType.connection_type, null: true,
           description: 'Labels of the issue'
     field :milestone, Types::MilestoneType, null: true,
-          description: 'Milestone of the issue',
-          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Milestone, obj.milestone_id).find }
+          description: 'Milestone of the issue'
 
     field :due_date, Types::TimeType, null: true,
           description: 'Due date of the issue'
     field :confidential, GraphQL::BOOLEAN_TYPE, null: false,
           description: 'Indicates the issue is confidential'
     field :discussion_locked, GraphQL::BOOLEAN_TYPE, null: false,
-          description: 'Indicates discussion is locked on the issue',
-          resolve: -> (obj, _args, _ctx) { !!obj.discussion_locked }
+          description: 'Indicates discussion is locked on the issue'
 
     field :upvotes, GraphQL::INT_TYPE, null: false,
           description: 'Number of upvotes the issue has received'
@@ -108,6 +105,18 @@ module Types
 
     field :severity, Types::IssuableSeverityEnum, null: true,
           description: 'Severity level of the incident'
+
+    def author
+      Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find
+    end
+
+    def milestone
+      Gitlab::Graphql::Loaders::BatchModelLoader.new(Milestone, object.milestone_id).find
+    end
+
+    def discussion_locked
+      !!object.discussion_locked
+    end
   end
 end
 

app/graphql/types/project_member_type.rb

@@ -12,7 +12,10 @@ module Types
     authorize :read_project
 
     field :project, Types::ProjectType, null: true,
-          description: 'Project that User is a member of',
-          resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, obj.source_id).find }
+          description: 'Project that User is a member of'
+
+    def project
+      Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, object.source_id).find
+    end
   end
 end

app/graphql/types/snippet_type.rb

@@ -24,16 +24,14 @@ module Types
     field :project, Types::ProjectType,
           description: 'The project the snippet is associated with',
           null: true,
-          authorize: :read_project,
-          resolve: -> (snippet, args, context) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, snippet.project_id).find }
+          authorize: :read_project
 
     # Author can be nil in some scenarios. For example,
     # when the admin setting restricted visibility
     # level is set to public
     field :author, Types::UserType,
           description: 'The owner of the snippet',
-          null: true,
-          resolve: -> (snippet, args, context) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, snippet.author_id).find }
+          null: true
 
     field :file_name, GraphQL::STRING_TYPE,
           description: 'File Name of the snippet',
@@ -86,5 +84,13 @@ module Types
           null: true
 
     markdown_field :description_html, null: true, method: :description
+
+    def author
+      Gitlab::Graphql::Loaders::BatchModelLoader.new(User, object.author_id).find
+    end
+
+    def project
+      Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, object.project_id).find
+    end
   end
 end

lib/gitlab/graphql/markdown_field.rb

@@ -12,13 +12,19 @@ module Gitlab
           end
 
           method_name = kwargs.delete(:method) || name.to_s.sub(/_html$/, '')
-          kwargs[:resolve] = Gitlab::Graphql::MarkdownField::Resolver.new(method_name.to_sym).proc
+          resolver_method = "#{name}_resolver".to_sym
+          kwargs[:resolver_method] = resolver_method
 
           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
+
+          define_method resolver_method do
+            # We need to `dup` the context so the MarkdownHelper doesn't modify it
+            ::MarkupHelper.markdown_field(object, method_name.to_sym, context.to_h.dup)
+          end
         end
       end
     end

lib/gitlab/graphql/markdown_field/resolver.rb

-# 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

spec/lib/gitlab/graphql/markdown_field/resolver_spec.rb

-# frozen_string_literal: true
-require 'spec_helper'
-
-RSpec.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

spec/lib/gitlab/graphql/markdown_field_spec.rb

@@ -2,6 +2,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Graphql::MarkdownField do
+  include Gitlab::Routing
+
   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']
@@ -13,7 +15,7 @@ RSpec.describe Gitlab::Graphql::MarkdownField do
     end
 
     context 'developer warnings' do
-      let(:expected_error) { /Only `method` is allowed to specify the markdown field/ }
+      let_it_be(: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') }
@@ -27,30 +29,61 @@ RSpec.describe Gitlab::Graphql::MarkdownField do
     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>' }
-      let(:query_type) { GraphQL::ObjectType.new }
-      let(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)}
-      let(:context) { GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: nil, object: nil) }
+      let_it_be(:note) { build(:note, note: '# Markdown!') }
+      let_it_be(:expected_markdown) { '<h1 data-sourcepos="1:1-1:11" dir="auto">Markdown!</h1>' }
+      let_it_be(:query_type) { GraphQL::ObjectType.new }
+      let_it_be(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)}
+      let_it_be(:query) { GraphQL::Query.new(schema, document: nil, context: {}, variables: {}) }
+      let_it_be(:context) { GraphQL::Query::Context.new(query: query, values: {}, object: nil) }
+
+      let(:type_class) { class_with_markdown_field(:note_html, null: false) }
+      let(:type_instance) { type_class.authorized_new(note, context) }
+      let(:field) { type_class.fields['noteHtml'] }
 
       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(type_instance, {}, context)).to eq(expected_markdown)
+      end
+
+      context 'when a `method` argument is passed' do
+        let(:type_class) { class_with_markdown_field(:test_html, null: false, method: :note) }
+        let(:field) { type_class.fields['testHtml'] }
 
-        expect(field.to_graphql.resolve(thing_with_markdown, {}, context)).to eq(expected_markdown)
+        it 'renders markdown from a specific property' do
+          expect(field.to_graphql.resolve(type_instance, {}, context)).to eq(expected_markdown)
         end
+      end
+
+      describe 'basic verification that references work' do
+        let_it_be(:project) { create(:project, :public) }
+        let(:issue) { create(:issue, project: project) }
+        let(:note) { build(:note, note: "Referencing #{issue.to_reference(full: true)}") }
 
-      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']
+        it 'renders markdown correctly' do
+          expect(field.to_graphql.resolve(type_instance, {}, context)).to include(issue_path(issue))
+        end
 
-        expect(field.to_graphql.resolve(thing_with_markdown, {}, context)).to eq(expected_markdown)
+        context 'when the issue is not publicly accessible' do
+          let_it_be(:project) { create(:project, :private) }
+
+          it 'hides the references from users that are not allowed to see the reference' do
+            expect(field.to_graphql.resolve(type_instance, {}, context)).not_to include(issue_path(issue))
+          end
+
+          it 'shows the reference to users that are allowed to see it' do
+            context = GraphQL::Query::Context.new(query: query, values: { current_user: project.owner }, object: nil)
+            type_instance = type_class.authorized_new(note, context)
+
+            expect(field.to_graphql.resolve(type_instance, {}, context)).to include(issue_path(issue))
+          end
+        end
       end
     end
   end
 
   def class_with_markdown_field(name, **args)
-    Class.new(GraphQL::Schema::Object) do
+    Class.new(Types::BaseObject) do
       prepend Gitlab::Graphql::MarkdownField
+      graphql_name 'MarkdownFieldTest'
 
       markdown_field name, **args
     end