Commit 49e4c4fc authored by Lucas Zampieri's avatar Lucas Zampieri Committed by Patrick Bajao

Add url, approved_by, and merged_by to merge template

parent 87fe2564
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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)
......
......@@ -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!
......
......@@ -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
......@@ -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.
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
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