diff options
author | Phil Hughes <me@iamphill.com> | 2019-03-13 15:44:17 +0000 |
---|---|---|
committer | John T Skarbek <jtslear@gmail.com> | 2019-03-14 09:19:40 -0400 |
commit | e371b4ebc462e9dd24402bd0ee6f0e2e19b026a8 (patch) | |
tree | 24d4328aa5815abb9a1f9d3f3a24a4c0c111c0d8 | |
parent | 17426abf3196d4a298c9f789c07034b70903c08f (diff) | |
download | gitlab-ce-e371b4ebc462e9dd24402bd0ee6f0e2e19b026a8.tar.gz |
Merge branch '48324-enable-squash-message-on-fast-forward' into 'master'
Allow modifying squash commit message for fast-forward only merge method
Closes #48324, #58805, and #58631
See merge request gitlab-org/gitlab-ce!26017
6 files changed, 205 insertions, 48 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/commits_header.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/commits_header.vue index 33963d5e1e6..0312b147b62 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/commits_header.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/commits_header.vue @@ -14,6 +14,10 @@ export default { type: Boolean, required: true, }, + isFastForwardEnabled: { + type: Boolean, + required: true, + }, commitsCount: { type: Number, required: false, @@ -37,16 +41,22 @@ export default { return n__(__('%d commit'), __('%d commits'), this.isSquashEnabled ? 1 : this.commitsCount); }, modifyLinkMessage() { - return this.isSquashEnabled ? __('Modify commit messages') : __('Modify merge commit'); + if (this.isFastForwardEnabled) return __('Modify commit message'); + else if (this.isSquashEnabled) return __('Modify commit messages'); + return __('Modify merge commit'); }, ariaLabel() { return this.expanded ? __('Collapse') : __('Expand'); }, message() { + const message = this.isFastForwardEnabled + ? s__('mrWidgetCommitsAdded|%{commitCount} will be added to %{targetBranch}.') + : s__( + 'mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}.', + ); + return sprintf( - s__( - 'mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}.', - ), + message, { commitCount: `<strong class="commits-count-message">${this.commitsCountMessage}</strong>`, mergeCommitCount: `<strong>${s__('mrWidgetCommitsAdded|1 merge commit')}</strong>`, diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index 9b4e80ee3a3..8e043ed50c9 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -113,6 +113,12 @@ export default { shouldShowMergeControls() { return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText; }, + shouldShowSquashEdit() { + return this.squashBeforeMerge && this.shouldShowSquashBeforeMerge; + }, + shouldShowMergeEdit() { + return !this.mr.ffOnlyEnabled; + }, }, methods: { updateMergeCommitMessage(includeDescription) { @@ -321,43 +327,44 @@ export default { <div v-if="mr.ffOnlyEnabled" class="mr-fast-forward-message"> {{ __('Fast-forward merge without a merge commit') }} </div> - <template v-else> - <commits-header - :is-squash-enabled="squashBeforeMerge" - :commits-count="mr.commitsCount" - :target-branch="mr.targetBranch" - > - <ul class="border-top content-list commits-list flex-list"> - <commit-edit - v-if="squashBeforeMerge && shouldShowSquashBeforeMerge" + <commits-header + v-if="shouldShowSquashEdit || shouldShowMergeEdit" + :is-squash-enabled="squashBeforeMerge" + :commits-count="mr.commitsCount" + :target-branch="mr.targetBranch" + :is-fast-forward-enabled="mr.ffOnlyEnabled" + > + <ul class="border-top content-list commits-list flex-list"> + <commit-edit + v-if="shouldShowSquashEdit" + v-model="squashCommitMessage" + :label="__('Squash commit message')" + input-id="squash-message-edit" + squash + > + <commit-message-dropdown + slot="header" v-model="squashCommitMessage" - :label="__('Squash commit message')" - input-id="squash-message-edit" - squash - > - <commit-message-dropdown - slot="header" - v-model="squashCommitMessage" - :commits="mr.commits" + :commits="mr.commits" + /> + </commit-edit> + <commit-edit + v-if="shouldShowMergeEdit" + v-model="commitMessage" + :label="__('Merge commit message')" + input-id="merge-message-edit" + > + <label slot="checkbox"> + <input + id="include-description" + type="checkbox" + @change="updateMergeCommitMessage($event.target.checked)" /> - </commit-edit> - <commit-edit - v-model="commitMessage" - :label="__('Merge commit message')" - input-id="merge-message-edit" - > - <label slot="checkbox"> - <input - id="include-description" - type="checkbox" - @change="updateMergeCommitMessage($event.target.checked)" - /> - {{ __('Include merge request description') }} - </label> - </commit-edit> - </ul> - </commits-header> - </template> + {{ __('Include merge request description') }} + </label> + </commit-edit> + </ul> + </commits-header> </template> </div> </template> diff --git a/changelogs/unreleased/48324-enable-squash-message-on-fast-forward.yml b/changelogs/unreleased/48324-enable-squash-message-on-fast-forward.yml new file mode 100644 index 00000000000..789ff4f9f89 --- /dev/null +++ b/changelogs/unreleased/48324-enable-squash-message-on-fast-forward.yml @@ -0,0 +1,5 @@ +--- +title: Allow modifying squash commit message for fast-forward only merge method +merge_request: 26017 +author: +type: fixed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1928418dbd5..701c2bdb918 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4878,6 +4878,9 @@ msgstr "" msgid "Modal|Close" msgstr "" +msgid "Modify commit message" +msgstr "" + msgid "Modify commit messages" msgstr "" @@ -9029,6 +9032,9 @@ msgstr "" msgid "mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}." msgstr "" +msgid "mrWidgetCommitsAdded|%{commitCount} will be added to %{targetBranch}." +msgstr "" + msgid "mrWidgetCommitsAdded|1 merge commit" msgstr "" diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_commits_header_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_commits_header_spec.js index 5cf6408cf34..9ee2f88c78d 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_commits_header_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_commits_header_spec.js @@ -15,6 +15,7 @@ describe('Commits header component', () => { isSquashEnabled: false, targetBranch: 'master', commitsCount: 5, + isFastForwardEnabled: false, ...props, }, }); @@ -31,6 +32,27 @@ describe('Commits header component', () => { const findTargetBranchMessage = () => wrapper.find('.label-branch'); const findModifyButton = () => wrapper.find('.modify-message-button'); + describe('when fast-forward is enabled', () => { + beforeEach(() => { + createComponent({ + isFastForwardEnabled: true, + isSquashEnabled: true, + }); + }); + + it('has commits count message showing 1 commit', () => { + expect(findCommitsCountMessage().text()).toBe('1 commit'); + }); + + it('has button with modify commit message', () => { + expect(findModifyButton().text()).toBe('Modify commit message'); + }); + + it('does not have merge commit part of the message', () => { + expect(findHeaderWrapper().text()).not.toContain('1 merge commit'); + }); + }); + describe('when collapsed', () => { it('toggle has aria-label equal to Expand', () => { createComponent(); @@ -78,6 +100,10 @@ describe('Commits header component', () => { expect(findTargetBranchMessage().text()).toBe('master'); }); + + it('does has merge commit part of the message', () => { + expect(findHeaderWrapper().text()).toContain('1 merge commit'); + }); }); describe('when expanded', () => { diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index 08e173b0a10..6ed654250e6 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -18,6 +18,7 @@ const createTestMr = customConfig => { isPipelinePassing: false, isMergeAllowed: true, onlyAllowMergeIfPipelineSucceeds: false, + ffOnlyEnabled: false, hasCI: false, ciStatus: null, sha: '12345678', @@ -624,6 +625,10 @@ describe('ReadyToMerge', () => { const findCommitsHeaderElement = () => wrapper.find(CommitsHeader); const findCommitEditElements = () => wrapper.findAll(CommitEdit); const findCommitDropdownElement = () => wrapper.find(CommitMessageDropdown); + const findFirstCommitEditLabel = () => + findCommitEditElements() + .at(0) + .props('label'); describe('squash checkbox', () => { it('should be rendered when squash before merge is enabled and there is more than 1 commit', () => { @@ -648,31 +653,129 @@ describe('ReadyToMerge', () => { }); describe('commits count collapsible header', () => { - it('should be rendered if fast-forward is disabled', () => { + it('should be rendered when fast-forward is disabled', () => { createLocalComponent(); expect(findCommitsHeaderElement().exists()).toBeTruthy(); }); - it('should not be rendered if fast-forward is enabled', () => { - createLocalComponent({ mr: { ffOnlyEnabled: true } }); + describe('when fast-forward is enabled', () => { + it('should be rendered if squash and squash before are enabled and there is more than 1 commit', () => { + createLocalComponent({ + mr: { + ffOnlyEnabled: true, + enableSquashBeforeMerge: true, + squash: true, + commitsCount: 2, + }, + }); + + expect(findCommitsHeaderElement().exists()).toBeTruthy(); + }); + + it('should not be rendered if squash before merge is disabled', () => { + createLocalComponent({ + mr: { + ffOnlyEnabled: true, + enableSquashBeforeMerge: false, + squash: true, + commitsCount: 2, + }, + }); + + expect(findCommitsHeaderElement().exists()).toBeFalsy(); + }); + + it('should not be rendered if squash is disabled', () => { + createLocalComponent({ + mr: { + ffOnlyEnabled: true, + squash: false, + enableSquashBeforeMerge: true, + commitsCount: 2, + }, + }); + + expect(findCommitsHeaderElement().exists()).toBeFalsy(); + }); + + it('should not be rendered if commits count is 1', () => { + createLocalComponent({ + mr: { + ffOnlyEnabled: true, + squash: true, + enableSquashBeforeMerge: true, + commitsCount: 1, + }, + }); - expect(findCommitsHeaderElement().exists()).toBeFalsy(); + expect(findCommitsHeaderElement().exists()).toBeFalsy(); + }); }); }); describe('commits edit components', () => { + describe('when fast-forward merge is enabled', () => { + it('should not be rendered if squash is disabled', () => { + createLocalComponent({ + mr: { + ffOnlyEnabled: true, + squash: false, + enableSquashBeforeMerge: true, + commitsCount: 2, + }, + }); + + expect(findCommitEditElements().length).toBe(0); + }); + + it('should not be rendered if squash before merge is disabled', () => { + createLocalComponent({ + mr: { + ffOnlyEnabled: true, + squash: true, + enableSquashBeforeMerge: false, + commitsCount: 2, + }, + }); + + expect(findCommitEditElements().length).toBe(0); + }); + + it('should not be rendered if there is only one commit', () => { + createLocalComponent({ + mr: { + ffOnlyEnabled: true, + squash: true, + enableSquashBeforeMerge: true, + commitsCount: 1, + }, + }); + + expect(findCommitEditElements().length).toBe(0); + }); + + it('should have one edit component if squash is enabled and there is more than 1 commit', () => { + createLocalComponent({ + mr: { + ffOnlyEnabled: true, + squash: true, + enableSquashBeforeMerge: true, + commitsCount: 2, + }, + }); + + expect(findCommitEditElements().length).toBe(1); + expect(findFirstCommitEditLabel()).toBe('Squash commit message'); + }); + }); + it('should have one edit component when squash is disabled', () => { createLocalComponent(); expect(findCommitEditElements().length).toBe(1); }); - const findFirstCommitEditLabel = () => - findCommitEditElements() - .at(0) - .props('label'); - it('should have two edit components when squash is enabled and there is more than 1 commit', () => { createLocalComponent({ mr: { |