Merge branch '326439-fj-add-settings-menu' into 'master'

Add Settings menu to project sidebar refactor

See merge request gitlab-org/gitlab!60708

app/helpers/projects_helper.rb

@@ -285,10 +285,6 @@ module ProjectsHelper
     !disabled && !compact_mode
   end
 
-  def settings_operations_available?
-    !@project.archived? && can?(current_user, :admin_operations, @project)
-  end
-
   def error_tracking_setting_project_json
     setting = @project.error_tracking_setting
 
@@ -666,26 +662,6 @@ module ProjectsHelper
     "#{request.path}?#{options.to_param}"
   end
 
-  def sidebar_settings_paths
-    %w[
-      projects#edit
-      integrations#show
-      services#edit
-      hooks#index
-      hooks#edit
-      access_tokens#index
-      hook_logs#show
-      repository#show
-      ci_cd#show
-      operations#show
-      badges#index
-      pages#show
-      packages_and_registries#show
-      projects/runners#show
-      projects/runners#edit
-    ]
-  end
-
   def sidebar_operations_paths
     %w[
       environments

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

-- if project_nav_tab? :settings
-  = nav_link(path: sidebar_settings_paths) do
-    = link_to edit_project_path(@project) do
-      .nav-icon-container
-        = sprite_icon('settings')
-      %span.nav-item-name.qa-settings-item#js-onboarding-settings-link
-        = _('Settings')
-
-    %ul.sidebar-sub-level-items
-      - can_edit = can?(current_user, :admin_project, @project)
-      - if can_edit
-        = nav_link(path: sidebar_settings_paths, html_options: { class: "fly-out-top-item" } ) do
-          = link_to edit_project_path(@project) do
-            %strong.fly-out-top-item-name
-              = _('Settings')
-        %li.divider.fly-out-top-item
-        = nav_link(path: %w[projects#edit]) do
-          = link_to edit_project_path(@project), title: _('General'), class: 'qa-general-settings-link' do
-            %span
-              = _('General')
-      - if can_edit
-        = nav_link(controller: [:integrations, :services]) do
-          = link_to project_settings_integrations_path(@project), title: _('Integrations'), data: { qa_selector: 'integrations_settings_link' } do
-            %span
-              = _('Integrations')
-        = nav_link(controller: [:hooks, :hook_logs]) do
-          = link_to project_hooks_path(@project), title: _('Webhooks'), data: { qa_selector: 'webhooks_settings_link' } do
-            %span
-              = _('Webhooks')
-        - if can?(current_user, :read_resource_access_tokens, @project)
-          = nav_link(controller: [:access_tokens]) do
-            = link_to project_settings_access_tokens_path(@project), title: _('Access Tokens'), data: { qa_selector: 'access_tokens_settings_link' } do
-              %span
-                = _('Access Tokens')
-        = nav_link(controller: :repository) do
-          = link_to project_settings_repository_path(@project), title: _('Repository') do
-            %span
-              = _('Repository')
-        - if !@project.archived? && @project.feature_available?(:builds, current_user)
-          = nav_link(controller: [:ci_cd, 'projects/runners']) do
-            = link_to project_settings_ci_cd_path(@project), title: _('CI/CD') do
-              %span
-                = _('CI/CD')
-        - if settings_operations_available?
-          = nav_link(controller: [:operations]) do
-            = link_to project_settings_operations_path(@project), title: _('Operations'), data: { qa_selector: 'operations_settings_link' } do
-              = _('Operations')
-        - if @project.pages_available?
-          = nav_link(controller: :pages) do
-            = link_to project_pages_path(@project), title: _('Pages') do
-              %span
-                = _('Pages')
-        - if settings_packages_and_registries_enabled?(@project)
-          = nav_link(controller: :packages_and_registries) do
-            = link_to project_settings_packages_and_registries_path(@project), title: _('Packages & Registries'), data: { qa_selector: 'project_package_settings_link' } do
-              %span
-                = _('Packages & Registries')
-
 -# Shortcut to Project > Activity
 %li.hidden
   = link_to activity_project_path(@project), title: _('Activity'), class: 'shortcuts-project-activity' do

ee/app/helpers/ee/projects_helper.rb

@@ -4,13 +4,6 @@ module EE
   module ProjectsHelper
     extend ::Gitlab::Utils::Override
 
-    override :sidebar_settings_paths
-    def sidebar_settings_paths
-      super + %w[
-        operations#show
-      ]
-    end
-
     override :sidebar_operations_paths
     def sidebar_operations_paths
       super + %w[

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

@@ -399,27 +399,27 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
   end
 
-  describe 'Settings > Operations' do
-    it 'is not visible when no valid license' do
-      allow(view).to receive(:can?).and_return(true)
-
-      render
-
-      expect(rendered).not_to have_link project_settings_operations_path(project)
+  describe 'Settings' do
+    before do
+      allow(view).to receive(:current_user).and_return(user)
     end
 
-    it 'is not visible to unauthorized user' do
-      render
+    describe 'Operations' do
+      it 'links to settings page' do
+        render
 
-      expect(rendered).not_to have_link project_settings_operations_path(project)
-    end
+        expect(rendered).to have_link('Operations', href: project_settings_operations_path(project))
+      end
 
-    it 'links to settings page' do
-      allow(view).to receive(:can?).and_return(true)
+      context 'when user is not authorized' do
+        let(:user) { nil }
 
-      render
+        it 'does not display the link' do
+          render
 
-      expect(rendered).to have_link('Operations', href: project_settings_operations_path(project))
+          expect(rendered).not_to have_link('Operations', href: project_settings_operations_path(project))
+        end
+      end
     end
   end
 end

lib/sidebars/projects/menus/settings_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Projects
+    module Menus
+      class SettingsMenu < ::Sidebars::Menu
+        override :configure_menu_items
+        def configure_menu_items
+          return false unless can?(context.current_user, :admin_project, context.project)
+
+          add_item(general_menu_item)
+          add_item(integrations_menu_item)
+          add_item(webhooks_menu_item)
+          add_item(access_tokens_menu_item)
+          add_item(repository_menu_item)
+          add_item(ci_cd_menu_item)
+          add_item(operations_menu_item)
+          add_item(pages_menu_item)
+          add_item(packages_and_registries_menu_item)
+
+          true
+        end
+
+        override :link
+        def link
+          edit_project_path(context.project)
+        end
+
+        override :title
+        def title
+          _('Settings')
+        end
+
+        override :title_html_options
+        def title_html_options
+          {
+            id: 'js-onboarding-settings-link'
+          }
+        end
+
+        override :sprite_icon
+        def sprite_icon
+          'settings'
+        end
+
+        private
+
+        def general_menu_item
+          ::Sidebars::MenuItem.new(
+            title: _('General'),
+            link: edit_project_path(context.project),
+            active_routes: { path: 'projects#edit' },
+            item_id: :general
+          )
+        end
+
+        def integrations_menu_item
+          ::Sidebars::MenuItem.new(
+            title: _('Integrations'),
+            link: project_settings_integrations_path(context.project),
+            active_routes: { path: %w[integrations#show services#edit] },
+            item_id: :integrations
+          )
+        end
+
+        def webhooks_menu_item
+          ::Sidebars::MenuItem.new(
+            title: _('Webhooks'),
+            link: project_hooks_path(context.project),
+            active_routes: { path: %w[hooks#index hooks#edit hook_logs#show] },
+            item_id: :webhooks
+          )
+        end
+
+        def access_tokens_menu_item
+          return unless can?(context.current_user, :read_resource_access_tokens, context.project)
+
+          ::Sidebars::MenuItem.new(
+            title: _('Access Tokens'),
+            link: project_settings_access_tokens_path(context.project),
+            active_routes: { path: 'access_tokens#index' },
+            item_id: :access_tokens
+          )
+        end
+
+        def repository_menu_item
+          ::Sidebars::MenuItem.new(
+            title: _('Repository'),
+            link: project_settings_repository_path(context.project),
+            active_routes: { path: 'repository#show' },
+            item_id: :repository
+          )
+        end
+
+        def ci_cd_menu_item
+          return if context.project.archived?
+          return unless context.project.feature_available?(:builds, context.current_user)
+
+          ::Sidebars::MenuItem.new(
+            title: _('CI/CD'),
+            link: project_settings_ci_cd_path(context.project),
+            active_routes: { path: 'ci_cd#show' },
+            item_id: :ci_cd
+          )
+        end
+
+        def operations_menu_item
+          return if context.project.archived?
+          return unless can?(context.current_user, :admin_operations, context.project)
+
+          ::Sidebars::MenuItem.new(
+            title: _('Operations'),
+            link: project_settings_operations_path(context.project),
+            active_routes: { path: 'operations#show' },
+            item_id: :operations
+          )
+        end
+
+        def pages_menu_item
+          return unless context.project.pages_available?
+
+          ::Sidebars::MenuItem.new(
+            title: _('Pages'),
+            link: project_pages_path(context.project),
+            active_routes: { path: 'pages#show' },
+            item_id: :pages
+          )
+        end
+
+        def packages_and_registries_menu_item
+          return unless Gitlab.config.registry.enabled
+          return if Feature.disabled?(:sidebar_refactor, context.current_user)
+          return unless can?(context.current_user, :destroy_container_image, context.project)
+
+          ::Sidebars::MenuItem.new(
+            title: _('Packages & Registries'),
+            link: project_settings_packages_and_registries_path(context.project),
+            active_routes: { path: 'packages_and_registries#index' },
+            item_id: :packages_and_registries
+          )
+        end
+      end
+    end
+  end
+end

lib/sidebars/projects/panel.rb

@@ -24,6 +24,7 @@ module Sidebars
         add_menu(Sidebars::Projects::Menus::ExternalWikiMenu.new(context))
         add_menu(Sidebars::Projects::Menus::SnippetsMenu.new(context))
         add_menu(Sidebars::Projects::Menus::MembersMenu.new(context))
+        add_menu(Sidebars::Projects::Menus::SettingsMenu.new(context))
       end
 
       override :render_raw_menus_partial

qa/qa/ee/page/project/sub_menus/settings.rb

@@ -18,8 +18,8 @@ module QA
 
             def hover_settings
               within_sidebar do
-                scroll_to_element(:settings_item)
-                find_element(:settings_item).hover
+                scroll_to_element(:sidebar_menu_link, menu_item: 'Settings')
+                find_element(:sidebar_menu_link, menu_item: 'Settings').hover
 
                 yield
               end

qa/qa/page/project/sub_menus/settings.rb

@@ -12,21 +12,13 @@ module QA
 
             base.class_eval do
               include QA::Page::Project::SubMenus::Common
-
-              view 'app/views/layouts/nav/sidebar/_project_menus.html.haml' do
-                element :settings_item
-                element :general_settings_link
-                element :integrations_settings_link
-                element :operations_settings_link
-                element :access_tokens_settings_link
-              end
             end
           end
 
           def go_to_ci_cd_settings
             hover_settings do
               within_submenu do
-                click_link('CI/CD')
+                click_element(:sidebar_menu_item_link, menu_item: 'CI/CD')
               end
             end
           end
@@ -34,7 +26,7 @@ module QA
           def go_to_repository_settings
             hover_settings do
               within_submenu do
-                click_link('Repository')
+                click_element(:sidebar_menu_item_link, menu_item: 'Repository')
               end
             end
           end
@@ -42,21 +34,21 @@ module QA
           def go_to_general_settings
             hover_settings do
               within_submenu do
-                click_element :general_settings_link
+                click_element(:sidebar_menu_item_link, menu_item: 'General')
               end
             end
           end
 
           def click_settings
             within_sidebar do
-              click_on 'Settings'
+              click_element(:sidebar_menu_link, menu_item: 'Settings')
             end
           end
 
           def go_to_integrations_settings
             hover_settings do
               within_submenu do
-                click_element :integrations_settings_link
+                click_element(:sidebar_menu_item_link, menu_item: 'Integrations')
               end
             end
           end
@@ -64,7 +56,7 @@ module QA
           def go_to_operations_settings
             hover_settings do
               within_submenu do
-                click_element :operations_settings_link
+                click_element(:sidebar_menu_item_link, menu_item: 'Operations')
               end
             end
           end
@@ -72,7 +64,7 @@ module QA
           def go_to_access_token_settings
             hover_settings do
               within_submenu do
-                click_element :access_tokens_settings_link
+                click_element(:sidebar_menu_item_link, menu_item: 'Access Tokens')
               end
             end
           end
@@ -81,8 +73,8 @@ module QA
 
           def hover_settings
             within_sidebar do
-              scroll_to_element(:settings_item)
-              find_element(:settings_item).hover
+              scroll_to_element(:sidebar_menu_link, menu_item: 'Settings')
+              find_element(:sidebar_menu_link, menu_item: 'Settings').hover
 
               yield
             end

spec/features/projects/settings/operations_settings_spec.rb

@@ -3,21 +3,20 @@
 require 'spec_helper'
 
 RSpec.describe 'Projects > Settings > For a forked project', :js do
-  let(:user) { create(:user) }
-  let(:project) { create(:project, :repository, create_templates: :issue) }
-  let(:role) { :maintainer }
+  let_it_be(:project) { create(:project, :repository, create_templates: :issue) }
+
+  let(:user) { project.owner}
 
   before do
     sign_in(user)
-    project.add_role(user, role)
   end
 
   describe 'Sidebar > Operations' do
-    it 'renders the settings link in the sidebar' do
+    it 'renders the menu in the sidebar' do
       visit project_path(project)
       wait_for_requests
 
-      expect(page).to have_selector('a[title="Operations"]', visible: false)
+      expect(page).to have_selector('.sidebar-sub-level-items a[aria-label="Operations"]', text: 'Operations', visible: false)
     end
   end
 

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Projects::Menus::SettingsMenu do
+  let(:project) { build(:project) }
+  let(:user) { project.owner }
+  let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) }
+
+  subject { described_class.new(context) }
+
+  describe '#render?' do
+    it 'returns false when menu does not have any menu items' do
+      allow(subject).to receive(:has_items?).and_return(false)
+
+      expect(subject.render?).to be false
+    end
+  end
+
+  describe 'Menu items' do
+    subject { described_class.new(context).items.index { |e| e.item_id == item_id } }
+
+    shared_examples 'access rights checks' do
+      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 'General' do
+      let(:item_id) { :general }
+
+      it_behaves_like 'access rights checks'
+    end
+
+    describe 'Integrations' do
+      let(:item_id) { :integrations }
+
+      it_behaves_like 'access rights checks'
+    end
+
+    describe 'Webhooks' do
+      let(:item_id) { :webhooks }
+
+      it_behaves_like 'access rights checks'
+    end
+
+    describe 'Access Tokens' do
+      let(:item_id) { :access_tokens }
+
+      it_behaves_like 'access rights checks'
+    end
+
+    describe 'Repository' do
+      let(:item_id) { :repository }
+
+      it_behaves_like 'access rights checks'
+    end
+
+    describe 'CI/CD' do
+      let(:item_id) { :ci_cd }
+
+      describe 'when project is archived' do
+        before do
+          allow(project).to receive(:archived?).and_return(true)
+        end
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when project is not archived' do
+        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
+
+    describe 'Operations' do
+      let(:item_id) { :operations }
+
+      describe 'when project is archived' do
+        before do
+          allow(project).to receive(:archived?).and_return(true)
+        end
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when project is not archived' do
+        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
+
+    describe 'Pages' do
+      let(:item_id) { :pages }
+
+      before do
+        allow(project).to receive(:pages_available?).and_return(pages_enabled)
+      end
+
+      describe 'when pages are enabled' do
+        let(:pages_enabled) { true }
+
+        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 'when pages are not enabled' do
+        let(:pages_enabled) { false }
+
+        specify { is_expected.to be_nil }
+      end
+    end
+
+    describe 'Packages & Registries' do
+      let(:item_id) { :packages_and_registries }
+
+      before do
+        stub_container_registry_config(enabled: container_enabled)
+      end
+
+      describe 'when config registry setting is disabled' do
+        let(:container_enabled) { false }
+
+        specify { is_expected.to be_nil }
+      end
+
+      describe 'when config registry setting is enabled' do
+        let(:container_enabled) { true }
+
+        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
+
+        describe 'when the user does not have access' do
+          let(:user) { nil }
+
+          specify { is_expected.to be_nil }
+        end
+      end
+    end
+  end
+end

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

@@ -903,56 +903,168 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
   end
 
-  describe 'operations settings tab' do
-    describe 'archive projects' do
-      before do
-        project.update!(archived: project_archived)
+  describe 'Settings' do
+    describe 'General' do
+      it 'has a link to the General settings' do
+        render
+
+        expect(rendered).to have_link('General', href: edit_project_path(project))
+      end
+    end
+
+    describe 'Integrations' do
+      it 'has a link to the Integrations settings' do
+        render
+
+        expect(rendered).to have_link('Integrations', href: project_settings_integrations_path(project))
+      end
+    end
+
+    describe 'WebHooks' do
+      it 'has a link to the WebHooks settings' do
+        render
+
+        expect(rendered).to have_link('Webhooks', href: project_hooks_path(project))
+      end
+    end
+
+    describe 'Access Tokens' do
+      context 'self-managed instance' do
+        before do
+          allow(Gitlab).to receive(:com?).and_return(false)
+        end
+
+        it 'has a link to the Access Tokens settings' do
+          render
+
+          expect(rendered).to have_link('Access Tokens', href: project_settings_access_tokens_path(project))
+        end
+      end
+
+      context 'gitlab.com' do
+        before do
+          allow(Gitlab).to receive(:com?).and_return(true)
+        end
+
+        it 'has a link to the Access Tokens settings' do
+          render
+
+          expect(rendered).to have_link('Access Tokens', href: project_settings_access_tokens_path(project))
+        end
+      end
+    end
+
+    describe 'Repository' do
+      it 'has a link to the Repository settings' do
+        render
+
+        expect(rendered).to have_link('Repository', href: project_settings_repository_path(project))
       end
+    end
 
+    describe 'CI/CD' do
       context 'when project is archived' do
-        let(:project_archived) { true }
+        before do
+          project.update!(archived: true)
+        end
 
-        it 'does not show the operations settings tab' do
+        it 'does not have a link to the CI/CD settings' do
           render
 
-          expect(rendered).not_to have_link('Operations', href: project_settings_operations_path(project))
+          expect(rendered).not_to have_link('CI/CD', href: project_settings_ci_cd_path(project))
+        end
+      end
+
+      context 'when project is not archived' do
+        it 'has a link to the CI/CD settings' do
+          render
+
+          expect(rendered).to have_link('CI/CD', href: project_settings_ci_cd_path(project))
         end
       end
+    end
+
+    describe 'Operations' do
+      context 'when project is archived' do
+        before do
+          project.update!(archived: true)
+        end
 
-      context 'when project is active' do
-        let(:project_archived) { false }
+        it 'does not have a link to the Operations settings' do
+          render
+
+          expect(rendered).not_to have_link('Operations', href: project_settings_operations_path(project))
+        end
+      end
 
-        it 'shows the operations settings tab' do
+      context 'when project is not archived active' do
+        it 'has a link to the Operations settings' do
           render
 
           expect(rendered).to have_link('Operations', href: project_settings_operations_path(project))
         end
       end
     end
-  end
 
-  describe 'project access tokens' do
-    context 'self-managed instance' do
+    describe 'Pages' do
       before do
-        allow(Gitlab).to receive(:com?).and_return(false)
+        stub_config(pages: { enabled: pages_enabled })
       end
 
-      it 'displays "Access Tokens" nav item' do
-        render
+      context 'when pages are enabled' do
+        let(:pages_enabled) { true }
+
+        it 'has a link to the Pages settings' do
+          render
+
+          expect(rendered).to have_link('Pages', href: project_pages_path(project))
+        end
+      end
+
+      context 'when pages are not enabled' do
+        let(:pages_enabled) { false }
 
-        expect(rendered).to have_link('Access Tokens', href: project_settings_access_tokens_path(project))
+        it 'does not have a link to the Pages settings' do
+          render
+
+          expect(rendered).not_to have_link('Pages', href: project_pages_path(project))
+        end
       end
     end
 
-    context 'gitlab.com' do
+    describe 'Packages & Registries' do
       before do
-        allow(Gitlab).to receive(:com?).and_return(true)
+        stub_container_registry_config(enabled: registry_enabled)
       end
 
-      it 'displays "Access Tokens" nav item' do
-        render
+      context 'when registry is enabled' do
+        let(:registry_enabled) { true }
+
+        it 'has a link to the Packages & Registries settings' do
+          render
+
+          expect(rendered).to have_link('Packages & Registries', href: project_settings_packages_and_registries_path(project))
+        end
+
+        context 'when feature flag :sidebar_refactor is disabled' do
+          it 'does not have a link to the Packages & Registries settings' do
+            stub_feature_flags(sidebar_refactor: false)
+
+            render
+
+            expect(rendered).not_to have_link('Packages & Registries', href: project_settings_packages_and_registries_path(project))
+          end
+        end
+      end
 
-        expect(rendered).to have_link('Access Tokens', href: project_settings_access_tokens_path(project))
+      context 'when registry is not enabled' do
+        let(:registry_enabled) { false }
+
+        it 'does not have a link to the Packages & Registries settings' do
+          render
+
+          expect(rendered).not_to have_link('Packages & Registries', href: project_settings_packages_and_registries_path(project))
+        end
       end
     end
   end