Merge branch 'drop-merge-trains-option' into 'master'

Enable merge trains project-level config by default

See merge request gitlab-org/gitlab-ee!14429

ee/app/assets/javascripts/pages/projects/edit/index.js

@@ -6,7 +6,6 @@ import UserCallout from '~/user_callout';
 import groupsSelect from '~/groups_select';
 import mountApprovals from 'ee/approvals/mount_project_settings';
 import initServiceDesk from 'ee/projects/settings_service_desk';
-import initMergeOptionsCheckboxSync from './init_merge_options_checkbox_sync';
 
 document.addEventListener('DOMContentLoaded', () => {
   new UsersSelect();
@@ -16,6 +15,4 @@ document.addEventListener('DOMContentLoaded', () => {
   new UserCallout({ className: 'js-mr-approval-callout' });
   initServiceDesk();
   mountApprovals(document.getElementById('js-mr-approvals-settings'));
-
-  initMergeOptionsCheckboxSync();
 });

ee/app/assets/javascripts/pages/projects/edit/init_merge_options_checkbox_sync.js

-export default () => {
-  const mergePipelinesCheckbox = document.querySelector('.js-merge-options-merge-pipelines');
-  const mergeTrainsCheckbox = document.querySelector('.js-merge-options-merge-trains');
-
-  if (mergePipelinesCheckbox && mergeTrainsCheckbox) {
-    mergePipelinesCheckbox.addEventListener('change', event => {
-      if (!event.target.checked && mergeTrainsCheckbox.checked) {
-        mergeTrainsCheckbox.click();
-      }
-    });
-
-    mergeTrainsCheckbox.addEventListener('change', event => {
-      if (event.target.checked && !mergePipelinesCheckbox.checked) {
-        mergePipelinesCheckbox.click();
-      }
-    });
-  }
-};

ee/app/controllers/ee/projects_controller.rb

@@ -51,8 +51,6 @@ module EE
         attrs << %i[merge_pipelines_enabled]
       end
 
-      attrs << %i[merge_trains_enabled] if allow_merge_trains_params?
-
       if allow_mirror_params?
         attrs + mirror_params
       else
@@ -79,9 +77,5 @@ module EE
     def allow_merge_pipelines_params?
       project&.feature_available?(:merge_pipelines)
     end
-
-    def allow_merge_trains_params?
-      project&.feature_available?(:merge_trains)
-    end
   end
 end

ee/app/models/ee/project_ci_cd_setting.rb

@@ -8,8 +8,12 @@ module EE
       project.feature_available?(:merge_pipelines) && super
     end
 
+    # As of GitLab 12.1, merge trains option is enabled by default for all projects.
+    # We should drop `merge_trains_enabled` column after this application has been deployed.
+    # See more https://gitlab.com/gitlab-org/gitlab-ee/issues/11222.
     def merge_trains_enabled?
-      merge_pipelines_enabled? && project.feature_available?(:merge_trains) && super
+      merge_pipelines_enabled? && project.feature_available?(:merge_trains) &&
+        ::Feature.enabled?(:merge_trains_enabled, project, default_enabled: true)
     end
   end
 end

ee/app/serializers/ee/merge_request_widget_entity.rb

@@ -135,10 +135,6 @@ module EE
         merge_request.target_project.merge_pipelines_enabled?
       end
 
-      expose :merge_trains_enabled?, as: :merge_trains_enabled do |merge_request|
-        merge_trains_enabled?
-      end
-
       expose :merge_trains_count, if: -> (*) { merge_trains_enabled? } do |merge_request|
         MergeTrain.total_count_in_train(merge_request)
       end

ee/app/services/merge_trains/refresh_merge_request_service.rb

@@ -26,7 +26,7 @@ module MergeTrains
     private
 
     def validate!
-      unless project.merge_trains_enabled? && project.merge_pipelines_enabled?
+      unless project.merge_trains_enabled?
         raise ProcessError, 'project disabled merge trains'
       end
 

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

@@ -10,8 +10,3 @@
         help_page_path('ci/merge_request_pipelines/index.md',
         anchor: 'pipelines-for-merge-requests'),
         target: '_blank'
-    - if merge_trains_available?
-      .form-check.mb-2
-        = form.check_box :merge_trains_enabled, class: 'form-check-input js-merge-options-merge-trains'
-        = form.label :merge_trains_enabled, class: 'form-check-label' do
-          = s_('ProjectSettings|Allow merge trains')

ee/changelogs/unreleased/drop-merge-trains-option.yml

+---
+title: Remove "Allow merge trains" option from project settings page
+merge_request: 14429
+author:
+type: changed

ee/spec/controllers/projects_controller_spec.rb

@@ -259,48 +259,6 @@ 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_trains: true)
-      end
-
-      it 'updates the attribute' do
-        request
-
-        expect(project.reload.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.reload.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.reload.merge_trains_enabled).to be_falsy
-        end
-      end
-    end
-
     context 'repository mirrors' do
       let(:params) do
         {

ee/spec/features/merge_trains/two_merge_requests_on_train_spec.rb

@@ -31,7 +31,7 @@ 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, merge_trains_enabled: true)
+    project.update!(merge_pipelines_enabled: true)
     stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml))
 
     head_pipeline = double('Ci::Pipeline')
