Manageiq-ui-classic: About modal iterates through plugins on every page load

[skateman]

Any time you navigate to a page that re-renders the whole browser window (i.e. all non-AJAX requests) will also re-render the _about_modal.html.haml page that now has an iteration through all the plugins. This is wasting resources as it happens during ~70% of navigation events and the about modal is being displayed only in a few cases.

To verify this, just add a debugger symbol into the about modal and navigate through the application a little...

@miq-bot assign @AparnaKarve
@miq-bot add_label bug, performance

[himdel]

Maybe "give me a list of plugins" should be an API call, so that we can do it on demand...

[martinpovolny]

A proper fix can be either:

1. calculate the values once. Cache it. This way it will only slow the first load per process.
2. dynamically load the content of the about modal. This way it will only get calculate when the modal is displayed (but displaying it will take a while).
   3) display the modal and then dynamically load the plugin information from an API or JSON endpoint (small React component maybe)?

I'd advocate against 1. as most users will never need the information and therefor this is an unnecessary calculation.

If the calculation takes long the modal can show the basic information and then indicate loading. Best UX afaik. Ping @terezanovotna. @skateman : how significant is the time delay?

[skateman]

@martinpovolny the delay isn't significant, and it seems the caching is sorted out on the backend already. Anyway, I'd vote for solution no. 3 as well.

[martinpovolny]

If the operation is quite fast then 2 should be enough. A couple of lines of javascript plus a new action returning the HTML markup. No API needed.

[dclarizio]

@AparnaKarve can this be changed to only fetch the plugin info when About modal is displayed? I'm guessing that 99% of users won't even go to this page and, if they do, they can wait a few seconds for the plugin info to be rendered.
Would like to get this fixed before Hammer is GA.

[AparnaKarve]

This issue is not specific to the plugin code at all - in fact, everything in the About Modal that is being displayed is being recalculated on a new page load.
The new plugin code addition is simply using the existing (faulty) pattern, which I believe existed all along.

@skateman If you can, please change the title of the issue appropriately to reflect that this is a systemic issue with the About Modal (and not necessarily related to displaying plugins). Thanks!

[AparnaKarve]

@dclarizio As we discussed, the caching-via-cookies solution may not be perfect but it would be relatively quicker.

The other option is to call the method that displays the About Modal info on demand, either via an API or a controller action (which would mean creating a React/Angular component for About)

This could also be a good exercise for someone trying to get their feet wet with React.
Any takers? @skateman ? :)

[skateman]

Well, if we think about how many times this screen will be actually displayed it doesn't really make sense to cache it in a cookie. I will look into creating an API endpoint (or an AJAX controller endpoint) and then query it asynchronously when the user clicks on the expand button or when displays the modal.

[Fryguy]

It feels weird to me that this is rendered on every page at all... Nobody looks at about modal. On the off chance they do, can we defer rendering until then? Then, perhaps use a page fragment cache?

EDIT: My comment is more regarding the design of the UI in general. There are a lot of pages where we prerender stuff a user will never see, and I don't understand it. Settings page is a good example.

[himdel]

It would be nice if about modal was a proper component, reading all the data from the API.

But, we'd also need to expose these for that (some may already be available):

Settings.server.custom_login_logo
vmdb_build_info(:version)
vmdb_build_info(:build)
appliance_name
current_user.name
user_role_name
browser_info(:name_ui)
browser_info(:version)
browser_info(:os_ui)
Settings.server.custom_logo

[skateman]

Do you really want to retrieve the browser_info through the API or am I misunderstanding something? :scream:

[himdel]

Well, right now it's in session. Where it gets saved on session init, populated from params. Which gets populated from miqBrowserDetect() on the JS side.

So.. no really suggesting serving that via API, better to just remove the round trip and get rid of that code completely :).

[skateman]

I see what I can do :wink:

[skateman]

@himdel the custom logos are coming in https://github.com/ManageIQ/manageiq-api/pull/438, plugins in https://github.com/ManageIQ/manageiq-api/pull/473. The rest is more or less available, maybe the current_user/user_role isn't. The browser stuff can and should be retrieved without an API request :trollface:

[himdel]

:+1:

Current user/role should already be available on GET /api (identity).. SUI about modal is using it.

And appliance name is probably in the same place, server_info, together with build and version.