Merge branch '244831-split-pipelines-for-merged-results-and-merge-train-check-boxes' into 'master'

Split  "Pipelines for merged results" and "Merge Train" check boxes

See merge request gitlab-org/gitlab!46357

app/views/projects/_merge_request_merge_options_settings.html.haml

@@ -4,6 +4,7 @@
   %b= s_('ProjectSettings|Merge options')
   %p.text-secondary= s_('ProjectSettings|Additional merge request capabilities that influence how and when merges will be performed')
   = render_if_exists 'projects/merge_pipelines_settings', form: form
+  = render_if_exists 'projects/merge_trains_settings', form: form
   .form-check.mb-2
     = form.check_box :resolve_outdated_diff_discussions, class: 'form-check-input'
     = form.label :resolve_outdated_diff_discussions, class: 'form-check-label' do

db/migrate/20201112195322_reseed_merge_trains_enabled.rb

+# frozen_string_literal: true
+
+class ReseedMergeTrainsEnabled < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    update_column_in_batches(:project_ci_cd_settings, :merge_trains_enabled, true) do |table, query|
+      query.where(table[:merge_pipelines_enabled].eq(true))
+    end
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema_migrations/20201112195322

+644bf793b3b62330bfb1f4dc406e6dd01cbcf4651ebfd2873fb551c757c8a3b4
\ No newline at end of file

doc/ci/merge_request_pipelines/pipelines_for_merged_results/index.md

