Merge branch '336968-fj-add-administration-menu' into 'master'

Add and refactor Administration menu

See merge request gitlab-org/gitlab!68292

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

-= render_if_exists "groups/ee/administration_nav"
-
 = render 'shared/sidebar_toggle_button'

ee/app/helpers/ee/groups_helper.rb

@@ -4,14 +4,6 @@ module EE
   module GroupsHelper
     extend ::Gitlab::Utils::Override
 
-    def group_nav_link_paths
-      %w[saml_providers#show usage_quotas#index billings#index]
-    end
-
-    def group_administration_nav_link_paths
-      group_nav_link_paths
-    end
-
     def size_limit_message_for_group(group)
       show_lfs = group.lfs_enabled? ? 'including LFS files' : ''
 
@@ -64,30 +56,6 @@ module EE
       can?(current_user, :read_group_activity_analytics, @group)
     end
 
-    def show_usage_quotas_in_sidebar?
-      License.feature_available?(:usage_quotas)
-    end
-
-    def show_billing_in_sidebar?
-      ::Gitlab::CurrentSettings.should_check_namespace_plan?
-    end
-
-    def show_administration_nav?(group)
-      ::Feature.enabled?(:group_administration_nav_item, group) &&
-      group.parent.nil? &&
-      can?(current_user, :admin_group, group)
-    end
-
-    def administration_nav_path(group)
-      if show_saml_in_sidebar?(group)
-        group_saml_providers_path(group)
-      elsif show_usage_quotas_in_sidebar?
-        group_usage_quotas_path(group)
-      elsif show_billing_in_sidebar?
-        group_billings_path(group)
-      end
-    end
-
     def show_delayed_project_removal_setting?(group)
       group.licensed_feature_available?(:adjourned_deletion_for_projects_and_groups)
     end

ee/app/helpers/ee/saml_providers_helper.rb

@@ -2,10 +2,6 @@
 
 module EE
   module SamlProvidersHelper
-    def show_saml_in_sidebar?(group)
-      can?(current_user, :admin_group_saml, group)
-    end
-
     def saml_link_for_provider(text, provider, **args)
       saml_link(text, provider.group.full_path, **args)
     end

ee/app/views/groups/ee/_administration_nav.html.haml

-- return unless show_administration_nav?(@group)
-
-= nav_link(path: group_administration_nav_link_paths) do
-  = link_to administration_nav_path(@group) do
-    .nav-icon-container
-      = sprite_icon('admin')
-    %span.nav-item-name{ data: { qa_selector: 'group_administration_link' } }
-      = _('Administration')
-  %ul.sidebar-sub-level-items{ data: { qa_selector: 'group_sidebar_submenu_content' } }
-    = nav_link(path: group_administration_nav_link_paths, html_options: { class: "fly-out-top-item" } ) do
-      = link_to administration_nav_path(@group) do
-        %strong.fly-out-top-item-name
-          = _('Administration')
-    %li.divider.fly-out-top-item
-    - if show_saml_in_sidebar?(@group)
-      = nav_link(path: 'saml_providers#show') do
-        = link_to group_saml_providers_path(@group), title: _('SAML SSO'), data: { qa_selector: 'group_saml_sso_link' } do
-          %span
-            = _('SAML SSO')
-
-    - if show_usage_quotas_in_sidebar?
-      = nav_link(path: 'usage_quotas#index') do
-        = link_to group_usage_quotas_path(@group), title: s_('UsageQuota|Usage Quotas') do
-          %span
-            = s_('UsageQuota|Usage Quotas')
-
-    - if show_billing_in_sidebar?
-      = nav_link(path: 'billings#index') do
-        = link_to group_billings_path(@group), title: _('Billing') do
-          %span
-            = _('Billing')

ee/lib/ee/sidebars/groups/panel.rb

@@ -16,6 +16,7 @@ module EE
           insert_menu_after(::Sidebars::Groups::Menus::SecurityComplianceMenu, ::Sidebars::Groups::Menus::PushRulesMenu.new(context))
           insert_menu_after(::Sidebars::Groups::Menus::PackagesRegistriesMenu, ::Sidebars::Groups::Menus::AnalyticsMenu.new(context))
           insert_menu_after(::Sidebars::Groups::Menus::AnalyticsMenu, ::Sidebars::Groups::Menus::WikiMenu.new(context))
+          insert_menu_after(::Sidebars::Groups::Menus::SettingsMenu, ::Sidebars::Groups::Menus::AdministrationMenu.new(context))
         end
       end
     end

ee/lib/sidebars/groups/menus/administration_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Groups
+    module Menus
+      class AdministrationMenu < ::Sidebars::Menu
+        include Gitlab::Utils::StrongMemoize
+
+        override :configure_menu_items
+        def configure_menu_items
+          return false unless administration_menu_enabled?
+
+          add_item(saml_sso_menu_item)
+          add_item(usage_quotas_menu_item)
+          add_item(billing_menu_item)
+
+          true
+        end
+
+        override :link
+        def link
+          renderable_items.first.link
+        end
+
+        override :title
+        def title
+          _('Administration')
+        end
+
+        override :sprite_icon
+        def sprite_icon
+          'admin'
+        end
+
+        private
+
+        def administration_menu_enabled?
+          ::Feature.enabled?(:group_administration_nav_item, context.group) &&
+            context.group.root? &&
+            can?(context.current_user, :admin_group, context.group)
+        end
+
+        def saml_sso_menu_item
+          unless can?(context.current_user, :admin_group_saml, context.group)
+            return ::Sidebars::NilMenuItem.new(item_id: :saml_sso)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('SAML SSO'),
+            link: group_saml_providers_path(context.group),
+            active_routes: { path: 'saml_providers#show' },
+            item_id: :saml_sso
+          )
+        end
+
+        def usage_quotas_menu_item
+          unless usage_quotas_enabled?
+            return ::Sidebars::NilMenuItem.new(item_id: :usage_quotas)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: s_('UsageQuota|Usage Quotas'),
+            link: group_usage_quotas_path(context.group),
+            active_routes: { path: 'usage_quotas#index' },
+            item_id: :usage_quotas
+          )
+        end
+
+        def usage_quotas_enabled?
+          ::License.feature_available?(:usage_quotas) && context.group.root?
+        end
+
+        def billing_menu_item
+          unless ::Gitlab::CurrentSettings.should_check_namespace_plan?
+            return ::Sidebars::NilMenuItem.new(item_id: :billing)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Billing'),
+            link: group_billings_path(context.group),
+            active_routes: { path: 'billings#index' },
+            item_id: :billing
+          )
+        end
+      end
+    end
+  end
+end

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -267,124 +267,6 @@ RSpec.describe GroupsHelper do
     end
   end
 
-  describe '#show_usage_quotas_in_sidebar?' do
-    where(:usage_quotas_feature_available?, :expected) do
-      true  | true
-      false | false
-    end
-
-    with_them do
-      it do
-        stub_licensed_features(usage_quotas: usage_quotas_feature_available?)
-
-        expect(helper.show_usage_quotas_in_sidebar?).to eq(expected)
-      end
-    end
-  end
-
-  describe '#show_billing_in_sidebar?' do
-    where(:should_check_namespace_plan_return_value, :expected) do
-      true  | true
-      false | false
-    end
-
-    with_them do
-      it do
-        allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(should_check_namespace_plan_return_value)
-
-        expect(helper.show_billing_in_sidebar?).to eq(expected)
-      end
-    end
-  end
-
-  describe '#show_administration_nav?' do
-    context 'when user does not have admin_group permissions' do
-      before do
-        allow(helper).to receive(:can?).and_return(true)
-        allow(helper).to receive(:can?).with(current_user, :admin_group, group).and_return(false)
-      end
-
-      it 'returns false' do
-        expect(helper.show_administration_nav?(group)).to be false
-      end
-    end
-
-    context 'when user has admin_group permissions' do
-      before do
-        allow(helper).to receive(:can?).and_return(false)
-        allow(helper).to receive(:can?).with(current_user, :admin_group, group).and_return(true)
-      end
-
-      it 'returns true' do
-        allow(helper).to receive(:show_saml_in_sidebar?).with(group).and_return(true)
-
-        expect(helper.show_administration_nav?(group)).to be true
-      end
-
-      it 'returns false for a subgroup' do
-        subgroup = create(:group, :private, parent: group)
-
-        expect(helper.show_administration_nav?(subgroup)).to be false
-      end
-
-      context 'when `group_administration_nav_item` feature flag is enabled for another group' do
-        let(:another_group) { create(:group) }
-
-        before do
-          stub_feature_flags(group_administration_nav_item: another_group)
-        end
-
-        it 'returns false' do
-          expect(helper.show_administration_nav?(group)).to be false
-        end
-      end
-    end
-  end
-
-  describe '#administration_nav_path' do
-    context 'when SAML providers feature is available' do
-      before do
-        allow(helper).to receive(:show_saml_in_sidebar?).with(group).and_return(true)
-      end
-
-      it 'returns path to SAML providers' do
-        expect(helper.administration_nav_path(group)).to eq(group_saml_providers_path(group))
-      end
-    end
-
-    context 'when SAML providers feature is not available' do
-      before do
-        allow(helper).to receive(:show_saml_in_sidebar?).with(group).and_return(false)
-      end
-
-      context 'and usage quotas feature is available' do
-        before do
-          allow(helper).to receive(:show_usage_quotas_in_sidebar?).and_return(true)
-        end
-
-        it 'returns path to usage quotas' do
-          expect(helper.administration_nav_path(group)).to eq(group_usage_quotas_path(group))
-        end
-      end
-
-      context 'and usage quotas feature is not available' do
-        before do
-          allow(helper).to receive(:show_usage_quotas_in_sidebar?).and_return(false)
-        end
-
-        context 'and billing feature is available' do
-          before do
-            allow(helper).to receive(:show_billing_in_sidebar?).and_return(true)
-          end
-
-          it 'returns path to billing' do
-            expect(helper.administration_nav_path(group)).to eq(group_billings_path(group))
-          end
-        end
-      end
-    end
-  end
-
   describe '#show_delayed_project_removal_setting?' do
     before do
       stub_licensed_features(adjourned_deletion_for_projects_and_groups: licensed?)

