Merge branch '326435-fj-add-new-analytics-menu' into 'master'

Add Analytics menu to project sidebar refactor

See merge request gitlab-org/gitlab!60599

app/helpers/analytics/navbar_helper.rb

@@ -13,14 +13,6 @@ module Analytics
       end
     end
 
-    def project_analytics_navbar_links(project, current_user)
-      [
-        cycle_analytics_navbar_link(project, current_user),
-        repository_analytics_navbar_link(project, current_user),
-        ci_cd_analytics_navbar_link(project, current_user)
-      ].compact
-    end
-
     def group_analytics_navbar_links(group, current_user)
       []
     end
@@ -30,39 +22,6 @@ module Analytics
     def navbar_sub_item(args)
       NavbarSubItem.new(**args)
     end
-
-    def cycle_analytics_navbar_link(project, current_user)
-      return unless project_nav_tab?(:cycle_analytics)
-
-      navbar_sub_item(
-        title: _('Value Stream'),
-        path: 'cycle_analytics#show',
-        link: project_cycle_analytics_path(project),
-        link_to_options: { class: 'shortcuts-project-cycle-analytics' }
-      )
-    end
-
-    def repository_analytics_navbar_link(project, current_user)
-      return if project.empty_repo?
-
-      navbar_sub_item(
-        title: _('Repository'),
-        path: 'graphs#charts',
-        link: charts_project_graph_path(project, current_ref),
-        link_to_options: { class: 'shortcuts-repository-charts' }
-      )
-    end
-
-    def ci_cd_analytics_navbar_link(project, current_user)
-      return unless project_nav_tab?(:pipelines)
-      return unless project.feature_available?(:builds, current_user) || !project.empty_repo?
-
-      navbar_sub_item(
-        title: _('CI/CD'),
-        path: 'pipelines#charts',
-        link: charts_project_pipelines_path(project)
-      )
-    end
   end
 end
 

app/views/layouts/nav/sidebar/_project_menus.html.haml

-- if project_nav_tab? :analytics
-  = render 'layouts/nav/sidebar/analytics_links', links: project_analytics_navbar_links(@project, current_user)
-
 - if project_nav_tab?(:confluence)
   - confluence_url = project_wikis_confluence_path(@project)
   = nav_link do

ee/app/helpers/ee/analytics/navbar_helper.rb

@@ -5,16 +5,6 @@ module EE
     module NavbarHelper
       extend ::Gitlab::Utils::Override
 
-      override :project_analytics_navbar_links
-      def project_analytics_navbar_links(project, current_user)
-        super + [
-          insights_navbar_link(project, current_user),
-          code_review_analytics_navbar_link(project, current_user),
-          project_issues_analytics_navbar_link(project, current_user),
-          project_merge_request_analytics_navbar_link(project, current_user)
-        ].compact
-      end
-
       override :group_analytics_navbar_links
       def group_analytics_navbar_links(group, current_user)
         super + [
@@ -32,27 +22,6 @@ module EE
 
       private
 
-      def project_issues_analytics_navbar_link(project, current_user)
-        return unless ::Feature.enabled?(:project_level_issues_analytics, project, default_enabled: true)
-        return unless project_nav_tab?(:issues_analytics)
-
-        navbar_sub_item(
-          title: _('Issue'),
-          path: 'issues_analytics#show',
-          link: project_analytics_issues_analytics_path(project)
-        )
-      end
-
-      def project_merge_request_analytics_navbar_link(project, current_user)
-        return unless project_nav_tab?(:merge_request_analytics)
-
-        navbar_sub_item(
-          title: _('Merge Request'),
-          path: 'projects/analytics/merge_request_analytics#show',
-          link: project_analytics_merge_request_analytics_path(project)
-        )
-      end
-
       # Currently an empty page, so don't show it on the navbar for now
       def group_merge_request_analytics_navbar_link(group, current_user)
         return
@@ -148,27 +117,6 @@ module EE
           link: group_analytics_repository_analytics_path(group)
         )
       end
-
-      def insights_navbar_link(project, current_user)
-        return unless project_nav_tab?(:project_insights)
-
-        navbar_sub_item(
-          title: _('Insights'),
-          path: 'insights#show',
-          link: project_insights_path(project),
-          link_to_options: { class: 'shortcuts-project-insights', data: { qa_selector: 'project_insights_link' } }
-        )
-      end
-
-      def code_review_analytics_navbar_link(project, current_user)
-        return unless project_nav_tab?(:code_review)
-
-        navbar_sub_item(
-          title: _('Code Review'),
-          path: 'projects/analytics/code_reviews#index',
-          link: project_analytics_code_reviews_path(project)
-        )
-      end
     end
   end
 end

