Merge branch '326428-fj-add-external-issue-tracker-or-jira-menu' into 'master'

Add External Issue Tracker, Jira, and Labels menus

See merge request gitlab-org/gitlab!59372

app/helpers/sidebars_helper.rb

@@ -39,7 +39,8 @@ module SidebarsHelper
       current_user: user,
       container: project,
       learn_gitlab_experiment_enabled: learn_gitlab_experiment_enabled?(project),
-      current_ref: current_ref
+      current_ref: current_ref,
+      jira_issues_integration: project_jira_issues_integration?
     }
   end
 end

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

-- if project_nav_tab?(:external_issue_tracker)
-  - issue_tracker = @project.external_issue_tracker
-  - if issue_tracker.is_a?(JiraService) && project_jira_issues_integration?
-    = render_if_exists 'layouts/nav/sidebar/project_jira_issues_link', issue_tracker: issue_tracker
-  - else
-    = nav_link do
-      = link_to issue_tracker.issue_tracker_path, target: '_blank', rel: 'noopener noreferrer', class: 'shortcuts-external_tracker' do
-        .nav-icon-container
-          = sprite_icon('external-link')
-        %span.nav-item-name
-          = issue_tracker.title
-      %ul.sidebar-sub-level-items.is-fly-out-only
-        = nav_link(html_options: { class: "fly-out-top-item" } ) do
-          = link_to issue_tracker.issue_tracker_path, target: '_blank', rel: 'noopener noreferrer' do
-            %strong.fly-out-top-item-name
-              = issue_tracker.title
-
-- if (project_nav_tab? :labels) && !@project.issues_enabled?
-  = nav_link(controller: [:labels]) do
-    = link_to project_labels_path(@project), title: _('Labels'), class: 'shortcuts-labels qa-labels-items' do
-      .nav-icon-container
-        = sprite_icon('label')
-      %span.nav-item-name#js-onboarding-labels-link
-        = _('Labels')
-
 - if project_nav_tab? :merge_requests
   = nav_link(controller: @project.issues_enabled? ? :merge_requests : [:merge_requests, :milestones]) do
     = link_to project_merge_requests_path(@project), class: 'shortcuts-merge_requests', data: { qa_selector: 'merge_requests_link' } do

app/views/shared/nav/_sidebar_menu.html.haml

@@ -15,7 +15,7 @@
 
   %ul.sidebar-sub-level-items{ class: ('is-fly-out-only' unless sidebar_menu.has_items?) }
     = nav_link(**sidebar_menu.all_active_routes, html_options: { class: 'fly-out-top-item' } ) do
-      = link_to sidebar_menu.link, aria: { label: sidebar_menu.title } do
+      = link_to sidebar_menu.link, **sidebar_menu.collapsed_container_html_options do
         %strong.fly-out-top-item-name
           = sidebar_menu.title
         - if sidebar_menu.has_pill?

ee/app/helpers/ee/projects_helper.rb

@@ -198,12 +198,6 @@ module EE
       ]
     end
 
