+
+
${testTextMuted}
`, }); }); @@ -74,11 +75,11 @@ describe('Commits edit component', () => { expect(headerSlotElement.text()).toBe(testCommitMessage); }); - it('renders checkbox slot correctly', () => { - const checkboxSlotElement = wrapper.find('.test-checkbox'); + it('renders text-muted slot correctly', () => { + const textMutedElement = wrapper.find('.test-text-muted'); - expect(checkboxSlotElement.exists()).toBe(true); - expect(checkboxSlotElement.text()).toBe(testLabel); + expect(textMutedElement.exists()).toBe(true); + expect(textMutedElement.text()).toBe(testTextMuted); }); }); }); diff --git a/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index f0fbb1d5851fdf072aa7ecd419421cc8c0b2b5d5..016b6b2220bdf68823e855a398ea4c92edde107e 100644 --- a/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/frontend/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -269,19 +269,6 @@ describe('ReadyToMerge', () => { }); describe('methods', () => { - describe('updateMergeCommitMessage', () => { - it('should revert flag and change commitMessage', () => { - createComponent(); - - wrapper.vm.updateMergeCommitMessage(true); - - expect(wrapper.vm.commitMessage).toEqual(commitMessageWithDescription); - wrapper.vm.updateMergeCommitMessage(false); - - expect(wrapper.vm.commitMessage).toEqual(commitMessage); - }); - }); - describe('handleMergeButtonClick', () => { const returnPromise = (status) => new Promise((resolve) => { diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index fc2f67f2451749cc479f5fa7403651ca68595428..4f205e861dd4b139a6611164056475a0bae3d225 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -34,7 +34,7 @@ container_repositories container_repositories_count pipeline_analytics squash_read_only sast_ci_configuration cluster_agent cluster_agents agent_configurations - ci_template timelogs + ci_template timelogs merge_commit_template ] expect(described_class).to include_graphql_fields(*expected_fields) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 4b125cab49b14646be8558ce4cb3b6d966da2289..7aa21d7bc0f4a23aa2d8615f898207912f878be5 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -561,6 +561,7 @@ Project: - require_password_to_approve - autoclose_referenced_issues - suggestion_commit_message +- merge_commit_template ProjectTracingSetting: - external_url Author: diff --git a/spec/lib/gitlab/merge_requests/merge_commit_message_spec.rb b/spec/lib/gitlab/merge_requests/merge_commit_message_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5b99aa3cd06d04c9b50880adaaf2aa2c79e743fa --- /dev/null +++ b/spec/lib/gitlab/merge_requests/merge_commit_message_spec.rb @@ -0,0 +1,186 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::MergeRequests::MergeCommitMessage do + let(:merge_commit_template) { nil } + let(:project) { create(:project, :public, :repository, merge_commit_template: merge_commit_template) } + let(:user) { project.creator } + let(:merge_request_description) { "Merge Request Description\nNext line" } + let(:merge_request_title) { 'Bugfix' } + let(:merge_request) do + create( + :merge_request, + :simple, + source_project: project, + target_project: project, + author: user, + description: merge_request_description, + title: merge_request_title + ) + end + + subject { described_class.new(merge_request: merge_request) } + + it 'returns nil when template is not set in target project' do + expect(subject.message).to be_nil + end + + context 'when project has custom merge commit template' do + let(:merge_commit_template) { <<~MSG.rstrip } + %{title} + + See merge request %{reference} + MSG + + it 'uses custom template' do + expect(subject.message).to eq <<~MSG.rstrip + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + + context 'when project has merge commit template with closed issues' do + let(:merge_commit_template) { <<~MSG.rstrip } + Merge branch '%{source_branch}' into '%{target_branch}' + + %{title} + + %{issues} + + See merge request %{reference} + MSG + + it 'omits issues and new lines when no issues are mentioned in description' do + expect(subject.message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + + context 'when MR closes issues' do + let(:issue_1) { create(:issue, project: project) } + let(:issue_2) { create(:issue, project: project) } + let(:merge_request_description) { "Description\n\nclosing #{issue_1.to_reference}, #{issue_2.to_reference}" } + + it 'includes them and keeps new line characters' do + expect(subject.message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + Closes #{issue_1.to_reference} and #{issue_2.to_reference} + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + end + + context 'when project has merge commit template with description' do + let(:merge_commit_template) { <<~MSG.rstrip } + Merge branch '%{source_branch}' into '%{target_branch}' + + %{title} + + %{description} + + See merge request %{reference} + MSG + + it 'uses template' do + expect(subject.message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + Merge Request Description + Next line + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + + context 'when description is empty string' do + let(:merge_request_description) { '' } + + it 'skips description placeholder and removes new line characters before it' do + expect(subject.message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + + context 'when description is nil' do + let(:merge_request_description) { nil } + + it 'skips description placeholder and removes new line characters before it' do + expect(subject.message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + + context 'when description is blank string' do + let(:merge_request_description) { "\n\r \n" } + + it 'skips description placeholder and removes new line characters before it' do + expect(subject.message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + end + + context 'when custom merge commit template contains placeholder in the middle or beginning of the line' do + let(:merge_commit_template) { <<~MSG.rstrip } + Merge branch '%{source_branch}' into '%{target_branch}' + + %{description} %{title} + + See merge request %{reference} + MSG + + it 'uses custom template' do + expect(subject.message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Merge Request Description + Next line Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + + context 'when description is empty string' do + let(:merge_request_description) { '' } + + it 'does not remove new line characters before empty placeholder' do + expect(subject.message).to eq <<~MSG.rstrip + Merge branch 'feature' into 'master' + + Bugfix + + See merge request #{merge_request.to_reference(full: true)} + MSG + end + end + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d871453e062249196fb10227e39816f0c53ecfd5..22d54177f23d12f1d7d1536c2714979b7ccd4678 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1638,6 +1638,22 @@ def set_compare(merge_request) expect(request.default_merge_commit_message) .not_to match("By removing all code\n\n") end + + it 'uses template from target project' do + request = build(:merge_request, title: 'Fix everything') + subject.target_project.merge_commit_template = '%{title}' + + expect(request.default_merge_commit_message) + .to eq('Fix everything') + end + + it 'ignores template when include_description is true' do + request = build(:merge_request, title: 'Fix everything') + subject.target_project.merge_commit_template = '%{title}' + + expect(request.default_merge_commit_message(include_description: true)) + .to match("See merge request #{request.to_reference(full: true)}") + end end describe "#auto_merge_strategy" do diff --git a/spec/views/projects/edit.html.haml_spec.rb b/spec/views/projects/edit.html.haml_spec.rb index b44d07d2ee4537c16f7085fedb176af89866eb76..60f4c1664f7549e976dd8091b49519f053530141 100644 --- a/spec/views/projects/edit.html.haml_spec.rb +++ b/spec/views/projects/edit.html.haml_spec.rb @@ -57,6 +57,41 @@ end end + context 'merge commit template' do + it 'displays all possible variables' do + render + + expect(rendered).to have_content('%{source_branch}') + expect(rendered).to have_content('%{target_branch}') + expect(rendered).to have_content('%{title}') + expect(rendered).to have_content('%{issues}') + expect(rendered).to have_content('%{description}') + expect(rendered).to have_content('%{reference}') + end + + it 'displays a placeholder if none is set' do + render + + expect(rendered).to have_field('project[merge_commit_template]', placeholder: <<~MSG.rstrip) + Merge branch '%{source_branch}' into '%{target_branch}' + + %{title} + + %{issues} + + See merge request %{reference} + MSG + end + + it 'displays the user entered value' do + project.update!(merge_commit_template: '%{title}') + + render + + expect(rendered).to have_field('project[merge_commit_template]', with: '%{title}') + end + end + context 'forking' do before do assign(:project, project)