ee/lib/ee/sidebars/projects/menus/analytics_menu.rb

+# frozen_string_literal: true
+
+module EE
+  module Sidebars
+    module Projects
+      module Menus
+        module AnalyticsMenu
+          extend ::Gitlab::Utils::Override
+
+          override :configure_menu_items
+          def configure_menu_items
+            return false unless can?(context.current_user, :read_analytics, context.project)
+
+            add_item(ci_cd_analytics_menu_item)
+            add_item(code_review_analytics_menu_item)
+            add_item(insights_menu_item)
+            add_item(issues_analytics_menu_item)
+            add_item(merge_request_analytics_menu_item)
+            add_item(repository_analytics_menu_item)
+            add_item(cycle_analytics_menu_item)
+
+            true
+          end
+
+          private
+
+          def insights_menu_item
+            return unless context.project.insights_available?
+
+            ::Sidebars::MenuItem.new(
+              title: _('Insights'),
+              link: project_insights_path(context.project),
+              active_routes: { path: 'insights#show' },
+              container_html_options: { class: 'shortcuts-project-insights' },
+              item_id: :insights
+            )
+          end
+
+          def code_review_analytics_menu_item
+            return unless can?(context.current_user, :read_code_review_analytics, context.project)
+
+            ::Sidebars::MenuItem.new(
+              title: _('Code Review'),
+              link: project_analytics_code_reviews_path(context.project),
+              active_routes: { path: 'projects/analytics/code_reviews#index' },
+              item_id: :code_review
+            )
+          end
+
+          def issues_analytics_menu_item
+            return unless ::Feature.enabled?(:project_level_issues_analytics, context.project, default_enabled: true)
+            return unless context.project.licensed_feature_available?(:issues_analytics)
+            return unless can?(context.current_user, :read_project, context.project)
+
+            ::Sidebars::MenuItem.new(
+              title: _('Issue'),
+              link: project_analytics_issues_analytics_path(context.project),
+              active_routes: { path: 'issues_analytics#show' },
+              item_id: :issues
+            )
+          end
+
+          def merge_request_analytics_menu_item
+            return unless can?(context.current_user, :read_project_merge_request_analytics, context.project)
+
+            ::Sidebars::MenuItem.new(
+              title: _('Merge Request'),
+              link: project_analytics_merge_request_analytics_path(context.project),
+              active_routes: { path: 'projects/analytics/merge_request_analytics#show' },
+              item_id: :merge_requests
+            )
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/ee/sidebars/projects/menus/analytics_menu_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do
+  let_it_be(:project) { create(:project, :repository) }
+
+  let(:user) { project.owner }
+  let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, current_ref: project.repository.root_ref) }
+
+  subject { described_class.new(context) }
+
+  describe 'Menu items' do
+    subject { described_class.new(context).items.index { |e| e.item_id == item_id } }
+
+    describe 'Code Review' do
+      let(:item_id) { :code_review }
+
+      specify { is_expected.not_to be_nil }
+
+      describe 'when the user does not have access' do
+        let(:user) { nil }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+
+    describe 'Insights' do
+      let(:item_id) { :insights }
+      let(:insights_available) { true }
+
+      before do
+        allow(project).to receive(:insights_available?).and_return(insights_available)
+      end
+
+      specify { is_expected.not_to be_nil }
+
+      context 'when insights are not available' do
+        let(:insights_available) { false }
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when the user does not have access' do
+        let(:user) { nil }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+
+    describe 'Issue' do
+      let(:item_id) { :issues }
+      let(:flag_enabled) { true }
+
+      before do
+        stub_licensed_features(issues_analytics: flag_enabled)
+      end
+
+      specify { is_expected.not_to be_nil }
+
+      describe 'when the user does not have access' do
+        let(:user) { nil }
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when feature flag :project_level_issues_analytics is not enabled' do
+        before do
+          stub_feature_flags(project_level_issues_analytics: false)
+        end
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when licensed feature issues analytics is not enabled' do
+        let(:flag_enabled) { false }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+
+    describe 'Merge Request' do
+      let(:item_id) { :merge_requests }
+
+      specify { is_expected.not_to be_nil }
+
+      describe 'when the user does not have access' do
+        let(:user) { nil }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+  end
+end

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

