Move Labels menu item to project information menu

In this commit we move the menu item "Labels" from the Issues
menu and its own menu when the issues are disabled, to the
project information menu.

That way, we'll show always the labels menu no matter the status
of the issues feature.

ee/spec/features/projects/navbar_spec.rb

@@ -7,86 +7,90 @@ RSpec.describe 'Project navbar' do
 
   include_context 'project navbar structure'
 
-  let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, :repository) }
 
-  before do
-    stub_feature_flags(sidebar_refactor: false)
-    insert_package_nav(_('Operations'))
-    insert_infrastructure_registry_nav
-    stub_config(registry: { enabled: false })
+  let(:user) { project.owner }
 
-    project.add_maintainer(user)
+  before do
     sign_in(user)
   end
 
-  context 'when issue analytics is available' do
+  context 'when sidebar refactor feature flag is disabled' do
     before do
-      stub_licensed_features(issues_analytics: true)
+      stub_feature_flags(sidebar_refactor: false)
+      insert_package_nav(_('Operations'))
+      insert_infrastructure_registry_nav
+      stub_config(registry: { enabled: false })
 
       insert_after_sub_nav_item(
-        _('Code Review'),
-        within: _('Analytics'),
-        new_sub_nav_item_name: _('Issue')
+        _('Boards'),
+        within: _('Issues'),
+        new_sub_nav_item_name: _('Labels')
       )
-
-      visit project_path(project)
     end
 
-    it_behaves_like 'verified navigation bar'
-  end
+    context 'when issue analytics is available' do
+      before do
+        stub_licensed_features(issues_analytics: true)
 
-  context 'when security dashboard is available' do
-    let(:security_and_compliance_nav_item) do
-      {
-        nav_item: _('Security & Compliance'),
-        nav_sub_items: [
-          _('Security Dashboard'),
-          _('Vulnerability Report'),
-          s_('OnDemandScans|On-demand Scans'),
-          _('Configuration'),
-          _('Audit Events')
-        ]
-      }
-    end
+        insert_after_sub_nav_item(
+          _('Code Review'),
+          within: _('Analytics'),
+          new_sub_nav_item_name: _('Issue')
+        )
 
-    before do
-      stub_licensed_features(security_dashboard: true, security_on_demand_scans: true)
+        visit project_path(project)
+      end
 
-      visit project_path(project)
+      it_behaves_like 'verified navigation bar'
     end
 
-    it_behaves_like 'verified navigation bar'
-  end
+    context 'when security dashboard is available' do
+      let(:security_and_compliance_nav_item) do
+        {
+          nav_item: _('Security & Compliance'),
+          nav_sub_items: [
+            _('Security Dashboard'),
+            _('Vulnerability Report'),
+            s_('OnDemandScans|On-demand Scans'),
+            _('Configuration'),
+            _('Audit Events')
+          ]
+        }
+      end
 
-  context 'when packages are available' do
-    before do
-      stub_config(packages: { enabled: true }, registry: { enabled: false })
-      stub_feature_flags(sidebar_refactor: false)
+      before do
+        stub_licensed_features(security_dashboard: true, security_on_demand_scans: true)
 
-      visit project_path(project)
+        visit project_path(project)
+      end
+
+      it_behaves_like 'verified navigation bar'
     end
 
-    context 'when container registry is available' do
+    context 'when packages are available' do
       before do
-        stub_config(registry: { enabled: true })
-
-        insert_container_nav
+        stub_config(packages: { enabled: true }, registry: { enabled: false })
 
         visit project_path(project)
       end
 
-      it_behaves_like 'verified navigation bar'
-    end
-  end
+      context 'when container registry is available' do
+        before do
+          stub_config(registry: { enabled: true })
 
-  context 'when requirements is available' do
-    before do
-      stub_licensed_features(requirements: true)
+          insert_container_nav
+
+          visit project_path(project)
+        end
+
+        it_behaves_like 'verified navigation bar'
+      end
     end
 
-    context 'with flag disabled' do
+    context 'when requirements is available' do
       before do