@@ -56,7 +56,7 @@ To enable pipelines for merged results for your project:
 1. [Configure your CI/CD configuration file](../index.md#configuring-pipelines-for-merge-requests)
    so that the pipeline or individual jobs run for merge requests.
 1. Visit your project's **Settings > General** and expand **Merge requests**.
-1. Check **Enable merge trains and pipelines for merged results**.
+1. Check **Enable merged results pipelines.**.
 1. Click **Save changes**.
 
 CAUTION: **Caution:**

doc/ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md

@@ -82,9 +82,13 @@ To enable merge trains for your project:
 1. If you are on a self-managed GitLab instance, ensure the [feature flag](#merge-trains-feature-flag) is set correctly.
 1. [Configure your CI/CD configuration file](../../index.md#configuring-pipelines-for-merge-requests)
    so that the pipeline or individual jobs run for merge requests.
-1. Visit your project's **Settings > General** and expand **Merge requests**.
-1. Check **Enable merge trains and pipelines for merged results**.
-1. Click **Save changes**.
+1. Visit your project's **Settings > General** and expand **Merge requests**
+1. Check **Enable merged results pipelines.** (if not enabled)
+1. Check **Enable merge trains.**
+1. Click **Save changes**
+
+In GitLab 13.5 and earlier, there is only one checkbox, named
+**Enable merge trains and pipelines for merged results**.
 
 CAUTION: **Caution:**
 If you select the check box but don't configure your CI/CD to use
@@ -200,10 +204,13 @@ for more information.
 
 ### Merge Trains feature flag **(PREMIUM ONLY)**
 
-Merge trains are automatically enabled when [pipelines for merged results](../index.md#pipelines-for-merged-results)
-are enabled. To use pipelines for merged results without using merge trains, you must
-enable a [feature flag](../../../../user/feature_flags.md) that blocks the merge trains
-feature.
+In [GitLab 13.6 and later](https://gitlab.com/gitlab-org/gitlab/-/issues/244831),
+you can [enable or disable merge trains in the project settings](#enable-merge-trains).
+
+In GitLab 13.5 and earlier, merge trains are automatically enabled when
+[pipelines for merged results](../index.md#pipelines-for-merged-results) are enabled.
+To use pipelines for merged results without using merge trains, you can enable a
+[feature flag](../../../../user/feature_flags.md) that blocks the merge trains feature.
 
 [GitLab administrators with access to the GitLab Rails console](../../../../administration/feature_flags.md)
 can enable the feature flag to disable merge trains:

ee/app/controllers/ee/projects_controller.rb

@@ -88,9 +88,8 @@ module EE
         group_with_project_templates_id
       ]
 
-      if allow_merge_pipelines_params?
-        attrs << %i[merge_pipelines_enabled]
-      end
+      attrs << %i[merge_pipelines_enabled] if allow_merge_pipelines_params?
+      attrs << %i[merge_trains_enabled] if allow_merge_trains_params?
 
       attrs += merge_request_rules_params
 
@@ -144,6 +143,10 @@ module EE
       project&.feature_available?(:merge_pipelines)
     end
 
+    def allow_merge_trains_params?
+      project&.feature_available?(:merge_trains)
+    end
+
     def compliance_framework_params
       return [] unless current_user.can?(:admin_compliance_framework, project)
 

ee/app/models/ee/project.rb

@@ -186,7 +186,8 @@ module EE
         to: :import_state, prefix: :mirror, allow_nil: true
 
       delegate :merge_pipelines_enabled, :merge_pipelines_enabled=, :merge_pipelines_enabled?, :merge_pipelines_were_disabled?, to: :ci_cd_settings
-      delegate :merge_trains_enabled?, to: :ci_cd_settings
+      delegate :merge_trains_enabled, :merge_trains_enabled=, :merge_trains_enabled?, to: :ci_cd_settings
+
       delegate :auto_rollback_enabled, :auto_rollback_enabled=, :auto_rollback_enabled?, to: :ci_cd_settings
       delegate :closest_gitlab_subscription, to: :namespace
       delegate :jira_vulnerabilities_integration_enabled?, to: :jira_service, allow_nil: true

ee/app/models/ee/project_ci_cd_setting.rb

@@ -14,9 +14,10 @@ module EE
     # This feature flag is never meant to be enabled for the entire instance.
     # See more context in https://gitlab.com/gitlab-org/gitlab/issues/200037
     def merge_trains_enabled?
-      return false if ::Feature.enabled?(:disable_merge_trains, project)
-
-      merge_pipelines_enabled? && project.feature_available?(:merge_trains)
+      super &&
+        merge_pipelines_enabled? &&
+        project.feature_available?(:merge_trains) &&
+        !merge_trains_disabled?(project)
     end
 
     def merge_pipelines_were_disabled?
@@ -26,5 +27,11 @@ module EE
     def auto_rollback_enabled?
       super && ::Gitlab::Ci::Features.auto_rollback_available?(project)
     end
+
+    private
+
+    def merge_trains_disabled?(project)
+      ::Feature.enabled?(:disable_merge_trains, project)
+    end
   end
 end

ee/app/views/projects/_merge_pipelines_settings.html.haml

@@ -3,10 +3,11 @@
 .form-check.builds-feature
   = form.check_box :merge_pipelines_enabled, class: 'form-check-input js-merge-options-merge-pipelines', data: { qa_selector: 'merged_results_pipeline_checkbox' }
   = form.label :merge_pipelines_enabled, class: 'form-check-label' do
-    = s_('ProjectSettings|Enable merge trains and pipelines for merged results')
+    = s_('ProjectSettings|Enable merged results pipelines.')
     .text-secondary.mb-2
-      = s_('ProjectSettings|Pipelines for merge requests must be enabled in the CI/CD configuration file, or pipelines could be unresolvable or dropped')
+      = s_('ProjectSettings|If pipelines for merge requests are enabled in the CI/CD configuration file, pipelines validate the combined results of the source and target branches.')
       = link_to sprite_icon('question-o'),
         help_page_path('ci/merge_request_pipelines/index.md',
         anchor: 'configuring-pipelines-for-merge-requests'),
-        target: '_blank'
+        target: '_blank',
+        rel: 'noopener noreferer'

ee/app/views/projects/_merge_trains_settings.html.haml

+- return unless merge_trains_available?
+
+.form-check
+  = form.check_box :merge_trains_enabled, class: 'form-check-input'
+  = form.label :merge_trains_enabled, class: 'form-check-label' do
+    = s_('ProjectSettings|Enable merge trains.')
+    .text-secondary.mb-2
+      = s_('ProjectSettings|When approved for merge, merge requests are queued and pipelines validate the combined results of the source and target branches before merge.')
+      = link_to sprite_icon('question-o'),
+        help_page_path('ci/merge_request_pipelines/pipelines_for_merged_results/merge_trains/index.md',
+        anchor: 'merge-trains'),
+        target: '_blank',
+        rel: 'noopener noreferer'

ee/changelogs/unreleased/244831-split-pipelines-for-merged-results-and-merge-train-check-boxes.yml

+---
+title: Split 'Pipelines for merged results' and 'Merge Train' check boxes
+merge_request: 46357
+author:
+type: changed

ee/spec/controllers/projects_controller_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe ProjectsController do
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
 
-  let(:project) { create(:project) }
+  let_it_be(:project, reload: true) { create(:project) }
 
   let_it_be(:public_project) { create(:project, :public, :repository, namespace: group) }
 
@@ -311,6 +311,48 @@ RSpec.describe ProjectsController do
       end
     end
 
+    context 'when merge_trains_enabled param is specified' do
+      let(:params) { { merge_trains_enabled: true } }
+
+      let(:request) do
+        put :update, params: { namespace_id: project.namespace, id: project, project: params }
+      end
+
+      before do
+        stub_licensed_features(merge_pipelines: true, merge_trains: true)
+      end
+
+      it 'updates the attribute' do
+        request
+
+        expect(project.merge_trains_enabled).to be_truthy
+      end
+
+      context 'when feature flag is disabled' do
+        before do
+          stub_feature_flags(merge_trains: false)
+        end
+
+        it 'does not update the attribute' do
+          request
+
+          expect(project.merge_trains_enabled).to be_falsy
+        end
+      end
+
+      context 'when license is not sufficient' do
+        before do
+          stub_licensed_features(merge_trains: false)
+        end
+
+        it 'does not update the attribute' do
+          request
+
+          expect(project.merge_trains_enabled).to be_falsy
+        end
+      end
+    end
+
     context 'when auto_rollback_enabled param is specified' do
       let(:params) { { auto_rollback_enabled: true } }
 

ee/spec/features/merge_request/user_merges_immediately_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe 'Merge requests > User merges immediately', :js do
 
   before_all do
     project.add_maintainer(user)
-    project.update!(merge_pipelines_enabled: true)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
     merge_request.all_pipelines.first.succeed!
     merge_request.update_head_pipeline
   end

ee/spec/features/merge_trains/two_merge_requests_on_train_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe 'Two merge requests on a merge train' do
     project.add_maintainer(maintainer_1)
     project.add_maintainer(maintainer_2)
     stub_licensed_features(merge_pipelines: true, merge_trains: true)
-    project.update!(merge_pipelines_enabled: true)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
     stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml))
 
     head_pipeline = double('Ci::Pipeline')

ee/spec/features/merge_trains/user_adds_merge_request_to_merge_train_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe 'User adds a merge request to a merge train', :js do
     stub_feature_flags(disable_merge_trains: false)
     stub_licensed_features(merge_pipelines: true, merge_trains: true)
     project.add_maintainer(user)
-    project.update!(merge_pipelines_enabled: true)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
     merge_request.all_pipelines.first.succeed!
     merge_request.update_head_pipeline
     stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml))

ee/spec/features/merge_trains/user_adds_to_merge_train_when_pipeline_succeeds_spec.rb

@@ -18,7 +18,8 @@ RSpec.describe 'User adds to merge train when pipeline succeeds', :js do
     stub_feature_flags(disable_merge_trains: false)
     stub_licensed_features(merge_pipelines: true, merge_trains: true)
     project.add_maintainer(user)
-    project.update!(merge_pipelines_enabled: true)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
+
     merge_request.update_head_pipeline
 
     sign_in(user)

ee/spec/features/projects/settings/user_manages_merge_pipelines_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe 'User manages merge pipelines option', :js do
     before do
       visit edit_project_path(project)
 
-      check('Enable merge trains and pipelines for merged results')
+      check('Enable merged results pipelines.')
     end
 
     it 'sees enabled merge pipeline checkbox' do

ee/spec/features/projects/settings/user_manages_merge_trains_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User manages merge trains option', :js do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+
+  before do
+    stub_licensed_features(merge_pipelines: true, merge_trains: true)
+
+    project.add_maintainer(user)
+    sign_in(user)
+  end
+
+  it 'sees unchecked merge trains checkbox' do
+    visit edit_project_path(project)
+
+    expect(page.find('#project_merge_trains_enabled')).not_to be_checked
+  end
+
+  context 'when user enabled the checkbox' do
+    before do
+      visit edit_project_path(project)
+
+      check('Enable merge trains.')
+    end
+
+    it 'sees enabled merge trains checkbox' do
+      expect(page.find('#project_merge_trains_enabled')).to be_checked
+    end
+  end
+
+  context 'when license is insufficient' do
+    before do
+      stub_licensed_features(merge_pipelines: false, merge_trains: false)
+    end
+
+    it 'does not see the checkbox' do
+      expect(page).not_to have_css('#project_merge_trains_enabled')
+    end
+  end
+
+  context 'when feature flag is disabled' do
+    before do
+      stub_feature_flags(merge_pipelines: false, merge_trains: false)
+    end
+
+    it 'does not see the checkbox' do
+      expect(page).not_to have_css('#project_merge_trains_enabled')
+    end
+  end
+end

ee/spec/models/project_ci_cd_setting_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe ProjectCiCdSetting do
+  using RSpec::Parameterized::TableSyntax
+
   before do
     stub_feature_flags(disable_merge_trains: false)
   end
@@ -47,25 +49,30 @@ RSpec.describe ProjectCiCdSetting do
   end
 
   describe '#merge_trains_enabled?' do
-    subject { project.merge_trains_enabled? }
+    subject(:result) { project.merge_trains_enabled? }
 
     let(:project) { create(:project) }
 
-    context 'when Merge trains (EEP) is available' do
-      before do
-        stub_licensed_features(merge_pipelines: true, merge_trains: true)
-        project.merge_pipelines_enabled = true
-      end
-
-      it { is_expected.to be_truthy }
+    where(:merge_pipelines_enabled, :merge_trains_enabled, :feature_available, :expected_result) do
+      true      | true     | true    | true
+      true      | false    | true    | false
+      false     | false    | true    | false
+      false     | true     | true    | false
+      true      | true     | false   | false
+      true      | false    | false   | false
+      false     | false    | false   | false
     end
 
-    context 'when Merge trains (EEP) is unavailable' do
+    with_them do
       before do
-        stub_licensed_features(merge_trains: false)
+        stub_licensed_features(merge_pipelines: feature_available, merge_trains: feature_available)
       end
 
-      it { is_expected.to be_falsy }
+      it 'returns merge trains availability' do
+        project.update!(merge_pipelines_enabled: merge_pipelines_enabled, merge_trains_enabled: merge_trains_enabled)
+
+        expect(result).to eq(expected_result)
+      end
     end
   end
 

ee/spec/serializers/merge_request_poll_widget_entity_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe MergeRequestPollWidgetEntity do
 
     before do
       stub_licensed_features(merge_pipelines: true, merge_trains: true)
-      project.update!(merge_pipelines_enabled: true)
+      project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
     end
 
     it 'has merge train entity' do

ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb

@@ -16,11 +16,11 @@ RSpec.describe AutoMerge::AddToMergeTrainWhenPipelineSucceedsService do
   let(:pipeline) { merge_request.reload.all_pipelines.first }
 
   before do
+    project.add_maintainer(user)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
     stub_feature_flags(ci_disallow_to_create_merge_request_pipelines_in_target_project: false)
     stub_feature_flags(disable_merge_trains: false)
     stub_licensed_features(merge_trains: true, merge_pipelines: true)
-    project.add_maintainer(user)
-    project.update!(merge_pipelines_enabled: true)
     allow(AutoMergeProcessWorker).to receive(:perform_async) { }
     merge_request.update_head_pipeline
   end

ee/spec/services/auto_merge/merge_train_service_spec.rb

@@ -17,13 +17,13 @@ RSpec.describe AutoMerge::MergeTrainService do
 
   before do
     project.add_maintainer(user)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
 
     allow(AutoMergeProcessWorker).to receive(:perform_async) { }
 
     stub_feature_flags(ci_disallow_to_create_merge_request_pipelines_in_target_project: false)
     stub_feature_flags(disable_merge_trains: false)
     stub_licensed_features(merge_trains: true, merge_pipelines: true)
-    project.update!(merge_pipelines_enabled: true)
   end
 
   describe '#execute' do
@@ -349,7 +349,15 @@ RSpec.describe AutoMerge::MergeTrainService do
       2.times { is_expected.to be_truthy }
     end
 
-    context 'when merge trains project option is disabled' do
+    context 'when merge trains flag is disabled' do
+      before do
+        project.update!(merge_trains_enabled: false)
+      end
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'when merge train ci setting is disabled' do
       before do
         stub_feature_flags(disable_merge_trains: true)
       end

ee/spec/services/merge_trains/check_status_service_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe MergeTrains::CheckStatusService do
   before do
     stub_feature_flags(disable_merge_trains: false)
     stub_licensed_features(merge_pipelines: true, merge_trains: true)
-    project.update!(merge_pipelines_enabled: true)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
   end
 
   describe '#execute' do

ee/spec/services/merge_trains/create_pipeline_service_spec.rb

@@ -9,11 +9,11 @@ RSpec.describe MergeTrains::CreatePipelineService do
   let(:previous_ref) { 'refs/heads/master' }
 
   before do
+    project.add_maintainer(maintainer)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
     stub_feature_flags(ci_disallow_to_create_merge_request_pipelines_in_target_project: false)
     stub_feature_flags(disable_merge_trains: false)
-    project.add_maintainer(maintainer)
     stub_licensed_features(merge_pipelines: true, merge_trains: true)
-    project.update!(merge_pipelines_enabled: true)
   end
 
   describe '#execute' do
@@ -35,7 +35,7 @@ RSpec.describe MergeTrains::CreatePipelineService do
       end
     end
 
-    context 'when merge trains option is disabled' do
+    context 'when merge trains flag is disabled' do
       before do
         stub_feature_flags(disable_merge_trains: true)
       end
@@ -45,6 +45,16 @@ RSpec.describe MergeTrains::CreatePipelineService do
       end
     end
 
+    context 'when merge trains setting is disabled' do
+      before do
+        project.update!(merge_trains_enabled: false)
+      end
+
+      it_behaves_like 'returns an error' do
+        let(:expected_reason) { 'merge trains is disabled' }
+      end
+    end
+
     context 'when merge request is not on a merge train' do
       let!(:merge_request) do
         create(:merge_request,

ee/spec/services/merge_trains/refresh_merge_request_service_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe MergeTrains::RefreshMergeRequestService do
     stub_feature_flags(disable_merge_trains: false)
     project.add_maintainer(maintainer)
     stub_licensed_features(merge_pipelines: true, merge_trains: true)
-    project.update!(merge_pipelines_enabled: true)
+    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
   end
 
   describe '#execute' do
@@ -96,6 +96,20 @@ RSpec.describe MergeTrains::RefreshMergeRequestService do
       end
     end
 
+    context 'when merge trains not enabled' do
+      before do
+        project.update!(merge_trains_enabled: false)
+      end
+
+      it_behaves_like 'drops the merge request from the merge train' do
+        let(:expected_reason) { 'project disabled merge trains' }
+      end
+
+      after do
+        project.update!(merge_pipelines_enabled: true)
+      end
+    end
+
     context 'when merge request is not under a mergeable state' do
       before do
         merge_request.update!(title: merge_request.wip_title)

locale/gitlab.pot

@@ -21225,7 +21225,10 @@ msgstr ""
 msgid "ProjectSettings|Enable 'Delete source branch' option by default"
 msgstr ""
 
-msgid "ProjectSettings|Enable merge trains and pipelines for merged results"
+msgid "ProjectSettings|Enable merge trains."
+msgstr ""
+
+msgid "ProjectSettings|Enable merged results pipelines."
 msgstr ""
 
 msgid "ProjectSettings|Encourage"
@@ -21267,6 +21270,9 @@ msgstr ""
 msgid "ProjectSettings|Global"
 msgstr ""
 
+msgid "ProjectSettings|If pipelines for merge requests are enabled in the CI/CD configuration file, pipelines validate the combined results of the source and target branches."
+msgstr ""
+
 msgid "ProjectSettings|Internal"
 msgstr ""
 
@@ -21327,9 +21333,6 @@ msgstr ""
 msgid "ProjectSettings|Pipelines"
 msgstr ""
 
-msgid "ProjectSettings|Pipelines for merge requests must be enabled in the CI/CD configuration file, or pipelines could be unresolvable or dropped"
-msgstr ""
-
 msgid "ProjectSettings|Pipelines must succeed"
 msgstr ""
 
@@ -21426,6 +21429,9 @@ msgstr ""
 msgid "ProjectSettings|View and edit files in this project. Non-project members will only have read access"
 msgstr ""
 
+msgid "ProjectSettings|When approved for merge, merge requests are queued and pipelines validate the combined results of the source and target branches before merge."
+msgstr ""
+
 msgid "ProjectSettings|When conflicts arise the user is given the option to rebase"
 msgstr ""
 

spec/migrations/reseed_merge_trains_enabled_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20201112195322_reseed_merge_trains_enabled.rb')
+
+RSpec.describe ReseedMergeTrainsEnabled do
+  describe 'migrate' do
+    let(:project_ci_cd_settings) { table(:project_ci_cd_settings) }
+    let(:projects) { table(:projects) }
+    let(:namespaces) { table(:namespaces) }
+
+    context 'when on Gitlab.com' do
+      before do
+        namespace = namespaces.create!(name: 'hello', path: 'hello/')
+        project1 = projects.create!(namespace_id: namespace.id)
+        project2 = projects.create!(namespace_id: namespace.id)
+        project_ci_cd_settings.create!(project_id: project1.id, merge_pipelines_enabled: true)
+        project_ci_cd_settings.create!(project_id: project2.id, merge_pipelines_enabled: false)
+      end
+
+      it 'updates merge_trains_enabled to true for where merge_pipelines_enabled is true' do
+        expect { migrate! }.to change(project_ci_cd_settings.where(merge_trains_enabled: true), :count).by(1)
+      end
+    end
+  end
+end