Commit e90e6d49 authored by Arturo Herrero's avatar Arturo Herrero

Merge branch 'fj-implement-renderable-menu-items-in-sidebar' into 'master'

Fix bug in sidebar when menu item is not present

See merge request gitlab-org/gitlab!61160
parents 158e6716 226678b9
......@@ -11,5 +11,5 @@
- if sidebar.render_raw_menus_partial
= render sidebar.render_raw_menus_partial
= render partial: 'shared/nav/sidebar_hidden_menu_item', collection: sidebar.hidden_menu&.items
= render partial: 'shared/nav/sidebar_hidden_menu_item', collection: sidebar.hidden_menu&.renderable_items
= render 'shared/sidebar_toggle_button'
......@@ -13,7 +13,7 @@
%span.badge.badge-pill.count{ **sidebar_menu.pill_html_options }
= number_with_delimiter(sidebar_menu.pill_count)
%ul.sidebar-sub-level-items{ class: ('is-fly-out-only' unless sidebar_menu.has_items?) }
%ul.sidebar-sub-level-items{ class: ('is-fly-out-only' unless sidebar_menu.has_renderable_items?) }
= nav_link(**sidebar_menu.all_active_routes, html_options: { class: 'fly-out-top-item' } ) do
= link_to sidebar_menu.link, **sidebar_menu.collapsed_container_html_options do
%strong.fly-out-top-item-name
......@@ -24,4 +24,4 @@
- if sidebar_menu.has_items?
%li.divider.fly-out-top-item
= render partial: 'shared/nav/sidebar_menu_item', collection: sidebar_menu.items
= render partial: 'shared/nav/sidebar_menu_item', collection: sidebar_menu.renderable_items
......@@ -31,7 +31,7 @@ module EE
return audit_events_menu_item.link if audit_events_menu_item
return dependencies_menu_item.link if dependencies_menu_item
items.first.link
renderable_items.first.link
end
override :render?
......
......@@ -11,7 +11,7 @@ RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do
subject { described_class.new(context) }
describe 'Menu items' do
subject { described_class.new(context).items.index { |e| e.item_id == item_id } }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
describe 'Code Review' do
let(:item_id) { :code_review }
......
......@@ -8,7 +8,7 @@ RSpec.describe Sidebars::Projects::Menus::CiCdMenu do
let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, can_view_pipeline_editor: true) }
describe 'Test Cases' do
subject { described_class.new(context).items.index { |e| e.item_id == :test_cases} }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == :test_cases} }
context 'when licensed feature quality_management is not enabled' do
before do
......
......@@ -8,7 +8,7 @@ RSpec.describe Sidebars::Projects::Menus::IssuesMenu do
let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) }
describe 'Iterations' do
subject { described_class.new(context).items.index { |e| e.item_id == :iterations} }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == :iterations} }
context 'when licensed feature iterations is not enabled' do
it 'does not include iterations menu item' do
......
......@@ -7,35 +7,39 @@ RSpec.describe Sidebars::Projects::Menus::OperationsMenu do
let(:user) { project.owner }
let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, show_cluster_hint: true) }
describe 'On-call Schedules' do
subject { described_class.new(context).items.index { |e| e.item_id == :on_call_schedules } }
describe 'Menu items' do
subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
before do
stub_licensed_features(oncall_schedules: true)
end
describe 'On-call Schedules' do
let(:item_id) { :on_call_schedules }
before do
stub_licensed_features(oncall_schedules: true)
end
specify { is_expected.not_to be_nil }
specify { is_expected.not_to be_nil }
describe 'when the user does not have access' do
let(:user) { nil }
describe 'when the user does not have access' do
let(:user) { nil }
specify { is_expected.to be_nil }
specify { is_expected.to be_nil }
end
end
end
describe 'Escalation policies' do
subject { described_class.new(context).items.index { |e| e.item_id == :escalation_policies } }
describe 'Escalation policies' do
let(:item_id) { :escalation_policies }
before do
stub_licensed_features(oncall_schedules: true, escalation_policies: true)
end
before do
stub_licensed_features(oncall_schedules: true, escalation_policies: true)
end
specify { is_expected.not_to be_nil }
specify { is_expected.not_to be_nil }
describe 'when the user does not have access' do
let(:user) { nil }
describe 'when the user does not have access' do
let(:user) { nil }
specify { is_expected.to be_nil }
specify { is_expected.to be_nil }
end
end
end
end
......@@ -9,7 +9,7 @@ RSpec.describe Sidebars::Projects::Menus::RepositoryMenu do
let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project, current_ref: 'master') }
describe 'File Locks' do
subject { described_class.new(context).items.index { |e| e.item_id == :file_locks} }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == :file_locks} }
context 'when licensed feature file locks is not enabled' do
it 'does not include file locks menu item' do
......
......@@ -37,9 +37,9 @@ RSpec.describe Sidebars::Projects::Menus::JiraMenu do
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
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
......
......@@ -33,7 +33,7 @@ RSpec.describe Sidebars::Projects::Menus::RequirementsMenu do
end
it 'does not contain any menu item' do
expect(subject.items).to be_empty
expect(subject.renderable_items).to be_empty
end
end
......@@ -47,7 +47,7 @@ RSpec.describe Sidebars::Projects::Menus::RequirementsMenu do
end
it 'contains list menu item' do
expect(subject.items[0].item_id).to eq :requirements_list
expect(subject.renderable_items[0].item_id).to eq :requirements_list
end
end
end
......
......@@ -13,7 +13,7 @@ module Sidebars
include ::Sidebars::Concerns::ContainerWithHtmlOptions
include ::Sidebars::Concerns::HasActiveRoutes
attr_reader :context, :items
attr_reader :context
delegate :current_user, :container, to: :@context
def initialize(context)
......@@ -29,7 +29,7 @@ module Sidebars
override :render?
def render?
has_items?
has_renderable_items?
end
# Menus might have or not a link
......@@ -43,7 +43,7 @@ module Sidebars
# This method filters the information and returns: { path: ['foo', 'bar'], controller: :foo }
def all_active_routes
@all_active_routes ||= begin
([active_routes] + items.map(&:active_routes)).flatten.each_with_object({}) do |pairs, hash|
([active_routes] + renderable_items.map(&:active_routes)).flatten.each_with_object({}) do |pairs, hash|
pairs.each do |k, v|
hash[k] ||= []
hash[k] += Array(v)
......@@ -55,10 +55,22 @@ module Sidebars
end
end
# Returns whether the menu has any menu item, no
# matter whether it is renderable or not
def has_items?
@items.any?
end
# Returns all renderable menu items
def renderable_items
@renderable_items ||= @items.select(&:render?)
end
# Returns whether the menu has any renderable menu item
def has_renderable_items?
renderable_items.any?
end
def add_item(item)
add_element(@items, item)
end
......
......@@ -18,5 +18,9 @@ module Sidebars
def show_hint?
hint_html_options.present?
end
def render?
true
end
end
end
# frozen_string_literal: true
module Sidebars
class NilMenuItem < MenuItem
extend ::Gitlab::Utils::Override
def initialize(item_id:)
super(item_id: item_id, title: nil, link: nil, active_routes: {})
end
override :render?
def render?
false
end
end
end
......@@ -21,7 +21,7 @@ module Sidebars
def link
return cycle_analytics_menu_item.link if cycle_analytics_menu_item
items.first.link
renderable_items.first.link
end
override :extra_container_html_options
......
......@@ -15,7 +15,7 @@ module Sidebars
override :link
def link
items.first.link
renderable_items.first.link
end
override :title
......
......@@ -5,15 +5,18 @@ require 'spec_helper'
RSpec.describe Sidebars::Menu do
let(:menu) { described_class.new(context) }
let(:context) { Sidebars::Context.new(current_user: nil, container: nil) }
let(:nil_menu_item) { Sidebars::NilMenuItem.new(item_id: :foo) }
describe '#all_active_routes' do
it 'gathers all active routes of items and the current menu' do
menu.add_item(Sidebars::MenuItem.new(title: 'foo1', link: 'foo1', active_routes: { path: %w(bar test) }))
menu.add_item(Sidebars::MenuItem.new(title: 'foo2', link: 'foo2', active_routes: { controller: 'fooc' }))
menu.add_item(Sidebars::MenuItem.new(title: 'foo3', link: 'foo3', active_routes: { controller: 'barc' }))
menu.add_item(nil_menu_item)
allow(menu).to receive(:active_routes).and_return({ path: 'foo' })
expect(menu).to receive(:renderable_items).and_call_original
expect(menu.all_active_routes).to eq({ path: %w(foo bar test), controller: %w(fooc barc) })
end
end
......@@ -31,6 +34,60 @@ RSpec.describe Sidebars::Menu do
expect(menu.render?).to be true
end
context 'when menu items are NilMenuItem' do
it 'returns false' do
menu.add_item(nil_menu_item)
expect(menu.render?).to be false
end
end
end
end
describe '#has_items?' do
it 'returns true when there are regular menu items' do
menu.add_item(Sidebars::MenuItem.new(title: 'foo1', link: 'foo1', active_routes: {}))
expect(menu.has_items?).to be true
end
it 'returns true when there are nil menu items' do
menu.add_item(nil_menu_item)
expect(menu.has_items?).to be true
end
end
describe '#has_renderable_items?' do
it 'returns true when there are regular menu items' do
menu.add_item(Sidebars::MenuItem.new(title: 'foo1', link: 'foo1', active_routes: {}))
expect(menu.has_renderable_items?).to be true
end
it 'returns false when there are nil menu items' do
menu.add_item(nil_menu_item)
expect(menu.has_renderable_items?).to be false
end
it 'returns true when there are both regular and nil menu items' do
menu.add_item(Sidebars::MenuItem.new(title: 'foo1', link: 'foo1', active_routes: {}))
menu.add_item(nil_menu_item)
expect(menu.has_renderable_items?).to be true
end
end
describe '#renderable_items' do
it 'returns only regular menu items' do
item = Sidebars::MenuItem.new(title: 'foo1', link: 'foo1', active_routes: {})
menu.add_item(item)
menu.add_item(nil_menu_item)
expect(menu.renderable_items.size).to eq 1
expect(menu.renderable_items.first).to eq item
end
end
......
......@@ -26,7 +26,7 @@ RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do
context 'when menu does not have any menu items' do
it 'returns false' do
allow(subject).to receive(:has_items?).and_return(false)
allow(subject).to receive(:has_renderable_items?).and_return(false)
expect(subject.render?).to be false
end
......@@ -49,13 +49,13 @@ RSpec.describe Sidebars::Projects::Menus::AnalyticsMenu do
it 'returns link to the the first visible menu item' do
allow(subject).to receive(:cycle_analytics_menu_item).and_return(nil)
expect(subject.link).to eq subject.items.first.link
expect(subject.link).to eq subject.renderable_items.first.link
end
end
end
describe 'Menu items' do
subject { described_class.new(context).items.index { |e| e.item_id == item_id } }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
describe 'CI/CD' do
let(:item_id) { :ci_cd_analytics }
......
......@@ -26,40 +26,44 @@ RSpec.describe Sidebars::Projects::Menus::CiCdMenu do
end
end
describe 'Pipelines Editor' do
subject { described_class.new(context).items.index { |e| e.item_id == :pipelines_editor } }
describe 'Menu items' do
subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
context 'when user cannot view pipeline editor' do
let(:can_view_pipeline_editor) { false }
describe 'Pipelines Editor' do
let(:item_id) { :pipelines_editor }
it 'does not include pipeline editor menu item' do
is_expected.to be_nil
context 'when user cannot view pipeline editor' do
let(:can_view_pipeline_editor) { false }
it 'does not include pipeline editor menu item' do
is_expected.to be_nil
end
end
end
context 'when user can view pipeline editor' do
it 'includes pipeline editor menu item' do
is_expected.not_to be_nil
context 'when user can view pipeline editor' do
it 'includes pipeline editor menu item' do
is_expected.not_to be_nil
end
end
end
end
describe 'Artifacts' do
subject { described_class.new(context).items.index { |e| e.item_id == :artifacts } }
describe 'Artifacts' do
let(:item_id) { :artifacts }
context 'when feature flag :artifacts_management_page is disabled' do
it 'does not include artifacts menu item' do
stub_feature_flags(artifacts_management_page: false)
context 'when feature flag :artifacts_management_page is disabled' do
it 'does not include artifacts menu item' do
stub_feature_flags(artifacts_management_page: false)
is_expected.to be_nil
is_expected.to be_nil
end
end
end
context 'when feature flag :artifacts_management_page is enabled' do
it 'includes artifacts menu item' do
stub_feature_flags(artifacts_management_page: true)
context 'when feature flag :artifacts_management_page is enabled' do
it 'includes artifacts menu item' do
stub_feature_flags(artifacts_management_page: true)
is_expected.not_to be_nil
is_expected.not_to be_nil
end
end
end
end
......
......@@ -36,7 +36,7 @@ RSpec.describe Sidebars::Projects::Menus::ConfluenceMenu do
end
it 'does not contain any sub menu' do
expect(subject.items).to be_empty
expect(subject.has_items?).to be false
end
end
end
......
......@@ -11,7 +11,7 @@ RSpec.describe Sidebars::Projects::Menus::ExternalIssueTrackerMenu do
subject { described_class.new(context) }
it 'does not contain any sub menu' do
expect(subject.items).to be_empty
expect(subject.has_items?).to be false
end
describe '#render?' do
......
......@@ -10,7 +10,7 @@ RSpec.describe Sidebars::Projects::Menus::ExternalWikiMenu do
subject { described_class.new(context) }
it 'does not contain any sub menu' do
expect(subject.items).to be_empty
expect(subject.has_items?).to be false
end
describe '#render?' do
......
......@@ -13,7 +13,7 @@ RSpec.describe Sidebars::Projects::Menus::HiddenMenu do
context 'when menu does not have any menu items' do
it 'returns false' do
allow(subject).to receive(:has_items?).and_return(false)
allow(subject).to receive(:has_renderable_items?).and_return(false)
expect(subject.render?).to be false
end
......@@ -27,7 +27,7 @@ RSpec.describe Sidebars::Projects::Menus::HiddenMenu do
end
describe 'Menu items' do
subject { described_class.new(context).items.index { |e| e.item_id == item_id } }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
shared_examples 'access rights checks' do
specify { is_expected.not_to be_nil }
......
......@@ -10,7 +10,7 @@ RSpec.describe Sidebars::Projects::Menus::LabelsMenu do
subject { described_class.new(context) }
it 'does not contain any sub menu' do
expect(subject.items).to be_empty
expect(subject.has_items?).to eq false
end
describe '#render?' do
......
......@@ -19,7 +19,7 @@ RSpec.describe Sidebars::Projects::Menus::LearnGitlabMenu do
subject { described_class.new(context) }
it 'does not contain any sub menu' do
expect(subject.instance_variable_get(:@items)).to be_empty
expect(subject.has_items?).to be false
end
describe '#nav_link_html_options' do
......
......@@ -21,9 +21,9 @@ RSpec.describe Sidebars::Projects::Menus::OperationsMenu do
end
context 'when operation feature is enabled' do
context 'when menu does not have any menu items' do
context 'when menu does not have any renderable menu items' do
it 'returns false' do
allow(subject).to receive(:has_items?).and_return(false)
allow(subject).to receive(:has_renderable_items?).and_return(false)
expect(subject.render?).to be false
end
......@@ -54,7 +54,7 @@ RSpec.describe Sidebars::Projects::Menus::OperationsMenu do
end
context 'Menu items' do
subject { described_class.new(context).items.index { |e| e.item_id == item_id } }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
describe 'Metrics Dashboard' do
let(:item_id) { :metrics }
......
......@@ -12,7 +12,7 @@ RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do
describe '#render?' do
context 'when menu does not have any menu item to show' do
it 'returns false' do
allow(subject).to receive(:has_items?).and_return(false)
allow(subject).to receive(:has_renderable_items?).and_return(false)
expect(subject.render?).to eq false
end
......@@ -36,7 +36,7 @@ RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do
context 'when Packages Registry is visible' do
it 'menu link points to Packages Registry page' do
expect(subject.link).to eq described_class.new(context).items.find { |i| i.item_id == :packages_registry }.link
expect(subject.link).to eq described_class.new(context).renderable_items.find { |i| i.item_id == :packages_registry }.link
end
end
......@@ -44,95 +44,99 @@ RSpec.describe Sidebars::Projects::Menus::PackagesRegistriesMenu do
let(:packages_enabled) { false }
it 'menu link points to Container Registry page' do
expect(subject.link).to eq described_class.new(context).items.find { |i| i.item_id == :container_registry }.link
expect(subject.link).to eq described_class.new(context).renderable_items.find { |i| i.item_id == :container_registry }.link
end
context 'when Container Registry is not visible' do
let(:registry_enabled) { false }
it 'menu link points to Infrastructure Registry page' do
expect(subject.link).to eq described_class.new(context).items.find { |i| i.item_id == :infrastructure_registry }.link
expect(subject.link).to eq described_class.new(context).renderable_items.find { |i| i.item_id == :infrastructure_registry }.link
end
end
end
end
describe 'Packages Registry' do
subject { described_class.new(context).items.find { |i| i.item_id == :packages_registry }}
describe 'Menu items' do
subject { described_class.new(context).renderable_items.find { |i| i.item_id == item_id } }
context 'when user can read packages' do
context 'when config package setting is disabled' do
it 'the menu item is not added to list of menu items' do
stub_config(packages: { enabled: false })
describe 'Packages Registry' do
let(:item_id) { :packages_registry }
is_expected.to be_nil
context 'when user can read packages' do
context 'when config package setting is disabled' do
it 'the menu item is not added to list of menu items' do
stub_config(packages: { enabled: false })
is_expected.to be_nil
end
end
end
context 'when config package setting is enabled' do
it 'the menu item is added to list of menu items' do
stub_config(packages: { enabled: true })
context 'when config package setting is enabled' do
it 'the menu item is added to list of menu items' do
stub_config(packages: { enabled: true })
is_expected.not_to be_nil
is_expected.not_to be_nil
end
end
end
end
context 'when user cannot read packages' do
let(:user) { nil }
context 'when user cannot read packages' do
let(:user) { nil }
it 'the menu item is not added to list of menu items' do
is_expected.to be_nil
it 'the menu item is not added to list of menu items' do
is_expected.to be_nil
end
end
end
end
describe 'Container Registry' do
subject { described_class.new(context).items.find { |i| i.item_id == :container_registry }}
describe 'Container Registry' do
let(:item_id) { :container_registry }
context 'when user can read container images' do
context 'when config registry setting is disabled' do
it 'the menu item is not added to list of menu items' do
stub_container_registry_config(enabled: false)
context 'when user can read container images' do
context 'when config registry setting is disabled' do
it 'the menu item is not added to list of menu items' do
stub_container_registry_config(enabled: false)
is_expected.to be_nil
is_expected.to be_nil
end
end
end
context 'when config registry setting is enabled' do
it 'the menu item is added to list of menu items' do
stub_container_registry_config(enabled: true)
context 'when config registry setting is enabled' do
it 'the menu item is added to list of menu items' do
stub_container_registry_config(enabled: true)
is_expected.not_to be_nil
is_expected.not_to be_nil
end
end
end
end
context 'when user cannot read container images' do
let(:user) { nil }
context 'when user cannot read container images' do
let(:user) { nil }
it 'the menu item is not added to list of menu items' do
is_expected.to be_nil
it 'the menu item is not added to list of menu items' do
is_expected.to be_nil
end
end
end
end
describe 'Infrastructure Registry' do
subject { described_class.new(context).items.find { |i| i.item_id == :infrastructure_registry }}
describe 'Infrastructure Registry' do
let(:item_id) { :infrastructure_registry }
context 'when feature flag :infrastructure_registry_page is enabled' do
it 'the menu item is added to list of menu items' do
stub_feature_flags(infrastructure_registry_page: true)
context 'when feature flag :infrastructure_registry_page is enabled' do
it 'the menu item is added to list of menu items' do
stub_feature_flags(infrastructure_registry_page: true)
is_expected.not_to be_nil
is_expected.not_to be_nil
end
end
end
context 'when feature flag :infrastructure_registry_page is disabled' do
it 'the menu item is not added to list of menu items' do
stub_feature_flags(infrastructure_registry_page: false)
context 'when feature flag :infrastructure_registry_page is disabled' do
it 'the menu item is not added to list of menu items' do
stub_feature_flags(infrastructure_registry_page: false)
is_expected.to be_nil
is_expected.to be_nil
end
end
end
end
......
......@@ -9,7 +9,7 @@ RSpec.describe Sidebars::Projects::Menus::ProjectInformationMenu do
let(:context) { Sidebars::Projects::Context.new(current_user: user, container: project) }
describe 'Releases' do
subject { described_class.new(context).items.index { |e| e.item_id == :releases } }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == :releases } }
context 'when project repository is empty' do
it 'does not include releases menu item' do
......
......@@ -11,14 +11,14 @@ RSpec.describe Sidebars::Projects::Menus::SettingsMenu do
describe '#render?' do
it 'returns false when menu does not have any menu items' do
allow(subject).to receive(:has_items?).and_return(false)
allow(subject).to receive(:has_renderable_items?).and_return(false)
expect(subject.render?).to be false
end
end
describe 'Menu items' do
subject { described_class.new(context).items.index { |e| e.item_id == item_id } }
subject { described_class.new(context).renderable_items.index { |e| e.item_id == item_id } }
shared_examples 'access rights checks' do
specify { is_expected.not_to be_nil }
......
......@@ -10,7 +10,7 @@ RSpec.describe Sidebars::Projects::Menus::WikiMenu do
subject { described_class.new(context) }
it 'does not contain any sub menu' do
expect(subject.items).to be_empty
expect(subject.has_items?).to be false
end
describe '#render?' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment