Merge branch...

Merge branch '342466-make-jira-issue-menu-items-sub-menu-items-of-the-issues-top-level-menu-item' into 'master'

Move Jira issues menu to be submenu of Issues

See merge request gitlab-org/gitlab!74601

app/assets/images/logos/jira-gray.svg

-<svg id="Logos" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="80" height="80" viewBox="0 0 80 80"><defs><style>.cls-1{fill:#7a869a;}.cls-2{fill:url(#linear-gradient);}.cls-3{fill:url(#linear-gradient-2);}</style><linearGradient id="linear-gradient" x1="38.11" y1="18.54" x2="23.17" y2="33.48" gradientUnits="userSpaceOnUse"><stop offset="0.18" stop-color="#344563"/><stop offset="1" stop-color="#7a869a"/></linearGradient><linearGradient id="linear-gradient-2" x1="42.07" y1="61.47" x2="56.98" y2="46.55" xlink:href="#linear-gradient"/></defs><title>jira software-icon-gradient-neutral</title><path class="cls-1" d="M74.18,38,43,6.9l-3-3h0L16.58,27.32h0L5.86,38a2.86,2.86,0,0,0,0,4.05L27.28,63.51,40,76.25,63.47,52.81l.36-.36L74.18,42.09A2.86,2.86,0,0,0,74.18,38ZM40,50.77l-10.7-10.7L40,29.37l10.7,10.7Z"/><path class="cls-2" d="M40,29.37A18,18,0,0,1,40,4L16.54,27.37,29.28,40.11,40,29.37Z"/><path class="cls-3" d="M50.75,40,40,50.77a18,18,0,0,1,0,25.48h0L63.5,52.78Z"/></svg>

doc/integration/jira/issues.md

@@ -101,10 +101,10 @@ Consider this example:
 You can browse, search, and view issues from a selected Jira project directly in GitLab,
 if your GitLab administrator [has configured it](configure.md).
 
-To do this, in GitLab, go to your project and select **Jira > Issues list**. The issue list
+To do this, in GitLab, go to your project and select **Issues > Jira issues**. The issue list
 sorts by **Created date** by default, with the newest issues listed at the top:
 
-![Jira issues integration enabled](img/open_jira_issues_list_v13.2.png)
+![Jira issues integration enabled](img/open_jira_issues_list_v14_6.png)
 
 - To display the most recently updated issues first, select **Last updated**.
 - You can [search and filter](#search-and-filter-the-issues-list) the issues list.

ee/app/views/projects/integrations/jira/issues/index.html.haml

-- page_title _('Jira Issues')
+- page_title s_('JiraService|Jira issues')
 - add_page_specific_style 'page_bundles/issues_list'
 
 .js-jira-issues-list{ data: { issues_fetch_path: project_integrations_jira_issues_path(@project, format: :json),

ee/app/views/projects/integrations/jira/issues/show.html.haml

-- add_to_breadcrumbs _('Jira Issues'), project_integrations_jira_issues_path(@project)
+- add_to_breadcrumbs s_('JiraService|Jira issues'), project_integrations_jira_issues_path(@project)
 - breadcrumb_title jira_issue_breadcrumb_link(@issue_json[:references][:relative])
 - page_title html_escape(@issue_json[:title])
 

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

@@ -13,10 +13,16 @@ module EE
 
             add_item(iterations_menu_item)
             add_item(requirements_menu_item)
+            add_item(jira_issue_list_menu_item)
+            add_item(jira_external_link_menu_item)
 
             true
           end
 
+          def show_jira_menu_items?
+            external_issue_tracker.is_a?(Integrations::Jira) && context.jira_issues_integration
+          end
+
           private
 
           def iterations_menu_item
@@ -49,6 +55,37 @@ module EE
               item_id: :requirements
             )
           end
+
+          def external_issue_tracker
+            @external_issue_tracker ||= context.project.external_issue_tracker
+          end
+
+          def jira_issue_list_menu_item
+            return ::Sidebars::NilMenuItem.new(item_id: :jira_issue_list) unless show_jira_menu_items?
+
+            ::Sidebars::MenuItem.new(
+              title: s_('JiraService|Jira issues'),
+              link: project_integrations_jira_issues_path(context.project),
+              active_routes: { controller: 'projects/integrations/jira/issues' },
+              item_id: :jira_issue_list
+            )
+          end
+
+          def jira_external_link_menu_item
+            return ::Sidebars::NilMenuItem.new(item_id: :jira_external_link) unless show_jira_menu_items?
+
+            ::Sidebars::MenuItem.new(
+              title: s_('JiraService|Open Jira'),
+              link: external_issue_tracker.issue_tracker_path,
+              active_routes: {},
+              item_id: :jira_external_link,
+              sprite_icon: 'external-link',
+              container_html_options: {
+                target: '_blank',
+                rel: 'noopener noreferrer'
+              }
+            )
+          end
         end
       end
     end

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

@@ -10,16 +10,10 @@ module EE
         def configure_menus
           super
 
-          if jira_menu.render?
-            replace_menu(::Sidebars::Projects::Menus::ExternalIssueTrackerMenu, jira_menu)
+          if ::Sidebars::Projects::Menus::IssuesMenu.new(context).show_jira_menu_items?
+            remove_menu(::Sidebars::Projects::Menus::ExternalIssueTrackerMenu)
           end
         end
-
-        private
-
-        def jira_menu
-          @jira_menu ||= ::Sidebars::Projects::Menus::JiraMenu.new(context)
-        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?(Integrations::Jira) && 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: { controller: 'projects/integrations/jira/issues' },
-            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',
-            container_html_options: {
-              target: '_blank',
-              rel: 'noopener noreferrer'
-            }
-          )
-        end
-      end
-    end
-  end
-end

ee/spec/features/projects/integrations/user_activates_jira_spec.rb

@@ -25,10 +25,9 @@ RSpec.describe 'User activates Jira', :js do
         click_test_then_save_integration(expect_test_to_fail: false)
       end
 
-      it 'adds Jira links to sidebar menu' do
+      it 'adds Jira links to "Issues" sidebar menu' do
         page.within('.nav-sidebar') do
-          expect(page).to have_link('Jira Issues', href: project_integrations_jira_issues_path(project))
-          expect(page).to have_link('Issue List', href: project_integrations_jira_issues_path(project), visible: false)
+          expect(page).to have_link('Jira issues', href: project_integrations_jira_issues_path(project), visible: false)
           expect(page).to have_link('Open Jira', href: url, visible: false)
           expect(page).not_to have_link('Jira', href: url)
         end
@@ -44,10 +43,9 @@ RSpec.describe 'User activates Jira', :js do
         click_save_integration
       end
 
-      it 'does not show Jira links in sidebar menu' do
+      it 'does not show Jira links in "Issues" sidebar menu' do
         page.within('.nav-sidebar') do
-          expect(page).not_to have_link('Jira Issues', href: project_integrations_jira_issues_path(project))
-          expect(page).not_to have_link('Issue List', href: project_integrations_jira_issues_path(project), visible: false)
+          expect(page).not_to have_link('Jira issues', href: project_integrations_jira_issues_path(project), visible: false)
           expect(page).not_to have_link('Open Jira', href: url, visible: false)
           expect(page).to have_link('Jira', href: url)
         end

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

@@ -70,4 +70,43 @@ RSpec.describe Sidebars::Projects::Menus::IssuesMenu do
       end
     end
   end
+
+  describe 'Jira issues' do
+    let_it_be_with_refind(:project) { create(:project, has_external_issue_tracker: true) }
+
+    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) }
+
+    context 'when issue tracker is not Jira' do
+      it 'does not include Jira issues menu items' do
+        create(:custom_issue_tracker_integration, active: true, project: project, project_url: 'http://test.com')
+
+        expect(subject.show_jira_menu_items?).to eq(false)
+        expect(subject.renderable_items.any? { |e| e.item_id == :jira_issue_list}).to eq(false)
+      end
+    end
+
+    context 'when issue tracker is Jira' do
+      let_it_be(:jira) { create(:jira_integration, project: project, project_key: 'GL') }
+
+      context 'when issues integration is disabled' do
+        it 'does not include Jira issues menu items' do
+          expect(subject.show_jira_menu_items?).to eq(false)
+          expect(subject.renderable_items.any? { |e| e.item_id == :jira_issue_list}).to eq(false)
+        end
+      end
+
+      context 'when issues integration is enabled' do
+        let(:jira_issues_integration) { true }
+
+        it 'includes Jira issues menu items' do
+          expect(subject.show_jira_menu_items?).to eq(true)
+          expect(subject.renderable_items.any? { |e| e.item_id == :jira_issue_list}).to eq(true)
+          expect(subject.renderable_items.any? { |e| e.item_id == :jira_external_link}).to eq(true)
+        end
+      end
+    end
+  end
 end

