From 590486d8c077e8cb95f8c6c98d36f7a4c7cb0ad5 Mon Sep 17 00:00:00 2001
From: Martin Povolny <mpovolny@redhat.com>
Date: Wed, 22 May 2019 10:56:07 +0200
Subject: [PATCH 1/5] Allow adding custom top-level menu items via settings.

---
 app/presenters/menu/item.rb                  | 20 +++++++++--
 app/presenters/menu/manager.rb               | 17 +++++----
 app/presenters/menu/settings_loader.rb       | 36 ++++++++++++++++++++
 app/presenters/menu/yaml_loader.rb           | 24 ++++++++++---
 app/views/layouts/_vertical_navbar.html.haml | 10 +++++-
 5 files changed, 94 insertions(+), 13 deletions(-)
 create mode 100644 app/presenters/menu/settings_loader.rb

diff --git a/app/presenters/menu/item.rb b/app/presenters/menu/item.rb
index 0e324fd66c5..82d5e3140ba 100644
--- a/app/presenters/menu/item.rb
+++ b/app/presenters/menu/item.rb
@@ -1,5 +1,5 @@
 module Menu
-  Item = Struct.new(:id, :name, :feature, :rbac_feature, :href, :type, :parent_id, :defaults) do
+  Item = Struct.new(:id, :name, :feature, :rbac_feature, :href, :type, :parent_id, :defaults, :icon) do
     extend ActiveModel::Naming
 
     def self.base_class
@@ -10,7 +10,7 @@ def self.base_model
       model_name
     end
 
-    def initialize(an_id, a_name, features, rbac_feature, href, type = :default, parent_id = nil, defaults = nil)
+    def initialize(an_id, a_name, features, rbac_feature, href, type = :default, parent_id = nil, defaults = nil, icon = nil)
       super
       @parent = nil
       @name = a_name.kind_of?(Proc) ? a_name : -> { a_name }
@@ -58,5 +58,21 @@ def parent_path
     def item(item_id)
       item_id == id ? self : nil
     end
+
+    def placement
+      @parent&.placement || :default
+    end
+
+    def contains_item_id?(item_id)
+      item_id == id
+    end
+
+    def subsection?
+      false
+    end
+
+    def items
+      []
+    end
   end
 end
diff --git a/app/presenters/menu/manager.rb b/app/presenters/menu/manager.rb
index c6c98e9d5e3..c18762615c0 100644
--- a/app/presenters/menu/manager.rb
+++ b/app/presenters/menu/manager.rb
@@ -26,6 +26,8 @@ def items
 
     def item(item_id)
       @menu.each do |menu_section|
+        return menu_section if Menu::Item === menu_section && menu_section.id == item_id
+
         menu_section.items.each do |el|
           the_item = el.item(item_id)
           return the_item if the_item.present?
@@ -73,6 +75,7 @@ def initialize
       load_default_items
       load_custom_items(Menu::YamlLoader)
       load_custom_items(Menu::CustomLoader)
+      load_custom_items(Menu::SettingsLoader)
     end
 
     def merge_sections(sections)
@@ -100,10 +103,12 @@ def merge_sections(sections)
     def merge_items(items)
       items.each do |item|
         parent = @id_to_section[item.parent_id]
-        raise InvalidMenuDefinition, 'Invalid parent' if parent.nil?
-
-        parent.items << item
-        item.parent = parent
+        if parent.nil?
+          @menu << item
+        else
+          parent.items << item
+          item.parent = parent
+        end
       end
     end
 
@@ -123,12 +128,12 @@ def preprocess_sections
       @id_to_section = @menu.index_by(&:id)
       # recursively add subsections to the @id_to_section hash
       @menu.each do |section|
-        section.preprocess_sections(@id_to_section)
+        section.preprocess_sections(@id_to_section) if section.respond_to?(:preprocess_sections)
       end
     end
 
     def valid_sections
-      # format is {"vi" => :vi, "svc" => :svc . . }
+     # format is {"vi" => :vi, "svc" => :svc . . }
       @valid_sections ||= @id_to_section.keys.index_by(&:to_s)
     end
   end
