Refactor Group information menu

In this commit we're refactoring the Group information
menu in the group sidebar.

We're moving away from partial defined logic to object
contained one. Now menus are defined in objects that
will contain all the information instead of in partials.

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