Commit 143e577c authored by Piotr Stankowski's avatar Piotr Stankowski

Add co_authored_by to merge commit templates

Changelog: added
parent 23b81f0a
...@@ -68,6 +68,7 @@ GitLab creates a squash commit message with this template: ...@@ -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. > - [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/-/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/-/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: Commit message templates support these variables:
...@@ -84,6 +85,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` | | `%{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>` | | `%{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>` | | `%{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. Empty variables that are the only word in a line are removed, along with all newline characters preceding it.
......
...@@ -16,7 +16,7 @@ module Gitlab ...@@ -16,7 +16,7 @@ module Gitlab
def squash_message def squash_message
return unless @merge_request.target_project.squash_commit_template.present? 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 end
private private
...@@ -25,10 +25,10 @@ module Gitlab ...@@ -25,10 +25,10 @@ module Gitlab
attr_reader :current_user attr_reader :current_user
PLACEHOLDERS = { PLACEHOLDERS = {
'source_branch' => ->(merge_request, _) { merge_request.source_branch.to_s }, 'source_branch' => ->(merge_request, _, _) { merge_request.source_branch.to_s },
'target_branch' => ->(merge_request, _) { merge_request.target_branch.to_s }, 'target_branch' => ->(merge_request, _, _) { merge_request.target_branch.to_s },
'title' => ->(merge_request, _) { merge_request.title }, 'title' => ->(merge_request, _, _) { merge_request.title },
'issues' => ->(merge_request, _) do 'issues' => ->(merge_request, _, _) do
return if merge_request.visible_closing_issues_for.blank? return if merge_request.visible_closing_issues_for.blank?
closes_issues_references = merge_request.visible_closing_issues_for.map do |issue| closes_issues_references = merge_request.visible_closing_issues_for.map do |issue|
...@@ -36,24 +36,32 @@ module Gitlab ...@@ -36,24 +36,32 @@ module Gitlab
end end
"Closes #{closes_issues_references.to_sentence}" "Closes #{closes_issues_references.to_sentence}"
end, end,
'description' => ->(merge_request, _) { merge_request.description }, 'description' => ->(merge_request, _, _) { merge_request.description },
'reference' => ->(merge_request, _) { merge_request.to_reference(full: true) }, 'reference' => ->(merge_request, _, _) { merge_request.to_reference(full: true) },
'first_commit' => -> (merge_request, _) { merge_request.first_commit&.safe_message&.strip }, '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 }, '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) }, '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") }, '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}>" } '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 }.freeze
PLACEHOLDERS_COMBINED_REGEX = /%{(#{Regexp.union(PLACEHOLDERS.keys)})}/.freeze PLACEHOLDERS_COMBINED_REGEX = /%{(#{Regexp.union(PLACEHOLDERS.keys)})}/.freeze
def replace_placeholders(message) def replace_placeholders(message, squash: false)
# Convert CRLF to LF. # Convert CRLF to LF.
message = message.delete("\r") message = message.delete("\r")
used_variables = message.scan(PLACEHOLDERS_COMBINED_REGEX).map { |value| value[0] }.uniq used_variables = message.scan(PLACEHOLDERS_COMBINED_REGEX).map { |value| value[0] }.uniq
values = used_variables.to_h do |variable_name| 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 end
names_of_empty_variables = values.filter_map { |name, value| name if value.blank? } names_of_empty_variables = values.filter_map { |name, value| name if value.blank? }
......
...@@ -15,9 +15,8 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do ...@@ -15,9 +15,8 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
) )
end end
let(:owner) { project.creator } let(:current_user) { create(:user, name: 'John Doe', email: 'john.doe@example.com') }
let(:developer) { create(:user, access_level: Gitlab::Access::DEVELOPER) } let(:author) { project.creator }
let(:maintainer) { create(:user, access_level: Gitlab::Access::MAINTAINER) }
let(:source_branch) { 'feature' } let(:source_branch) { 'feature' }
let(:merge_request_description) { "Merge Request Description\nNext line" } let(:merge_request_description) { "Merge Request Description\nNext line" }
let(:merge_request_title) { 'Bugfix' } let(:merge_request_title) { 'Bugfix' }
...@@ -29,13 +28,13 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do ...@@ -29,13 +28,13 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
target_project: project, target_project: project,
target_branch: 'master', target_branch: 'master',
source_branch: source_branch, source_branch: source_branch,
author: owner, author: author,
description: merge_request_description, description: merge_request_description,
title: merge_request_title title: merge_request_title
) )
end 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| shared_examples_for 'commit message with template' do |message_template_name|
it 'returns nil when template is not set in target project' do it 'returns nil when template is not set in target project' do
...@@ -291,6 +290,8 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do ...@@ -291,6 +290,8 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
end end
context 'when project has merge commit template with approvers' do context 'when project has merge commit template with approvers' do
let(:user1) { create(:user) }
let(:user2) { create(:user) }
let(message_template_name) do let(message_template_name) do
"Merge Request approved by:\n%{approved_by}" "Merge Request approved by:\n%{approved_by}"
end end
...@@ -309,27 +310,27 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do ...@@ -309,27 +310,27 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
context "and mr has one approval" do context "and mr has one approval" do
before do before do
merge_request.approved_by_users = [developer] merge_request.approved_by_users = [user1]
end end
it "returns user name and email" do it "returns user name and email" do
expect(result_message).to eq <<~MSG.rstrip expect(result_message).to eq <<~MSG.rstrip
Merge Request approved by: Merge Request approved by:
Approved-by: #{developer.name} <#{developer.email}> Approved-by: #{user1.name} <#{user1.email}>
MSG MSG
end end
end end
context "and mr has multiple approvals" do context "and mr has multiple approvals" do
before do before do
merge_request.approved_by_users = [developer, maintainer] merge_request.approved_by_users = [user1, user2]
end end
it "returns users names and emails" do it "returns users names and emails" do
expect(result_message).to eq <<~MSG.rstrip expect(result_message).to eq <<~MSG.rstrip
Merge Request approved by: Merge Request approved by:
Approved-by: #{developer.name} <#{developer.email}> Approved-by: #{user1.name} <#{user1.email}>
Approved-by: #{maintainer.name} <#{maintainer.email}> Approved-by: #{user2.name} <#{user2.email}>
MSG MSG
end end
end end
...@@ -357,7 +358,7 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do ...@@ -357,7 +358,7 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
context "and current_user is passed" do context "and current_user is passed" do
it "returns user name and email" do it "returns user name and email" do
expect(result_message).to eq <<~MSG.rstrip 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 MSG
end end
end end
...@@ -366,30 +367,32 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do ...@@ -366,30 +367,32 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
context 'user' do context 'user' do
subject { described_class.new(merge_request: merge_request, current_user: nil) } subject { described_class.new(merge_request: merge_request, current_user: nil) }
let(:user1) { create(:user) }
let(:user2) { create(:user) }
let(message_template_name) do let(message_template_name) do
"Merge Request merged by '%{merged_by}'" "Merge Request merged by '%{merged_by}'"
end end
context 'comes from metrics' do context 'comes from metrics' do
before do before do
merge_request.metrics.merged_by = developer merge_request.metrics.merged_by = user1
end end
it "returns user name and email" do it "returns user name and email" do
expect(result_message).to eq <<~MSG.rstrip expect(result_message).to eq <<~MSG.rstrip
Merge Request merged by '#{developer.name} <#{developer.email}>' Merge Request merged by '#{user1.name} <#{user1.email}>'
MSG MSG
end end
end end
context 'comes from merge_user' do context 'comes from merge_user' do
before do before do
merge_request.merge_user = maintainer merge_request.merge_user = user2
end end
it "returns user name and email" do it "returns user name and email" do
expect(result_message).to eq <<~MSG.rstrip expect(result_message).to eq <<~MSG.rstrip
Merge Request merged by '#{maintainer.name} <#{maintainer.email}>' Merge Request merged by '#{user2.name} <#{user2.email}>'
MSG MSG
end end
end end
...@@ -423,6 +426,7 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do ...@@ -423,6 +426,7 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
url:%{url} url:%{url}
approved_by:%{approved_by} approved_by:%{approved_by}
merged_by:%{merged_by} merged_by:%{merged_by}
co_authored_by:%{co_authored_by}
MSG MSG
it 'uses custom template' do it 'uses custom template' do
...@@ -441,21 +445,123 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do ...@@ -441,21 +445,123 @@ RSpec.describe Gitlab::MergeRequests::CommitMessageGenerator do
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
url:#{Gitlab::UrlBuilder.build(merge_request)} url:#{Gitlab::UrlBuilder.build(merge_request)}
approved_by: 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 MSG
end end
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 end
describe '#merge_message' do describe '#merge_message' do
let(:result_message) { subject.merge_message } let(:result_message) { subject.merge_message }
it_behaves_like 'commit message with template', :merge_commit_template 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 end
describe '#squash_message' do describe '#squash_message' do
let(:result_message) { subject.squash_message } let(:result_message) { subject.squash_message }
it_behaves_like 'commit message with template', :squash_commit_template 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment