Merge branch '328503-fj-add-group-information-menu' into 'master'

Add Group information menu

See merge request gitlab-org/gitlab!65819

app/helpers/groups_helper.rb

 # frozen_string_literal: true
 
 module GroupsHelper
-  def group_overview_nav_link_paths
-    %w[
-      groups#activity
-      groups#subgroups
-      labels#index
-      group_members#index
-    ]
-  end
-
   def group_settings_nav_link_paths
     %w[
       groups#projects
@@ -38,10 +29,6 @@ module GroupsHelper
     ]
   end
 
-  def group_information_title(group)
-    group.subgroup? ? _('Subgroup information') : _('Group information')
-  end
-
   def group_container_registry_nav?
     Gitlab.config.registry.enabled &&
       can?(current_user, :read_container_image, @group)

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

 - issues_count = cached_issuables_count(@group, type: :issues)
 - merge_requests_count = cached_issuables_count(@group, type: :merge_requests)
 
-- if group_sidebar_link?(:overview)
-  - paths = group_overview_nav_link_paths
-  = nav_link(path: paths, unless: -> { current_path?('groups/contribution_analytics#show') }, html_options: { class: 'home' }) do
-    = link_to activity_group_path(@group), class: 'has-sub-items', data: { qa_selector: 'group_information_link' } do
-      .nav-icon-container
-        = sprite_icon('group')
-      %span.nav-item-name
-        = group_information_title(@group)
-
-    %ul.sidebar-sub-level-items{ data: { qa_selector: 'group_information_submenu'} }
-      = nav_link(path: paths, html_options: { class: "fly-out-top-item" } ) do
-        = link_to activity_group_path(@group) do
-          %strong.fly-out-top-item-name
-            = group_information_title(@group)
-      %li.divider.fly-out-top-item
-
-      - if group_sidebar_link?(:activity)
-        = nav_link(path: 'groups#activity') do
-          = link_to activity_group_path(@group), title: _('Activity') do
-            %span
-              = _('Activity')
-
-      - if group_sidebar_link?(:labels)
-        = nav_link(path: 'labels#index') do
-          = link_to group_labels_path(@group), title: _('Labels') do
-            %span
-              = _('Labels')
-
-      - if group_sidebar_link?(:group_members)
-        = nav_link(path: 'group_members#index') do
-          = link_to group_group_members_path(@group), title: _('Members'), data: { qa_selector: 'group_members_item' } do
-            %span
-              = _('Members')
-
 = render_if_exists "layouts/nav/ee/epic_link", group: @group
 
 - if group_sidebar_link?(:issues)

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

@@ -10,7 +10,7 @@ module EE
         def configure_menus
           super
 
-          add_menu(::Sidebars::Groups::Menus::TrialExperimentMenu.new(context))
+          insert_menu_before(::Sidebars::Groups::Menus::GroupInformationMenu, ::Sidebars::Groups::Menus::TrialExperimentMenu.new(context))
         end
       end
     end

lib/sidebars/groups/menus/group_information_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Groups
+    module Menus
+      class GroupInformationMenu < ::Sidebars::Menu
+        override :configure_menu_items
+        def configure_menu_items
+          add_item(activity_menu_item)
+          add_item(labels_menu_item)
+          add_item(members_menu_item)
+
+          true
+        end
+
+        override :link
+        def link
+          renderable_items.first.link
+        end
+
+        override :title
+        def title
+          context.group.subgroup? ? _('Subgroup information') : _('Group information')
+        end
+
+        override :sprite_icon
+        def sprite_icon
+          'group'
+        end
+
+        override :active_routes
+        def active_routes
+          { path: 'groups#subgroups' }
+        end
+
+        private
+
+        def activity_menu_item
+          unless can?(context.current_user, :read_group_activity, context.group)
+            return ::Sidebars::NilMenuItem.new(item_id: :activity)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Activity'),
+            link: activity_group_path(context.group),
+            active_routes: { path: 'groups#activity' },
+            item_id: :activity
+          )
+        end
+
+        def labels_menu_item
+          unless can?(context.current_user, :read_group_labels, context.group)
+            return ::Sidebars::NilMenuItem.new(item_id: :labels)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Labels'),
+            link: group_labels_path(context.group),
+            active_routes: { controller: :labels },
+            item_id: :labels
+          )
+        end
+
+        def members_menu_item
+          unless can?(context.current_user, :read_group_member, context.group)
+            return ::Sidebars::NilMenuItem.new(item_id: :members)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Members'),
+            link: group_group_members_path(context.group),
+            active_routes: { path: 'group_members#index' },
+            item_id: :members
+          )
+        end
+      end
+    end
+  end
+end

lib/sidebars/groups/panel.rb

@@ -6,6 +6,8 @@ module Sidebars
       override :configure_menus
       def configure_menus
         set_scope_menu(Sidebars::Groups::Menus::ScopeMenu.new(context))
+
+        add_menu(Sidebars::Groups::Menus::GroupInformationMenu.new(context))
       end
 
       override :render_raw_menus_partial

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

@@ -110,14 +110,6 @@ module QA
             end
           end
 