@@ -241,28 +241,4 @@ describe 'Two merge requests on a merge train' do
       end
     end
   end
-
-  context 'when merge trains option is disabled' do
-    before do
-      project.update!(merge_trains_enabled: false)
-      merge_request_1.merge_train.pipeline.succeed!
-
-      merge_request_1.reload
-      merge_request_2.reload
-    end
-
-    it 'drops merge request 1 from the merge train' do
-      expect(merge_request_1.merge_train).to be_nil
-      expect(merge_request_1.notes.last.note).to eq('removed this merge request from the merge train because project disabled merge trains')
-    end
-
-    it 'drops merge request 2 from the merge train' do
-      expect(merge_request_2.merge_train).to be_nil
-      expect(merge_request_2.notes.last.note).to eq('removed this merge request from the merge train because project disabled merge trains')
-    end
-
-    after do
-      project.update!(merge_trains_enabled: true)
-    end
-  end
 end

ee/spec/features/merge_trains/user_adds_merge_request_to_merge_train_spec.rb

@@ -19,7 +19,7 @@ describe 'User adds a merge request to a merge train', :js do
   before do
     stub_licensed_features(merge_pipelines: true, merge_trains: true)
     project.add_maintainer(user)
-    project.update!(merge_pipelines_enabled: true, merge_trains_enabled: true)
+    project.update!(merge_pipelines_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

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

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

@@ -123,79 +123,4 @@ describe 'Project settings > [EE] Merge Requests', :js do
       expect(page).to have_content('Choose your merge method, merge options, merge checks, and set up a default description template for merge requests.')
     end
   end
-
-  shared_examples 'the merge train feature is not available' do
-    it 'does not render the merge trains checkbox' do
-      visit edit_project_path(project)
-
-      expect(page).not_to have_selector('#project_merge_trains_enabled')
-    end
-  end
-
-  context 'when merge_pipelines and merge_trains are disabled' do
-    before do
-      stub_licensed_features(merge_pipelines: false, merge_trains: false)
-    end
-
-    it_behaves_like 'the merge train feature is not available'
-  end
-
-  context 'when merge_pipelines is disabled and merge_trains is enabled' do
-    before do
-      stub_licensed_features(merge_pipelines: false, merge_trains: true)
-    end
-
-    it_behaves_like 'the merge train feature is not available'
-  end
-
-  context 'when merge_trains is disabled and merge_pipelines is enabled' do
-    before do
-      stub_licensed_features(merge_pipelines: true, merge_trains: false)
-    end
-
-    it_behaves_like 'the merge train feature is not available'
-  end
-
-  context 'when merge_pipelines and merge_trains are enabled' do
-    before do
-      stub_licensed_features(merge_pipelines: true, merge_trains: true)
-    end
-
-    context 'when both the merge pipelines and merge trains checkboxes are unchecked' do
-      before do
-        visit edit_project_path(project)
-      end
-
-      it 'automatically checks the merge pipelines checkbox when the merge trains checkbox is checked' do
-        expect(page.find('#project_merge_trains_enabled').checked?).to be false
-
-        expect(page.find('#project_merge_pipelines_enabled').checked?).to be false
-
-        check('project_merge_trains_enabled')
-
-        expect(page.find('#project_merge_trains_enabled').checked?).to be true
-
-        expect(page.find('#project_merge_pipelines_enabled').checked?).to be true
-      end
-    end
-
-    context 'when both the merge pipelines and merge trains checkboxes are checked' do
-      before do
-        project.update(merge_pipelines_enabled: true, merge_trains_enabled: true)
-        visit edit_project_path(project)
-      end
-
-      it 'automatically unchecks the merge trains checkbox when the merge pipelines checkbox is unchecked' do
-        expect(page.find('#project_merge_trains_enabled').checked?).to be true
-
-        expect(page.find('#project_merge_pipelines_enabled').checked?).to be true
-
-        uncheck('project_merge_pipelines_enabled')
-
-        expect(page.find('#project_merge_trains_enabled').checked?).to be false
-
-        expect(page.find('#project_merge_pipelines_enabled').checked?).to be false
-      end
-    end
-  end
 end

ee/spec/models/project_ci_cd_setting_spec.rb

@@ -46,11 +46,6 @@ describe ProjectCiCdSetting do
     subject { project.merge_trains_enabled? }
 
     let(:project) { create(:project) }
-    let(:merge_trains_enabled) { true }
-
-    before do
-      project.merge_trains_enabled = merge_trains_enabled
-    end
 
     context 'when Merge trains (EEP) is available' do
       before do
@@ -59,12 +54,6 @@ describe ProjectCiCdSetting do
       end
 
       it { is_expected.to be_truthy }
-
-      context 'when project setting is disabled' do
-        let(:merge_trains_enabled) { false }
-
-        it { is_expected.to be_falsy }
-      end
     end
 
     context 'when Merge trains (EEP) is unavailable' do
@@ -73,12 +62,6 @@ describe ProjectCiCdSetting do
       end
 
       it { is_expected.to be_falsy }
-
-      context 'when project setting is disabled' do
-        let(:merge_trains_enabled) { false }
-
-        it { is_expected.to be_falsy }
-      end
     end
   end
 end

ee/spec/serializers/merge_request_widget_entity_spec.rb

@@ -203,18 +203,17 @@ describe MergeRequestWidgetEntity do
 
     before do
       stub_licensed_features(merge_pipelines: true, merge_trains: true)
-      project.update!(merge_trains_enabled: true, merge_pipelines_enabled: true)
+      project.update!(merge_pipelines_enabled: true)
     end
 
     it 'has merge train entity' do
-      expect(subject.as_json).to include(:merge_trains_enabled)
       expect(subject.as_json).to include(:merge_trains_count)
       expect(subject.as_json).to include(:merge_train_index)
     end
 
     context 'when the merge train feature is disabled' do
       before do
-        project.update!(merge_trains_enabled: false)
+        stub_feature_flags(merge_trains_enabled: false)
       end
 
       it 'does not have merge trains count' do

ee/spec/services/auto_merge/merge_train_service_spec.rb

@@ -204,14 +204,10 @@ describe AutoMerge::MergeTrainService do
 
     context 'when merge trains project option is disabled' do
       before do
-        project.update!(merge_trains_enabled: false)
+        stub_feature_flags(merge_trains_enabled: false)
       end
 
       it { is_expected.to be_falsy }
-
-      after do
-        project.update!(merge_trains_enabled: true)
-      end
     end
 
     context 'when merge request is not mergeable' do

ee/spec/services/merge_trains/create_pipeline_service_spec.rb

@@ -35,16 +35,12 @@ describe MergeTrains::CreatePipelineService do
 
     context 'when merge trains option is disabled' do
       before do
-        project.update!(merge_trains_enabled: false)
+        stub_feature_flags(merge_trains_enabled: false)
       end
 
       it_behaves_like 'returns an error' do
         let(:expected_reason) { 'merge trains is disabled' }
       end
-
-      after do
-        project.update!(merge_trains_enabled: true)
-      end
     end
 
     context 'when merge request is not on a merge train' do

locale/gitlab.pot

@@ -10788,9 +10788,6 @@ msgstr ""
 msgid "ProjectSettings|All discussions must be resolved"
 msgstr ""
 
-msgid "ProjectSettings|Allow merge trains"
-msgstr ""
-
 msgid "ProjectSettings|Automatically resolve merge request diff discussions when they become outdated"
 msgstr ""