Commit ba21c00f authored by Robert Schilling's avatar Robert Schilling

Delete notes via API

parent 734df1bb
...@@ -46,6 +46,11 @@ v 8.6.5 ...@@ -46,6 +46,11 @@ v 8.6.5
- Return status code 303 after a branch DELETE operation to avoid project deletion (Stan Hu). !3583 - 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 - Unblock user when active_directory is disabled and it can be found !3550
- Fix a 2FA authentication spoofing vulnerability. - 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 v 8.6.4
- Don't attempt to fetch any tags from a forked repo (Stan Hu) - Don't attempt to fetch any tags from a forked repo (Stan Hu)
......
...@@ -39,8 +39,7 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -39,8 +39,7 @@ class Projects::NotesController < Projects::ApplicationController
def destroy def destroy
if note.editable? if note.editable?
note.destroy Notes::DeleteService.new(project, current_user).execute(note)
note.reset_events_cache
end end
respond_to do |format| respond_to do |format|
...@@ -73,7 +72,7 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -73,7 +72,7 @@ class Projects::NotesController < Projects::ApplicationController
note = noteable.notes.find_by(data) note = noteable.notes.find_by(data)
if note if note
note.destroy Notes::DeleteService.new(project, current_user).execute(note)
else else
Notes::CreateService.new(project, current_user, note_params).execute Notes::CreateService.new(project, current_user, note_params).execute
end end
......
module Notes
class DeleteService < BaseService
def execute(note)
note.destroy
note.reset_events_cache
end
end
end
...@@ -105,6 +105,21 @@ Parameters: ...@@ -105,6 +105,21 @@ Parameters:
- `note_id` (required) - The ID of a note - `note_id` (required) - The ID of a note
- `body` (required) - The content 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 ## Snippets
### List all snippet notes ### List all snippet notes
...@@ -182,6 +197,21 @@ Parameters: ...@@ -182,6 +197,21 @@ Parameters:
- `note_id` (required) - The ID of a note - `note_id` (required) - The ID of a note
- `body` (required) - The content 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 ## Merge Requests
### List all merge request notes ### List all merge request notes
...@@ -262,3 +292,18 @@ Parameters: ...@@ -262,3 +292,18 @@ Parameters:
- `merge_request_id` (required) - The ID of a merge request - `merge_request_id` (required) - The ID of a merge request
- `note_id` (required) - The ID of a note - `note_id` (required) - The ID of a note
- `body` (required) - The content 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
...@@ -112,6 +112,23 @@ module API ...@@ -112,6 +112,23 @@ module API
end end
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 end
end end
......
...@@ -241,4 +241,47 @@ describe API::API, api: true do ...@@ -241,4 +241,47 @@ describe API::API, api: true do
end end
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 end
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment