Address MR comments

app/assets/javascripts/notes.js

@@ -250,7 +250,7 @@
         return;
       }
-      if (note.commands_changes && note.commands_changes.includes('merge')) {
+      if (note.commands_changes && note.commands_changes.indexOf('merge') !== -1) {
         $.get(mrRefreshWidgetUrl);
       }
     };

app/controllers/projects/merge_requests_controller.rb

@@ -348,7 +348,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   end
   def merge_widget_refresh
-    if merge_request.in_progress_merge_commit_sha
+    if merge_request.in_progress_merge_commit_sha || merge_request.state == 'merged'
       @status = :success
     elsif merge_request.merge_when_build_succeeds
       @status = :merge_when_build_succeeds

app/helpers/merge_requests_helper.rb

@@ -19,6 +19,14 @@ module MergeRequestsHelper
     }
   end
+  def mr_widget_refresh_url(mr)
+    if mr && mr.source_project
+      merge_widget_refresh_namespace_project_merge_request_url(mr.source_project.namespace, mr.source_project, mr)
+    else
+      ''
+    end
+  end
+
   def mr_css_classes(mr)
     classes = "merge-request"
     classes << " closed" if mr.closed?

app/services/slash_commands/interpret_service.rb

@@ -61,10 +61,10 @@ module SlashCommands
     desc 'Merge (when build succeeds)'
     condition do
-      last_diff_sha = params.to_h[:merge_request_diff_head_sha]
+      last_diff_sha = params && params[:merge_request_diff_head_sha]
       issuable.is_a?(MergeRequest) &&
-        issuable.mergeable_with_slash_command?(current_user, autocomplete_precheck: !last_diff_sha, last_diff_sha: last_diff_sha) &&
-        issuable.persisted?
+        issuable.persisted? &&
+        issuable.mergeable_with_slash_command?(current_user, autocomplete_precheck: !last_diff_sha, last_diff_sha: last_diff_sha)
     end
     command :merge do
       @updates[:merge] = params[:merge_request_diff_head_sha]

app/views/projects/merge_requests/_show.html.haml

@@ -113,4 +113,5 @@
     action: "#{controller.action_name}"
   });
-  var mrRefreshWidgetUrl = "#{@merge_request && @merge_request.source_project ? merge_widget_refresh_namespace_project_merge_request_url(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request) : ''}";
+  var mrRefreshWidgetUrl = "#{mr_widget_refresh_url(@merge_request)}";
+

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -1071,8 +1071,22 @@ describe Projects::MergeRequestsController do
         expect(response).to have_http_status(:ok)
       end
-      it 'returns :success' do
+      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
@@ -1083,8 +1097,9 @@ describe Projects::MergeRequestsController do
         expect(response).to have_http_status(:ok)
       end
-      it 'returns :merge_when_build_succeeds' do
+      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
@@ -1095,8 +1110,9 @@ describe Projects::MergeRequestsController do
         expect(response).to have_http_status(:ok)
       end
-      it 'returns nil' do
+      it 'sets status to nil' do
         expect(assigns(:status)).to be_nil
+        expect(response).to render_template('merge')
       end
     end
   end

spec/features/merge_requests/user_uses_slash_commands_spec.rb

@@ -73,7 +73,7 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do
         it 'merges the MR' do
           write_note("/merge")
-          expect(page).to have_content 'Your commands have been executed!'
+          expect(page).to have_content 'Commands applied'
           expect(merge_request.reload).to be_merged
         end
@@ -81,20 +81,8 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do
       context 'when the head diff changes in the meanwhile' do
         before do
-          path = File.expand_path("#{project.repository_storage_path}/#{project.namespace.path}/#{project.path}/new_file.txt")
-
-          params = {
-            source_project: merge_request.project,
-            target_project: merge_request.project,
-            target_branch: merge_request.source_branch,
-            source_branch: merge_request.source_branch,
-            file_path: path,
-            file_content: 'some content',
-            commit_message: 'additional commit',
-          }
-
-          Files::UpdateService.new(project, user, params).execute
-          merge_request.reload_diff
+          merge_request.source_branch = 'another_branch'
+          merge_request.save
         end
         it 'does not merge the MR' do

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

spec/models/merge_request_spec.rb

@@ -1578,6 +1578,12 @@ describe MergeRequest, models: true do
         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)

spec/services/merge_requests/update_service_spec.rb

@@ -179,7 +179,7 @@ describe MergeRequests::UpdateService, services: true do
         it { service.execute(merge_request) }
       end
-      context 'MR can not be merged by non authorised user' do
+      context 'with a non-authorised user' do
         let(:visitor) { create(:user) }
         let(:service) { MergeRequests::UpdateService.new(project, visitor, opts) }

spec/services/slash_commands/interpret_service_spec.rb

@@ -283,7 +283,7 @@ describe SlashCommands::InterpretService, services: true do
         end
       end
-      context 'non merge request object cant be merged' do
+      context 'issue can not be merged' do
         it_behaves_like 'empty command' do
           let(:content) { "/merge" }
           let(:issuable) { issue }