Merge branch 'bvl-comments-graphql' into 'master'

Expose comments on Noteables in GraphQL

Closes #62825

See merge request gitlab-org/gitlab-ce!29212

13 files changed, 285 insertions, 0 deletions

diff --git a/spec/graphql/types/issue_type_spec.rb b/spec/graphql/types/issue_type_spec.rb
index bae560829cc..210932f8488 100644
--- a/spec/graphql/types/issue_type_spec.rb
+++ b/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)
diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb
index 89c12879074..fd1c782bcc5 100644
--- a/spec/graphql/types/merge_request_type_spec.rb
+++ b/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
diff --git a/spec/graphql/types/notes/diff_position_type_spec.rb b/spec/graphql/types/notes/diff_position_type_spec.rb
new file mode 100644
index 00000000000..2f8724d7f0d
--- /dev/null
+++ b/spec/graphql/types/notes/diff_position_type_spec.rb
@@ -0,0 +1,12 @@
+# 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
diff --git a/spec/graphql/types/notes/discussion_type_spec.rb b/spec/graphql/types/notes/discussion_type_spec.rb
new file mode 100644
index 00000000000..2a1eb0efd35
--- /dev/null
+++ b/spec/graphql/types/notes/discussion_type_spec.rb
@@ -0,0 +1,8 @@
+# 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
diff --git a/spec/graphql/types/notes/note_type_spec.rb b/spec/graphql/types/notes/note_type_spec.rb
new file mode 100644
index 00000000000..8022b20f9dd
--- /dev/null
+++ b/spec/graphql/types/notes/note_type_spec.rb
@@ -0,0 +1,15 @@
+# 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
diff --git a/spec/graphql/types/notes/noteable_type_spec.rb b/spec/graphql/types/notes/noteable_type_spec.rb
new file mode 100644
index 00000000000..d10c79b5344
--- /dev/null
+++ b/spec/graphql/types/notes/noteable_type_spec.rb
@@ -0,0 +1,13 @@
+# 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
diff --git a/spec/graphql/types/permission_types/note_spec.rb b/spec/graphql/types/permission_types/note_spec.rb
new file mode 100644
index 00000000000..32d56eb1f7a
--- /dev/null
+++ b/spec/graphql/types/permission_types/note_spec.rb
@@ -0,0 +1,11 @@
+require 'spec_helper'
+
+describe GitlabSchema.types['NotePermissions'] do
+  it 'has the expected fields' do
+    expected_permissions = [
+      :read_note, :create_note, :admin_note, :resolve_note, :award_emoji
+    ]
+
+    is_expected.to have_graphql_fields(expected_permissions)
+  end
+end
diff --git a/spec/lib/gitlab/diff/position_spec.rb b/spec/lib/gitlab/diff/position_spec.rb
index cc4faf6f10b..aea02d21048 100644
--- a/spec/lib/gitlab/diff/position_spec.rb
+++ b/spec/lib/gitlab/diff/position_spec.rb
@@ -46,6 +46,9 @@ describe Gitlab::Diff::Position do
       )
     end
 
+    it { is_expected.to be_on_text }
+    it { is_expected.not_to be_on_image }
+
     describe "#diff_file" do
       it "returns the correct diff file" do
         diff_file = subject.diff_file(project.repository)
@@ -91,6 +94,9 @@ describe Gitlab::Diff::Position do
       )
     end
 
+    it { is_expected.not_to be_on_text }
+    it { is_expected.to be_on_image }
+
     it "returns the correct diff file" do
       diff_file = subject.diff_file(project.repository)
 
diff --git a/spec/models/discussion_spec.rb b/spec/models/discussion_spec.rb
index 0d02165787a..22d4dab0617 100644
--- a/spec/models/discussion_spec.rb
+++ b/spec/models/discussion_spec.rb
@@ -29,4 +29,12 @@ describe Discussion do
       ])
     end
   end
+
+  describe 'authorization' do
+    it 'delegates to the first note' do
+      policy = DeclarativePolicy.policy_for(instance_double(User, id: 1), subject)
+
+      expect(policy).to be_a(NotePolicy)
+    end
+  end
 end
diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb
index 4be7a0266d1..bcf021f1dfd 100644
--- a/spec/policies/note_policy_spec.rb
+++ b/spec/policies/note_policy_spec.rb
@@ -133,6 +133,25 @@ describe NotePolicy do
           end
         end
       end
+
+      context 'for discussions' do
+        let(:policy) { described_class.new(user, note.discussion) }
+
+        it 'allows the author to manage the discussion' do
+          expect(policy).to be_allowed(:admin_note)
+          expect(policy).to be_allowed(:resolve_note)
+          expect(policy).to be_allowed(:read_note)
+          expect(policy).to be_allowed(:award_emoji)
+        end
+
+        context 'when the user does not have access to the noteable' do
+          before do
+            noteable.update_attribute(:confidential, true)
+          end
+
+          it_behaves_like 'a discussion with a private noteable'
+        end
+      end
     end
   end
 end