+        stub_licensed_features(requirements: true)
         insert_after_nav_item(
           _('Merge requests'),
           new_nav_item: {
@@ -100,34 +104,59 @@ RSpec.describe 'Project navbar' do
 
       it_behaves_like 'verified navigation bar'
     end
+  end
+
+  context 'when sidebar refactor feature flag is enabled' do
+    let(:operations_menu_items) do
+      [
+        _('Metrics'),
+        _('Logs'),
+        _('Tracing'),
+        _('Error Tracking'),
+        _('Alerts'),
+        _('Incidents'),
+        _('Environments'),
+        _('Feature Flags'),
+        _('Product Analytics')
+      ]
+    end
 
-    context 'with flag enabled' do
-      let(:operations_menu_items) do
-        [
-          _('Metrics'),
-          _('Logs'),
-          _('Tracing'),
-          _('Error Tracking'),
-          _('Alerts'),
-          _('Incidents'),
-          _('Environments'),
-          _('Feature Flags'),
-          _('Product Analytics')
+    let(:project_information_nav_item) do
+      {
+        nav_item: _('Project information'),
+        nav_sub_items: [
+          _('Activity'),
+          _('Releases'),
+          _('Labels')
         ]
-      end
+      }
+    end
 
-      let(:project_information_nav_item) do
-        {
-          nav_item: _('Project information'),
+    before do
+      stub_feature_flags(sidebar_refactor: true)
+      insert_package_nav(_('Operations'))
+      insert_infrastructure_registry_nav
+
+      insert_after_nav_item(
+        _('Operations'),
+        new_nav_item: {
+          nav_item: _('Infrastructure'),
           nav_sub_items: [
-            _('Activity'),
-            _('Releases')
+            _('Kubernetes clusters'),
+            _('Serverless platform'),
+            _('Terraform')
           ]
         }
-      end
+      )
+
+      visit project_path(project)
+    end
+
+    it_behaves_like 'verified navigation bar'
 
+    context 'when requirements is available' do
       before do
-        stub_feature_flags(sidebar_refactor: true)
+        stub_licensed_features(requirements: true)
 
         insert_after_nav_item(
           _('Merge requests'),
@@ -137,18 +166,6 @@ RSpec.describe 'Project navbar' do
           }
         )
 
-        insert_after_nav_item(
-          _('Operations'),
-          new_nav_item: {
-            nav_item: _('Infrastructure'),
-            nav_sub_items: [
-              _('Kubernetes clusters'),
-              _('Serverless platform'),
-              _('Terraform')
-            ]
-          }
-        )
-
         visit project_path(project)
       end
 

lib/sidebars/projects/menus/issues_menu.rb

@@ -98,6 +98,10 @@ module Sidebars
         end
 
         def labels_menu_item
+          if Feature.enabled?(:sidebar_refactor, context.current_user)
+            return ::Sidebars::NilMenuItem.new(item_id: :labels)
+          end
+
           ::Sidebars::MenuItem.new(
             title: _('Labels'),
             link: project_labels_path(context.project),

lib/sidebars/projects/menus/labels_menu.rb

@@ -40,6 +40,8 @@ module Sidebars
 
         override :render?
         def render?
+          return false if Feature.enabled?(:sidebar_refactor, context.current_user)
+
           can?(context.current_user, :read_label, context.project) && !context.project.issues_enabled?
         end
       end

lib/sidebars/projects/menus/project_information_menu.rb

@@ -9,6 +9,7 @@ module Sidebars
           add_item(details_menu_item)
           add_item(activity_menu_item)
           add_item(releases_menu_item)
+          add_item(labels_menu_item)
 
           true
         end
@@ -95,6 +96,19 @@ module Sidebars
             container_html_options: { class: 'shortcuts-project-releases' }
           )
         end
+
+        def labels_menu_item
+          if Feature.disabled?(:sidebar_refactor, context.current_user)
+            return ::Sidebars::NilMenuItem.new(item_id: :labels)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Labels'),
+            link: project_labels_path(context.project),
+            active_routes: { controller: :labels },
+            item_id: :labels
+          )
+        end
       end
     end
   end

spec/features/projects/active_tabs_spec.rb

@@ -69,20 +69,37 @@ RSpec.describe 'Project active tab' do
   end
 
   context 'on project Issues' do
+    let(:feature_flag_value) { true }
+
     before do
+      stub_feature_flags(sidebar_refactor: feature_flag_value)
+
       visit project_issues_path(project)
     end
 
     it_behaves_like 'page has active tab', 'Issues'
 
-    %w(Milestones Labels).each do |sub_menu|
-      context "on project Issues/#{sub_menu}" do
-        before do
-          click_tab(sub_menu)
-        end
+    context "on project Issues/Milestones" do
+      before do
+        click_tab('Milestones')
+      end
 
-        it_behaves_like 'page has active tab', 'Issues'
-        it_behaves_like 'page has active sub tab', sub_menu
+      it_behaves_like 'page has active tab', 'Issues'
+      it_behaves_like 'page has active sub tab', 'Milestones'
+    end
+
+    context 'when feature flag is disabled' do
+      let(:feature_flag_value) { false }
+
+      %w(Milestones Labels).each do |sub_menu|
+        context "on project Issues/#{sub_menu}" do
+          before do
+            click_tab(sub_menu)
+          end
+
+          it_behaves_like 'page has active tab', 'Issues'
+          it_behaves_like 'page has active sub tab', sub_menu
+        end
       end
     end
   end

spec/features/projects/features_visibility_spec.rb

@@ -54,17 +54,30 @@ RSpec.describe 'Edit Project Settings' do
     end
 
     context 'When external issue tracker is enabled and issues disabled on project settings' do
-      it 'hides issues tab and show labels tab' do
+      before do
         project.issues_enabled = false
         project.save!
         allow_next_instance_of(Project) do |instance|
           allow(instance).to receive(:external_issue_tracker).and_return(JiraService.new)
         end
+      end
 
+      it 'hides issues tab' do
         visit project_path(project)
 
         expect(page).not_to have_selector('.shortcuts-issues')
-        expect(page).to have_selector('.shortcuts-labels')
+        expect(page).not_to have_selector('.shortcuts-labels')
+      end
+
+      context 'when feature flag :sidebar_refactor is disabled' do
+        it 'hides issues tab and show labels tab' do
+          stub_feature_flags(sidebar_refactor: false)
+
+          visit project_path(project)
+
+          expect(page).not_to have_selector('.shortcuts-issues')
+          expect(page).to have_selector('.shortcuts-labels')
+        end
       end
     end
 

spec/features/projects/navbar_spec.rb

@@ -8,70 +8,81 @@ RSpec.describe 'Project navbar' do
 
   include_context 'project navbar structure'
 
-  let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project, :repository) }
 
-  before do
-    stub_feature_flags(sidebar_refactor: false)
-    insert_package_nav(_('Operations'))
-    insert_infrastructure_registry_nav
-    stub_config(registry: { enabled: false })
+  let(:user) { project.owner }
 
-    project.add_maintainer(user)
+  before do
     sign_in(user)
   end
 
-  it_behaves_like 'verified navigation bar' do
+  context 'when sidebar refactor feature flag is disabled' do
     before do
-      visit project_path(project)
+      stub_feature_flags(sidebar_refactor: false)
+      insert_package_nav(_('Operations'))
+      insert_infrastructure_registry_nav
+
+      insert_after_sub_nav_item(
+        _('Boards'),
+        within: _('Issues'),
+        new_sub_nav_item_name: _('Labels')
+      )
+
+      stub_config(registry: { enabled: false })
     end
-  end
 
-  context 'when value stream is available' do
-    before do
-      visit project_path(project)
+    it_behaves_like 'verified navigation bar' do
+      before do
+        visit project_path(project)
+      end
     end
 
-    it 'redirects to value stream when Analytics item is clicked' do
-      page.within('.sidebar-top-level-items') do
-        find('.shortcuts-analytics').click
+    context 'when value stream is available' do
+      before do
+        visit project_path(project)
       end
 
-      wait_for_requests
+      it 'redirects to value stream when Analytics item is clicked' do
+        page.within('.sidebar-top-level-items') do
+          find('.shortcuts-analytics').click
+        end
+
+        wait_for_requests
 
-      expect(page).to have_current_path(project_cycle_analytics_path(project))
+        expect(page).to have_current_path(project_cycle_analytics_path(project))
+      end
     end
-  end
 
-  context 'when pages are available' do
-    before do
-      stub_config(pages: { enabled: true })
+    context 'when pages are available' do
+      before do
+        stub_config(pages: { enabled: true })
 
-      insert_after_sub_nav_item(
-        _('Operations'),
-        within: _('Settings'),
-        new_sub_nav_item_name: _('Pages')
-      )
+        insert_after_sub_nav_item(
+          _('Operations'),
+          within: _('Settings'),
+          new_sub_nav_item_name: _('Pages')
+        )
 
-      visit project_path(project)
+        visit project_path(project)
+      end
+
+      it_behaves_like 'verified navigation bar'
     end
 
-    it_behaves_like 'verified navigation bar'
-  end
+    context 'when container registry is available' do
+      before do
+        stub_config(registry: { enabled: true })
 
-  context 'when container registry is available' do
-    before do
-      stub_config(registry: { enabled: true })
+        insert_container_nav
 
-      insert_container_nav
+        visit project_path(project)
+      end
 
-      visit project_path(project)
+      it_behaves_like 'verified navigation bar'
     end
-
-    it_behaves_like 'verified navigation bar'
   end
 
-  context 'when sidebar refactor feature flag is on' do
+  context 'when sidebar refactor feature flag is enabled' do
     let(:operations_menu_items) do
       [
         _('Metrics'),
@@ -91,7 +102,8 @@ RSpec.describe 'Project navbar' do
         nav_item: _('Project information'),
         nav_sub_items: [
           _('Activity'),
-          _('Releases')
+          _('Releases'),
+          _('Labels')
         ]
       }
     end
@@ -99,7 +111,8 @@ RSpec.describe 'Project navbar' do
     before do
       stub_feature_flags(sidebar_refactor: true)
       stub_config(registry: { enabled: true })
-
+      insert_package_nav(_('Operations'))
+      insert_infrastructure_registry_nav
       insert_container_nav
 
       insert_after_sub_nav_item(

spec/lib/sidebars/projects/menus/issues_menu_spec.rb

@@ -65,4 +65,22 @@ RSpec.describe Sidebars::Projects::Menus::IssuesMenu do
       end
     end
   end
+
+  describe 'Menu Items' do
+    subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
+
+    describe 'Labels' do
+      let(:item_id) { :labels }
+
+      specify { is_expected.to be_nil }
+
+      context 'when feature flag :sidebar_refactor is disabled' do
+        before do
+          stub_feature_flags(sidebar_refactor: false)
+        end
+
+        specify { is_expected.not_to be_nil }
+      end
+    end
+  end
 end

spec/lib/sidebars/projects/menus/labels_menu_spec.rb

@@ -20,27 +20,41 @@ RSpec.describe Sidebars::Projects::Menus::LabelsMenu do
       allow(project).to receive(:issues_enabled?).and_return(issues_enabled)
     end
 
-    context 'when user can read labels' do
-      context 'when issues feature is enabled' do
-        it 'returns false' do
-          expect(subject.render?).to be_falsey
-        end
+    context 'when feature flag :sidebar_refactor is enabled' do
+      let(:issues_enabled) { false }
+
+      it 'returns false' do
+        expect(subject.render?).to be_falsey
       end
+    end
 
-      context 'when issues feature is disabled' do
-        let(:issues_enabled) { false }
+    context 'when feature flag :sidebar_refactor is disabled' do
+      before do
+        stub_feature_flags(sidebar_refactor: false)
+      end
 
-        it 'returns true' do
-          expect(subject.render?).to be_truthy
+      context 'when user can read labels' do
+        context 'when issues feature is enabled' do
+          it 'returns false' do
+            expect(subject.render?).to be_falsey
+          end
+        end
+
+        context 'when issues feature is disabled' do
+          let(:issues_enabled) { false }
+
+          it 'returns true' do
+            expect(subject.render?).to be_truthy
+          end
         end
       end
-    end
 
-    context 'when user cannot read labels' do
-      let(:user) { nil }
+      context 'when user cannot read labels' do
+        let(:user) { nil }
 
-      it 'returns false' do
-        expect(subject.render?).to be_falsey
+        it 'returns false' do
+          expect(subject.render?).to be_falsey
+        end
       end
     end
   end

spec/lib/sidebars/projects/menus/project_information_menu_spec.rb

@@ -8,30 +8,48 @@ RSpec.describe Sidebars::Projects::Menus::ProjectInformationMenu do
   let(:user) { project.owner }
   let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) }
 
-  describe 'Releases' do
-    subject { described_class.new(context).renderable_items.index { |e| e.item_id == :releases } }
+  describe 'Menu Items' do
+    subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
 
-    context 'when project repository is empty' do
-      it 'does not include releases menu item' do
-        allow(project).to receive(:empty_repo?).and_return(true)
+    describe 'Releases' do
+      let(:item_id) { :releases }
 