-    def sidebar_external_tracker_paths
-      %w[
-        projects/integrations/jira/issues#index
-      ]
-    end
-
     def sidebar_on_demand_scans_paths
       %w[
         projects/on_demand_scans#index

ee/app/helpers/ee/services_helper.rb

@@ -6,7 +6,7 @@ module EE
 
     override :project_jira_issues_integration?
     def project_jira_issues_integration?
-      @project.jira_issues_integration_available? && @project.jira_service.issues_enabled
+      @project.jira_issues_integration_available? && @project.jira_service&.issues_enabled
     end
 
     override :integration_form_data

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

-= nav_link(path: sidebar_external_tracker_paths) do
-  = link_to project_integrations_jira_issues_path(@project) do
-    .nav-icon-container
-      -# Hardcode sizes so image doesn't flash before CSS loads https://gitlab.com/gitlab-org/gitlab/-/issues/321022
-      = image_tag('logos/jira-gray.svg', size: 16)
-    %span.nav-item-name.qa-settings-item#js-onboarding-settings-link
-      = s_('JiraService|Jira Issues')
-
-  %ul.sidebar-sub-level-items
-    = nav_link(path: sidebar_external_tracker_paths, html_options: { class: 'fly-out-top-item' } ) do
-      = link_to project_integrations_jira_issues_path(@project) do
-        %strong.fly-out-top-item-name
-          = s_('JiraService|Jira Issues')
-    %li.divider.fly-out-top-item
-    = nav_link(path: sidebar_external_tracker_paths[0]) do
-      = link_to project_integrations_jira_issues_path(@project), title: s_('JiraService|Issue List') do
-        %span
-          = s_('JiraService|Issue List')
-    = nav_link do
-      = link_to issue_tracker.issue_tracker_path, target: '_blank', rel: 'noopener noreferrer' do
-        %span
-          = s_('JiraService|Open Jira')
-          = sprite_icon('external-link', css_class: 'gl-vertical-align-text-bottom')

ee/lib/ee/sidebars/projects/panel.rb

+# frozen_string_literal: true
+
+module EE
+  module Sidebars
+    module Projects
+      module Panel
+        extend ::Gitlab::Utils::Override
+
+        override :configure_menus
+        def configure_menus
+          super
+
+          if jira_menu.render?
+            replace_menu(::Sidebars::Projects::Menus::ExternalIssueTrackerMenu, jira_menu)
+          end
+        end
+
+        private
+
+        def jira_menu
+          @jira_menu ||= ::Sidebars::Projects::Menus::JiraMenu.new(context)
+        end
+      end
+    end
+  end
+end

ee/lib/sidebars/projects/menus/jira_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Projects
+    module Menus
+      class JiraMenu < ::Sidebars::Menu
+        override :configure_menu_items
+        def configure_menu_items
+          return false unless external_issue_tracker
+
+          add_item(issue_list_menu_item)
+          add_item(open_jira_menu_item)
+
+          true
+        end
+
+        override :link
+        def link
+          project_integrations_jira_issues_path(context.project)
+        end
+
+        override :title
+        def title
+          s_('JiraService|Jira Issues')
+        end
+
+        override :title_html_options
+        def title_html_options
+          {
+            id: 'js-onboarding-settings-link'
+          }
+        end
+
+        override :image_path
+        def image_path
+          'logos/jira-gray.svg'
+        end
+
+        # Hardcode sizes so image doesn't flash before CSS loads https://gitlab.com/gitlab-org/gitlab/-/issues/321022
+        override :image_html_options
+        def image_html_options
+          {
+            size: 16
+          }
+        end
+
+        override :render?
+        def render?
+          external_issue_tracker.is_a?(JiraService) && context.jira_issues_integration
+        end
+
+        private
+
+        def external_issue_tracker
+          @external_issue_tracker ||= context.project.external_issue_tracker
+        end
+
+        def issue_list_menu_item
+          ::Sidebars::MenuItem.new(
+            title: s_('JiraService|Issue List'),
+            link: project_integrations_jira_issues_path(context.project),
+            active_routes: { path: 'projects/integrations/jira/issues#index' },
+            item_id: :issue_list
+          )
+        end
+
+        def open_jira_menu_item
+          ::Sidebars::MenuItem.new(
+            title: s_('JiraService|Open Jira'),
+            link: external_issue_tracker.issue_tracker_path,
+            active_routes: {},
+            item_id: :open_jira,
+            sprite_icon: 'external-link',
+            sprite_icon_html_options: { css_class: 'gl-vertical-align-text-bottom' },
+            container_html_options: {
+              target: '_blank',
+              rel: 'noopener noreferrer'
+            }
+          )
+        end
+      end
+    end
+  end
+end

ee/spec/lib/sidebars/projects/menus/jira_menu_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Projects::Menus::JiraMenu do
+  let_it_be_with_refind(:project) { create(:project, has_external_issue_tracker: true) }
+
+  let(:user) { project.owner }
+  let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, jira_issues_integration: jira_issues_integration) }
+  let(:jira_issues_integration) { false }
+
+  subject { described_class.new(context) }
+
+  describe 'render?' do
+    context 'when issue tracker is not a JiraService' do
+      it 'returns false' do
+        create(:custom_issue_tracker_service, active: true, project: project, project_url: 'http://test.com')
+
+        expect(subject.render?).to eq false
+      end
+    end
+
+    context 'when issue tracker is a JiraService' do
+      let!(:jira) { create(:jira_service, project: project, project_key: 'GL') }
+
+      context 'when issues integration is disabled' do
+        it 'returns false' do
+          expect(subject.render?).to eq false
+        end
+      end
+
+      context 'when issues integration is enabled' do
+        let(:jira_issues_integration) { true }
+
+        it 'returns true' do
+          expect(subject.render?).to eq true
+        end
+
+        it 'contains issue list and open jira menu items' do
+          expect(subject.items).not_to be_empty
+          expect(subject.items[0].item_id).to eq :issue_list
+          expect(subject.items[1].item_id).to eq :open_jira
+        end
+      end
+    end
+  end
+end

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

