Merge commit 'ca3c5c295ed653b483fe81c3918ffe60f46666b9' into rename-ci-commit

* commit 'ca3c5c295ed653b483fe81c3918ffe60f46666b9':
  Let contributors know where to start
  Ensure branch cleanup regardless of whether the import process succeeds
  Fix failing todo tests
  Reorder the todos because the use of the project finder attempts to order them differently
  Update target todo test to use a public project
  Use the project finder in the todos finder to limit todos to just ones within projects you have access to.
  Move filtering todos by projects not pending deletion into a scope on the todo model
  Reduce the filters on the todos joins project query by being explicit about the join
  Ensure we don't show TODOS for projects pending delete
  Fix deprecation warnings in spec/services/issues/bulk_update_service_spec.rb
  Remove unused Issuable#is_assigned? method
  fixup! Don't allow merges with new commits
  fixup! Add `sha` parameter to MR accept API
  Reduce Namespace queries in UserReferenceFilter
  Added ReferenceFilter#nodes
  Returning enums in ReferenceFilter#each_node
  Don't allow merges with new commits
  Add `sha` parameter to MR accept API

21 files changed, 270 insertions, 22 deletions

diff --git a/CHANGELOG b/CHANGELOG
index ec026b8f398..7215a919d79 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -14,6 +14,8 @@ v 8.9.0 (unreleased)
   - Fix groups API to list only user's accessible projects
   - Redesign account and email confirmation emails
   - Use gitlab-shell v3.0.0
+  - Add `sha` parameter to MR merge API, to ensure only reviewed changes are merged
+  - Don't allow MRs to be merged when commits were added since the last review / page load
   - Add DB index on users.state
   - Add rake task 'gitlab:db:configure' for conditionally seeding or migrating the database
   - Changed the Slack build message to use the singular duration if necessary (Aran Koning)
@@ -34,6 +36,13 @@ v 8.9.0 (unreleased)
   - Improve error handling importing projects
   - Put project Files and Commits tabs under Code tab
 
+v 8.8.4
+  - Fix todos page throwing errors when you have a project pending deletion
+  - Reduce number of SQL queries when rendering user references
+
+v 8.8.4 (unreleased)
+  - Ensure branch cleanup regardless of whether the GitHub import process succeeds
+
 v 8.8.3
   - Fix 404 page when viewing TODOs that contain milestones or labels in different projects. !4312
   - Fixed JS error when trying to remove discussion form. !4303
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index a15f8c4fec7..e952855fde1 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -308,7 +308,7 @@ tests are least likely to receive timely feedback. The workflow to make a merge
 request is as follows:
 
 1. Fork the project into your personal space on GitLab.com
-1. Create a feature branch
+1. Create a feature branch, branch away from `master`.
 1. Write [tests](https://gitlab.com/gitlab-org/gitlab-development-kit#running-the-tests) and code
 1. Add your changes to the [CHANGELOG](CHANGELOG)
 1. If you are writing documentation, make sure to read the [documentation styleguide][doc-styleguide]
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 21a70fd69a3..e6924a6a450 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -190,6 +190,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       return
     end
 
+    if params[:sha] != @merge_request.source_sha
+      @status = :sha_mismatch
+      return
+    end
+
     TodoService.new.merge_merge_request(merge_request, current_user)
 
     @merge_request.update(merge_error: nil)
diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index 4bd46a76087..1d88116d7d2 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -30,7 +30,7 @@ class TodosFinder
     items = by_state(items)
     items = by_type(items)
 
-    items
+    items.reorder(id: :desc)
   end
 
   private
@@ -78,6 +78,16 @@ class TodosFinder
     @project
   end
 
+  def projects
+    return @projects if defined?(@projects)
+
+    if project?
+      @projects = project
+    else
+      @projects = ProjectsFinder.new.execute(current_user)
+    end
+  end
+
   def type?
     type.present? && ['Issue', 'MergeRequest'].include?(type)
   end
@@ -105,6 +115,8 @@ class TodosFinder
   def by_project(items)
     if project?
       items = items.where(project: project)
+    elsif projects
+      items = items.merge(projects).joins(:project)
     end
 
     items
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index e86d5236abb..1a4fbbe70d0 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -171,10 +171,6 @@ module Issuable
     today? && created_at == updated_at
   end
 
-  def is_assigned?
-    !!assignee_id
-  end
-
   def is_being_reassigned?
     assignee_id_changed?
   end
diff --git a/app/views/projects/merge_requests/merge.js.haml b/app/views/projects/merge_requests/merge.js.haml
index 92ce479d463..84b6c9ebc5c 100644
--- a/app/views/projects/merge_requests/merge.js.haml
+++ b/app/views/projects/merge_requests/merge.js.haml
@@ -5,6 +5,9 @@
 - when :merge_when_build_succeeds
   :plain
     $('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/merge_when_build_succeeds'))}");
