diff --git a/doc/ci/pipelines/merge_trains.md b/doc/ci/pipelines/merge_trains.md index cf771e3296d1d9ce8d33d7d27c128ddbb77d2db1..e0e2fb8aeab549c16b73055cd2ff331e459b6276 100644 --- a/doc/ci/pipelines/merge_trains.md +++ b/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 diff --git a/ee/app/assets/javascripts/vue_merge_request_widget/components/merge_train_position_indicator.vue b/ee/app/assets/javascripts/vue_merge_request_widget/components/merge_train_position_indicator.vue index 80eab4209f0df69999fce93f3c53d29688b44570..5549b592566777cfb85c28d8463004f6a48cd74a 100644 --- a/ee/app/assets/javascripts/vue_merge_request_widget/components/merge_train_position_indicator.vue +++ b/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> diff --git a/ee/app/controllers/ee/projects/merge_requests_controller.rb b/ee/app/controllers/ee/projects/merge_requests_controller.rb index bf81baccc5a4743985c0f38216d2748032939f1b..c21f08be0179d7c1a58fd240b5d7defaf91faae2 100644 --- a/ee/app/controllers/ee/projects/merge_requests_controller.rb +++ b/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] diff --git a/ee/app/controllers/projects/merge_trains_controller.rb b/ee/app/controllers/projects/merge_trains_controller.rb index 353f3a8ab6b5babb7402ce884e21e2872dec67bd..608051ede1aef6b82e6b461544e5fd03ca937fd0 100644 --- a/ee/app/controllers/projects/merge_trains_controller.rb +++ b/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 diff --git a/ee/app/graphql/mutations/merge_trains/cars/delete.rb b/ee/app/graphql/mutations/merge_trains/cars/delete.rb index 0d96ec2f1c22fe991193188953d23556096393fe..e2fc8e14aab49135d9e43202ba1ea083abbd9ebe 100644 --- a/ee/app/graphql/mutations/merge_trains/cars/delete.rb +++ b/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 diff --git a/ee/app/graphql/resolvers/merge_trains/trains_resolver.rb b/ee/app/graphql/resolvers/merge_trains/trains_resolver.rb index 77cf7bd6b1f37572bc541885c74e5ebfb3e7db94..1c63ee82df04d3cadc87212cdbe487bc649d39db 100644 --- a/ee/app/graphql/resolvers/merge_trains/trains_resolver.rb +++ b/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? diff --git a/ee/config/feature_flags/wip/merge_trains_viz.yml b/ee/config/feature_flags/wip/merge_trains_viz.yml deleted file mode 100644 index b9b9e01aad31e64cea8a1335751e172ff37091a1..0000000000000000000000000000000000000000 --- a/ee/config/feature_flags/wip/merge_trains_viz.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -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 diff --git a/ee/spec/frontend/vue_merge_request_widget/components/merge_train_position_indicator_spec.js b/ee/spec/frontend/vue_merge_request_widget/components/merge_train_position_indicator_spec.js index a5d28790dfd124250d006388ac6dc199d859a6fc..123e7d6577097d412f0d97bd659f52f7037632e9 100644 --- a/ee/spec/frontend/vue_merge_request_widget/components/merge_train_position_indicator_spec.js +++ b/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', () => { diff --git a/ee/spec/requests/api/graphql/merge_trains/trains_spec.rb b/ee/spec/requests/api/graphql/merge_trains/trains_spec.rb index 3a0faad882b32056c698ec67ac2ee5147dbe7ccb..5a3a0dce6465ed6cd1cef8da950fb8a197d47be3 100644 --- a/ee/spec/requests/api/graphql/merge_trains/trains_spec.rb +++ b/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] } diff --git a/ee/spec/requests/api/graphql/mutations/merge_trains/cars/delete_spec.rb b/ee/spec/requests/api/graphql/mutations/merge_trains/cars/delete_spec.rb index 952fda587bace7a98fbb04e1410523fcc76d3bff..4da723c108f71c1362990f238da819398774f0f6 100644 --- a/ee/spec/requests/api/graphql/mutations/merge_trains/cars/delete_spec.rb +++ b/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 } diff --git a/ee/spec/requests/projects/merge_trains_controller_spec.rb b/ee/spec/requests/projects/merge_trains_controller_spec.rb index ef15f3f71ea2a0a9e36ef5fdb23cb99a12115e75..9992f9983d9c9fb43e4f513afed013172a519bec 100644 --- a/ee/spec/requests/projects/merge_trains_controller_spec.rb +++ b/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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c9a3ebb2c2b59fadc75be0974487ad93878c723f..3990745fd762f36725ae17f81b264dc3fe7ebad3 100644 --- a/locale/gitlab.pot +++ b/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 ""