Merge branch '331336-fj-rearrange-security-menu' into 'master'

Rearrange 'Security & Compliance' menu items

See merge request gitlab-org/gitlab!62448

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

@@ -19,22 +19,12 @@ module EE
             add_item(license_compliance_menu_item)
             add_item(threat_monitoring_menu_item)
             add_item(scan_policies_menu_item)
-            add_item(configuration_menu_item)
             add_item(audit_events_menu_item)
+            add_item(configuration_menu_item)
 
             true
           end
 
-          override :link
-          def link
-            return discover_project_security_menu_item.link if discover_project_security_menu_item.render?
-            return security_dashboard_menu_item.link if security_dashboard_menu_item.render?
-            return audit_events_menu_item.link if audit_events_menu_item.render?
-            return dependencies_menu_item.link if dependencies_menu_item.render?
-
-            renderable_items.first&.link
-          end
-
           private
 
           override :configuration_menu_item_paths
@@ -58,143 +48,125 @@ module EE
           end
 
           def discover_project_security_menu_item
-            strong_memoize(:discover_project_security_menu_item) do
-              unless context.show_discover_project_security
-                next ::Sidebars::NilMenuItem.new(item_id: :discover_project_security)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: _('Discover'),
-                link: project_security_discover_path(context.project),
-                active_routes: { path: 'projects/security/discover#show' },
-                item_id: :discover_project_security
-              )
+            unless context.show_discover_project_security
+              return ::Sidebars::NilMenuItem.new(item_id: :discover_project_security)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: _('Discover'),
+              link: project_security_discover_path(context.project),
+              active_routes: { path: 'projects/security/discover#show' },
+              item_id: :discover_project_security
+            )
           end
 
           def security_dashboard_menu_item
-            strong_memoize(:security_dashboard_menu_item) do
-              unless can?(context.current_user, :read_project_security_dashboard, context.project)
-                next ::Sidebars::NilMenuItem.new(item_id: :dashboard)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: _('Security Dashboard'),
-                link: project_security_dashboard_index_path(context.project),
-                active_routes: { path: 'projects/security/dashboard#index' },
-                item_id: :dashboard
-              )
+            unless can?(context.current_user, :read_project_security_dashboard, context.project)
+              return ::Sidebars::NilMenuItem.new(item_id: :dashboard)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: _('Security Dashboard'),
+              link: project_security_dashboard_index_path(context.project),
+              active_routes: { path: 'projects/security/dashboard#index' },
+              item_id: :dashboard
+            )
           end
 
           def vulnerability_report_menu_item
-            strong_memoize(:vulnerability_report_menu_item) do
-              unless can?(context.current_user, :read_project_security_dashboard, context.project)
-                next ::Sidebars::NilMenuItem.new(item_id: :vulnerability_report)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: _('Vulnerability Report'),
-                link: project_security_vulnerability_report_index_path(context.project),
-                active_routes: { path: %w[projects/security/vulnerability_report#index projects/security/vulnerabilities#show] },
-                item_id: :vulnerability_report
-              )
+            unless can?(context.current_user, :read_project_security_dashboard, context.project)
+              return ::Sidebars::NilMenuItem.new(item_id: :vulnerability_report)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: _('Vulnerability Report'),
+              link: project_security_vulnerability_report_index_path(context.project),
+              active_routes: { path: %w[projects/security/vulnerability_report#index projects/security/vulnerabilities#show] },
+              item_id: :vulnerability_report
+            )
           end
 
           def on_demand_scans_menu_item
-            strong_memoize(:on_demand_scans_menu_item) do
-              unless can?(context.current_user, :read_on_demand_scans, context.project)
-                next ::Sidebars::NilMenuItem.new(item_id: :on_demand_scans)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: s_('OnDemandScans|On-demand Scans'),
-                link: new_project_on_demand_scan_path(context.project),
-                item_id: :on_demand_scans,
-                active_routes: { path: %w[
-                  projects/on_demand_scans#index
-                  projects/on_demand_scans#new
-                  projects/on_demand_scans#edit
-                ] }
-              )
+            unless can?(context.current_user, :read_on_demand_scans, context.project)
+              return ::Sidebars::NilMenuItem.new(item_id: :on_demand_scans)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: s_('OnDemandScans|On-demand Scans'),
+              link: new_project_on_demand_scan_path(context.project),
+              item_id: :on_demand_scans,
+              active_routes: { path: %w[
+                projects/on_demand_scans#index
+                projects/on_demand_scans#new
+                projects/on_demand_scans#edit
+              ] }
+            )
           end
 
           def dependencies_menu_item
