API routes referencing a specific issue should use the issue `iid`

- As opposed to the issue `id` that was previously being used.
- This brings the API routes closer to the web interface's routes.
- This is specific to API v4.
parent 6b2d4947
......@@ -5,6 +5,8 @@ class API < Grape::API
version %w(v3 v4), using: :path
version 'v3', using: :path do
helpers ::API::V3::Helpers
mount ::API::V3::AwardEmoji
mount ::API::V3::Boards
mount ::API::V3::Branches
......
......@@ -82,8 +82,8 @@ def find_project_label(id)
label || not_found!('Label')
end
def find_project_issue(id)
IssuesFinder.new(current_user, project_id: user_project.id).find(id)
def find_project_issue(iid)
IssuesFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
end
def find_project_merge_request(id)
......
......@@ -102,10 +102,10 @@ def find_issues(args = {})
success Entities::Issue
end
params do
requires :issue_id, type: Integer, desc: 'The ID of a project issue'
requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
end
get ":id/issues/:issue_id" do
issue = find_project_issue(params[:issue_id])
get ":id/issues/:issue_iid" do
issue = find_project_issue(params[:issue_iid])
present issue, with: Entities::Issue, current_user: current_user, project: user_project
end
......@@ -152,7 +152,7 @@ def find_issues(args = {})
success Entities::Issue
end
params do
requires :issue_id, type: Integer, desc: 'The ID of a project issue'
requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
optional :title, type: String, desc: 'The title of an issue'
optional :updated_at, type: DateTime,
desc: 'Date time when the issue was updated. Available only for admins and project owners.'
......@@ -161,8 +161,8 @@ def find_issues(args = {})
at_least_one_of :title, :description, :assignee_id, :milestone_id,
:labels, :created_at, :due_date, :confidential, :state_event
end
put ':id/issues/:issue_id' do
issue = user_project.issues.find(params.delete(:issue_id))
put ':id/issues/:issue_iid' do
issue = user_project.issues.find_by!(iid: params.delete(:issue_iid))
authorize! :update_issue, issue
# Setting created_at time only allowed for admins and project owners
......@@ -189,11 +189,11 @@ def find_issues(args = {})
success Entities::Issue
end
params do
requires :issue_id, type: Integer, desc: 'The ID of a project issue'
requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
requires :to_project_id, type: Integer, desc: 'The ID of the new project'
end
post ':id/issues/:issue_id/move' do
issue = user_project.issues.find_by(id: params[:issue_id])
post ':id/issues/:issue_iid/move' do
issue = user_project.issues.find_by(iid: params[:issue_iid])
not_found!('Issue') unless issue
new_project = Project.find_by(id: params[:to_project_id])
......@@ -209,10 +209,10 @@ def find_issues(args = {})
desc 'Delete a project issue'
params do
requires :issue_id, type: Integer, desc: 'The ID of a project issue'
requires :issue_iid, type: Integer, desc: 'The internal ID of a project issue'
end
delete ":id/issues/:issue_id" do
issue = user_project.issues.find_by(id: params[:issue_id])
delete ":id/issues/:issue_iid" do
issue = user_project.issues.find_by(iid: params[:issue_iid])
not_found!('Issue') unless issue
authorize!(:destroy_issue, issue)
......
module API
module V3
module Helpers
def find_project_issue(id)
IssuesFinder.new(current_user, project_id: user_project.id).find(id)
end
end
end
end
......@@ -757,9 +757,9 @@
end
end
describe "GET /projects/:id/issues/:issue_id" do
describe "GET /projects/:id/issues/:issue_iid" do
it 'exposes known attributes' do
get api("/projects/#{project.id}/issues/#{issue.id}", user)
get api("/projects/#{project.id}/issues/#{issue.iid}", user)
expect(response).to have_http_status(200)
expect(json_response['id']).to eq(issue.id)
......@@ -777,8 +777,8 @@
expect(json_response['confidential']).to be_falsy
end
it "returns a project issue by id" do
get api("/projects/#{project.id}/issues/#{issue.id}", user)
it "returns a project issue by internal id" do
get api("/projects/#{project.id}/issues/#{issue.iid}", user)
expect(response).to have_http_status(200)
expect(json_response['title']).to eq(issue.title)
......@@ -790,40 +790,46 @@
expect(response).to have_http_status(404)
end
it "returns 404 if the issue ID is used" do
get api("/projects/#{project.id}/issues/#{issue.id}", user)
expect(response).to have_http_status(404)
end
context 'confidential issues' do
it "returns 404 for non project members" do
get api("/projects/#{project.id}/issues/#{confidential_issue.id}", non_member)
get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", non_member)
expect(response).to have_http_status(404)
end
it "returns 404 for project members with guest role" do
get api("/projects/#{project.id}/issues/#{confidential_issue.id}", guest)
get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", guest)
expect(response).to have_http_status(404)
end
it "returns confidential issue for project members" do
get api("/projects/#{project.id}/issues/#{confidential_issue.id}", user)
get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user)
expect(response).to have_http_status(200)
expect(json_response['title']).to eq(confidential_issue.title)
expect(json_response['iid']).to eq(confidential_issue.iid)
end
it "returns confidential issue for author" do
get api("/projects/#{project.id}/issues/#{confidential_issue.id}", author)
get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", author)
expect(response).to have_http_status(200)
expect(json_response['title']).to eq(confidential_issue.title)
expect(json_response['iid']).to eq(confidential_issue.iid)
end
it "returns confidential issue for assignee" do
get api("/projects/#{project.id}/issues/#{confidential_issue.id}", assignee)
get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", assignee)
expect(response).to have_http_status(200)
expect(json_response['title']).to eq(confidential_issue.title)
expect(json_response['iid']).to eq(confidential_issue.iid)
end
it "returns confidential issue for admin" do
get api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin)
get api("/projects/#{project.id}/issues/#{confidential_issue.iid}", admin)
expect(response).to have_http_status(200)
expect(json_response['title']).to eq(confidential_issue.title)
expect(json_response['iid']).to eq(confidential_issue.iid)
......@@ -1004,23 +1010,29 @@
end
end
describe "PUT /projects/:id/issues/:issue_id to update only title" do
describe "PUT /projects/:id/issues/:issue_iid to update only title" do
it "updates a project issue" do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
title: 'updated title'
expect(response).to have_http_status(200)
expect(json_response['title']).to eq('updated title')
end
it "returns 404 error if issue id not found" do
it "returns 404 error if issue iid not found" do
put api("/projects/#{project.id}/issues/44444", user),
title: 'updated title'
expect(response).to have_http_status(404)
end
it 'allows special label names' do
it "returns 404 error if issue id is used instead of the iid" do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
title: 'updated title'
expect(response).to have_http_status(404)
end
it 'allows special label names' do
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
title: 'updated title',
labels: 'label, label?, label&foo, ?, &'
......@@ -1034,40 +1046,40 @@
context 'confidential issues' do
it "returns 403 for non project members" do
put api("/projects/#{project.id}/issues/#{confidential_issue.id}", non_member),
put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", non_member),
title: 'updated title'
expect(response).to have_http_status(403)
end
it "returns 403 for project members with guest role" do
put api("/projects/#{project.id}/issues/#{confidential_issue.id}", guest),
put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", guest),
title: 'updated title'
expect(response).to have_http_status(403)
end
it "updates a confidential issue for project members" do
put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user),
put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user),
title: 'updated title'
expect(response).to have_http_status(200)
expect(json_response['title']).to eq('updated title')
end
it "updates a confidential issue for author" do
put api("/projects/#{project.id}/issues/#{confidential_issue.id}", author),
put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", author),
title: 'updated title'
expect(response).to have_http_status(200)
expect(json_response['title']).to eq('updated title')
end
it "updates a confidential issue for admin" do
put api("/projects/#{project.id}/issues/#{confidential_issue.id}", admin),
put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", admin),
title: 'updated title'
expect(response).to have_http_status(200)
expect(json_response['title']).to eq('updated title')
end
it 'sets an issue to confidential' do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
confidential: true
expect(response).to have_http_status(200)
......@@ -1075,7 +1087,7 @@
end
it 'makes a confidential issue public' do
put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user),
put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user),
confidential: false
expect(response).to have_http_status(200)
......@@ -1083,7 +1095,7 @@
end
it 'does not update a confidential issue with wrong confidential flag' do
put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user),
put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", user),
confidential: 'foo'
expect(response).to have_http_status(400)
......@@ -1092,7 +1104,7 @@
end
end
describe 'PUT /projects/:id/issues/:issue_id with spam filtering' do
describe 'PUT /projects/:id/issues/:issue_iid with spam filtering' do
let(:params) do
{
title: 'updated title',
......@@ -1105,7 +1117,7 @@
allow_any_instance_of(SpamService).to receive_messages(check_for_spam?: true)
allow_any_instance_of(AkismetService).to receive_messages(is_spam?: true)
put api("/projects/#{project.id}/issues/#{issue.id}", user), params
put api("/projects/#{project.id}/issues/#{issue.iid}", user), params
expect(response).to have_http_status(400)
expect(json_response['message']).to eq({ "error" => "Spam detected" })
......@@ -1119,12 +1131,12 @@
end
end
describe 'PUT /projects/:id/issues/:issue_id to update labels' do
describe 'PUT /projects/:id/issues/:issue_iid to update labels' do
let!(:label) { create(:label, title: 'dummy', project: project) }
let!(:label_link) { create(:label_link, label: label, target: issue) }
it 'does not update labels if not present' do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
title: 'updated title'
expect(response).to have_http_status(200)
expect(json_response['labels']).to eq([label.title])
......@@ -1135,7 +1147,7 @@
label.toggle_subscription(user2, project)
perform_enqueued_jobs do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
title: 'updated title', labels: label.title
end
......@@ -1143,14 +1155,14 @@
end
it 'removes all labels' do
put api("/projects/#{project.id}/issues/#{issue.id}", user), labels: ''
put api("/projects/#{project.id}/issues/#{issue.iid}", user), labels: ''
expect(response).to have_http_status(200)
expect(json_response['labels']).to eq([])
end
it 'updates labels' do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
labels: 'foo,bar'
expect(response).to have_http_status(200)
expect(json_response['labels']).to include 'foo'
......@@ -1158,7 +1170,7 @@
end
it 'allows special label names' do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
labels: 'label:foo, label-bar,label_bar,label/bar,label?bar,label&bar,?,&'
expect(response.status).to eq(200)
expect(json_response['labels']).to include 'label:foo'
......@@ -1172,7 +1184,7 @@
end
it 'returns 400 if title is too long' do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
title: 'g' * 256
expect(response).to have_http_status(400)
expect(json_response['message']['title']).to eq([
......@@ -1181,9 +1193,9 @@
end
end
describe "PUT /projects/:id/issues/:issue_id to update state and label" do
describe "PUT /projects/:id/issues/:issue_iid to update state and label" do
it "updates a project issue" do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
labels: 'label2', state_event: "close"
expect(response).to have_http_status(200)
......@@ -1192,7 +1204,7 @@
end
it 'reopens a project isssue' do
put api("/projects/#{project.id}/issues/#{closed_issue.id}", user), state_event: 'reopen'
put api("/projects/#{project.id}/issues/#{closed_issue.iid}", user), state_event: 'reopen'
expect(response).to have_http_status(200)
expect(json_response['state']).to eq 'reopened'
......@@ -1201,7 +1213,7 @@
context 'when an admin or owner makes the request' do
it 'accepts the update date to be set' do
update_time = 2.weeks.ago
put api("/projects/#{project.id}/issues/#{issue.id}", user),
put api("/projects/#{project.id}/issues/#{issue.iid}", user),
labels: 'label3', state_event: 'close', updated_at: update_time
expect(response).to have_http_status(200)
......@@ -1211,25 +1223,25 @@
end
end
describe 'PUT /projects/:id/issues/:issue_id to update due date' do
describe 'PUT /projects/:id/issues/:issue_iid to update due date' do
it 'creates a new project issue' do
due_date = 2.weeks.from_now.strftime('%Y-%m-%d')
put api("/projects/#{project.id}/issues/#{issue.id}", user), due_date: due_date
put api("/projects/#{project.id}/issues/#{issue.iid}", user), due_date: due_date
expect(response).to have_http_status(200)
expect(json_response['due_date']).to eq(due_date)
end
end
describe "DELETE /projects/:id/issues/:issue_id" do
describe "DELETE /projects/:id/issues/:issue_iid" do
it "rejects a non member from deleting an issue" do
delete api("/projects/#{project.id}/issues/#{issue.id}", non_member)
delete api("/projects/#{project.id}/issues/#{issue.iid}", non_member)
expect(response).to have_http_status(403)
end
it "rejects a developer from deleting an issue" do
delete api("/projects/#{project.id}/issues/#{issue.id}", author)
delete api("/projects/#{project.id}/issues/#{issue.iid}", author)
expect(response).to have_http_status(403)
end
......@@ -1238,7 +1250,7 @@
let(:project) { create(:empty_project, namespace: owner.namespace) }
it "deletes the issue if an admin requests it" do
delete api("/projects/#{project.id}/issues/#{issue.id}", owner)
delete api("/projects/#{project.id}/issues/#{issue.iid}", owner)
expect(response).to have_http_status(204)
end
......@@ -1251,14 +1263,20 @@
expect(response).to have_http_status(404)
end
end
it 'returns 404 when using the issue ID instead of IID' do
delete api("/projects/#{project.id}/issues/#{issue.id}", user)
expect(response).to have_http_status(404)
end
end
describe '/projects/:id/issues/:issue_id/move' do
describe '/projects/:id/issues/:issue_iid/move' do
let!(:target_project) { create(:empty_project, path: 'project2', creator_id: user.id, namespace: user.namespace ) }
let!(:target_project2) { create(:empty_project, creator_id: non_member.id, namespace: non_member.namespace ) }
it 'moves an issue' do
post api("/projects/#{project.id}/issues/#{issue.id}/move", user),
post api("/projects/#{project.id}/issues/#{issue.iid}/move", user),
to_project_id: target_project.id
expect(response).to have_http_status(201)
......@@ -1267,7 +1285,7 @@
context 'when source and target projects are the same' do
it 'returns 400 when trying to move an issue' do
post api("/projects/#{project.id}/issues/#{issue.id}/move", user),
post api("/projects/#{project.id}/issues/#{issue.iid}/move", user),
to_project_id: project.id
expect(response).to have_http_status(400)
......@@ -1277,7 +1295,7 @@
context 'when the user does not have the permission to move issues' do
it 'returns 400 when trying to move an issue' do
post api("/projects/#{project.id}/issues/#{issue.id}/move", user),
post api("/projects/#{project.id}/issues/#{issue.iid}/move", user),
to_project_id: target_project2.id
expect(response).to have_http_status(400)
......@@ -1286,13 +1304,23 @@
end
it 'moves the issue to another namespace if I am admin' do
post api("/projects/#{project.id}/issues/#{issue.id}/move", admin),
post api("/projects/#{project.id}/issues/#{issue.iid}/move", admin),
to_project_id: target_project2.id
expect(response).to have_http_status(201)
expect(json_response['project_id']).to eq(target_project2.id)
end
context 'when using the issue ID instead of iid' do
it 'returns 404 when trying to move an issue' do
post api("/projects/#{project.id}/issues/#{issue.id}/move", user),
to_project_id: target_project.id
expect(response).to have_http_status(404)
expect(json_response['message']).to eq('404 Issue Not Found')
end
end
context 'when issue does not exist' do
it 'returns 404 when trying to move an issue' do
post api("/projects/#{project.id}/issues/123/move", user),
......@@ -1305,7 +1333,7 @@
context 'when source project does not exist' do
it 'returns 404 when trying to move an issue' do
post api("/projects/123/issues/#{issue.id}/move", user),
post api("/projects/123/issues/#{issue.iid}/move", user),
to_project_id: target_project.id
expect(response).to have_http_status(404)
......@@ -1315,7 +1343,7 @@
context 'when target project does not exist' do
it 'returns 404 when trying to move an issue' do
post api("/projects/#{project.id}/issues/#{issue.id}/move", user),
post api("/projects/#{project.id}/issues/#{issue.iid}/move", user),
to_project_id: 123
expect(response).to have_http_status(404)
......@@ -1323,16 +1351,16 @@
end
end
describe 'POST :id/issues/:issue_id/subscribe' do
describe 'POST :id/issues/:issue_iid/subscribe' do
it 'subscribes to an issue' do
post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user2)
post api("/projects/#{project.id}/issues/#{issue.iid}/subscribe", user2)
expect(response).to have_http_status(201)
expect(json_response['subscribed']).to eq(true)
end
it 'returns 304 if already subscribed' do
post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user)
post api("/projects/#{project.id}/issues/#{issue.iid}/subscribe", user)
expect(response).to have_http_status(304)
end
......@@ -1343,8 +1371,14 @@
expect(response).to have_http_status(404)
end
it 'returns 404 if the issue ID is used instead of the iid' do
post api("/projects/#{project.id}/issues/#{issue.id}/subscribe", user)
expect(response).to have_http_status(404)
end
it 'returns 404 if the issue is confidential' do
post api("/projects/#{project.id}/issues/#{confidential_issue.id}/subscribe", non_member)
post api("/projects/#{project.id}/issues/#{confidential_issue.iid}/subscribe", non_member)
expect(response).to have_http_status(404)
end
......@@ -1352,14 +1386,14 @@
describe 'POST :id/issues/:issue_id/unsubscribe' do
it 'unsubscribes from an issue' do
post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user)
post api("/projects/#{project.id}/issues/#{issue.iid}/unsubscribe", user)
expect(response).to have_http_status(201)
expect(json_response['subscribed']).to eq(false)
end
it 'returns 304 if not subscribed' do
post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user2)
post api("/projects/#{project.id}/issues/#{issue.iid}/unsubscribe", user2)
expect(response).to have_http_status(304)
end
......@@ -1370,8 +1404,14 @@
expect(response).to have_http_status(404)
end
it 'returns 404 if using the issue ID instead of iid' do
post api("/projects/#{project.id}/issues/#{issue.id}/unsubscribe", user)
expect(response).to have_http_status(404)
end
it 'returns 404 if the issue is confidential' do
post api("/projects/#{project.id}/issues/#{confidential_issue.id}/unsubscribe", non_member)
post api("/projects/#{project.id}/issues/#{confidential_issue.iid}/unsubscribe", non_member)
expect(response).to have_http_status(404)
end
......
<
......@@ -7,13 +7,13 @@
describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_estimate" do
context 'with an unauthorized user' do
subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", non_member), duration: '1w') }
subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", non_member), duration: '1w') }
it_behaves_like 'an unauthorized API user'
end
it "sets the time estimate for #{issuable_name}" do
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w'
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), duration: '1w'
expect(response).to have_http_status(200)
expect(json_response['human_time_estimate']).to eq('1w')
......@@ -21,12 +21,12 @@
describe 'updating the current estimate' do
before do
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '1w'
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), duration: '1w'
end
context 'when duration has a bad format' do
it 'does not modify the original estimate' do
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: 'foo'
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), duration: 'foo'
expect(response).to have_http_status(400)
expect(issuable.reload.human_time_estimate).to eq('1w')
......@@ -35,7 +35,7 @@
context 'with a valid duration' do
it 'updates the estimate' do
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_estimate", user), duration: '3w1h'
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_estimate", user), duration: '3w1h'
expect(response).to have_http_status(200)
expect(issuable.reload.human_time_estimate).to eq('3w 1h')
......@@ -46,13 +46,13 @@
describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/reset_time_estimate" do
context 'with an unauthorized user' do
subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", non_member)) }
subject { post(api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_time_estimate", non_member)) }
it_behaves_like 'an unauthorized API user'
end
it "resets the time estimate for #{issuable_name}" do
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/reset_time_estimate", user)
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/reset_time_estimate", user)
expect(response).to have_http_status(200)
expect(json_response['time_estimate']).to eq(0)
......@@ -62,7 +62,7 @@
describe "POST /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/add_spent_time" do
context 'with an unauthorized user' do
subject do
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", non_member),
post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", non_member),
duration: '2h'
end