@@ -309,6 +309,96 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
   end
 
+  describe 'Analytics' do
+    before do
+      allow(view).to receive(:current_user).and_return(user)
+    end
+
+    describe 'Code Review' do
+      it 'has a link to the Code Review analytics page' do
+        render
+
+        expect(rendered).to have_link('Code Review', href: project_analytics_code_reviews_path(project))
+      end
+
+      context 'when user does not have access' do
+        let(:user) { nil }
+
+        it 'does not have a link to the Code Review analytics page' do
+          render
+
+          expect(rendered).not_to have_link('Code Review', href: project_analytics_code_reviews_path(project))
+        end
+      end
+    end
+
+    describe 'Insights' do
+      before do
+        stub_licensed_features(insights: true)
+      end
+
+      it 'has a link to the Insights analytics page' do
+        render
+
+        expect(rendered).to have_link('Insights', href: project_insights_path(project))
+      end
+
+      context 'when user does not have access' do
+        let(:user) { nil }
+
+        it 'does not have a link to the Insights analytics page' do
+          render
+
+          expect(rendered).not_to have_link('Insights', href: project_insights_path(project))
+        end
+      end
+    end
+
+    describe 'Issue' do
+      before do
+        stub_licensed_features(issues_analytics: true)
+      end
+
+      it 'has a link to the issue analytics page' do
+        render
+
+        expect(rendered).to have_link('Issue', href: project_analytics_issues_analytics_path(project))
+      end
+
+      context 'when user does not have access' do
+        let(:user) { nil }
+
+        it 'does not have a link to the issue analytics page' do
+          render
+
+          expect(rendered).not_to have_link('Issue', href: project_analytics_issues_analytics_path(project))
+        end
+      end
+    end
+
+    describe 'Merge Request' do
+      before do
+        stub_licensed_features(project_merge_request_analytics: true)
+      end
+
+      it 'has a link to the merge request analytics page' do
+        render
+
+        expect(rendered).to have_link('Merge Request', href: project_analytics_merge_request_analytics_path(project))
+      end
+
+      context 'when user does not have access' do
+        let(:user) { nil }
+
+        it 'does not have a link to the merge request analytics page' do
+          render
+
+          expect(rendered).not_to have_link('Merge Request', href: project_analytics_merge_request_analytics_path(project))
+        end
+      end
+    end
+  end
+
   describe 'Settings > Operations' do
     it 'is not visible when no valid license' do
       allow(view).to receive(:can?).and_return(true)

lib/sidebars/projects/menus/analytics_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Projects
+    module Menus
+      class AnalyticsMenu < ::Sidebars::Menu
+        include Gitlab::Utils::StrongMemoize
+
+        override :configure_menu_items
+        def configure_menu_items
+          return false unless can?(context.current_user, :read_analytics, context.project)
+
+          add_item(ci_cd_analytics_menu_item)
+          add_item(repository_analytics_menu_item)
+          add_item(cycle_analytics_menu_item)
+
+          true
+        end
+
+        override :link
+        def link
+          return cycle_analytics_menu_item.link if cycle_analytics_menu_item
+
+          items.first.link
+        end
+
+        override :extra_container_html_options
+        def extra_container_html_options
+          {
+            class: 'shortcuts-analytics'
+          }
+        end
+
+        override :title
+        def title
+          _('Analytics')
+        end
+
+        override :sprite_icon
+        def sprite_icon
+          'chart'
+        end
+
+        private
+
+        def ci_cd_analytics_menu_item
+          return if context.project.empty_repo?
+          return unless context.project.feature_available?(:builds, context.current_user)
+          return unless can?(context.current_user, :read_build, context.project)
+
+          ::Sidebars::MenuItem.new(
+            title: _('CI/CD'),
+            link: charts_project_pipelines_path(context.project),
+            active_routes: { path: 'pipelines#charts' },
+            item_id: :ci_cd_analytics
+          )
+        end
+
+        def repository_analytics_menu_item
+          return if context.project.empty_repo?
+
+          ::Sidebars::MenuItem.new(
+            title: _('Repository'),
+            link: charts_project_graph_path(context.project, context.current_ref),
+            container_html_options: { class: 'shortcuts-repository-charts' },
+            active_routes: { path: 'graphs#charts' },
+            item_id: :repository_analytics
+          )
+        end
+
+        def cycle_analytics_menu_item
+          strong_memoize(:cycle_analytics_menu_item) do
+            next unless can?(context.current_user, :read_cycle_analytics, context.project)
+
+            ::Sidebars::MenuItem.new(
+              title: _('Value Stream'),
+              link: project_cycle_analytics_path(context.project),
+              container_html_options: { class: 'shortcuts-project-cycle-analytics' },
+              active_routes: { path: 'cycle_analytics#show' },
+              item_id: :cycle_analytics
+            )
+          end
+        end
+      end
+    end
+  end
+end
+
+Sidebars::Projects::Menus::AnalyticsMenu.prepend_if_ee('EE::Sidebars::Projects::Menus::AnalyticsMenu')