-            strong_memoize(:dependencies_menu_item) do
-              unless can?(context.current_user, :read_dependencies, context.project)
-                next ::Sidebars::NilMenuItem.new(item_id: :dependency_list)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: _('Dependency List'),
-                link: project_dependencies_path(context.project),
-                active_routes: { path: 'projects/dependencies#index' },
-                item_id: :dependency_list
-              )
+            unless can?(context.current_user, :read_dependencies, context.project)
+              return ::Sidebars::NilMenuItem.new(item_id: :dependency_list)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: _('Dependency List'),
+              link: project_dependencies_path(context.project),
+              active_routes: { path: 'projects/dependencies#index' },
+              item_id: :dependency_list
+            )
           end
 
           def license_compliance_menu_item
-            strong_memoize(:license_compliance_menu_item) do
-              unless can?(context.current_user, :read_licenses, context.project)
-                next ::Sidebars::NilMenuItem.new(item_id: :license_compliance)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: _('License Compliance'),
-                link: project_licenses_path(context.project),
-                active_routes: { path: 'projects/licenses#index' },
-                item_id: :license_compliance
-              )
+            unless can?(context.current_user, :read_licenses, context.project)
+              return ::Sidebars::NilMenuItem.new(item_id: :license_compliance)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: _('License Compliance'),
+              link: project_licenses_path(context.project),
+              active_routes: { path: 'projects/licenses#index' },
+              item_id: :license_compliance
+            )
           end
 
           def threat_monitoring_menu_item
-            strong_memoize(:threat_monitoring_menu_item) do
-              unless can?(context.current_user, :read_threat_monitoring, context.project)
-                next ::Sidebars::NilMenuItem.new(item_id: :threat_monitoring)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: _('Threat Monitoring'),
-                link: project_threat_monitoring_path(context.project),
-                active_routes: { controller: ['projects/threat_monitoring'] },
-                item_id: :threat_monitoring
-              )
+            unless can?(context.current_user, :read_threat_monitoring, context.project)
+              return ::Sidebars::NilMenuItem.new(item_id: :threat_monitoring)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: _('Threat Monitoring'),
+              link: project_threat_monitoring_path(context.project),
+              active_routes: { controller: ['projects/threat_monitoring'] },
+              item_id: :threat_monitoring
+            )
           end
 
           def scan_policies_menu_item
-            strong_memoize(:scan_policies_menu_item) do
-              if ::Feature.disabled?(:security_orchestration_policies_configuration, context.project) ||
-                !can?(context.current_user, :security_orchestration_policies, context.project)
-                next ::Sidebars::NilMenuItem.new(item_id: :scan_policies)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: _('Scan Policies'),
-                link: project_security_policy_path(context.project),
-                active_routes: { controller: ['projects/security/policies'] },
-                item_id: :scan_policies
-              )
+            if ::Feature.disabled?(:security_orchestration_policies_configuration, context.project) ||
+              !can?(context.current_user, :security_orchestration_policies, context.project)
+              return ::Sidebars::NilMenuItem.new(item_id: :scan_policies)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: _('Scan Policies'),
+              link: project_security_policy_path(context.project),
+              active_routes: { controller: ['projects/security/policies'] },
+              item_id: :scan_policies
+            )
           end
 
           def audit_events_menu_item
-            strong_memoize(:audit_events_menu_item) do
-              unless show_audit_events?
-                next ::Sidebars::NilMenuItem.new(item_id: :audit_events)
-              end
-
-              ::Sidebars::MenuItem.new(
-                title: _('Audit Events'),
-                link: project_audit_events_path(context.project),
-                active_routes: { controller: :audit_events },
-                item_id: :audit_events
-              )
+            unless show_audit_events?
+              return ::Sidebars::NilMenuItem.new(item_id: :audit_events)
             end
+
+            ::Sidebars::MenuItem.new(
+              title: _('Audit Events'),
+              link: project_audit_events_path(context.project),
+              active_routes: { controller: :audit_events },
+              item_id: :audit_events
+            )
           end
 
           def show_audit_events?

ee/spec/features/projects/navbar_spec.rb

@@ -65,8 +65,8 @@ RSpec.describe 'Project navbar' do
             _('Security Dashboard'),
             _('Vulnerability Report'),
             s_('OnDemandScans|On-demand Scans'),