diff --git a/app/presenters/menu/settings_loader.rb b/app/presenters/menu/settings_loader.rb
new file mode 100644
index 00000000000..7f7e406ecd6
--- /dev/null
+++ b/app/presenters/menu/settings_loader.rb
@@ -0,0 +1,36 @@
+module Menu
+  class SettingsLoader < YamlLoader
+    include Singleton
+
+    def self.load
+      instance.load_from_settings
+    end
+
+    def load_from_settings
+      begin
+        settings = ::Settings.ui.custom_menu
+        items = (settings || []).map { |i| create_custom_item(HashWithIndifferentAccess.new(i)) }
+      rescue => e
+        # if we encounter an error while loading the menus, we ignore the whole settings
+        $log.error("Error loading custom menu from settings: #{e}")
+        $log.error("Settings were: #{settings}")
+        return [[], []]
+      end
+      [[], items]
+    end
+
+    private 
+
+    def create_custom_item(item)
+      # only alow:
+      #  * items,
+      #  * displayed in the iframe,
+      #  * and at the top menu level.
+      create_custom_menu_item(item.merge(
+        'type'      => 'items',
+        'item_type' => 'big_iframe',
+        'parent'    => nil
+      ))
+    end
+  end
+end
diff --git a/app/presenters/menu/yaml_loader.rb b/app/presenters/menu/yaml_loader.rb
index e6127833dbb..16d2108dc31 100644
--- a/app/presenters/menu/yaml_loader.rb
+++ b/app/presenters/menu/yaml_loader.rb
@@ -26,15 +26,28 @@ def load_custom_item(file_name)
       end
     end
 
+    # In case `rbac` is a Hash, convert keys to symbols.
+    #   Example: { :feature => 'vm_explorer', :any => true }
+    #
+    # Else assume string and return:
+    #   { :feature => rbac }
+    def parse_rbac_property(rbac)
+      rbac === Hash ?
+        rbac.each_with_object({}) { |(k, v), h| h[k.to_sym] = v } :
+        { :feature => rbac }
+    end
+
     def create_custom_menu_item(properties)
-      rbac = properties['rbac'].each_with_object({}) { |(k, v), h| h[k.to_sym] = v }
-      item_type = properties.key?('item_type') ? properties['item_type'].to_sym : :default
-      %w[id name rbac parent].each do |property|
+      %w[id name rbac].each do |property|
         if properties[property].blank?
           raise Menu::Manager::InvalidMenuDefinition,
                 "incomplete definition -- missing #{property}"
         end
       end
+
+      rbac = parse_rbac_property(properties['rbac'])
+      item_type = properties.key?('item_type') ? properties['item_type'].to_sym : :default
+
       item = Item.new(
         properties['id'],
         properties['name'],
@@ -42,7 +55,9 @@ def create_custom_menu_item(properties)
         rbac,
         properties['href'],
         item_type,
-        properties['parent'].to_sym
+        properties['parent']&.to_sym,
+        nil,
+        properties['icon']
       )
       item
     end
@@ -53,6 +68,7 @@ def create_custom_menu_section(properties)
       before       = properties.key?('before') ? properties['before'].to_sym : nil
       section_type = properties.key?('section_type') ? properties['section_type'].to_sym : :default
       href         = properties.key?('href') ? properties['href'].to_sym : nil
+      # no parent_id here?
       Section.new(properties['id'].to_sym, properties['name'], icon, [], placement, before, section_type, href)
     end
   end
diff --git a/app/views/layouts/_vertical_navbar.html.haml b/app/views/layouts/_vertical_navbar.html.haml
index 5850bdc01ca..4f0a0af24e3 100644
--- a/app/views/layouts/_vertical_navbar.html.haml
+++ b/app/views/layouts/_vertical_navbar.html.haml
@@ -1,7 +1,15 @@
 .nav-pf-vertical.nav-pf-vertical-with-sub-menus.nav-pf-vertical-collapsible-menus
   %ul#maintab.list-group
     - Menu::Manager.menu do |menu_section|
-      - if menu_section.visible?
+      - next unless menu_section.visible?
+      - if Menu::Item === menu_section
+        %li.list-group-item{:class => item_nav_class(menu_section), :id => "menu_item_#{menu_section.id}"}
+          %a{menu_section.link_params}
+            %span{:class => menu_section.icon}
+            %span.list-group-item-value
+              = _(menu_section.name)
+
+      - else
         %li.list-group-item.secondary-nav-item-pf{"data-target" => "#menu-#{menu_section.id}", :class => section_nav_class(menu_section)}
           %a{menu_section.link_params}
             %span{:class => menu_section.icon}

From 02d68089a9bc0bc9f2f7b5c74d80e791e86c8453 Mon Sep 17 00:00:00 2001
From: Martin Povolny <mpovolny@redhat.com>
Date: Thu, 30 May 2019 18:57:33 +0200
Subject: [PATCH 2/5] Adding custom top-level menu items via settings: review
 fixes.

