Merge branch '455342-ff-cleanup' into 'master'

Remove merge_trains_viz feature flag

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/160262



Merged-by: Max Fan <mfan@gitlab.com>
Approved-by: Max Fan <mfan@gitlab.com>
Reviewed-by: Max Fan <mfan@gitlab.com>
Co-authored-by: Payton Burdette <pburdette@gitlab.com>

doc/ci/pipelines/merge_trains.md

@@ -145,7 +145,7 @@ To add a merge request to a merge train:
    - When a pipeline is running, [**Set to auto-merge**](../../user/project/merge_requests/auto_merge.md).
 
 The merge request's merge train status displays under the pipeline widget with a
-message similar to `Added to the merge train. There are 2 merge requests waiting to be merged.`
+message similar to `This merge request is 2 of 3 in queue.`
 
 Each merge train can run a maximum of twenty pipelines in parallel. If you add more than
 twenty merge requests to the merge train, the extra merge requests are queued, waiting

ee/app/assets/javascripts/vue_merge_request_widget/components/merge_train_position_indicator.vue

@@ -3,14 +3,12 @@ import { isNumber } from 'lodash';
 import { GlLink } from '@gitlab/ui';
 import { s__, sprintf } from '~/locale';
 import { STATUS_OPEN } from '~/issues/constants';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
   name: 'MergeTrainPositionIndicator',
   components: {
     GlLink,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     mergeRequestState: {
       type: String,
@@ -34,9 +32,6 @@ export default {
     },
   },
   computed: {
-    mergeTrainsVizEnabled() {
-      return this.glFeatures.mergeTrainsViz;
-    },
     isMergeRequestOpen() {
       return this.mergeRequestState === STATUS_OPEN;
     },
@@ -47,23 +42,12 @@ export default {
         );
       }
 
-      if (this.mergeTrainsVizEnabled) {
-        if (isNumber(this.mergeTrainIndex) && isNumber(this.mergeTrainsCount)) {
-          return sprintf(
-            s__('mrWidget|This merge request is #%{mergeTrainPosition} of %{total} in queue.'),
-            {
-              mergeTrainPosition: this.mergeTrainIndex + 1,
-              total: this.mergeTrainsCount,
-            },
-          );
-        }
-      } else if (isNumber(this.mergeTrainIndex)) {
+      if (isNumber(this.mergeTrainIndex) && isNumber(this.mergeTrainsCount)) {
         return sprintf(
-          s__(
-            'mrWidget|Added to the merge train. There are %{mergeTrainPosition} merge requests waiting to be merged',
-          ),
+          s__('mrWidget|This merge request is #%{mergeTrainPosition} of %{total} in queue.'),
           {
             mergeTrainPosition: this.mergeTrainIndex + 1,
+            total: this.mergeTrainsCount,
           },
         );
       }
@@ -92,7 +76,7 @@ export default {
   <div v-if="message" class="pt-2 pb-2 pl-3 plr-3 merge-train-position-indicator">
     <div class="media-body gl-text-secondary">
       {{ message }}
-      <gl-link v-if="mergeTrainsVizEnabled && mergeTrainsPath" :href="mergeTrainsPath">
+      <gl-link v-if="mergeTrainsPath" :href="mergeTrainsPath">
         {{ s__('mrWidget|View merge train details.') }}
       </gl-link>
     </div>

ee/app/controllers/ee/projects/merge_requests_controller.rb

@@ -11,7 +11,6 @@ module MergeRequestsController
 
         before_action only: [:show] do
           push_frontend_feature_flag(:merge_trains_skip_train, @project)
-          push_frontend_feature_flag(:merge_trains_viz, @project)
         end
 
         before_action :authorize_read_pipeline!, only: [:metrics_reports]

ee/app/controllers/projects/merge_trains_controller.rb

@@ -3,16 +3,19 @@
 module Projects
   class MergeTrainsController < Projects::ApplicationController
     feature_category :merge_trains
-
     before_action :authorize_read_merge_train!
-    before_action :check_enabled!
+    before_action :authorize!
 
     def index; end
 
     private
 
-    def check_enabled!
-      render_404 unless Feature.enabled?(:merge_trains_viz, project)
+    def authorize!
+      render_404 unless current_user && merge_trains_available?
+    end
+
+    def merge_trains_available?
+      project.licensed_feature_available?(:merge_trains)
     end
   end
 end

ee/app/graphql/mutations/merge_trains/cars/delete.rb

@@ -29,7 +29,7 @@ def resolve(car_id:)
         private
 
         def ensure_feature_available!
-          return if Feature.enabled?(:merge_trains_viz, project) && merge_trains_available?
+          return if merge_trains_available?
 
           raise_resource_not_available_error!
         end

ee/app/graphql/resolvers/merge_trains/trains_resolver.rb

@@ -31,7 +31,7 @@ def resolve(status: nil, target_branches: [])
       private
 
       def ensure_feature_available
-        (Feature.enabled?(:merge_trains_viz, project) && merge_trains_available?) || raise_resource_not_available_error!
+        merge_trains_available? || raise_resource_not_available_error!
       end
 
       def merge_trains_available?

ee/config/feature_flags/wip/merge_trains_viz.yml

----
-name: merge_trains_viz
-feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/454179
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/149025
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/455342
-milestone: '16.11'
-group: group::pipeline execution
-type: wip
-default_enabled: false

ee/spec/frontend/vue_merge_request_widget/components/merge_train_position_indicator_spec.js

@@ -10,17 +10,12 @@ describe('MergeTrainPositionIndicator', () => {
 
   const findLink = () => wrapper.findComponent(GlLink);
 
-  const createComponent = (props, mergeTrainsViz = false) => {
+  const createComponent = (props) => {
     wrapper = shallowMount(MergeTrainPositionIndicator, {
       propsData: {
         mergeTrainsPath: 'namespace/project/-/merge_trains',
         ...props,
       },
-      provide: {
-        glFeatures: {
-          mergeTrainsViz,
-        },
-      },
       mocks: {
         $toast: {
           show: mockToast,
@@ -29,74 +24,37 @@ describe('MergeTrainPositionIndicator', () => {
     });
   };
 
-  describe('with mergeTrainsViz enabled', () => {
-    it('should show message when position is higher than 1', () => {
-      createComponent(
-        {
-          mergeTrainIndex: 3,
-          mergeTrainsCount: 5,
-        },
-        true,
-      );
-
-      expect(trimText(wrapper.text())).toBe(
-        'This merge request is #4 of 5 in queue. View merge train details.',
-      );
-      expect(findLink().attributes('href')).toBe('namespace/project/-/merge_trains');
+  it('should show message when position is higher than 1', () => {
+    createComponent({
+      mergeTrainIndex: 3,
+      mergeTrainsCount: 5,
     });
 
-    it('should show message when the position is 1', () => {
-      createComponent({ mergeTrainIndex: 0, mergeTrainsCount: 0 }, true);
-
-      expect(trimText(wrapper.text())).toBe(
-        'A new merge train has started and this merge request is the first of the queue. View merge train details.',
-      );
-      expect(findLink().attributes('href')).toBe('namespace/project/-/merge_trains');
-    });
-
-    it('should not render when merge request is not in train', () => {
-      createComponent(
-        {
-          mergeTrainIndex: null,
-          mergeTrainsCount: 1,
-        },
-        true,
-      );
-
-      expect(wrapper.text()).toBe('');
-    });
+    expect(trimText(wrapper.text())).toBe(
+      'This merge request is #4 of 5 in queue. View merge train details.',
+    );
+    expect(findLink().attributes('href')).toBe('namespace/project/-/merge_trains');
   });
 
-  describe('with mergeTrainsViz disabled', () => {
-    it('should show message when position is higher than 1', () => {
-      createComponent({ mergeTrainIndex: 3 });
-
-      expect(trimText(wrapper.text())).toBe(
-        'Added to the merge train. There are 4 merge requests waiting to be merged',
-      );
-      expect(findLink().exists()).toBe(false);
-    });
+  it('should show message when the position is 1', () => {
+    createComponent({ mergeTrainIndex: 0, mergeTrainsCount: 0 }, true);
 
-    it('should show message when the position is 1', () => {
-      createComponent({ mergeTrainIndex: 0 });
-
-      expect(trimText(wrapper.text())).toBe(
-        'A new merge train has started and this merge request is the first of the queue.',
-      );
-      expect(findLink().exists()).toBe(false);
-    });
+    expect(trimText(wrapper.text())).toBe(
+      'A new merge train has started and this merge request is the first of the queue. View merge train details.',
+    );
+    expect(findLink().attributes('href')).toBe('namespace/project/-/merge_trains');
+  });
 
-    it('should not render when merge request is not in train', () => {
-      createComponent(
-        {
-          mergeTrainIndex: null,
-          mergeTrainsCount: 1,
-        },
-        true,
-      );
+  it('should not render when merge request is not in train', () => {
+    createComponent(
+      {
+        mergeTrainIndex: null,
+        mergeTrainsCount: 1,
+      },
+      true,
+    );
 
-      expect(wrapper.text()).toBe('');
-    });
+    expect(wrapper.text()).toBe('');
   });
 
   describe('when position in the train changes', () => {

ee/spec/requests/api/graphql/merge_trains/trains_spec.rb

@@ -130,20 +130,6 @@
   end
 
   context 'when the user has the right permissions' do
-    context 'when the feature is disabled' do
-      before do
-        stub_feature_flags(merge_trains_viz: false)
-      end
-
-      it 'returns a resource not available error' do
-        post_query
-        expect_graphql_errors_to_include(
-          "The resource that you are attempting to access does not exist " \
-            "or you don't have permission to perform this action"
-        )
-      end
-    end
-
     context 'when only the project is provided' do
       it_behaves_like 'fetches the requested trains' do
         let(:expected_branches) { %w[master feature-1] }

ee/spec/requests/api/graphql/mutations/merge_trains/cars/delete_spec.rb

@@ -90,20 +90,6 @@
       end
     end
 
-    context 'when the feature is disabled' do
-      before do
-        stub_feature_flags(merge_trains_viz: false)
-        post_mutation
-      end
-
-      it 'returns a resource not available error' do
-        expect_graphql_errors_to_include(
-          "The resource that you are attempting to access does not exist " \
-            "or you don't have permission to perform this action"
-        )
-      end
-    end
-
     context 'when the car does not exist' do
       let(:target_car_id) { build(:merge_train_car, id: non_existing_record_iid).to_gid }
 

ee/spec/requests/projects/merge_trains_controller_spec.rb

@@ -13,27 +13,29 @@
       project.add_maintainer(user)
     end
 
-    before do
-      sign_in(user)
-    end
+    context 'when feature is enabled' do
+      before do
+        stub_licensed_features(merge_trains: true)
+        sign_in(user)
 
-    context 'when feature flag "merge_trains_viz" is enabled' do
-      it 'renders the merge trains index template' do
         request
+      end
 
+      it 'renders the merge trains index template' do
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to render_template('projects/merge_trains/index')
       end
     end
 
-    context 'when feature flag "merge_trains_viz" is disabled' do
+    context 'when feature is disabled' do
       before do
-        stub_feature_flags(merge_trains_viz: false)
-      end
+        stub_licensed_features(merge_trains: false)
+        sign_in(user)
 
-      it 'returns "not found response"' do
         request
+      end
 
+      it 'returns "not found response"' do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end

locale/gitlab.pot

@@ -63541,9 +63541,6 @@ msgstr ""
 msgid "mrWidget|Added to the merge train by %{merge_author}"
 msgstr ""
-msgid "mrWidget|Added to the merge train. There are %{mergeTrainPosition} merge requests waiting to be merged"
-msgstr ""
-
 msgid "mrWidget|An error occurred while removing your approval."
 msgstr ""