Commit ae78409e authored by Lee Tickett's avatar Lee Tickett Committed by Phil Hughes

Remove `mr_tabs_position` feature flag

parent 5adf8f0a
<script>
import { GlPopover, GlDeprecatedButton, GlLink } from '@gitlab/ui';
import Icon from '~/vue_shared/components/icon.vue';
import axios from '~/lib/utils/axios_utils';
export default {
components: {
GlPopover,
GlDeprecatedButton,
GlLink,
Icon,
},
props: {
dismissEndpoint: {
type: String,
required: true,
},
featureId: {
type: String,
required: true,
},
},
data() {
return {
showPopover: false,
};
},
mounted() {
setTimeout(() => {
this.showPopover = true;
}, 2000);
},
methods: {
onDismiss() {
this.showPopover = false;
axios.post(this.dismissEndpoint, {
feature_name: this.featureId,
});
},
},
};
</script>
<template>
<gl-popover target="#diffs-tab" placement="bottom" :show="showPopover">
<p class="mb-2">
{{
__(
'Now you can access the merge request navigation tabs at the top, where they’re easier to find.',
)
}}
</p>
<p>
<gl-link href="https://gitlab.com/gitlab-org/gitlab/issues/36125" target="_blank">
{{ __('More information and share feedback') }}
<icon name="external-link" :size="10" />
</gl-link>
</p>
<gl-deprecated-button
variant="primary"
size="sm"
data-qa-selector="dismiss_popover_button"
@click="onDismiss"
>
{{ __('Got it') }}
</gl-deprecated-button>
</gl-popover>
</template>
import Vue from 'vue';
import Popover from './components/popover.vue';
export default el =>
new Vue({
el,
render(createElement) {
return createElement(Popover, {
props: { dismissEndpoint: el.dataset.dismissEndpoint, featureId: el.dataset.featureId },
});
},
});
...@@ -6,7 +6,6 @@ import howToMerge from '~/how_to_merge'; ...@@ -6,7 +6,6 @@ import howToMerge from '~/how_to_merge';
import initPipelines from '~/commit/pipelines/pipelines_bundle'; import initPipelines from '~/commit/pipelines/pipelines_bundle';
import initVueIssuableSidebarApp from '~/issuable_sidebar/sidebar_bundle'; import initVueIssuableSidebarApp from '~/issuable_sidebar/sidebar_bundle';
import initSourcegraph from '~/sourcegraph'; import initSourcegraph from '~/sourcegraph';
import initPopover from '~/mr_tabs_popover';
export default function() { export default function() {
new ZenMode(); // eslint-disable-line no-new new ZenMode(); // eslint-disable-line no-new
...@@ -20,10 +19,4 @@ export default function() { ...@@ -20,10 +19,4 @@ export default function() {
handleLocationHash(); handleLocationHash();
howToMerge(); howToMerge();
initSourcegraph(); initSourcegraph();
const tabHighlightEl = document.querySelector('.js-tabs-feature-highlight');
if (tabHighlightEl) {
initPopover(tabHighlightEl);
}
} }
...@@ -170,12 +170,6 @@ module MergeRequestsHelper ...@@ -170,12 +170,6 @@ module MergeRequestsHelper
current_user.fork_of(project) current_user.fork_of(project)
end end
end end
def mr_tabs_position_enabled?
strong_memoize(:mr_tabs_position_enabled) do
Feature.enabled?(:mr_tabs_position, @project, default_enabled: true)
end
end
end end
MergeRequestsHelper.prepend_if_ee('EE::MergeRequestsHelper') MergeRequestsHelper.prepend_if_ee('EE::MergeRequestsHelper')
...@@ -37,10 +37,6 @@ module UserCalloutsHelper ...@@ -37,10 +37,6 @@ module UserCalloutsHelper
!user_dismissed?(SUGGEST_POPOVER_DISMISSED) !user_dismissed?(SUGGEST_POPOVER_DISMISSED)
end end
def show_tabs_feature_highlight?
current_user && !user_dismissed?(TABS_POSITION_HIGHLIGHT) && !Rails.env.test?
end
def show_webhooks_moved_alert? def show_webhooks_moved_alert?
!user_dismissed?(WEBHOOKS_MOVED) !user_dismissed?(WEBHOOKS_MOVED)
end end
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
- hide_top_links = @hide_top_links || false - hide_top_links = @hide_top_links || false
%nav.breadcrumbs{ role: "navigation", class: [container, @content_class] } %nav.breadcrumbs{ role: "navigation", class: [container, @content_class] }
.breadcrumbs-container{ class: ("border-bottom-0" if @no_breadcrumb_border && mr_tabs_position_enabled?) } .breadcrumbs-container{ class: ("border-bottom-0" if @no_breadcrumb_border) }
- if defined?(@left_sidebar) - if defined?(@left_sidebar)
= button_tag class: 'toggle-mobile-nav', type: 'button' do = button_tag class: 'toggle-mobile-nav', type: 'button' do
%span.sr-only= _("Open sidebar") %span.sr-only= _("Open sidebar")
......
.content-block.content-block-small.emoji-list-container.js-noteable-awards .content-block.content-block-small.emoji-list-container.js-noteable-awards
= render 'award_emoji/awards_block', awardable: @merge_request, inline: true do = render 'award_emoji/awards_block', awardable: @merge_request, inline: true do
- if mr_tabs_position_enabled?
.ml-auto.mt-auto.mb-auto .ml-auto.mt-auto.mb-auto
#js-vue-sort-issue-discussions #js-vue-sort-issue-discussions
= render "projects/merge_requests/discussion_filter" = render "projects/merge_requests/discussion_filter"
.detail-page-description{ class: ("py-2" if mr_tabs_position_enabled?) } .detail-page-description.py-2
%h2.title.qa-title{ class: ("mb-0" if mr_tabs_position_enabled?) } %h2.title.qa-title.mb-0
= markdown_field(@merge_request, :title) = markdown_field(@merge_request, :title)
- unless mr_tabs_position_enabled?
= render "projects/merge_requests/description"
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
.alert.alert-danger .alert.alert-danger
The source project of this merge request has been removed. The source project of this merge request has been removed.
.detail-page-header{ class: ("border-bottom-0 pt-0 pb-0" if mr_tabs_position_enabled?) } .detail-page-header.border-bottom-0.pt-0.pb-0
.detail-page-header-body .detail-page-header-body
.issuable-status-box.status-box{ class: status_box_class(@merge_request) } .issuable-status-box.status-box{ class: status_box_class(@merge_request) }
= sprite_icon(state_icon_name, css_class: 'd-block d-sm-none') = sprite_icon(state_icon_name, css_class: 'd-block d-sm-none')
......
...@@ -17,20 +17,12 @@ ...@@ -17,20 +17,12 @@
.merge-request-details.issuable-details{ data: { id: @merge_request.project.id } } .merge-request-details.issuable-details{ data: { id: @merge_request.project.id } }
= render "projects/merge_requests/mr_box" = render "projects/merge_requests/mr_box"
- unless mr_tabs_position_enabled?
= render "projects/merge_requests/widget"
= render "projects/merge_requests/awards_block"
.merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') }
.merge-request-tabs-container .merge-request-tabs-container
%ul.merge-request-tabs.nav-tabs.nav.nav-links %ul.merge-request-tabs.nav-tabs.nav.nav-links
= render "projects/merge_requests/tabs/tab", class: "notes-tab", qa_selector: "notes_tab" do = render "projects/merge_requests/tabs/tab", class: "notes-tab", qa_selector: "notes_tab" do
= tab_link_for @merge_request, :show, force_link: @commit.present? do = tab_link_for @merge_request, :show, force_link: @commit.present? do
- if mr_tabs_position_enabled?
= _("Overview") = _("Overview")
- else
= _("Discussion")
%span.badge.badge-pill= @merge_request.related_notes.user.count %span.badge.badge-pill= @merge_request.related_notes.user.count
- if @merge_request.source_project - if @merge_request.source_project
= render "projects/merge_requests/tabs/tab", name: "commits", class: "commits-tab" do = render "projects/merge_requests/tabs/tab", name: "commits", class: "commits-tab" do
...@@ -46,11 +38,7 @@ ...@@ -46,11 +38,7 @@
= tab_link_for @merge_request, :diffs do = tab_link_for @merge_request, :diffs do
= _("Changes") = _("Changes")
%span.badge.badge-pill= @merge_request.diff_size %span.badge.badge-pill= @merge_request.diff_size
- if mr_tabs_position_enabled? && show_tabs_feature_highlight?
.js-tabs-feature-highlight{ data: { dismiss_endpoint: user_callouts_path, feature_id: UserCalloutsHelper::TABS_POSITION_HIGHLIGHT } }
.d-flex.flex-wrap.align-items-center.justify-content-lg-end .d-flex.flex-wrap.align-items-center.justify-content-lg-end
- unless mr_tabs_position_enabled?
= render "projects/merge_requests/discussion_filter"
#js-vue-discussion-counter #js-vue-discussion-counter
.tab-content#diff-notes-app .tab-content#diff-notes-app
...@@ -62,7 +50,6 @@ ...@@ -62,7 +50,6 @@
%section.col-md-12 %section.col-md-12
%script.js-notes-data{ type: "application/json" }= initial_notes_data(true).to_json.html_safe %script.js-notes-data{ type: "application/json" }= initial_notes_data(true).to_json.html_safe
.issuable-discussion.js-vue-notes-event .issuable-discussion.js-vue-notes-event
- if mr_tabs_position_enabled?
- if @merge_request.description.present? - if @merge_request.description.present?
.detail-page-description .detail-page-description
= render "projects/merge_requests/description" = render "projects/merge_requests/description"
......
---
title: Remove mr_tabs_position feature flag
merge_request: 29340
author: Lee Tickett
type: other
...@@ -16763,9 +16763,6 @@ msgstr "" ...@@ -16763,9 +16763,6 @@ msgstr ""
msgid "Novice" msgid "Novice"
msgstr "" msgstr ""
msgid "Now you can access the merge request navigation tabs at the top, where they’re easier to find."
msgstr ""
msgid "Nuget metadatum must have at least license_url, project_url or icon_url set" msgid "Nuget metadatum must have at least license_url, project_url or icon_url set"
msgstr "" msgstr ""
......
...@@ -7,10 +7,6 @@ module QA ...@@ -7,10 +7,6 @@ module QA
include Page::Component::Note include Page::Component::Note
include Page::Component::Issuable::Sidebar include Page::Component::Issuable::Sidebar
view 'app/assets/javascripts/mr_tabs_popover/components/popover.vue' do
element :dismiss_popover_button
end
view 'app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue' do view 'app/assets/javascripts/vue_merge_request_widget/components/mr_widget_header.vue' do
element :dropdown_toggle element :dropdown_toggle
element :download_email_patches element :download_email_patches
......
...@@ -8,7 +8,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -8,7 +8,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator } let(:user) { project.creator }
let(:enable_mr_tabs_position_flag) { true }
let(:config) do let(:config) do
{ {
...@@ -27,7 +26,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -27,7 +26,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
end end
before do before do
stub_feature_flags(mr_tabs_position: enable_mr_tabs_position_flag)
stub_application_setting(auto_devops_enabled: false) stub_application_setting(auto_devops_enabled: false)
stub_feature_flags(ci_merge_request_pipeline: true) stub_feature_flags(ci_merge_request_pipeline: true)
stub_ci_pipeline_yaml_file(YAML.dump(config)) stub_ci_pipeline_yaml_file(YAML.dump(config))
...@@ -54,8 +52,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -54,8 +52,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
.execute(:merge_request_event, merge_request: merge_request) .execute(:merge_request_event, merge_request: merge_request)
end end
let(:enable_mr_tabs_position_flag) { false }
before do before do
visit project_merge_request_path(project, merge_request) visit project_merge_request_path(project, merge_request)
...@@ -71,17 +67,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -71,17 +67,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
end end
end end
context 'when merge request tabs feature flag is disabled' do
it 'sees the latest detached merge request pipeline as the head pipeline', :sidekiq_might_not_need_inline do
page.within('.ci-widget-content') do
expect(page).to have_content("##{detached_merge_request_pipeline.id}")
end
end
end
context 'when merge request tabs feature flag is enabled' do
let(:enable_mr_tabs_position_flag) { true }
it 'sees the latest detached merge request pipeline as the head pipeline', :sidekiq_might_not_need_inline do it 'sees the latest detached merge request pipeline as the head pipeline', :sidekiq_might_not_need_inline do
click_link "Overview" click_link "Overview"
...@@ -89,7 +74,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -89,7 +74,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
expect(page).to have_content("##{detached_merge_request_pipeline.id}") expect(page).to have_content("##{detached_merge_request_pipeline.id}")
end end
end end
end
context 'when a user updated a merge request in the parent project', :sidekiq_might_not_need_inline do context 'when a user updated a merge request in the parent project', :sidekiq_might_not_need_inline do
let!(:push_pipeline_2) do let!(:push_pipeline_2) do
...@@ -145,6 +129,8 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -145,6 +129,8 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
end end
it 'sees the latest detached merge request pipeline as the head pipeline' do it 'sees the latest detached merge request pipeline as the head pipeline' do
click_link 'Overview'
page.within('.ci-widget-content') do page.within('.ci-widget-content') do
expect(page).to have_content("##{detached_merge_request_pipeline_2.id}") expect(page).to have_content("##{detached_merge_request_pipeline_2.id}")
end end
...@@ -153,6 +139,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -153,6 +139,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
context 'when a user merges a merge request in the parent project', :sidekiq_might_not_need_inline do context 'when a user merges a merge request in the parent project', :sidekiq_might_not_need_inline do
before do before do
click_link 'Overview'
click_button 'Merge when pipeline succeeds' click_button 'Merge when pipeline succeeds'
wait_for_requests wait_for_requests
...@@ -180,6 +167,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -180,6 +167,7 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
context 'when branch pipeline succeeds' do context 'when branch pipeline succeeds' do
before do before do
click_link 'Overview'
push_pipeline.succeed! push_pipeline.succeed!
wait_for_requests wait_for_requests
...@@ -215,6 +203,8 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -215,6 +203,8 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
end end
it 'sees the latest branch pipeline as the head pipeline', :sidekiq_might_not_need_inline do it 'sees the latest branch pipeline as the head pipeline', :sidekiq_might_not_need_inline do
click_link 'Overview'
page.within('.ci-widget-content') do page.within('.ci-widget-content') do
expect(page).to have_content("##{push_pipeline.id}") expect(page).to have_content("##{push_pipeline.id}")
end end
...@@ -261,7 +251,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -261,7 +251,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
end end
end end
context 'when merge request tabs feature flag is enabled' do
it 'sees the latest detached merge request pipeline as the head pipeline' do it 'sees the latest detached merge request pipeline as the head pipeline' do
click_link "Overview" click_link "Overview"
...@@ -269,17 +258,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request', ...@@ -269,17 +258,6 @@ RSpec.describe 'Merge request > User sees pipelines triggered by merge request',
expect(page).to have_content("##{detached_merge_request_pipeline.id}") expect(page).to have_content("##{detached_merge_request_pipeline.id}")
end end
end end
end
context 'when merge request tabs feature flag is disabled' do
let(:enable_mr_tabs_position_flag) { false }
it 'sees the latest detached merge request pipeline as the head pipeline' do
page.within('.ci-widget-content') do
expect(page).to have_content("##{detached_merge_request_pipeline.id}")
end
end
end
it 'sees pipeline list in forked project' do it 'sees pipeline list in forked project' do
visit project_pipelines_path(forked_project) visit project_pipelines_path(forked_project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment