Commit be3578d2 authored by Alexandru Croitor's avatar Alexandru Croitor

Add array support for labels

* Support label parameter as comma separated and array of strings
for merge requests and issues api endpoints
parent ee8cb2d1
...@@ -89,7 +89,7 @@ class IssuableBaseService < BaseService ...@@ -89,7 +89,7 @@ class IssuableBaseService < BaseService
return unless labels return unless labels
params[:label_ids] = labels.split(",").map do |label_name| params[:label_ids] = labels.map do |label_name|
label = Labels::FindOrCreateService.new( label = Labels::FindOrCreateService.new(
current_user, current_user,
parent, parent,
......
...@@ -34,7 +34,7 @@ module API ...@@ -34,7 +34,7 @@ module API
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
params :issues_params do params :issues_params do
optional :labels, type: String, desc: 'Comma-separated list of label names' optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
optional :milestone, type: String, desc: 'Milestone title' optional :milestone, type: String, desc: 'Milestone title'
optional :order_by, type: String, values: %w[created_at updated_at], default: 'created_at', 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.' desc: 'Return issues ordered by `created_at` or `updated_at` fields.'
...@@ -65,7 +65,7 @@ module API ...@@ -65,7 +65,7 @@ module API
optional :assignee_ids, type: Array[Integer], desc: 'The array of user IDs to assign issue' optional :assignee_ids, type: Array[Integer], desc: 'The array of user IDs to assign issue'
optional :assignee_id, type: Integer, desc: '[Deprecated] The ID of a user to assign issue' optional :assignee_id, type: Integer, desc: '[Deprecated] The ID of a user to assign issue'
optional :milestone_id, type: Integer, desc: 'The ID of a milestone 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 :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY' optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY'
optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential' optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential'
optional :discussion_locked, type: Boolean, desc: " Boolean parameter indicating if the issue's discussion is locked" optional :discussion_locked, type: Boolean, desc: " Boolean parameter indicating if the issue's discussion is locked"
......
...@@ -95,7 +95,7 @@ module API ...@@ -95,7 +95,7 @@ module API
optional :sort, type: String, values: %w[asc desc], default: 'desc', optional :sort, type: String, values: %w[asc desc], default: 'desc',
desc: 'Return merge requests sorted in `asc` or `desc` order.' desc: 'Return merge requests sorted in `asc` or `desc` order.'
optional :milestone, type: String, desc: 'Return merge requests for a specific milestone' optional :milestone, type: String, desc: 'Return merge requests for a specific milestone'
optional :labels, type: String, desc: 'Comma-separated list of label names' optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
optional :created_after, type: DateTime, desc: 'Return merge requests created after the specified time' optional :created_after, type: DateTime, desc: 'Return merge requests created after the specified time'
optional :created_before, type: DateTime, desc: 'Return merge requests created before the specified time' optional :created_before, type: DateTime, desc: 'Return merge requests created before the specified time'
optional :updated_after, type: DateTime, desc: 'Return merge requests updated after the specified time' optional :updated_after, type: DateTime, desc: 'Return merge requests updated after the specified time'
...@@ -179,7 +179,7 @@ module API ...@@ -179,7 +179,7 @@ module API
optional :description, type: String, desc: 'The description of the merge request' optional :description, type: String, desc: 'The description of the merge request'
optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request' optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request'
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request' optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request'
optional :labels, type: String, coerce_with: ->(val) { val.is_a?(Array) ? val.join(', ') : val }, desc: 'Comma-separated list of label names' optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging' optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
optional :allow_collaboration, type: Boolean, desc: 'Allow commits from members who can merge to the target branch' optional :allow_collaboration, type: Boolean, desc: 'Allow commits from members who can merge to the target branch'
optional :allow_maintainer_to_push, type: Boolean, as: :allow_collaboration, desc: '[deprecated] See allow_collaboration' optional :allow_maintainer_to_push, type: Boolean, as: :allow_collaboration, desc: '[deprecated] See allow_collaboration'
......
# frozen_string_literal: true
module API
module Validations
module Types
class LabelsList
def self.coerce
lambda do |value|
case value
when String
value.split(',').map(&:strip)
when Array
value.map { |v| v.to_s.split(',').map(&:strip) }.flatten
when LabelsList
value
else
[]
end
end
end
end
end
end
end
This diff is collapsed.
...@@ -617,19 +617,24 @@ describe API::MergeRequests do ...@@ -617,19 +617,24 @@ describe API::MergeRequests do
end end
end end
describe "POST /projects/:id/merge_requests" do describe 'POST /projects/:id/merge_requests' do
context 'between branches projects' do context 'between branches projects' do
it "returns merge_request" do context 'different labels' do
post api("/projects/#{project.id}/merge_requests", user), let(:params) do
params: { {
title: 'Test merge_request', title: 'Test merge_request',
source_branch: 'feature_conflict', source_branch: 'feature_conflict',
target_branch: 'master', target_branch: 'master',
author: user, author_id: user.id,
labels: 'label, label2',
milestone_id: milestone.id, milestone_id: milestone.id,
squash: true squash: true
} }
end
shared_examples_for 'creates merge request with labels' do
it 'returns merge_request' do
params[:labels] = labels
post api("/projects/#{project.id}/merge_requests", user), params: params
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['title']).to eq('Test merge_request') expect(json_response['title']).to eq('Test merge_request')
...@@ -638,6 +643,90 @@ describe API::MergeRequests do ...@@ -638,6 +643,90 @@ describe API::MergeRequests do
expect(json_response['squash']).to be_truthy expect(json_response['squash']).to be_truthy
expect(json_response['force_remove_source_branch']).to be_falsy expect(json_response['force_remove_source_branch']).to be_falsy
end end
end
it_behaves_like 'creates merge request with labels' do
let(:labels) { 'label, label2' }
end
it_behaves_like 'creates merge request with labels' do
let(:labels) { %w(label label2) }
end
it_behaves_like 'creates merge request with labels' do
let(:labels) { %w(label label2) }
end
it 'creates merge request with special label names' do
params[:labels] = 'label, label?, label&foo, ?, &'
post api("/projects/#{project.id}/merge_requests", user), params: params
expect(response).to have_gitlab_http_status(201)
expect(json_response['labels']).to include 'label'
expect(json_response['labels']).to include 'label?'
expect(json_response['labels']).to include 'label&foo'
expect(json_response['labels']).to include '?'
expect(json_response['labels']).to include '&'
end
it 'creates merge request with special label names as array' do
params[:labels] = ['label', 'label?', 'label&foo, ?, &', '1, 2', 3, 4]
post api("/projects/#{project.id}/merge_requests", user), params: params
expect(response).to have_gitlab_http_status(201)
expect(json_response['labels']).to include 'label'
expect(json_response['labels']).to include 'label?'
expect(json_response['labels']).to include 'label&foo'
expect(json_response['labels']).to include '?'
expect(json_response['labels']).to include '&'
expect(json_response['labels']).to include '1'
expect(json_response['labels']).to include '2'
expect(json_response['labels']).to include '3'
expect(json_response['labels']).to include '4'
end
it 'empty label param does not add any labels' do
params[:labels] = ''
post api("/projects/#{project.id}/merge_requests", user), params: params
expect(response).to have_gitlab_http_status(201)
expect(json_response['labels']).to eq([])
end
it 'empty label param as array does not add any labels, but only explicitly as json' do
params[:labels] = []
post api("/projects/#{project.id}/merge_requests", user),
params: params.to_json,
headers: { 'Content-Type': 'application/json' }
expect(response).to have_gitlab_http_status(201)
expect(json_response['labels']).to eq([])
end
xit 'empty label param as array, does not add any labels' do
params[:labels] = []
post api("/projects/#{project.id}/merge_requests", user), params: params
expect(response).to have_gitlab_http_status(201)
expect(json_response['labels']).to eq([])
end
it 'array with one empty string element does not add labels' do
params[:labels] = ['']
post api("/projects/#{project.id}/merge_requests", user), params: params
expect(response).to have_gitlab_http_status(201)
expect(json_response['labels']).to eq([])
end
it 'array with multiple empty string elements, does not add labels' do
params[:labels] = ['', '', '']
post api("/projects/#{project.id}/merge_requests", user), params: params
expect(response).to have_gitlab_http_status(201)
expect(json_response['labels']).to eq([])
end
end
it "returns 422 when source_branch equals target_branch" do it "returns 422 when source_branch equals target_branch" do
post api("/projects/#{project.id}/merge_requests", user), post api("/projects/#{project.id}/merge_requests", user),
...@@ -663,23 +752,6 @@ describe API::MergeRequests do ...@@ -663,23 +752,6 @@ describe API::MergeRequests do
expect(response).to have_gitlab_http_status(400) expect(response).to have_gitlab_http_status(400)
end end
it 'allows special label names' do
post api("/projects/#{project.id}/merge_requests", user),
params: {
title: 'Test merge_request',
source_branch: 'markdown',
target_branch: 'master',
author: user,
labels: 'label, label?, label&foo, ?, &'
}
expect(response).to have_gitlab_http_status(201)
expect(json_response['labels']).to include 'label'
expect(json_response['labels']).to include 'label?'
expect(json_response['labels']).to include 'label&foo'
expect(json_response['labels']).to include '?'
expect(json_response['labels']).to include '&'
end
context 'with existing MR' do context 'with existing MR' do
before do before do
post api("/projects/#{project.id}/merge_requests", user), post api("/projects/#{project.id}/merge_requests", user),
...@@ -1122,6 +1194,7 @@ describe API::MergeRequests do ...@@ -1122,6 +1194,7 @@ describe API::MergeRequests do
expect(json_response['force_remove_source_branch']).to be_truthy expect(json_response['force_remove_source_branch']).to be_truthy
end end
context 'when updating labels' do
it 'allows special label names' do it 'allows special label names' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user),
params: { params: {
...@@ -1139,8 +1212,10 @@ describe API::MergeRequests do ...@@ -1139,8 +1212,10 @@ describe API::MergeRequests do
it 'also accepts labels as an array' do it 'also accepts labels as an array' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user),
params: {
title: 'new issue', title: 'new issue',
labels: ['label', 'label?', 'label&foo', '?', '&'] labels: ['label', 'label?', 'label&foo, ?, &', '1, 2', 3, 4]
}
expect(response.status).to eq(200) expect(response.status).to eq(200)
expect(json_response['labels']).to include 'label' expect(json_response['labels']).to include 'label'
...@@ -1148,6 +1223,68 @@ describe API::MergeRequests do ...@@ -1148,6 +1223,68 @@ describe API::MergeRequests do
expect(json_response['labels']).to include 'label&foo' expect(json_response['labels']).to include 'label&foo'
expect(json_response['labels']).to include '?' expect(json_response['labels']).to include '?'
expect(json_response['labels']).to include '&' expect(json_response['labels']).to include '&'
expect(json_response['labels']).to include '1'
expect(json_response['labels']).to include '2'
expect(json_response['labels']).to include '3'
expect(json_response['labels']).to include '4'
end
it 'empty label param removes labels' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user),
params: {
title: 'new issue',
labels: ''
}
expect(response.status).to eq(200)
expect(json_response['labels']).to eq []
end
it 'label param as empty array, but only explicitly as json, removes labels' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user),
params: {
title: 'new issue',
labels: []
}.to_json,
headers: { 'Content-Type' => 'application/json' }
expect(response.status).to eq(200)
expect(json_response['labels']).to eq []
end
xit 'empty label as array, removes labels' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user),
params: {
title: 'new issue',
labels: []
}
expect(response.status).to eq(200)
# fails, as grape ommits for some reason empty array as optional param value, so nothing it passed along
expect(json_response['labels']).to eq []
end
it 'array with one empty string element removes labels' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user),
params: {
title: 'new issue',
labels: ['']
}
expect(response.status).to eq(200)
expect(json_response['labels']).to eq []
end
it 'array with multiple empty string elements, removes labels' do
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user),
params: {
title: 'new issue',
labels: ['', '', '']
}
expect(response.status).to eq(200)
expect(json_response['labels']).to eq []
end
end end
it 'does not update state when title is empty' do it 'does not update state when title is empty' do
......
...@@ -186,6 +186,37 @@ shared_examples 'merge requests list' do ...@@ -186,6 +186,37 @@ shared_examples 'merge requests list' do
expect(json_response.length).to eq(0) expect(json_response.length).to eq(0)
end end
it 'returns an array of labeled merge requests where all labels match' do
path = endpoint_path + "?labels[]=#{label.title}&labels[]=#{label2.title}"
get api(path, user)
expect(response).to have_gitlab_http_status(200)
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
expect(json_response.first['labels']).to eq([label2.title, label.title])
end
it 'returns an array of merge requests with any label when filtering by any label' do
get api(endpoint_path, user), params: { labels: [" #{label.title} ", " #{label2.title} "] }
expect_paginated_array_response
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
expect(json_response.first['labels']).to eq([label2.title, label.title])
expect(json_response.first['id']).to eq(merge_request.id)
end
it 'returns an array of merge requests with any label when filtering by any label' do
get api(endpoint_path, user), params: { labels: ["#{label.title} , #{label2.title}"] }
expect_paginated_array_response
expect(json_response).to be_an Array
expect(json_response.length).to eq(1)
expect(json_response.first['labels']).to eq([label2.title, label.title])
expect(json_response.first['id']).to eq(merge_request.id)
end
it 'returns an array of merge requests with any label when filtering by any label' do it 'returns an array of merge requests with any label when filtering by any label' do
get api(endpoint_path, user), params: { labels: IssuesFinder::FILTER_ANY } get api(endpoint_path, user), params: { labels: IssuesFinder::FILTER_ANY }
......
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