ee/spec/helpers/ee/saml_providers_helper_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe EE::SamlProvidersHelper do
-  def stub_can(permission, value)
-    allow(helper).to receive(:can?).with(user, permission, group).and_return(value)
-  end
-
-  let_it_be(:group) { create(:group) }
-  let_it_be(:user) { create(:user) }
-
-  before do
-    allow(helper).to receive(:current_user).and_return(user)
-  end
-
-  describe '#show_saml_in_sidebar?' do
-    subject { helper.show_saml_in_sidebar?(group) }
-
-    context 'when the user can admin group saml' do
-      before do
-        stub_can(:admin_group_saml, true)
-      end
-
-      it { is_expected.to eq(true) }
-    end
-
-    context 'when the user cannot admin group saml' do
-      before do
-        stub_can(:admin_group_saml, false)
-      end
-
-      it { is_expected.to eq(false) }
-    end
-  end
-end

ee/spec/lib/sidebars/groups/menus/administration_menu_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Groups::Menus::AdministrationMenu do
+  let_it_be(:owner) { create(:user) }
+  let_it_be_with_refind(:parent_group) do
+    create(:group, :private).tap do |g|
+      g.add_owner(owner)
+    end
+  end
+
+  let_it_be_with_refind(:child_group) do
+    create(:group, :private, parent: parent_group).tap do |g|
+      g.add_owner(owner)
+    end
+  end
+
+  let(:group) { parent_group }
+  let(:user) { owner }
+  let(:context) { Sidebars::Groups::Context.new(current_user: user, container: group) }
+  let(:menu) { described_class.new(context) }
+
+  describe '#render?' do
+    subject { menu.render? }
+
+    context 'when feature flag :group_administration_nav_item is enabled' do
+      specify { is_expected.to be true }
+
+      context 'when group is a subgroup' do
+        let(:group) { child_group }
+
+        specify { is_expected.to be false }
+      end
+
+      context 'when user cannot admin group' do
+        let(:user) { nil }
+
+        specify { is_expected.to be false }
+      end
+    end
+
+    context 'when feature flag :group_administration_nav_item is disabled' do
+      specify do
+        stub_feature_flags(group_administration_nav_item: false)
+
+        is_expected.to be false
+      end
+    end
+  end
+
+  describe 'Menu items' do
+    subject { menu.renderable_items.index { |e| e.item_id == item_id } }
+
+    describe 'SAML SSO menu' do
+      let(:item_id) { :saml_sso }
+      let(:saml_enabled) { true }
+
+      before do
+        stub_licensed_features(group_saml: saml_enabled)
+        allow(::Gitlab::Auth::GroupSaml::Config).to receive(:enabled?).and_return(saml_enabled)
+      end
+
+      context 'when SAML is disabled' do
+        let(:saml_enabled) { false }
+
+        specify { is_expected.to be_nil }
+      end
+
+      context 'when SAML is enabled' do
+        specify { is_expected.to be_present }
+
+        context 'when user does not have access' do
+          let(:user) { nil }
+
+          specify { is_expected.to be_nil }
+        end
+      end
+    end
+
+    describe 'Usage quotas menu' do
+      let(:item_id) { :usage_quotas }
+      let(:usage_quotas_enabled) { true }
+
+      before do
+        stub_licensed_features(usage_quotas: usage_quotas_enabled)
+      end
+
+      specify { is_expected.to be_present }
+
+      context 'when user does not have access' do
+        let(:user) { nil }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+
+    describe 'Billing menu' do
+      let(:item_id) { :billing }
+      let(:check_billing) { true }
+
+      before do
+        allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(check_billing)
+      end
+
+      specify { is_expected.to be_present }
+
+      context 'when 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/_group.html.haml_spec.rb

