Skip to content
Snippets Groups Projects
Commit 202ed42f authored by David Barr's avatar David Barr
Browse files

Add correct Default.md MR template precedence handling

When a Default.md file exists in the repository and a merge request
description has already been set via a git commit message or via
"closes" or "related to", the Default.md template is now preferred

Changelog: fixed
parent b6a449ec
No related branches found
No related tags found
2 merge requests!87845Add correct Default.md MR template precedence handling,!82398Default.md MR template does not take precedence over Git commit message body on MR creation
......@@ -217,6 +217,7 @@ def target_branch_exists?
# more than one commit in the MR
#
def assign_title_and_description
assign_description_from_repository_template if Feature.enabled?(:mr_default_description_from_repo, target_project)
assign_title_and_description_from_commits
merge_request.title ||= title_from_issue if target_project.issues_enabled? || target_project.external_issue_tracker
merge_request.title ||= source_branch.titleize.humanize
......@@ -280,6 +281,37 @@ def title_from_issue
title_parts.join(' ')
end
def assign_description_from_repository_template
return unless merge_request.description.blank?
# Use TemplateFinder to load the default template. We need this mainly for
# the project_id, in case it differs from the target project. Conveniently,
# since the underlying merge_request_template_names_hash is cached, this
# should also be relatively cheap and allows us to bail early if the project
# does not have a default template.
templates = TemplateFinder.all_template_names(target_project, :merge_requests)
template = templates.values.flatten.find { |tmpl| tmpl[:name].casecmp?('default') }
return unless template
begin
repository_template = TemplateFinder.build(
:merge_requests,
target_project,
{
name: template[:name],
source_template_project_id: template[:project_id]
}
).execute
rescue ::Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError
return
end
return unless repository_template.present?
merge_request.description = repository_template.content
end
def issue_iid
strong_memoize(:issue_iid) do
@params_issue_iid || begin
......
---
name: mr_default_description_from_repo
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82398
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/361753
milestone: '15.0'
type: development
group: group::code review
default_enabled: false
......@@ -69,6 +69,19 @@
end
end
context 'a Default.md file exists in the repository' do
let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } }
let(:project) { create(:project, :repository, :custom_repo, merge_requests_template: template, files: files ) }
before do
stub_feature_flags(mr_default_description_from_repo: true)
end
it 'prefers the project default template' do
expect(merge_request.description).to eq(template)
end
end
context 'when MR is set to close an issue' do
let(:issue) { create(:issue, project: project) }
......
......@@ -79,6 +79,31 @@ def stub_compare
end
end
shared_examples 'with a Default.md template' do
let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } }
let(:project) { create(:project, :custom_repo, files: files ) }
context 'when mr_default_description_from_repo feature flag is enabled' do
before do
stub_feature_flags(mr_default_description_from_repo: project)
end
it 'the template description is preferred' do
expect(merge_request.description).to eq('Default template contents')
end
end
context 'when mr_default_description_from_repo feature flag is disabled' do
before do
stub_feature_flags(mr_default_description_from_repo: false)
end
it 'the template description is not preferred' do
expect(merge_request.description).not_to eq('Default template contents')
end
end
end
describe '#execute' do
it 'calls the compare service with the correct arguments' do
allow_any_instance_of(described_class).to receive(:projects_and_branches_valid?).and_return(true)
......@@ -221,6 +246,7 @@ def stub_compare
end
it_behaves_like 'allows the merge request to be created'
it_behaves_like 'with a Default.md template'
it 'uses the title of the commit as the title of the merge request' do
expect(merge_request.title).to eq(commit_2.safe_message.split("\n").first)
......@@ -241,6 +267,8 @@ def stub_compare
context 'commit has no description' do
let(:commits) { Commit.decorate([commit_3], project) }
it_behaves_like 'with a Default.md template'
it 'uses the title of the commit as the title of the merge request' do
expect(merge_request.title).to eq(commit_3.safe_message)
end
......@@ -279,6 +307,35 @@ def stub_compare
expect(merge_request.description).to eq(expected_description)
end
context 'a Default.md template is defined' do
let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } }
let(:project) { create(:project, :custom_repo, files: files ) }
context 'when mr_default_description_from_repo feature flag is enabled' do
before do
stub_feature_flags(mr_default_description_from_repo: project)
end
it 'appends the closing description to a Default.md template' do
expected_description = ['Default template contents', closing_message].compact.join("\n\n")
expect(merge_request.description).to eq(expected_description)
end
end
context 'when mr_default_description_from_repo feature flag is disabled' do
before do
stub_feature_flags(mr_default_description_from_repo: false)
end
it 'appends the closing description to the commit description' do
expected_description = ['Create the app', closing_message].compact.join("\n\n")
expect(merge_request.description).to eq(expected_description)
end
end
end
end
context 'when the source branch matches an internal issue' do
......@@ -332,6 +389,7 @@ def stub_compare
end
it_behaves_like 'allows the merge request to be created'
it_behaves_like 'with a Default.md template'
it 'uses the title of the branch as the merge request title' do
expect(merge_request.title).to eq('Feature branch')
......@@ -347,6 +405,31 @@ def stub_compare
it 'keeps the description from the initial params' do
expect(merge_request.description).to eq(description)
end
context 'a Default.md template is defined' do
let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } }
let(:project) { create(:project, :custom_repo, files: files ) }
context 'when mr_default_description_from_repo feature flag is enabled' do
before do
stub_feature_flags(mr_default_description_from_repo: project)
end
it 'keeps the description from the initial params' do
expect(merge_request.description).to eq(description)
end
end
context 'when mr_default_description_from_repo feature flag is disabled' do
before do
stub_feature_flags(mr_default_description_from_repo: false)
end
it 'keeps the description from the initial params' do
expect(merge_request.description).to eq(description)
end
end
end
end
context 'when the source branch matches an issue' do
......@@ -377,6 +460,33 @@ def stub_compare
it 'sets the closing description' do
expect(merge_request.description).to eq(closing_message)
end
context 'a Default.md template is defined' do
let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } }
let(:project) { create(:project, :custom_repo, files: files ) }
context 'when mr_default_description_from_repo feature flag is enabled' do
before do
stub_feature_flags(mr_default_description_from_repo: project)
end
it 'appends the closing description to a Default.md template' do
expected_description = ['Default template contents', closing_message].compact.join("\n\n")
expect(merge_request.description).to eq(expected_description)
end
end
context 'when mr_default_description_from_repo feature flag is disabled' do
before do
stub_feature_flags(mr_default_description_from_repo: false)
end
it 'sets the closing description' do
expect(merge_request.description).to eq(closing_message)
end
end
end
end
end
end
......@@ -389,6 +499,7 @@ def stub_compare
end
it_behaves_like 'allows the merge request to be created'
it_behaves_like 'with a Default.md template'
it 'uses the first line of the first multi-line commit message as the title' do
expect(merge_request.title).to eq('Closes #1234 Second commit')
......@@ -426,6 +537,35 @@ def stub_compare
it 'sets the closing description' do
expect(merge_request.description).to eq("Create the app#{closing_message ? "\n\n" + closing_message : ''}")
end
context 'a Default.md template is defined' do
let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } }
let(:project) { create(:project, :custom_repo, files: files ) }
context 'when mr_default_description_from_repo feature flag is enabled' do
before do
stub_feature_flags(mr_default_description_from_repo: project)
end
it 'appends the closing description to a Default.md template' do
expected_description = ['Default template contents', closing_message].compact.join("\n\n")
expect(merge_request.description).to eq(expected_description)
end
end
context 'when mr_default_description_from_repo feature flag is disabled' do
before do
stub_feature_flags(mr_default_description_from_repo: false)
end
it 'appends the closing description to the commit description' do
expected_description = ['Create the app', closing_message].compact.join("\n\n")
expect(merge_request.description).to eq(expected_description)
end
end
end
end
end
......@@ -626,4 +766,52 @@ def stub_compare
end
end
end
describe '#assign_description_from_repository_template' do
subject { service.send(:assign_description_from_repository_template) }
it 'performs no action if the merge request description is not blank' do
merge_request.description = 'foo'
subject
expect(merge_request.description).to eq 'foo'
end
context 'when a Default template is not found' do
it 'does not modify the merge request description' do
merge_request.description = nil
subject
expect(merge_request.description).to be_nil
end
end
context 'when a Default template is found' do
context 'when its contents cannot be retrieved' do
let(:files) { { '.gitlab/merge_request_templates/OtherTemplate.md' => 'Other template contents' } }
let(:project) { create(:project, :custom_repo, files: files ) }
it 'does not modify the merge request description' do
allow(TemplateFinder).to receive(:all_template_names).and_return({
merge_requests: [
{ name: 'Default', id: 'default', key: 'default', project_id: project.id }
]
})
merge_request.description = nil
subject
expect(merge_request.description).to be_nil
end
end
context 'when its contents can be retrieved' do
let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } }
let(:project) { create(:project, :custom_repo, files: files ) }
it 'modifies the merge request description' do
merge_request.description = nil
subject
expect(merge_request.description).to eq 'Default template contents'
end
end
end
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment