Merge branch 'merge_template_add_co_authors' into 'master'

Add co_authored_by to merge commit templates

See merge request gitlab-org/gitlab!76042

doc/user/project/merge_requests/commit_templates.md

@@ -68,6 +68,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.7.
+> - [Added](https://gitlab.com/gitlab-org/gitlab/-/issues/20421) `co_authored_by` variable in GitLab 14.7.
 
 Commit message templates support these variables:
 
@@ -84,6 +85,7 @@ Commit message templates support these variables:
 | `%{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: Sidney Jones <sjones@example.com>` <br> `Approved-by: Zhang Wei <zwei@example.com>` |
 | `%{merged_by}`     | User who merged the merge request. | `Alex Garcia <agarcia@example.com>` |
+| `%{co_authored_by}` | Names and emails of commit authors in a `Co-authored-by` Git commit trailer format. Limited to authors of 100 most recent commits in merge request. | `Co-authored-by: Zane Doe <zdoe@example.com>` <br> `Co-authored-by: Blake Smith <bsmith@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

@@ -16,7 +16,7 @@ module Gitlab
       def squash_message
         return unless @merge_request.target_project.squash_commit_template.present?
 
-        replace_placeholders(@merge_request.target_project.squash_commit_template)
+        replace_placeholders(@merge_request.target_project.squash_commit_template, squash: true)
       end
 
       private
@@ -25,10 +25,10 @@ module Gitlab
       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|
@@ -36,24 +36,32 @@ module Gitlab
           end
           "Closes #{closes_issues_references.to_sentence}"
         end,
-        'description' => ->(merge_request, _) { merge_request.description },
-        'reference' => ->(merge_request, _) { merge_request.to_reference(full: true) },
-        'first_commit' => -> (merge_request, _) { merge_request.first_commit&.safe_message&.strip },
-        '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}>" }
+        'description' => ->(merge_request, _, _) { merge_request.description },
+        'reference' => ->(merge_request, _, _) { merge_request.to_reference(full: true) },
+        'first_commit' => -> (merge_request, _, _) { merge_request.first_commit&.safe_message&.strip },
+        '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}>" },
+        'co_authored_by' => ->(merge_request, merged_by, squash) do
+          commit_author = squash ? merge_request.author : merged_by
+          merge_request.recent_commits
+                       .to_h { |commit| [commit.author_email, commit.author_name] }
+                       .except(commit_author&.commit_email_or_default)
+                       .map { |author_email, author_name| "Co-authored-by: #{author_name} <#{author_email}>" }
+                       .join("\n")
+        end
       }.freeze
 
       PLACEHOLDERS_COMBINED_REGEX = /%{(#{Regexp.union(PLACEHOLDERS.keys)})}/.freeze
 
-      def replace_placeholders(message)
+      def replace_placeholders(message, squash: false)
         # Convert CRLF to LF.
         message = message.delete("\r")
 
         used_variables = message.scan(PLACEHOLDERS_COMBINED_REGEX).map { |value| value[0] }.uniq
         values = used_variables.to_h do |variable_name|
-          ["%{#{variable_name}}", PLACEHOLDERS[variable_name].call(merge_request, current_user)]
+          ["%{#{variable_name}}", PLACEHOLDERS[variable_name].call(merge_request, current_user, squash)]
         end
         names_of_empty_variables = values.filter_map { |name, value| name if value.blank? }
 

spec/lib/gitlab/merge_requests/commit_message_generator_spec.rb

@@ -15,9 +15,8 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
     )
   end
 
-  let(:owner) { project.creator }
-  let(:developer) { create(:user, access_level: Gitlab::Access::DEVELOPER) }
-  let(:maintainer) { create(:user, access_level: Gitlab::Access::MAINTAINER) }
+  let(:current_user) { create(:user, name: 'John Doe', email: 'john.doe@example.com') }
+  let(:author) { project.creator }
   let(:source_branch) { 'feature' }
   let(:merge_request_description) { "Merge Request Description\nNext line" }
   let(:merge_request_title) { 'Bugfix' }
@@ -29,13 +28,13 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
       target_project: project,
       target_branch: 'master',
       source_branch: source_branch,
-      author: owner,
+      author: author,
       description: merge_request_description,
       title: merge_request_title
     )
   end
 
-  subject { described_class.new(merge_request: merge_request, current_user: maintainer) }
+  subject { described_class.new(merge_request: merge_request, current_user: current_user) }
 
   shared_examples_for 'commit message with template' do |message_template_name|
     it 'returns nil when template is not set in target project' do
@@ -291,6 +290,8 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
     end
 
     context 'when project has merge commit template with approvers' do
+      let(:user1) { create(:user) }
+      let(:user2) { create(:user) }
       let(message_template_name) do
         "Merge Request approved by:\n%{approved_by}"
       end
@@ -309,27 +310,27 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
 
       context "and mr has one approval" do
         before do
-          merge_request.approved_by_users = [developer]
+          merge_request.approved_by_users = [user1]
         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}>
+          Approved-by: #{user1.name} <#{user1.email}>
           MSG
         end
       end
 
       context "and mr has multiple approvals" do
         before do
-          merge_request.approved_by_users = [developer, maintainer]
+          merge_request.approved_by_users = [user1, user2]
         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}>
+          Approved-by: #{user1.name} <#{user1.email}>
+          Approved-by: #{user2.name} <#{user2.email}>
           MSG
         end
       end
@@ -357,7 +358,7 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
       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}>'
+          Merge Request merged by '#{current_user.name} <#{current_user.email}>'
           MSG
         end
       end
@@ -366,30 +367,32 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
     context 'user' do
       subject { described_class.new(merge_request: merge_request, current_user: nil) }
 
+      let(:user1) { create(:user) }
+      let(:user2) { create(:user) }
       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
+          merge_request.metrics.merged_by = user1
         end
 
         it "returns user name and email" do
           expect(result_message).to eq <<~MSG.rstrip
-          Merge Request merged by '#{developer.name} <#{developer.email}>'
+          Merge Request merged by '#{user1.name} <#{user1.email}>'
           MSG
         end
       end
 
       context 'comes from merge_user' do
         before do
-          merge_request.merge_user = maintainer
+          merge_request.merge_user = user2
         end
 
         it "returns user name and email" do
           expect(result_message).to eq <<~MSG.rstrip
-          Merge Request merged by '#{maintainer.name} <#{maintainer.email}>'
+          Merge Request merged by '#{user2.name} <#{user2.email}>'
           MSG
         end
       end
@@ -423,6 +426,7 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
         url:%{url}
         approved_by:%{approved_by}
         merged_by:%{merged_by}
+        co_authored_by:%{co_authored_by}
       MSG
 
       it 'uses custom template' do
@@ -441,21 +445,123 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
           Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
           url:#{Gitlab::UrlBuilder.build(merge_request)}
           approved_by:
-          merged_by:#{maintainer.name} <#{maintainer.commit_email_or_default}>
+          merged_by:#{current_user.name} <#{current_user.commit_email_or_default}>
+          co_authored_by:Co-authored-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
         MSG
       end
     end
+
+    context 'when project has merge commit template with co_authored_by' do
+      let(:source_branch) { 'signed-commits' }
+      let(message_template_name) { <<~MSG.rstrip }
+        %{title}
+
+        %{co_authored_by}
+      MSG
+
+      it 'uses custom template' do
+        expect(result_message).to eq <<~MSG.rstrip
+          Bugfix
+
+          Co-authored-by: Nannie Bernhard <nannie.bernhard@example.com>
+          Co-authored-by: Winnie Hellmann <winnie@gitlab.com>
+        MSG
+      end
+
+      context 'when author and merging user is one of the commit authors' do
+        let(:author) { create(:user, email: 'nannie.bernhard@example.com') }
+
+        before do
+          merge_request.merge_user = author
+        end
+
+        it 'skips his mail in coauthors' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Bugfix
+
+            Co-authored-by: Winnie Hellmann <winnie@gitlab.com>
+          MSG
+        end
+      end
+
+      context 'when author and merging user is the only author of commits' do
+        let(:author) { create(:user, email: 'dmitriy.zaporozhets@gmail.com') }
+        let(:source_branch) { 'feature' }
+
+        before do
+          merge_request.merge_user = author
+        end
+
+        it 'skips coauthors and empty lines before it' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Bugfix
+          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
+
+    context 'when project has merge commit template with co_authored_by' do
+      let(:source_branch) { 'signed-commits' }
+      let(:merge_commit_template) { <<~MSG.rstrip }
+        %{title}
+
+        %{co_authored_by}
+      MSG
+
+      context 'when author and merging user are one of the commit authors' do
+        let(:author) { create(:user, email: 'nannie.bernhard@example.com') }
+        let(:merge_user) { create(:user, email: 'winnie@gitlab.com') }
+
+        before do
+          merge_request.merge_user = merge_user
+        end
+
+        it 'skips merging user, but does not skip merge request author' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Bugfix
+
+            Co-authored-by: Nannie Bernhard <nannie.bernhard@example.com>
+          MSG
+        end
+      end
+    end
   end
 
   describe '#squash_message' do
     let(:result_message) { subject.squash_message }
 
     it_behaves_like 'commit message with template', :squash_commit_template
+
+    context 'when project has merge commit template with co_authored_by' do
+      let(:source_branch) { 'signed-commits' }
+      let(:squash_commit_template) { <<~MSG.rstrip }
+        %{title}
+
+        %{co_authored_by}
+      MSG
+
+      context 'when author and merging user are one of the commit authors' do
+        let(:author) { create(:user, email: 'nannie.bernhard@example.com') }
+        let(:merge_user) { create(:user, email: 'winnie@gitlab.com') }
+
+        before do
+          merge_request.merge_user = merge_user
+        end
+
+        it 'skips merge request author, but does not skip merging user' do
+          expect(result_message).to eq <<~MSG.rstrip
+            Bugfix
+
+            Co-authored-by: Winnie Hellmann <winnie@gitlab.com>
+          MSG
+        end
+      end
+    end
   end
 end