Merge branch 'master' into fix/rename-mwbs-to-merge-when-pipeline-succeeds

* master: (312 commits)
  Fix bad selection on dropdown menu for tags filter
  Fixed issue boards scrolling with a lot of lists & issues
  You can only assign default_branch when editing a project ...
  Don't convert data which already is the target type
  Stop supporting Google and Azure as backup strategies
  renames some of the specs and adds changelog entry
  Fixed dragging issue moving wrong issue after multiple drags of issue
  Fixed issue boards issue sorting when dragging issue into list
  Rephrase some system notes to be compatible with new system note style
  Add missing JIRA file that redirects to the new location
  Fix documentation to create the `pg_trm` extension before creating the DB
  Document that we always use `do...end` for `before` in RSpec
  Backport Note#commands_changes from EE
  Log mv_namespace parameters
  Add default_branch attr to Project API payload in docs.
  Fix title case to sentence case
  properly escape username validation error message flash
  Remove header ids from University docs
  Add missing documentation.
  Added test that checks the correct select box is there for the LFS ...
  ...

Conflicts:
	app/services/system_note_service.rb
	spec/features/merge_requests/merge_when_pipeline_succeeds_spec.rb
	spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
	spec/services/system_note_service_spec.rb

7 files changed, 69 insertions, 40 deletions

diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
index dd656c3bbb7..a44312dd363 100644
--- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
+++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb
@@ -1,13 +1,22 @@
 require 'spec_helper'
 
-# Write specs in this file.
 describe MergeRequests::AddTodoWhenBuildFailsService do
   let(:user) { create(:user) }
   let(:merge_request) { create(:merge_request) }
   let(:project) { create(:project) }
   let(:sha) { '1234567890abcdef1234567890abcdef12345678' }
-  let(:pipeline) { create(:ci_pipeline_with_one_job, ref: merge_request.source_branch, project: project, sha: sha) }
-  let(:service) { MergeRequests::AddTodoWhenBuildFailsService.new(project, user, commit_message: 'Awesome message') }
+  let(:ref) { merge_request.source_branch }
+
+  let(:pipeline) do
+    create(:ci_pipeline_with_one_job, ref: ref,
+                                      project: project,
+                                      sha: sha)
+  end
+
+  let(:service) do
+    described_class.new(project, user, commit_message: 'Awesome message')
+  end
+
   let(:todo_service) { TodoService.new }
 
   let(:merge_request) do
@@ -23,7 +32,9 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
 
   describe '#execute' do
     context 'commit status with ref' do
-      let(:commit_status) { create(:generic_commit_status, ref: merge_request.source_branch, pipeline: pipeline) }
+      let(:commit_status) do
+        create(:generic_commit_status, ref: ref, pipeline: pipeline)
+      end
 
       it 'notifies the todo service' do
         expect(todo_service).to receive(:merge_request_build_failed).with(merge_request)
@@ -32,7 +43,7 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
     end
 
     context 'commit status with non-HEAD ref' do
-      let(:commit_status) { create(:generic_commit_status, ref: merge_request.source_branch) }
+      let(:commit_status) { create(:generic_commit_status, ref: ref) }
 
       it 'does not notify the todo service' do
         expect(todo_service).not_to receive(:merge_request_build_failed)
@@ -48,6 +59,18 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
         service.execute(commit_status)
       end
     end
+
+    context 'when commit status is a build allowed to fail' do
+      let(:commit_status) do
+        create(:ci_build, :allowed_to_fail, ref: ref, pipeline: pipeline)
+      end
+
+      it 'does not create todo' do
+        expect(todo_service).not_to receive(:merge_request_build_failed)
+
+        service.execute(commit_status)
+      end
+    end
   end
 
   describe '#close' do
diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb
index 24c25e4350f..5f6a7716beb 100644
--- a/spec/services/merge_requests/close_service_spec.rb
+++ b/spec/services/merge_requests/close_service_spec.rb
@@ -42,7 +42,7 @@ describe MergeRequests::CloseService, services: true do
 
       it 'creates system note about merge_request reassign' do
         note = @merge_request.notes.last
-        expect(note.note).to include 'Status changed to closed'
+        expect(note.note).to include 'closed'
       end
 
       it 'marks todos as done' do
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index 1fd9f5a4910..dff1781d2aa 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -34,7 +34,7 @@ describe MergeRequests::MergeService, services: true do
 
       it 'creates system note about merge_request merge' do
         note = merge_request.notes.last
-        expect(note.note).to include 'Status changed to merged'
+        expect(note.note).to include 'merged'
       end
     end
 
@@ -59,10 +59,14 @@ describe MergeRequests::MergeService, services: true do
         include JiraServiceHelper
 
         let(:jira_tracker) { project.create_jira_service }
+        let(:jira_issue)   { ExternalIssue.new('JIRA-123', project) }
+        let(:commit)       { double('commit', safe_message: "Fixes #{jira_issue.to_reference}") }
 
         before do
           project.update_attributes!(has_external_issue_tracker: true)
           jira_service_settings
+          stub_jira_urls(jira_issue.id)
+          allow(merge_request).to receive(:commits).and_return([commit])
         end
 
         it 'closes issues on JIRA issue tracker' do
@@ -76,6 +80,18 @@ describe MergeRequests::MergeService, services: true do
           service.execute(merge_request)
         end
 
+        context "when jira_issue_transition_id is not present" do
+          before { allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(nil) }
+
+          it "does not close issue" do
+            allow(jira_tracker).to receive_messages(jira_issue_transition_id: nil)
+
+            expect_any_instance_of(JiraService).not_to receive(:transition_issue)
+
+            service.execute(merge_request)
+          end
+        end
+
         context "wrong issue markdown" do
           it 'does not close issues on JIRA issue tracker' do
             jira_issue = ExternalIssue.new('#JIRA-123', project)
diff --git a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
index 3b7c9204a35..b3aa63817c7 100644
--- a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
+++ b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
@@ -40,7 +40,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
 
       it 'creates a system note' do
         note = merge_request.notes.last
-        expect(note.note).to match /Enabled an automatic merge when the pipeline for (\w+\/\w+@)?[0-9a-z]{8}/
+        expect(note.note).to match /enabled an automatic merge when the pipeline for (\w+\/\w+@)?\h{8}/
       end
     end
 
@@ -119,7 +119,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
 
     it 'Posts a system note' do
       note = mr_merge_if_green_enabled.notes.last
-      expect(note.note).to include 'Canceled the automatic merge'
+      expect(note.note).to include 'canceled the automatic merge'
     end
   end
 
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 0220f7e1db2..bc340ff9d3c 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -76,10 +76,10 @@ describe MergeRequests::RefreshService, services: true do
         reload_mrs
       end
 
-      it { expect(@merge_request.notes.last.note).to include('changed to merged') }
+      it { expect(@merge_request.notes.last.note).to include('merged') }
       it { expect(@merge_request).to be_merged }
       it { expect(@fork_merge_request).to be_merged }
-      it { expect(@fork_merge_request.notes.last.note).to include('changed to merged') }
+      it { expect(@fork_merge_request.notes.last.note).to include('merged') }
       it { expect(@build_failed_todo).to be_done }
       it { expect(@fork_build_failed_todo).to be_done }
     end
@@ -95,11 +95,11 @@ describe MergeRequests::RefreshService, services: true do
         reload_mrs
       end
 
-      it { expect(@merge_request.notes.last.note).to include('changed to merged') }
+      it { expect(@merge_request.notes.last.note).to include('merged') }
       it { expect(@merge_request).to be_merged }
       it { expect(@merge_request.diffs.size).to be > 0 }
       it { expect(@fork_merge_request).to be_merged }
-      it { expect(@fork_merge_request.notes.last.note).to include('changed to merged') }
+      it { expect(@fork_merge_request.notes.last.note).to include('merged') }
       it { expect(@build_failed_todo).to be_done }
       it { expect(@fork_build_failed_todo).to be_done }
     end
@@ -119,7 +119,7 @@ describe MergeRequests::RefreshService, services: true do
 
       it { expect(@merge_request.notes).to be_empty }
       it { expect(@merge_request).to be_open }
-      it { expect(@fork_merge_request.notes.last.note).to include('Added 28 commits') }
+      it { expect(@fork_merge_request.notes.last.note).to include('added 28 commits') }
       it { expect(@fork_merge_request).to be_open }
       it { expect(@build_failed_todo).to be_pending }
       it { expect(@fork_build_failed_todo).to be_pending }