-          def click_group_members_item
-            hover_element(:group_information_link) do
-              within_submenu(:group_information_submenu) do
-                click_element(:group_members_item)
-              end
-            end
-          end
-
           def click_group_general_settings_item
             hover_element(:group_settings) do
               within_submenu(:group_sidebar_submenu) do

qa/qa/page/group/menu.rb

@@ -9,11 +9,8 @@ module QA
         view 'app/views/layouts/nav/sidebar/_group_menus.html.haml' do
           element :general_settings_link
           element :group_issues_item
-          element :group_members_item
           element :group_milestones_link
           element :group_settings
-          element :group_information_link
-          element :group_information_submenu
         end
 
         view 'app/views/groups/sidebar/_packages_settings.html.haml' do
@@ -26,9 +23,17 @@ module QA
         end
 
         def click_group_members_item
-          hover_element(:group_information_link) do
-            within_submenu(:group_information_submenu) do
-              click_element(:group_members_item)
+          hover_group_information do
+            within_submenu do
+              click_element(:sidebar_menu_item_link, menu_item: 'Members')
+            end
+          end
+        end
+
+        def click_subgroup_members_item
+          hover_subgroup_information do
+            within_submenu do
+              click_element(:sidebar_menu_item_link, menu_item: 'Members')
             end
           end
         end
@@ -82,6 +87,22 @@ module QA
             yield
           end
         end
+
+        def hover_group_information
+          within_sidebar do
+            find_element(:sidebar_menu_link, menu_item: 'Group information').hover
+
+            yield
+          end
+        end
+
+        def hover_subgroup_information
+          within_sidebar do
+            find_element(:sidebar_menu_link, menu_item: 'Subgroup information').hover
+
+            yield
+          end
+        end
       end
     end
   end

qa/qa/specs/features/ee/browser_ui/1_manage/group/group_audit_logs_1_spec.rb

@@ -90,7 +90,7 @@ module QA
         before do
           sign_in
           group.visit!
-          Page::Group::Menu.perform(&:click_group_members_item)
+          Page::Group::Menu.perform(&:click_subgroup_members_item)
           Page::Group::Members.perform do |members_page|
             members_page.add_member(user.username, 'Guest')
             members_page.update_access_level(user.username, "Developer")

qa/qa/specs/features/ee/browser_ui/1_manage/group/group_ldap_sync_spec.rb

@@ -95,7 +95,7 @@ module QA
             settings.click_add_sync_button
           end
 
-          Page::Group::Menu.perform(&:click_group_members_item)
+          Page::Group::Menu.perform(&:click_subgroup_members_item)
         end
 
         it 'has LDAP users synced', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/670' do
@@ -151,7 +151,7 @@ module QA
             settings.click_add_sync_button
           end
 
-          Page::Group::Menu.perform(&:click_group_members_item)
+          Page::Group::Menu.perform(&:click_subgroup_members_item)
         end
 
         it 'has LDAP users synced', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/669' do

qa/qa/specs/features/ee/browser_ui/1_manage/group/share_group_with_group_spec.rb

@@ -42,7 +42,7 @@ module QA
 
         target_group_with_project.visit!
 
-        Page::Group::Menu.perform(&:click_group_members_item)
+        Page::Group::Menu.perform(&:click_subgroup_members_item)
         Page::Group::Members.perform do |members|
           members.invite_group(source_group_with_members.path)
 

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Groups::Menus::GroupInformationMenu do
+  let_it_be(:owner) { create(:user) }
+  let_it_be(:root_group) do
+    build(:group, :private).tap do |g|
+      g.add_owner(owner)
+    end
+  end
+
+  let(:group) { root_group }
+  let(:user) { owner }
+  let(:context) { Sidebars::Groups::Context.new(current_user: user, container: group) }
+
+  describe '#title' do
+    subject { described_class.new(context).title }
+
+    context 'when group is a root group' do
+      specify { is_expected.to eq 'Group information'}
+    end
+
+    context 'when group is a child group' do
+      let(:group) { build(:group, parent: root_group) }
+
+      specify { is_expected.to eq 'Subgroup information'}
+    end
+  end
+
+  describe 'Menu Items' do
+    subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
+
+    shared_examples 'menu access rights' 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 'Activity' do
+      let(:item_id) { :activity }
+
+      specify { is_expected.not_to be_nil }
+
+      it_behaves_like 'menu access rights'
+    end
+
+    describe 'Labels' do
+      let(:item_id) { :labels }
+
+      it_behaves_like 'menu access rights'
+    end
+
+    describe 'Members' do
+      let(:item_id) { :members }
+
+      it_behaves_like 'menu access rights'
+    end
+  end
+end

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

@@ -27,10 +27,15 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
       expect(rendered).to have_link('Group information', href: activity_group_path(group))
     end
 
+    it 'has a link to the group labels path' do
+      render
+
+      expect(rendered).to have_link('Labels', href: group_labels_path(group))
+    end
+
     it 'has a link to the members page' do
       render
 
-      expect(rendered).to have_selector('.sidebar-top-level-items > li.home a[title="Members"]')
       expect(rendered).to have_link('Members', href: group_group_members_path(group))
     end
   end