Address review feedback for workflow support detection

Author: bstepanovski

apps/dashboard/app/controllers/application_controller.rb

@@ -9,12 +9,6 @@ class ApplicationController < ActionController::Base
   before_action :set_featured_group, :set_custom_navigation
   before_action :check_required_announcements
 
-  helper_method :workflows_supported?
-
-  def workflows_supported?
-    @workflows_supported ||= Workflow.supported?
-  end
-  
   def check_required_announcements
     return if instance_of?(SettingsController)
 

apps/dashboard/app/controllers/projects_controller.rb

@@ -15,7 +15,7 @@ def show
       end
     else
       @launchers = Launcher.all(@project.directory)
-      @workflows = workflows_supported? ? Workflow.all(@project.directory) : []
+      @workflows = Workflow.supported? ? Workflow.all(@project.directory) : []
       @valid_project = Launcher.clusters?
       @valid_scripts = Launcher.scripts?(@project.directory)
 

apps/dashboard/app/controllers/workflows_controller.rb

@@ -123,7 +123,7 @@ def submit
   private
 
   def ensure_workflows_supported!
-    return if workflows_supported?
+    return if Workflow.supported?
 
     message = I18n.t('dashboard.jobs_workflows_not_supported')
 

apps/dashboard/app/helpers/projects_helper.rb

@@ -2,6 +2,10 @@
 
 # Helpers for the projects page
 module ProjectsHelper
+  def workflows_supported?
+    Workflow.supported?
+  end
+  
   def render_readme(readme_location)
     file_content = File.read(readme_location)
 

apps/dashboard/app/models/launcher.rb

@@ -55,10 +55,6 @@ def scripts?(project_dir)
       script_attribute = SmartAttributes::AttributeFactory.build('auto_scripts', { directory: project_dir })
       script_attribute.select_choices(hide_excludable: false).any?
     end
-
-    def workflows_supported?
-      Workflow.supported?
-    end
   end
 
   ID_REX = /\A\w{8}\Z/.freeze

apps/dashboard/app/models/workflow.rb

@@ -6,19 +6,11 @@ class Workflow
 
   class << self
     def supported?
-      dependency_keys = %i[after afterok afternotok afterany].freeze
+      enabled = Configuration.dashboard_workflows_enabled
+      return enabled unless enabled.nil?
 
-      clusters_obj = OodAppkit.clusters
-      clusters =
-        if clusters_obj.respond_to?(:values)
-          Array(clusters_obj.values)
-        elsif clusters_obj.respond_to?(:to_a)
-          Array(clusters_obj.to_a)
-        elsif clusters_obj.respond_to?(:each)
-          clusters_obj.each.to_a
-        else
-          []
-        end.compact
+      dependency_keys = %i[after afterok afternotok afterany].freeze
+      clusters = Configuration.job_clusters.to_a
       return false if clusters.empty?
 
       clusters.any? do |cluster|

apps/dashboard/config/configuration_singleton.rb

@@ -141,6 +141,13 @@ def job_clusters
     )
   end
 
+  # @return [Boolean, nil] configured workflow support override when set, otherwise nil to fall back to auto-detection
+  def dashboard_workflows_enabled
+    return nil unless ENV.key?('OOD_DASHBOARD_WORKFLOWS_ENABLED')
+
+    read_bool(ENV['OOD_DASHBOARD_WORKFLOWS_ENABLED'])
+  end
+
   # @return [String, nil] version string from VERSION file, or nil if no file avail
   def version_from_file(dir)
     file = Pathname.new(dir).join("VERSION")

apps/dashboard/test/config/configuration_singleton_test.rb

@@ -210,6 +210,22 @@ def no_config_env
     end
   end
 
+  test "dashboard_workflows_enabled is nil when unset" do
+    with_modified_env(OOD_DASHBOARD_WORKFLOWS_ENABLED: nil) do
+      assert_nil ConfigurationSingleton.new.dashboard_workflows_enabled
+    end
+  end
+
+  test "dashboard_workflows_enabled can force workflows on or off" do
+    with_modified_env(OOD_DASHBOARD_WORKFLOWS_ENABLED: 'true') do
+      assert ConfigurationSingleton.new.dashboard_workflows_enabled
+    end
+
+    with_modified_env(OOD_DASHBOARD_WORKFLOWS_ENABLED: 'false') do
+      refute ConfigurationSingleton.new.dashboard_workflows_enabled
+    end
+  end
+
   test "support_ticket_enabled? is false by default" do
     assert_equal false, ConfigurationSingleton.new.support_ticket_enabled?
   end

