Incorporate review feedback

Use shared examples for issues and merge requests
rather than a loop creating common specs.

4 files changed, 108 insertions, 106 deletions

diff --git a/doc/api/issues.md b/doc/api/issues.md
index ccd28d21e98..81553746834 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -637,7 +637,7 @@ Example response:
 
 ## Bulk update issues
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21368) in GitLab 11.8.
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21368) in GitLab 11.9.
 
 Update multiple issues using a single API call. Returns the number of successfully updated issues.
 
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 05bc3d098c5..78cb4db74e1 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -958,7 +958,7 @@ Must include at least one non-required attribute from above.
 
 ## Bulk update merge requests
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21368) in GitLab 11.8.
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/21368) in GitLab 11.9.
 
 Update multiple merge requests using a single API call. Returns the number of successfully updated merge requests.
 
diff --git a/lib/api/issuable_bulk_update.rb b/lib/api/issuable_bulk_update.rb
index bcda8b32976..5ac6c252d96 100644
--- a/lib/api/issuable_bulk_update.rb
+++ b/lib/api/issuable_bulk_update.rb
@@ -11,7 +11,7 @@ module API
           detail 'This feature was introduced in 11.9'
         end
         params do
-          requires :issuable_ids, type: Array[Integer], desc: "Array or #{issuable.pluralize} IDs to be updates"
+          requires :issuable_ids, type: Array[Integer], desc: "Array of #{issuable.pluralize} IDs to be updated"
           optional :state_event, type: String, values: %w(reopen close), desc: 'Reopens or closes a resource'
           optional :milestone_id, type: Integer, desc: 'The milestone ID number'
           optional :add_label_ids, type: Array[Integer], desc: 'IDs of labels to be added'
@@ -20,7 +20,7 @@ module API
                                         desc: 'Subscribes or unsubscribes from a resource'
 
           if issuable == 'issue'
-            optional :assignee_ids, type: Array[Integer], desc: 'List of assignees IDs'
+            optional :assignee_ids, type: Array[Integer], desc: 'List of assignee IDs'
             at_least_one_of :state_event, :milestone_id, :add_label_ids, :remove_label_ids,
                             :subscription_event, :assignee_ids
           else
diff --git a/spec/requests/api/issuable_bulk_update_spec.rb b/spec/requests/api/issuable_bulk_update_spec.rb
index 810fa8dc529..6463f3f5d35 100644
--- a/spec/requests/api/issuable_bulk_update_spec.rb
+++ b/spec/requests/api/issuable_bulk_update_spec.rb
@@ -3,150 +3,152 @@
 require 'spec_helper'
 
 describe API::IssuableBulkUpdate do
-  set(:user) { create(:user) }
-  set(:project) do
-    create(:project, :public, creator_id: user.id, namespace: user.namespace)
-  end
-
-  %w(issue merge_request).each do |issuable|
-    describe "PUT /projects/:id/#{issuable.pluralize}/bulk_update" do
-      let(:merge_request_1) { create(:merge_request, source_project: project) }
-      let(:merge_request_2) { create(:merge_request, :simple, source_project: project) }
-      let!(:issuables) { issuable == 'issue' ? create_list(:issue, 2, project: project) : [merge_request_1, merge_request_2] }
+  set(:project) { create(:project) }
+  set(:user) { project.creator }
 
-      def bulk_update(issuable, issuables, params, update_user = user)
-        put api("/projects/#{project.id}/#{issuable.pluralize}/bulk_update", update_user),
-          params: { issuable_ids: Array(issuables).map(&:id) }.merge(params)
-      end
+  shared_examples "PUT /projects/:id/:issuable/bulk_update" do |issuable|
+    def bulk_update(issuable, issuables, params, update_user = user)
+      put api("/projects/#{project.id}/#{issuable.pluralize}/bulk_update", update_user),
+        params: { issuable_ids: Array(issuables).map(&:id) }.merge(params)
+    end
 
-      context 'with not enough permissions' do
-        it 'returns 403 for guest users' do
-          guest = create(:user)
-          project.add_guest(guest)
+    context 'with not enough permissions' do
+      it 'returns 403 for guest users' do
+        guest = create(:user)
+        project.add_guest(guest)
 
-          bulk_update(issuable, issuables, { state_event: 'close' }, guest)
+        bulk_update(issuable, issuables, { state_event: 'close' }, guest)
 