@@ -39,6 +39,48 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
   end
 
+  describe 'Jira' do
+    let_it_be_with_refind(:project) { create(:project, has_external_issue_tracker: true) }
+
+    context 'when Jira service integration is not set' do
+      it 'does not have a link to the Jira issues menu' do
+        render
+
+        expect(rendered).not_to have_link('Jira Issues', href: project_integrations_jira_issues_path(project))
+      end
+    end
+
+    context 'when Jira service integration is set' do
+      let!(:jira) { create(:jira_service, project: project, issues_enabled: true, project_key: 'GL') }
+
+      before do
+        stub_licensed_features(jira_issues_integration: true)
+      end
+
+      it 'has a link to the Jira issue tracker' do
+        render
+
+        expect(rendered).to have_link('Jira Issues', href: project_integrations_jira_issues_path(project))
+      end
+
+      describe 'Issue List' do
+        it 'has a link to Jira issue list' do
+          render
+
+          expect(rendered).to have_link('Issue List', href: project_integrations_jira_issues_path(project))
+        end
+      end
+
+      describe 'Open Jira' do
+        it 'has a link to open Jira' do
+          render
+
+          expect(rendered).to have_link('Open Jira', href: project.external_issue_tracker.issue_tracker_path)
+        end
+      end
+    end
+  end
+
   describe 'Operations main link' do
     let(:user) { create(:user) }
 

lib/sidebars/concerns/container_with_html_options.rb

@@ -18,6 +18,22 @@ module Sidebars
         {}
       end
 
+      # The attributes returned from this method
+      # will be applied to helper methods like
+      # `link_to` or the div containing the container
+      # when it is collapsed.
+      def collapsed_container_html_options
+        {
+          aria: { label: title }
+        }.merge(extra_collapsed_container_html_options)
+      end
+
+      # Classes should mostly override this method
+      # and not `collapsed_container_html_options`.
+      def extra_collapsed_container_html_options
+        {}
+      end
+
       # Attributes to pass to the html_options attribute
       # in the helper method that sets the active class
       # on each element.

lib/sidebars/concerns/positionable_list.rb

@@ -34,6 +34,16 @@ module Sidebars
         end
       end
 
+      def replace_element(list, element_to_replace, new_element)
+        return unless new_element
+
+        index = index_of(list, element_to_replace)
+
+        return unless index
+
+        list[index] = new_element
+      end
+
       private
 
       # Classes including this method will have to define

lib/sidebars/panel.rb

@@ -32,6 +32,10 @@ module Sidebars
       insert_element_after(@menus, after_menu, new_menu)
     end
 
+    def replace_menu(menu_to_replace, new_menu)
+      replace_element(@menus, menu_to_replace, new_menu)
+    end
+
     def set_scope_menu(scope_menu)
       @scope_menu = scope_menu
     end

