Commit 2e341629 authored by Phil Hughes's avatar Phil Hughes

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
parents d3842c82 2a75f979
...@@ -14,6 +14,10 @@ export default { ...@@ -14,6 +14,10 @@ export default {
type: Boolean, type: Boolean,
required: true, required: true,
}, },
isFastForwardEnabled: {
type: Boolean,
required: true,
},
commitsCount: { commitsCount: {
type: Number, type: Number,
required: false, required: false,
...@@ -37,16 +41,22 @@ export default { ...@@ -37,16 +41,22 @@ export default {
return n__(__('%d commit'), __('%d commits'), this.isSquashEnabled ? 1 : this.commitsCount); return n__(__('%d commit'), __('%d commits'), this.isSquashEnabled ? 1 : this.commitsCount);
}, },
modifyLinkMessage() { 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() { ariaLabel() {
return this.expanded ? __('Collapse') : __('Expand'); return this.expanded ? __('Collapse') : __('Expand');
}, },
message() { 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( return sprintf(
s__( message,
'mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}.',
),
{ {
commitCount: `<strong class="commits-count-message">${this.commitsCountMessage}</strong>`, commitCount: `<strong class="commits-count-message">${this.commitsCountMessage}</strong>`,
mergeCommitCount: `<strong>${s__('mrWidgetCommitsAdded|1 merge commit')}</strong>`, mergeCommitCount: `<strong>${s__('mrWidgetCommitsAdded|1 merge commit')}</strong>`,
......
...@@ -113,6 +113,12 @@ export default { ...@@ -113,6 +113,12 @@ export default {
shouldShowMergeControls() { shouldShowMergeControls() {
return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText; return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText;
}, },
shouldShowSquashEdit() {
return this.squashBeforeMerge && this.shouldShowSquashBeforeMerge;
},
shouldShowMergeEdit() {
return !this.mr.ffOnlyEnabled;
},
}, },
methods: { methods: {
updateMergeCommitMessage(includeDescription) { updateMergeCommitMessage(includeDescription) {
...@@ -321,43 +327,44 @@ export default { ...@@ -321,43 +327,44 @@ export default {
<div v-if="mr.ffOnlyEnabled" class="mr-fast-forward-message"> <div v-if="mr.ffOnlyEnabled" class="mr-fast-forward-message">
{{ __('Fast-forward merge without a merge commit') }} {{ __('Fast-forward merge without a merge commit') }}
</div> </div>
<template v-else> <commits-header
<commits-header v-if="shouldShowSquashEdit || shouldShowMergeEdit"
:is-squash-enabled="squashBeforeMerge" :is-squash-enabled="squashBeforeMerge"
:commits-count="mr.commitsCount" :commits-count="mr.commitsCount"
:target-branch="mr.targetBranch" :target-branch="mr.targetBranch"
> :is-fast-forward-enabled="mr.ffOnlyEnabled"
<ul class="border-top content-list commits-list flex-list"> >
<commit-edit <ul class="border-top content-list commits-list flex-list">
v-if="squashBeforeMerge && shouldShowSquashBeforeMerge" <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" v-model="squashCommitMessage"
:label="__('Squash commit message')" :commits="mr.commits"
input-id="squash-message-edit" />
squash </commit-edit>
> <commit-edit
<commit-message-dropdown v-if="shouldShowMergeEdit"
slot="header" v-model="commitMessage"
v-model="squashCommitMessage" :label="__('Merge commit message')"
:commits="mr.commits" input-id="merge-message-edit"
>
<label slot="checkbox">
<input
id="include-description"
type="checkbox"
@change="updateMergeCommitMessage($event.target.checked)"
/> />
</commit-edit> {{ __('Include merge request description') }}
<commit-edit </label>
v-model="commitMessage" </commit-edit>
:label="__('Merge commit message')" </ul>
input-id="merge-message-edit" </commits-header>
>
<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>
</template> </template>
</div> </div>
</template> </template>
---
title: Allow modifying squash commit message for fast-forward only merge method
merge_request: 26017
author:
type: fixed
...@@ -4920,6 +4920,9 @@ msgstr "" ...@@ -4920,6 +4920,9 @@ msgstr ""
msgid "Modal|Close" msgid "Modal|Close"
msgstr "" msgstr ""
msgid "Modify commit message"
msgstr ""
msgid "Modify commit messages" msgid "Modify commit messages"
msgstr "" msgstr ""
...@@ -9089,6 +9092,9 @@ msgstr "" ...@@ -9089,6 +9092,9 @@ msgstr ""
msgid "mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}." msgid "mrWidgetCommitsAdded|%{commitCount} and %{mergeCommitCount} will be added to %{targetBranch}."
msgstr "" msgstr ""
msgid "mrWidgetCommitsAdded|%{commitCount} will be added to %{targetBranch}."
msgstr ""
msgid "mrWidgetCommitsAdded|1 merge commit" msgid "mrWidgetCommitsAdded|1 merge commit"
msgstr "" msgstr ""
......
...@@ -15,6 +15,7 @@ describe('Commits header component', () => { ...@@ -15,6 +15,7 @@ describe('Commits header component', () => {
isSquashEnabled: false, isSquashEnabled: false,
targetBranch: 'master', targetBranch: 'master',
commitsCount: 5, commitsCount: 5,
isFastForwardEnabled: false,
...props, ...props,
}, },
}); });
...@@ -31,6 +32,27 @@ describe('Commits header component', () => { ...@@ -31,6 +32,27 @@ describe('Commits header component', () => {
const findTargetBranchMessage = () => wrapper.find('.label-branch'); const findTargetBranchMessage = () => wrapper.find('.label-branch');
const findModifyButton = () => wrapper.find('.modify-message-button'); 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', () => { describe('when collapsed', () => {
it('toggle has aria-label equal to Expand', () => { it('toggle has aria-label equal to Expand', () => {
createComponent(); createComponent();
...@@ -78,6 +100,10 @@ describe('Commits header component', () => { ...@@ -78,6 +100,10 @@ describe('Commits header component', () => {
expect(findTargetBranchMessage().text()).toBe('master'); expect(findTargetBranchMessage().text()).toBe('master');
}); });
it('does has merge commit part of the message', () => {
expect(findHeaderWrapper().text()).toContain('1 merge commit');
});
}); });
describe('when expanded', () => { describe('when expanded', () => {
......
...@@ -18,6 +18,7 @@ const createTestMr = customConfig => { ...@@ -18,6 +18,7 @@ const createTestMr = customConfig => {
isPipelinePassing: false, isPipelinePassing: false,
isMergeAllowed: true, isMergeAllowed: true,
onlyAllowMergeIfPipelineSucceeds: false, onlyAllowMergeIfPipelineSucceeds: false,
ffOnlyEnabled: false,
hasCI: false, hasCI: false,
ciStatus: null, ciStatus: null,
sha: '12345678', sha: '12345678',
...@@ -624,6 +625,10 @@ describe('ReadyToMerge', () => { ...@@ -624,6 +625,10 @@ describe('ReadyToMerge', () => {
const findCommitsHeaderElement = () => wrapper.find(CommitsHeader); const findCommitsHeaderElement = () => wrapper.find(CommitsHeader);
const findCommitEditElements = () => wrapper.findAll(CommitEdit); const findCommitEditElements = () => wrapper.findAll(CommitEdit);
const findCommitDropdownElement = () => wrapper.find(CommitMessageDropdown); const findCommitDropdownElement = () => wrapper.find(CommitMessageDropdown);
const findFirstCommitEditLabel = () =>
findCommitEditElements()
.at(0)
.props('label');
describe('squash checkbox', () => { describe('squash checkbox', () => {
it('should be rendered when squash before merge is enabled and there is more than 1 commit', () => { it('should be rendered when squash before merge is enabled and there is more than 1 commit', () => {
...@@ -648,31 +653,129 @@ describe('ReadyToMerge', () => { ...@@ -648,31 +653,129 @@ describe('ReadyToMerge', () => {
}); });
describe('commits count collapsible header', () => { describe('commits count collapsible header', () => {
it('should be rendered if fast-forward is disabled', () => { it('should be rendered when fast-forward is disabled', () => {
createLocalComponent(); createLocalComponent();
expect(findCommitsHeaderElement().exists()).toBeTruthy(); expect(findCommitsHeaderElement().exists()).toBeTruthy();
}); });
it('should not be rendered if fast-forward is enabled', () => { describe('when fast-forward is enabled', () => {
createLocalComponent({ mr: { ffOnlyEnabled: true } }); 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('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', () => { it('should have one edit component when squash is disabled', () => {
createLocalComponent(); createLocalComponent();
expect(findCommitEditElements().length).toBe(1); 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', () => { it('should have two edit components when squash is enabled and there is more than 1 commit', () => {
createLocalComponent({ createLocalComponent({
mr: { mr: {
......
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