Update main nav and UI for local Advisor Engine

[jeremylenz]

Introduce a new SETTING to tell RH Cloud if you're using Insights on Prem. In response to this setting,

• hide the Inventory Upload page from the main nav
• hide the "Sync automatically" toggle from the Recommendations page (formerly the Insights page)
• hide the 'View in Red Hat Insights' links from various locations on the Recommendations page (formerly the Insights page)

goes with #939

Before

After

Testing

In your Foreman settings.yaml, include a new section

:foreman_rh_cloud:
  :use_local_advisor_engine: true

[parthaa]

Can you also hide both these entries from the insights recommendations page.

Same for

Looks good otherwise.

[parthaa]

Also recommended patch. This redirects the remediations + certs to use localhost . May make sense in a separate PR though

diff --git a/lib/foreman_rh_cloud.rb b/lib/foreman_rh_cloud.rb
index f9e6430..406703f 100644
--- a/lib/foreman_rh_cloud.rb
+++ b/lib/foreman_rh_cloud.rb
@@ -3,17 +3,23 @@ require 'cgi'
 require 'uri'
 
 module ForemanRhCloud
+  def self.on_premise_url
+    return unless ForemanRhCloud.with_insights_on_premise?
+    port = ENV['INSIGHTS_ADVISOR_PORT'] || "8000"
+    "http://#{ForemanRhCloud.foreman_host.fqdn || 'localhost'}:#{port}"
+  end
+
   def self.base_url
     # for testing set ENV to 'https://ci.cloud.redhat.com'
-    @base_url ||= ENV['SATELLITE_RH_CLOUD_URL'] || 'https://cloud.redhat.com'
+    @base_url ||= on_premise_url || ENV['SATELLITE_RH_CLOUD_URL'] || 'https://cloud.redhat.com'
   end
 
   def self.cert_base_url
-    @cert_base_url ||= ENV['SATELLITE_CERT_RH_CLOUD_URL'] || 'https://cert.cloud.redhat.com'
+    @cert_base_url ||= on_premise_url || ENV['SATELLITE_CERT_RH_CLOUD_URL'] || 'https://cert.cloud.redhat.com'
   end
 
   def self.legacy_insights_url
-    @legacy_insights_url ||= ENV['SATELLITE_LEGACY_INSIGHTS_URL'] || 'https://cert-api.access.redhat.com'
+    @legacy_insights_url ||= on_premise_url || ENV['SATELLITE_LEGACY_INSIGHTS_URL'] || 'https://cert-api.access.redhat.com'
   end
 
   def self.verify_ssl_method

[jeremylenz]

Also recommended patch. This redirects the remediations + certs to use localhost . May make sense in a separate PR though

I agree, I think this makes more sense as a separate PR. Opened #936

[jeremylenz]

Updated to use local_advisor_engine naming per offline discussion.

[ehelms]

You could consider splitting out the first part, navigation rename, to a stand-alone PR as the two changes are independent.

[ekohl]

You could consider splitting out the first part, navigation rename, to a stand-alone PR as the two changes are independent.

I think that's also a wise thing.

As for the specific setting: I wonder if it should be part of ForemanContext though we may not have a good way to extend that from plugins.

Design wise I don't like that you can change a global setting from regular workflows. That's not how we do things in the application because it completely ignores the multi tenancy model.

[jeremylenz]

As for the specific setting: I wonder if it should be part of ForemanContext though we may not have a good way to extend that from plugins.

This was one of the first things I tried; it would make a lot of sense! It was apparent after an hour or so that adding it to ForemanContext was going to be slow and painful, because plugins were not considered at all there and I hit some dead ends trying to overwrite Ruby private methods from a plugin. I'm sure it's possible somehow but I've moved on to other solutions now. I think I may just add a small API endpoint.

[jeremylenz]

Tasos found that I can't use React context for InsightsTable because the component is reused on the host details page, and thus breaks. So stand by for more updates.

[parthaa]

Please hide sync recommendations

[jeremylenz]

🍏

[parthaa]

Works well