---
 app/presenters/menu/manager.rb         |  4 ++--
 app/presenters/menu/settings_loader.rb | 21 ++++++++++++---------
 app/presenters/menu/yaml_loader.rb     |  6 ++----
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/app/presenters/menu/manager.rb b/app/presenters/menu/manager.rb
index c18762615c0..6546596ccc3 100644
--- a/app/presenters/menu/manager.rb
+++ b/app/presenters/menu/manager.rb
@@ -26,7 +26,7 @@ def items
 
     def item(item_id)
       @menu.each do |menu_section|
-        return menu_section if Menu::Item === menu_section && menu_section.id == item_id
+        return menu_section if menu_section.kind_of?(Menu::Item) && menu_section.id == item_id
 
         menu_section.items.each do |el|
           the_item = el.item(item_id)
@@ -133,7 +133,7 @@ def preprocess_sections
     end
 
     def valid_sections
-     # format is {"vi" => :vi, "svc" => :svc . . }
+      # format is {"vi" => :vi, "svc" => :svc . . }
       @valid_sections ||= @id_to_section.keys.index_by(&:to_s)
     end
   end
diff --git a/app/presenters/menu/settings_loader.rb b/app/presenters/menu/settings_loader.rb
index 7f7e406ecd6..cae13998413 100644
--- a/app/presenters/menu/settings_loader.rb
+++ b/app/presenters/menu/settings_loader.rb
@@ -1,6 +1,7 @@
 module Menu
   class SettingsLoader < YamlLoader
     include Singleton
+    include Vmdb::Logging
 
     def self.load
       instance.load_from_settings
@@ -10,27 +11,29 @@ def load_from_settings
       begin
         settings = ::Settings.ui.custom_menu
         items = (settings || []).map { |i| create_custom_item(HashWithIndifferentAccess.new(i)) }
-      rescue => e
+      rescue StandardError => e
         # if we encounter an error while loading the menus, we ignore the whole settings
-        $log.error("Error loading custom menu from settings: #{e}")
-        $log.error("Settings were: #{settings}")
+        _log.error("Error loading custom menu from settings: #{e}")
+        _log.error("Settings were: #{settings}")
         return [[], []]
       end
       [[], items]
     end
 
-    private 
+    private
 
     def create_custom_item(item)
       # only alow:
       #  * items,
       #  * displayed in the iframe,
       #  * and at the top menu level.
-      create_custom_menu_item(item.merge(
-        'type'      => 'items',
-        'item_type' => 'big_iframe',
-        'parent'    => nil
-      ))
+      create_custom_menu_item(
+        item.merge(
+          'type'      => 'items',
+          'item_type' => 'big_iframe',
+          'parent'    => nil
+        )
+      )
     end
   end
 end
diff --git a/app/presenters/menu/yaml_loader.rb b/app/presenters/menu/yaml_loader.rb
index 16d2108dc31..0745af66a2d 100644
--- a/app/presenters/menu/yaml_loader.rb
+++ b/app/presenters/menu/yaml_loader.rb
@@ -32,9 +32,7 @@ def load_custom_item(file_name)
     # Else assume string and return:
     #   { :feature => rbac }
     def parse_rbac_property(rbac)
-      rbac === Hash ?
-        rbac.each_with_object({}) { |(k, v), h| h[k.to_sym] = v } :
-        { :feature => rbac }
+      rbac.kind_of?(Hash) ? rbac.symbolize_keys : { :feature => rbac }
     end
 
     def create_custom_menu_item(properties)
@@ -46,7 +44,7 @@ def create_custom_menu_item(properties)
       end
 
       rbac = parse_rbac_property(properties['rbac'])
