Delete notes via API

7 files changed, 135 insertions, 3 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 593e8f77ab4..f4dbab8889a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -46,6 +46,11 @@ v 8.6.5
   - Return status code 303 after a branch DELETE operation to avoid project deletion (Stan Hu). !3583
   - Unblock user when active_directory is disabled and it can be found !3550
   - Fix a 2FA authentication spoofing vulnerability.
+  - API: Delete notes of issues, snippets, and merge requests (Robert Schilling)
+
+v 8.6.5 (unreleased)
+  - Only update repository language if it is not set to improve performance
+  - Check permissions when user attempts to import members from another project
 
 v 8.6.4
   - Don't attempt to fetch any tags from a forked repo (Stan Hu)
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index 1b9dd568043..a9a69573eed 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -39,8 +39,7 @@ class Projects::NotesController < Projects::ApplicationController
 
   def destroy
     if note.editable?
-      note.destroy
-      note.reset_events_cache
+      Notes::DeleteService.new(project, current_user).execute(note)
     end
 
     respond_to do |format|
@@ -73,7 +72,7 @@ class Projects::NotesController < Projects::ApplicationController
     note = noteable.notes.find_by(data)
 
     if note
-      note.destroy
+      Notes::DeleteService.new(project, current_user).execute(note)
     else
       Notes::CreateService.new(project, current_user, note_params).execute
     end
diff --git a/app/services/notes/delete_service.rb b/app/services/notes/delete_service.rb
new file mode 100644
index 00000000000..7f1b30ec84e
--- /dev/null
+++ b/app/services/notes/delete_service.rb
@@ -0,0 +1,8 @@
+module Notes
+  class DeleteService < BaseService
+    def execute(note)
+      note.destroy
+      note.reset_events_cache
+    end
+  end
+end
diff --git a/doc/api/notes.md b/doc/api/notes.md
index 9168ab00d7e..82494bf83ff 100644
--- a/doc/api/notes.md
+++ b/doc/api/notes.md
@@ -105,6 +105,21 @@ Parameters:
 - `note_id` (required) - The ID of a note
 - `body` (required) - The content of a note
 
+### Delete existing issue note
+
+Deletes an existing note of an issue. On success, this API method returns 200.
+If the note does not exist, the API returns 404.
+
+```
+DELETE /projects/:id/issues/:issue_id/notes/:note_id
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `issue_id` (required) - The ID of an issue
+- `note_id` (required) - The ID of a note
+
 ## Snippets
 
 ### List all snippet notes
@@ -182,6 +197,21 @@ Parameters:
 - `note_id` (required) - The ID of a note
 - `body` (required) - The content of a note
 
+### Delete existing snippet note
+
+Deletes an existing note of a snippet. On success, this API method returns 200.
+If the note does not exist, the API returns 404.
+
+```
+DELETE /projects/:id/snippets/:snippet_id/notes/:note_id
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `snippet_id` (required) - The ID of a snippet
+- `note_id` (required) - The ID of a note
+
 ## Merge Requests
 
 ### List all merge request notes
@@ -262,3 +292,18 @@ Parameters:
 - `merge_request_id` (required) - The ID of a merge request
 - `note_id` (required) - The ID of a note
 - `body` (required) - The content of a note
+
+### Delete existing snippet note
+
+Deletes an existing note of a merge request. On success, this API method returns
+200. If the note does not exist, the API returns 404.
+
+```
+DELETE /projects/:id/merge_requests/:merge_request_id/notes/:note_id
+```
+
+Parameters:
+
+- `id` (required) - The ID of a project
+- `merge_request_id` (required) - The ID of a merge request
+- `note_id` (required) - The ID of a note
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 174473f5371..fd1704d395c 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -112,6 +112,23 @@ module API
           end
         end
 
+        # Delete a +notable+ note
+        #
+        # Parameters:
+        # Parameters:
+        #   id (required) - The ID of a project
+        #   noteable_id (required) - The ID of an issue, MR, or snippet
+        #   node_id (required) - The ID of a note
+        # Example Request:
+        #   DELETE /projects/:id/issues/:noteable_id/notes/:note_id
+        #   DELETE /projects/:id/snippets/:noteable_id/notes/:node_id
+        delete ":id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id" do
+          note = user_project.notes.find(params[:note_id])
+          not_found!('Note') unless note
+          authorize! :admin_note, note
+          ::Notes::DeleteService.new(user_project, current_user).execute(note)
+          true
+        end
       end
     end
   end
diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb
index 39f9a06fe1b..23d3c63bc1c 100644
--- a/spec/requests/api/notes_spec.rb
+++ b/spec/requests/api/notes_spec.rb
@@ -241,4 +241,47 @@ describe API::API, api: true  do
     end
   end
 
+  describe ':id/#{noteables_str}/:#{noteable_id_str}/notes/:note_id' do
+    context 'when noteable is an Issue' do
+      it 'should delete a note' do
+        delete api("/projects/#{project.id}/issues/#{issue.id}/"\
+                   "notes/#{issue_note.id}", user)
+        expect(response.status).to eq(200)
+      end
+
+      it 'should return a 404 error when note id not found' do
+        delete api("/projects/#{project.id}/issues/#{issue.id}/notes/123", user)
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'when noteable is a Snippet' do
+      it 'should delete a note' do
+        delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\
+                   "notes/#{snippet_note.id}", user)
+        expect(response.status).to eq(200)
+      end
+
+      it 'should return a 404 error when note id not found' do
+        delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\
+                   "notes/123", user)
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'when noteable is a Merge Request' do
+      it 'should delete a note' do
+        delete api("/projects/#{project.id}/merge_requests/"\
+                   "#{merge_request.id}/notes/#{merge_request_note.id}", user)
+        expect(response.status).to eq(200)
+      end
+
+      it 'should return a 404 error when note id not found' do
+        delete api("/projects/#{project.id}/merge_requests/"\
+                   "#{merge_request.id}/notes/123", user)
+        expect(response.status).to eq(404)
+      end
+    end
+  end
+
 end
diff --git a/spec/services/notes/delete_service_spec.rb b/spec/services/notes/delete_service_spec.rb
new file mode 100644
index 00000000000..88e71c135d3
--- /dev/null
+++ b/spec/services/notes/delete_service_spec.rb
@@ -0,0 +1,15 @@
+require 'spec_helper'
+
+describe Notes::DeleteService, services: true do
+  let(:project) { create(:empty_project) }
+  let(:issue) { create(:issue, project: project) }
+  let(:user) { create(:user) }
+  let(:note) { create(:note, project: project, noteable: issue, author: user, note: 'Note') }
+
+  describe '#execute' do
+    it 'deletes a note' do
+      Notes::DeleteService.new(project, user).execute(note)
+      expect(project.issues.find(issue.id).notes).to_not include(note)
+    end
+  end
+end