Merge request and commit discussions API

5 files changed, 198 insertions, 7 deletions

diff --git a/spec/fixtures/api/schemas/public_api/v4/notes.json b/spec/fixtures/api/schemas/public_api/v4/notes.json
index 4c4ca3b582f..0f9c221fd6d 100644
--- a/spec/fixtures/api/schemas/public_api/v4/notes.json
+++ b/spec/fixtures/api/schemas/public_api/v4/notes.json
@@ -24,7 +24,10 @@
       "system": { "type": "boolean" },
       "noteable_id": { "type": "integer" },
       "noteable_iid": { "type": "integer" },
-      "noteable_type": { "type": "string" }
+      "noteable_type": { "type": "string" },
+      "resolved": { "type": "boolean" },
+      "resolvable": { "type": "boolean" },
+      "resolved_by": { "type": ["string", "null"] }
     },
     "required": [
       "id", "body", "attachment", "author", "created_at", "updated_at",
diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb
index 4a44b219a67..ef34192f888 100644
--- a/spec/requests/api/discussions_spec.rb
+++ b/spec/requests/api/discussions_spec.rb
@@ -2,32 +2,53 @@ require 'spec_helper'
 
 describe API::Discussions do
   let(:user) { create(:user) }
-  let!(:project) { create(:project, :public, namespace: user.namespace) }
+  let!(:project) { create(:project, :public, :repository, namespace: user.namespace) }
   let(:private_user)    { create(:user) }
 
   before do
-    project.add_reporter(user)
+    project.add_developer(user)
   end
 
-  context "when noteable is an Issue" do
+  context 'when noteable is an Issue' do
     let!(:issue) { create(:issue, project: project, author: user) }
     let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) }
 
-    it_behaves_like "discussions API", 'projects', 'issues', 'iid' do
+    it_behaves_like 'discussions API', 'projects', 'issues', 'iid' do
       let(:parent) { project }
       let(:noteable) { issue }
       let(:note) { issue_note }
     end
   end
 
-  context "when noteable is a Snippet" do
+  context 'when noteable is a Snippet' do
     let!(:snippet) { create(:project_snippet, project: project, author: user) }
     let!(:snippet_note) { create(:discussion_note_on_snippet, noteable: snippet, project: project, author: user) }
 
-    it_behaves_like "discussions API", 'projects', 'snippets', 'id' do
+    it_behaves_like 'discussions API', 'projects', 'snippets', 'id' do
       let(:parent) { project }
       let(:noteable) { snippet }
       let(:note) { snippet_note }
     end
   end
+
+  context 'when noteable is a Merge Request' do
+    let!(:noteable) { create(:merge_request_with_diffs, source_project: project, target_project: project, author: user) }
+    let!(:note) { create(:discussion_note_on_merge_request, noteable: noteable, project: project, author: user) }
+    let!(:diff_note) { create(:diff_note_on_merge_request, noteable: noteable, project: project, author: user) }
+    let(:parent) { project }
+
+    it_behaves_like 'discussions API', 'projects', 'merge_requests', 'iid'
+    it_behaves_like 'diff discussions API', 'projects', 'merge_requests', 'iid'
+    it_behaves_like 'resolvable discussions API', 'projects', 'merge_requests', 'iid'
+  end
+
+  context 'when noteable is a Commit' do
+    let!(:noteable) { create(:commit, project: project, author: user) }
+    let!(:note) { create(:discussion_note_on_commit, commit_id: noteable.id, project: project, author: user) }
+    let!(:diff_note) { create(:diff_note_on_commit, commit_id: noteable.id, project: project, author: user) }
+    let(:parent) { project }
+
+    it_behaves_like 'discussions API', 'projects', 'repository/commits', 'id'
+    it_behaves_like 'diff discussions API', 'projects', 'repository/commits', 'id'
+  end
 end
