Merge branch '24915_merge_slash_command' into 'master'

Support `/merge` slash command for MRs

Closes #24915

See merge request !7746

8 files changed, 450 insertions, 1 deletions

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 2a411d78395..7ea3ea4f376 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -1048,4 +1048,72 @@ describe Projects::MergeRequestsController do
       end
     end
   end
+
+  describe 'GET merge_widget_refresh' do
+    let(:params) do
+      {
+        namespace_id: project.namespace.path,
+        project_id: project.path,
+        id: merge_request.iid,
+        format: :raw
+      }
+    end
+
+    before do
+      project.team << [user, :developer]
+      xhr :get, :merge_widget_refresh, params
+    end
+
+    context 'when merge in progress' do
+      let(:merge_request) { create(:merge_request, source_project: project, in_progress_merge_commit_sha: 'sha') }
+
+      it 'returns an OK response' do
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'sets status to :success' do
+        expect(assigns(:status)).to eq(:success)
+        expect(response).to render_template('merge')
+      end
+    end
+
+    context 'when merge request was merged already' do
+      let(:merge_request) { create(:merge_request, source_project: project, state: :merged) }
+
+      it 'returns an OK response' do
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'sets status to :success' do
+        expect(assigns(:status)).to eq(:success)
+        expect(response).to render_template('merge')
+      end
+    end
+
+    context 'when waiting for build' do
+      let(:merge_request) { create(:merge_request, source_project: project, merge_when_build_succeeds: true, merge_user: user) }
+
+      it 'returns an OK response' do
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'sets status to :merge_when_build_succeeds' do
+        expect(assigns(:status)).to eq(:merge_when_build_succeeds)
+        expect(response).to render_template('merge')
+      end
+    end
+
+    context 'when no special status for MR' do
+      let(:merge_request) { create(:merge_request, source_project: project) }
+
+      it 'returns an OK response' do
+        expect(response).to have_http_status(:ok)
+      end
+
+      it 'sets status to nil' do
+        expect(assigns(:status)).to be_nil
+        expect(response).to render_template('merge')
+      end
+    end
+  end
 end
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 92e38b02615..9f6d4ec6537 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -14,6 +14,54 @@ describe Projects::NotesController do
     }
   end
 
+  describe 'POST create' do
+    let(:merge_request) { create(:merge_request) }
+    let(:request_params) do
+      {
+        note: { note: 'some note', noteable_id: merge_request.id, noteable_type: 'MergeRequest' },
+        namespace_id: project.namespace,
+        project_id: project,
+        merge_request_diff_head_sha: 'sha'
+      }
+    end
+
+    before do
+      sign_in(user)
+      project.team << [user, :developer]
+    end
+
+    it "returns status 302 for html" do
+      post :create, request_params
+
+      expect(response).to have_http_status(302)
+    end
+
+    it "returns status 200 for json" do
+      post :create, request_params.merge(format: :json)
+
+      expect(response).to have_http_status(200)
+    end
+
+    context 'when merge_request_diff_head_sha present' do
+      before do
+        service_params = {
+          note: 'some note',
+          noteable_id: merge_request.id.to_s,
+          noteable_type: 'MergeRequest',
+          merge_request_diff_head_sha: 'sha'
+        }
+
+        expect(Notes::CreateService).to receive(:new).with(project, user, service_params).and_return(double(execute: true))
+      end
+
+      it "returns status 302 for html" do
+        post :create, request_params
+
+        expect(response).to have_http_status(302)
+      end
+    end
+  end
+
   describe 'POST toggle_award_emoji' do
     before do
       sign_in(user)
diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb
index b1b3a47a1ce..b13674b4db9 100644
--- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb
+++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb
@@ -68,6 +68,51 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do
       end
     end
 
