Merge branch '337286-fj-clean-code-after-group-sidebar-refactor' into 'master'

Clean code after group sidebar refactor

See merge request gitlab-org/gitlab!68338

app/helpers/groups_helper.rb

 # frozen_string_literal: true
 
 module GroupsHelper
-  def group_sidebar_links
-    @group_sidebar_links ||= get_group_sidebar_links
-  end
-
-  def group_sidebar_link?(link)
-    group_sidebar_links.include?(link)
-  end
-
   def can_change_group_visibility_level?(group)
     can?(current_user, :change_visibility_level, group)
   end
@@ -33,20 +25,6 @@ module GroupsHelper
     Ability.allowed?(current_user, :admin_group_member, group)
   end
 
-  def group_issues_count(state:)
-    IssuesFinder
-      .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true)
-      .execute
-      .count
-  end
-
-  def group_merge_requests_count(state:)
-    MergeRequestsFinder
-      .new(current_user, group_id: @group.id, state: state, non_archived: true, include_subgroups: true)
-      .execute
-      .count
-  end
-
   def group_dependency_proxy_image_prefix(group)
     # The namespace path can include uppercase letters, which
     # Docker doesn't allow. The proxy expects it to be downcased.
@@ -181,36 +159,6 @@ module GroupsHelper
     group.member_count > 1 || group.members_with_parents.count > 1
   end
 
-  def get_group_sidebar_links
-    links = [:overview, :group_members]
-
-    resources = [:activity, :issues, :boards, :labels, :milestones,
-                 :merge_requests]
-    links += resources.select do |resource|
-      can?(current_user, "read_group_#{resource}".to_sym, @group)
-    end
-
-    # TODO Proper policies, such as `read_group_runners, should be implemented per
-    # See https://gitlab.com/gitlab-org/gitlab/-/issues/334802
-    if can?(current_user, :admin_group, @group) && Feature.enabled?(:runner_list_group_view_vue_ui, @group, default_enabled: :yaml)
-      links << :runners
-    end
-
-    if can?(current_user, :read_cluster, @group)
-      links << :kubernetes
-    end
-
-    if can?(current_user, :admin_group, @group)
-      links << :settings
-    end
-
-    if can?(current_user, :read_wiki, @group)
-      links << :wiki
-    end
-
-    links
-  end
-
   def group_title_link(group, hidable: false, show_avatar: false, for_dropdown: false)
     link_to(group_path(group), class: "group-path #{'breadcrumb-item-text' unless for_dropdown} js-breadcrumb-item-text #{'hidable' if hidable}") do
       icon = group_icon(group, class: "avatar-tile", width: 15, height: 15) if (group.try(:avatar_url) || show_avatar) && !Rails.env.test?

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

--# We're migration the group sidebar to a logical model based structure. If you need to update
--# any of the existing menus, you can find them in app/views/layouts/nav/sidebar/_group_menus.html.haml.
 = render partial: 'shared/nav/sidebar', object: Sidebars::Groups::Panel.new(group_sidebar_context(@group, current_user))

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

-= render 'shared/sidebar_toggle_button'

ee/app/helpers/ee/groups_helper.rb

@@ -10,10 +10,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
 
-    def group_path_params(group)
-      { group_id: group }
-    end
-
     override :remove_group_message
     def remove_group_message(group)
       return super unless group.licensed_feature_available?(:adjourned_deletion_for_projects_and_groups)
@@ -63,55 +59,5 @@ module EE
     def show_product_purchase_success_alert?
       !params[:purchased_product].blank?
     end
-
-    private
-
-    def get_group_sidebar_links
-      links = super
-
-      resources = [:cycle_analytics, :merge_request_analytics, :repository_analytics]
-
-      links += resources.select do |resource|
-        can?(current_user, "read_group_#{resource}".to_sym, @group)
-      end
-
-      if can?(current_user, :read_group_contribution_analytics, @group) || show_promotions?
-        links << :contribution_analytics
-      end
-
-      if can?(current_user, :read_epic, @group)
-        links << :epics
-      end
-
-      if @group.licensed_feature_available?(:issues_analytics)
-        links << :analytics
-      end
-
-      if @group.insights_available?
-        links << :group_insights
-      end
-
-      if @group.licensed_feature_available?(:productivity_analytics) && can?(current_user, :view_productivity_analytics, @group)
-        links << :productivity_analytics
-      end
-
-      if ::Feature.enabled?(:group_iterations, @group, default_enabled: true) && @group.licensed_feature_available?(:iterations)
-        if ::Feature.enabled?(:iteration_cadences, @group, default_enabled: :yaml) && can?(current_user, :read_iteration_cadence, @group)
-          links << :iteration_cadences
-        elsif can?(current_user, :read_iteration, @group)
-          links << :iterations
-        end
-      end
-
-      if ::Feature.enabled?(:group_ci_cd_analytics_page, @group, default_enabled: true) && @group.licensed_feature_available?(:group_ci_cd_analytics) && can?(current_user, :view_group_ci_cd_analytics, @group)
-        links << :group_ci_cd_analytics
-      end
-
-      if can?(current_user, :view_group_devops_adoption, @group)
-        links << :group_devops_adoption
-      end
-
-      links
-    end
   end
 end

ee/app/views/layouts/nav/_issues_analytics_link.html.haml

-- if group_sidebar_link?(:analytics)
-  = nav_link(path: 'issues_analytics#show') do
-    = link_to group_issues_analytics_path(@group) do
-      %span
-        = _('Analytics')

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -16,87 +16,6 @@ RSpec.describe GroupsHelper do
     group.add_owner(owner)
   end
 
