Merge branch '36017-move-analytics-out-of-more-in-top-navbar' into 'master'

Resolve "Move Analytics out of "More" in top navbar"

See merge request gitlab-org/gitlab!21112

app/helpers/dashboard_helper.rb

@@ -58,6 +58,10 @@ module DashboardHelper
       links += [:activity, :milestones]
     end
 
+    if can?(current_user, :read_instance_statistics)
+      links << :analytics
+    end
+
     links
   end
 end

app/views/layouts/instance_statistics.html.haml

-- page_title _('Instance Statistics')
-- header_title _('Instance Statistics'), instance_statistics_root_path
+- page_title _('Analytics')
+- header_title _('Analytics'), instance_statistics_root_path
 - nav 'instance_statistics'
 - @left_sidebar = true
 

app/views/layouts/nav/_analytics_link.html.haml

+- return unless dashboard_nav_link?(:analytics)
+= nav_link(controller: [:dev_ops_score, :cohorts], html_options: { class: "d-none d-xl-block"}) do
+  = link_to instance_statistics_root_path, class: 'chart-icon', title: _('Analytics'), aria: { label: _('Analytics') }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
+    = sprite_icon('chart', size: 18)

app/views/layouts/nav/_dashboard.html.haml

@@ -47,10 +47,7 @@
 
           %li.dropdown
             = render_if_exists 'dashboard/nav_link_list'
-          - if can?(current_user, :read_instance_statistics)
-            = nav_link(controller: [:dev_ops_score, :cohorts]) do
-              = link_to instance_statistics_root_path do
-                = _('Instance Statistics')
+
           - if current_user.admin?
             = nav_link(controller: 'admin/dashboard') do
               = link_to admin_root_path, class: 'admin-icon qa-admin-area-link d-xl-none' do
@@ -69,6 +66,8 @@
               = link_to sherlock_transactions_path, class: 'admin-icon' do
                 = _('Sherlock Transactions')
 
+  = render_if_exists 'layouts/nav/analytics_link'
+
   - if current_user.admin?
     = nav_link(controller: 'admin/dashboard', html_options: { class: "d-none d-xl-block"}) do
       = link_to admin_root_path, class: 'admin-icon qa-admin-area-link', title: _('Admin Area'), aria: { label: _('Admin Area') }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do

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

+- return unless dashboard_nav_link?(:analytics)
+= nav_link(controller: [:dev_ops_score, :cohorts]) do
+  = link_to instance_statistics_root_path, class: 'd-xl-none' do
+    = _('Analytics')

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

 .nav-sidebar{ class: ("sidebar-collapsed-desktop" if collapsed_sidebar?) }
   .nav-sidebar-inner-scroll
     .context-header
-      = link_to instance_statistics_root_path, title: _('Instance Statistics') do
+      = link_to instance_statistics_root_path, title: _('Analytics') do
         .avatar-container.s40.settings-avatar
           = sprite_icon('chart', size: 24)
-        .sidebar-context-title= _('Instance Statistics')
+        .sidebar-context-title= _('Analytics')
     %ul.sidebar-top-level-items
-      = nav_link(controller: :dev_ops_score) do
-        = link_to instance_statistics_dev_ops_score_index_path do
-          .nav-icon-container
-            = sprite_icon('comment')
-          %span.nav-item-name
-            = _('DevOps Score')
-        %ul.sidebar-sub-level-items.is-fly-out-only
-          = nav_link(controller: :dev_ops_score, html_options: { class: "fly-out-top-item" } ) do
-            = link_to instance_statistics_dev_ops_score_index_path do
-              %strong.fly-out-top-item-name
-                = _('DevOps Score')
-
-      - if Gitlab::CurrentSettings.usage_ping_enabled
-        = nav_link(controller: :cohorts) do
-          = link_to instance_statistics_cohorts_path do
-            .nav-icon-container
-              = sprite_icon('users')
-            %span.nav-item-name
-              = _('Cohorts')
-          %ul.sidebar-sub-level-items.is-fly-out-only
-            = nav_link(controller: :cohorts, html_options: { class: "fly-out-top-item" } ) do
-              = link_to instance_statistics_cohorts_path do
-                %strong.fly-out-top-item-name
-                  = _('Cohorts')
+      = render 'layouts/nav/sidebar/instance_statistics_links'
 
     = render 'shared/sidebar_toggle_button'

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

