Merge branch 'new_fields_on_merge_template' into 'master'

Add url, approved_by, and  merged_by to merge template

See merge request gitlab-org/gitlab!75639

app/graphql/types/merge_request_type.rb

@@ -249,10 +249,18 @@ module Types
       !!object.discussion_locked
     end
 
+    def default_merge_commit_message
+      object.default_merge_commit_message(include_description: false, user: current_user)
+    end
+
     def default_merge_commit_message_with_description
       object.default_merge_commit_message(include_description: true)
     end
 
+    def default_squash_commit_message
+      object.default_squash_commit_message(user: current_user)
+    end
+
     def available_auto_merge_strategies
       AutoMergeService.new(object.project, current_user).available_strategies(object)
     end

app/models/merge_request.rb

@@ -1315,9 +1315,9 @@ class MergeRequest < ApplicationRecord
     self.target_project.repository.branch_exists?(self.target_branch)
   end
 
-  def default_merge_commit_message(include_description: false)
+  def default_merge_commit_message(include_description: false, user: nil)
     if self.target_project.merge_commit_template.present? && !include_description
-      return ::Gitlab::MergeRequests::CommitMessageGenerator.new(merge_request: self).merge_message
+      return ::Gitlab::MergeRequests::CommitMessageGenerator.new(merge_request: self, current_user: user).merge_message
     end
 
     closes_issues_references = visible_closing_issues_for.map do |issue|
@@ -1339,9 +1339,9 @@ class MergeRequest < ApplicationRecord
     message.join("\n\n")
   end
 
-  def default_squash_commit_message
+  def default_squash_commit_message(user: nil)
     if self.target_project.squash_commit_template.present?
-      return ::Gitlab::MergeRequests::CommitMessageGenerator.new(merge_request: self).squash_message
+      return ::Gitlab::MergeRequests::CommitMessageGenerator.new(merge_request: self, current_user: user).squash_message
     end
 
     title

app/serializers/merge_request_poll_cached_widget_entity.rb

@@ -17,7 +17,6 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity
   expose :target_project_id
   expose :squash
   expose :rebase_in_progress?, as: :rebase_in_progress
-  expose :default_squash_commit_message
   expose :commits_count
   expose :merge_ongoing?, as: :merge_ongoing
   expose :work_in_progress?, as: :work_in_progress
@@ -27,6 +26,10 @@ class MergeRequestPollCachedWidgetEntity < IssuableEntity
   expose :source_branch_exists?, as: :source_branch_exists
   expose :branch_missing?, as: :branch_missing
 
+  expose :default_squash_commit_message do |merge_request|
+    merge_request.default_squash_commit_message(user: request.current_user)
+  end
+
   expose :commits_without_merge_commits, using: MergeRequestWidgetCommitEntity do |merge_request|
     merge_request.recent_commits.without_merge_commits
   end

app/serializers/merge_request_poll_widget_entity.rb

@@ -19,7 +19,9 @@ class MergeRequestPollWidgetEntity < Grape::Entity
   # User entities
   expose :merge_user, using: UserEntity
 
-  expose :default_merge_commit_message
+  expose :default_merge_commit_message do |merge_request, options|
+    merge_request.default_merge_commit_message(include_description: false, user: current_user)
+  end
 
   expose :mergeable do |merge_request, options|
     next merge_request.mergeable? if Feature.disabled?(:check_mergeability_async_in_widget, merge_request.project, default_enabled: :yaml)

app/services/merge_requests/merge_base_service.rb

@@ -56,7 +56,7 @@ module MergeRequests
 
     def commit_message
       params[:commit_message] ||
-        merge_request.default_merge_commit_message
+        merge_request.default_merge_commit_message(user: current_user)
     end
 
     def squash_sha!

app/services/merge_requests/squash_service.rb

@@ -39,7 +39,7 @@ module MergeRequests
     end
 
     def message
-      params[:squash_commit_message].presence || merge_request.default_squash_commit_message
+      params[:squash_commit_message].presence || merge_request.default_squash_commit_message(user: current_user)
     end
   end
 end

doc/user/project/merge_requests/commit_templates.md

@@ -67,6 +67,7 @@ GitLab creates a squash commit message with this template:
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/20263) in GitLab 14.5.
 > - [Added](https://gitlab.com/gitlab-org/gitlab/-/issues/346805) `first_commit` and `first_multiline_commit` variables in GitLab 14.6.
+> - [Added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75639) `url`, `approved_by`, and `merged_by` variables in GitLab 14.6.
 
 Commit message templates support these variables:
 
@@ -80,6 +81,9 @@ Commit message templates support these variables:
 | `%{reference}`     | Reference to the merge request. | `group-name/project-name!72359` |
 | `%{first_commit}`  | Full message of the first commit in merge request diff. | `Update README.md` |
 | `%{first_multiline_commit}` | Full message of the first commit that's not a merge commit and has more than one line in message body. Merge Request title if all commits aren't multiline. | `Update README.md`<br><br>`Improved project description in readme file.` |
+| `%{url}`           | Full URL to the merge request. | `https://gitlab.com/gitlab-org/gitlab/-/merge_requests/1` |
+| `%{approved_by}`      | Line-separated list of the merge request approvers. This value is not updated until the first page refresh after an approval. | `Approved-by: User A <user@example.com>` <br> `Approved-by: User B <user@gitlab.com>` |
+| `%{merged_by}` | User who merged the merge request. | `Some User <user@example.com>` |
 
 Empty variables that are the only word in a line are removed, along with all newline characters preceding it.  
 

lib/gitlab/merge_requests/commit_message_generator.rb

@@ -2,8 +2,9 @@
 module Gitlab
   module MergeRequests
     class CommitMessageGenerator
-      def initialize(merge_request:)
+      def initialize(merge_request:, current_user:)
         @merge_request = merge_request
+        @current_user = @merge_request.metrics&.merged_by || @merge_request.merge_user || current_user
       end
 
       def merge_message
@@ -21,12 +22,13 @@ module Gitlab
       private
 
       attr_reader :merge_request
+      attr_reader :current_user
 
       PLACEHOLDERS = {
-        'source_branch' => ->(merge_request) { merge_request.source_branch.to_s },
-        'target_branch' => ->(merge_request) { merge_request.target_branch.to_s },
-        'title' => ->(merge_request) { merge_request.title },
-        'issues' => ->(merge_request) do
+        'source_branch' => ->(merge_request, _) { merge_request.source_branch.to_s },
+        'target_branch' => ->(merge_request, _) { merge_request.target_branch.to_s },
+        'title' => ->(merge_request, _) { merge_request.title },
+        'issues' => ->(merge_request, _) do
           return "" if merge_request.visible_closing_issues_for.blank?
 
           closes_issues_references = merge_request.visible_closing_issues_for.map do |issue|
@@ -34,10 +36,13 @@ module Gitlab
           end
           "Closes #{closes_issues_references.to_sentence}"
         end,
-        'description' => ->(merge_request) { merge_request.description.presence || '' },
-        'reference' => ->(merge_request) { merge_request.to_reference(full: true) },
-        'first_commit' => -> (merge_request) { merge_request.first_commit&.safe_message&.strip.presence || '' },
-        'first_multiline_commit' => -> (merge_request) { merge_request.first_multiline_commit&.safe_message&.strip.presence || merge_request.title }
+        'description' => ->(merge_request, _) { merge_request.description.presence || '' },
+        'reference' => ->(merge_request, _) { merge_request.to_reference(full: true) },
+        'first_commit' => -> (merge_request, _) { merge_request.first_commit&.safe_message&.strip.presence || '' },
+        'first_multiline_commit' => -> (merge_request, _) { merge_request.first_multiline_commit&.safe_message&.strip.presence || merge_request.title },
+        'url' => ->(merge_request, _) { Gitlab::UrlBuilder.build(merge_request) },
+        'approved_by' => ->(merge_request, _) { merge_request.approved_by_users.map { |user| "Approved-by: #{user.name} <#{user.commit_email_or_default}>" }.join("\n") },
+        'merged_by' => ->(_, user) { "#{user&.name} <#{user&.commit_email_or_default}>" }
       }.freeze
 
       PLACEHOLDERS_REGEX = Regexp.union(PLACEHOLDERS.keys.map do |key|
@@ -57,14 +62,14 @@ module Gitlab
         # 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?
+          unless value.call(merge_request, current_user).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)
+          PLACEHOLDERS[key].call(merge_request, current_user)
         end
       end
     end

spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb

@@ -15,7 +15,9 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
     )
   end
 
-  let(:user) { project.creator }
+  let(:owner) { project.creator }
+  let(:developer) { create(:user, access_level: Gitlab::Access::DEVELOPER) }
+  let(:maintainer) { create(:user, access_level: Gitlab::Access::MAINTAINER) }
   let(:source_branch) { 'feature' }
   let(:merge_request_description) { "Merge Request Description\nNext line" }
   let(:merge_request_title) { 'Bugfix' }
