Create default squash commit message using customizable template

Use new squash_commit_template field (when set) to create default value
for squash commit message.

Changelog: added

app/models/merge_request.rb

@@ -1317,7 +1317,7 @@ class MergeRequest < ApplicationRecord
 
   def default_merge_commit_message(include_description: false)
     if self.target_project.merge_commit_template.present? && !include_description
-      return ::Gitlab::MergeRequests::MergeCommitMessage.new(merge_request: self).message
+      return ::Gitlab::MergeRequests::CommitMessageGenerator.new(merge_request: self).merge_message
     end
 
     closes_issues_references = visible_closing_issues_for.map do |issue|
@@ -1340,6 +1340,10 @@ class MergeRequest < ApplicationRecord
   end
 
   def default_squash_commit_message
+    if self.target_project.squash_commit_template.present?
+      return ::Gitlab::MergeRequests::CommitMessageGenerator.new(merge_request: self).squash_message
+    end
+
     title
   end
 

app/views/projects/_merge_request_merge_commit_template.html.haml

@@ -12,6 +12,6 @@
     %p.form-text.text-muted
       = s_('ProjectSettings|Maximum 500 characters.')
       = s_('ProjectSettings|Supported variables:')
-      - Gitlab::MergeRequests::MergeCommitMessage::PLACEHOLDERS.keys.each do |placeholder|
+      - Gitlab::MergeRequests::CommitMessageGenerator::PLACEHOLDERS.keys.each do |placeholder|
         %code
           = "%{#{placeholder}}".html_safe

lib/gitlab/merge_requests/merge_commit_message.rb → lib/gitlab/merge_requests/commit_message_generator.rb

 # frozen_string_literal: true
 module Gitlab
   module MergeRequests
-    class MergeCommitMessage
+    class CommitMessageGenerator
       def initialize(merge_request:)
         @merge_request = merge_request
       end
 
-      def message
+      def merge_message
         return unless @merge_request.target_project.merge_commit_template.present?
 
-        message = @merge_request.target_project.merge_commit_template
-        message = message.delete("\r")
+        replace_placeholders(@merge_request.target_project.merge_commit_template)
+      end
 
-        # Remove placeholders that correspond to empty values and are the last word in the line
-        # along with all whitespace characters preceding them.
-        # This allows us to recreate previous default merge commit message behaviour - we skipped new line character
-        # before empty description and before closed issues when none were present.
-        PLACEHOLDERS.each do |key, value|
-          unless value.call(merge_request).present?
-            message = message.gsub(BLANK_PLACEHOLDERS_REGEXES[key], '')
-          end
-        end
+      def squash_message
+        return unless @merge_request.target_project.squash_commit_template.present?
 
-        Gitlab::StringPlaceholderReplacer
-          .replace_string_placeholders(message, PLACEHOLDERS_REGEX) do |key|
-          PLACEHOLDERS[key].call(merge_request)
-        end
+        replace_placeholders(@merge_request.target_project.squash_commit_template)
       end
 
       private
@@ -55,6 +45,26 @@ module Gitlab
       BLANK_PLACEHOLDERS_REGEXES = (PLACEHOLDERS.map do |key, value|
         [key, Regexp.new("[\n\r]+%{#{Regexp.escape(key)}}$")]
       end).to_h.freeze
+
+      def replace_placeholders(message)
+        # convert CRLF to LF
+        message = message.delete("\r")
+
+        # Remove placeholders that correspond to empty values and are the last word in the line
+        # along with all whitespace characters preceding them.
+        # This allows us to recreate previous default merge commit message behaviour - we skipped new line character
+        # before empty description and before closed issues when none were present.
+        PLACEHOLDERS.each do |key, value|
+          unless value.call(merge_request).present?
+            message = message.gsub(BLANK_PLACEHOLDERS_REGEXES[key], '')
+          end
+        end
+
+        Gitlab::StringPlaceholderReplacer
+          .replace_string_placeholders(message, PLACEHOLDERS_REGEX) do |key|
+          PLACEHOLDERS[key].call(merge_request)
+        end
+      end
     end
   end
 end

