Commit a77b40d4 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'api-notes-entity-fields' into 'master'

Remove deprecated `upvotes` and `downvotes` from the notes API

Closes #28441

See merge request !9384
parents 8cc61d0b 931db796
---
title: 'API: Remove deprecated fields Notes#upvotes and Notes#downvotes'
merge_request: 9384
author: Robert Schilling
......@@ -34,8 +34,6 @@ Parameters:
"created_at": "2013-10-02T09:22:45Z",
"updated_at": "2013-10-02T10:22:45Z",
"system": true,
"upvote": false,
"downvote": false,
"noteable_id": 377,
"noteable_type": "Issue"
},
......@@ -54,8 +52,6 @@ Parameters:
"created_at": "2013-10-02T09:56:03Z",
"updated_at": "2013-10-02T09:56:03Z",
"system": true,
"upvote": false,
"downvote": false,
"noteable_id": 121,
"noteable_type": "Issue"
}
......@@ -147,9 +143,7 @@ Example Response:
"created_at": "2016-04-05T22:10:44.164Z",
"system": false,
"noteable_id": 11,
"noteable_type": "Issue",
"upvote": false,
"downvote": false
"noteable_type": "Issue"
}
```
......@@ -271,9 +265,7 @@ Example Response:
"created_at": "2016-04-06T16:51:53.239Z",
"system": false,
"noteable_id": 52,
"noteable_type": "Snippet",
"upvote": false,
"downvote": false
"noteable_type": "Snippet"
}
```
......@@ -322,8 +314,6 @@ Parameters:
"created_at": "2013-10-02T08:57:14Z",
"updated_at": "2013-10-02T08:57:14Z",
"system": false,
"upvote": false,
"downvote": false,
"noteable_id": 2,
"noteable_type": "MergeRequest"
}
......@@ -400,8 +390,6 @@ Example Response:
"created_at": "2016-04-05T22:11:59.923Z",
"system": false,
"noteable_id": 7,
"noteable_type": "MergeRequest",
"upvote": false,
"downvote": false
"noteable_type": "MergeRequest"
}
```
......@@ -407,8 +407,6 @@ Parameters:
},
"created_at": "2015-12-04T10:33:56.698Z",
"system": false,
"upvote": false,
"downvote": false,
"noteable_id": 377,
"noteable_type": "Issue"
},
......
......@@ -812,8 +812,6 @@ Example response:
},
"created_at": "2015-12-04T10:33:56.698Z",
"system": false,
"upvote": false,
"downvote": false,
"noteable_id": 377,
"noteable_type": "Issue"
},
......
......@@ -40,3 +40,4 @@ changes are in V4:
- POST/PUT/DELETE `:id/repository/files`
- Renamed `branch_name` to `branch` on DELETE `id/repository/branches/:branch` response [!8936](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8936)
- Remove `public` param from create and edit actions of projects [!8736](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8736)
- Notes do not return deprecated field `upvote` and `downvote` [!9384](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9384)
......@@ -15,6 +15,7 @@ module API
mount ::API::V3::Members
mount ::API::V3::MergeRequestDiffs
mount ::API::V3::MergeRequests
mount ::API::V3::Notes
mount ::API::V3::ProjectHooks
mount ::API::V3::Projects
mount ::API::V3::ProjectSnippets
......
......@@ -339,9 +339,6 @@ module API
expose :created_at, :updated_at
expose :system?, as: :system
expose :noteable_id, :noteable_type
# upvote? and downvote? are deprecated, always return false
expose(:upvote?) { |note| false }
expose(:downvote?) { |note| false }
end
class AwardEmoji < Grape::Entity
......
......@@ -11,6 +11,40 @@ module API
Gitlab::UrlBuilder.build(snippet)
end
end
class Note < Grape::Entity
expose :id
expose :note, as: :body
expose :attachment_identifier, as: :attachment
expose :author, using: ::API::Entities::UserBasic
expose :created_at, :updated_at
expose :system?, as: :system
expose :noteable_id, :noteable_type
# upvote? and downvote? are deprecated, always return false
expose(:upvote?) { |note| false }
expose(:downvote?) { |note| false }
end
class Event < Grape::Entity
expose :title, :project_id, :action_name
expose :target_id, :target_type, :author_id
expose :data, :target_title
expose :created_at
expose :note, using: Entities::Note, if: ->(event, options) { event.note? }
expose :author, using: ::API::Entities::UserBasic, if: ->(event, options) { event.author }
expose :author_username do |event, options|
event.author&.username
end
end
class AwardEmoji < Grape::Entity
expose :id
expose :name
expose :user, using: ::API::Entities::UserBasic
expose :created_at, :updated_at
expose :awardable_id, :awardable_type
end
end
end
end
module API
module V3
class Notes < Grape::API
include PaginationParams
before { authenticate! }
NOTEABLE_TYPES = [Issue, MergeRequest, Snippet]
params do
requires :id, type: String, desc: 'The ID of a project'
end
resource :projects do
NOTEABLE_TYPES.each do |noteable_type|
noteables_str = noteable_type.to_s.underscore.pluralize
desc 'Get a list of project +noteable+ notes' do
success ::API::V3::Entities::Note
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
use :pagination
end
get ":id/#{noteables_str}/:noteable_id/notes" do
noteable = user_project.send(noteables_str.to_sym).find(params[:noteable_id])
if can?(current_user, noteable_read_ability_name(noteable), noteable)
# We exclude notes that are cross-references and that cannot be viewed
# by the current user. By doing this exclusion at this level and not
# at the DB query level (which we cannot in that case), the current
# page can have less elements than :per_page even if
# there's more than one page.
notes =
# paginate() only works with a relation. This could lead to a
# mismatch between the pagination headers info and the actual notes
# array returned, but this is really a edge-case.
paginate(noteable.notes).
reject { |n| n.cross_reference_not_visible_for?(current_user) }
present notes, with: ::API::V3::Entities::Note
else
not_found!("Notes")
end
end
desc 'Get a single +noteable+ note' do
success ::API::V3::Entities::Note
end
params do
requires :note_id, type: Integer, desc: 'The ID of a note'
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
end
get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
noteable = user_project.send(noteables_str.to_sym).find(params[:noteable_id])
note = noteable.notes.find(params[:note_id])
can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user)
if can_read_note
present note, with: ::API::V3::Entities::Note
else
not_found!("Note")
end
end
desc 'Create a new +noteable+ note' do
success ::API::V3::Entities::Note
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
requires :body, type: String, desc: 'The content of a note'
optional :created_at, type: String, desc: 'The creation date of the note'
end
post ":id/#{noteables_str}/:noteable_id/notes" do
opts = {
note: params[:body],
noteable_type: noteables_str.classify,
noteable_id: params[:noteable_id]
}
noteable = user_project.send(noteables_str.to_sym).find(params[:noteable_id])
if can?(current_user, noteable_read_ability_name(noteable), noteable)
if params[:created_at] && (current_user.is_admin? || user_project.owner == current_user)
opts[:created_at] = params[:created_at]
end
note = ::Notes::CreateService.new(user_project, current_user, opts).execute
if note.valid?
present note, with: ::API::V3::Entities::const_get(note.class.name)
else
not_found!("Note #{note.errors.messages}")
end
else
not_found!("Note")
end
end
desc 'Update an existing +noteable+ note' do
success ::API::V3::Entities::Note
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
requires :note_id, type: Integer, desc: 'The ID of a note'
requires :body, type: String, desc: 'The content of a note'
end
put ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
note = user_project.notes.find(params[:note_id])
authorize! :admin_note, note
opts = {
note: params[:body]
}
note = ::Notes::UpdateService.new(user_project, current_user, opts).execute(note)
if note.valid?
present note, with: ::API::V3::Entities::Note
else
render_api_error!("Failed to save note #{note.errors.messages}", 400)
end
end
desc 'Delete a +noteable+ note' do
success ::API::V3::Entities::Note
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
requires :note_id, type: Integer, desc: 'The ID of a note'
end
delete ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
note = user_project.notes.find(params[:note_id])
authorize! :admin_note, note
::Notes::DestroyService.new(user_project, current_user).execute(note)
present note, with: ::API::V3::Entities::Note
end
end
end
helpers do
def noteable_read_ability_name(noteable)
"read_#{noteable.class.to_s.underscore}".to_sym
end
end
end
end
end
......@@ -232,13 +232,13 @@ module API
end
desc 'Get events for a single project' do
success ::API::Entities::Event
success ::API::V3::Entities::Event
end
params do
use :pagination
end
get ":id/events" do
present paginate(user_project.events.recent), with: ::API::Entities::Event
present paginate(user_project.events.recent), with: ::API::V3::Entities::Event
end
desc 'Fork new project for the current user or provided namespace.' do
......
......@@ -71,6 +71,27 @@ module API
user.activate
end
end
desc 'Get the contribution events of a specified user' do
detail 'This feature was introduced in GitLab 8.13.'
success ::API::V3::Entities::Event
end
params do
requires :id, type: Integer, desc: 'The ID of the user'
use :pagination
end
get ':id/events' do
user = User.find_by(id: params[:id])
not_found!('User') unless user
events = user.events.
merge(ProjectsFinder.new.execute(current_user)).
references(:project).
with_associations.
recent
present paginate(events), with: ::API::V3::Entities::Event
end
end
resource :user do
......
This diff is collapsed.
......@@ -186,4 +186,81 @@ describe API::V3::Users, api: true do
expect(response).to have_http_status(404)
end
end
describe 'GET /users/:id/events' do
let(:user) { create(:user) }
let(:project) { create(:empty_project) }
let(:note) { create(:note_on_issue, note: 'What an awesome day!', project: project) }
before do
project.add_user(user, :developer)
EventCreateService.new.leave_note(note, user)
end
context "as a user than cannot see the event's project" do
it 'returns no events' do
other_user = create(:user)
get api("/users/#{user.id}/events", other_user)
expect(response).to have_http_status(200)
expect(json_response).to be_empty
end
end
context "as a user than can see the event's project" do
context 'joined event' do
it 'returns the "joined" event' do
get v3_api("/users/#{user.id}/events", user)
expect(response).to have_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
comment_event = json_response.find { |e| e['action_name'] == 'commented on' }
expect(comment_event['project_id'].to_i).to eq(project.id)
expect(comment_event['author_username']).to eq(user.username)
expect(comment_event['note']['id']).to eq(note.id)
expect(comment_event['note']['body']).to eq('What an awesome day!')
joined_event = json_response.find { |e| e['action_name'] == 'joined' }
expect(joined_event['project_id'].to_i).to eq(project.id)
expect(joined_event['author_username']).to eq(user.username)
expect(joined_event['author']['name']).to eq(user.name)
end
end
context 'when there are multiple events from different projects' do
let(:second_note) { create(:note_on_issue, project: create(:empty_project)) }
let(:third_note) { create(:note_on_issue, project: project) }
before do
second_note.project.add_user(user, :developer)
[second_note, third_note].each do |note|
EventCreateService.new.leave_note(note, user)
end
end
it 'returns events in the correct order (from newest to oldest)' do
get v3_api("/users/#{user.id}/events", user)
comment_events = json_response.select { |e| e['action_name'] == 'commented on' }
expect(comment_events[0]['target_id']).to eq(third_note.id)
expect(comment_events[1]['target_id']).to eq(second_note.id)
expect(comment_events[2]['target_id']).to eq(note.id)
end
end
end
it 'returns a 404 error if not found' do
get v3_api('/users/42/events', user)
expect(response).to have_http_status(404)
expect(json_response['message']).to eq('404 User Not Found')
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