diff --git a/.gitignore b/.gitignore index db904a1b..96308787 100644 --- a/.gitignore +++ b/.gitignore @@ -42,7 +42,6 @@ tags !/tmp/.keep /public/assets .env.local -/config/initializers/ood.rb # Ignore staged app /vendor/my_app diff --git a/app/apps/manifest.rb b/app/apps/manifest.rb index fc0ec775..ea5a4b5b 100644 --- a/app/apps/manifest.rb +++ b/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] :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] role as string + def clusters + Array.wrap(@manifest_options[:clusters]).compact + end + # Manifest objects are valid # # @return [true] Always return true diff --git a/app/apps/ood_app.rb b/app/apps/ood_app.rb index 85bc17e7..97bf5c9f 100644 --- a/app/apps/ood_app.rb +++ b/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? diff --git a/app/models/batch_connect/app.rb b/app/models/batch_connect/app.rb index 56a15751..f06677be 100644 --- a/app/models/batch_connect/app.rb +++ b/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 diff --git a/app/models/batch_connect/app_stub.rb b/app/models/batch_connect/app_stub.rb new file mode 100644 index 00000000..43668bb1 --- /dev/null +++ b/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 diff --git a/config/configuration_singleton.rb b/config/configuration_singleton.rb index bee674e1..341ca971 100644 --- a/config/configuration_singleton.rb +++ b/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 diff --git a/test/controllers/dashboard_controller_test.rb b/test/controllers/dashboard_controller_test.rb index f948ce4b..aad84957 100644 --- a/test/controllers/dashboard_controller_test.rb +++ b/test/controllers/dashboard_controller_test.rb @@ -78,10 +78,13 @@ 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') @@ -89,17 +92,73 @@ def dropdown_list_items(list) 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) diff --git a/test/models/batch_connect/app_test.rb b/test/models/batch_connect/app_test.rb index 471a6606..ee3fa3cb 100644 --- a/test/models/batch_connect/app_test.rb +++ b/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" }