+    describe 'merging the MR from the note' do
+      context 'when the current user can merge the MR' do
+        it 'merges the MR' do
+          write_note("/merge")
+
+          expect(page).to have_content 'Commands applied'
+
+          expect(merge_request.reload).to be_merged
+        end
+      end
+
+      context 'when the head diff changes in the meanwhile' do
+        before do
+          merge_request.source_branch = 'another_branch'
+          merge_request.save
+        end
+
+        it 'does not merge the MR' do
+          write_note("/merge")
+
+          expect(page).not_to have_content 'Your commands have been executed!'
+
+          expect(merge_request.reload).not_to be_merged
+        end
+      end
+
+      context 'when the current user cannot merge the MR' do
+        let(:guest) { create(:user) }
+        before do
+          project.team << [guest, :guest]
+          logout
+          login_with(guest)
+          visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+        end
+
+        it 'does not merge the MR' do
+          write_note("/merge")
+
+          expect(page).not_to have_content 'Your commands have been executed!'
+
+          expect(merge_request.reload).not_to be_merged
+        end
+      end
+    end
+
     describe 'adding a due date from note' do
       it 'does not recognize the command nor create a note' do
         write_note('/due 2016-08-28')
diff --git a/spec/helpers/merge_requests_helper_spec.rb b/spec/helpers/merge_requests_helper_spec.rb
index 903224589dd..1f221487393 100644
--- a/spec/helpers/merge_requests_helper_spec.rb
+++ b/spec/helpers/merge_requests_helper_spec.rb
@@ -62,4 +62,19 @@ describe MergeRequestsHelper do
       it { is_expected.to eq([source_title, target_title]) }
     end
   end
+
+  describe 'mr_widget_refresh_url' do
+    let(:merge_request) { create(:merge_request, source_project: project) }
+    let(:project)       { create(:project) }
+
+    it 'returns correct url for MR' do
+      expected_url = "#{project.path_with_namespace}/merge_requests/#{merge_request.iid}/merge_widget_refresh"
+
+      expect(mr_widget_refresh_url(merge_request)).to end_with(expected_url)
+    end
+
+    it 'returns empty string for nil' do
+      expect(mr_widget_refresh_url(nil)).to end_with('')
+    end
+  end
 end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 8d1385016fd..861426acbc3 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1514,6 +1514,108 @@ describe MergeRequest, models: true do
     end
   end
 
+  describe '#mergeable_with_slash_command?' do
+    def create_pipeline(status)
+      create(:ci_pipeline_with_one_job,
+        project: project,
+        ref:     merge_request.source_branch,
+        sha:     merge_request.diff_head_sha,
+        status:  status)
+    end
+
+    let(:project)       { create(:project, :public, only_allow_merge_if_build_succeeds: true) }
+    let(:developer)     { create(:user) }
+    let(:user)          { create(:user) }
+    let(:merge_request) { create(:merge_request, source_project: project) }
+    let(:mr_sha)        { merge_request.diff_head_sha }
+
+    before do
+      project.team << [developer, :developer]
+    end
+
+    context 'when autocomplete_precheck is set to true' do
+      it 'is mergeable by developer' do
+        expect(merge_request.mergeable_with_slash_command?(developer, autocomplete_precheck: true)).to be_truthy
+      end
+
+      it 'is not mergeable by normal user' do
+        expect(merge_request.mergeable_with_slash_command?(user, autocomplete_precheck: true)).to be_falsey
+      end
+    end
+
+    context 'when autocomplete_precheck is set to false' do
+      it 'is mergeable by developer' do
+        expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_truthy
+      end
+
+      it 'is not mergeable by normal user' do
+        expect(merge_request.mergeable_with_slash_command?(user, last_diff_sha: mr_sha)).to be_falsey
+      end
+
+      context 'closed MR'  do
+        before do
+          merge_request.update_attribute(:state, :closed)
+        end
+
+        it 'is not mergeable' do
+          expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_falsey
+        end
+      end
+
+      context 'MR with WIP'  do
+        before do
+          merge_request.update_attribute(:title, 'WIP: some MR')
+        end
+
+        it 'is not mergeable' do
+          expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_falsey
+        end
+      end
+
+      context 'sha differs from the MR diff_head_sha'  do
+        it 'is not mergeable' do
+          expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: 'some other sha')).to be_falsey
+        end
+      end
+
+      context 'sha is not provided'  do
+        it 'is not mergeable' do
+          expect(merge_request.mergeable_with_slash_command?(developer)).to be_falsey
+        end
+      end
+
+      context 'with pipeline ok'  do
+        before do
+          create_pipeline(:success)
+        end
+
+        it 'is mergeable' do
+          expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_truthy
+        end
+      end
+
+      context 'with failing pipeline'  do
+        before do
+          create_pipeline(:failed)
+        end
+
+        it 'is not mergeable' do
+          expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_falsey
+        end
+      end
+
+      context 'with running pipeline'  do
+        before do
+          create_pipeline(:running)
+        end
+
+        it 'is mergeable' do
+          expect(merge_request.mergeable_with_slash_command?(developer, last_diff_sha: mr_sha)).to be_truthy
+        end
+      end
+    end
+  end
+
   describe '#has_commits?' do
     before do
       allow(subject.merge_request_diff).to receive(:commits_count).
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 88c786947d3..7d73c0ea5d0 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -121,6 +121,99 @@ describe MergeRequests::UpdateService, services: true do
       end
     end
 
