Merge branch '328509-fj-add-packages-menu' into 'master'

Add Packages menu

See merge request gitlab-org/gitlab!66897

app/helpers/groups_helper.rb

@@ -22,18 +22,6 @@ module GroupsHelper
     ]
   end
 
-  def group_packages_nav_link_paths
-    %w[
-      groups/packages#index
-      groups/container_registries#index
-    ]
-  end
-
-  def group_container_registry_nav?
-    Gitlab.config.registry.enabled &&
-      can?(current_user, :read_container_image, @group)
-  end
-
   def group_sidebar_links
     @group_sidebar_links ||= get_group_sidebar_links
   end
@@ -179,19 +167,6 @@ module GroupsHelper
     groups.to_json
   end
 
-  def group_packages_nav?
-    group_packages_list_nav? ||
-      group_container_registry_nav?
-  end
-
-  def group_dependency_proxy_nav?
-    @group.dependency_proxy_feature_available?
-  end
-
-  def group_packages_list_nav?
-    @group.packages_feature_enabled?
-  end
-
   def show_invite_banner?(group)
     can?(current_user, :admin_group, group) &&
     !just_created? &&

app/views/groups/sidebar/_packages.html.haml

-- packages_link = group_packages_list_nav? ? group_packages_path(@group) : group_container_registries_path(@group)
-
-- if group_packages_nav?
-  = nav_link(controller: ['groups/packages', 'groups/registry/repositories', 'groups/dependency_proxies']) do
-    = link_to packages_link, title: _('Packages'), class: 'has-sub-items' do
-      .nav-icon-container
-        = sprite_icon('package')
-      %span.nav-item-name
-        = _('Packages & Registries')
-    %ul.sidebar-sub-level-items
-      = nav_link(controller: [:packages, :repositories], html_options: { class: "fly-out-top-item" } ) do
-        = link_to packages_link, title: _('Packages & Registries') do
-          %strong.fly-out-top-item-name
-            = _('Packages & Registries')
-      %li.divider.fly-out-top-item
-      - if group_packages_list_nav?
-        = nav_link(controller: 'groups/packages') do
-          = link_to group_packages_path(@group), title: _('Packages') do
-            %span= _('Package Registry')
-      - if group_container_registry_nav?
-        = nav_link(controller: 'groups/registry/repositories') do
-          = link_to group_container_registries_path(@group), title: _('Container Registry') do
-            %span= _('Container Registry')
-      - if group_dependency_proxy_nav?
-        = nav_link(controller: 'groups/dependency_proxies') do
-          = link_to group_dependency_proxy_path(@group), title: _('Dependency Proxy') do
-            %span= _('Dependency Proxy')

app/views/groups/sidebar/_packages_settings.html.haml

-- if group_packages_list_nav?
+- if @group.packages_feature_enabled?
   = nav_link(controller: :packages_and_registries) do
     = link_to group_settings_packages_and_registries_path(@group), title: _('Packages & Registries'), data: { qa_selector: 'group_package_settings_link' } do
       %span

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

-= render 'groups/sidebar/packages'
-
 = render 'layouts/nav/sidebar/analytics_links', links: group_analytics_navbar_links(@group, current_user)
 
 - if group_sidebar_link?(:wiki)

ee/app/helpers/ee/groups_helper.rb

@@ -26,24 +26,6 @@ module EE
       "Max size for repositories within this group #{show_lfs}. Can be overridden inside each project. For no limit, enter 0. To inherit the global value, leave blank."
     end
 
-    override :group_packages_nav_link_paths
-    def group_packages_nav_link_paths
-      %w[
-        groups/packages#index
-        groups/dependency_proxies#show
-        groups/container_registries#index
-      ]
-    end
-
-    override :group_packages_nav?
-    def group_packages_nav?
-      super || group_dependency_proxy_nav?
-    end
-
-    def group_dependency_proxy_nav?
-      @group.dependency_proxy_feature_available?
-    end
-
     def group_path_params(group)
       { group_id: group }
     end

ee/app/views/groups/sidebar/_packages.html.haml