spec/features/merge_request/user_customizes_merge_commit_message_spec.rb

@@ -7,19 +7,26 @@ RSpec.describe 'Merge request < User customizes merge commit message', :js do
   let(:user) { project.creator }
   let(:issue_1) { create(:issue, project: project)}
   let(:issue_2) { create(:issue, project: project)}
+  let(:source_branch) { 'csv' }
+  let(:target_branch) { 'master' }
+  let(:squash) { false }
   let(:merge_request) do
     create(
       :merge_request,
-      :simple,
       source_project: project,
-      description: "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}"
+      target_project: project,
+      source_branch: source_branch,
+      target_branch: target_branch,
+      description: "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}",
+      squash: squash
     )
   end
 
-  let(:textbox) { page.find(:css, '#merge-message-edit', visible: false) }
-  let(:default_message) do
+  let(:merge_textbox) { page.find(:css, '#merge-message-edit', visible: false) }
+  let(:squash_textbox) { page.find(:css, '#squash-message-edit', visible: false) }
+  let(:default_merge_commit_message) do
     [
-      "Merge branch 'feature' into 'master'",
+      "Merge branch '#{source_branch}' into '#{target_branch}'",
       merge_request.title,
       "Closes #{issue_1.to_reference} and #{issue_2.to_reference}",
       "See merge request #{merge_request.to_reference(full: true)}"
@@ -35,8 +42,8 @@ RSpec.describe 'Merge request < User customizes merge commit message', :js do
   it 'has commit message without description' do
     expect(page).not_to have_selector('#merge-message-edit')
     first('.js-mr-widget-commits-count').click
-    expect(textbox).to be_visible
-    expect(textbox.value).to eq(default_message)
+    expect(merge_textbox).to be_visible
+    expect(merge_textbox.value).to eq(default_merge_commit_message)
   end
 
   context 'when target project has merge commit template set' do
@@ -45,8 +52,34 @@ RSpec.describe 'Merge request < User customizes merge commit message', :js do
     it 'uses merge commit template' do
       expect(page).not_to have_selector('#merge-message-edit')
       first('.js-mr-widget-commits-count').click
-      expect(textbox).to be_visible
-      expect(textbox.value).to eq(merge_request.title)
+      expect(merge_textbox).to be_visible
+      expect(merge_textbox.value).to eq(merge_request.title)
+    end
+  end
+
+  context 'when squash is performed' do
+    let(:squash) { true }
+
+    it 'has default message with merge request title' do
+      expect(page).not_to have_selector('#squash-message-edit')
+      first('.js-mr-widget-commits-count').click
+      expect(squash_textbox).to be_visible
+      expect(merge_textbox).to be_visible
+      expect(squash_textbox.value).to eq(merge_request.title)
+      expect(merge_textbox.value).to eq(default_merge_commit_message)
+    end
+
+    context 'when target project has squash commit template set' do
+      let(:project) { create(:project, :public, :repository, squash_commit_template: '%{description}') }
+
+      it 'uses squash commit template' do
+        expect(page).not_to have_selector('#squash-message-edit')
+        first('.js-mr-widget-commits-count').click
+        expect(squash_textbox).to be_visible
+        expect(merge_textbox).to be_visible
+        expect(squash_textbox.value).to eq(merge_request.description)
+        expect(merge_textbox.value).to eq(default_merge_commit_message)
+      end
     end
   end
 end

spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
+  let(:merge_commit_template) { nil }
+  let(:squash_commit_template) { nil }
+  let(:project) do
+    create(
+      :project,
+      :public,
+      :repository,
+      merge_commit_template: merge_commit_template,
+      squash_commit_template: squash_commit_template
+    )
+  end
+
+  let(:user) { project.creator }
+  let(:merge_request_description) { "Merge Request Description\nNext line" }
+  let(:merge_request_title) { 'Bugfix' }
+  let(:merge_request) do
+    create(
+      :merge_request,
+      :simple,
+      source_project: project,
+      target_project: project,
+      author: user,
+      description: merge_request_description,
+      title: merge_request_title
+    )
+  end
+
+  subject { described_class.new(merge_request: merge_request) }
+
+  shared_examples_for 'commit message with template' do |message_template_name|
+    it 'returns nil when template is not set in target project' do
+      expect(result_message).to be_nil
+    end
+
+    context 'when project has custom commit template' do
+      let(message_template_name) { <<~MSG.rstrip }
+        %{title}
+
+        See merge request %{reference}
+      MSG
+
+      it 'uses custom template' do
+        expect(result_message).to eq <<~MSG.rstrip
+          Bugfix
+
+          See merge request #{merge_request.to_reference(full: true)}
+        MSG
+      end
+    end
+
+    context 'when project has commit template with closed issues' do
+      let(message_template_name) { <<~MSG.rstrip }
+        Merge branch '%{source_branch}' into '%{target_branch}'
+
+        %{title}
+
+        %{issues}
+
+        See merge request %{reference}
+      MSG
+
+      it 'omits issues and new lines when no issues are mentioned in description' do
+        expect(result_message).to eq <<~MSG.rstrip
+          Merge branch 'feature' into 'master'
+
+          Bugfix
+
+          See merge request #{merge_request.to_reference(full: true)}
+        MSG
+      end
+
+      context 'when MR closes issues' do
+        let(:issue_1) { create(:issue, project: project) }
+        let(:issue_2) { create(:issue, project: project) }
+        let(:merge_request_description) { "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" }
+
+        it 'includes them and keeps new line characters' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Merge branch 'feature' into 'master'
+
+            Bugfix
+
+            Closes #{issue_1.to_reference} and #{issue_2.to_reference}
+
+            See merge request #{merge_request.to_reference(full: true)}
+          MSG
+        end
+      end
+    end
+
+    context 'when project has commit template with description' do
+      let(message_template_name) { <<~MSG.rstrip }
+        Merge branch '%{source_branch}' into '%{target_branch}'
+
+        %{title}
+
+        %{description}
+
+        See merge request %{reference}
+      MSG
+
+      it 'uses template' do
+        expect(result_message).to eq <<~MSG.rstrip
+          Merge branch 'feature' into 'master'
+
+          Bugfix
+
+          Merge Request Description
+          Next line
+
+          See merge request #{merge_request.to_reference(full: true)}
+        MSG
+      end
+
+      context 'when description is empty string' do
+        let(:merge_request_description) { '' }
+
+        it 'skips description placeholder and removes new line characters before it' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Merge branch 'feature' into 'master'
+
+            Bugfix
+
+            See merge request #{merge_request.to_reference(full: true)}
+          MSG
+        end
+      end
+
+      context 'when description is nil' do
+        let(:merge_request_description) { nil }
+
+        it 'skips description placeholder and removes new line characters before it' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Merge branch 'feature' into 'master'
+
+            Bugfix
+
+            See merge request #{merge_request.to_reference(full: true)}
+          MSG
+        end
+      end
+
+      context 'when description is blank string' do
+        let(:merge_request_description) { "\n\r  \n" }
+
+        it 'skips description placeholder and removes new line characters before it' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Merge branch 'feature' into 'master'
+
+            Bugfix
+
+            See merge request #{merge_request.to_reference(full: true)}
+          MSG
+        end
+      end
+    end
+
+    context 'when custom commit template contains placeholder in the middle or beginning of the line' do
+      let(message_template_name) { <<~MSG.rstrip }
+        Merge branch '%{source_branch}' into '%{target_branch}'
+
+        %{description} %{title}
+
+        See merge request %{reference}
+      MSG
+
+      it 'uses custom template' do
+        expect(result_message).to eq <<~MSG.rstrip
+          Merge branch 'feature' into 'master'
+
+          Merge Request Description
+          Next line Bugfix
+
+          See merge request #{merge_request.to_reference(full: true)}
+        MSG
+      end
+
+      context 'when description is empty string' do
+        let(:merge_request_description) { '' }
+
+        it 'does not remove new line characters before empty placeholder' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Merge branch 'feature' into 'master'
+
+             Bugfix
+
+            See merge request #{merge_request.to_reference(full: true)}
+          MSG
+        end
+      end
+    end
+
+    context 'when project has template with CRLF newlines' do
+      let(message_template_name) do
+        "Merge branch '%{source_branch}' into '%{target_branch}'\r\n\r\n%{title}\r\n\r\n%{description}\r\n\r\nSee merge request %{reference}"
+      end
+
+      it 'converts it to LF newlines' do
+        expect(result_message).to eq <<~MSG.rstrip
+          Merge branch 'feature' into 'master'
+
+          Bugfix
+
+          Merge Request Description
+          Next line
+
+          See merge request #{merge_request.to_reference(full: true)}
+        MSG
+      end
+
+      context 'when description is empty string' do
+        let(:merge_request_description) { '' }
+
+        it 'skips description placeholder and removes new line characters before it' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Merge branch 'feature' into 'master'
+
+            Bugfix
+
+            See merge request #{merge_request.to_reference(full: true)}
+          MSG
+        end
+      end
+    end
+  end
+
+  describe '#merge_message' do
+    let(:result_message) { subject.merge_message }
+
+    it_behaves_like 'commit message with template', :merge_commit_template
+  end
+
+  describe '#squash_message' do
+    let(:result_message) { subject.squash_message }
+
+    it_behaves_like 'commit message with template', :squash_commit_template
+  end
+end

spec/lib/gitlab/merge_requests/merge_commit_message_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::MergeRequests::MergeCommitMessage do
-  let(:merge_commit_template) { nil }
-  let(:project) { create(:project, :public, :repository, merge_commit_template: merge_commit_template) }
-  let(:user) { project.creator }
-  let(:merge_request_description) { "Merge Request Description\nNext line" }
-  let(:merge_request_title) { 'Bugfix' }
-  let(:merge_request) do
-    create(
-      :merge_request,
-      :simple,
-      source_project: project,
-      target_project: project,
-      author: user,
-      description: merge_request_description,
-      title: merge_request_title
-    )
-  end
-
-  subject { described_class.new(merge_request: merge_request) }
-
-  it 'returns nil when template is not set in target project' do
-    expect(subject.message).to be_nil
-  end
-
-  context 'when project has custom merge commit template' do
-    let(:merge_commit_template) { <<~MSG.rstrip }
-      %{title}
-
-      See merge request %{reference}
-    MSG
-
-    it 'uses custom template' do
-      expect(subject.message).to eq <<~MSG.rstrip
-        Bugfix
-
-        See merge request #{merge_request.to_reference(full: true)}
-      MSG
-    end
-  end
-
-  context 'when project has merge commit template with closed issues' do
-    let(:merge_commit_template) { <<~MSG.rstrip }
-      Merge branch '%{source_branch}' into '%{target_branch}'
-
-      %{title}
-
-      %{issues}
-
-      See merge request %{reference}
-    MSG
-
-    it 'omits issues and new lines when no issues are mentioned in description' do
-      expect(subject.message).to eq <<~MSG.rstrip
-        Merge branch 'feature' into 'master'
-
-        Bugfix
-
-        See merge request #{merge_request.to_reference(full: true)}
-      MSG
-    end
-
-    context 'when MR closes issues' do
-      let(:issue_1) { create(:issue, project: project) }
-      let(:issue_2) { create(:issue, project: project) }
-      let(:merge_request_description) { "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" }
-
-      it 'includes them and keeps new line characters' do
-        expect(subject.message).to eq <<~MSG.rstrip
-          Merge branch 'feature' into 'master'
-
-          Bugfix
-
-          Closes #{issue_1.to_reference} and #{issue_2.to_reference}
-
-          See merge request #{merge_request.to_reference(full: true)}
-        MSG
-      end
-    end
-  end
-
-  context 'when project has merge commit template with description' do
-    let(:merge_commit_template) { <<~MSG.rstrip }
-      Merge branch '%{source_branch}' into '%{target_branch}'
-
-      %{title}
-
-      %{description}
-
-      See merge request %{reference}
-    MSG
-
-    it 'uses template' do
-      expect(subject.message).to eq <<~MSG.rstrip
-        Merge branch 'feature' into 'master'
-
-        Bugfix
-
-        Merge Request Description
-        Next line
-
-        See merge request #{merge_request.to_reference(full: true)}
-      MSG
-    end
-
-    context 'when description is empty string' do
-      let(:merge_request_description) { '' }
-
-      it 'skips description placeholder and removes new line characters before it' do
-        expect(subject.message).to eq <<~MSG.rstrip
-          Merge branch 'feature' into 'master'
-
-          Bugfix
-
-          See merge request #{merge_request.to_reference(full: true)}
-        MSG
-      end
-    end
-
-    context 'when description is nil' do
-      let(:merge_request_description) { nil }
-
-      it 'skips description placeholder and removes new line characters before it' do
-        expect(subject.message).to eq <<~MSG.rstrip
-          Merge branch 'feature' into 'master'
-
-          Bugfix
-
-          See merge request #{merge_request.to_reference(full: true)}
-        MSG
-      end
-    end
-
-    context 'when description is blank string' do
-      let(:merge_request_description) { "\n\r  \n" }
-
-      it 'skips description placeholder and removes new line characters before it' do
-        expect(subject.message).to eq <<~MSG.rstrip
-          Merge branch 'feature' into 'master'
-
-          Bugfix
-
-          See merge request #{merge_request.to_reference(full: true)}
-        MSG
-      end
-    end
-  end
-
-  context 'when custom merge commit template contains placeholder in the middle or beginning of the line' do
-    let(:merge_commit_template) { <<~MSG.rstrip }
-      Merge branch '%{source_branch}' into '%{target_branch}'
-
-      %{description} %{title}
-
-      See merge request %{reference}
-    MSG
-
-    it 'uses custom template' do
-      expect(subject.message).to eq <<~MSG.rstrip
-        Merge branch 'feature' into 'master'
-
-        Merge Request Description
-        Next line Bugfix
-
-        See merge request #{merge_request.to_reference(full: true)}
-      MSG
-    end
-
-    context 'when description is empty string' do
-      let(:merge_request_description) { '' }
-
-      it 'does not remove new line characters before empty placeholder' do
-        expect(subject.message).to eq <<~MSG.rstrip
-          Merge branch 'feature' into 'master'
-
-           Bugfix
-
-          See merge request #{merge_request.to_reference(full: true)}
-        MSG
-      end
-    end
-  end
-
-  context 'when project has template with CRLF newlines' do
-    let(:merge_commit_template) do
-      "Merge branch '%{source_branch}' into '%{target_branch}'\r\n\r\n%{title}\r\n\r\n%{description}\r\n\r\nSee merge request %{reference}"
-    end
-
-    it 'converts it to LF newlines' do
-      expect(subject.message).to eq <<~MSG.rstrip
-        Merge branch 'feature' into 'master'
-
-        Bugfix
-
-        Merge Request Description
-        Next line
-
-        See merge request #{merge_request.to_reference(full: true)}
-      MSG
-    end
-
-    context 'when description is empty string' do
-      let(:merge_request_description) { '' }
-
-      it 'skips description placeholder and removes new line characters before it' do
-        expect(subject.message).to eq <<~MSG.rstrip
-          Merge branch 'feature' into 'master'
-
-          Bugfix
-
-          See merge request #{merge_request.to_reference(full: true)}
-        MSG
-      end
-    end
-  end
-end

spec/models/merge_request_spec.rb

@@ -178,6 +178,13 @@ RSpec.describe MergeRequest, factory_default: :keep do
     it 'returns the merge request title' do
       expect(subject.default_squash_commit_message).to eq(subject.title)
     end
+
+    it 'uses template from target project' do
+      subject.target_project.squash_commit_template = 'Squashed branch %{source_branch} into %{target_branch}'
+
+      expect(subject.default_squash_commit_message)
+        .to eq('Squashed branch master into feature')
+    end
   end
 
   describe 'modules' do