diff --git a/spec/requests/api/graphql/project/issue/notes_spec.rb b/spec/requests/api/graphql/project/issue/notes_spec.rb
new file mode 100644
index 00000000000..bfc89434370
--- /dev/null
+++ b/spec/requests/api/graphql/project/issue/notes_spec.rb
@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'getting notes for an issue' do
+  include GraphqlHelpers
+
+  let(:noteable) { create(:issue) }
+  let(:noteable_data) { graphql_data['project']['issue'] }
+
+  def noteable_query(noteable_fields)
+    <<~QRY
+      {
+        project(fullPath: "#{noteable.project.full_path}") {
+          issue(iid: "#{noteable.iid}") {
+            #{noteable_fields}
+          }
+        }
+      }
+    QRY
+  end
+
+  it_behaves_like 'exposing regular notes on a noteable in GraphQL'
+end
diff --git a/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb b/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb
new file mode 100644
index 00000000000..e260e4463f4
--- /dev/null
+++ b/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb
@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'getting notes for a merge request' do
+  include GraphqlHelpers
+
+  let(:noteable) { create(:merge_request) }
+
+  def noteable_query(noteable_fields)
+    <<~QRY
+      {
+        project(fullPath: "#{noteable.project.full_path}") {
+          id
+          mergeRequest(iid: "#{noteable.iid}") {
+            #{noteable_fields}
+          }
+        }
+      }
+    QRY
+  end
+  let(:noteable_data) { graphql_data['project']['mergeRequest'] }
+
+  it_behaves_like "exposing regular notes on a noteable in GraphQL"
+
+  context 'diff notes on a merge request' do
+    let(:project) { noteable.project }
+    let!(:note) { create(:diff_note_on_merge_request, noteable: noteable, project: project) }
+    let(:user) { note.author }
+
+    let(:query) do
+      noteable_query(
+        <<~NOTES
+        notes {
+          edges {
+            node {
+              #{all_graphql_fields_for('Note')}
+            }
+          }
+        }
+      NOTES
+      )
+    end
+
+    it_behaves_like 'a working graphql query' do
+      before do
+        post_graphql(query, current_user: user)
+      end
+    end
+
+    it 'includes the note' do
+      post_graphql(query, current_user: user)
+
+      expect(graphql_data['project']['mergeRequest']['notes']['edges'].last['node']['body'])
+        .to eq(note.note)
+    end
+
+    context 'the position of the diffnote' do
+      it 'includes a correct position' do
+        post_graphql(query, current_user: user)
+
+        note_data = noteable_data['notes']['edges'].last['node']
+
+        expect(note_data['position']['positionType']).to eq('text')
+        expect(note_data['position']['newLine']).to be_present
+        expect(note_data['position']['x']).not_to be_present
+        expect(note_data['position']['y']).not_to be_present
+        expect(note_data['position']['width']).not_to be_present
+        expect(note_data['position']['height']).not_to be_present
+      end
+
+      context 'with a note on an image' do
+        let(:note) { create(:image_diff_note_on_merge_request, noteable: noteable, project: project) }
+
+        it 'includes a correct position' do
+          post_graphql(query, current_user: user)
+
+          note_data = noteable_data['notes']['edges'].last['node']
+
+          expect(note_data['position']['positionType']).to eq('image')
+          expect(note_data['position']['x']).to be_present
+          expect(note_data['position']['y']).to be_present
+          expect(note_data['position']['width']).to be_present
+          expect(note_data['position']['height']).to be_present
+          expect(note_data['position']['newLine']).not_to be_present
+        end
+      end
+    end
+  end
+end
diff --git a/spec/support/shared_examples/graphql/notes_on_noteables_shared_examples.rb b/spec/support/shared_examples/graphql/notes_on_noteables_shared_examples.rb
new file mode 100644
index 00000000000..323d1c51ffd
--- /dev/null
+++ b/spec/support/shared_examples/graphql/notes_on_noteables_shared_examples.rb
@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+shared_context 'exposing regular notes on a noteable in GraphQL' do
+  include GraphqlHelpers
+
+  let(:note) do
+    create(:note,
+           noteable: noteable,
+           project: (noteable.project if noteable.respond_to?(:project)))
+  end
+  let(:user) { note.author }
+
+  context 'for regular notes' do
+    let(:query) do
+      note_fields = <<~NOTES
+      notes {
+        edges {
+          node {
+            #{all_graphql_fields_for('Note')}
+          }
+        }
+      }
+      NOTES
+
+      noteable_query(note_fields)
+    end
+
+    it_behaves_like 'a working graphql query' do
+      before do
+        post_graphql(query, current_user: user)
+      end
+    end
+
+    it 'includes the note' do
+      post_graphql(query, current_user: user)
+
+      expect(noteable_data['notes']['edges'].first['node']['body'])
+        .to eq(note.note)
+    end
+  end
+
+  context "for discussions" do
+    let(:query) do
+      discussion_fields = <<~DISCUSSIONS
+      discussions {
+        edges {
+          node {
+            #{all_graphql_fields_for('Discussion')}
+          }
+        }
+      }
+      DISCUSSIONS
+
+      noteable_query(discussion_fields)
+    end
+
+    let!(:reply) { create(:note, noteable: noteable, in_reply_to: note, discussion_id: note.discussion_id) }
+
+    it_behaves_like 'a working graphql query' do
+      before do
+        post_graphql(query, current_user: user)
+      end
+    end
+
+    it 'includes all discussion notes' do
+      post_graphql(query, current_user: user)
+
+      discussion = noteable_data['discussions']['edges'].first['node']
+      ids = discussion['notes']['edges'].map { |note_edge| note_edge['node']['id'] }
+
+      expect(ids).to eq([note.to_global_id.to_s, reply.to_global_id.to_s])
+    end
+  end
+end