-- packages_link = group_packages_list_nav? ? group_packages_path(@group) : group_container_registries_path(@group)
-
-- if group_packages_nav?
-  = nav_link(controller: ['groups/packages', 'groups/registry/repositories', 'groups/dependency_proxies']) do
-    = link_to packages_link, title: _('Packages'), data: { qa_selector: 'group_packages_item' }, class: 'has-sub-items' do
-      .nav-icon-container
-        = sprite_icon('package')
-      %span.nav-item-name
-        = _('Packages & Registries')
-    %ul.sidebar-sub-level-items{ data: { qa_selector: 'group_packages_submenu' } }
-      = nav_link(controller: [:packages, :repositories], html_options: { class: "fly-out-top-item" } ) do
-        = link_to packages_link, title: _('Packages & Registries') do
-          %strong.fly-out-top-item-name
-            = _('Packages & Registries')
-      %li.divider.fly-out-top-item
-      - if group_packages_list_nav?
-        = nav_link(controller: 'groups/packages') do
-          = link_to group_packages_path(@group), title: _('Packages'), data: { qa_selector: 'group_packages_link' } do
-            %span= _('Package Registry')
-      - if group_container_registry_nav?
-        = nav_link(controller: 'groups/registry/repositories') do
-          = link_to group_container_registries_path(@group), title: _('Container Registry') do
-            %span= _('Container Registry')
-      - if group_dependency_proxy_nav?
-        = nav_link(controller: 'groups/dependency_proxies') do
-          = link_to group_dependency_proxy_path(@group), title: _('Dependency Proxy') do
-            %span= _('Dependency Proxy')

lib/sidebars/groups/menus/packages_registries_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Groups
+    module Menus
+      class PackagesRegistriesMenu < ::Sidebars::Menu
+        override :configure_menu_items
+        def configure_menu_items
+          add_item(packages_registry_menu_item)
+          add_item(container_registry_menu_item)
+          add_item(dependency_proxy_menu_item)
+
+          true
+        end
+
+        override :link
+        def link
+          renderable_items.first.link
+        end
+
+        override :title
+        def title
+          _('Packages & Registries')
+        end
+
+        override :sprite_icon
+        def sprite_icon
+          'package'
+        end
+
+        private
+
+        def packages_registry_menu_item
+          unless context.group.packages_feature_enabled?
+            return ::Sidebars::NilMenuItem.new(item_id: :packages_registry)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Package Registry'),
+            link: group_packages_path(context.group),
+            active_routes: { controller: 'groups/packages' },
+            item_id: :packages_registry
+          )
+        end
+
+        def container_registry_menu_item
+          if !::Gitlab.config.registry.enabled || !can?(context.current_user, :read_container_image, context.group)
+            return ::Sidebars::NilMenuItem.new(item_id: :container_registry)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Container Registry'),
+            link: group_container_registries_path(context.group),
+            active_routes: { controller: 'groups/registry/repositories' },
+            item_id: :container_registry
+          )
+        end
+
+        def dependency_proxy_menu_item
+          unless context.group.dependency_proxy_feature_available?
+            return ::Sidebars::NilMenuItem.new(item_id: :dependency_proxy)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Dependency Proxy'),
+            link: group_dependency_proxy_path(context.group),
+            active_routes: { controller: 'groups/dependency_proxies' },
+            item_id: :dependency_proxy
+          )
+        end
+      end
+    end
+  end
+end

lib/sidebars/groups/panel.rb

@@ -12,6 +12,7 @@ module Sidebars
         add_menu(Sidebars::Groups::Menus::MergeRequestsMenu.new(context))
         add_menu(Sidebars::Groups::Menus::CiCdMenu.new(context))
         add_menu(Sidebars::Groups::Menus::KubernetesMenu.new(context))
+        add_menu(Sidebars::Groups::Menus::PackagesRegistriesMenu.new(context))
       end
 
       override :render_raw_menus_partial

locale/gitlab.pot

@@ -23892,9 +23892,6 @@ msgstr ""
 msgid "PackageRegistry|published by %{author}"
 msgstr ""
 