+- when :sha_mismatch
+  :plain
+    $('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/sha_mismatch'))}");
 - else
   :plain
     $('.mr-widget-body').html("#{escape_javascript(render('projects/merge_requests/widget/open/reload'))}");
diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml
index cfdf4edac37..0d49b6471a9 100644
--- a/app/views/projects/merge_requests/widget/open/_accept.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml
@@ -2,6 +2,7 @@
 
 = form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-quick-submit js-requires-input' } do |f|
   = hidden_field_tag :authenticity_token, form_authenticity_token
+  = hidden_field_tag :sha, @merge_request.source_sha
   .accept-merge-holder.clearfix.js-toggle-container
     .clearfix
       .accept-action
diff --git a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
index b83ddcab3a4..ad898ff153b 100644
--- a/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
+++ b/app/views/projects/merge_requests/widget/open/_merge_when_build_succeeds.html.haml
@@ -16,7 +16,7 @@
   - if remove_source_branch_button || user_can_cancel_automatic_merge
     .clearfix.prepend-top-10
       - if remove_source_branch_button
-        = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
+        = link_to merge_namespace_project_merge_request_path(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request, merge_when_build_succeeds: true, should_remove_source_branch: true, sha: @merge_request.source_sha), remote: true, method: :post, class: "btn btn-grouped btn-primary btn-sm remove_source_branch" do
           = icon('times')
           Remove Source Branch When Merged
 
diff --git a/app/views/projects/merge_requests/widget/open/_sha_mismatch.html.haml b/app/views/projects/merge_requests/widget/open/_sha_mismatch.html.haml
new file mode 100644
index 00000000000..499624f8dd8
--- /dev/null
+++ b/app/views/projects/merge_requests/widget/open/_sha_mismatch.html.haml
@@ -0,0 +1,6 @@
+%h4
+  = icon("exclamation-triangle")
+  This merge request has received new commits since the page was loaded.
+
+%p
+  Please reload the page to review the new commits before merging.
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 8217e30fe25..16b892dc3b7 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -413,11 +413,13 @@ curl -X DELETE -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.c
 
 Merge changes submitted with MR using this API.
 
-If merge success you get `200 OK`.
+If the merge succeeds you'll get a `200 OK`.
 
-If it has some conflicts and can not be merged - you get 405 and error message 'Branch cannot be merged'
+If it has some conflicts and can not be merged - you'll get a 405 and the error message 'Branch cannot be merged'
 
-If merge request is already merged or closed - you get 405 and error message 'Method Not Allowed'
+If merge request is already merged or closed - you'll get a 406 and the error message 'Method Not Allowed'
+
+If the `sha` parameter is passed and does not match the HEAD of the source - you'll get a 409 and the error message 'SHA does not match HEAD of source branch'
 
 If you don't have permissions to accept this merge request - you'll get a 401
 
@@ -431,7 +433,8 @@ Parameters:
 - `merge_request_id` (required)             - ID of MR
 - `merge_commit_message` (optional)         - Custom merge commit message
 - `should_remove_source_branch` (optional)  - if `true` removes the source branch