lib/sidebars/projects/panel.rb

@@ -18,6 +18,7 @@ module Sidebars
         add_menu(Sidebars::Projects::Menus::SecurityComplianceMenu.new(context))
         add_menu(Sidebars::Projects::Menus::OperationsMenu.new(context))
         add_menu(Sidebars::Projects::Menus::PackagesRegistriesMenu.new(context))
+        add_menu(Sidebars::Projects::Menus::AnalyticsMenu.new(context))
       end
 
       override :render_raw_menus_partial

qa/qa/ee.rb

@@ -118,7 +118,7 @@ module QA
           autoload :SecurityCompliance, 'qa/ee/page/project/sub_menus/security_compliance'
           autoload :Repository, 'qa/ee/page/project/sub_menus/repository'
           autoload :Settings, 'qa/ee/page/project/sub_menus/settings'
-          autoload :Project, 'qa/ee/page/project/sub_menus/project'
+          autoload :Analytics, 'qa/ee/page/project/sub_menus/analytics'
           autoload :LicenseCompliance, 'qa/ee/page/project/sub_menus/license_compliance'
         end
 

qa/qa/ee/page/project/menu.rb

@@ -14,7 +14,7 @@ module QA
               prepend QA::Page::Project::SubMenus::Common
               prepend SubMenus::LicenseCompliance
               prepend SubMenus::SecurityCompliance
-              prepend SubMenus::Project
+              prepend SubMenus::Analytics
               prepend SubMenus::Repository
               prepend SubMenus::Settings
             end

qa/qa/ee/page/project/sub_menus/project.rb → qa/qa/ee/page/project/sub_menus/analytics.rb

@@ -5,14 +5,22 @@ module QA
     module Page
       module Project
         module SubMenus
-          module Project
+          module Analytics
             def click_project_insights_link
-              hover_element(:analytics_link) do
-                within_submenu(:analytics_sidebar_submenu) do
-                  click_element(:project_insights_link)
+              hover_analytics do
+                within_submenu do
+                  click_element(:sidebar_menu_item_link, menu_item: 'Insights')
                 end
               end
             end
+
+            def hover_analytics
+              within_sidebar do
+                find_element(:sidebar_menu_link, menu_item: 'Analytics').hover
+
+                yield
+              end
+            end
           end
         end
       end

spec/features/projects/navbar_spec.rb

@@ -34,7 +34,7 @@ RSpec.describe 'Project navbar' do
 
     it 'redirects to value stream when Analytics item is clicked' do
       page.within('.sidebar-top-level-items') do