-      item_type = properties.key?('item_type') ? properties['item_type'].to_sym : :default
+      item_type = properties.fetch('item_type', :default).to_sym
 
       item = Item.new(
         properties['id'],

From 01fbfd249b997f848ba7e7f38c1a5d3a71409547 Mon Sep 17 00:00:00 2001
From: Martin Povolny <mpovolny@redhat.com>
Date: Thu, 30 May 2019 19:02:44 +0200
Subject: [PATCH 3/5] Custom top-level menu settings: build-in empty value.

---
 config/settings.yml | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 config/settings.yml

diff --git a/config/settings.yml b/config/settings.yml
new file mode 100644
index 00000000000..8f647b088f5
--- /dev/null
+++ b/config/settings.yml
@@ -0,0 +1,2 @@
+:ui:
+  :custom_menu:

From 2f2cb654c8c1304023d38e07b6d07db1a7e633b1 Mon Sep 17 00:00:00 2001
From: Martin Povolny <mpovolny@redhat.com>
Date: Thu, 30 May 2019 19:22:53 +0200
Subject: [PATCH 4/5] Custom top-level menu items via settings: add specs.

---
 spec/presenters/menu/menu_manager_spec.rb    | 11 ++++++++++
 spec/presenters/menu/settings_loader_spec.rb | 10 ++++++++++
 spec/support/menu_helper.rb                  | 21 ++++++++++++++++++++
 3 files changed, 42 insertions(+)
 create mode 100644 spec/presenters/menu/settings_loader_spec.rb

diff --git a/spec/presenters/menu/menu_manager_spec.rb b/spec/presenters/menu/menu_manager_spec.rb
index a93c92fdabc..b6e961ba519 100644
--- a/spec/presenters/menu/menu_manager_spec.rb
+++ b/spec/presenters/menu/menu_manager_spec.rb
@@ -38,4 +38,15 @@
       end
     end
   end
+
+  context "menu" do
+    it "knows about custom items from settings" do
+      ::Settings.ui.custom_menu = settings_custom_items
+      count = 0
+      Menu::Manager.menu do |item|
+        count += 1 if item.kind_of?(Menu::Item) && item.name =~ /^Custom Item/
+      end
+      expect(count).to eq(2)
+    end
+  end
 end
diff --git a/spec/presenters/menu/settings_loader_spec.rb b/spec/presenters/menu/settings_loader_spec.rb
new file mode 100644
index 00000000000..348916bd1b6
--- /dev/null
+++ b/spec/presenters/menu/settings_loader_spec.rb
@@ -0,0 +1,10 @@
+describe Menu::SettingsLoader do
+  include Spec::Support::MenuHelper
+  it "loads custom menu items" do
+    ::Settings.ui.custom_menu = settings_custom_items
+    sections, items = described_class.load
+
+    expect(sections.length).to be(0)
+    expect(items.length).to be(2)
+  end
+end
diff --git a/spec/support/menu_helper.rb b/spec/support/menu_helper.rb
index b4fb1afecbd..1db2df0d430 100644
--- a/spec/support/menu_helper.rb
+++ b/spec/support/menu_helper.rb
@@ -36,6 +36,27 @@ def section_file
       def item_file
         create_temp_file(ITEM_DEF)
       end
+
+      def settings_custom_items
+        [
+          {
+            :type => 'item',
+            :icon => 'fa fa-bug',
+            :id   => 'custom_i1',
+            :name => 'Custom Item 1',
+            :href => 'https://www.redhat.com',
+            :rbac => 'vm_explorer'
+          },
+          {
+            :type => 'item',
+            :icon => 'pficon pficon-help',
+            :id   => 'custom_i2',
+            :name => 'Custom Item 2',
+            :href => 'https://www.hmpf.cz',
+            :rbac => 'vm_explorer'
+          }
+        ]
+      end
     end
   end
 end

From 99d5cb835a08393790d19572360e60e5e5b114ff Mon Sep 17 00:00:00 2001
From: Martin Povolny <mpovolny@redhat.com>
Date: Wed, 19 Jun 2019 14:46:07 +0200
Subject: [PATCH 5/5] Custom top-level menu items are not on RBAC tree root
 level.

---
 app/presenters/tree_builder_ops_rbac_features.rb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/presenters/tree_builder_ops_rbac_features.rb b/app/presenters/tree_builder_ops_rbac_features.rb
index dbf1904b453..a42b6c67978 100644
--- a/app/presenters/tree_builder_ops_rbac_features.rb
+++ b/app/presenters/tree_builder_ops_rbac_features.rb
@@ -19,7 +19,9 @@ def initialize(name, sandbox, build, **params)
   private
 
   def x_get_tree_roots(count_only = false, _options)
-    top_nodes = Menu::Manager.items.select { |section| Vmdb::PermissionStores.instance.can?(section.id) }
+    top_nodes = Menu::Manager.items.select do |section|
+      Vmdb::PermissionStores.instance.can?(section.id) && !section.kind_of?(Menu::Item)
+    end
 
     top_nodes += %w[all_vm_rules api_exclusive sui ops_explorer].collect do |additional_feature|
       MiqProductFeature.obj_features[additional_feature] &&