-        is_expected.to be_nil
+      context 'when project repository is empty' do
+        it 'does not include releases menu item' do
+          allow(project).to receive(:empty_repo?).and_return(true)
+
+          is_expected.to be_nil
+        end
       end
-    end
 
-    context 'when project repository is not empty' do
-      context 'when user can download code' do
-        it 'includes releases menu item' do
-          is_expected.to be_present
+      context 'when project repository is not empty' do
+        context 'when user can download code' do
+          it 'includes releases menu item' do
+            is_expected.to be_present
+          end
+        end
+
+        context 'when user cannot download code' do
+          let(:user) { nil }
+
+          it 'does not include releases menu item' do
+            is_expected.to be_nil
+          end
         end
       end
+    end
 
-      context 'when user cannot download code' do
-        let(:user) { nil }
+    describe 'Labels' do
+      let(:item_id) { :labels }
 
-        it 'does not include releases menu item' do
-          is_expected.to be_nil
+      specify { is_expected.not_to be_nil }
+
+      context 'when feature flag :sidebar_refactor is disabled' do
+        before do
+          stub_feature_flags(sidebar_refactor: false)
         end
+
+        specify { is_expected.to be_nil }
       end
     end
   end

spec/support/shared_contexts/navbar_structure_context.rb

@@ -73,7 +73,6 @@ RSpec.shared_context 'project navbar structure' do
         nav_sub_items: [
           _('List'),
           _('Boards'),
-          _('Labels'),
           _('Service Desk'),
           _('Milestones'),
           (_('Iterations') if Gitlab.ee?)

spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb

@@ -72,6 +72,27 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
         expect(rendered).to have_link('Releases', href: project_releases_path(project), class: 'shortcuts-project-releases')
       end
     end
+
+    describe 'Labels' do
+      let(:page) { Nokogiri::HTML.parse(rendered) }
+
+      it 'has a link to the labels path' do
+        render
+
+        expect(page.at_css('.shortcuts-project').parent.css('[aria-label="Labels"]')).not_to be_empty
+        expect(rendered).to have_link('Labels', href: project_labels_path(project))
+      end
+
+      context 'when feature flag :sidebar_refactor is disabled' do
+        it 'does not have the labels menu item' do
+          stub_feature_flags(sidebar_refactor: false)
+
+          render
+
+          expect(page.at_css('.shortcuts-project').parent.css('[aria-label="Labels"]')).to be_empty
+        end
+      end
+    end
   end
 
   describe 'Learn GitLab' do
@@ -181,10 +202,23 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
 
     describe 'Labels' do
-      it 'has a link to the labels path' do
+      let(:page) { Nokogiri::HTML.parse(rendered) }
+
+      it 'does not have a link to the labels page' do
         render
 
-        expect(rendered).to have_link('Labels', href: project_labels_path(project))
+        expect(page.at_css('.shortcuts-issues').parent.css('[aria-label="Labels"]')).to be_empty
+      end
+
+      context 'when feature flag :sidebar_refactor is disabled' do
+        it 'has a link to the labels page' do
+          stub_feature_flags(sidebar_refactor: false)
+
+          render
+
+          expect(page.at_css('.shortcuts-issues').parent.css('[aria-label="Labels"]')).not_to be_empty
+          expect(rendered).to have_link('Labels', href: project_labels_path(project))
+        end
       end
     end
 
@@ -248,21 +282,35 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
   end
 
   describe 'Labels' do
-    context 'when issues are not enabled' do
-      it 'has a link to the labels path' do
-        project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
+    it 'does not show the labels menu' do
+      project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
 
-        render
+      render
 
-        expect(rendered).to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels')
-      end
+      expect(rendered).not_to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels')
     end
 
-    context 'when issues are enabled' do
-      it 'does not have a link to the labels path' do
-        render
+    context 'when feature flag :sidebar_refactor is disabled' do
+      before do
+        stub_feature_flags(sidebar_refactor: false)
+      end
+
+      context 'when issues are not enabled' do
+        it 'has a link to the labels path' do
+          project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
 
-        expect(rendered).not_to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels')
+          render
+
+          expect(rendered).to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels')
+        end
+      end
+
+      context 'when issues are enabled' do
+        it 'does not have a link to the labels path' do
+          render
+
+          expect(rendered).not_to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels')
+        end
       end
     end
   end