Expose comments on Noteables in GraphQL

This exposes `Note`s on Issues & MergeRequests using a
`Types::Notes::NoteableType` in GraphQL.

Exposing notes on a new type can be done by implementing the
`NoteableType` interface on the type. The presented object should
be a `Noteable`.

app/graphql/gitlab_schema.rb

@@ -7,8 +7,8 @@ class GitlabSchema < GraphQL::Schema
   AUTHENTICATED_COMPLEXITY = 250
   ADMIN_COMPLEXITY         = 300
-  DEFAULT_MAX_DEPTH = 10
-  AUTHENTICATED_MAX_DEPTH = 15
+  DEFAULT_MAX_DEPTH = 15
+  AUTHENTICATED_MAX_DEPTH = 20
   use BatchLoader::GraphQL
   use Gitlab::Graphql::Authorize

app/graphql/types/issue_type.rb

@@ -4,6 +4,8 @@ module Types
   class IssueType < BaseObject
     graphql_name 'Issue'
+    implements(Types::Notes::NoteableType)
+
     authorize :read_issue
     expose_permissions Types::PermissionTypes::Issue

app/graphql/types/merge_request_type.rb

@@ -4,6 +4,8 @@ module Types
   class MergeRequestType < BaseObject
     graphql_name 'MergeRequest'
+    implements(Types::Notes::NoteableType)
+
     authorize :read_merge_request
     expose_permissions Types::PermissionTypes::MergeRequest

app/graphql/types/notes/diff_position_type.rb

+# frozen_string_literal: true
+
+module Types
+  module Notes
+    class DiffPositionType < BaseObject
+      graphql_name 'DiffPosition'
+
+      field :head_sha, GraphQL::STRING_TYPE, null: false,
+            description: "The sha of the head at the time the comment was made"
+      field :base_sha,  GraphQL::STRING_TYPE, null: true,
+            description: "The merge base of the branch the comment was made on"
+      field :start_sha, GraphQL::STRING_TYPE, null: false,
+            description: "The sha of the branch being compared against"
+
+      field :file_path, GraphQL::STRING_TYPE, null: false,
+            description: "The path of the file that was changed"
+      field :old_path, GraphQL::STRING_TYPE, null: true,
+            description: "The path of the file on the start sha."
+      field :new_path, GraphQL::STRING_TYPE, null: true,
+            description: "The path of the file on the head sha."
+      field :position_type, Types::Notes::PositionTypeEnum, null: false
+
+      # Fields for text positions
+      field :old_line, GraphQL::INT_TYPE, null: true,
+            description: "The line on start sha that was changed",
+            resolve: -> (position, _args, _ctx) { position.old_line if position.on_text? }
+      field :new_line, GraphQL::INT_TYPE, null: true,
+            description: "The line on head sha that was changed",
+            resolve: -> (position, _args, _ctx) { position.new_line if position.on_text? }
+
+      # Fields for image positions
+      field :x, GraphQL::INT_TYPE, null: true,
+            description: "The X postion on which the comment was made",
+            resolve: -> (position, _args, _ctx) { position.x if position.on_image? }
+      field :y, GraphQL::INT_TYPE, null: true,
+            description: "The Y position on which the comment was made",
+            resolve: -> (position, _args, _ctx) { position.y if position.on_image? }
+      field :width, GraphQL::INT_TYPE, null: true,
+            description: "The total width of the image",
+            resolve: -> (position, _args, _ctx) { position.width if position.on_image? }
+      field :height, GraphQL::INT_TYPE, null: true,
+            description: "The total height of the image",
+            resolve: -> (position, _args, _ctx) { position.height if position.on_image? }
+    end
+  end
+end

app/graphql/types/notes/discussion_type.rb

+# frozen_string_literal: true
+
+module Types
+  module Notes
+    class DiscussionType < BaseObject
+      graphql_name 'Discussion'
+
+      authorize :read_note
+
+      field :id, GraphQL::ID_TYPE, null: false
+      field :created_at, Types::TimeType, null: false
+      field :notes, Types::Notes::NoteType.connection_type, null: false, description: "All notes in the discussion"
+    end
+  end
+end

app/graphql/types/notes/note_type.rb

+# frozen_string_literal: true
+
+module Types
+  module Notes
+    class NoteType < BaseObject
+      graphql_name 'Note'
+
+      authorize :read_note
+
+      expose_permissions Types::PermissionTypes::Note
+
+      field :id, GraphQL::ID_TYPE, null: false
+
+      field :project, Types::ProjectType,
+            null: true,
+            description: "The project this note is associated to",
+            resolve: -> (note, args, context) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Project, note.project_id).find }
+
+      field :author, Types::UserType,
+            null: false,
+            description: "The user who wrote this note",
+            resolve: -> (note, args, context) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, note.author_id).find }
+
+      field :resolved_by, Types::UserType,
+            null: true,
+            description: "The user that resolved the discussion",
+            resolve: -> (note, _args, _context) { Gitlab::Graphql::Loaders::BatchModelLoader.new(User, note.resolved_by_id).find }
+
+      field :system, GraphQL::BOOLEAN_TYPE,
+            null: false,
+            description: "Whether or not this note was created by the system or by a user"
+
+      field :body, GraphQL::STRING_TYPE,
+            null: false,
+            method: :note,
+            description: "The content note itself"
+
+      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"
+      field :resolvable, GraphQL::BOOLEAN_TYPE, null: false, method: :resolvable?
+      field :resolved_at, Types::TimeType, null: true, description: "The time the discussion was resolved"
+      field :position, Types::Notes::DiffPositionType, null: true, description: "The position of this note on a diff"
+    end
+  end
+end

app/graphql/types/notes/noteable_type.rb

+# frozen_string_literal: true
+
+module Types
+  module Notes
+    module NoteableType
+      include Types::BaseInterface
+
+      field :notes, Types::Notes::NoteType.connection_type, null: false, description: "All notes on this noteable"
+      field :discussions, Types::Notes::DiscussionType.connection_type, null: false, description: "All discussions on this noteable"
+
+      definition_methods do
+        def resolve_type(object, context)
+          case object
+          when Issue
+            Types::IssueType
+          when MergeRequest
+            Types::MergeRequestType
+          else
+            raise "Unknown GraphQL type for #{object}"
+          end
+        end
+      end
+    end
+  end
+end

app/graphql/types/notes/position_type_enum.rb

+# frozen_string_literal: true
+
+module Types
+  module Notes
+    class PositionTypeEnum < BaseEnum
+      graphql_name 'DiffPositionType'
+      description 'Type of file the position refers to'
+
+      value 'text'
+      value 'image'
+    end
+  end
+end

app/graphql/types/permission_types/note.rb

+# frozen_string_literal: true
+
+module Types
+  module PermissionTypes
+    class Note < BasePermissionType
+      graphql_name 'NotePermissions'
+
+      abilities :read_note, :create_note, :admin_note, :resolve_note, :award_emoji
+    end
+  end
+end

app/models/concerns/diff_positionable_note.rb

@@ -3,6 +3,7 @@ module DiffPositionableNote
   extend ActiveSupport::Concern
   included do
+    delegate :on_text?, :on_image?, to: :position, allow_nil: true
     before_validation :set_original_position, on: :create
     before_validation :update_position, on: :create, if: :on_text?
@@ -28,14 +29,6 @@ module DiffPositionableNote
     end
   end
-  def on_text?
-    position&.position_type == "text"
-  end
-
-  def on_image?
-    position&.position_type == "image"
-  end
-
   def supported?
     for_commit? || self.noteable.has_complete_diff_refs?
   end

app/models/discussion.rb

@@ -4,6 +4,7 @@
 #
 # A discussion of this type can be resolvable.
 class Discussion
+  include GlobalID::Identification
   include ResolvableDiscussion
   attr_reader :notes, :context_noteable
@@ -11,14 +12,19 @@ class Discussion
   delegate  :created_at,
             :project,
             :author,
-
             :noteable,
             :commit_id,
             :for_commit?,
             :for_merge_request?,
+            :to_ability_name,
+            :editable?,
             to: :first_note
+  def declarative_policy_delegate
+    first_note
+  end
+
   def project_id
     project&.id
   end

app/models/note.rb

@@ -342,7 +342,7 @@ class Note < ApplicationRecord
   end
   def to_ability_name
-    for_snippet? ? noteable.class.name.underscore : noteable_type.underscore
+    for_snippet? ? noteable.class.name.underscore : noteable_type.demodulize.underscore
   end
   def can_be_discussion_note?

changelogs/unreleased/bvl-comments-graphql.yml

+---
+title: Expose notes and discussions in GraphQL
+merge_request: 29212
+author:
+type: added

lib/gitlab/diff/position.rb

@@ -134,6 +134,14 @@ module Gitlab
         @line_code ||= diff_file(repository)&.line_code_for_position(self)
       end
+      def on_image?
+        position_type == 'image'
+      end
+
+      def on_text?
+        position_type == 'text'
+      end
+
       private
       def find_diff_file(repository)

spec/graphql/types/issue_type_spec.rb

@@ -7,6 +7,8 @@ describe GitlabSchema.types['Issue'] do
   it { expect(described_class).to require_graphql_authorizations(:read_issue) }
+  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|
       expect(described_class).to have_graphql_field(field_name)

spec/graphql/types/merge_request_type_spec.rb

@@ -5,6 +5,8 @@ describe GitlabSchema.types['MergeRequest'] do
   it { expect(described_class).to require_graphql_authorizations(:read_merge_request) }
+  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) }
   end

spec/graphql/types/notes/diff_position_type_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe GitlabSchema.types['DiffPosition'] do
+  it 'exposes the expected fields' do
+    expected_fields = [:head_sha, :base_sha, :start_sha, :file_path, :old_path,
+                       :new_path, :position_type, :old_line, :new_line, :x, :y,
+                       :width, :height]
+
+    is_expected.to have_graphql_field(*expected_fields)
+  end
+end

spec/graphql/types/notes/discussion_type_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe GitlabSchema.types['Discussion'] do
+  it { is_expected.to have_graphql_fields(:id, :created_at, :notes) }
+
+  it { is_expected.to require_graphql_authorizations(:read_note) }
+end

spec/graphql/types/notes/note_type_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+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]
+
+    is_expected.to have_graphql_fields(*expected_fields)
+  end
+
+  it { is_expected.to expose_permissions_using(Types::PermissionTypes::Note) }
+  it { is_expected.to require_graphql_authorizations(:read_note) }
+end

spec/graphql/types/notes/noteable_type_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Types::Notes::NoteableType do
+  it { is_expected.to have_graphql_fields(:notes, :discussions) }
+
+  describe ".resolve_type" do
+    it 'knows the correct type for objects' do
+      expect(described_class.resolve_type(build(:issue), {})).to eq(Types::IssueType)
+      expect(described_class.resolve_type(build(:merge_request), {})).to eq(Types::MergeRequestType)
+    end
+  end
+end