-            _('Configuration'),
-            _('Audit Events')
+            _('Audit Events'),
+            _('Configuration')
           ]
         }
       end

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

@@ -10,44 +10,20 @@ RSpec.describe Sidebars::Projects::Menus::SecurityComplianceMenu do
   let(:show_discover_project_security) { true }
   let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, show_promotions: show_promotions, show_discover_project_security: show_discover_project_security) }
 
-  subject(:menu) { described_class.new(context) }
-
-  describe 'render?' do
-    subject { menu.render? }
-
-    context 'when user is not authenticated' do
-      let(:user) { nil }
-
-      it { is_expected.to be_falsey }
-    end
-
-    context 'when user is authenticated' do
-      context 'when the Security & Compliance is disabled' do
-        before do
-          allow(Ability).to receive(:allowed?).with(user, :access_security_and_compliance, project).and_return(false)
-        end
-
-        it { is_expected.to be_falsey }
-      end
-
-      context 'when the Security & Compliance is not disabled' do
-        it { is_expected.to be_truthy }
-      end
-    end
-  end
-
   describe '#link' do
+    subject { described_class.new(context) }
+
     let(:show_promotions) { false }
     let(:show_discover_project_security) { false }
 
     using RSpec::Parameterized::TableSyntax
 
-    where(:security_dashboard_feature, :audit_events_feature, :dependency_scanning_feature, :show_discover_project_security, :expected_link) do
-      true  | true  | true  | false | "/-/security/dashboard"
-      false | true  | true  | false | "/-/audit_events"
-      false | false | true  | false | "/-/dependencies"
-      false | false | true  | true  | "/-/security/discover"
-      false | false | false | false | "/-/security/configuration"
+    where(:show_discover_project_security, :security_dashboard_feature, :dependency_scanning_feature, :audit_events_feature, :expected_link) do
+      true  | true  | true  | true  | '/-/security/discover'
+      false | true  | true  | true  | '/-/security/dashboard'
+      false | false | true  | true  | '/-/dependencies'
+      false | false | false | true  | '/-/audit_events'
+      false | false | false | false | '/-/security/configuration'
     end
 
     with_them do

lib/sidebars/projects/menus/security_compliance_menu.rb

@@ -17,7 +17,7 @@ module Sidebars
 
         override :link
         def link
-          project_security_configuration_path(context.project)
+          renderable_items.first&.link
         end
 
         override :title
@@ -33,18 +33,16 @@ module Sidebars
         private
 
         def configuration_menu_item
-          strong_memoize(:configuration_menu_item) do
-            unless render_configuration_menu_item?
-              next ::Sidebars::NilMenuItem.new(item_id: :configuration)
-            end
-
-            ::Sidebars::MenuItem.new(
-              title: _('Configuration'),
-              link: project_security_configuration_path(context.project),
-              active_routes: { path: configuration_menu_item_paths },
-              item_id: :configuration
-            )
+          unless render_configuration_menu_item?
+            return ::Sidebars::NilMenuItem.new(item_id: :configuration)
           end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Configuration'),
+            link: project_security_configuration_path(context.project),
+            active_routes: { path: configuration_menu_item_paths },
+            item_id: :configuration
+          )
         end
 
         def render_configuration_menu_item?

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Projects::Menus::SecurityComplianceMenu do
+  let_it_be(:project) { create(:project) }
+
+  let(:user) { project.owner }
+  let(:show_promotions) { true }
+  let(:show_discover_project_security) { true }
+  let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, show_promotions: show_promotions, show_discover_project_security: show_discover_project_security) }
+
+  describe 'render?' do
+    subject { described_class.new(context).render? }
+
+    context 'when user is not authenticated' do
+      let(:user) { nil }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when user is authenticated' do
+      context 'when the Security & Compliance is disabled' do
+        before do
+          allow(Ability).to receive(:allowed?).with(user, :access_security_and_compliance, project).and_return(false)
+        end
+
+        it { is_expected.to be_falsey }
+      end
+
+      context 'when the Security & Compliance is not disabled' do
+        it { is_expected.to be_truthy }
+      end
+    end
+  end
+end

spec/support/shared_contexts/navbar_structure_context.rb

@@ -18,8 +18,8 @@ RSpec.shared_context 'project navbar structure' do
     {
       nav_item: _('Security & Compliance'),
       nav_sub_items: [
-        _('Configuration'),
-        (_('Audit Events') if Gitlab.ee?)
+        (_('Audit Events') if Gitlab.ee?),
+        _('Configuration')
       ]
     }
   end