Improving packages navigation in sidebar

Made "Packages" top level sidebar item available in projects
Added list sub item
Added container registry sub item
Added logic to apply active class to either nav item
Added logic to change packages link depending on config
Made list nav item only available in EE
Added new tests for menu changes

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

@@ -274,19 +274,6 @@
 
             = render_if_exists 'layouts/nav/sidebar/project_feature_flags_link'
 
-      - if project_nav_tab? :container_registry
-        = nav_link(controller: %w[projects/registry/repositories]) do
-          = link_to project_container_registry_index_path(@project), class: 'shortcuts-container-registry' do
-            .nav-icon-container
-              = sprite_icon('disk')
-            %span.nav-item-name
-              = _('Registry')
-          %ul.sidebar-sub-level-items.is-fly-out-only
-            = nav_link(controller: %w[projects/registry/repositories], html_options: { class: "fly-out-top-item" } ) do
-              = link_to project_container_registry_index_path(@project) do
-                %strong.fly-out-top-item-name
-                  = _('Registry')
-
       = render_if_exists 'layouts/nav/sidebar/project_packages_link'
 
       - if project_nav_tab? :wiki

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

+- if project_nav_tab? :container_registry
+  = nav_link controller: :repositories do
+    = link_to project_container_registry_index_path(@project) do
+      .nav-icon-container
+        = sprite_icon('package')
+      %span.nav-item-name
+        = _('Packages')
+    %ul.sidebar-sub-level-items
+      = nav_link(controller: :repositories, html_options: { class: "fly-out-top-item" } ) do
+        = link_to project_container_registry_index_path(@project) do
+          %strong.fly-out-top-item-name
+            = _('Packages')
+      %li.divider.fly-out-top-item
+      = nav_link controller: :repositories do
+        = link_to project_container_registry_index_path(@project), class: 'shortcuts-container-registry', title: _('Container Registry') do
+          %span= _('Container Registry')

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

-- if project_nav_tab? :packages
-  = nav_link(controller: %w[projects/packages/packages]) do
-    = link_to project_packages_path(@project) do
+- packages_link = project_nav_tab?(:packages) ? project_packages_path(@project) : project_container_registry_index_path(@project)
+
+- if (project_nav_tab?(:packages) || project_nav_tab?(:container_registry))
+  = nav_link controller: [:packages, :repositories] do
+    = link_to packages_link do
       .nav-icon-container
         = sprite_icon('package')
       %span.nav-item-name
         = _('Packages')
-    %ul.sidebar-sub-level-items.is-fly-out-only
-      = nav_link(controller: %w[projects/packages/packages], html_options: { class: "fly-out-top-item" } ) do
-        = link_to project_packages_path(@project) do
+    %ul.sidebar-sub-level-items
+      = nav_link(controller: [:packages, :repositories], html_options: { class: "fly-out-top-item" } ) do
+        = link_to packages_link do
           %strong.fly-out-top-item-name
             = _('Packages')
+      %li.divider.fly-out-top-item
+      - if project_nav_tab? :packages
+        = nav_link controller: :packages do
+          = link_to project_packages_path(@project), title: _('Packages') do
+            %span= _('List')
+      - if project_nav_tab? :container_registry
+        = nav_link controller: :repositories do
+          = link_to project_container_registry_index_path(@project), class: 'shortcuts-container-registry', title: _('Container Registry') do
+            %span= _('Container Registry')

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

@@ -10,7 +10,7 @@ describe 'layouts/nav/sidebar/_project' do
     assign(:repository, project.repository)
     allow(view).to receive(:current_ref).and_return('master')
 
-    stub_licensed_features(tracing: true)
+    stub_licensed_features(tracing: true, packages: true)
   end
 
   describe 'issue boards' do
@@ -122,6 +122,93 @@ describe 'layouts/nav/sidebar/_project' do
     end
   end
 
+  describe 'Packages' do
+    let(:user) { create(:user) }
+
+    before do
+      project.team.add_developer(user)
+      sign_in(user)
+      stub_container_registry_config(enabled: true)
+    end
+
+    context 'when packages is enabled' do
+      it 'packages link is visible' do
+        render
+
+        expect(rendered).to have_link('Packages', href: project_packages_path(project))
+      end
+
+      it 'packages list link is visible' do
+        render
+
+        expect(rendered).to have_link('List', href: project_packages_path(project))
+      end
+
+      it 'container registry link is visible' do
+        render
+
+        expect(rendered).to have_link('Container Registry', href: project_container_registry_index_path(project))
+      end
+    end
+
+    context 'when packages are disabled' do
+      before do
+        stub_licensed_features(packages: false)
+      end
+
+      it 'packages list link is not visible' do
+        render
+
+        expect(rendered).not_to have_link('List', href: project_packages_path(project))
+      end
+
+      it 'top level packages link links to container registry' do
+        render
+
+        expect(rendered).to have_link('Packages', href: project_container_registry_index_path(project))
+      end
+
+      it 'packages top level and container registry links are visible' do
+        render
+
+        expect(rendered).to have_link('Packages', href: project_container_registry_index_path(project))
+        expect(rendered).to have_link('Container Registry', href: project_container_registry_index_path(project))
+      end
+    end
+
+    context 'when container registry is disabled' do
+      before do
+        stub_container_registry_config(enabled: false)
+      end
+
+      it 'packages top level and list link are visible' do
+        render
+
+        expect(rendered).to have_link('Packages', href: project_packages_path(project))
+        expect(rendered).to have_link('List', href: project_packages_path(project))
+      end
+
+      it 'container registry link is not visible' do
+        render
+
+        expect(rendered).not_to have_link('Container Registry', href: project_container_registry_index_path(project))
+      end
+    end
+
+    context 'when both packages and container registry are disabled' do
+      before do
+        stub_licensed_features(packages: false)
+        stub_container_registry_config(enabled: false)
+      end
+
+      it 'packages top level item is not visible' do
+        render
+
+        expect(rendered).not_to have_link('Packages', href: project_packages_path(project))
+      end
+    end
+  end
+
   describe 'Settings > Operations' do
     it 'is not visible when no valid license' do
       allow(view).to receive(:can?).and_return(true)

locale/gitlab.pot

@@ -11249,9 +11249,6 @@ msgstr ""
 msgid "Registration"
 msgstr ""
 
-msgid "Registry"
-msgstr ""
-
 msgid "Related Deployed Jobs"
 msgstr ""
 

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

@@ -21,7 +21,7 @@ describe 'layouts/nav/sidebar/_project' do
     end
   end
 
-  describe 'container registry tab' do
+  describe 'packages tab' do
     before do
       stub_container_registry_config(enabled: true)
 
@@ -31,24 +31,17 @@ describe 'layouts/nav/sidebar/_project' do
         .and_return('projects/registry/repositories')
     end
 
-    it 'has both Registry and Repository tabs' do
-      render
-
-      expect(rendered).to have_text 'Repository'
-      expect(rendered).to have_text 'Registry'
-    end
-
     it 'highlights sidebar item and flyout' do
       render
 
       expect(rendered).to have_css('.sidebar-top-level-items > li.active', count: 1)
-      expect(rendered).to have_css('.is-fly-out-only > li.active', count: 1)
+      expect(rendered).to have_css('.sidebar-sub-level-items > li.fly-out-top-item.active', count: 1)
     end
 
     it 'highlights container registry tab' do
       render
 
-      expect(rendered).to have_css('.sidebar-top-level-items > li.active', text: 'Registry')
+      expect(rendered).to have_css('.sidebar-sub-level-items > li:not(.fly-out-top-item).active', text: 'Container Registry')
     end
   end