[DO NOT MERGE] Batch connect erb fix

.gitignore

@@ -42,7 +42,6 @@ tags
 !/tmp/.keep
 /public/assets
 .env.local
-/config/initializers/ood.rb
 
 # Ignore staged app
 /vendor/my_app

app/apps/manifest.rb

@@ -58,6 +58,7 @@ def self.load_from_string(yaml)
   # @option opts [String] :icon The icon used on the dashboard, optionally a Font Awesome tag
   # @option opts [String] :role Dashboard categorization
   # @option opts [String] :url An optional redirect URL
+  # @option opts [String, Array<String>] :clusters A cluster id or an array of cluster ids
   def initialize(opts)
     raise InvalidContentError.new unless(opts && opts.respond_to?(:to_h))
 
@@ -113,6 +114,13 @@ def role
     @manifest_options[:role] || ""
   end
 
+  # Return array of the cluster ids for cluster(s) this app depends on
+  #
+  # @return [Array<String>] role as string
+  def clusters
+    Array.wrap(@manifest_options[:clusters]).compact
+  end
+
   # Manifest objects are valid
   #
   # @return [true] Always return true

app/apps/ood_app.rb

@@ -25,12 +25,12 @@ def manifest?
     manifest.valid?
   end
 
-  def invalid_batch_connect_app?
-    batch_connect_app? && batch_connect.sub_app_list.none?(&:valid?)
+  def authorized_batch_connect_app?
+    batch_connect_app? && batch_connect.sub_app_list.none?(&:authorized?)
   end
 
   def should_appear_in_nav?
-    manifest? && ! (invalid_batch_connect_app? || category.empty?)
+    manifest? && ! (authorized_batch_connect_app? || category.empty?)
   end
 
   def initialize(router)
@@ -114,7 +114,7 @@ def links
         end.sort_by { |lnk| lnk.title }
       end
     elsif role == "batch_connect"
-      batch_connect.sub_app_list.select(&:valid?).map(&:link)
+      batch_connect.sub_app_list.select(&:authorized?).map(&:link)
     else
       [
         OodAppLink.new(
@@ -145,7 +145,7 @@ def batch_connect_app?
   end
 
   def batch_connect
-    @batch_connect ||= BatchConnect::App.new(router: router)
+    @batch_connect ||= BatchConnect::AppStub.new(router: router)
   end
 
   def has_gemfile?

app/models/batch_connect/app.rb

@@ -85,9 +85,11 @@ def title
     # Default title for the batch connect app
     # @return [String] default title of app
     def default_title
-      title  = ood_app.title
-      title += ": #{sub_app.titleize}" if sub_app
-      title
+      if sub_app
+        sub_app_manifest.name.presence || sub_app.titleize
+      else
+        ood_app.title
+      end
     end
 
     # Description for the batch connect app
@@ -99,7 +101,21 @@ def description
     # Default description for the batch connect app
     # @return [String] default description of app
     def default_description
-      ood_app.manifest.description
+      if sub_app
+        sub_app_manifest.name.presence || ood_app.manifest.description
+      else
+        ood_app.manifest.description
+      end
+    end
+
+    def sub_app_manifest
+      return @sub_app_manifest if defined?(@sub_app_manifest)
+
+      if sub_app
+        @sub_app_manifest = Manifest.load(sub_app_root.join("#{sub_app}.manifest.yml").to_s)
+      else
+        @sub_app_manifest = MissingManifest.new({})
+      end
     end
 
     def link
@@ -114,10 +130,15 @@ def link
       )
     end
 
-    # Cluster id the batch connect app uses
+    #Cluster id the batch connect app uses
     # @return [String, nil] cluster id used by app
     def cluster_id
-      form_config.fetch(:cluster, nil)
+      # FIXME: if form_config is empty, use ood_app manifest
+      # this is so at OSC, for example we can have an app depend on
+      # Ruby and it be hidden from non Ruby users, and still avoid rendering
+      # erb. This needs to be fixed to handle multiple clusters when we add support
+      # for batch connect apps with multiple clusters.
+      form_config[:cluster] || sub_app_manifest.clusters.first || ood_app.manifest.clusters.first
     end
 
     # The cluster that the batch connect app uses
@@ -126,9 +147,22 @@ def cluster
       OodAppkit.clusters[cluster_id] if cluster_id
     end
 
+
+    # Whether this is an app the user is allowed to use
+    # @return [Boolean] whether user is authorized to use app
+    def authorized?
+      cluster ? cluster.job_allow? : true
+    end
+
     # Whether this is a valid app the user can use
     # @return [Boolean] whether valid app
     def valid?
+      # FIXME: validation reason and valid needs revisited
+      # we should display error when loading page if
+      #
+      #   form config is malformed - erb or yaml error should be displayed to
+      #   user/admin
+      #
       (! form_config.empty?) && cluster && cluster.job_allow?
     end
 

app/models/batch_connect/app_stub.rb

@@ -0,0 +1,12 @@
+module BatchConnect
+  class AppStub < App
+
+    def form_config(binding: nil)
+      if Configuration.render_batch_connect_erb_for_nav?
+        super
+      else
+        {}
+      end
+    end
+  end
+end

config/configuration_singleton.rb

@@ -175,6 +175,10 @@ def locales_root
     Pathname.new(ENV['OOD_LOCALES_ROOT'] || "/etc/ood/config/locales")
   end
 
+  def render_batch_connect_erb_for_nav?
+    to_bool(ENV['OOD_RENDER_BATCH_CONNECT_ERB_FOR_NAV'])
+  end
+
   private
 
   # The environment

test/controllers/dashboard_controller_test.rb

@@ -78,28 +78,87 @@ def dropdown_list_items(list)
     end
   end
 
-  test "should create Interactive Apps dropdown" do
+  test "should create Interactive Apps dropdown (with no manifests)" do
+    Configuration.stubs(:render_batch_connect_erb_for_nav?).returns(false)
     SysRouter.stubs(:base_path).returns(Rails.root.join("test/fixtures/sys_with_interactive_apps"))
     OodAppkit.stubs(:clusters).returns(OodCore::Clusters.load_file("test/fixtures/config/clusters.d"))
 
+    BatchConnect::App.any_instance.expects(:read_yaml_erb).with(any_parameters).never
+
     get :index
 
     dd = dropdown_list('Interactive Apps')
     dditems = dropdown_list_items(dd)
     assert dditems.any?, "dropdown list items not found"
     assert_equal [
       {header: "Apps"},
-      "Jupyter Notebook",
+      "Jupyter",
       "Paraview",
       :divider,
       {header: "Desktops"},
-      "Oakley Desktop"], dditems
+      "Oakley"], dditems
 
