Commit bdeafd14 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'grapify-issues-api' into 'master'

Grapify the issues API

Related to #22928

See merge request !7968
parents bf76ef8e 3e7818e9
...@@ -217,22 +217,6 @@ module API ...@@ -217,22 +217,6 @@ module API
end end
end end
def issuable_order_by
if params["order_by"] == 'updated_at'
'updated_at'
else
'created_at'
end
end
def issuable_sort
if params["sort"] == 'asc'
:asc
else
:desc
end
end
def filter_by_iid(items, iid) def filter_by_iid(items, iid)
items.where(iid: iid) items.where(iid: iid)
end end
......
module API module API
# Issues API
class Issues < Grape::API class Issues < Grape::API
include PaginationParams
before { authenticate! } before { authenticate! }
helpers do helpers do
...@@ -20,77 +21,68 @@ module API ...@@ -20,77 +21,68 @@ module API
issues.includes(:milestone).where('milestones.title' => milestone) issues.includes(:milestone).where('milestones.title' => milestone)
end end
def issue_params params :issues_params do
new_params = declared(params, include_parent_namespace: false, include_missing: false).to_h optional :labels, type: String, desc: 'Comma-separated list of label names'
new_params = new_params.with_indifferent_access optional :order_by, type: String, values: %w[created_at updated_at], default: 'created_at',
new_params.delete(:id) desc: 'Return issues ordered by `created_at` or `updated_at` fields.'
new_params.delete(:issue_id) optional :sort, type: String, values: %w[asc desc], default: 'desc',
desc: 'Return issues sorted in `asc` or `desc` order.'
use :pagination
end
new_params params :issue_params do
optional :description, type: String, desc: 'The description of an issue'
optional :assignee_id, type: Integer, desc: 'The ID of a user to assign issue'
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue'
optional :labels, type: String, desc: 'Comma-separated list of label names'
optional :due_date, type: String, desc: 'Date time string in the format YEAR-MONTH-DAY'
optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential'
optional :state_event, type: String, values: %w[open close],
desc: 'State of the issue'
end end
end end
resource :issues do resource :issues do
# Get currently authenticated user's issues desc "Get currently authenticated user's issues" do
# success Entities::Issue
# Parameters: end
# state (optional) - Return "opened" or "closed" issues params do
# labels (optional) - Comma-separated list of label names optional :state, type: String, values: %w[opened closed all], default: 'all',
# order_by (optional) - Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` desc: 'Return opened, closed, or all issues'
# sort (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` use :issues_params
# end
# Example Requests:
# GET /issues
# GET /issues?state=opened
# GET /issues?state=closed
# GET /issues?labels=foo
# GET /issues?labels=foo,bar
# GET /issues?labels=foo,bar&state=opened
get do get do
issues = current_user.issues.inc_notes_with_associations issues = current_user.issues.inc_notes_with_associations
issues = filter_issues_state(issues, params[:state]) unless params[:state].nil? issues = filter_issues_state(issues, params[:state])
issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil? issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil?
issues = issues.reorder(issuable_order_by => issuable_sort) issues = issues.reorder(params[:order_by] => params[:sort])
present paginate(issues), with: Entities::Issue, current_user: current_user present paginate(issues), with: Entities::Issue, current_user: current_user
end end
end end
params do
requires :id, type: String, desc: 'The ID of a group'
end
resource :groups do resource :groups do
# Get a list of group issues desc 'Get a list of group issues' do
# success Entities::Issue
# Parameters: end
# id (required) - The ID of a group params do
# state (optional) - Return "opened" or "closed" issues optional :state, type: String, values: %w[opened closed all], default: 'opened',
# labels (optional) - Comma-separated list of label names desc: 'Return opened, closed, or all issues'
# milestone (optional) - Milestone title use :issues_params
# order_by (optional) - Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` end
# sort (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc`
#
# Example Requests:
# GET /groups/:id/issues
# GET /groups/:id/issues?state=opened
# GET /groups/:id/issues?state=closed
# GET /groups/:id/issues?labels=foo
# GET /groups/:id/issues?labels=foo,bar
# GET /groups/:id/issues?labels=foo,bar&state=opened
# GET /groups/:id/issues?milestone=1.0.0
# GET /groups/:id/issues?milestone=1.0.0&state=closed
get ":id/issues" do get ":id/issues" do
group = find_group!(params[:id]) group = find_group!(params.delete(:id))
params[:state] ||= 'opened'
params[:group_id] = group.id params[:group_id] = group.id
params[:milestone_title] = params.delete(:milestone) params[:milestone_title] = params.delete(:milestone)
params[:label_name] = params.delete(:labels) params[:label_name] = params.delete(:labels)
if params[:order_by] || params[:sort]
# The Sortable concern takes 'created_desc', not 'created_at_desc' (for example)
params[:sort] = "#{issuable_order_by.sub('_at', '')}_#{issuable_sort}"
end
issues = IssuesFinder.new(current_user, params).execute issues = IssuesFinder.new(current_user, params).execute
issues = issues.reorder(params[:order_by] => params[:sort])
present paginate(issues), with: Entities::Issue, current_user: current_user present paginate(issues), with: Entities::Issue, current_user: current_user
end end
end end
...@@ -98,32 +90,19 @@ module API ...@@ -98,32 +90,19 @@ module API
params do params do
requires :id, type: String, desc: 'The ID of a project' requires :id, type: String, desc: 'The ID of a project'
end end
resource :projects do resource :projects do
# Get a list of project issues desc 'Get a list of project issues' do
# success Entities::Issue
# Parameters: end
# id (required) - The ID of a project params do
# iid (optional) - Return the project issue having the given `iid` optional :state, type: String, values: %w[opened closed all], default: 'all',
# state (optional) - Return "opened" or "closed" issues desc: 'Return opened, closed, or all issues'
# labels (optional) - Comma-separated list of label names optional :iid, type: Integer, desc: 'The IID of the issue'
# milestone (optional) - Milestone title use :issues_params
# order_by (optional) - Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` end
# sort (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc`
#
# Example Requests:
# GET /projects/:id/issues
# GET /projects/:id/issues?state=opened
# GET /projects/:id/issues?state=closed
# GET /projects/:id/issues?labels=foo
# GET /projects/:id/issues?labels=foo,bar
# GET /projects/:id/issues?labels=foo,bar&state=opened
# GET /projects/:id/issues?milestone=1.0.0
# GET /projects/:id/issues?milestone=1.0.0&state=closed
# GET /issues?iid=42
get ":id/issues" do get ":id/issues" do
issues = IssuesFinder.new(current_user, project_id: user_project.id).execute.inc_notes_with_associations issues = IssuesFinder.new(current_user, project_id: user_project.id).execute.inc_notes_with_associations
issues = filter_issues_state(issues, params[:state]) unless params[:state].nil? issues = filter_issues_state(issues, params[:state])
issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil? issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil?
issues = filter_by_iid(issues, params[:iid]) unless params[:iid].nil? issues = filter_by_iid(issues, params[:iid]) unless params[:iid].nil?
...@@ -131,59 +110,49 @@ module API ...@@ -131,59 +110,49 @@ module API
issues = filter_issues_milestone(issues, params[:milestone]) issues = filter_issues_milestone(issues, params[:milestone])
end end
issues = issues.reorder(issuable_order_by => issuable_sort) issues = issues.reorder(params[:order_by] => params[:sort])
present paginate(issues), with: Entities::Issue, current_user: current_user, project: user_project present paginate(issues), with: Entities::Issue, current_user: current_user, project: user_project
end end
# Get a single project issue desc 'Get a single project issue' do
# success Entities::Issue
# Parameters: end
# id (required) - The ID of a project params do
# issue_id (required) - The ID of a project issue requires :issue_id, type: Integer, desc: 'The ID of a project issue'
# Example Request: end
# GET /projects/:id/issues/:issue_id
get ":id/issues/:issue_id" do get ":id/issues/:issue_id" do
@issue = find_project_issue(params[:issue_id]) issue = find_project_issue(params[:issue_id])
present @issue, with: Entities::Issue, current_user: current_user, project: user_project present issue, with: Entities::Issue, current_user: current_user, project: user_project
end end
# Create a new project issue desc 'Create a new project issue' do
# success Entities::Issue
# Parameters: end
# id (required) - The ID of a project params do
# title (required) - The title of an issue requires :title, type: String, desc: 'The title of an issue'
# description (optional) - The description of an issue optional :created_at, type: DateTime,
# assignee_id (optional) - The ID of a user to assign issue desc: 'Date time when the issue was created. Available only for admins and project owners.'
# milestone_id (optional) - The ID of a milestone to assign issue optional :merge_request_for_resolving_discussions, type: Integer,
# labels (optional) - The labels of an issue desc: 'The IID of a merge request for which to resolve discussions'
# created_at (optional) - Date time string, ISO 8601 formatted use :issue_params
# due_date (optional) - Date time string in the format YEAR-MONTH-DAY end
# confidential (optional) - Boolean parameter if the issue should be confidential
# merge_request_for_resolving_discussions (optional) - The IID of a merge request for which to resolve discussions
# Example Request:
# POST /projects/:id/issues
post ':id/issues' do post ':id/issues' do
required_attributes! [:title] # Setting created_at time only allowed for admins and project owners
unless current_user.admin? || user_project.owner == current_user
keys = [:title, :description, :assignee_id, :milestone_id, :due_date, :confidential, :labels, :merge_request_for_resolving_discussions] params.delete(:created_at)
keys << :created_at if current_user.admin? || user_project.owner == current_user end
attrs = attributes_for_keys(keys)
attrs[:labels] = params[:labels] if params[:labels] issue_params = declared_params(include_missing: false)
if merge_request_iid = params[:merge_request_for_resolving_discussions] if merge_request_iid = params[:merge_request_for_resolving_discussions]
attrs[:merge_request_for_resolving_discussions] = MergeRequestsFinder.new(current_user, project_id: user_project.id). issue_params[:merge_request_for_resolving_discussions] = MergeRequestsFinder.new(current_user, project_id: user_project.id).
execute. execute.
find_by(iid: merge_request_iid) find_by(iid: merge_request_iid)
end end
# Convert and filter out invalid confidential flags issue = ::Issues::CreateService.new(user_project,
attrs['confidential'] = to_boolean(attrs['confidential']) current_user,
attrs.delete('confidential') if attrs['confidential'].nil? issue_params.merge(request: request, api: true)).execute
issue = ::Issues::CreateService.new(user_project, current_user, attrs.merge(request: request, api: true)).execute
if issue.spam? if issue.spam?
render_api_error!({ error: 'Spam detected' }, 400) render_api_error!({ error: 'Spam detected' }, 400)
end end
...@@ -199,31 +168,26 @@ module API ...@@ -199,31 +168,26 @@ module API
success Entities::Issue success Entities::Issue
end end
params do params do
requires :id, type: String, desc: 'The ID of a project' requires :issue_id, type: Integer, desc: 'The ID of a project issue'
requires :issue_id, type: Integer, desc: "The ID of a project issue" optional :title, type: String, desc: 'The title of an issue'
optional :title, type: String, desc: 'The new title of the issue' optional :updated_at, type: DateTime,
optional :description, type: String, desc: 'The description of an issue' desc: 'Date time when the issue was updated. Available only for admins and project owners.'
optional :assignee_id, type: Integer, desc: 'The ID of a user to assign issue' use :issue_params
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue' at_least_one_of :title, :description, :assignee_id, :milestone_id,
optional :labels, type: String, desc: 'The labels of an issue' :labels, :created_at, :due_date, :confidential, :state_event
optional :state_event, type: String, values: ['close', 'reopen'], desc: 'The state event of an issue'
# TODO 9.0, use the Grape DateTime type here
optional :updated_at, type: String, desc: 'Date time string, ISO 8601 formatted'
optional :due_date, type: String, desc: 'Date time string in the format YEAR-MONTH-DAY'
# TODO 9.0, use the Grape boolean type here
optional :confidential, type: String, desc: 'Boolean parameter if the issue should be confidential'
end end
put ':id/issues/:issue_id' do put ':id/issues/:issue_id' do
issue = user_project.issues.find(params[:issue_id]) issue = user_project.issues.find(params.delete(:issue_id))
authorize! :update_issue, issue authorize! :update_issue, issue
# Convert and filter out invalid confidential flags # Setting created_at time only allowed for admins and project owners
params[:confidential] = to_boolean(params[:confidential]) unless current_user.admin? || user_project.owner == current_user
params.delete(:confidential) if params[:confidential].nil? params.delete(:updated_at)
end
params.delete(:updated_at) unless current_user.admin? || user_project.owner == current_user
issue = ::Issues::UpdateService.new(user_project, current_user, issue_params).execute(issue) issue = ::Issues::UpdateService.new(user_project,
current_user,
declared_params(include_missing: false)).execute(issue)
if issue.valid? if issue.valid?
present issue, with: Entities::Issue, current_user: current_user, project: user_project present issue, with: Entities::Issue, current_user: current_user, project: user_project
...@@ -232,19 +196,19 @@ module API ...@@ -232,19 +196,19 @@ module API
end end
end end
# Move an existing issue desc 'Move an existing issue' do
# success Entities::Issue
# Parameters: end
# id (required) - The ID of a project params do
# issue_id (required) - The ID of a project issue requires :issue_id, type: Integer, desc: 'The ID of a project issue'
# to_project_id (required) - The ID of the new project requires :to_project_id, type: Integer, desc: 'The ID of the new project'
# Example Request: end
# POST /projects/:id/issues/:issue_id/move
post ':id/issues/:issue_id/move' do post ':id/issues/:issue_id/move' do
required_attributes! [:to_project_id] issue = user_project.issues.find_by(id: params[:issue_id])
not_found!('Issue') unless issue
issue = user_project.issues.find(params[:issue_id]) new_project = Project.find_by(id: params[:to_project_id])
new_project = Project.find(params[:to_project_id]) not_found!('Project') unless new_project
begin begin
issue = ::Issues::MoveService.new(user_project, current_user).execute(issue, new_project) issue = ::Issues::MoveService.new(user_project, current_user).execute(issue, new_project)
...@@ -254,16 +218,13 @@ module API ...@@ -254,16 +218,13 @@ module API
end end
end end
# desc 'Delete a project issue'
# Delete a project issue params do
# requires :issue_id, type: Integer, desc: 'The ID of a project issue'
# Parameters: end
# id (required) - The ID of a project
# issue_id (required) - The ID of a project issue
# Example Request:
# DELETE /projects/:id/issues/:issue_id
delete ":id/issues/:issue_id" do delete ":id/issues/:issue_id" do
issue = user_project.issues.find_by(id: params[:issue_id]) issue = user_project.issues.find_by(id: params[:issue_id])
not_found!('Issue') unless issue
authorize!(:destroy_issue, issue) authorize!(:destroy_issue, issue)
issue.destroy issue.destroy
......
...@@ -72,13 +72,6 @@ describe API::Issues, api: true do ...@@ -72,13 +72,6 @@ describe API::Issues, api: true do
expect(json_response.last).to have_key('web_url') expect(json_response.last).to have_key('web_url')
end end
it "adds pagination headers and keep query params" do
get api("/issues?state=closed&per_page=3", user)
expect(response.headers['Link']).to eq(
'<http://www.example.com/api/v3/issues?page=1&per_page=3&private_token=%s&state=closed>; rel="first", <http://www.example.com/api/v3/issues?page=1&per_page=3&private_token=%s&state=closed>; rel="last"' % [user.private_token, user.private_token]
)
end
it 'returns an array of closed issues' do it 'returns an array of closed issues' do
get api('/issues?state=closed', user) get api('/issues?state=closed', user)
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
...@@ -649,9 +642,8 @@ describe API::Issues, api: true do ...@@ -649,9 +642,8 @@ describe API::Issues, api: true do
post api("/projects/#{project.id}/issues", user), post api("/projects/#{project.id}/issues", user),
title: 'new issue', confidential: 'foo' title: 'new issue', confidential: 'foo'
expect(response).to have_http_status(201) expect(response).to have_http_status(400)
expect(json_response['title']).to eq('new issue') expect(json_response['error']).to eq('confidential is invalid')
expect(json_response['confidential']).to be_falsy
end end
it "sends notifications for subscribers of newly added labels" do it "sends notifications for subscribers of newly added labels" do
...@@ -862,8 +854,8 @@ describe API::Issues, api: true do ...@@ -862,8 +854,8 @@ describe API::Issues, api: true do
put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user), put api("/projects/#{project.id}/issues/#{confidential_issue.id}", user),
confidential: 'foo' confidential: 'foo'
expect(response).to have_http_status(200) expect(response).to have_http_status(400)
expect(json_response['confidential']).to be_truthy expect(json_response['error']).to eq('confidential is invalid')
end end
end end
end end
...@@ -985,6 +977,14 @@ describe API::Issues, api: true do ...@@ -985,6 +977,14 @@ describe API::Issues, api: true do
expect(json_response['state']).to eq 'opened' expect(json_response['state']).to eq 'opened'
end end
end end
context 'when issue does not exist' do
it 'returns 404 when trying to move an issue' do
delete api("/projects/#{project.id}/issues/123", user)
expect(response).to have_http_status(404)
end
end
end end
describe '/projects/:id/issues/:issue_id/move' do describe '/projects/:id/issues/:issue_id/move' do
...@@ -1033,6 +1033,7 @@ describe API::Issues, api: true do ...@@ -1033,6 +1033,7 @@ describe API::Issues, api: true do
to_project_id: target_project.id to_project_id: target_project.id
expect(response).to have_http_status(404) expect(response).to have_http_status(404)
expect(json_response['message']).to eq('404 Issue Not Found')
end end
end end
...@@ -1042,6 +1043,7 @@ describe API::Issues, api: true do ...@@ -1042,6 +1043,7 @@ describe API::Issues, api: true do
to_project_id: target_project.id to_project_id: target_project.id
expect(response).to have_http_status(404) expect(response).to have_http_status(404)
expect(json_response['message']).to eq('404 Project Not Found')
end 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