ee/spec/lib/ee/sidebars/projects/panel_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Projects::Panel do
+  let(:project) { build(:project) }
+  let(:context) { Sidebars::Projects::Context.new(current_user: nil, container: project) }
+
+  describe 'ExternalIssueTrackerMenu' do
+    before do
+      allow_next_instance_of(Sidebars::Projects::Menus::IssuesMenu) do |issues_menu|
+        allow(issues_menu).to receive(:show_jira_menu_items?).and_return(show_jira_menu_items)
+      end
+    end
+
+    subject { described_class.new(context) }
+
+    def contains_external_issue_tracker_menu
+      subject.instance_variable_get(:@menus).any? { |i| i.is_a?(Sidebars::Projects::Menus::ExternalIssueTrackerMenu) }
+    end
+
+    context 'when show_jira_menu_items? is false' do
+      let(:show_jira_menu_items) { false }
+
+      it 'contains ExternalIssueTracker menu' do
+        expect(contains_external_issue_tracker_menu).to be(true)
+      end
+    end
+
+    context 'when show_jira_menu_items? is true' do
+      let(:show_jira_menu_items) { true }
+
+      it 'does not contain ExternalIssueTracker menu' do
+        expect(contains_external_issue_tracker_menu).to be(false)
+      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 Jira' do
-      it 'returns false' do
-        create(:custom_issue_tracker_integration, active: true, project: project, project_url: 'http://test.com')
-
-        expect(subject.render?).to eq false
-      end
-    end
-
-    context 'when issue tracker is Jira' do
-      let!(:jira) { create(:jira_integration, 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.renderable_items).not_to be_empty
-          expect(subject.renderable_items[0].item_id).to eq :issue_list
-          expect(subject.renderable_items[1].item_id).to eq :open_jira
-        end
-      end
-    end
-  end
-end

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

@@ -60,42 +60,38 @@ RSpec.describe 'layouts/nav/sidebar/_project' do
         end
       end
     end
-  end
 
     describe 'Jira' do
       let_it_be_with_refind(:project) { create(:project, has_external_issue_tracker: true) }
 
+      let(:user) { project.owner }
+
+      before do
+        allow(view).to receive(:current_user).and_return(user)
+      end
+
       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))