lib/sidebars/projects/menus/external_issue_tracker_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Projects
+    module Menus
+      class ExternalIssueTrackerMenu < ::Sidebars::Menu
+        override :link
+        def link
+          external_issue_tracker.issue_tracker_path
+        end
+
+        override :extra_container_html_options
+        def extra_container_html_options
+          {
+            target: '_blank',
+            rel: 'noopener noreferrer',
+            class: 'shortcuts-external_tracker'
+          }
+        end
+
+        override :extra_collapsed_container_html_options
+        def extra_collapsed_container_html_options
+          {
+            target: '_blank',
+            rel: 'noopener noreferrer'
+          }
+        end
+
+        override :title
+        def title
+          external_issue_tracker.title
+        end
+
+        override :title_html_options
+        def title_html_options
+          {
+            id: 'js-onboarding-issues-link'
+          }
+        end
+
+        override :sprite_icon
+        def sprite_icon
+          'external-link'
+        end
+
+        override :render?
+        def render?
+          external_issue_tracker.present?
+        end
+
+        private
+
+        def external_issue_tracker
+          @external_issue_tracker ||= context.project.external_issue_tracker
+        end
+      end
+    end
+  end
+end

lib/sidebars/projects/menus/labels_menu.rb

+# frozen_string_literal: true
+
+module Sidebars
+  module Projects
+    module Menus
+      class LabelsMenu < ::Sidebars::Menu
+        override :link
+        def link
+          project_labels_path(context.project)
+        end
+
+        override :extra_container_html_options
+        def extra_container_html_options
+          {
+            class: 'shortcuts-labels'
+          }
+        end
+
+        override :title
+        def title
+          _('Labels')
+        end
+
+        override :title_html_options
+        def title_html_options
+          {
+            id: 'js-onboarding-labels-link'
+          }
+        end
+
+        override :active_routes
+        def active_routes
+          { controller: :labels }
+        end
+
+        override :sprite_icon
+        def sprite_icon
+          'label'
+        end
+
+        override :render?
+        def render?
+          can?(context.current_user, :read_label, context.project) && !context.project.issues_enabled?
+        end
+      end
+    end
+  end
+end

lib/sidebars/projects/panel.rb

@@ -11,6 +11,8 @@ module Sidebars
         add_menu(Sidebars::Projects::Menus::LearnGitlabMenu.new(context))
         add_menu(Sidebars::Projects::Menus::RepositoryMenu.new(context))
         add_menu(Sidebars::Projects::Menus::IssuesMenu.new(context))
+        add_menu(Sidebars::Projects::Menus::ExternalIssueTrackerMenu.new(context))
+        add_menu(Sidebars::Projects::Menus::LabelsMenu.new(context))
       end
 
       override :render_raw_menus_partial
@@ -25,3 +27,5 @@ module Sidebars
     end
   end
 end
+
+Sidebars::Projects::Panel.prepend_if_ee('EE::Sidebars::Projects::Panel')

spec/lib/sidebars/concerns/container_with_html_options_spec.rb

@@ -18,4 +18,10 @@ RSpec.describe Sidebars::Concerns::ContainerWithHtmlOptions do
       expect(subject.container_html_options).to eq(aria: { label: 'Foo' })
     end
   end
+
+  describe '#collapsed_container_html_options' do
+    it 'includes by default aria-label attribute' do
+      expect(subject.collapsed_container_html_options).to eq(aria: { label: 'Foo' })
+    end
+  end
 end

spec/lib/sidebars/panel_spec.rb

@@ -101,4 +101,27 @@ RSpec.describe Sidebars::Panel do
       end
     end
   end
+
+  describe '#replace_element' do
+    let(:user) { build(:user) }
+    let(:list) { [1, user] }
+
+    it 'replace existing element in the list' do
+      panel.replace_element(list, Integer, 2)
+
+      expect(list).to eq [2, user]
+    end
+
+    it 'does not add nil elements' do
+      panel.replace_element(list, Integer, nil)
+
+      expect(list).to eq [1, user]
+    end
+
+    it 'does not add the element if the other element is not found' do
+      panel.replace_element(list, Project, 2)
+
+      expect(list).to eq [1, user]
+    end
+  end
 end

spec/lib/sidebars/projects/menus/external_issue_tracker_menu_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Projects::Menus::ExternalIssueTrackerMenu do
+  let(:project) { build(:project) }
+  let(:user) { project.owner }
+  let(:jira_issues_integration_active) { false }
+  let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, jira_issues_integration: jira_issues_integration_active) }
+
+  subject { described_class.new(context) }
+
+  it 'does not contain any sub menu' do
+    expect(subject.items).to be_empty
+  end
+
+  describe '#render?' do
+    before do
+      expect(subject).to receive(:external_issue_tracker).and_return(external_issue_tracker).at_least(1)
+    end
+
+    context 'when active external issue tracker' do
+      let(:external_issue_tracker) { build(:custom_issue_tracker_service, project: project) }
+
+      context 'is present' do
+        it 'returns true' do
+          expect(subject.render?).to be_truthy
+        end
+      end
+
+      context 'is not present' do
+        let(:external_issue_tracker) { nil }
+
+        it 'returns false' do
+          expect(subject.render?).to be_falsey
+        end
+      end
+    end
+  end
+end

spec/lib/sidebars/projects/menus/labels_menu_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Projects::Menus::LabelsMenu do
+  let(:project) { build(:project) }
+  let(:user) { project.owner }
+  let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) }
+
+  subject { described_class.new(context) }
+
+  it 'does not contain any sub menu' do
+    expect(subject.items).to be_empty
+  end
+
+  describe '#render?' do
+    let(:issues_enabled) { true }
+
+    before do
+      allow(project).to receive(:issues_enabled?).and_return(issues_enabled)
+    end
+
+    context 'when user can read labels' do
+      context 'when issues feature is enabled' do
+        it 'returns false' do
+          expect(subject.render?).to be_falsey
+        end
+      end
+
+      context 'when issues feature is disabled' do
+        let(:issues_enabled) { false }
+
+        it 'returns true' do
+          expect(subject.render?).to be_truthy
+        end
+      end
+    end
+
+    context 'when user cannot read labels' do
+      let(:user) { nil }
+
+      it 'returns false' do
+        expect(subject.render?).to be_falsey
+      end
+    end
+  end
+end

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

@@ -181,6 +181,68 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
     end
   end
 
+  describe 'External Issue Tracker' do
+    let_it_be_with_refind(:project) { create(:project, has_external_issue_tracker: true) }
+
+    context 'with custom external issue tracker' do
+      let(:external_issue_tracker_url) { 'http://test.com' }
+
+      let!(:external_issue_tracker) do
+        create(:custom_issue_tracker_service, active: external_issue_tracker_active, project: project, project_url: external_issue_tracker_url)
+      end
+
+      context 'when external issue tracker is configured and active' do
+        let(:external_issue_tracker_active) { true }
+
+        it 'has a link to the external issue tracker' do
+          render
+
+          expect(rendered).to have_link(external_issue_tracker.title, href: external_issue_tracker_url)
+        end
+      end
+
+      context 'when external issue tracker is not configured and active' do
+        let(:external_issue_tracker_active) { false }
+
+        it 'does not have a link to the external issue tracker' do
+          render
+
+          expect(rendered).not_to have_link(external_issue_tracker.title)
+        end
+      end
+    end
+
+    context 'with Jira issue tracker' do
+      let_it_be(:jira) { create(:jira_service, project: project, issues_enabled: false) }
+
+      it 'has a link to the Jira issue tracker' do
+        render
+
+        expect(rendered).to have_link('Jira', href: project.external_issue_tracker.issue_tracker_path)
+      end
+    end
+  end
+
+  describe 'Labels' do
+    context 'when issues are not enabled' do
+      it 'has a link to the labels path' do
+        project.project_feature.update!(issues_access_level: ProjectFeature::DISABLED)
+
+        render
+
+        expect(rendered).to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels')
+      end
+    end
+
+    context 'when issues are enabled' do
+      it 'does not have a link to the labels path' do
+        render
+
+        expect(rendered).not_to have_link('Labels', href: project_labels_path(project), class: 'shortcuts-labels')
+      end
+    end
+  end
+
   describe 'packages tab' do
     before do
       stub_container_registry_config(enabled: true)