kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

API: Implement project issues iid param with IssuesFinder and add tests

Browse source 
- Use IssuesFinder for the /issues API resouce
- Tests for iid filter in project issues API resource
- Tests for No Milestone filter in issues API resources
  The "No Milestone" case was not previously tested, and the `/issues`
  resource did not support the the `milestone` parameter.
- Return issues where all labels match from the issues and project issues
  API resources, like the group issues resource already does. See
  https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6825#note_17474533

Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
David Eisner 2016-10-26 09:08:58 +01:00 committed by Rémy Coutable
parent 5c253116ae
commit 7ef1c6408e
4 changed files with 110 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
changelogs/unreleased/fix-no-milestone-option-for-projects-endpoint-23194.yml Normal file
View file

@ -0,0 +1,4 @@
---
title: 'API: fix query response for `/projects/:id/issues?milestone="No%20Milestone"`'
merge_request: 8457
author: Panagiotis Atmatzidis, David Eisner

3
doc/api/issues.md
View file

@ -23,12 +23,15 @@ GET /issues?state=closed
GET /issues?labels=foo
GET /issues?labels=foo,bar
GET /issues?labels=foo,bar&state=opened
GET /issues?milestone=1.0.0
GET /issues?milestone=1.0.0&state=opened
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `state` | string | no | Return all issues or just those that are `opened` or `closed`|
| `labels` | string | no | Comma-separated list of label names, issues with any of the labels will be returned |
| `milestone` | string| no | The milestone title |
| `order_by`| string | no | Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` |
| `sort` | string | no | Return requests sorted in `asc` or `desc` order. Default is `desc` |

47
lib/api/issues.rb
View file

@ -5,13 +5,31 @@ module API
before { authenticate! }
helpers do
# TODO: Remove in 9.0 and switch to IssueFinder-based label filtering
def filter_issues_labels(issues, labels)
issues.includes(:labels).where('labels.title' => labels.split(','))
def find_issues(args = {})
args = params.merge(args)
args.delete(:id)
args[:milestone_title] = args.delete(:milestone)
match_all_labels = args.delete(:match_all_labels)
labels = args.delete(:labels)
args[:label_name] = labels if match_all_labels
args[:search] = "#{Issue.reference_prefix}#{args.delete(:iid)}" if args.key?(:iid)
issues = IssuesFinder.new(current_user, args).execute.inc_notes_with_associations
# TODO: Remove in 9.0 pass `label_name: args.delete(:labels)` to IssuesFinder
if !match_all_labels && labels.present?
issues = issues.includes(:labels).where('labels.title' => labels.split(','))
end
issues.reorder(args[:order_by] => args[:sort])
end
params :issues_params do
optional :labels, type: String, desc: 'Comma-separated list of label names'
optional :milestone, type: String, desc: 'Milestone title'
optional :order_by, type: String, values: %w[created_at updated_at], default: 'created_at',
desc: 'Return issues ordered by `created_at` or `updated_at` fields.'
optional :sort, type: String, values: %w[asc desc], default: 'desc',
@ -40,9 +58,7 @@ module API
use :issues_params
end
get do
issues = IssuesFinder.new(current_user, scope: 'all', author_id: current_user.id, state: params[:state]).execute.inc_notes_with_associations
issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil?
issues = issues.reorder(params[:order_by] => params[:sort])
issues = find_issues(scope: 'authored')
present paginate(issues), with: Entities::Issue, current_user: current_user
end
@ -61,15 +77,10 @@ module API
use :issues_params
end
get ":id/issues" do
group = find_group!(params.delete(:id))
group = find_group!(params[:id])
params[:group_id] = group.id
params[:milestone_title] = params.delete(:milestone)
params[:label_name] = params.delete(:labels)
issues = find_issues(group_id: group.id, state: params[:state] || 'opened', match_all_labels: true)
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
end
end
@ -84,19 +95,13 @@ module API
params do
optional :state, type: String, values: %w[opened closed all], default: 'all',
desc: 'Return opened, closed, or all issues'
optional :iid, type: Integer, desc: 'The IID of the issue'
optional :iid, type: Integer, desc: 'Return the issue having the given `iid`'
use :issues_params
end
get ":id/issues" do
project = find_project(params[:id])
params[:state] ||= 'opened'
params[:project_id] = project.id
params[:milestone_title] = params.delete(:milestone)
params[:label_name] = params.delete(:labels)
issues = IssuesFinder.new(current_user, params).execute
issues = issues.reorder(params[:order_by] => params[:sort])
issues = find_issues(project_id: project.id)
present paginate(issues), with: Entities::Issue, current_user: current_user, project: user_project
end

80
spec/requests/api/issues_spec.rb
View file

@ -107,6 +107,7 @@ describe API::Issues, api: true do
it 'returns an array of labeled issues when at least one label matches' do
get api("/issues?labels=#{label.title},foo,bar", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
@ -136,6 +137,51 @@ describe API::Issues, api: true do
expect(json_response.length).to eq(0)
end
it 'returns an empty array if no issue matches milestone' do
get api("/issues?milestone=#{empty_milestone.title}", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(0)
end
it 'returns an empty array if milestone does not exist' do
get api("/issues?milestone=foo", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(0)
end
it 'returns an array of issues in given milestone' do
get api("/issues?milestone=#{milestone.title}", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(2)
expect(json_response.first['id']).to eq(issue.id)
expect(json_response.second['id']).to eq(closed_issue.id)
end
it 'returns an array of issues matching state in milestone' do
get api("/issues?milestone=#{milestone.title}"\
'&state=closed', user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
expect(json_response.first['id']).to eq(closed_issue.id)
end
it 'returns an array of issues with no milestone' do
get api("/issues?milestone=#{Milestone::None.title}", author)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
expect(json_response.first['id']).to eq(confidential_issue.id)
end
it 'sorts by created_at descending by default' do
get api('/issues', user)
response_dates = json_response.map { |issue| issue['created_at'] }
@ -318,6 +364,15 @@ describe API::Issues, api: true do
expect(json_response.first['id']).to eq(group_closed_issue.id)
end
it 'returns an array of issues with no milestone' do
get api("#{base_url}?milestone=#{Milestone::None.title}", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
expect(json_response.first['id']).to eq(group_confidential_issue.id)
end
it 'sorts by created_at descending by default' do
get api(base_url, user)
response_dates = json_response.map { |issue| issue['created_at'] }
@ -357,7 +412,6 @@ describe API::Issues, api: true do
describe "GET /projects/:id/issues" do
let(:base_url) { "/projects/#{project.id}" }
let(:title) { milestone.title }
it "returns 404 on private projects for other users" do
private_project = create(:empty_project, :private)
@ -433,8 +487,9 @@ describe API::Issues, api: true do
expect(json_response.first['labels']).to eq([label.title])
end
it 'returns an array of labeled project issues when at least one label matches' do
it 'returns an array of labeled project issues where all labels match' do
get api("#{base_url}/issues?labels=#{label.title},foo,bar", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
@ -463,7 +518,8 @@ describe API::Issues, api: true do
end
it 'returns an array of issues in given milestone' do
get api("#{base_url}/issues?milestone=#{title}", user)
get api("#{base_url}/issues?milestone=#{milestone.title}", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(2)
@ -480,6 +536,15 @@ describe API::Issues, api: true do
expect(json_response.first['id']).to eq(closed_issue.id)
end
it 'returns an array of issues with no milestone' do
get api("#{base_url}/issues?milestone=#{Milestone::None.title}", user)
expect(response).to have_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
expect(json_response.first['id']).to eq(confidential_issue.id)
end
it 'sorts by created_at descending by default' do
get api("#{base_url}/issues", user)
response_dates = json_response.map { |issue| issue['created_at'] }
@ -547,12 +612,21 @@ describe API::Issues, api: true do
it 'returns a project issue by iid' do
get api("/projects/#{project.id}/issues?iid=#{issue.iid}", user)
expect(response.status).to eq 200
expect(json_response.length).to eq 1
expect(json_response.first['title']).to eq issue.title
expect(json_response.first['id']).to eq issue.id
expect(json_response.first['iid']).to eq issue.iid
end
it 'returns an empty array for an unknown project issue iid' do
get api("/projects/#{project.id}/issues?iid=#{issue.iid + 10}", user)
expect(response.status).to eq 200
expect(json_response.length).to eq 0
end
it "returns 404 if issue id not found" do
get api("/projects/#{project.id}/issues/54321", user)
expect(response).to have_http_status(404)