@@ -27,13 +29,13 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
       target_project: project,
       target_branch: 'master',
       source_branch: source_branch,
-      author: user,
+      author: owner,
       description: merge_request_description,
       title: merge_request_title
     )
   end
 
-  subject { described_class.new(merge_request: merge_request) }
+  subject { described_class.new(merge_request: merge_request, current_user: maintainer) }
 
   shared_examples_for 'commit message with template' do |message_template_name|
     it 'returns nil when template is not set in target project' do
@@ -274,6 +276,111 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
         end
       end
     end
+
+    context 'when project has merge commit template with approvers' do
+      let(message_template_name) do
+        "Merge Request approved by:\n%{approved_by}"
+      end
+
+      context "and mr has no approval" do
+        before do
+          merge_request.approved_by_users = []
+        end
+
+        it "returns empty string" do
+          expect(result_message).to eq <<~MSG.rstrip
+          Merge Request approved by:
+          MSG
+        end
+      end
+
+      context "and mr has one approval" do
+        before do
+          merge_request.approved_by_users = [developer]
+        end
+
+        it "returns user name and email" do
+          expect(result_message).to eq <<~MSG.rstrip
+          Merge Request approved by:
+          Approved-by: #{developer.name} <#{developer.email}>
+          MSG
+        end
+      end
+
+      context "and mr has multiple approvals" do
+        before do
+          merge_request.approved_by_users = [developer, maintainer]
+        end
+
+        it "returns users names and emails" do
+          expect(result_message).to eq <<~MSG.rstrip
+          Merge Request approved by:
+          Approved-by: #{developer.name} <#{developer.email}>
+          Approved-by: #{maintainer.name} <#{maintainer.email}>
+          MSG
+        end
+      end
+    end
+
+    context 'when project has merge commit template with url' do
+      let(message_template_name) do
+        "Merge Request URL is '%{url}'"
+      end
+
+      context "and merge request has url" do
+        it "returns mr url" do
+          expect(result_message).to eq <<~MSG.rstrip
+          Merge Request URL is '#{Gitlab::UrlBuilder.build(merge_request)}'
+          MSG
+        end
+      end
+    end
+
+    context 'when project has merge commit template with merged_by' do
+      let(message_template_name) do
+        "Merge Request merged by '%{merged_by}'"
+      end
+
+      context "and current_user is passed" do
+        it "returns user name and email" do
+          expect(result_message).to eq <<~MSG.rstrip
+          Merge Request merged by '#{maintainer.name} <#{maintainer.email}>'
+          MSG
+        end
+      end
+    end
+
+    context 'user' do
+      subject { described_class.new(merge_request: merge_request, current_user: nil) }
+
+      let(message_template_name) do
+        "Merge Request merged by '%{merged_by}'"
+      end
+
+      context 'comes from metrics' do
+        before do
+          merge_request.metrics.merged_by = developer
+        end
+
+        it "returns user name and email" do
+          expect(result_message).to eq <<~MSG.rstrip
+          Merge Request merged by '#{developer.name} <#{developer.email}>'
+          MSG
+        end
+      end
+
+      context 'comes from merge_user' do
+        before do
+          merge_request.merge_user = maintainer
+        end
+
+        it "returns user name and email" do
+          expect(result_message).to eq <<~MSG.rstrip
+          Merge Request merged by '#{maintainer.name} <#{maintainer.email}>'
+          MSG
+        end
+      end
+    end
   end
 
   describe '#merge_message' do

spec/models/merge_request_spec.rb

@@ -1647,13 +1647,13 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
 
     it 'uses template from target project' do
-      request = build(:merge_request, title: 'Fix everything')
-      request.compare_commits = [
+      subject.title = 'Fix everything'
+      subject.compare_commits = [
         double(safe_message: 'Commit message', gitaly_commit?: true, merge_commit?: false, description?: false)
       ]
       subject.target_project.merge_commit_template = '%{title}'
 
-      expect(request.default_merge_commit_message)
+      expect(subject.default_merge_commit_message)
         .to eq('Fix everything')
     end
 

spec/views/projects/edit.html.haml_spec.rb

@@ -67,6 +67,9 @@ RSpec.describe 'projects/edit' do
       expect(rendered).to have_content('%{issues}')
       expect(rendered).to have_content('%{description}')
       expect(rendered).to have_content('%{reference}')
+      expect(rendered).to have_content('%{approved_by}')
+      expect(rendered).to have_content('%{url}')
+      expect(rendered).to have_content('%{merged_by}')
     end
 
     it 'displays a placeholder if none is set' do