diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index 3ff2488d8787e4b7713cce078bf2e78788ee94ec..1dc7bf37119b0d0ec4a88db64a7d655da159e468 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -356,7 +356,7 @@ export default { }; }, isSkipMergeTrainAvailable() { - return this.mergeTrainsSkipAllowed && this.glFeatures.mergeTrainsSkipTrain; + return this.mergeTrainsSkipAllowed; }, displaySkipMergeTrainOptions() { return this.shouldDisplayMergeImmediatelyDropdownOptions && this.isSkipMergeTrainAvailable; diff --git a/config/feature_flags/development/merge_trains_skip_train.yml b/config/feature_flags/development/merge_trains_skip_train.yml deleted file mode 100644 index b1e215c69f1325dd20a5185c1d902a868d6e1afa..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/merge_trains_skip_train.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: merge_trains_skip_train -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129422 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/422111 -milestone: '16.4' -type: development -group: group::pipeline execution -default_enabled: true diff --git a/doc/ci/pipelines/merge_trains.md b/doc/ci/pipelines/merge_trains.md index 130ee137cb7a81b7a295c30440c5badd56b1ddf1..dbc9622c8cefcc8ff2e342384e4a06f34ee742cc 100644 --- a/doc/ci/pipelines/merge_trains.md +++ b/doc/ci/pipelines/merge_trains.md @@ -247,17 +247,10 @@ merge method and the source branch is behind the target branch. See [issue 43407 - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/414505) in GitLab 16.5 [with a flag](../../administration/feature_flags/_index.md) named `merge_trains_skip_train`. Disabled by default. - [Enabled](https://gitlab.com/gitlab-org/gitlab/-/issues/422111) as an [experiment feature](../../policy/development_stages_support.md) in GitLab 16.10. +- [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/422111) in GitLab 18.5. Feature flag `merge_trains_skip_train` removed. {{< /history >}} -{{< alert type="flag" >}} - -On GitLab Self-Managed, by default this feature is available. To hide the feature, -an administrator can [disable the feature flag](../../administration/feature_flags/_index.md) -named `merge_trains_skip_train`. On GitLab.com and GitLab Dedicated, this feature is available. - -{{< /alert >}} - You can allow merge requests to be merged without completely restarting a running merge train. Use this feature to quickly merge changes that can safely skip the pipeline, for example minor documentation updates. diff --git a/ee/app/controllers/ee/projects/merge_requests_controller.rb b/ee/app/controllers/ee/projects/merge_requests_controller.rb index 3b7f5aedfd4f6c8738767dbda0fa9a6fc85d0099..4e295fcf8eb1c246d4cea4b23e326a52c8dcd313 100644 --- a/ee/app/controllers/ee/projects/merge_requests_controller.rb +++ b/ee/app/controllers/ee/projects/merge_requests_controller.rb @@ -10,7 +10,6 @@ module MergeRequestsController include GeoInstrumentation before_action only: [:show] do - push_frontend_feature_flag(:merge_trains_skip_train, @project) push_frontend_feature_flag(:vulnerability_partial_scans, @project) push_frontend_ability(ability: :resolve_vulnerability_with_ai, resource: @project, user: current_user) push_frontend_ability(ability: :measure_comment_temperature, resource: merge_request, user: current_user) diff --git a/ee/app/controllers/ee/projects_controller.rb b/ee/app/controllers/ee/projects_controller.rb index d1fb6251ead46bd479a69023ed237963cc280fc9..35bcb6c4cfe4ea41c59d9a2efd0148560df7f0e0 100644 --- a/ee/app/controllers/ee/projects_controller.rb +++ b/ee/app/controllers/ee/projects_controller.rb @@ -100,7 +100,6 @@ def project_params_ee attrs << :merge_pipelines_enabled if allow_merge_pipelines_params? attrs << :merge_trains_enabled if allow_merge_trains_params? - attrs << :merge_trains_skip_train_allowed if allow_merge_trains_params? attrs += merge_request_rules_params diff --git a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb index 179f42d5bbf5f78acd04e617f03d55c05ae78f74..533b8929529687128301112057019abec15a8086 100644 --- a/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb +++ b/ee/app/graphql/ee/mutations/ci/project_ci_cd_settings_update.rb @@ -18,8 +18,7 @@ module ProjectCiCdSettingsUpdate argument :merge_trains_skip_train_allowed, GraphQL::Types::Boolean, required: false, - description: 'Indicates whether an option is allowed to merge without refreshing the merge train. ' \ - 'Ignored unless the `merge_trains_skip_train` feature flag is also enabled.' + description: 'Indicates whether an option is allowed to merge without refreshing the merge train.' end override :resolve @@ -44,11 +43,7 @@ def resolve(**args) override :project_update_params def project_update_params(project, **args) result = super - result.merge!(args.slice(:merge_pipelines_enabled, :merge_trains_enabled)) - - if ::Feature.enabled?(:merge_trains_skip_train, project) - result.merge!(args.slice(:merge_trains_skip_train_allowed)) - end + result.merge!(args.slice(:merge_pipelines_enabled, :merge_trains_enabled, :merge_trains_skip_train_allowed)) result.compact end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 89f71aecdd2433bf85adbd1c9cd5f3112c4b7c16..369bca87af01c9471be02077e61da9bdbdc12c83 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -1327,7 +1327,7 @@ def merge_trains_enabled? def merge_trains_skip_train_allowed? return false unless ci_cd_settings - ci_cd_settings.merge_trains_skip_train_allowed? + true end def auto_rollback_enabled? diff --git a/ee/app/models/ee/project_ci_cd_setting.rb b/ee/app/models/ee/project_ci_cd_setting.rb index bc35020a3f099fafc388ef0c199d33d466456989..868fc3032ea33b617d258d266357d351e732f5a3 100644 --- a/ee/app/models/ee/project_ci_cd_setting.rb +++ b/ee/app/models/ee/project_ci_cd_setting.rb @@ -33,8 +33,7 @@ def auto_rollback_enabled? def merge_trains_skip_train_allowed? merge_trains_skip_train_allowed && merge_trains_enabled? && - !project.ff_merge_must_be_possible? && # Not yet supported, see https://gitlab.com/gitlab-org/gitlab/-/issues/429009 - ::Feature.enabled?(:merge_trains_skip_train, project) + !project.ff_merge_must_be_possible? # Not yet supported, see https://gitlab.com/gitlab-org/gitlab/-/issues/429009 end end end diff --git a/ee/app/views/projects/settings/merge_requests/_merge_trains_settings.html.haml b/ee/app/views/projects/settings/merge_requests/_merge_trains_settings.html.haml index 7c4090c4740645adcdcdd5793281241e2efdac34..376d83f455de620a8d8d959b52e5d6a624608dbc 100644 --- a/ee/app/views/projects/settings/merge_requests/_merge_trains_settings.html.haml +++ b/ee/app/views/projects/settings/merge_requests/_merge_trains_settings.html.haml @@ -14,8 +14,7 @@ s_('ProjectSettings|Enable merge trains'), help_text: s_('ProjectSettings|Merge requests approved for merge are queued, and pipelines validate the combined results of the source and target branches before merge. %{link_start}What are merge trains?%{link_end}').html_safe % { link_start: merge_trains_help_link_start, link_end: ''.html_safe }, checkbox_options: { class: 'js-merge-options-merge-trains', data: { testid: 'merge-trains-checkbox' } } - - if Feature.enabled?(:merge_trains_skip_train) - = form.gitlab_ui_checkbox_component :merge_trains_skip_train_allowed, - s_('ProjectSettings|Allow skipping the merge train'), - help_text: full_skip_trains_help_test, - checkbox_options: { class: 'js-merge-options-merge-trains-skip-train-allowed' } + = form.gitlab_ui_checkbox_component :merge_trains_skip_train_allowed, + s_('ProjectSettings|Allow skipping the merge train'), + help_text: full_skip_trains_help_test, + checkbox_options: { class: 'js-merge-options-merge-trains-skip-train-allowed' } diff --git a/ee/lib/ee/api/entities/project.rb b/ee/lib/ee/api/entities/project.rb index 0d996d02705dec5b67e6eaaddc29b6ac44e84839..5a5f655daedc058f8f98fc697d49f34fbfd1e2a8 100644 --- a/ee/lib/ee/api/entities/project.rb +++ b/ee/lib/ee/api/entities/project.rb @@ -60,7 +60,7 @@ def preload_relation(projects_relation, options = {}) } expose :merge_pipelines_enabled?, as: :merge_pipelines_enabled, if: ->(project, _) { project.feature_available?(:merge_pipelines) } expose :merge_trains_enabled?, as: :merge_trains_enabled, if: ->(project, _) { project.feature_available?(:merge_pipelines) } - expose :merge_trains_skip_train_allowed?, as: :merge_trains_skip_train_allowed, if: ->(project, _) { project.feature_available?(:merge_pipelines) } + expose :true, as: :merge_trains_skip_train_allowed, if: ->(project, _) { project.feature_available?(:merge_pipelines) } expose :only_allow_merge_if_all_status_checks_passed, if: ->(project, _) { project.feature_available?(:external_status_checks) } expose :allow_pipeline_trigger_approve_deployment, documentation: { type: 'boolean' }, if: ->(project, _) { project.feature_available?(:protected_environments) } expose :prevent_merge_without_jira_issue, if: ->(project, _) { project.feature_available?(:jira_issue_association_enforcement) } diff --git a/ee/spec/features/merge_request/user_merges_immediately_spec.rb b/ee/spec/features/merge_request/user_merges_immediately_spec.rb index 5ab58b5a42529c805d1b71198402bb64c9f89ef9..be40796f30098b3715cd2d2920f6aa61cd6bd6c1 100644 --- a/ee/spec/features/merge_request/user_merges_immediately_spec.rb +++ b/ee/spec/features/merge_request/user_merges_immediately_spec.rb @@ -50,42 +50,9 @@ def merge_from_warning_dialog( end end - context 'when the merge request is on the merge train and the merge_trains_skip_train feature flag is disabled' do + context 'when the merge request is on the merge train' do before do stub_licensed_features(merge_pipelines: true, merge_trains: true) - stub_feature_flags(merge_trains_skip_train: false) - stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml)) - - sign_in(user) - visit project_merge_request_path(project, merge_request) - wait_for_requests - end - - it 'shows a warning dialog and does nothing if the user selects "Cancel"' do - Sidekiq::Testing.fake! do - open_warning_dialog - - find(':focus').send_keys :enter - - expect(merge_button).to have_content('Merge') - end - end - - it 'shows a warning dialog and merges immediately after the user confirms' do - Sidekiq::Testing.fake! do - open_warning_dialog - - click_button 'Merge immediately' - - expect(find_by_testid('merging-state')).to have_content('Merging!') - end - end - end - - context 'when the merge request is on the merge train and the merge_trains_skip_train feature flag is enabled' do - before do - stub_licensed_features(merge_pipelines: true, merge_trains: true) - stub_feature_flags(merge_trains_skip_train: true) stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml)) sign_in(user) diff --git a/ee/spec/features/projects/settings/merge_requests/merge_trains/enable_merge_trains_skip_train_allowed_spec.rb b/ee/spec/features/projects/settings/merge_requests/merge_trains/enable_merge_trains_skip_train_allowed_spec.rb index f716c40aaebe33777df904ff48cf772add9107af..a05eec77e864a393f7d1c075aa5169a7aa0f2418 100644 --- a/ee/spec/features/projects/settings/merge_requests/merge_trains/enable_merge_trains_skip_train_allowed_spec.rb +++ b/ee/spec/features/projects/settings/merge_requests/merge_trains/enable_merge_trains_skip_train_allowed_spec.rb @@ -36,14 +36,4 @@ expect(project.ci_cd_settings.merge_trains_skip_train_allowed).to eq(true) end end - - context 'when the feature flag is disabled' do - before do - stub_feature_flags(merge_trains_skip_train: false) - end - - it 'does not show the checkbox' do - expect(page).not_to have_checked_field('#project_merge_trains_skip_train_allowed') - end - end end diff --git a/ee/spec/frontend/vue_merge_request_widget/components/states/mr_widget_ready_to_merge_spec.js b/ee/spec/frontend/vue_merge_request_widget/components/states/mr_widget_ready_to_merge_spec.js index 66d4b64b3bcddf14a3aaec3c56577c20b094c14b..5c7dce941e24c40240150cad2eb2492e0a4e9daa 100644 --- a/ee/spec/frontend/vue_merge_request_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/ee/spec/frontend/vue_merge_request_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -60,7 +60,6 @@ describe('ReadyToMerge', () => { mrUpdates = {}, mountFn = shallowMountExtended, data = {}, - mergeTrainsSkipTrainFF = false, // eslint-disable-next-line max-params ) => { wrapper = mountFn(ReadyToMerge, { @@ -88,11 +87,6 @@ describe('ReadyToMerge', () => { GlDisclosureDropdown, GlDisclosureDropdownItem, }, - provide: { - glFeatures: { - mergeTrainsSkipTrain: mergeTrainsSkipTrainFF, - }, - }, }); }; @@ -166,7 +160,7 @@ describe('ReadyToMerge', () => { ); }); - it('should display the new merge dropdown options for merge trains when the skip trains feature flag is enabled', () => { + it('should display the merge dropdown options for merge trains', () => { createComponent( { availableAutoMergeStrategies: [MT_MERGE_STRATEGY], @@ -175,8 +169,6 @@ describe('ReadyToMerge', () => { mergeTrainsSkipAllowed: true, }, shallowMountExtended, - {}, - true, ); expect(findMergeTrainMergeNowRestartTrainButton().exists()).toBe(true); @@ -263,7 +255,7 @@ describe('ReadyToMerge', () => { expect(mr.transitionStateMachine).toHaveBeenCalled(); }); - it('starts to merge a merge request when restarting a merge train with the new confirmation dialog', async () => { + it('starts to merge a merge request when restarting a merge train with the confirmation dialog', async () => { createComponent( { availableAutoMergeStrategies: [MT_MERGE_STRATEGY], @@ -275,8 +267,6 @@ describe('ReadyToMerge', () => { mergeTrainsSkipAllowed: true, }, shallowMountExtended, - {}, - true, ); findMergeTrainRestartTrainConfirmationDialog().vm.$emit('show'); @@ -293,27 +283,16 @@ describe('ReadyToMerge', () => { expect(mr.transitionStateMachine).toHaveBeenCalledWith({ transition: 'start-merge' }); }); - it('does not contain the new confirmation dialog for merging merge trains immediately when the mergeTrainSkipTrain feature flag is disabled', () => { - createComponent({ - availableAutoMergeStrategies: [MT_MERGE_STRATEGY], - mergeTrainsSkipAllowed: true, - }); - - expect(findMergeTrainRestartTrainConfirmationDialog().exists()).toBe(false); - }); - - it('contains the new confirmation dialog for merging merge trains immediately when the mergeTrainSkipTrain feature flag is enabled', () => { + it('contains the confirmation dialog for merging merge trains immediately', () => { createComponent( { availableAutoMergeStrategies: [MT_MERGE_STRATEGY], mergeTrainsSkipAllowed: true }, shallowMountExtended, - {}, - true, ); expect(findMergeTrainRestartTrainConfirmationDialog().exists()).toBe(true); }); - it('should not ask for confirmation in non-merge train scenarios with the new confirmation dialog', async () => { + it('should not ask for confirmation in non-merge train scenarios with the confirmation dialog', async () => { createComponent( { headPipeline: { id: 'gid://gitlab/Pipeline/1', path: 'path/to/pipeline', active: true }, @@ -321,8 +300,6 @@ describe('ReadyToMerge', () => { mergeTrainsSkipAllowed: true, }, shallowMountExtended, - {}, - true, ); await clickMergeImmediately(); diff --git a/ee/spec/services/merge_requests/merge_service_spec.rb b/ee/spec/services/merge_requests/merge_service_spec.rb index 114e6fec324967cc55f987fc59214d648f98f348..ef029e6aba44674ac3d8e07f0fa7c4eefabb7faa 100644 --- a/ee/spec/services/merge_requests/merge_service_spec.rb +++ b/ee/spec/services/merge_requests/merge_service_spec.rb @@ -205,18 +205,6 @@ expect(train.sha_exists_in_history?(merge_request.merge_commit_sha)).to be_falsy end end - - context 'when the merge_train_skip_trains feature flag is disabled' do - before do - stub_feature_flags(merge_trains_skip_train: false) - end - - it 'does not create any new Car record or add to the train revision history' do - expect { subject }.not_to change { MergeTrains::Car.count } - expect(merge_request).to be_merged - expect(train.sha_exists_in_history?(merge_request.merge_commit_sha)).to be_falsy - end - end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c81cd117d19716abff071d2ded67b83199b4f526..3b573b3b34df40a2dbfa06dadc37205b5d1c3e47 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1749,7 +1749,6 @@ merge_pipelines_enabled merge_trains_enabled auto_rollback_enabled - merge_trains_skip_train_allowed restrict_pipeline_cancellation_role ] end