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

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>`,

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>

changelogs/unreleased/48324-enable-squash-message-on-fast-forward.yml

+---
+title: Allow modifying squash commit message for fast-forward only merge method
+merge_request: 26017
+author:
+type: fixed

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 ""

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', () => {

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 } });
-        expect(findCommitsHeaderElement().exists()).toBeFalsy();
+      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();
+        });
       });
     });
     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: {