+          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_integration, project: project, issues_enabled: true, project_key: 'GL') }
+        let_it_be(:jira) { create(:jira_integration, 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
+        it 'has a link to Jira issues list' do
           render
 
-          expect(rendered).to have_link('Issue List', href: project_integrations_jira_issues_path(project))
-        end
+          expect(rendered).to have_link('Jira issues', href: project_integrations_jira_issues_path(project))
         end
 
-      describe 'Open Jira' do
-        it 'has a link to open Jira' do
+        it 'has an external link to open Jira' do
           render
 
           expect(rendered).to have_link('Open Jira', href: project.external_issue_tracker.issue_tracker_path)

lib/sidebars/concerns/positionable_list.rb

@@ -44,6 +44,14 @@ module Sidebars
         list[index] = new_element
       end
 
+      def remove_element(list, element_to_remove)
+        index = index_of(list, element_to_remove)
+
+        return unless index
+
+        list.slice!(index)
+      end
+
       private
 
       # Classes including this method will have to define

lib/sidebars/panel.rb

@@ -37,6 +37,10 @@ module Sidebars
       replace_element(@menus, menu_to_replace, new_menu)
     end
 
+    def remove_menu(menu_to_remove)
+      remove_element(@menus, menu_to_remove)
+    end
+
     def set_scope_menu(scope_menu)
       @scope_menu = scope_menu
     end

locale/gitlab.pot

@@ -19648,9 +19648,6 @@ msgstr ""
 msgid "Japanese language support using"
 msgstr ""
 
-msgid "Jira Issues"
-msgstr ""
-
 msgid "Jira display name"
 msgstr ""
 
@@ -19780,18 +19777,12 @@ msgstr ""
 msgid "JiraService|If different from Web URL."
 msgstr ""
 
-msgid "JiraService|Issue List"
-msgstr ""
-
 msgid "JiraService|Issues created from vulnerabilities in this project will be Jira issues, even if GitLab issues are enabled."
 msgstr ""
 
 msgid "JiraService|Jira API URL"
 msgstr ""
 
-msgid "JiraService|Jira Issues"
-msgstr ""
-
 msgid "JiraService|Jira comments are created when an issue is referenced in a commit."
 msgstr ""
 
@@ -19801,6 +19792,9 @@ msgstr ""
 msgid "JiraService|Jira issue type"
 msgstr ""
 
+msgid "JiraService|Jira issues"
+msgstr ""
+
 msgid "JiraService|Jira project key"
 msgstr ""
 

spec/features/projects/integrations/user_activates_jira_spec.rb

@@ -26,8 +26,7 @@ RSpec.describe 'User activates Jira', :js do
       unless Gitlab.ee?
         it 'adds Jira link to sidebar menu' do
           page.within('.nav-sidebar') do
-            expect(page).not_to have_link('Jira Issues')
-            expect(page).not_to have_link('Issue List', visible: false)
+            expect(page).not_to have_link('Jira issues', visible: false)
             expect(page).not_to have_link('Open Jira', href: url, visible: false)
             expect(page).to have_link('Jira', href: url)
           end

spec/lib/sidebars/menu_spec.rb

@@ -153,6 +153,25 @@ RSpec.describe Sidebars::Menu do
     end
   end
 
+  describe '#remove_element' do
+    let(:item1) { Sidebars::MenuItem.new(title: 'foo1', link: 'foo1', active_routes: {}, item_id: :foo1) }
+    let(:item2) { Sidebars::MenuItem.new(title: 'foo2', link: 'foo2', active_routes: {}, item_id: :foo2) }
+    let(:item3) { Sidebars::MenuItem.new(title: 'foo3', link: 'foo3', active_routes: {}, item_id: :foo3) }
+    let(:list) { [item1, item2, item3] }
+
+    it 'removes specific element' do
+      menu.remove_element(list, :foo2)
+
+      expect(list).to eq [item1, item3]
+    end
+
+    it 'does not remove nil elements' do
+      menu.remove_element(list, nil)
+
+      expect(list).to eq [item1, item2, item3]
+    end
+  end
+
   describe '#container_html_options' do
     before do
       allow(menu).to receive(:title).and_return('Foo Menu')