-          expect(response).to have_gitlab_http_status(403)
-        end
+        expect(response).to have_gitlab_http_status(403)
       end
+    end
 
-      context 'when modifying the state' do
-        it "closes #{issuable}" do
-          bulk_update(issuable, issuables, { state_event: 'close' })
+    context 'when modifying the state' do
+      it "closes #{issuable}" do
+        bulk_update(issuable, issuables, { state_event: 'close' })
 
-          expect(response).to have_gitlab_http_status(200)
-          expect(json_response['message']).to eq("#{issuables.count} #{issuable.pluralize(issuables.count)} updated")
-          expect(project.public_send(issuable.pluralize).opened).to be_empty
-          expect(project.public_send(issuable.pluralize).closed).not_to be_empty
-        end
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['message']).to eq("#{issuables.count} #{issuable.pluralize(issuables.count)} updated")
+        expect(project.public_send(issuable.pluralize).opened).to be_empty
+        expect(project.public_send(issuable.pluralize).closed).not_to be_empty
+      end
 
-        it "opens #{issuable}" do
-          closed_issuables = create_list("closed_#{issuable}".to_sym, 2)
+      it "opens #{issuable}" do
+        closed_issuables = create_list("closed_#{issuable}".to_sym, 2)
 
-          bulk_update(issuable, closed_issuables, { state_event: 'reopen' })
+        bulk_update(issuable, closed_issuables, { state_event: 'reopen' })
 
-          expect(response).to have_gitlab_http_status(200)
-          expect(project.public_send(issuable.pluralize).closed).to be_empty
-        end
+        expect(response).to have_gitlab_http_status(200)
+        expect(project.public_send(issuable.pluralize).closed).to be_empty
       end
+    end
 
-      context 'when modifying the milestone' do
-        let(:milestone) { create(:milestone, project: project) }
+    context 'when modifying the milestone' do
+      let(:milestone) { create(:milestone, project: project) }
 
-        it "adds a milestone #{issuable}" do
-          bulk_update(issuable, issuables, { milestone_id: milestone.id })
+      it "adds a milestone #{issuable}" do
+        bulk_update(issuable, issuables, { milestone_id: milestone.id })
 
-          expect(response).to have_gitlab_http_status(200)
-          issuables.each do |issuable|
-            expect(issuable.reload.milestone).to eq(milestone)
-          end
+        expect(response).to have_gitlab_http_status(200)
+        issuables.each do |issuable|
+          expect(issuable.reload.milestone).to eq(milestone)
         end
+      end
 
-        it 'removes a milestone' do
-          issuables.first.milestone = milestone
-          milestone_issuable = issuables.first
+      it 'removes a milestone' do
+        issuables.first.milestone = milestone
+        milestone_issuable = issuables.first
 
-          bulk_update(issuable, [milestone_issuable], { milestone_id: 0 })
+        bulk_update(issuable, [milestone_issuable], { milestone_id: 0 })
 
-          expect(response).to have_gitlab_http_status(200)
-          expect(milestone_issuable.reload.milestone).to eq(nil)
-        end
+        expect(response).to have_gitlab_http_status(200)
+        expect(milestone_issuable.reload.milestone).to eq(nil)
       end
+    end
 
-      context 'when modifying the subscription state' do
-        it "subscribes to #{issuable}" do
-          bulk_update(issuable, issuables, { subscription_event: 'subscribe' })
+    context 'when modifying the subscription state' do
+      it "subscribes to #{issuable}" do
+        bulk_update(issuable, issuables, { subscription_event: 'subscribe' })
 
-          expect(response).to have_gitlab_http_status(200)
-          expect(issuables).to all(be_subscribed(user, project))
-        end
+        expect(response).to have_gitlab_http_status(200)
+        expect(issuables).to all(be_subscribed(user, project))
+      end
 
-        it 'unsubscribes from issues' do
-          issuables.each do |issuable|
-            issuable.subscriptions.create(user: user, project: project, subscribed: true)
-          end
+      it 'unsubscribes from issues' do
+        issuables.each do |issuable|
+          issuable.subscriptions.create(user: user, project: project, subscribed: true)
+        end
 
-          bulk_update(issuable, issuables, { subscription_event: 'unsubscribe' })
+        bulk_update(issuable, issuables, { subscription_event: 'unsubscribe' })
 