apps/dashboard/test/models/launcher_test.rb

@@ -157,31 +157,4 @@ def setup
       refute(Dir.exist?(Launcher.launchers_dir(tmp).to_s))
     end
   end
-
-  class PositionalHashSubmitAdapter
-    def submit(_script, _opts = {}); end
-  end
-
-  class KeywordSubmitAdapter
-    def submit(_script, after: [], afterok: [], afternotok: [], afterany: []); end
-  end
-
-  class KeyrestSubmitAdapter
-    def submit(_script, **_kwargs); end
-  end
-
-  test 'workflows_supported? returns false when submit does not accept keywords' do
-    OodAppkit.stubs(:clusters).returns({ test: OpenStruct.new(job_adapter: PositionalHashSubmitAdapter.new) })
-    assert_equal false, Launcher.workflows_supported?
-  end
-
-  test 'workflows_supported? returns true when submit accepts dependency keywords' do
-    OodAppkit.stubs(:clusters).returns({ test: OpenStruct.new(job_adapter: KeywordSubmitAdapter.new) })
-    assert_equal true, Launcher.workflows_supported?
-  end
-
-  test 'workflows_supported? returns true when submit accepts arbitrary keywords' do
-    OodAppkit.stubs(:clusters).returns({ test: OpenStruct.new(job_adapter: KeyrestSubmitAdapter.new) })
-    assert_equal true, Launcher.workflows_supported?
-  end
 end

apps/dashboard/test/models/workflow_test.rb

@@ -183,6 +183,52 @@ class WorkflowsTest < ActiveSupport::TestCase
     assert_equal({'bc_num_hours' => '1', 'auto_queues' => 'batch', 'afterok' => ['1234'], 'ood_workflow_sync_key' => 'abc123TOKEN'}, with_key)
   end
 
+  class PositionalHashSubmitAdapter
+    def submit(_script, _opts = {}); end
+  end
+
+  class KeywordSubmitAdapter
+    def submit(_script, after: [], afterok: [], afternotok: [], afterany: []); end
+  end
+
+  class KeyrestSubmitAdapter
+    def submit(_script, **_kwargs); end
+  end
+
+  test 'supported? returns false when no job clusters exist' do
+    Configuration.stubs(:job_clusters).returns([])
+    assert_equal false, Workflow.supported?
+  end
+
+  test 'supported? returns false when submit does not accept dependency keywords' do
+    Configuration.stubs(:job_clusters).returns([OpenStruct.new(job_adapter: PositionalHashSubmitAdapter.new)])
+    assert_equal false, Workflow.supported?
+  end
+
+  test 'supported? returns true when submit accepts dependency keywords' do
+    Configuration.stubs(:job_clusters).returns([OpenStruct.new(job_adapter: KeywordSubmitAdapter.new)])
+    assert_equal true, Workflow.supported?
+  end
+
+  test 'supported? returns true when submit accepts arbitrary keywords' do
+    Configuration.stubs(:job_clusters).returns([OpenStruct.new(job_adapter: KeyrestSubmitAdapter.new)])
+    assert_equal true, Workflow.supported?
+  end
+
+  test 'supported? honors OOD_DASHBOARD_WORKFLOWS_ENABLED=false override' do
+    Configuration.stubs(:job_clusters).returns([OpenStruct.new(job_adapter: KeywordSubmitAdapter.new)])
+    with_modified_env(OOD_DASHBOARD_WORKFLOWS_ENABLED: 'false') do
+      assert_equal false, Workflow.supported?
+    end
+  end
+
+  test 'supported? honors OOD_DASHBOARD_WORKFLOWS_ENABLED=true override' do
+    Configuration.stubs(:job_clusters).returns([])
+    with_modified_env(OOD_DASHBOARD_WORKFLOWS_ENABLED: 'true') do
+      assert_equal true, Workflow.supported?
+    end
+  end
+  
   def create_workflow(id: nil, name: 'test-workflow', description: 'description', project_dir: nil, launcher_ids: [], sync_key_enabled: '0')
     attrs = { name: name, id: id, description: description, project_dir: project_dir, launcher_ids: launcher_ids, sync_key_enabled: sync_key_enabled}
     workflow = Workflow.new(attrs)