-msgid "Packages"
-msgstr ""
-
 msgid "Packages & Registries"
 msgstr ""
 

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

@@ -36,12 +36,6 @@ module QA
               view 'ee/app/views/layouts/nav/_group_insights_link.html.haml' do
                 element :group_insights_link
               end
-
-              view 'ee/app/views/groups/sidebar/_packages.html.haml' do
-                element :group_packages_item
-                element :group_packages_link
-                element :group_packages_submenu
-              end
             end
           end
 
@@ -123,14 +117,6 @@ module QA
             end
           end
 
-          def go_to_group_packages
-            hover_element(:group_packages_item) do
-              within_submenu(:group_packages_submenu) do
-                click_element(:group_packages_link)
-              end
-            end
-          end
-
           def click_group_wiki_link
             within_sidebar do
               click_element(:wiki_link)

qa/qa/page/group/menu.rb

@@ -75,6 +75,14 @@ module QA
           end
         end
 
+        def go_to_group_packages
+          hover_group_packages do
+            within_submenu do
+              click_element(:sidebar_menu_item_link, menu_item: 'Package Registry')
+            end
+          end
+        end
+
         private
 
         def hover_issues
@@ -101,6 +109,15 @@ module QA
             yield
           end
         end
+
+        def hover_group_packages
+          within_sidebar do
+            scroll_to_element(:sidebar_menu_link, menu_item: 'Packages & Registries')
+            find_element(:sidebar_menu_link, menu_item: 'Packages & Registries').hover
+
+            yield
+          end
+        end
       end
     end
   end

spec/helpers/groups_helper_spec.rb

@@ -263,42 +263,6 @@ RSpec.describe GroupsHelper do
     end
   end
 
-  describe '#group_container_registry_nav' do
-    let_it_be(:group) { create(:group, :public) }
-    let_it_be(:user) { create(:user) }
-
-    before do
-      stub_container_registry_config(enabled: true)
-      allow(helper).to receive(:current_user) { user }
-      allow(helper).to receive(:can?).with(user, :read_container_image, group) { true }
-      helper.instance_variable_set(:@group, group)
-    end
-
-    subject { helper.group_container_registry_nav? }
-
-    context 'when container registry is enabled' do
-      it { is_expected.to be_truthy }
-
-      it 'is disabled for guest' do
-        allow(helper).to receive(:can?).with(user, :read_container_image, group) { false }
-        expect(subject).to be false
-      end
-    end
-
-    context 'when container registry is not enabled' do
-      before do
-        stub_container_registry_config(enabled: false)
-      end
-
-      it { is_expected.to be_falsy }
-
-      it 'is disabled for guests' do
-        allow(helper).to receive(:can?).with(user, :read_container_image, group) { false }
-        expect(subject).to be false
-      end
-    end
-  end
-
   describe '#group_sidebar_links' do
     let_it_be(:group) { create(:group, :public) }
     let_it_be(:user) { create(:user) }