diff --git a/spec/services/notes/resolve_service_spec.rb b/spec/services/notes/resolve_service_spec.rb
new file mode 100644
index 00000000000..b54d40a7a5c
--- /dev/null
+++ b/spec/services/notes/resolve_service_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe Notes::ResolveService do
+  let(:merge_request) { create(:merge_request) }
+  let(:note) { create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.project) }
+  let(:user) { merge_request.author }
+
+  describe '#execute' do
+    it "resolves the note" do
+      described_class.new(merge_request.project, user).execute(note)
+      note.reload
+
+      expect(note.resolved?).to be true
+      expect(note.resolved_by).to eq(user)
+    end
+
+    it "sends notifications if all discussions are resolved" do
+      expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(merge_request)
+
+      described_class.new(merge_request.project, user).execute(note)
+    end
+  end
+end
diff --git a/spec/support/shared_examples/requests/api/diff_discussions.rb b/spec/support/shared_examples/requests/api/diff_discussions.rb
new file mode 100644
index 00000000000..85a4bd8ca27
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/diff_discussions.rb
@@ -0,0 +1,57 @@
+shared_examples 'diff discussions API' do |parent_type, noteable_type, id_name|
+  describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
+    it "includes diff discussions" do
+      get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user)
+
+      discussion = json_response.find { |record| record['id'] == diff_note.discussion_id }
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(discussion).not_to be_nil
+      expect(discussion['individual_note']).to eq(false)
+      expect(discussion['notes'].first['body']).to eq(diff_note.note)
+    end
+  end
+
+  describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do
+    it "returns a discussion by id" do
+      get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{diff_note.discussion_id}", user)
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['id']).to eq(diff_note.discussion_id)
+      expect(json_response['notes'].first['body']).to eq(diff_note.note)
+      expect(json_response['notes'].first['position']).to eq(diff_note.position.to_h.stringify_keys)
+    end
+  end
+
+  describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
+    it "creates a new diff note" do
+      position = diff_note.position.to_h
+
+      post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!', position: position
+
+      expect(response).to have_gitlab_http_status(201)
+      expect(json_response['notes'].first['body']).to eq('hi!')
+      expect(json_response['notes'].first['type']).to eq('DiffNote')
+      expect(json_response['notes'].first['position']).to eq(position.stringify_keys)
+    end
+
+    it "returns a 400 bad request error when position is invalid" do
+      position = diff_note.position.to_h.merge(new_line: '100000')
+
+      post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!', position: position
+
+      expect(response).to have_gitlab_http_status(400)
+    end
+  end
+
+  describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do
+    it 'adds a new note to the diff discussion' do
+      post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+               "discussions/#{diff_note.discussion_id}/notes", user), body: 'hi!'
+
+      expect(response).to have_gitlab_http_status(201)
+      expect(json_response['body']).to eq('hi!')
+      expect(json_response['type']).to eq('DiffNote')
+    end
+  end
+end
diff --git a/spec/support/shared_examples/requests/api/resolvable_discussions.rb b/spec/support/shared_examples/requests/api/resolvable_discussions.rb
new file mode 100644
index 00000000000..408ad08cc48
--- /dev/null
+++ b/spec/support/shared_examples/requests/api/resolvable_discussions.rb
@@ -0,0 +1,87 @@
+shared_examples 'resolvable discussions API' do |parent_type, noteable_type, id_name|
+  describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do
+    it "resolves discussion if resolved is true" do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}", user), resolved: true
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['notes'].size).to eq(1)
+      expect(json_response['notes'][0]['resolved']).to eq(true)
+    end
+
+    it "unresolves discussion if resolved is false" do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}", user), resolved: false
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['notes'].size).to eq(1)
+      expect(json_response['notes'][0]['resolved']).to eq(false)
+    end
+
+    it "returns a 400 bad request error if resolved parameter is not passed" do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}", user)
+
+      expect(response).to have_gitlab_http_status(400)
+    end
+
+    it "returns a 401 unauthorized error if user is not authenticated" do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}"), resolved: true
+
+      expect(response).to have_gitlab_http_status(401)
+    end
+
+    it "returns a 403 error if user resolves discussion of someone else" do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}", private_user), resolved: true
+
+      expect(response).to have_gitlab_http_status(403)
+    end
+
+    context 'when user does not have access to read the discussion' do
+      before do
+        parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+      end
+
+      it 'responds with 404' do
+        put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+                "discussions/#{note.discussion_id}", private_user), resolved: true
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+
+  describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
+    it 'returns resolved note when resolved parameter is true' do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}/notes/#{note.id}", user), resolved: true
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response['resolved']).to eq(true)
+    end
+
+    it 'returns a 404 error when note id not found' do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}/notes/12345", user),
+              body: 'Hello!'
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    it 'returns a 400 bad request error if neither body nor resolved parameter is given' do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}/notes/#{note.id}", user)
+
+      expect(response).to have_gitlab_http_status(400)
+    end
+
+    it "returns a 403 error if user resolves note of someone else" do
+      put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
+              "discussions/#{note.discussion_id}/notes/#{note.id}", private_user), resolved: true
+
+      expect(response).to have_gitlab_http_status(403)
+    end
+  end
+end