Commit 18819eec authored by trakos's avatar trakos Committed by Piotr Stankowski

Remove include MR description checkbox and deprecate related API field

Now that we added merge commit template function,
we don't need this checkbox. It wouldn't work with template.
This commit also deprecates defaultMergeCommitMessageWithDescription
in graphql. This field ignores mergeCommitTemplate, and doesn't
serve any function internally anymore.
Users are encouraged to use new merge commit template feature instead.

Changelog: deprecated
parent 6b903e48
...@@ -41,7 +41,7 @@ export default { ...@@ -41,7 +41,7 @@ export default {
rows="7" rows="7"
@input="$emit('input', $event.target.value)" @input="$emit('input', $event.target.value)"
></textarea> ></textarea>
<slot name="checkbox"></slot> <slot name="text-muted"></slot>
</div> </div>
</li> </li>
</template> </template>
...@@ -18,9 +18,10 @@ import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests ...@@ -18,9 +18,10 @@ import { refreshUserMergeRequestCounts } from '~/commons/nav/user_merge_requests
import createFlash from '~/flash'; import createFlash from '~/flash';
import { secondsToMilliseconds } from '~/lib/utils/datetime_utility'; import { secondsToMilliseconds } from '~/lib/utils/datetime_utility';
import simplePoll from '~/lib/utils/simple_poll'; import simplePoll from '~/lib/utils/simple_poll';
import { __ } from '~/locale'; import { __, s__ } from '~/locale';
import SmartInterval from '~/smart_interval'; import SmartInterval from '~/smart_interval';
import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { helpPagePath } from '~/helpers/help_page_helper';
import MergeRequest from '../../../merge_request'; import MergeRequest from '../../../merge_request';
import { import {
AUTO_MERGE_STRATEGIES, AUTO_MERGE_STRATEGIES,
...@@ -179,6 +180,11 @@ export default { ...@@ -179,6 +180,11 @@ export default {
return this.mr.canRemoveSourceBranch; return this.mr.canRemoveSourceBranch;
}, },
commitTemplateHelpPage() {
return helpPagePath('user/project/merge_requests/commit_templates.md', {
anchor: 'merge-commit-message-template',
});
},
commits() { commits() {
if (this.glFeatures.mergeRequestWidgetGraphql) { if (this.glFeatures.mergeRequestWidgetGraphql) {
return this.state.commitsWithoutMergeCommits.nodes; return this.state.commitsWithoutMergeCommits.nodes;
...@@ -347,15 +353,6 @@ export default { ...@@ -347,15 +353,6 @@ export default {
updateGraphqlState() { updateGraphqlState() {
return this.$apollo.queries.state.refetch(); return this.$apollo.queries.state.refetch();
}, },
updateMergeCommitMessage(includeDescription) {
const commitMessage = this.glFeatures.mergeRequestWidgetGraphql
? this.state.defaultMergeCommitMessage
: this.mr.commitMessage;
const commitMessageWithDescription = this.glFeatures.mergeRequestWidgetGraphql
? this.state.defaultMergeCommitMessageWithDescription
: this.mr.commitMessageWithDescription;
this.commitMessage = includeDescription ? commitMessageWithDescription : commitMessage;
},
handleMergeButtonClick(useAutoMerge, mergeImmediately = false, confirmationClicked = false) { handleMergeButtonClick(useAutoMerge, mergeImmediately = false, confirmationClicked = false) {
if (this.showFailedPipelineModal && !confirmationClicked) { if (this.showFailedPipelineModal && !confirmationClicked) {
this.isPipelineFailedModalVisible = true; this.isPipelineFailedModalVisible = true;
...@@ -508,6 +505,11 @@ export default { ...@@ -508,6 +505,11 @@ export default {
}); });
}, },
}, },
i18n: {
mergeCommitTemplateHintText: s__(
'mrWidget|To change this default message, edit the template for merge commit messages. %{linkStart}Learn more.%{linkEnd}',
),
},
}; };
</script> </script>
...@@ -679,15 +681,20 @@ export default { ...@@ -679,15 +681,20 @@ export default {
input-id="merge-message-edit" input-id="merge-message-edit"
class="gl-m-0! gl-p-0!" class="gl-m-0! gl-p-0!"
> >
<template #checkbox> <template #text-muted>
<label> <p class="form-text text-muted">
<input <gl-sprintf :message="$options.i18n.mergeCommitTemplateHintText">
id="include-description" <template #link="{ content }">
type="checkbox" <gl-link
@change="updateMergeCommitMessage($event.target.checked)" :href="commitTemplateHelpPage"
/> class="inline-link"
{{ __('Include merge request description') }} target="_blank"
</label> >
{{ content }}
</gl-link>
</template>
</gl-sprintf>
</p>
</template> </template>
</commit-edit> </commit-edit>
</ul> </ul>
...@@ -792,15 +799,16 @@ export default { ...@@ -792,15 +799,16 @@ export default {
:label="__('Merge commit message')" :label="__('Merge commit message')"
input-id="merge-message-edit" input-id="merge-message-edit"
> >
<template #checkbox> <template #text-muted>
<label> <p class="form-text text-muted">
<input <gl-sprintf :message="$options.i18n.mergeCommitTemplateHintText">
id="include-description" <template #link="{ content }">
type="checkbox" <gl-link :href="commitTemplateHelpPage" class="inline-link" target="_blank">
@change="updateMergeCommitMessage($event.target.checked)" {{ content }}
/> </gl-link>
{{ __('Include merge request description') }} </template>
</label> </gl-sprintf>
</p>
</template> </template>
</commit-edit> </commit-edit>
</ul> </ul>
......
...@@ -102,7 +102,8 @@ module Types ...@@ -102,7 +102,8 @@ module Types
field :default_merge_commit_message, GraphQL::Types::String, null: true, field :default_merge_commit_message, GraphQL::Types::String, null: true,
description: 'Default merge commit message of the merge request.' description: 'Default merge commit message of the merge request.'
field :default_merge_commit_message_with_description, GraphQL::Types::String, null: true, field :default_merge_commit_message_with_description, GraphQL::Types::String, null: true,
description: 'Default merge commit message of the merge request with description.' description: 'Default merge commit message of the merge request with description. Will have the same value as `defaultMergeCommitMessage` when project has `mergeCommitTemplate` set.',
deprecated: { reason: 'Define merge commit template in project and use `defaultMergeCommitMessage`', milestone: '14.5' }
field :default_squash_commit_message, GraphQL::Types::String, null: true, calls_gitaly: true, field :default_squash_commit_message, GraphQL::Types::String, null: true, calls_gitaly: true,
description: 'Default squash commit message of the merge request.' description: 'Default squash commit message of the merge request.'
field :merge_ongoing, GraphQL::Types::Boolean, method: :merge_ongoing?, null: false, field :merge_ongoing, GraphQL::Types::Boolean, method: :merge_ongoing?, null: false,
......
...@@ -11513,7 +11513,7 @@ Maven metadata. ...@@ -11513,7 +11513,7 @@ Maven metadata.
| <a id="mergerequestconflicts"></a>`conflicts` | [`Boolean!`](#boolean) | Indicates if the merge request has conflicts. | | <a id="mergerequestconflicts"></a>`conflicts` | [`Boolean!`](#boolean) | Indicates if the merge request has conflicts. |
| <a id="mergerequestcreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of when the merge request was created. | | <a id="mergerequestcreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of when the merge request was created. |
| <a id="mergerequestdefaultmergecommitmessage"></a>`defaultMergeCommitMessage` | [`String`](#string) | Default merge commit message of the merge request. | | <a id="mergerequestdefaultmergecommitmessage"></a>`defaultMergeCommitMessage` | [`String`](#string) | Default merge commit message of the merge request. |
| <a id="mergerequestdefaultmergecommitmessagewithdescription"></a>`defaultMergeCommitMessageWithDescription` | [`String`](#string) | Default merge commit message of the merge request with description. | | <a id="mergerequestdefaultmergecommitmessagewithdescription"></a>`defaultMergeCommitMessageWithDescription` **{warning-solid}** | [`String`](#string) | **Deprecated** in 14.5. Define merge commit template in project and use `defaultMergeCommitMessage`. |
| <a id="mergerequestdefaultsquashcommitmessage"></a>`defaultSquashCommitMessage` | [`String`](#string) | Default squash commit message of the merge request. | | <a id="mergerequestdefaultsquashcommitmessage"></a>`defaultSquashCommitMessage` | [`String`](#string) | Default squash commit message of the merge request. |
| <a id="mergerequestdescription"></a>`description` | [`String`](#string) | Description of the merge request (Markdown rendered as HTML for caching). | | <a id="mergerequestdescription"></a>`description` | [`String`](#string) | Description of the merge request (Markdown rendered as HTML for caching). |
| <a id="mergerequestdescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. | | <a id="mergerequestdescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. |
...@@ -18266,9 +18266,6 @@ msgstr "" ...@@ -18266,9 +18266,6 @@ msgstr ""
msgid "Include description in commit message" msgid "Include description in commit message"
msgstr "" msgstr ""
msgid "Include merge request description"
msgstr ""
msgid "Include new features from all tiers." msgid "Include new features from all tiers."
msgstr "" msgstr ""
...@@ -41388,6 +41385,9 @@ msgstr "" ...@@ -41388,6 +41385,9 @@ msgstr ""
msgid "mrWidget|To approve this merge request, please enter your password. This project requires all approvals to be authenticated." msgid "mrWidget|To approve this merge request, please enter your password. This project requires all approvals to be authenticated."
msgstr "" msgstr ""
msgid "mrWidget|To change this default message, edit the template for merge commit messages. %{linkStart}Learn more.%{linkEnd}"
msgstr ""
msgid "mrWidget|To merge, a Jira issue key must be mentioned in the title or description." msgid "mrWidget|To merge, a Jira issue key must be mentioned in the title or description."
msgstr "" msgstr ""
......
...@@ -26,33 +26,27 @@ RSpec.describe 'Merge request < User customizes merge commit message', :js do ...@@ -26,33 +26,27 @@ RSpec.describe 'Merge request < User customizes merge commit message', :js do
].join("\n\n") ].join("\n\n")
end end
let(:message_with_description) do
[
"Merge branch 'feature' into 'master'",
merge_request.title,
merge_request.description,
"See merge request #{merge_request.to_reference(full: true)}"
].join("\n\n")
end
before do before do
project.add_maintainer(user) project.add_maintainer(user)
sign_in(user) sign_in(user)
visit project_merge_request_path(project, merge_request) visit project_merge_request_path(project, merge_request)
end end
it 'toggles commit message between message with description and without description' do it 'has commit message without description' do
expect(page).not_to have_selector('#merge-message-edit') expect(page).not_to have_selector('#merge-message-edit')
first('.js-mr-widget-commits-count').click first('.js-mr-widget-commits-count').click
expect(textbox).to be_visible expect(textbox).to be_visible
expect(textbox.value).to eq(default_message) expect(textbox.value).to eq(default_message)
end
check('Include merge request description') context 'when target project has merge commit template set' do
let(:project) { create(:project, :public, :repository, merge_commit_template: '%{title}') }
expect(textbox.value).to eq(message_with_description)
uncheck('Include merge request description')
expect(textbox.value).to eq(default_message) it 'uses merge commit template' do
expect(page).not_to have_selector('#merge-message-edit')
first('.js-mr-widget-commits-count').click
expect(textbox).to be_visible
expect(textbox.value).to eq(merge_request.title)
end
end end
end end
...@@ -3,6 +3,7 @@ import CommitEdit from '~/vue_merge_request_widget/components/states/commit_edit ...@@ -3,6 +3,7 @@ import CommitEdit from '~/vue_merge_request_widget/components/states/commit_edit
const testCommitMessage = 'Test commit message'; const testCommitMessage = 'Test commit message';
const testLabel = 'Test label'; const testLabel = 'Test label';
const testTextMuted = 'Test text muted';
const testInputId = 'test-input-id'; const testInputId = 'test-input-id';
describe('Commits edit component', () => { describe('Commits edit component', () => {
...@@ -63,7 +64,7 @@ describe('Commits edit component', () => { ...@@ -63,7 +64,7 @@ describe('Commits edit component', () => {
beforeEach(() => { beforeEach(() => {
createComponent({ createComponent({
header: `<div class="test-header">${testCommitMessage}</div>`, header: `<div class="test-header">${testCommitMessage}</div>`,
checkbox: `<label class="test-checkbox">${testLabel}</label >`, 'text-muted': `<p class="test-text-muted">${testTextMuted}</p>`,
}); });
}); });
...@@ -74,11 +75,11 @@ describe('Commits edit component', () => { ...@@ -74,11 +75,11 @@ describe('Commits edit component', () => {
expect(headerSlotElement.text()).toBe(testCommitMessage); expect(headerSlotElement.text()).toBe(testCommitMessage);
}); });
it('renders checkbox slot correctly', () => { it('renders text-muted slot correctly', () => {
const checkboxSlotElement = wrapper.find('.test-checkbox'); const textMutedElement = wrapper.find('.test-text-muted');
expect(checkboxSlotElement.exists()).toBe(true); expect(textMutedElement.exists()).toBe(true);
expect(checkboxSlotElement.text()).toBe(testLabel); expect(textMutedElement.text()).toBe(testTextMuted);
}); });
}); });
}); });
...@@ -269,19 +269,6 @@ describe('ReadyToMerge', () => { ...@@ -269,19 +269,6 @@ describe('ReadyToMerge', () => {
}); });
describe('methods', () => { 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', () => { describe('handleMergeButtonClick', () => {
const returnPromise = (status) => const returnPromise = (status) =>
new Promise((resolve) => { new Promise((resolve) => {
......
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