+- return unless dashboard_nav_link?(:analytics)
+= nav_link(controller: :dev_ops_score) do
+  = link_to instance_statistics_dev_ops_score_index_path do
+    .nav-icon-container
+      = sprite_icon('comment')
+    %span.nav-item-name
+      = _('DevOps Score')
+  %ul.sidebar-sub-level-items.is-fly-out-only
+    = nav_link(controller: :dev_ops_score, html_options: { class: "fly-out-top-item" } ) do
+      = link_to instance_statistics_dev_ops_score_index_path do
+        %strong.fly-out-top-item-name
+          = _('DevOps Score')
+
+- if Gitlab::CurrentSettings.usage_ping_enabled
+  = nav_link(controller: :cohorts) do
+    = link_to instance_statistics_cohorts_path do
+      .nav-icon-container
+        = sprite_icon('users')
+      %span.nav-item-name
+        = _('Cohorts')
+    %ul.sidebar-sub-level-items.is-fly-out-only
+      = nav_link(controller: :cohorts, html_options: { class: "fly-out-top-item" } ) do
+        = link_to instance_statistics_cohorts_path do
+          %strong.fly-out-top-item-name
+            = _('Cohorts')

changelogs/unreleased/36017-move-analytics-out-of-more-in-top-navbar.yml

+---
+title: Move instance statistics into analytics namespace
+merge_request: 21112
+author:
+type: changed

ee/app/helpers/ee/dashboard_helper.rb

@@ -31,6 +31,18 @@ module EE
       !current_license && current_user.admin?
     end
 
+    def analytics_nav_url
+      if ::Gitlab::Analytics.any_features_enabled?
+        return analytics_root_path
+      end
+
+      if can?(current_user, :read_instance_statistics)
+        return instance_statistics_root_path
+      end
+
+      'errors/not_found'
+    end
+
     private
 
     override :get_dashboard_nav_links

ee/app/helpers/ee/tab_helper.rb

@@ -5,7 +5,7 @@ module EE
     extend ::Gitlab::Utils::Override
 
     def analytics_controllers
-      ['analytics/productivity_analytics', 'analytics/cycle_analytics']
+      ['analytics/productivity_analytics', 'analytics/cycle_analytics', 'instance_statistics/dev_ops_score', 'instance_statistics/cohorts']
     end
   end
 end

ee/app/views/layouts/instance_statistics.html.haml

+- page_title _('Analytics')
+- header_title _('Analytics'), instance_statistics_root_path
+- nav 'analytics'
+- @left_sidebar = true
+
+= render template: 'layouts/application'

ee/app/views/layouts/nav/_analytics_link.html.haml

+- return unless dashboard_nav_link?(:analytics)
+= nav_link(controller: analytics_controllers, html_options: { class: "d-none d-xl-block"}) do
+  = link_to analytics_nav_url, class: 'chart-icon', title: _('Analytics'), aria: { label: _('Analytics') }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
+    = sprite_icon('chart', size: 18)

ee/app/views/layouts/nav/sidebar/_analytics.html.haml

@@ -3,7 +3,7 @@
     .context-header
       = link_to analytics_root_path, title: _('Analytics') do
         .avatar-container.s40.settings-avatar
-          = sprite_icon('log', size: 24)
+          = sprite_icon('chart', size: 24)
         .sidebar-context-title= _('Analytics')
     %ul.sidebar-top-level-items
       - if Gitlab::Analytics.productivity_analytics_enabled?
@@ -43,4 +43,7 @@
               = link_to analytics_code_analytics_path do
                 %strong.fly-out-top-item-name
                   = _('Code Analytics')
+
+      = render_ce 'layouts/nav/sidebar/instance_statistics_links'
+
     = render 'shared/sidebar_toggle_button'

ee/app/views/layouts/nav/sidebar/_analytics_link.html.haml

-- if dashboard_nav_link?(:analytics)
-  = nav_link(controller: analytics_controllers) do
-    = link_to analytics_root_path do
-      = _('Analytics')
-
+- return unless dashboard_nav_link?(:analytics)
+= nav_link(controller: analytics_controllers) do
+  = link_to analytics_root_path, class: 'd-xl-none' do
+    = _('Analytics')

ee/spec/features/dashboards/analytics_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Showing analytics' do
+  include AnalyticsHelpers
+
+  before do
+    sign_in user if user
+  end
+
+  # Using a path that is publicly accessible
+  subject { visit explore_projects_path }
+
+  context 'for regular users' do
+    let(:user) { create(:user) }
+
+    context 'with access to instance statistics or analytics features' do
+      it 'shows the analytics link' do
+        subject
+
+        expect(page).to have_link('Analytics')
+      end
+    end
+
+    context 'without access to instance statistics and analytics features' do
+      before do
+        disable_all_analytics_feature_flags
+        stub_application_setting(instance_statistics_visibility_private: true)
+      end
+
+      it 'does not show the analytics link' do
+        subject
+
+        expect(page).not_to have_link('Analytics')
+      end
+    end
+  end
+end

ee/spec/helpers/ee/dashboard_helper_spec.rb

@@ -32,8 +32,20 @@ describe DashboardHelper, type: :helper do
           disable_all_analytics_feature_flags
         end
 
-        it 'does not include analytics' do
-          expect(helper.dashboard_nav_links).not_to include(:analytics)
+        context 'and the user has no access to instance statistics features' do
+          before do
+            allow(helper).to receive(:can?) { false }
+          end
+
+          it 'does not include analytics' do
+            expect(helper.dashboard_nav_links).not_to include(:analytics)
+          end
+        end
+
+        context 'and the user has access to instance statistics features' do
+          it 'does include analytics' do
+            expect(helper.dashboard_nav_links).to include(:analytics)
+          end
         end
       end
     end
@@ -97,4 +109,42 @@ describe DashboardHelper, type: :helper do
       it { is_expected.to eq(output) }
     end
   end
+
+  describe 'analytics_nav_url' do
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    context 'when any analytics features are enabled' do
+      it 'returns the analytics root path' do
+        expect(helper.analytics_nav_url).to match(analytics_root_path)
+      end
+    end
+
+    context 'when analytics features are disabled' do
+      before do
+        disable_all_analytics_feature_flags
+      end
+
+      context 'and user has access to instance statistics features' do
+        before do
+          allow(helper).to receive(:can?) { true }
+        end
+
+        it 'returns the instance statistics root path' do
+          expect(helper.analytics_nav_url).to match(instance_statistics_root_path)
+        end
+      end
+
+      context 'and user does not have access to instance statistics features' do
+        before do
+          allow(helper).to receive(:can?) { false }
+        end
+
+        it 'returns the not found path' do
+          expect(helper.analytics_nav_url).to match('errors/not_found')
+        end
+      end
+    end
+  end
 end

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

@@ -16,7 +16,7 @@ describe 'layouts/nav/sidebar/_analytics' do
 
         expect(rendered).to have_content('Analytics')
         expect(rendered).to include(analytics_root_path)
-        expect(rendered).to match(/<use xlink:href=".+?icons-.+?#log">/)
+        expect(rendered).to match(/<use xlink:href=".+?icons-.+?#chart">/)
       end
 
       it 'has `Productivity Analytics` link' do
@@ -38,6 +38,35 @@ describe 'layouts/nav/sidebar/_analytics' do
         expect(rendered).to include(analytics_cycle_analytics_path)
         expect(rendered).to match(/<use xlink:href=".+?icons-.+?#repeat">/)
       end
