Merge branch 'api-mr-put-labels' into 'master'

PUT MergeRequest API endpoint - accept labels as an array

See merge request gitlab-org/gitlab-ce!19914

2 files changed, 408 insertions, 54 deletions

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 6272bb38d59..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,19 +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 '&'
+      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