@@ -146,7 +146,7 @@ describe MergeRequests::RefreshService, services: true do
         reload_mrs
       end
 
-      it { expect(@merge_request.notes.last.note).to include('changed to merged') }
+      it { expect(@merge_request.notes.last.note).to include('merged') }
       it { expect(@merge_request).to be_merged }
       it { expect(@fork_merge_request).to be_open }
       it { expect(@fork_merge_request.notes).to be_empty }
@@ -169,8 +169,8 @@ describe MergeRequests::RefreshService, services: true do
         expect(@merge_request).to be_open
 
         notes = @fork_merge_request.notes.reorder(:created_at).map(&:note)
-        expect(notes[0]).to include('Restored source branch `master`')
-        expect(notes[1]).to include('Added 28 commits')
+        expect(notes[0]).to include('restored source branch `master`')
+        expect(notes[1]).to include('added 28 commits')
         expect(@fork_merge_request).to be_open
       end
     end
@@ -227,16 +227,6 @@ describe MergeRequests::RefreshService, services: true do
       end
     end
 
-    context 'when the source branch is deleted' do
-      it 'does not create a MergeRequestDiff record' do
-        refresh_service = service.new(@project, @user)
-
-        expect do
-          refresh_service.execute(@oldrev, Gitlab::Git::BLANK_SHA, 'refs/heads/master')
-        end.not_to change { MergeRequestDiff.count }
-      end
-    end
-
     def reload_mrs
       @merge_request.reload
       @fork_merge_request.reload
diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb
index af7424a76a9..a99d4eac9bd 100644
--- a/spec/services/merge_requests/reopen_service_spec.rb
+++ b/spec/services/merge_requests/reopen_service_spec.rb
@@ -41,7 +41,7 @@ describe MergeRequests::ReopenService, services: true do
 
       it 'creates system note about merge_request reopen' do
         note = merge_request.notes.last
-        expect(note.note).to include 'Status changed to reopened'
+        expect(note.note).to include 'reopened'
       end
     end
 
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index cb5d7cdb467..0bd6db1810a 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -79,31 +79,31 @@ describe MergeRequests::UpdateService, services: true do
       end
 
       it 'creates system note about merge_request reassign' do
-        note = find_note('Reassigned to')
+        note = find_note('assigned to')
 
         expect(note).not_to be_nil
-        expect(note.note).to include "Reassigned to \@#{user2.username}"
+        expect(note.note).to include "assigned to #{user2.to_reference}"
       end
 
       it 'creates system note about merge_request label edit' do
-        note = find_note('Added ~')
+        note = find_note('added ~')
 
         expect(note).not_to be_nil
-        expect(note.note).to include "Added ~#{label.id} label"
+        expect(note.note).to include "added #{label.to_reference} label"
       end
 
       it 'creates system note about title change' do
-        note = find_note('Changed title:')
+        note = find_note('changed title')
 
         expect(note).not_to be_nil
-        expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**'
+        expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**'
       end
 
       it 'creates system note about branch change' do
-        note = find_note('Target')
+        note = find_note('changed target')
 
         expect(note).not_to be_nil
-        expect(note.note).to eq 'Target branch changed from `master` to `target`'
+        expect(note.note).to eq 'changed target branch from `master` to `target`'
       end
 
       context 'when not including source branch removal options' do
@@ -258,8 +258,8 @@ describe MergeRequests::UpdateService, services: true do
         before { update_merge_request({ description: "- [x] Task 1\n- [X] Task 2" }) }
 
         it 'creates system note about task status change' do
-          note1 = find_note('Marked the task **Task 1** as completed')
-          note2 = find_note('Marked the task **Task 2** as completed')
+          note1 = find_note('marked the task **Task 1** as completed')
+          note2 = find_note('marked the task **Task 2** as completed')
 
           expect(note1).not_to be_nil
           expect(note2).not_to be_nil
@@ -273,8 +273,8 @@ describe MergeRequests::UpdateService, services: true do
         end
 
         it 'creates system note about task status change' do
-          note1 = find_note('Marked the task **Task 1** as incomplete')
-          note2 = find_note('Marked the task **Task 2** as incomplete')
+          note1 = find_note('marked the task **Task 1** as incomplete')
+          note2 = find_note('marked the task **Task 2** as incomplete')
 
           expect(note1).not_to be_nil
           expect(note2).not_to be_nil