-          expect(response).to have_gitlab_http_status(200)
-          issuables.each do |issuable|
-            expect(issuable).not_to be_subscribed(user, project)
-          end
+        expect(response).to have_gitlab_http_status(200)
+        issuables.each do |issuable|
+          expect(issuable).not_to be_subscribed(user, project)
         end
       end
+    end
 
-      context 'when modifying the assignee' do
-        it 'adds assignee to issues' do
-          params = issuable == 'issue' ? { assignee_ids: [user.id] } : { assignee_id: user.id }
+    context 'when modifying the assignee' do
+      it 'adds assignee to issues' do
+        params = issuable == 'issue' ? { assignee_ids: [user.id] } : { assignee_id: user.id }
 
-          bulk_update(issuable, issuables, params)
+        bulk_update(issuable, issuables, params)
 
-          expect(response).to have_gitlab_http_status(200)
-          issuables.each do |issuable|
-            expect(issuable.reload.assignees).to eq([user])
-          end
+        expect(response).to have_gitlab_http_status(200)
+        issuables.each do |issuable|
+          expect(issuable.reload.assignees).to eq([user])
         end
+      end
 
-        it 'removes assignee' do
-          assigned_issuable = issuables.first
-
-          if issuable == 'issue'
-            params = { assignee_ids: 0 }
-            assigned_issuable.assignees << user
-          else
-            params = { assignee_id: 0 }
-            assigned_issuable.update_attribute(:assignee, user)
-          end
+      it 'removes assignee' do
+        assigned_issuable = issuables.first
 
-          bulk_update(issuable, [assigned_issuable], params)
-          expect(assigned_issuable.reload.assignees).to eq([])
+        if issuable == 'issue'
+          params = { assignee_ids: 0 }
+          assigned_issuable.assignees << user
+        else
+          params = { assignee_id: 0 }
+          assigned_issuable.update_attribute(:assignee, user)
         end
+
+        bulk_update(issuable, [assigned_issuable], params)
+        expect(assigned_issuable.reload.assignees).to eq([])
       end
+    end
 
-      context 'when modifying labels' do
-        let(:bug) { create(:label, project: project) }
-        let(:regression) { create(:label, project: project) }
-        let(:feature) { create(:label, project: project) }
+    context 'when modifying labels' do
+      let(:bug) { create(:label, project: project) }
+      let(:regression) { create(:label, project: project) }
+      let(:feature) { create(:label, project: project) }
 
-        it 'adds new labels' do
-          bulk_update(issuable, issuables, { add_label_ids: [bug.id, regression.id, feature.id] })
+      it 'adds new labels' do
+        bulk_update(issuable, issuables, { add_label_ids: [bug.id, regression.id, feature.id] })
 
-          issuables.each do |issusable|
-            expect(issusable.reload.label_ids).to contain_exactly(bug.id, regression.id, feature.id)
-          end
+        issuables.each do |issusable|
+          expect(issusable.reload.label_ids).to contain_exactly(bug.id, regression.id, feature.id)
         end
+      end
 
-        it 'removes labels' do
-          labled_issuable = issuables.first
-          labled_issuable.labels << bug
-          labled_issuable.labels << regression
-          labled_issuable.labels << feature
+      it 'removes labels' do
+        labled_issuable = issuables.first
+        labled_issuable.labels << bug
+        labled_issuable.labels << regression
+        labled_issuable.labels << feature
 
-          bulk_update(issuable, [labled_issuable], { remove_label_ids: [bug.id, regression.id] })
+        bulk_update(issuable, [labled_issuable], { remove_label_ids: [bug.id, regression.id] })
 
-          expect(labled_issuable.reload.label_ids).to contain_exactly(feature.id)
-        end
+        expect(labled_issuable.reload.label_ids).to contain_exactly(feature.id)
       end
     end
   end
+
+  it_behaves_like 'PUT /projects/:id/:issuable/bulk_update', 'issue' do
+    let(:issuables) { create_list(:issue, 2, project: project) }
+  end
+
+  it_behaves_like 'PUT /projects/:id/:issuable/bulk_update', 'merge_request' do
+    let(:merge_request_1) { create(:merge_request, source_project: project) }
+    let(:merge_request_2) { create(:merge_request, :simple, source_project: project) }
+    let(:issuables) { [merge_request_1, merge_request_2] }
+  end
 end