Extend the Vmdb::Plugins singleton with UI-specific plugin support

[skateman]

We need a way to mark rails engines as plugins related to the User Interface. I found out that there is already a structure holding provider plugins, so I'd like to extend this if possible. By declaring a ui_plugin method inside the engine, the plugin would be registered under the @registered_ui_plugins instance variable that we would use in the UI on some places.

For now I require both the ui_plugin and the vmdb_plugin methods to be declared and return true, but I'm not 100% sure if this is The Right Way™. Renaming the method or requiring just one of the two methods are both an option, I just need some feedback 😉

ping @martinpovolny, @jrafanie, @Fryguy

[miq-bot]

Checked commit skateman@a7e2ecc with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

[jrafanie]

I can see us having public methods to get a list of provider_plugins or ui_plugins. What do you think @durandom?

[Fryguy]

I'm conflicted on the "Right Way".

I like the foo_plugin? direction and is something I thought we should do with provider plugins as well. Right now we hardcode if we see ManageIQ::Providers in the plugin name for providers, but I like the declarative nature of the method. Also, having a method allows a plugin to have multiple things in it by just having multiple methods.

However, on the flip side, a plugin can automatically be an automate plugin by having a content directory at the top-level. Pretend we required an automate_plugin? method or something, if the user forgets to add it, they might be scratching their heads why it doesn't work. Detecting the path is a convention over configuration approach, which is attractive.

cc @bdunne Thoughts?

@skateman Out of curiosity, how is this needed? Could we do this by also detecting paths, like app/controllers or something or is that potentially too open ended?

[bdunne]

I like the idea of implementing vmdb_plugin? and only that method (no ui_plugin?, no class name detection). Once a plugin is determined to be a vmdb plugin, I think we should autodetect the things inside it (content/automate, ui, etc.).

[Fryguy]

Thinking about it more I agree with @bdunne . I was confusing vmdb_plugin? with feature_plugin?

[martinpovolny]

@skateman Out of curiosity, how is this needed? Could we do this by also detecting paths, like app/controllers or something or is that potentially too open ended?

If we go this way then remember that one stat call might be cheap, but 1000 stat calls is not cheap at all and cache the information. Actually any discovery should be done only once.

[skateman]

So first of all this would be a nice to have thing for letting know the server that we want to dynamically load JS assets into the application.js, through a plugin.js.erb where I iterate through all the UI plugins. In this case we might go with the scanning mentioned by @bdunne as asset precompilation happens only once in production, however, I'm not sure about how this will affect the code/asset reloading in development.

Then we would like to able to declare new menu items and attach them to the already existing main menu. For now we're doing this in an initializer section in the engine.rb, but I would like to extract it into a separate file. However, I think it's a more rails-ish solution if we don't ask external developers to add an explicit require into their engine's initializer section. Here scanning could work, but I agree with @martinpovolny's opinion about this.

My vision was to have a special class (let's call it for now ManageIQ::UI:Plugin) that is inherited from Rails::Engine. This would implicitly have the ui_plugin method defined to return with true and provide a simple DSL where external developers can do something like this:

module MiqPluginExample
  class Engine < ::ManageIQ::UI::Plugin # inherited from ::Rails::Engine
    provides_menu # maybe a parameter pointing to the menu file
    provides_routes
    provides_js
    works_with_webpack
    requires [ManageIQ::Providers::Openstack, ManageIQ::Providers::Vmware]
    # etc
end

As we want the UI separated from the core as much as possible, I was thinking about going through UI-related plugins in a second iteration in our ui-classic codebase where we register (or scan?) menus, assets, unicorns, etc to some lazy-structures.

[durandom]

Scanning / Detection happens only at boot time.

Why not say that all UI Engines should be namespaced like the providers?

module ManageIQ
  module UI
    module Amazon
      class Engine < ::Rails::Engine
        isolate_namespace ManageIQ::UI::Amazon
      end
    end
  end
end

This would

• bring it in line with the providers
• easily autodetect the "type"
• re-use the provider generator
• reduce the learning curve for provider / UI plugin development

In the end, providers are also just rails engines...

[durandom]

As we want the UI separated from the core as much as possible

I'm not sure who depends on whom. Short term I'd say, like it is now, the core owns the UI server and hence has the right to know about UI plugins. But long term I'd say the UI is its own rails server and communicates to the core/platform server via defined APIs.

[skateman]

@martinpovolny you explicitly disabled isolated namespaces in your plugin example. Is there any reason behind this?

[martinpovolny]

@martinpovolny you explicitly disabled isolated namespaces in your plugin example. Is there any reason behind this?

Yes, it was dealing with splitting the UI making sure things work so it fitted me more. For plugin usecase we might want to isolate the namespaces.

For first shot proof of concept I'd do whatever is easier for you.

[himdel]

As long as any JS assets use webpacker, the current implementation (ManageIQ/manageiq-ui-classic@66f9dcc) will pick up any engine that contains an app/javascript/ folder.

We still need a way to find all the plugins' packs to include them in the layout. (Think the equivalent of adding a line to application.js.) But, like registering menu items, etc. this could probably be done regardless of plugin type, by calling something like add_pack in the plugin def, and iterating through the array of packs in app/views/layouts/application.html.haml.

[skateman]

Okay, based on the reactions I'm closing this PR.