diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2017-02-28 11:29:02 +0530 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2017-03-07 13:55:59 +0530 |
commit | 70584e6925698f9d5be335059f90f63a51821089 (patch) | |
tree | dd8a752fb7f19dbb716f0eec1c10a984094100be | |
parent | 402e0a2dd7cd9aa6766500b9b6809c97dfd40912 (diff) | |
download | gitlab-ce-70584e6925698f9d5be335059f90f63a51821089.tar.gz |
Migrate the AwardEmoji API to use `merge_request_iid` and `issue_iid`
- Instead of `merge_request_id` and `issue_id`
- The API also deals with `snippet_id`, which remains unchanged (since snippets
don't have `iid`s
- Duplicate the original `AwardEmoji` API (and spec) for use with the V3
API, since this is a breaking change.
-rw-r--r-- | lib/api/award_emoji.rb | 20 | ||||
-rw-r--r-- | lib/api/v3/award_emoji.rb | 79 | ||||
-rw-r--r-- | spec/requests/api/award_emoji_spec.rb | 50 | ||||
-rw-r--r-- | spec/requests/api/v3/award_emoji_spec.rb | 225 |
4 files changed, 337 insertions, 37 deletions
diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index 07a1bcdbe18..af16e016cdb 100644 --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -3,12 +3,16 @@ module API include PaginationParams before { authenticate! } - AWARDABLES = %w[issue merge_request snippet].freeze + AWARDABLES = [ + { type: 'issue', reference_by: :iid }, + { type: 'merge_request', reference_by: :iid }, + { type: 'snippet', reference_by: :id } + ].freeze resource :projects do - AWARDABLES.each do |awardable_type| - awardable_string = awardable_type.pluralize - awardable_id_string = "#{awardable_type}_id" + AWARDABLES.each do |awardable_params| + awardable_string = awardable_params[:type].pluralize + awardable_id_string = "#{awardable_params[:type]}_#{awardable_params[:reference_by]}" params do requires :id, type: String, desc: 'The ID of a project' @@ -104,10 +108,10 @@ module API note_id = params.delete(:note_id) awardable.notes.find(note_id) - elsif params.include?(:issue_id) - user_project.issues.find(params[:issue_id]) - elsif params.include?(:merge_request_id) - user_project.merge_requests.find(params[:merge_request_id]) + elsif params.include?(:issue_iid) + user_project.issues.find_by!(iid: params[:issue_iid]) + elsif params.include?(:merge_request_iid) + user_project.merge_requests.find_by!(iid: params[:merge_request_iid]) else user_project.snippets.find(params[:snippet_id]) end diff --git a/lib/api/v3/award_emoji.rb b/lib/api/v3/award_emoji.rb index 1e35283631f..c558c745b8c 100644 --- a/lib/api/v3/award_emoji.rb +++ b/lib/api/v3/award_emoji.rb @@ -16,11 +16,64 @@ module API requires :"#{awardable_id_string}", type: Integer, desc: "The ID of an Issue, Merge Request or Snippet" end - [":id/#{awardable_string}/:#{awardable_id_string}/award_emoji", - ":id/#{awardable_string}/:#{awardable_id_string}/notes/:note_id/award_emoji"].each do |endpoint| + [ + ":id/#{awardable_string}/:#{awardable_id_string}/award_emoji", + ":id/#{awardable_string}/:#{awardable_id_string}/notes/:note_id/award_emoji" + ].each do |endpoint| + + desc 'Get a list of project +awardable+ award emoji' do + detail 'This feature was introduced in 8.9' + success Entities::AwardEmoji + end + params do + use :pagination + end + get endpoint do + if can_read_awardable? + awards = awardable.award_emoji + present paginate(awards), with: Entities::AwardEmoji + else + not_found!("Award Emoji") + end + end + + desc 'Get a specific award emoji' do + detail 'This feature was introduced in 8.9' + success Entities::AwardEmoji + end + params do + requires :award_id, type: Integer, desc: 'The ID of the award' + end + get "#{endpoint}/:award_id" do + if can_read_awardable? + present awardable.award_emoji.find(params[:award_id]), with: Entities::AwardEmoji + else + not_found!("Award Emoji") + end + end + + desc 'Award a new Emoji' do + detail 'This feature was introduced in 8.9' + success Entities::AwardEmoji + end + params do + requires :name, type: String, desc: 'The name of a award_emoji (without colons)' + end + post endpoint do + not_found!('Award Emoji') unless can_read_awardable? && can_award_awardable? + + award = awardable.create_award_emoji(params[:name], current_user) + + if award.persisted? + present award, with: Entities::AwardEmoji + else + not_found!("Award Emoji #{award.errors.messages}") + end + end + desc 'Delete a +awardables+ award emoji' do detail 'This feature was introduced in 8.9' - success ::API::Entities::AwardEmoji + success Entities::AwardEmoji end params do requires :award_id, type: Integer, desc: 'The ID of an award emoji' @@ -30,13 +83,22 @@ module API unauthorized! unless award.user == current_user || current_user.admin? - present award.destroy, with: ::API::Entities::AwardEmoji + award.destroy + present award, with: ::API::Entities::AwardEmoji end end end end helpers do + def can_read_awardable? + can?(current_user, read_ability(awardable), awardable) + end + + def can_award_awardable? + awardable.user_can_award?(current_user, params[:name]) + end + def awardable @awardable ||= begin @@ -53,6 +115,15 @@ module API end end end + + def read_ability(awardable) + case awardable + when Note + read_ability(awardable.noteable) + else + :"read_#{awardable.class.to_s.underscore}" + end + end end end end diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 9756991162e..f4d4a8a2cc7 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -15,7 +15,7 @@ describe API::AwardEmoji, api: true do describe "GET /projects/:id/awardable/:awardable_id/award_emoji" do context 'on an issue' do it "returns an array of award_emoji" do - get api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user) expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -31,7 +31,7 @@ describe API::AwardEmoji, api: true do context 'on a merge request' do it "returns an array of award_emoji" do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji", user) expect(response).to have_http_status(200) expect(response).to include_pagination_headers @@ -57,7 +57,7 @@ describe API::AwardEmoji, api: true do it 'returns a status code 404' do user1 = create(:user) - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji", user1) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji", user1) expect(response).to have_http_status(404) end @@ -68,7 +68,7 @@ describe API::AwardEmoji, api: true do let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket') } it 'returns an array of award emoji' do - get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user) expect(response).to have_http_status(200) expect(json_response).to be_an Array @@ -79,7 +79,7 @@ describe API::AwardEmoji, api: true do describe "GET /projects/:id/awardable/:awardable_id/award_emoji/:award_id" do context 'on an issue' do it "returns the award emoji" do - get api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/#{award_emoji.id}", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/#{award_emoji.id}", user) expect(response).to have_http_status(200) expect(json_response['name']).to eq(award_emoji.name) @@ -88,7 +88,7 @@ describe API::AwardEmoji, api: true do end it "returns a 404 error if the award is not found" do - get api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/12345", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/12345", user) expect(response).to have_http_status(404) end @@ -96,7 +96,7 @@ describe API::AwardEmoji, api: true do context 'on a merge request' do it 'returns the award emoji' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji/#{downvote.id}", user) expect(response).to have_http_status(200) expect(json_response['name']).to eq(downvote.name) @@ -123,7 +123,7 @@ describe API::AwardEmoji, api: true do it 'returns a status code 404' do user1 = create(:user) - get api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user1) + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji/#{downvote.id}", user1) expect(response).to have_http_status(404) end @@ -134,7 +134,7 @@ describe API::AwardEmoji, api: true do let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket') } it 'returns an award emoji' do - get api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji/#{rocket.id}", user) + get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji/#{rocket.id}", user) expect(response).to have_http_status(200) expect(json_response).not_to be_an Array @@ -147,7 +147,7 @@ describe API::AwardEmoji, api: true do context "on an issue" do it "creates a new award emoji" do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'blowfish' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), name: 'blowfish' expect(response).to have_http_status(201) expect(json_response['name']).to eq('blowfish') @@ -155,13 +155,13 @@ describe API::AwardEmoji, api: true do end it "returns a 400 bad request error if the name is not given" do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user) + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user) expect(response).to have_http_status(400) end it "returns a 401 unauthorized error if the user is not authenticated" do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji"), name: 'thumbsup' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji"), name: 'thumbsup' expect(response).to have_http_status(401) end @@ -173,15 +173,15 @@ describe API::AwardEmoji, api: true do end it "normalizes +1 as thumbsup award" do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: '+1' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), name: '+1' expect(issue.award_emoji.last.name).to eq("thumbsup") end context 'when the emoji already has been awarded' do it 'returns a 404 status code' do - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup' - post api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), name: 'thumbsup' + post api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji", user), name: 'thumbsup' expect(response).to have_http_status(404) expect(json_response["message"]).to match("has already been taken") @@ -207,7 +207,7 @@ describe API::AwardEmoji, api: true do it 'creates a new award emoji' do expect do - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), name: 'rocket' end.to change { note.award_emoji.count }.from(0).to(1) expect(response).to have_http_status(201) @@ -215,21 +215,21 @@ describe API::AwardEmoji, api: true do end it "it returns 404 error when user authored note" do - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note2.id}/award_emoji", user), name: 'thumbsup' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note2.id}/award_emoji", user), name: 'thumbsup' expect(response).to have_http_status(404) end it "normalizes +1 as thumbsup award" do - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: '+1' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), name: '+1' expect(note.award_emoji.last.name).to eq("thumbsup") end context 'when the emoji already has been awarded' do it 'returns a 404 status code' do - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' - post api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), name: 'rocket' + post api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji", user), name: 'rocket' expect(response).to have_http_status(404) expect(json_response["message"]).to match("has already been taken") @@ -241,14 +241,14 @@ describe API::AwardEmoji, api: true do context 'when the awardable is an Issue' do it 'deletes the award' do expect do - delete api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/#{award_emoji.id}", user) + delete api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/#{award_emoji.id}", user) expect(response).to have_http_status(204) end.to change { issue.award_emoji.count }.from(1).to(0) end it 'returns a 404 error when the award emoji can not be found' do - delete api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/12345", user) + delete api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/12345", user) expect(response).to have_http_status(404) end @@ -257,14 +257,14 @@ describe API::AwardEmoji, api: true do context 'when the awardable is a Merge Request' do it 'deletes the award' do expect do - delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user) + delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji/#{downvote.id}", user) expect(response).to have_http_status(204) end.to change { merge_request.award_emoji.count }.from(1).to(0) end it 'returns a 404 error when note id not found' do - delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/notes/12345", user) + delete api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes/12345", user) expect(response).to have_http_status(404) end @@ -289,7 +289,7 @@ describe API::AwardEmoji, api: true do it 'deletes the award' do expect do - delete api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji/#{rocket.id}", user) + delete api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji/#{rocket.id}", user) expect(response).to have_http_status(204) end.to change { note.award_emoji.count }.from(1).to(0) diff --git a/spec/requests/api/v3/award_emoji_spec.rb b/spec/requests/api/v3/award_emoji_spec.rb index 91145c8e72c..eeb4d128c1b 100644 --- a/spec/requests/api/v3/award_emoji_spec.rb +++ b/spec/requests/api/v3/award_emoji_spec.rb @@ -13,6 +13,231 @@ describe API::V3::AwardEmoji, api: true do before { project.team << [user, :master] } + describe "GET /projects/:id/awardable/:awardable_id/award_emoji" do + context 'on an issue' do + it "returns an array of award_emoji" do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(award_emoji.name) + end + + it "returns a 404 error when issue id not found" do + get v3_api("/projects/#{project.id}/issues/12345/award_emoji", user) + + expect(response).to have_http_status(404) + end + end + + context 'on a merge request' do + it "returns an array of award_emoji" do + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(downvote.name) + end + end + + context 'on a snippet' do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:award) { create(:award_emoji, awardable: snippet) } + + it 'returns the awarded emoji' do + get v3_api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(award.name) + end + end + + context 'when the user has no access' do + it 'returns a status code 404' do + user1 = create(:user) + + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji", user1) + + expect(response).to have_http_status(404) + end + end + end + + describe 'GET /projects/:id/awardable/:awardable_id/notes/:note_id/award_emoji' do + let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket') } + + it 'returns an array of award emoji' do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).to eq(rocket.name) + end + end + + describe "GET /projects/:id/awardable/:awardable_id/award_emoji/:award_id" do + context 'on an issue' do + it "returns the award emoji" do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/#{award_emoji.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(award_emoji.name) + expect(json_response['awardable_id']).to eq(issue.id) + expect(json_response['awardable_type']).to eq("Issue") + end + + it "returns a 404 error if the award is not found" do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/12345", user) + + expect(response).to have_http_status(404) + end + end + + context 'on a merge request' do + it 'returns the award emoji' do + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(downvote.name) + expect(json_response['awardable_id']).to eq(merge_request.id) + expect(json_response['awardable_type']).to eq("MergeRequest") + end + end + + context 'on a snippet' do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:award) { create(:award_emoji, awardable: snippet) } + + it 'returns the awarded emoji' do + get v3_api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji/#{award.id}", user) + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq(award.name) + expect(json_response['awardable_id']).to eq(snippet.id) + expect(json_response['awardable_type']).to eq("Snippet") + end + end + + context 'when the user has no access' do + it 'returns a status code 404' do + user1 = create(:user) + + get v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user1) + + expect(response).to have_http_status(404) + end + end + end + + describe 'GET /projects/:id/awardable/:awardable_id/notes/:note_id/award_emoji/:award_id' do + let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket') } + + it 'returns an award emoji' do + get v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji/#{rocket.id}", user) + + expect(response).to have_http_status(200) + expect(json_response).not_to be_an Array + expect(json_response['name']).to eq(rocket.name) + end + end + + describe "POST /projects/:id/awardable/:awardable_id/award_emoji" do + let(:issue2) { create(:issue, project: project, author: user) } + + context "on an issue" do + it "creates a new award emoji" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'blowfish' + + expect(response).to have_http_status(201) + expect(json_response['name']).to eq('blowfish') + expect(json_response['user']['username']).to eq(user.username) + end + + it "returns a 400 bad request error if the name is not given" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user) + + expect(response).to have_http_status(400) + end + + it "returns a 401 unauthorized error if the user is not authenticated" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji"), name: 'thumbsup' + + expect(response).to have_http_status(401) + end + + it "returns a 404 error if the user authored issue" do + post v3_api("/projects/#{project.id}/issues/#{issue2.id}/award_emoji", user), name: 'thumbsup' + + expect(response).to have_http_status(404) + end + + it "normalizes +1 as thumbsup award" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: '+1' + + expect(issue.award_emoji.last.name).to eq("thumbsup") + end + + context 'when the emoji already has been awarded' do + it 'returns a 404 status code' do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup' + post v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji", user), name: 'thumbsup' + + expect(response).to have_http_status(404) + expect(json_response["message"]).to match("has already been taken") + end + end + end + + context 'on a snippet' do + it 'creates a new award emoji' do + snippet = create(:project_snippet, :public, project: project) + + post v3_api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji", user), name: 'blowfish' + + expect(response).to have_http_status(201) + expect(json_response['name']).to eq('blowfish') + expect(json_response['user']['username']).to eq(user.username) + end + end + end + + describe "POST /projects/:id/awardable/:awardable_id/notes/:note_id/award_emoji" do + let(:note2) { create(:note, project: project, noteable: issue, author: user) } + + it 'creates a new award emoji' do + expect do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + end.to change { note.award_emoji.count }.from(0).to(1) + + expect(response).to have_http_status(201) + expect(json_response['user']['username']).to eq(user.username) + end + + it "it returns 404 error when user authored note" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note2.id}/award_emoji", user), name: 'thumbsup' + + expect(response).to have_http_status(404) + end + + it "normalizes +1 as thumbsup award" do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: '+1' + + expect(note.award_emoji.last.name).to eq("thumbsup") + end + + context 'when the emoji already has been awarded' do + it 'returns a 404 status code' do + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + post v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji", user), name: 'rocket' + + expect(response).to have_http_status(404) + expect(json_response["message"]).to match("has already been taken") + end + end + end + describe 'DELETE /projects/:id/awardable/:awardable_id/award_emoji/:award_id' do context 'when the awardable is an Issue' do it 'deletes the award' do |