@@ -655,4 +655,31 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
       expect(rendered).to have_link('Billing', href: group_billings_path(group))
     end
   end
+
+  describe 'Administration' do
+    before do
+      group.add_owner(user)
+
+      stub_licensed_features(usage_quotas: true)
+
+      allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(true)
+      allow(group).to receive(:feature_available?).and_call_original
+      allow(group).to receive(:feature_available?).with(:group_saml).and_return(true)
+      allow(view).to receive(:current_user).and_return(user)
+
+      render
+    end
+
+    it 'has a link to the SAML SSO settings page' do
+      expect(rendered).to have_link('SAML SSO', href: group_saml_providers_path(group))
+    end
+
+    it 'has a link to the Usage Quotas settings page' do
+      expect(rendered).to have_link('Usage Quotas', href: group_usage_quotas_path(group))
+    end
+
+    it 'has a link to the Billing settings page' do
+      expect(rendered).to have_link('Billing', href: group_billings_path(group))
+    end
+  end
 end

qa/qa/ee/page/group/menu.rb

@@ -12,12 +12,6 @@ module QA
 
             base.class_eval do
               prepend QA::Page::Group::SubMenus::Common
-
-              view 'ee/app/views/groups/ee/_administration_nav.html.haml' do
-                element :group_administration_link
-                element :group_sidebar_submenu_content
-                element :group_saml_sso_link
-              end
             end
           end
 
@@ -38,9 +32,9 @@ module QA
           end
 
           def go_to_saml_sso_group_settings
-            hover_element(:group_administration_link) do
-              within_submenu(:group_sidebar_submenu_content) do
-                click_element(:group_saml_sso_link)
+            hover_group_administration do
+              within_submenu do
+                click_element(:sidebar_menu_item_link, menu_item: 'SAML SSO')
               end
             end
           end
@@ -133,6 +127,15 @@ module QA
               yield
             end
           end
+
+          def hover_group_administration
+            within_sidebar do
+              scroll_to_element(:sidebar_menu_link, menu_item: 'Administration')
+              find_element(:sidebar_menu_link, menu_item: 'Administration').hover
+
+              yield
+            end
+          end
         end
       end
     end