Automatic merge of gitlab-org/gitlab-ce master

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

@@ -117,6 +117,12 @@ export default {
     shouldShowMergeControls() {
       return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText;
     },
+    shouldShowSquashEdit() {
+      return this.squashBeforeMerge && this.shouldShowSquashBeforeMerge;
+    },
+    shouldShowMergeEdit() {
+      return !this.mr.ffOnlyEnabled;
+    },
   },
   methods: {
     updateMergeCommitMessage(includeDescription) {
@@ -325,43 +331,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>

app/serializers/pipeline_entity.rb

@@ -28,7 +28,8 @@ class PipelineEntity < Grape::Entity
     expose :can_retry?, as: :retryable
     expose :can_cancel?, as: :cancelable
     expose :failure_reason?, as: :failure_reason
-    expose :detached_merge_request_pipeline?, as: :detached
+    expose :detached_merge_request_pipeline?, as: :detached_merge_request_pipeline
+    expose :merge_request_pipeline?, as: :merge_request_pipeline
   end
 
   expose :details do

app/views/layouts/nav/sidebar/_admin.html.haml

@@ -267,7 +267,7 @@
                   = _('Repository')
             - if template_exists?('admin/application_settings/templates')
               = nav_link(path: 'application_settings#templates') do
-                = link_to templates_admin_application_settings_path, title: _('Templates') do
+                = link_to templates_admin_application_settings_path, title: _('Templates'), class: 'qa-admin-settings-template-item' do
                   %span
                     = _('Templates')
             = nav_link(path: 'application_settings#ci_cd') do

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

changelogs/unreleased/fix-pipeline-entity.yml

+---
+title: Add merge request pipeline flag to pipeline entity
+merge_request: 25846
+author:
+type: added

locale/gitlab.pot

@@ -12042,6 +12042,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 ""
 

qa/qa/page/component/select2.rb

@@ -3,7 +3,7 @@ module QA
     module Component
       module Select2
         def select_item(item_text)
-          find('.select2-result-label', text: item_text).click
+          find('.select2-result-label', text: item_text, match: :prefer_exact).click
         end
 
         def clear_current_selection_if_present

qa/qa/page/project/new.rb

@@ -25,11 +25,14 @@ module QA
         end
 
         def choose_test_namespace
+          choose_namespace(Runtime::Namespace.path)
+        end
+
+        def choose_namespace(namespace)
           retry_on_exception do
             click_body
             click_element :project_namespace_select
-
-            search_and_select(Runtime::Namespace.path)
+            search_and_select(namespace)
           end
         end
 

spec/factories/merge_requests.rb

@@ -101,17 +101,36 @@ FactoryBot.define do
       end
     end
 
-    trait :with_merge_request_pipeline do
+    trait :with_detached_merge_request_pipeline do
       after(:build) do |merge_request|
         merge_request.merge_request_pipelines << build(:ci_pipeline,
           source: :merge_request_event,
           merge_request: merge_request,
           project: merge_request.source_project,
-          ref: merge_request.source_branch,
+          ref: merge_request.ref_path,
           sha: merge_request.source_branch_sha)
       end
     end
 
+    trait :with_merge_request_pipeline do
+      transient do
+        merge_sha { 'test-merge-sha' }
+        source_sha { source_branch_sha }
+        target_sha { target_branch_sha }
+      end
+
+      after(:build) do |merge_request, evaluator|
+        merge_request.merge_request_pipelines << create(:ci_pipeline,
+          source: :merge_request_event,
+          merge_request: merge_request,
+          project: merge_request.source_project,
+          ref: merge_request.merge_ref_path,
+          sha: evaluator.merge_sha,
+          source_sha: evaluator.source_sha,
+          target_sha: evaluator.target_sha)
+      end
+    end
+
     trait :deployed_review_app do
       target_branch 'pages-deploy-target'
 

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 } });
+      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: {

spec/models/ci/build_spec.rb

@@ -2773,7 +2773,7 @@ describe Ci::Build do
     end
 
     context 'when ref is merge request' do
-      let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
+      let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
       let(:pipeline) { merge_request.merge_request_pipelines.first }
       let(:build) { create(:ci_build, ref: merge_request.source_branch, tag: false, pipeline: pipeline, project: project) }
 
@@ -2831,7 +2831,7 @@ describe Ci::Build do
     end
 
     context 'when ref is merge request' do
-      let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
+      let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
       let(:pipeline) { merge_request.merge_request_pipelines.first }
       let(:build) { create(:ci_build, ref: merge_request.source_branch, tag: false, pipeline: pipeline, project: project) }
 

spec/models/concerns/has_ref_spec.rb

@@ -18,7 +18,7 @@ describe HasRef do
       end
 
       context 'when it was triggered by merge request' do
-        let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
+        let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
         let(:pipeline) { merge_request.merge_request_pipelines.first }
         let(:build) { create(:ci_build, pipeline: pipeline) }
 
@@ -67,7 +67,7 @@ describe HasRef do
     end
 
     context 'when it is triggered by a merge request' do
-      let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
+      let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
       let(:pipeline) { merge_request.merge_request_pipelines.first }
       let(:build) { create(:ci_build, tag: false, pipeline: pipeline) }
 

spec/serializers/pipeline_entity_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 describe PipelineEntity do
   include Gitlab::Routing
 
+  set(:project) { create(:project) }
   set(:user) { create(:user) }
   set(:project) { create(:project) }
   let(:request) { double('request') }
@@ -134,12 +135,12 @@ describe PipelineEntity do
     end
 
     context 'when pipeline is detached merge request pipeline' do
-      let(:merge_request) { create(:merge_request, :with_merge_request_pipeline) }
+      let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) }
       let(:project) { merge_request.target_project }
       let(:pipeline) { merge_request.merge_request_pipelines.first }
 
       it 'makes detached flag true' do
-        expect(subject[:flags][:detached]).to be_truthy
+        expect(subject[:flags][:detached_merge_request_pipeline]).to be_truthy
       end
 
       context 'when user is a developer' do
@@ -175,5 +176,19 @@ describe PipelineEntity do
         end
       end
     end
+
+    context 'when pipeline is merge request pipeline' do
+      let(:merge_request) { create(:merge_request, :with_merge_request_pipeline, merge_sha: 'abc') }
+      let(:project) { merge_request.target_project }
+      let(:pipeline) { merge_request.merge_request_pipelines.first }
+
+      it 'makes detached flag false' do
+        expect(subject[:flags][:detached_merge_request_pipeline]).to be_falsy
+      end
+
+      it 'makes atached flag true' do
+        expect(subject[:flags][:merge_request_pipeline]).to be_truthy
+      end
+    end
   end
 end

spec/serializers/pipeline_serializer_spec.rb

@@ -102,7 +102,7 @@ describe PipelineSerializer do
 
       let!(:merge_request_1) do
         create(:merge_request,
-          :with_merge_request_pipeline,
+          :with_detached_merge_request_pipeline,
           target_project: project,
           target_branch: 'master',
           source_project: project,
@@ -111,7 +111,7 @@ describe PipelineSerializer do
 
       let!(:merge_request_2) do
         create(:merge_request,
-          :with_merge_request_pipeline,
+          :with_detached_merge_request_pipeline,
           target_project: project,
           target_branch: 'master',
           source_project: project,