-    assert_select dd, "li a", "Oakley Desktop" do |link|
+    assert_select dd, "li a", "Oakley" do |link|
       assert_equal "/batch_connect/sys/bc_desktop/oakley/session_contexts/new", link.first['href'], "Desktops link is incorrect"
     end
   end
 
+  test "should create Interactive Apps dropdown (with erb)" do
+    Configuration.stubs(:render_batch_connect_erb_for_nav?).returns(true)
+    SysRouter.stubs(:base_path).returns(Rails.root.join("test/fixtures/sys_with_interactive_apps"))
+    OodAppkit.stubs(:clusters).returns(OodCore::Clusters.load_file("test/fixtures/config/clusters.d"))
+
+
+    get :index
+
+    assert_equal [
+      {header: "Apps"},
+      "Jupyter Notebook",
+      "Paraview",
+      :divider,
+      {header: "Desktops"},
+      "Oakley Desktop"], dropdown_list_items(dropdown_list('Interactive Apps'))
+  end
+
+  test "should create Interactive Apps dropdown (with manifests)" do
+    Dir.mktmpdir do |dir|
+      base_path = Pathname.new(dir).join('root')
+      FileUtils.cp_r(Rails.root.join("test/fixtures/sys_with_interactive_apps").to_s, base_path.to_s)
+
+      Configuration.stubs(:render_batch_connect_erb_for_nav?).returns(false)
+      SysRouter.stubs(:base_path).returns(base_path)
+      OodAppkit.stubs(:clusters).returns(OodCore::Clusters.load_file("test/fixtures/config/clusters.d"))
+
+      # add manifests
+      base_path.join('bc_jupyter', 'manifest.yml').tap {|p| p.parent.mkpath }.write <<~MANIFEST
+        ---
+        name: Jupyter Notebook
+        category: Interactive Apps
+        subcategory: Apps
+        role: batch_connect
+      MANIFEST
+      base_path.join('bc_desktop', 'local', 'oakley.manifest.yml').tap {|p| p.parent.mkpath }.write <<~MANIFEST
+        ---
+        name: Oakley Desktop
+        category: Interactive Apps
+        subcategory: Desktops
+        role: batch_connect
+      MANIFEST
+
+      BatchConnect::App.any_instance.expects(:read_yaml_erb).with(any_parameters).never
+
+      get :index
+
+      assert_equal [
+        {header: "Apps"},
+        "Jupyter Notebook",
+        "Paraview",
+        :divider,
+        {header: "Desktops"},
+        "Oakley Desktop"], dropdown_list_items(dropdown_list('Interactive Apps'))
+    end
+  end
+
   test "should create My Interactive Apps link if Interactive Apps exist and not developer" do
     SysRouter.stubs(:base_path).returns(Rails.root.join("test/fixtures/sys_with_interactive_apps"))
     Configuration.stubs(:app_development_enabled?).returns(false)

test/models/batch_connect/app_test.rb

@@ -25,15 +25,17 @@ class BatchConnect::AppTest < ActiveSupport::TestCase
     Dir.mktmpdir { |dir|
       r = PathRouter.new(dir + "/missing_app")
       assert_equal "Missing App", BatchConnect::App.new(router: r).title
-      assert_equal "Missing App: Owens Vdi", BatchConnect::App.new(router: r, sub_app: "owens-vdi").title
+      assert_equal "Owens Vdi", BatchConnect::App.new(router: r, sub_app: "owens-vdi").title
     }
   end
 
   test "form.yml.erb can use __FILE__" do
     Dir.mktmpdir { |dir|
+
       r = PathRouter.new(dir)
       r.path.join("form.yml.erb").write("---\ntitle: <%= File.expand_path(File.dirname(__FILE__)) %>")
 
+      Configuration.stubs(:render_batch_connect_erb_for_nav?).returns(true)
       app = BatchConnect::App.new(router: r)
       assert_equal dir, app.title, "When rendering form.yml.erb __FILE__ doesn't return correct value"
     }