spec/lib/sidebars/groups/menus/packages_registries_menu_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Groups::Menus::PackagesRegistriesMenu do
+  let_it_be(:owner) { create(:user) }
+  let_it_be(:group) do
+    build(:group, :private).tap do |g|
+      g.add_owner(owner)
+    end
+  end
+
+  let(:user) { owner }
+  let(:context) { Sidebars::Groups::Context.new(current_user: user, container: group) }
+  let(:menu) { described_class.new(context) }
+
+  describe '#render?' do
+    context 'when menu has menu items to show' do
+      it 'returns true' do
+        expect(menu.render?).to eq true
+      end
+    end
+
+    context 'when menu does not have any menu item to show' do
+      it 'returns false' do
+        stub_container_registry_config(enabled: false)
+        stub_config(packages: { enabled: false })
+        stub_config(dependency_proxy: { enabled: false })
+
+        expect(menu.render?).to eq false
+      end
+    end
+  end
+
+  describe '#link' do
+    let(:registry_enabled) { true }
+    let(:packages_enabled) { true }
+
+    before do
+      stub_container_registry_config(enabled: registry_enabled)
+      stub_config(packages: { enabled: packages_enabled })
+      stub_config(dependency_proxy: { enabled: true })
+    end
+
+    subject { menu.link }
+
+    context 'when Packages Registry is visible' do
+      it 'menu link points to Packages Registry page' do
+        expect(subject).to eq find_menu(menu, :packages_registry).link
+      end
+    end
+
+    context 'when Packages Registry is not visible' do
+      let(:packages_enabled) { false }
+
+      it 'menu link points to Container Registry page' do
+        expect(subject).to eq find_menu(menu, :container_registry).link
+      end
+
+      context 'when Container Registry is not visible' do
+        let(:registry_enabled) { false }
+
+        it 'menu link points to Dependency Proxy page' do
+          expect(subject).to eq find_menu(menu, :dependency_proxy).link
+        end
+      end
+    end
+  end
+
+  describe 'Menu items' do
+    subject { find_menu(menu, item_id) }
+
+    describe 'Packages Registry' do
+      let(:item_id) { :packages_registry }
+
+      context 'when user can read packages' do
+        before do
+          stub_config(packages: { enabled: packages_enabled })
+        end
+
+        context 'when config package setting is disabled' do
+          let(:packages_enabled) { false }
+
+          it 'the menu item is not added to list of menu items' do
+            is_expected.to be_nil
+          end
+        end
+
+        context 'when config package setting is enabled' do
+          let(:packages_enabled) { true }
+
+          it 'the menu item is added to list of menu items' do
+            is_expected.not_to be_nil
+          end
+        end
+      end
+    end
+
+    describe 'Container Registry' do
+      let(:item_id) { :container_registry }
+
+      context 'when user can read container images' do
+        before do
+          stub_container_registry_config(enabled: container_enabled)
+        end
+
+        context 'when config registry setting is disabled' do
+          let(:container_enabled) { false }
+
+          it 'the menu item is not added to list of menu items' do
+            is_expected.to be_nil
+          end
+        end
+
+        context 'when config registry setting is enabled' do
+          let(:container_enabled) { true }
+
+          it 'the menu item is added to list of menu items' do
+            is_expected.not_to be_nil
+          end
+
+          context 'when user cannot read container images' do
+            let(:user) { nil }
+
+            it 'the menu item is not added to list of menu items' do
+              is_expected.to be_nil
+            end
+          end
+        end
+      end
+    end
+
+    describe 'Dependency Proxy' do
+      let(:item_id) { :dependency_proxy }
+
+      before do
+        stub_config(dependency_proxy: { enabled: dependency_enabled })
+      end
+
+      context 'when config dependency_proxy is enabled' do
+        let(:dependency_enabled) { true }
+
+        it 'the menu item is added to list of menu items' do
+          is_expected.not_to be_nil
+        end
+      end
+
+      context 'when config dependency_proxy is not enabled' do
+        let(:dependency_enabled) { false }
+
+        it 'the menu item is not added to list of menu items' do
+          is_expected.to be_nil
+        end
+      end
+    end
+  end
+
+  private
+
+  def find_menu(menu, item)
+    menu.renderable_items.find { |i| i.item_id == item }
+  end
+end

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

@@ -108,4 +108,30 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
       expect(rendered).to have_link('Kubernetes', href: group_clusters_path(group))
     end
   end
+
+  describe 'Packages & Registries' do
+    it 'has a link to the package registry page' do
+      stub_config(packages: { enabled: true })
+
+      render
+
+      expect(rendered).to have_link('Package Registry', href: group_packages_path(group))
+    end
+
+    it 'has a link to the container registry page' do
+      stub_container_registry_config(enabled: true)
+
+      render
+
+      expect(rendered).to have_link('Container Registry', href: group_container_registries_path(group))
+    end
+
+    it 'has a link to the dependency proxy page' do
+      stub_config(dependency_proxy: { enabled: true })
+
+      render
+
+      expect(rendered).to have_link('Dependency Proxy', href: group_dependency_proxy_path(group))
+    end
+  end
 end