diff options
author | Alexandru Croitor <acroitor@gitlab.com> | 2019-03-01 16:37:22 +0200 |
---|---|---|
committer | Alexandru Croitor <acroitor@gitlab.com> | 2019-03-06 13:28:42 +0200 |
commit | be3578d24585c7f330103cc546eb50416cd951d3 (patch) | |
tree | e423c5fac61e5f25e05c5cfc5f2853623f7deaa2 | |
parent | ee8cb2d1b37c5c0d990a35af29be2877092eef29 (diff) | |
download | gitlab-ce-be3578d24585c7f330103cc546eb50416cd951d3.tar.gz |
Add array support for labels
* Support label parameter as comma separated and array of strings
for merge requests and issues api endpoints
-rw-r--r-- | app/services/issuable_base_service.rb | 2 | ||||
-rw-r--r-- | lib/api/issues.rb | 4 | ||||
-rw-r--r-- | lib/api/merge_requests.rb | 4 | ||||
-rw-r--r-- | lib/api/validations/types/labels_list.rb | 24 | ||||
-rw-r--r-- | spec/requests/api/issues_spec.rb | 218 | ||||
-rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 257 | ||||
-rw-r--r-- | spec/support/shared_examples/requests/api/merge_requests_list.rb | 31 |
7 files changed, 468 insertions, 72 deletions
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 1e1f2fbd08e..f35ad2a9d8b 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -89,7 +89,7 @@ class IssuableBaseService < BaseService return unless labels - params[:label_ids] = labels.split(",").map do |label_name| + params[:label_ids] = labels.map do |label_name| label = Labels::FindOrCreateService.new( current_user, parent, diff --git a/lib/api/issues.rb b/lib/api/issues.rb index f43f4d961d6..d59d2f5a098 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -34,7 +34,7 @@ module API # rubocop: enable CodeReuse/ActiveRecord 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 :order_by, type: String, values: %w[created_at updated_at], default: 'created_at', desc: 'Return issues ordered by `created_at` or `updated_at` fields.' @@ -65,7 +65,7 @@ module API 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 :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 :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" diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 2e794076a43..b2a2aa8837d 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -95,7 +95,7 @@ module API optional :sort, type: String, values: %w[asc desc], default: 'desc', desc: 'Return merge requests sorted in `asc` or `desc` order.' 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_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' @@ -179,7 +179,7 @@ module API 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 :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 :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' diff --git a/lib/api/validations/types/labels_list.rb b/lib/api/validations/types/labels_list.rb new file mode 100644 index 00000000000..47cd83c29cf --- /dev/null +++ b/lib/api/validations/types/labels_list.rb @@ -0,0 +1,24 @@ +# 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 diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index f35dabf5d0f..a5434d3ea80 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -271,7 +271,14 @@ describe API::Issues do end it 'returns an array of labeled issues' do - get api("/issues", user), params: { labels: label.title } + get api('/issues', user), params: { labels: label.title } + + expect_paginated_array_response(issue.id) + expect(json_response.first['labels']).to eq([label.title]) + end + + it 'returns an array of labeled issues with labels param as array' do + get api('/issues', user), params: { labels: [label.title] } expect_paginated_array_response(issue.id) expect(json_response.first['labels']).to eq([label.title]) @@ -284,7 +291,20 @@ describe API::Issues do create(:label_link, label: label_b, target: issue) create(:label_link, label: label_c, target: issue) - get api("/issues", user), params: { labels: "#{label.title},#{label_b.title},#{label_c.title}" } + get api('/issues', user), params: { labels: "#{label.title},#{label_b.title},#{label_c.title}" } + + expect_paginated_array_response(issue.id) + expect(json_response.first['labels']).to eq([label_c.title, label_b.title, label.title]) + end + + it 'returns an array of labeled issues when all labels matches with labels param as array' do + label_b = create(:label, title: 'foo', project: project) + label_c = create(:label, title: 'bar', project: project) + + create(:label_link, label: label_b, target: issue) + create(:label_link, label: label_c, target: issue) + + get api('/issues', user), params: { labels: [label.title, label_b.title, label_c.title] } expect_paginated_array_response(issue.id) expect(json_response.first['labels']).to eq([label_c.title, label_b.title, label.title]) @@ -296,8 +316,22 @@ describe API::Issues do expect_paginated_array_response([]) end + it 'returns an empty array if no issue matches labels with labels param as array' do + get api('/issues', user), params: { labels: %w(foo bar) } + + expect_paginated_array_response([]) + end + it 'returns an array of labeled issues matching given state' do - get api("/issues", user), params: { labels: label.title, state: :opened } + get api('/issues', user), params: { labels: label.title, state: :opened } + + expect_paginated_array_response(issue.id) + expect(json_response.first['labels']).to eq([label.title]) + expect(json_response.first['state']).to eq('opened') + end + + it 'returns an array of labeled issues matching given state with labels param as array' do + get api('/issues', user), params: { labels: [label.title], state: :opened } expect_paginated_array_response(issue.id) expect(json_response.first['labels']).to eq([label.title]) @@ -305,25 +339,43 @@ describe API::Issues do end it 'returns an empty array if no issue matches labels and state filters' do - get api("/issues", user), params: { labels: label.title, state: :closed } + get api('/issues', user), params: { labels: label.title, state: :closed } expect_paginated_array_response([]) end it 'returns an array of issues with any label' do - get api("/issues", user), params: { labels: IssuesFinder::FILTER_ANY } + get api('/issues', user), params: { labels: IssuesFinder::FILTER_ANY } + + expect_paginated_array_response(issue.id) + end + + it 'returns an array of issues with any label with labels param as array' do + get api('/issues', user), params: { labels: [IssuesFinder::FILTER_ANY] } expect_paginated_array_response(issue.id) end it 'returns an array of issues with no label' do - get api("/issues", user), params: { labels: IssuesFinder::FILTER_NONE } + get api('/issues', user), params: { labels: IssuesFinder::FILTER_NONE } + + expect_paginated_array_response(closed_issue.id) + end + + it 'returns an array of issues with no label with labels param as array' do + get api('/issues', user), params: { labels: [IssuesFinder::FILTER_NONE] } expect_paginated_array_response(closed_issue.id) end it 'returns an array of issues with no label when using the legacy No+Label filter' do - get api("/issues", user), params: { labels: "No Label" } + get api('/issues', user), params: { labels: 'No Label' } + + expect_paginated_array_response(closed_issue.id) + end + + it 'returns an array of issues with no label when using the legacy No+Label filter with labels param as array' do + get api('/issues', user), params: { labels: ['No Label'] } expect_paginated_array_response(closed_issue.id) end @@ -588,12 +640,25 @@ describe API::Issues do expect(json_response.first['labels']).to eq([group_label.title]) end + it 'returns an array of labeled group issues with labels param as array' do + get api(base_url, user), params: { labels: [group_label.title] } + + expect_paginated_array_response(group_issue.id) + expect(json_response.first['labels']).to eq([group_label.title]) + end + it 'returns an array of labeled group issues where all labels match' do get api(base_url, user), params: { labels: "#{group_label.title},foo,bar" } expect_paginated_array_response([]) end + it 'returns an array of labeled group issues where all labels match with labels param as array' do + get api(base_url, user), params: { labels: [group_label.title, 'foo', 'bar'] } + + expect_paginated_array_response([]) + end + it 'returns issues matching given search string for title' do get api(base_url, user), params: { search: group_issue.title } @@ -619,6 +684,19 @@ describe API::Issues do expect(json_response.first['labels']).to eq([label_c.title, label_b.title, group_label.title]) end + it 'returns an array of labeled issues when all labels matches with labels param as array' do + label_b = create(:label, title: 'foo', project: group_project) + label_c = create(:label, title: 'bar', project: group_project) + + create(:label_link, label: label_b, target: group_issue) + create(:label_link, label: label_c, target: group_issue) + + get api(base_url, user), params: { labels: [group_label.title, label_b.title, label_c.title] } + + expect_paginated_array_response(group_issue.id) + expect(json_response.first['labels']).to eq([label_c.title, label_b.title, group_label.title]) + end + it 'returns an array of issues found by iids' do get api(base_url, user), params: { iids: [group_issue.iid] } @@ -645,12 +723,25 @@ describe API::Issues do expect(json_response.first['id']).to eq(group_issue.id) end + it 'returns an array of group issues with any label with labels param as array' do + get api(base_url, user), params: { labels: [IssuesFinder::FILTER_ANY] } + + expect_paginated_array_response(group_issue.id) + expect(json_response.first['id']).to eq(group_issue.id) + end + it 'returns an array of group issues with no label' do get api(base_url, user), params: { labels: IssuesFinder::FILTER_NONE } expect_paginated_array_response([group_closed_issue.id, group_confidential_issue.id]) end + it 'returns an array of group issues with no label with labels param as array' do + get api(base_url, user), params: { labels: [IssuesFinder::FILTER_NONE] } + + expect_paginated_array_response([group_closed_issue.id, group_confidential_issue.id]) + end + it 'returns an empty array if no issue matches milestone' do get api(base_url, user), params: { milestone: group_empty_milestone.title } @@ -842,6 +933,12 @@ describe API::Issues do expect_paginated_array_response(issue.id) end + it 'returns an array of labeled project issues with labels param as array' do + get api("#{base_url}/issues", user), params: { labels: [label.title] } + + expect_paginated_array_response(issue.id) + end + it 'returns an array of labeled issues when all labels matches' do label_b = create(:label, title: 'foo', project: project) label_c = create(:label, title: 'bar', project: project) @@ -854,6 +951,18 @@ describe API::Issues do expect_paginated_array_response(issue.id) end + it 'returns an array of labeled issues when all labels matches with labels param as array' do + label_b = create(:label, title: 'foo', project: project) + label_c = create(:label, title: 'bar', project: project) + + create(:label_link, label: label_b, target: issue) + create(:label_link, label: label_c, target: issue) + + get api("#{base_url}/issues", user), params: { labels: [label.title, label_b.title, label_c.title] } + + expect_paginated_array_response(issue.id) + end + it 'returns issues matching given search string for title' do get api("#{base_url}/issues?search=#{issue.title}", user) @@ -890,12 +999,24 @@ describe API::Issues do expect_paginated_array_response(issue.id) end + it 'returns an array of project issues with any label with labels param as array' do + get api("#{base_url}/issues", user), params: { labels: [IssuesFinder::FILTER_ANY] } + + expect_paginated_array_response(issue.id) + end + it 'returns an array of project issues with no label' do get api("#{base_url}/issues", user), params: { labels: IssuesFinder::FILTER_NONE } expect_paginated_array_response([confidential_issue.id, closed_issue.id]) end + it 'returns an array of project issues with no label with labels param as array' do + get api("#{base_url}/issues", user), params: { labels: [IssuesFinder::FILTER_NONE] } + + expect_paginated_array_response([confidential_issue.id, closed_issue.id]) + end + it 'returns an empty array if no project issue matches labels' do get api("#{base_url}/issues", user), params: { labels: 'foo,bar' } @@ -1215,6 +1336,19 @@ describe API::Issues do expect(json_response['assignees'].first['name']).to eq(user2.name) end + it 'creates a new project issue with labels param as array' do + post api("/projects/#{project.id}/issues", user), + params: { title: 'new issue', labels: %w(label label2), weight: 3, assignee_ids: [user2.id] } + + expect(response).to have_gitlab_http_status(201) + expect(json_response['title']).to eq('new issue') + expect(json_response['description']).to be_nil + expect(json_response['labels']).to eq(%w(label label2)) + expect(json_response['confidential']).to be_falsy + expect(json_response['assignee']['name']).to eq(user2.name) + expect(json_response['assignees'].first['name']).to eq(user2.name) + end + it 'creates a new confidential project issue' do post api("/projects/#{project.id}/issues", user), params: { title: 'new issue', confidential: true } @@ -1269,6 +1403,20 @@ describe API::Issues do expect(json_response['labels']).to include '&' end + it 'allows special label names with labels param as array' do + post api("/projects/#{project.id}/issues", user), + params: { + title: 'new issue', + labels: ['label', 'label?', 'label&foo, ?, &'] + } + expect(response.status).to eq(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 'returns 400 if title is too long' do post api("/projects/#{project.id}/issues", user), params: { title: 'g' * 256 } @@ -1377,6 +1525,12 @@ describe API::Issues do post api("/projects/#{project.id}/issues", non_member), params: { title: 'new issue', labels: 'label, label2' } end.not_to change { project.labels.count } end + + it 'cannot create new labels with labels param as array' do + expect do + post api("/projects/#{project.id}/issues", non_member), params: { title: 'new issue', labels: %w(label label2) } + end.not_to change { project.labels.count } + end end end @@ -1444,6 +1598,21 @@ describe API::Issues do expect(json_response['labels']).to include '&' end + it 'allows special label names with labels param as array' do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), + params: { + title: 'updated title', + labels: ['label', 'label?', 'label&foo, ?, &'] + } + + expect(response.status).to eq(200) + 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 'confidential issues' do it "returns 403 for non project members" do put api("/projects/#{project.id}/issues/#{confidential_issue.iid}", non_member), @@ -1603,6 +1772,16 @@ describe API::Issues do expect(json_response['updated_at']).to be > Time.now end + it 'removes all labels and touches the record with labels param as array' do + Timecop.travel(1.minute.from_now) do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: { labels: [''] } + end + + expect(response).to have_gitlab_http_status(200) + expect(json_response['labels']).to eq([]) + expect(json_response['updated_at']).to be > Time.now + end + it 'updates labels and touches the record' do Timecop.travel(1.minute.from_now) do put api("/projects/#{project.id}/issues/#{issue.iid}", user), @@ -1614,6 +1793,17 @@ describe API::Issues do expect(json_response['updated_at']).to be > Time.now end + it 'updates labels and touches the record with labels param as array' do + Timecop.travel(1.minute.from_now) do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), + params: { labels: %w(foo bar) } + end + expect(response).to have_gitlab_http_status(200) + expect(json_response['labels']).to include 'foo' + expect(json_response['labels']).to include 'bar' + expect(json_response['updated_at']).to be > Time.now + end + it 'allows special label names' do put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: { labels: 'label:foo, label-bar,label_bar,label/bar,label?bar,label&bar,?,&' } @@ -1628,6 +1818,20 @@ describe API::Issues do expect(json_response['labels']).to include '&' end + it 'allows special label names with labels param as array' do + put api("/projects/#{project.id}/issues/#{issue.iid}", user), + params: { 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' + expect(json_response['labels']).to include 'label-bar' + expect(json_response['labels']).to include 'label_bar' + expect(json_response['labels']).to include 'label/bar' + expect(json_response['labels']).to include 'label?bar' + expect(json_response['labels']).to include 'label&bar' + expect(json_response['labels']).to include '?' + expect(json_response['labels']).to include '&' + end + it 'returns 400 if title is too long' do put api("/projects/#{project.id}/issues/#{issue.iid}", user), params: { title: 'g' * 256 } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 79acb76a957..6752a1c36bf 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -617,26 +617,115 @@ describe API::MergeRequests do end end - describe "POST /projects/:id/merge_requests" do + describe 'POST /projects/:id/merge_requests' do context 'between branches projects' do - it "returns merge_request" do - post api("/projects/#{project.id}/merge_requests", user), - params: { - title: 'Test merge_request', - source_branch: 'feature_conflict', - target_branch: 'master', - author: user, - labels: 'label, label2', - milestone_id: milestone.id, - squash: true - } + context 'different labels' do + let(:params) do + { + title: 'Test merge_request', + source_branch: 'feature_conflict', + target_branch: 'master', + author_id: user.id, + milestone_id: milestone.id, + squash: true + } + end - expect(response).to have_gitlab_http_status(201) - expect(json_response['title']).to eq('Test merge_request') - expect(json_response['labels']).to eq(%w(label label2)) - expect(json_response['milestone']['id']).to eq(milestone.id) - expect(json_response['squash']).to be_truthy - expect(json_response['force_remove_source_branch']).to be_falsy + 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(json_response['title']).to eq('Test merge_request') + expect(json_response['labels']).to eq(%w(label label2)) + expect(json_response['milestone']['id']).to eq(milestone.id) + expect(json_response['squash']).to be_truthy + expect(json_response['force_remove_source_branch']).to be_falsy + 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 @@ -663,23 +752,6 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(400) 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 before do post api("/projects/#{project.id}/merge_requests", user), @@ -1122,32 +1194,97 @@ describe API::MergeRequests do expect(json_response['force_remove_source_branch']).to be_truthy end - it 'allows special label names' do - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), - params: { - title: 'new issue', - labels: 'label, label?, label&foo, ?, &' - } + context 'when updating labels' do + it 'allows special label names' do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), + params: { + title: 'new issue', + labels: 'label, label?, label&foo, ?, &' + } + + expect(response.status).to eq(200) + 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 - expect(response.status).to eq(200) - 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 'also accepts labels as an array' do - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), - title: 'new issue', - labels: ['label', 'label?', 'label&foo', '?', '&'] - - expect(response.status).to eq(200) - 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 '&' + it 'also accepts labels as an array' do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), + params: { + title: 'new issue', + labels: ['label', 'label?', 'label&foo, ?, &', '1, 2', 3, 4] + } + + expect(response.status).to eq(200) + 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 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 it 'does not update state when title is empty' do diff --git a/spec/support/shared_examples/requests/api/merge_requests_list.rb b/spec/support/shared_examples/requests/api/merge_requests_list.rb index 6713ec47ace..32e3b81c3c5 100644 --- a/spec/support/shared_examples/requests/api/merge_requests_list.rb +++ b/spec/support/shared_examples/requests/api/merge_requests_list.rb @@ -186,6 +186,37 @@ shared_examples 'merge requests list' do expect(json_response.length).to eq(0) 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 get api(endpoint_path, user), params: { labels: IssuesFinder::FILTER_ANY } |