+    context 'merge' do
+      let(:opts) do
+        {
+          merge: merge_request.diff_head_sha
+        }
+      end
+
+      let(:service) { MergeRequests::UpdateService.new(project, user, opts) }
+
+      context 'without pipeline' do
+        before do
+          merge_request.merge_error = 'Error'
+
+          perform_enqueued_jobs do
+            service.execute(merge_request)
+            @merge_request = MergeRequest.find(merge_request.id)
+          end
+        end
+
+        it { expect(@merge_request).to be_valid }
+        it { expect(@merge_request.state).to eq('merged') }
+        it { expect(@merge_request.merge_error).to be_nil }
+      end
+
+      context 'with finished pipeline' do
+        before do
+          create(:ci_pipeline_with_one_job,
+            project: project,
+            ref:     merge_request.source_branch,
+            sha:     merge_request.diff_head_sha,
+            status:  :success)
+
+          perform_enqueued_jobs do
+            @merge_request = service.execute(merge_request)
+            @merge_request = MergeRequest.find(merge_request.id)
+          end
+        end
+
+        it { expect(@merge_request).to be_valid }
+        it { expect(@merge_request.state).to eq('merged') }
+      end
+
+      context 'with active pipeline' do
+        before do
+          service_mock = double
+          create(:ci_pipeline_with_one_job,
+            project: project,
+            ref:     merge_request.source_branch,
+            sha:     merge_request.diff_head_sha)
+
+          expect(MergeRequests::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user).
+            and_return(service_mock)
+          expect(service_mock).to receive(:execute).with(merge_request)
+        end
+
+        it { service.execute(merge_request) }
+      end
+
+      context 'with a non-authorised user' do
+        let(:visitor) { create(:user) }
+        let(:service) { MergeRequests::UpdateService.new(project, visitor, opts) }
+
+        before do
+          merge_request.update_attribute(:merge_error, 'Error')
+
+          perform_enqueued_jobs do
+            @merge_request = service.execute(merge_request)
+            @merge_request = MergeRequest.find(merge_request.id)
+          end
+        end
+
+        it { expect(@merge_request.state).to eq('opened') }
+        it { expect(@merge_request.merge_error).not_to be_nil }
+      end
+
+      context 'MR can not be merged when note sha != MR sha' do
+        let(:opts) do
+          {
+            merge: 'other_commit'
+          }
+        end
+
+        before do
+          perform_enqueued_jobs do
+            @merge_request = service.execute(merge_request)
+            @merge_request = MergeRequest.find(merge_request.id)
+          end
+        end
+
+        it { expect(@merge_request.state).to eq('opened') }
+      end
+    end
+
     context 'todos' do
       let!(:pending_todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) }
 
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 25804696d2e..b0cc3ce5f5a 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -63,6 +63,17 @@ describe Notes::CreateService, services: true do
           expect(note.note).to eq "HELLO\nWORLD"
         end
       end