-        find('[data-qa-selector=analytics_anchor]').click
+        find('.shortcuts-analytics').click
       end
 
       wait_for_requests

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do
+  let_it_be(:project) { create(:project, :repository) }
+
+  let(:user) { project.owner }
+  let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, current_ref: project.repository.root_ref) }
+
+  subject { described_class.new(context) }
+
+  describe '#render?' do
+    context 'whe user cannot read analytics' do
+      let(:user) { nil }
+
+      it 'returns false' do
+        expect(subject.render?).to be false
+      end
+    end
+
+    context 'whe user can read analytics' do
+      it 'returns true' do
+        expect(subject.render?).to be true
+      end
+
+      context 'when menu does not have any menu items' do
+        it 'returns false' do
+          allow(subject).to receive(:has_items?).and_return(false)
+
+          expect(subject.render?).to be false
+        end
+      end
+
+      context 'when menu has menu items' do
+        it 'returns true' do
+          expect(subject.render?).to be true
+        end
+      end
+    end
+  end
+
+  describe '#link' do
+    it 'returns link to the value stream page' do
+      expect(subject.link).to include('/-/value_stream_analytics')
+    end
+
+    context 'when Value Stream is not visible' do
+      it 'returns link to the the first visible menu item' do
+        allow(subject).to receive(:cycle_analytics_menu_item).and_return(nil)
+
+        expect(subject.link).to eq subject.items.first.link
+      end
+    end
+  end
+
+  describe 'Menu items' do
+    subject { described_class.new(context).items.index { |e| e.item_id == item_id } }
+
+    describe 'CI/CD' do
+      let(:item_id) { :ci_cd_analytics }
+
+      specify { is_expected.not_to be_nil }
+
+      describe 'when the project repository is empty' do
+        before do
+          allow(project).to receive(:empty_repo?).and_return(true)
+        end
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when builds access level is DISABLED' do
+        before do
+          project.project_feature.update!(builds_access_level: Featurable::DISABLED)
+        end
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when the user does not have access' do
+        let(:user) { nil }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+
+    describe 'Repository' do
+      let(:item_id) { :repository_analytics }
+
+      specify { is_expected.not_to be_nil }
+
+      describe 'when the project repository is empty' do
+        before do
+          allow(project).to receive(:empty_repo?).and_return(true)
+        end
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when the user does not have access' do
+        let(:user) { nil }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+
+    describe 'Value Stream' do
+      let(:item_id) { :cycle_analytics }
+
+      specify { is_expected.not_to be_nil }
+
+      describe 'when the user does not have access' do
+        let(:user) { nil }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+  end
+end

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

@@ -650,6 +650,68 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
   end
 
+  describe 'Analytics' do
+    it 'top level navigation link is visible points to the value stream page' do
+      render
+
+      expect(rendered).to have_link('Analytics', href: project_cycle_analytics_path(project))
+    end
+
+    describe 'CI/CD' do
+      it 'has a link to the CI/CD analytics page' do
+        render
+
+        expect(rendered).to have_link('CI/CD', href: charts_project_pipelines_path(project))
+      end
+
+      context 'when user does not have access' do
+        let(:user) { nil }
+
+        it 'does not have a link to the CI/CD analytics page' do
+          render
+
+          expect(rendered).not_to have_link('CI/CD', href: charts_project_pipelines_path(project))
+        end
+      end
+    end
+
+    describe 'Repository' do
+      it 'has a link to the repository analytics page' do
+        render
+
+        expect(rendered).to have_link('Repository', href: charts_project_graph_path(project, 'master'))
+      end
+
+      context 'when user does not have access' do
+        let(:user) { nil }
+
+        it 'does not have a link to the repository analytics page' do
+          render
+
+          expect(rendered).not_to have_link('Repository', href: charts_project_graph_path(project, 'master'))
+        end
+      end
+    end
+
+    describe 'Value Stream' do
+      it 'has a link to the value stream page' do
+        render
+
+        expect(rendered).to have_link('Value Stream', href: project_cycle_analytics_path(project))
+      end
+
+      context 'when user does not have access' do
+        let(:user) { nil }
+
+        it 'does not have a link to the value stream page' do
+          render
+
+          expect(rendered).not_to have_link('Value Stream', href: project_cycle_analytics_path(project))
+        end
+      end
+    end
+  end
+
   describe 'wiki entry tab' do
     let(:can_read_wiki) { true }
 
@@ -736,32 +798,6 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
   end
 
-  describe 'value stream analytics entry' do
-    let(:read_cycle_analytics) { true }
-
-    before do
-      allow(view).to receive(:can?).with(user, :read_cycle_analytics, project).and_return(read_cycle_analytics)
-    end
-
-    describe 'when value stream analytics is enabled' do
-      it 'shows the value stream analytics entry' do
-        render
-
-        expect(rendered).to have_link('Value Stream', href: project_cycle_analytics_path(project))
-      end
-    end
-
-    describe 'when value stream analytics is disabled' do
-      let(:read_cycle_analytics) { false }
-
-      it 'does not show the value stream analytics entry' do
-        render
-
-        expect(rendered).not_to have_link('Value Stream', href: project_cycle_analytics_path(project))
-      end
-    end
-  end
-
   describe 'operations settings tab' do
     describe 'archive projects' do
       before do