-  describe '#group_sidebar_links' do
-    before do
-      allow(helper).to receive(:can?) { |*args| Ability.allowed?(*args) }
-      allow(helper).to receive(:show_promotions?) { false }
-    end
-
-    it 'shows the licensed features when they are available' do
-      stub_licensed_features(contribution_analytics: true,
-                             group_ci_cd_analytics: true,
-                             epics: true)
-
-      expect(helper.group_sidebar_links).to include(:contribution_analytics, :group_ci_cd_analytics, :epics)
-    end
-
-    it 'hides the licensed features when they are not available' do
-      stub_licensed_features(contribution_analytics: false,
-                             group_ci_cd_analytics: false,
-                             epics: false)
-
-      expect(helper.group_sidebar_links).not_to include(:contribution_analytics, :group_ci_cd_analytics, :epics)
-    end
-
-    context 'when contribution analytics is available' do
-      before do
-        stub_licensed_features(contribution_analytics: true)
-      end
-
-      context 'signed in user is a project member but not a member of the group' do
-        let(:current_user) { create(:user) }
-        let(:private_project) { create(:project, :private, group: group)}
-
-        it 'hides Contribution Analytics' do
-          expect(helper.group_sidebar_links).not_to include(:contribution_analytics)
-        end
-      end
-    end
-
-    context 'when the group_ci_cd_analytics_page feature flag is disabled' do
-      before do
-        stub_feature_flags(group_ci_cd_analytics_page: false)
-      end
-
-      it 'hides CI/CD Analytics' do
-        expect(helper.group_sidebar_links).not_to include(:group_ci_cd_analytics)
-      end
-    end
-
-    context 'when the user does not have permissions to view the CI/CD Analytics page' do
-      let(:current_user) { create(:user) }
-
-      before do
-        group.add_guest(current_user)
-      end
-
-      it 'hides CI/CD Analytics' do
-        expect(helper.group_sidebar_links).not_to include(:group_ci_cd_analytics)
-      end
-    end
-
-    context 'when iterations is available' do
-      before do
-        stub_licensed_features(iterations: true)
-        stub_feature_flags(iteration_cadences: false)
-      end
-
-      it 'shows iterations link' do
-        expect(helper.group_sidebar_links).to include(:iterations)
-      end
-
-      context 'when iteration_cadences is available' do
-        before do
-          stub_feature_flags(iteration_cadences: true)
-        end
-
-        it 'shows iterations link' do
-          expect(helper.group_sidebar_links).to include(:iteration_cadences)
-        end
-      end
-    end
-  end
-
   describe '#render_setting_to_allow_project_access_token_creation?' do
     context 'with self-managed' do
       let_it_be(:parent) { create(:group) }

lib/sidebars/groups/panel.rb

@@ -16,11 +16,6 @@ module Sidebars
         add_menu(Sidebars::Groups::Menus::SettingsMenu.new(context))
       end
 
-      override :render_raw_menus_partial
-      def render_raw_menus_partial
-        'layouts/nav/sidebar/group_menus'
-      end
-
       override :aria_label
       def aria_label
         context.group.subgroup? ? _('Subgroup navigation') : _('Group navigation')

spec/helpers/groups_helper_spec.rb

@@ -267,61 +267,6 @@ RSpec.describe GroupsHelper do
     end
   end
 
-  describe '#group_sidebar_links' do
-    let_it_be(:group) { create(:group, :public) }
-    let_it_be(:user) { create(:user) }
-
-    before do
-      group.add_owner(user)
-      allow(helper).to receive(:current_user) { user }
-      allow(helper).to receive(:can?) { |*args| Ability.allowed?(*args) }
-      helper.instance_variable_set(:@group, group)
-    end
-
-    it 'returns all the expected links' do
-      links = [
-        :overview, :activity, :issues, :labels, :milestones, :merge_requests,
-        :runners, :group_members, :settings
-      ]
-
-      expect(helper.group_sidebar_links).to include(*links)
-    end
-
-    it 'excludes runners when the user cannot admin the group' do
-      expect(helper).to receive(:current_user) { user }
-      # TODO Proper policies, such as `read_group_runners, should be implemented per
-      # See https://gitlab.com/gitlab-org/gitlab/-/issues/334802
-      expect(helper).to receive(:can?).twice.with(user, :admin_group, group) { false }
-
-      expect(helper.group_sidebar_links).not_to include(:runners)
-    end
-
-    it 'excludes runners when the feature "runner_list_group_view_vue_ui" is disabled' do
-      stub_feature_flags(runner_list_group_view_vue_ui: false)
-
-      expect(helper.group_sidebar_links).not_to include(:runners)
-    end
-
-    it 'excludes settings when the user can admin the group' do
-      expect(helper).to receive(:current_user) { user }
-      expect(helper).to receive(:can?).twice.with(user, :admin_group, group) { false }
-
-      expect(helper.group_sidebar_links).not_to include(:settings)
-    end
-
-    it 'excludes cross project features when the user cannot read cross project' do
-      cross_project_features = [:activity, :issues, :labels, :milestones,
-                                :merge_requests]
-
-      allow(Ability).to receive(:allowed?).and_call_original
-      cross_project_features.each do |feature|
-        expect(Ability).to receive(:allowed?).with(user, "read_group_#{feature}".to_sym, group) { false }
-      end
-
-      expect(helper.group_sidebar_links).not_to include(*cross_project_features)
-    end
-  end
-
   describe '#parent_group_options' do
     let_it_be(:current_user) { create(:user) }
     let_it_be(:group) { create(:group, name: 'group') }