+
+      describe '/merge with sha option' do
+        let(:note_text) { %(HELLO\n/merge\nWORLD) }
+        let(:params) { opts.merge(note: note_text, merge_request_diff_head_sha: 'sha') }
+
+        it 'saves the note and exectues merge command' do
+          note = described_class.new(project, user, params).execute
+
+          expect(note.note).to eq "HELLO\nWORLD"
+        end
+      end
     end
   end
 
diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb
index becf627a4f5..ffcf02d2c56 100644
--- a/spec/services/slash_commands/interpret_service_spec.rb
+++ b/spec/services/slash_commands/interpret_service_spec.rb
@@ -1,12 +1,13 @@
 require 'spec_helper'
 
 describe SlashCommands::InterpretService, services: true do
-  let(:project) { create(:empty_project, :public) }
+  let(:project) { create(:project, :public) }
   let(:developer) { create(:user) }
   let(:issue) { create(:issue, project: project) }
   let(:milestone) { create(:milestone, project: project, title: '9.10') }
   let(:inprogress) { create(:label, project: project, title: 'In Progress') }
   let(:bug) { create(:label, project: project, title: 'Bug') }
+  let(:note) { build(:note, commit_id: merge_request.diff_head_sha) }
 
   before do
     project.team << [developer, :developer]
@@ -218,6 +219,14 @@ describe SlashCommands::InterpretService, services: true do
       end
     end
 
+    shared_examples 'merge command' do
+      it 'runs merge command if content contains /merge' do
+        _, updates = service.execute(content, issuable)
+
+        expect(updates).to eq(merge: merge_request.diff_head_sha)
+      end
+    end
+
     it_behaves_like 'reopen command' do
       let(:content) { '/reopen' }
       let(:issuable) { issue }
@@ -238,6 +247,64 @@ describe SlashCommands::InterpretService, services: true do
       let(:issuable) { merge_request }
     end
 
+    context 'merge command' do
+      let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: merge_request.diff_head_sha }) }
+
+      it_behaves_like 'merge command' do
+        let(:content) { '/merge' }
+        let(:issuable) { merge_request }
+      end
+
+      context 'can not be merged when logged user does not have permissions' do
+        let(:service) { described_class.new(project, create(:user)) }
+
+        it_behaves_like 'empty command' do
+          let(:content) { "/merge" }
+          let(:issuable) { merge_request }
+        end
+      end
+
+      context 'can not be merged when sha does not match' do
+        let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: 'othersha' }) }
+
+        it_behaves_like 'empty command' do
+          let(:content) { "/merge" }
+          let(:issuable) { merge_request }
+        end
+      end
+
+      context 'when sha is missing' do
+        let(:service) { described_class.new(project, developer, {}) }
+
+        it 'precheck passes and returns merge command' do
+          _, updates = service.execute('/merge', merge_request)
+
+          expect(updates).to eq(merge: nil)
+        end
+      end
+
+      context 'issue can not be merged' do
+        it_behaves_like 'empty command' do
+          let(:content) { "/merge" }
+          let(:issuable) { issue }
+        end
+      end
+
+      context 'non persisted merge request  cant be merged' do
+        it_behaves_like 'empty command' do
+          let(:content) { "/merge" }
+          let(:issuable) { build(:merge_request) }
+        end
+      end
+
+      context 'not persisted merge request can not be merged' do
+        it_behaves_like 'empty command' do
+          let(:content) { "/merge" }
+          let(:issuable) { build(:merge_request, source_project: project) }
+        end
+      end
+    end
+
     it_behaves_like 'title command' do
       let(:content) { '/title A brand new title' }
       let(:issuable) { issue }