Menu item adding through settings

[martinpovolny]

https://bugzilla.redhat.com/show_bug.cgi?id=1678151

Requirements:

• only allow top-level items
• only at the bottom of the menu
• only allow items (not sections)

Example (under Server settings, Advanced settings):

:ui:
  :custom_menu:
  - :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

list of supported icons:

• https://rawgit.com/patternfly/patternfly/master-dist/dist/tests/icons.html
• https://manageiq.github.io/font-fabulous/
• https://fontawesome.com/v4.7.0/icons/

based on the source of the icon the specification is either fa fa-bug (where fa-bug is the font awesome icon) of pf pficon-help (where pficon-help is the patternfly icon) or ff fa-whatever (where ff-whatever is the font fabulous icon).

icons also can be seen in the font-picker when adding new custom buttons

rbac is an existing RBAC feature indentifier that must exist in the product or be added via the extension mechanism added in
ManageIQ/manageiq#18806

Added menu item does not appear as the top-level item in the RBAC tree (under Settings --> Access controll --> Roles).

[Fryguy]

As this is effectively a cache, what happens when someone changes the settings? Are we expecting it to show the new menus or do we expect them to bounce the workers?

[martinpovolny]

Yes, it's a cache and I planned to document that a worker restart is needed for this to take effect. Do you have a better idea? Do you think it is better to load the settings each time? Or is there some invalidation mechanism (on Settings.save) that I could utilize?

[Fryguy]

What happens if there is bad configuration? Will it blow up? I'm not sure what create_custom_menu_item does under the covers.

[martinpovolny]

The idea is that if there's a bad configuration something would get logged and the settings will be ignored.
Do you see any problem with this approach?

[Fryguy]

If we decide to go forward with this, can you also create a PR to add an empty :ui -> :custom_menu section to the built-in settings.yml? Alternately you can add it in this repo since settings is pluggable. I've been on a mission to get rid of "hidden" settings sections, and eventually put in a validator, so trying to avoid adding another one.

[martinpovolny]

@Fryguy : I see no further comments from you in the last 18 days (since I addressed your previous feedback).

I rebased now. Pinging @mzazrivec and @himdel to finish the review.

Thx!

[himdel]

Re failing tests... should the tree node be looking at @object.feature and not @object.rbac? What's the difference between feature and rbac?

[martinpovolny]

Re failing tests... should the tree node be looking at @object.feature and not @object.rbac? What's the difference between feature and rbac?

https://github.com/ManageIQ/guides/blob/master/ui/menus.md says:

Feature key is used in parent section calculation such as visibility of the parent section.

Rbac_feature is a hash that is passed to role_allows? call when decision is being made about displaying the menu item. In most cases, this is would contain just the key :feature that will match the feature.

Honestly I do not remember details, but the :any => true part is needed. Someone could refactor this so that only one feature is needed.

Looking at the tests.

[miq-bot]

Checked commits martinpovolny/manageiq-ui-classic@590486d~...99d5cb8 with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
8 files checked, 0 offenses detected
Everything looks fine. 🍰