-- `merged_when_build_succeeds` (optional)    - if `true` the MR is merge when the build succeeds
+- `merged_when_build_succeeds` (optional)   - if `true` the MR is merged when the build succeeds
+- `sha` (optional)                          - if present, then this SHA must match the HEAD of the source branch, otherwise the merge will fail
 
 ```json
 {
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index d129c510d63..2e7836dc8fb 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -218,6 +218,7 @@ module API
         #   merge_commit_message (optional)         - Custom merge commit message
         #   should_remove_source_branch (optional)  - When true, the source branch will be deleted if possible
         #   merge_when_build_succeeds (optional)    - When true, this MR will be merged when the build succeeds
+        #   sha (optional)                          - When present, must have the HEAD SHA of the source branch
         # Example:
         #   PUT /projects/:id/merge_requests/:merge_request_id/merge
         #
@@ -233,6 +234,10 @@ module API
 
           render_api_error!('Branch cannot be merged', 406) unless merge_request.can_be_merged?
 
+          if params[:sha] && merge_request.source_sha != params[:sha]
+            render_api_error!("SHA does not match HEAD of source branch: #{merge_request.source_sha}", 409)
+          end
+
           merge_params = {
             commit_message: params[:merge_commit_message],
             should_remove_source_branch: params[:should_remove_source_branch]
diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb
index 41ae0e1f9cc..2d6f34c9cd8 100644
--- a/lib/banzai/filter/reference_filter.rb
+++ b/lib/banzai/filter/reference_filter.rb
@@ -68,6 +68,8 @@ module Banzai
       # by `ignore_ancestor_query`. Link tags are not processed if they have a
       # "gfm" class or the "href" attribute is empty.
       def each_node
+        return to_enum(__method__) unless block_given?
+
         query = %Q{descendant-or-self::text()[not(#{ignore_ancestor_query})]
         | descendant-or-self::a[
           not(contains(concat(" ", @class, " "), " gfm ")) and not(@href = "")
@@ -78,6 +80,11 @@ module Banzai
         end
       end
 
+      # Returns an Array containing all HTML nodes.
+      def nodes
+        @nodes ||= each_node.to_a
+      end
+
       # Yields the link's URL and text whenever the node is a valid <a> tag.
       def yield_valid_link(node)
         link = CGI.unescape(node.attr('href').to_s)
diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb
index 331d8007257..5b0a6d8541b 100644
--- a/lib/banzai/filter/user_reference_filter.rb
+++ b/lib/banzai/filter/user_reference_filter.rb
@@ -29,7 +29,7 @@ module Banzai
         ref_pattern = User.reference_pattern
         ref_pattern_start = /\A#{ref_pattern}\z/
 
-        each_node do |node|
+        nodes.each do |node|
           if text_node?(node)
             replace_text_when_pattern_matches(node, ref_pattern) do |content|
               user_link_filter(content)
@@ -59,7 +59,7 @@ module Banzai
         self.class.references_in(text) do |match, username|
           if username == 'all'
             link_to_all(link_text: link_text)
-          elsif namespace = Namespace.find_by(path: username)
+          elsif namespace = namespaces[username]
             link_to_namespace(namespace, link_text: link_text) || match
           else
             match
@@ -67,6 +67,31 @@ module Banzai
         end
       end
 
+      # Returns a Hash containing all Namespace objects for the username
+      # references in the current document.
+      #
+      # The keys of this Hash are the namespace paths, the values the
+      # corresponding Namespace objects.
+      def namespaces
+        @namespaces ||=
+          Namespace.where(path: usernames).each_with_object({}) do |row, hash|
+            hash[row.path] = row
+          end
+      end
+
+      # Returns all usernames referenced in the current document.
+      def usernames
+        refs = Set.new
+
+        nodes.each do |node|
+          node.to_html.scan(User.reference_pattern) do
+            refs << $~[:user]
+          end
+        end
+
+        refs.to_a
+      end
+
       private
 
       def urls
diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb
index 408d9b79632..9d077e79c39 100644
--- a/lib/gitlab/github_import/importer.rb
+++ b/lib/gitlab/github_import/importer.rb
@@ -89,11 +89,11 @@ module Gitlab
           end
         end
 
-        delete_refs(branches_removed)
-
         true
       rescue ActiveRecord::RecordInvalid => e
         raise Projects::ImportService::Error, e.message
+      ensure
+        delete_refs(branches_removed)
       end
 
       def create_refs(branches)
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 4f621a43d7e..8499bf07e9f 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -185,6 +185,92 @@ describe Projects::MergeRequestsController do
     end
   end
 
+  describe 'POST #merge' do
+    let(:base_params) do
+      {
+        namespace_id: project.namespace.path,
+        project_id: project.path,
+        id: merge_request.iid,
+        format: 'raw'
+      }
+    end
+
+    context 'when the user does not have access' do
+      before do
+        project.team.truncate
+        project.team << [user, :reporter]
+        post :merge, base_params
+      end
+
+      it 'returns not found' do
+        expect(response).to be_not_found
+      end
+    end
+
+    context 'when the merge request is not mergeable' do
+      before do
+        merge_request.update_attributes(title: "WIP: #{merge_request.title}")
+
+        post :merge, base_params
+      end
+
+      it 'returns :failed' do
+        expect(assigns(:status)).to eq(:failed)
+      end
+    end
+
+    context 'when the sha parameter does not match the source SHA' do
+      before { post :merge, base_params.merge(sha: 'foo') }
+
+      it 'returns :sha_mismatch' do
+        expect(assigns(:status)).to eq(:sha_mismatch)
+      end
+    end
+
+    context 'when the sha parameter matches the source SHA' do
+      def merge_with_sha
+        post :merge, base_params.merge(sha: merge_request.source_sha)
+      end
+
+      it 'returns :success' do
+        merge_with_sha
+
+        expect(assigns(:status)).to eq(:success)
+      end
+
+      it 'starts the merge immediately' do
+        expect(MergeWorker).to receive(:perform_async).with(merge_request.id, anything, anything)
+
+        merge_with_sha
+      end
+
+      context 'when merge_when_build_succeeds is passed' do
+        def merge_when_build_succeeds
+          post :merge, base_params.merge(sha: merge_request.source_sha, merge_when_build_succeeds: '1')
+        end
+
+        before do
+          create(:ci_empty_commit, project: project, sha: merge_request.source_sha, ref: merge_request.source_branch)
+        end
+
+        it 'returns :merge_when_build_succeeds' do
+          merge_when_build_succeeds
+
+          expect(assigns(:status)).to eq(:merge_when_build_succeeds)
+        end
+
+        it 'sets the MR to merge when the build succeeds' do
+          service = double(:merge_when_build_succeeds_service)
+
+          expect(MergeRequests::MergeWhenBuildSucceedsService).to receive(:new).with(project, anything, anything).and_return(service)
+          expect(service).to receive(:execute).with(merge_request)
+
+          merge_when_build_succeeds
+        end
+      end
+    end
+  end
+
   describe "DELETE #destroy" do
     it "denies access to users unless they're admin or project owner" do
       delete :destroy, namespace_id: project.namespace.path, project_id: project.path, id: merge_request.iid
diff --git a/spec/features/todos/target_state_spec.rb b/spec/features/todos/target_state_spec.rb
index 72491ac7e61..32fa88a2b21 100644
--- a/spec/features/todos/target_state_spec.rb
+++ b/spec/features/todos/target_state_spec.rb
@@ -3,7 +3,7 @@ require 'rails_helper'
 feature 'Todo target states', feature: true do
   let(:user)    { create(:user) }
   let(:author)  { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
 
   before do
     login_as user
diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb
index 4e627753cc7..8e1833a069e 100644
--- a/spec/features/todos/todos_spec.rb
+++ b/spec/features/todos/todos_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe 'Dashboard Todos', feature: true do
   let(:user)    { create(:user) }
   let(:author)  { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) }
   let(:issue)   { create(:issue) }
 
   describe 'GET /dashboard/todos' do
@@ -49,7 +49,7 @@ describe 'Dashboard Todos', feature: true do
         note1 = create(:note_on_issue, note: "Hello #{label1.to_reference(format: :name)}", noteable_id: issue.id, noteable_type: 'Issue', project: issue.project)
         create(:todo, :mentioned, project: project, target: issue, user: user, note_id: note1.id)
 
-        project2 = create(:project)
+        project2 = create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC)
         label2 = create(:label, project: project2)
         issue2 = create(:issue, project: project2)
         note2 = create(:note_on_issue, note: "Test #{label2.to_reference(format: :name)}", noteable_id: issue2.id, noteable_type: 'Issue', project: project2)
@@ -98,5 +98,18 @@ describe 'Dashboard Todos', feature: true do
         end
       end
     end
+
+    context 'User has a Todo in a project pending deletion' do
+      before do
+        deleted_project = create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC, pending_delete: true)
+        create(:todo, :mentioned, user: user, project: deleted_project, target: issue, author: author)
+        login_as(user)
+        visit dashboard_todos_path
+      end
+
+      it 'shows "All done" message' do
+        expect(page).to have_content "You're all done!"
+      end
+    end
   end
 end
diff --git a/spec/lib/banzai/filter/reference_filter_spec.rb b/spec/lib/banzai/filter/reference_filter_spec.rb
new file mode 100644
index 00000000000..55e681f6faf
--- /dev/null
+++ b/spec/lib/banzai/filter/reference_filter_spec.rb
@@ -0,0 +1,45 @@
+require 'spec_helper'
+
+describe Banzai::Filter::ReferenceFilter, lib: true do
+  let(:project) { build(:project) }
+
+  describe '#each_node' do
+    it 'iterates over the nodes in a document' do
+      document = Nokogiri::HTML.fragment('<a href="foo">foo</a>')
+      filter = described_class.new(document, project: project)
+
+      expect { |b| filter.each_node(&b) }.
+        to yield_with_args(an_instance_of(Nokogiri::XML::Element))
+    end
+
+    it 'returns an Enumerator when no block is given' do
+      document = Nokogiri::HTML.fragment('<a href="foo">foo</a>')
+      filter = described_class.new(document, project: project)
+
+      expect(filter.each_node).to be_an_instance_of(Enumerator)
+    end
+
+    it 'skips links with a "gfm" class' do
+      document = Nokogiri::HTML.fragment('<a href="foo" class="gfm">foo</a>')
+      filter = described_class.new(document, project: project)
+
+      expect { |b| filter.each_node(&b) }.not_to yield_control
+    end
+
+    it 'skips text nodes in pre elements' do
+      document = Nokogiri::HTML.fragment('<pre>foo</pre>')
+      filter = described_class.new(document, project: project)
+
+      expect { |b| filter.each_node(&b) }.not_to yield_control
+    end
+  end
+
+  describe '#nodes' do
+    it 'returns an Array of the HTML nodes' do
+      document = Nokogiri::HTML.fragment('<a href="foo">foo</a>')
+      filter = described_class.new(document, project: project)
+
+      expect(filter.nodes).to eq([document.children[0]])
+    end
+  end
+end
diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb
index d7dfd6699ef..108b36a97cc 100644
--- a/spec/lib/banzai/filter/user_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb
@@ -136,4 +136,23 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do
       expect(link.attr('data-user')).to eq user.namespace.owner_id.to_s
     end
   end
+
+  describe '#namespaces' do
+    it 'returns a Hash containing all Namespaces' do
+      document = Nokogiri::HTML.fragment("<p>#{user.to_reference}</p>")
+      filter = described_class.new(document, project: project)
+      ns = user.namespace
+
+      expect(filter.namespaces).to eq({ ns.path => ns })
+    end
+  end
+
+  describe '#usernames' do
+    it 'returns the usernames mentioned in a document' do
+      document = Nokogiri::HTML.fragment("<p>#{user.to_reference}</p>")
+      filter = described_class.new(document, project: project)
+
+      expect(filter.usernames).to eq([user.username])
+    end
+  end
 end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index d8569d88ef0..04cf15641d0 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -428,6 +428,19 @@ describe API::API, api: true  do
       expect(json_response['message']).to eq('401 Unauthorized')
     end
 
+    it "returns 409 if the SHA parameter doesn't match" do
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.source_sha.succ
+
+      expect(response.status).to eq(409)
+      expect(json_response['message']).to start_with('SHA does not match HEAD of source branch')
+    end
+
+    it "succeeds if the SHA parameter matches" do
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user), sha: merge_request.source_sha
+
+      expect(response.status).to eq(200)
+    end
+
     it "enables merge when build succeeds if the ci is active" do
       allow_any_instance_of(MergeRequest).to receive(:pipeline).and_return(pipeline)
       allow(ci_commit).to receive(:active?).and_return(true)
diff --git a/spec/services/issues/bulk_update_service_spec.rb b/spec/services/issues/bulk_update_service_spec.rb
index 96f050bbd9b..454d5849495 100644
--- a/spec/services/issues/bulk_update_service_spec.rb
+++ b/spec/services/issues/bulk_update_service_spec.rb
@@ -18,7 +18,7 @@ describe Issues::BulkUpdateService, services: true do
       @issues = create_list(:issue, 5, project: @project)
       @params = {
         state_event: 'close',
-        issues_ids: @issues.map(&:id)
+        issues_ids: @issues.map(&:id).join(",")
       }
     end
 
@@ -38,7 +38,7 @@ describe Issues::BulkUpdateService, services: true do
       @issues = create_list(:closed_issue, 5, project: @project)
       @params = {
         state_event: 'reopen',
-        issues_ids: @issues.map(&:id)
+        issues_ids: @issues.map(&:id).join(",")
       }
     end
 
@@ -58,7 +58,7 @@ describe Issues::BulkUpdateService, services: true do
     before do
       @new_assignee = create :user
       @params = {
-        issues_ids: [issue.id],
+        issues_ids: issue.id.to_s,
         assignee_id: @new_assignee.id
       }
     end
@@ -97,7 +97,7 @@ describe Issues::BulkUpdateService, services: true do
     before do
       @milestone = create(:milestone, project: @project)
       @params = {
-        issues_ids: [issue.id],
+        issues_ids: issue.id.to_s,
         milestone_id: @milestone.id
       }
     end