Merge branch '14564-mr-automatic-title' into 'master'

Auto-set title for branches created from issues

This sets the title for a new MR to 'Resolves "$issue-title"' when:
- The source branch for the MR begins with a value iid.
- The MR has more than one commit in its diff (if there's one commit, keep using the commit's first line).
- The iid does not point to a confidential issue.

Single commit:

![A single commit uses the commit title](/uploads/cd34f59cd67f095c3034fae07950f8b5/image.png)

Multiple commits:

![Multiple commits use the issue title](/uploads/a322c406ddd56913c5aebd88d16e5a5e/image.png)

Confidential issue:

![A confidential issue uses the branch name](/uploads/7ae9b79de5f6101ced46802f3c3a6e71/image.png)

cc @DouweM @zj 

Closes #14564

See merge request !3966

2 files changed, 211 insertions, 6 deletions

diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 3544752d47a..cd4230aa5e4 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -41,21 +41,45 @@ module MergeRequests
         merge_request.can_be_created = false
       end
 
+      set_title_and_description(merge_request)
+    end
+
+    private
+
+    # When your branch name starts with an iid followed by a dash this pattern will be
+    # interpreted as the user wants to close that issue on this project.
+    #
+    # For example:
+    # - Issue 112 exists, title: Emoji don't show up in commit title
+    # - Source branch is: 112-fix-mep-mep
+    #
+    # Will lead to:
+    # - Appending `Closes #112` to the description
+    # - Setting the title as 'Resolves "Emoji don't show up in commit title"' if there is
+    #   more than one commit in the MR
+    #
+    def set_title_and_description(merge_request)
+      if match = merge_request.source_branch.match(/\A(\d+)-/)
+        iid = match[1]
+      end
+
       commits = merge_request.compare_commits
       if commits && commits.count == 1
         commit = commits.first
         merge_request.title       = commit.title
         merge_request.description ||= commit.description.try(:strip)
+      elsif iid && (issue = merge_request.target_project.get_issue(iid)) && !issue.try(:confidential?)
+        case issue
+        when Issue
+          merge_request.title = "Resolve \"#{issue.title}\""
+        when ExternalIssue
+          merge_request.title = "Resolve #{issue.title}"
+        end
       else
         merge_request.title = merge_request.source_branch.titleize.humanize
       end
 
-      # When your branch name starts with an iid followed by a dash this pattern will
-      # be interpreted as the use wants to close that issue on this project
-      # Pattern example: 112-fix-mep-mep
-      # Will lead to appending `Closes #112` to the description
-      if match = merge_request.source_branch.match(/\A(\d+)-/)
-        iid = match[1]
+      if iid
         closes_issue = "Closes ##{iid}"
 
         if merge_request.description.present?
diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb
new file mode 100644
index 00000000000..782d74ec5ec
--- /dev/null
+++ b/spec/services/merge_requests/build_service_spec.rb
@@ -0,0 +1,181 @@
+require 'spec_helper'
+
+describe MergeRequests::BuildService, services: true do
+  include RepoHelpers
+
+  let(:project) { create(:project) }
+  let(:user) { create(:user) }
+  let(:issue_confidential) { false }
+  let(:issue) { create(:issue, project: project, title: 'A bug', confidential: issue_confidential) }
+  let(:description) { nil }
+  let(:source_branch) { 'feature-branch' }
+  let(:target_branch) { 'master' }
+  let(:merge_request) { service.execute }
+  let(:compare) { double(:compare, commits: commits) }
+  let(:commit_1) { double(:commit_1, safe_message: "Initial commit\n\nCreate the app") }
+  let(:commit_2) { double(:commit_2, safe_message: 'This is a bad commit message!') }
+  let(:commits) { nil }
+
+  let(:service) do
+    MergeRequests::BuildService.new(project, user,
+                                    description: description,
+                                    source_branch: source_branch,
+                                    target_branch: target_branch)
+  end
+
+  before do
+    allow(CompareService).to receive_message_chain(:new, :execute).and_return(compare)
+  end
+
+  describe 'execute' do
+    context 'missing source branch' do
+      let(:source_branch) { '' }
+
+      it 'forbids the merge request from being created' do
+        expect(merge_request.can_be_created).to eq(false)
+      end
+
+      it 'adds an error message to the merge request' do
+        expect(merge_request.errors).to contain_exactly('You must select source and target branch')
+      end
+    end
+
+    context 'missing target branch' do
+      let(:target_branch) { '' }
+
+      it 'forbids the merge request from being created' do
+        expect(merge_request.can_be_created).to eq(false)
+      end
+
+      it 'adds an error message to the merge request' do
+        expect(merge_request.errors).to contain_exactly('You must select source and target branch')
+      end
+    end
+
+    context 'no commits in the diff' do
+      let(:commits) { [] }
+
+      it 'forbids the merge request from being created' do
+        expect(merge_request.can_be_created).to eq(false)
+      end
+    end
+
+    context 'one commit in the diff' do
+      let(:commits) { [commit_1] }
+
+      it 'allows the merge request to be created' do
+        expect(merge_request.can_be_created).to eq(true)
+      end
+
+      it 'uses the title of the commit as the title of the merge request' do
+        expect(merge_request.title).to eq(commit_1.safe_message.split("\n").first)
+      end
+
+      it 'uses the description of the commit as the description of the merge request' do
+        expect(merge_request.description).to eq(commit_1.safe_message.split(/\n+/, 2).last)
+      end
+
+      context 'merge request already has a description set' do
+        let(:description) { 'Merge request description' }
+
+        it 'keeps the description from the initial params' do
+          expect(merge_request.description).to eq(description)
+        end
+      end
+
+      context 'commit has no description' do
+        let(:commits) { [commit_2] }
+
+        it 'uses the title of the commit as the title of the merge request' do
+          expect(merge_request.title).to eq(commit_2.safe_message)
+        end
+
+        it 'sets the description to nil' do
+          expect(merge_request.description).to be_nil
+        end
+      end
+
+      context 'branch starts with issue IID followed by a hyphen' do
+        let(:source_branch) { "#{issue.iid}-fix-issue" }
+
+        it 'appends "Closes #$issue-iid" to the description' do
+          expect(merge_request.description).to eq("#{commit_1.safe_message.split(/\n+/, 2).last}\nCloses ##{issue.iid}")
+        end
+
+        context 'merge request already has a description set' do
+          let(:description) { 'Merge request description' }
+
+          it 'appends "Closes #$issue-iid" to the description' do
+            expect(merge_request.description).to eq("#{description}\nCloses ##{issue.iid}")
+          end
+        end
+
+        context 'commit has no description' do
+          let(:commits) { [commit_2] }
+
+          it 'sets the description to "Closes #$issue-iid"' do
+            expect(merge_request.description).to eq("Closes ##{issue.iid}")
+          end
+        end
+      end
+    end
+
+    context 'more than one commit in the diff' do
+      let(:commits) { [commit_1, commit_2] }
+
+      it 'allows the merge request to be created' do
+        expect(merge_request.can_be_created).to eq(true)
+      end
+
+      it 'uses the title of the branch as the merge request title' do
+        expect(merge_request.title).to eq('Feature branch')
+      end
+
+      it 'does not add a description' do
+        expect(merge_request.description).to be_nil
+      end
+
+      context 'merge request already has a description set' do
+        let(:description) { 'Merge request description' }
+
+        it 'keeps the description from the initial params' do
+          expect(merge_request.description).to eq(description)
+        end
+      end
+
+      context 'branch starts with GitLab issue IID followed by a hyphen' do
+        let(:source_branch) { "#{issue.iid}-fix-issue" }
+
+        it 'sets the title to: Resolves "$issue-title"' do
+          expect(merge_request.title).to eq("Resolve \"#{issue.title}\"")
+        end
+
+        context 'issue does not exist' do
+          let(:source_branch) { "#{issue.iid.succ}-fix-issue" }
+
+          it 'uses the title of the branch as the merge request title' do
+            expect(merge_request.title).to eq("#{issue.iid.succ} fix issue")
+          end
+        end
+
+        context 'issue is confidential' do
+          let(:issue_confidential) { true }
+
+          it 'uses the title of the branch as the merge request title' do
+            expect(merge_request.title).to eq("#{issue.iid} fix issue")
+          end
+        end
+      end
+
+      context 'branch starts with external issue IID followed by a hyphen' do
+        let(:source_branch) { '12345-fix-issue' }
+
+        before { allow(project).to receive(:default_issues_tracker?).and_return(false) }
+
+        it 'sets the title to: Resolves External Issue $issue-iid' do
+          expect(merge_request.title).to eq('Resolve External Issue 12345')
+        end
+      end
+    end
+  end
+end