+
+      context 'and user has access to instance statistics features' do
+        it 'has `DevOps Score` link' do
+          render
+
+          expect(rendered).to have_content('DevOps Score')
+          expect(rendered).to include(instance_statistics_dev_ops_score_index_path)
+          expect(rendered).to match(/<use xlink:href=".+?icons-.+?#comment">/)
+        end
+
+        it 'has `Cohorts` link' do
+          render
+
+          expect(rendered).to have_content('Cohorts')
+          expect(rendered).to include(instance_statistics_cohorts_path)
+          expect(rendered).to match(/<use xlink:href=".+?icons-.+?#users">/)
+        end
+      end
+
+      context 'and user does not have access to instance statistics features' do
+        it 'no instance statistics links are rendered' do
+          allow(view).to receive(:dashboard_nav_link?).and_return(false)
+
+          render
+
+          expect(rendered).not_to have_content('DevOps Score')
+          expect(rendered).not_to have_content('Cohorts')
+        end
+      end
     end
 
     context 'when feature flags are disabled' do
@@ -47,6 +76,35 @@ describe 'layouts/nav/sidebar/_analytics' do
         expect(rendered).not_to have_content('Productivity Analytics')
         expect(rendered).not_to have_content('Cycle Analytics')
       end
+
+      context 'and user has access to instance statistics features' do
+        it 'has `DevOps Score` link' do
+          render
+
+          expect(rendered).to have_content('DevOps Score')
+          expect(rendered).to include(instance_statistics_dev_ops_score_index_path)
+          expect(rendered).to match(/<use xlink:href=".+?icons-.+?#comment">/)
+        end
+
+        it 'has `Cohorts` link' do
+          render
+
+          expect(rendered).to have_content('Cohorts')
+          expect(rendered).to include(instance_statistics_cohorts_path)
+          expect(rendered).to match(/<use xlink:href=".+?icons-.+?#users">/)
+        end
+      end
+
+      context 'and user does not have access to instance statistics features' do
+        it 'no instance statistics links are rendered' do
+          allow(view).to receive(:dashboard_nav_link?).and_return(false)
+
+          render
+
+          expect(rendered).not_to have_content('DevOps Score')
+          expect(rendered).not_to have_content('Cohorts')
+        end
+      end
     end
   end
 end

locale/gitlab.pot

@@ -9776,9 +9776,6 @@ msgid_plural "Instances"
 msgstr[0] ""
 msgstr[1] ""
 
-msgid "Instance Statistics"
-msgstr ""
-
 msgid "Instance Statistics visibility"
 msgstr ""
 

spec/features/dashboard/instance_statistics_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe 'Showing instance statistics' do
+describe 'Showing analytics' do
   before do
     sign_in user if user
   end
@@ -13,10 +13,10 @@ describe 'Showing instance statistics' do
   context 'for unauthenticated users' do
     let(:user) { nil }
 
-    it 'does not show the instance statistics link' do
+    it 'does not show the Analytics link' do
       subject
 
-      expect(page).not_to have_link('Instance Statistics')
+      expect(page).not_to have_link('Analytics')
     end
   end
 
@@ -28,10 +28,10 @@ describe 'Showing instance statistics' do
         stub_application_setting(instance_statistics_visibility_private: false)
       end
 
-      it 'shows the instance statistics link' do
+      it 'shows the analytics link' do
         subject
 
-        expect(page).to have_link('Instance Statistics')
+        expect(page).to have_link('Analytics')
       end
     end
 
@@ -40,10 +40,14 @@ describe 'Showing instance statistics' do
         stub_application_setting(instance_statistics_visibility_private: true)
       end
 
-      it 'shows the instance statistics link' do
+      it 'does not show the analytics link' do
         subject
 
-        expect(page).not_to have_link('Instance Statistics')
+        # Skipping this test on EE as there is an EE specifc spec for this functionality
+        # ee/spec/features/dashboards/analytics_spec.rb
+        skip if Gitlab.ee?
+
+        expect(page).not_to have_link('Analytics')
       end
     end
   end
@@ -51,10 +55,10 @@ describe 'Showing instance statistics' do
   context 'for admins' do
     let(:user) { create(:admin) }
 
-    it 'shows the instance statistics link' do
+    it 'shows the analytics link' do
       subject
 
-      expect(page).to have_link('Instance Statistics')
+      expect(page).to have_link('Analytics')
     end
   end
 end