Merge branch '328505-fj-add-issues-menu' into 'master'

Add issues menu

See merge request gitlab-org/gitlab!66438

app/helpers/boards_helper.rb

@@ -119,14 +119,6 @@ module BoardsHelper
     }
   end
 
-  def boards_link_text
-    if current_board_parent.multiple_issue_boards_available?
-      s_("IssueBoards|Boards")
-    else
-      s_("IssueBoards|Board")
-    end
-  end
-
   def recent_boards_path
     recent_project_boards_path(@project) if current_board_parent.is_a?(Project)
   end

app/helpers/nav_helper.rb

@@ -67,15 +67,6 @@ module NavHelper
     %w(dev_ops_report usage_trends)
   end
 
-  def group_issues_sub_menu_items
-    %w[
-      groups#issues
-      milestones#index
-      boards#index
-      boards#show
-    ]
-  end
-
   private
 
   def get_header_links

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?(:issues)
-  = nav_link(path: group_issues_sub_menu_items, unless: -> { current_path?('issues_analytics#show') }) do
-    = link_to issues_group_path(@group), data: { qa_selector: 'group_issues_item' }, class: 'has-sub-items' do
-      .nav-icon-container
-        = sprite_icon('issues')
-      %span.nav-item-name
-        = _('Issues')
-      %span.badge.badge-pill.count= issues_count
-
-    %ul.sidebar-sub-level-items{ data: { qa_selector: 'group_issues_sidebar_submenu'} }
-      = nav_link(path: group_issues_sub_menu_items, html_options: { class: "fly-out-top-item" } ) do
-        = link_to issues_group_path(@group) do
-          %strong.fly-out-top-item-name
-            = _('Issues')
-          %span.badge.badge-pill.count.issue_counter.fly-out-badge= issues_count
-
-      %li.divider.fly-out-top-item
-      = nav_link(path: 'groups#issues', html_options: { class: 'home' }) do
-        = link_to issues_group_path(@group), title: _('List') do
-          %span
-            = _('List')
-
-      - if group_sidebar_link?(:boards)
-        = nav_link(path: ['boards#index', 'boards#show']) do
-          = link_to group_boards_path(@group), title: boards_link_text, data: { qa_selector: 'group_issue_boards_link' } do
-            %span
-              = boards_link_text
-
-      - if group_sidebar_link?(:milestones)
-        = nav_link(path: 'milestones#index') do
-          = link_to group_milestones_path(@group), title: _('Milestones'), data: { qa_selector: 'group_milestones_link' } do
-            %span
-              = _('Milestones')
-
-      = render_if_exists 'layouts/nav/sidebar/group_iterations_link'
-
 - if group_sidebar_link?(:merge_requests)
   = nav_link(path: 'groups#merge_requests') do
     = link_to merge_requests_group_path(@group) do

ee/app/helpers/ee/nav_helper.rb

@@ -19,26 +19,5 @@ module EE
       controllers = %w(audit_logs)
       super.concat(controllers)
     end
-
-    override :group_issues_sub_menu_items
-    def group_issues_sub_menu_items
-      controllers = %w(issues_analytics#show)
-
-      if @group&.feature_available?(:iterations)
-        controllers = iterations_sub_menu_controllers
-      end
-
-      super.concat(controllers)
-    end
-
-    def iterations_sub_menu_controllers
-      paths = ['iterations#index', 'iterations#show']
-
-      if ::Feature.enabled?(:iteration_cadences, @group, default_enabled: :yaml)
-        paths << 'iteration_cadences#index'
-      end
-
-      paths
-    end
   end
 end

ee/app/views/layouts/nav/sidebar/_group_iterations_link.html.haml

-- if group_sidebar_link?(:iteration_cadences) || group_sidebar_link?(:iterations)
-  - iterations_path = Feature.enabled?(:iteration_cadences, @group, default_enabled: :yaml) ? group_iteration_cadences_path(@group) : group_iterations_path(@group)
-  = nav_link(path: iterations_sub_menu_controllers) do
-    = link_to iterations_path, data: { qa_selector: 'group_iterations_link' } do
-      %span
-        = _('Iterations')

ee/lib/ee/sidebars/groups/menus/issues_menu.rb

+# frozen_string_literal: true
+
+module EE
+  module Sidebars
+    module Groups
+      module Menus
+        module IssuesMenu
+          extend ::Gitlab::Utils::Override
+
+          override :configure_menu_items
+          def configure_menu_items
+            return false unless super
+
+            add_item(iterations_menu_item)
+
+            true
+          end
+
+          private
+
+          def iterations_menu_item
+            if !iterations_enabled? || !user_can_access_iterations?
+              return ::Sidebars::NilMenuItem.new(item_id: :iterations)
+            end
+
+            ::Sidebars::MenuItem.new(
+              title: _('Iterations'),
+              link: iterations_link,
+              active_routes: { path: iterations_paths },
+              item_id: :iterations
+            )
+          end
+
+          def iterations_enabled?
+            ::Feature.enabled?(:group_iterations, context.group, default_enabled: true) && context.group.licensed_feature_available?(:iterations)
+          end
+
+          def user_can_access_iterations?
+            (context.group.iteration_cadences_feature_flag_enabled? && can?(context.current_user, :read_iteration_cadence, context.group)) ||
+              can?(context.current_user, :read_iteration, context.group)
+          end
+
+          def iterations_link
+            strong_memoize(:iterations_link) do
+              context.group.iteration_cadences_feature_flag_enabled? ? group_iteration_cadences_path(context.group) : group_iterations_path(context.group)
+            end
+          end
+
+          def iterations_paths
+            strong_memoize(:iterations_paths) do
+              %w[iterations#index iterations#show iterations#new].tap do |paths|
+                paths << 'iteration_cadences#index' if context.group.iteration_cadences_feature_flag_enabled?
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/helpers/ee/nav_helper_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe NavHelper do
-  describe '#iterations_sub_menu_controllers' do
-    context 'when :iteration_cadences is turned on' do
-      it 'includes iteration_cadences#index path in the list' do
-        expect(helper.iterations_sub_menu_controllers).to include('iteration_cadences#index')
-      end
-    end
-
-    context 'when :iteration_cadences is NOT turned on' do
-      before do
-        stub_feature_flags(iteration_cadences: false)
-      end
-
-      it 'includes iteration_cadences#index path in the list' do
-        expect(helper.iterations_sub_menu_controllers).not_to include('iteration_cadences#index')
-      end
-    end
-  end
-end

ee/spec/lib/ee/sidebars/groups/menus/issues_menu_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Groups::Menus::IssuesMenu do
+  let_it_be(:owner) { create(:user) }
+
+  let(: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) }
+
+  describe 'Menu Items' do
+    subject { described_class.new(context).renderable_items.find { |e| e.item_id == item_id} }
+
+    describe 'Iterations' do
+      let(:item_id) { :iterations }
+      let(:iterations_enabled) { true }
+
+      before do
+        stub_licensed_features(iterations: iterations_enabled)
+      end
+
+      context 'when licensed feature iterations is not enabled' do
+        let(:iterations_enabled) { false }
+
+        it 'does not include iterations menu item' do
+          is_expected.to be_nil
+        end
+      end
+
+      context 'when licensed feature iterations is enabled' do
+        context 'when user can read iterations' do
+          it 'includes iterations menu item' do
+            is_expected.to be_present
+          end
+        end
+
+        context 'when user cannot read iterations' do
+          let(:user) { nil }
+
+          it 'does not include iterations menu item' do
+            is_expected.to be_nil
+          end
+        end
+      end
+
+      context 'when iteration_cadences are not enabled' do
+        before do
+          stub_feature_flags(iteration_cadences: false)
+        end
+
+        it 'contains the interation link' do
+          expect(subject.link).to include "/groups/#{group.full_path}/-/iterations"
+        end
+
+        it 'includes iterations active routes' do
+          expect(subject.active_routes[:path]).to contain_exactly('iterations#index', 'iterations#show', 'iterations#new')
+        end
+      end
+
+      context 'when iteration_cadences is enabled' do
+        it 'contains the iteration cadences link' do
+          expect(subject.link).to include "/groups/#{group.full_path}/-/cadences"
+        end
+
+        it 'includes iteration and iteration_cadences active routes' do
+          expect(subject.active_routes[:path]).to contain_exactly('iterations#index', 'iterations#show', 'iterations#new', 'iteration_cadences#index')
+        end
+      end
+    end
+  end
+end

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

@@ -121,6 +121,60 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
     end
   end
 
+  describe 'Issues menu' do
+    describe 'iterations link' do
+      let_it_be(:current_user) { create(:user) }
+
+      before do
+        group.add_guest(current_user)
+
+        allow(view).to receive(:current_user).and_return(current_user)
+      end
+
+      context 'with iterations licensed feature available' do
+        before do
+          stub_licensed_features(iterations: true)
+        end
+
+        context 'with group iterations feature flag enabled' do
+          before do
+            stub_feature_flags(group_iterations: true)
+          end
+
+          it 'is visible' do
+            render
+
+            expect(rendered).to have_text 'Iterations'
+          end
+        end
+
+        context 'with iterations feature flag disabled' do
+          before do
+            stub_feature_flags(group_iterations: false)
+          end
+
+          it 'is not visible' do
+            render
+
+            expect(rendered).not_to have_text 'Iterations'
+          end
+        end
+      end
+
+      context 'with iterations licensed feature disabled' do
+        before do
+          stub_licensed_features(iterations: false)
+        end
+
+        it 'is not visible' do
+          render
+
+          expect(rendered).not_to have_text 'Iterations'
+        end
+      end
+    end
+  end
+
   describe 'DevOps adoption link' do
     let!(:current_user) { create(:user) }
 
@@ -409,56 +463,4 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
       end
     end
   end
-
-  describe 'iterations link' do
-    let_it_be(:current_user) { create(:user) }
-
-    before do
-      group.add_guest(current_user)
-
-      allow(view).to receive(:current_user).and_return(current_user)
-    end
-
-    context 'with iterations licensed feature available' do
-      before do
-        stub_licensed_features(iterations: true)
-      end
-
-      context 'with group iterations feature flag enabled' do
-        before do
-          stub_feature_flags(group_iterations: true)
-        end
-
-        it 'is visible' do
-          render
-
-          expect(rendered).to have_text 'Iterations'
-        end
-      end
-
-      context 'with iterations feature flag disabled' do
-        before do
-          stub_feature_flags(group_iterations: false)
-        end
-
-        it 'is not visible' do
-          render
-
-          expect(rendered).not_to have_text 'Iterations'
-        end
-      end
-    end
-
-    context 'with iterations licensed feature disabled' do
-      before do
-        stub_licensed_features(iterations: false)
-      end
-
-      it 'is not visible' do
-        render
-
-        expect(rendered).not_to have_text 'Iterations'
-      end
-    end
-  end
 end

lib/sidebars/groups/menus/issues_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Groups
+    module Menus
+      class IssuesMenu < ::Sidebars::Menu
+        include Gitlab::Utils::StrongMemoize
+
+        override :configure_menu_items
+        def configure_menu_items
+          return unless can?(context.current_user, :read_group_issues, context.group)
+
+          add_item(list_menu_item)
+          add_item(boards_menu_item)
+          add_item(milestones_menu_item)
+
+          true
+        end
+
+        override :link
+        def link
+          issues_group_path(context.group)
+        end
+
+        override :title
+        def title
+          _('Issues')
+        end
+
+        override :sprite_icon
+        def sprite_icon
+          'issues'
+        end
+
+        override :has_pill?
+        def has_pill?
+          true
+        end
+
+        override :pill_count
+        def pill_count
+          strong_memoize(:pill_count) do
+            count_service = ::Groups::OpenIssuesCountService
+            count = count_service.new(context.group, context.current_user).count
+
+            format_cached_count(count_service, count)
+          end
+        end
+
+        override :pill_html_options
+        def pill_html_options
+          {
+            class: 'issue_counter'
+          }
+        end
+
+        private
+
+        def list_menu_item
+          ::Sidebars::MenuItem.new(
+            title: _('List'),
+            link: issues_group_path(context.group),
+            active_routes: { path: 'groups#issues' },
+            container_html_options: { aria: { label: _('Issues') } },
+            item_id: :issue_list
+          )
+        end
+
+        def boards_menu_item
+          unless can?(context.current_user, :read_group_boards, context.group)
+            return ::Sidebars::NilMenuItem.new(item_id: :boards)
+          end
+
+          title = context.group.multiple_issue_boards_available? ? s_('IssueBoards|Boards') : s_('IssueBoards|Board')
+
+          ::Sidebars::MenuItem.new(
+            title: title,
+            link: group_boards_path(context.group),
+            active_routes: { path: %w[boards#index boards#show] },
+            item_id: :boards
+          )
+        end
+
+        def milestones_menu_item
+          unless can?(context.current_user, :read_group_milestones, context.group)
+            return ::Sidebars::NilMenuItem.new(item_id: :milestones)
+          end
+
+          ::Sidebars::MenuItem.new(
+            title: _('Milestones'),
+            link: group_milestones_path(context.group),
+            active_routes: { path: 'milestones#index' },
+            item_id: :milestones
+          )
+        end
+      end
+    end
+  end
+end
+
+Sidebars::Groups::Menus::IssuesMenu.prepend_mod_with('Sidebars::Groups::Menus::IssuesMenu')

lib/sidebars/groups/panel.rb

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

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

@@ -14,8 +14,6 @@ module QA
               prepend QA::Page::Group::SubMenus::Common
 
               view 'app/views/layouts/nav/sidebar/_group_menus.html.haml' do
-                element :group_issue_boards_link
-                element :group_issues_item
                 element :group_sidebar_submenu
                 element :group_settings
               end
@@ -47,10 +45,6 @@ module QA
                 element :group_insights_link
               end
 
-              view 'ee/app/views/layouts/nav/sidebar/_group_iterations_link.html.haml' do
-                element :group_iterations_link
-              end
-
               view 'ee/app/views/groups/sidebar/_packages.html.haml' do
                 element :group_packages_item
                 element :group_packages_link
@@ -68,17 +62,17 @@ module QA
           end
 
           def go_to_issue_boards
-            hover_element(:group_issues_item) do
-              within_submenu(:group_issues_sidebar_submenu) do
-                click_element(:group_issue_boards_link)
+            hover_issues do
+              within_submenu do
+                click_element(:sidebar_menu_item_link, menu_item: 'Boards')
               end
             end
           end
 
           def go_to_group_iterations
-            hover_element(:group_issues_item) do
-              within_submenu(:group_issues_sidebar_submenu) do
-                click_element(:group_iterations_link)
+            hover_issues do
+              within_submenu do
+                click_element(:sidebar_menu_item_link, menu_item: 'Iterations')
               end
             end
           end

qa/qa/page/group/menu.rb

@@ -8,8 +8,6 @@ module QA
 
         view 'app/views/layouts/nav/sidebar/_group_menus.html.haml' do
           element :general_settings_link
-          element :group_issues_item
-          element :group_milestones_link
           element :group_settings
         end
 
@@ -63,7 +61,7 @@ module QA
         def go_to_milestones
           hover_issues do
             within_submenu do
-              click_element(:group_milestones_link)
+              click_element(:sidebar_menu_item_link, menu_item: 'Milestones')
             end
           end
         end
@@ -81,8 +79,8 @@ module QA
 
         def hover_issues
           within_sidebar do
-            scroll_to_element(:group_issues_item)
-            find_element(:group_issues_item).hover
+            scroll_to_element(:sidebar_menu_link, menu_item: 'Issues')
+            find_element(:sidebar_menu_link, menu_item: 'Issues').hover
 
             yield
           end

spec/helpers/nav_helper_spec.rb

@@ -112,16 +112,6 @@ RSpec.describe NavHelper do
     it { is_expected.to all(be_a(String)) }
   end
 
-  describe '.group_issues_sub_menu_items' do
-    subject { helper.group_issues_sub_menu_items }
-
-    before do
-      allow(helper).to receive(:current_user).and_return(nil)
-    end
-
-    it { is_expected.to all(be_a(String)) }
-  end
-
   describe '#page_has_markdown?' do
     using RSpec::Parameterized::TableSyntax
 

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Groups::Menus::IssuesMenu 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 'Menu Items' do
+    subject { menu.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 'List' do
+      let(:item_id) { :issue_list }
+
+      specify { is_expected.not_to be_nil }
+
+      it_behaves_like 'menu access rights'
+    end
+
+    describe 'Boards' do
+      let(:item_id) { :boards }
+
+      it_behaves_like 'menu access rights'
+    end
+
+    describe 'Milestones' do
+      let(:item_id) { :milestones }
+
+      it_behaves_like 'menu access rights'
+    end
+  end
+
+  it_behaves_like 'pill_count formatted results' do
+    let(:count_service) { ::Groups::OpenIssuesCountService }
+  end
+end

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

@@ -39,4 +39,30 @@ RSpec.describe 'layouts/nav/sidebar/_group' do
       expect(rendered).to have_link('Members', href: group_group_members_path(group))
     end
   end
+
+  describe 'Issues' do
+    it 'has a default link to the issue list path' do
+      render
+
+      expect(rendered).to have_link('Issues', href: issues_group_path(group))
+    end
+
+    it 'has a link to the issue list page' do
+      render
+
+      expect(rendered).to have_link('List', href: issues_group_path(group))
+    end
+
+    it 'has a link to the boards page' do
+      render
+
+      expect(rendered).to have_link('Board', href: group_boards_path(group))
+    end
+
+    it 'has a link to the milestones page' do
+      render
+
+      expect(rendered).to have_link('Milestones', href: group_milestones_path(group))
+    end
+  end
 end