[DFCT0010053] Fix labels when showing parameter summary (#611)

* chore: Fixed labels when showing parameter summary

* chore: Fixed enum value display

* chore: fix check_box in NextflowComponent so that you can unset booleans when editting AutomatedWorkflowExecutions

* chore: refactored Irida::Pipelines to be a class and instantiate it during config initialization that way tests don't pollute other tests by modifying pipelines config

* chore: fix rubocop violations

* chore: fix more rubocop violations

* chore: fix file path joining in lib/irida/pipelines.rb

* chore: fix more rubocop violations

---------

Co-authored-by: Eric Enns <[email protected]>

app/components/nextflow_component.html.erb

@@ -25,15 +25,21 @@
     <%= text_for(@workflow.description) %>
   </p>
   <%= form_with url: url, method: (instance.nil? ? :post : :patch), data: { turbo_frame: "_top" } do |form| %>
-    <input id="submission_turbo" type="hidden" name="format" value="turbo_stream" />
+    <input id="submission_turbo" type="hidden" name="format" value="turbo_stream"/>
 
     <%= fields_for :workflow_execution do |workflow| %>
       <section class="mb-4">
         <%= workflow.label :name, class: "block text-sm font-medium text-slate-900 dark:text-white" do %>
           <%= t(".name.label") %>
         <% end %>
-        <p id="helper-text-explanation" class="mt-1 mb-2 text-sm text-slate-500 dark:text-slate-400"><%= t(".name.helper") %></p>
-        <%= workflow.text_field :name, value: instance.present? ? instance.name : nil, class: "bg-slate-50 border border-slate-300 text-slate-900 text-sm rounded-lg focus:ring-blue-500 focus:border-blue-500 block w-full p-2.5  dark:bg-slate-700 dark:border-slate-600 dark:placeholder-slate-400 dark:text-white dark:focus:ring-blue-500 dark:focus:border-blue-500" %>
+        <p
+          id="helper-text-explanation"
+          class="mt-1 mb-2 text-sm text-slate-500 dark:text-slate-400"
+        ><%= t(".name.helper") %></p>
+        <%= workflow.text_field :name,
+                            value: instance.present? ? instance.name : nil,
+                            class:
+                              "bg-slate-50 border border-slate-300 text-slate-900 text-sm rounded-lg focus:ring-blue-500 focus:border-blue-500 block w-full p-2.5  dark:bg-slate-700 dark:border-slate-600 dark:placeholder-slate-400 dark:text-white dark:focus:ring-blue-500 dark:focus:border-blue-500" %>
       </section>
 
       <%= workflow.hidden_field :namespace_id, value: @namespace_id %>
@@ -95,54 +101,51 @@
       <%- if @allowed_to_update_samples %>
         <div class="flex mb-4 items-center h-5">
           <%= workflow.check_box :update_samples,
-                                 {
-                                   checked: instance.present? && instance["update_samples"],
-                                   class:
-                                     "w-4
-                                            h-4
-                                            mr-2.5
-                                            text-primary-600
-                                            bg-slate-100
-                                            border-slate-300
-                                            rounded
-                                            focus:ring-primary-500
-                                            dark:focus:ring-primary-600
-                                            dark:ring-offset-slate-800
-                                            focus:ring-2
-                                            dark:bg-slate-700
-                                            dark:border-slate-600"
-                                 },
-                                 true,
-                                 false %>
+                             {
+                               checked: instance.present? && instance["update_samples"],
+                               class:
+                                 "w-4
+                                                      h-4
+                                                      mr-2.5
+                                                      text-primary-600
+                                                      bg-slate-100
+                                                      border-slate-300
+                                                      rounded
+                                                      focus:ring-primary-500
+                                                      dark:focus:ring-primary-600
+                                                      dark:ring-offset-slate-800
+                                                      focus:ring-2
+                                                      dark:bg-slate-700
+                                                      dark:border-slate-600"
+                             } %>
           <%= workflow.label :update_samples,
-                             t(:"components.nextflow.update_samples"),
-                             class: "mr-2 text-sm font-medium text-slate-900 dark:text-white" %>
+                         t(:"components.nextflow.update_samples"),
+                         class: "mr-2 text-sm font-medium text-slate-900 dark:text-white" %>
         </div>
       <%- end %>
       <div class="flex mb-4 items-center h-5">
         <%= workflow.check_box :email_notification,
-                               {
-                                 checked: instance.present? && instance["email_notification"],
-                                 class:
-                                   "w-4
-                                        h-4
-                                        mr-2.5
-                                        text-primary-600
-                                        bg-slate-100
-                                        border-slate-300
-                                        rounded
-                                        focus:ring-primary-500
-                                        dark:focus:ring-primary-600
-                                        dark:ring-offset-slate-800
-                                        focus:ring-2
-                                        dark:bg-slate-700
-                                        dark:border-slate-600"
-                               },
-                               true,
-                               false %>
+                           {
+                             checked:
+                               instance.present? && instance["email_notification"],
+                             class:
+                               "w-4
+                                                h-4
+                                                mr-2.5
+                                                text-primary-600
+                                                bg-slate-100
+                                                border-slate-300
+                                                rounded
+                                                focus:ring-primary-500
+                                                dark:focus:ring-primary-600
+                                                dark:ring-offset-slate-800
+                                                focus:ring-2
+                                                dark:bg-slate-700
+                                                dark:border-slate-600"
+                           } %>
         <%= workflow.label :email_notification,
-                           t(:"components.nextflow.email_notification"),
-                           class: "mr-2 text-sm font-medium text-slate-900 dark:text-white" %>
+                       t(:"components.nextflow.email_notification"),
+                       class: "mr-2 text-sm font-medium text-slate-900 dark:text-white" %>
       </div>
     <% end %>
 

app/controllers/concerns/workflow_execution_actions.rb

@@ -28,7 +28,7 @@ def show
       @attachments = Attachment.where(attachable: @workflow_execution)
                                .or(Attachment.where(attachable: @samples_worfklow_executions))
     elsif @tab == 'params'
-      @workflow = Irida::Pipelines.find_pipeline_by(@workflow_execution.metadata['workflow_name'],
+      @workflow = Irida::Pipelines.instance.find_pipeline_by(@workflow_execution.metadata['workflow_name'],
                                                     @workflow_execution.metadata['workflow_version'])
     end
   end

app/controllers/projects/automated_workflow_executions_controller.rb

@@ -22,13 +22,13 @@ def new
       authorize! @namespace, to: :create_automated_workflow_executions?
 
       @workflow = if params[:workflow_name].present? && params[:workflow_version].present?
-                    Irida::Pipelines.find_pipeline_by(params[:workflow_name], params[:workflow_version])
+                    Irida::Pipelines.instance.find_pipeline_by(params[:workflow_name], params[:workflow_version])
                   end
     end
 
     def edit
       authorize! @namespace, to: :update_automated_workflow_executions?
-      @workflow = Irida::Pipelines.find_pipeline_by(@automated_workflow_execution.metadata['workflow_name'],
+      @workflow = Irida::Pipelines.instance.find_pipeline_by(@automated_workflow_execution.metadata['workflow_name'],
                                                     @automated_workflow_execution.metadata['workflow_version'])
       render turbo_stream: turbo_stream.update('automated_workflow_execution_modal',
                                                partial: 'edit_dialog',
@@ -124,7 +124,7 @@ def automated_workflow_executions
     end
 
     def available_automated_workflows
-      @available_automated_workflows = Irida::Pipelines.automatable_pipelines
+      @available_automated_workflows = Irida::Pipelines.instance.automatable_pipelines
     end
 
     def context_crumbs

app/controllers/workflow_executions/submissions_controller.rb

@@ -26,14 +26,14 @@ def create
     private
 
     def workflows
-      @workflows = Irida::Pipelines.available_pipelines
+      @workflows = Irida::Pipelines.instance.available_pipelines
     end
 
     def workflow
       workflow_name = params[:workflow_name]
       workflow_version = params[:workflow_version]
 
-      @workflow = Irida::Pipelines.find_pipeline_by(workflow_name, workflow_version)
+      @workflow = Irida::Pipelines.instance.find_pipeline_by(workflow_name, workflow_version)
     end
 
     def samples

app/helpers/nextflow_helper.rb

@@ -55,7 +55,15 @@ def text_for(value)
     if value.instance_of?(String)
       value
     else
-      value[I18n.locale.to_s]
+      value[I18n.locale.to_s] || value[I18n.locale]
+    end
+  end
+
+  def formatted_workflow_param(property, original_value)
+    if !property.key?(:enum) || property[:enum].include?(original_value)
+      original_value
+    else
+      property[:enum].select { |(_label, value)| value == original_value }[0][0]
     end
   end
 end

app/services/automated_workflow_executions/launch_service.rb

@@ -11,7 +11,7 @@ def initialize(automated_workflow_execution, sample, pe_attachment_pair, user =
       @automated_workflow_execution = automated_workflow_execution
       @sample = sample
       @pe_attachment_pair = pe_attachment_pair
-      @workflow = Irida::Pipelines.find_pipeline_by(@automated_workflow_execution.metadata['workflow_name'],
+      @workflow = Irida::Pipelines.instance.find_pipeline_by(@automated_workflow_execution.metadata['workflow_name'],
                                                     @automated_workflow_execution.metadata['workflow_version'])
     end
 

app/services/workflow_executions/create_service.rb

@@ -30,7 +30,7 @@ def execute # rubocop:disable Metrics/AbcSize
     end
 
     def sanitized_workflow_params # rubocop:disable Metrics/AbcSize
-      workflow = Irida::Pipelines.find_pipeline_by(params[:metadata][:workflow_name],
+      workflow = Irida::Pipelines.instance.find_pipeline_by(params[:metadata][:workflow_name],
                                                    params[:metadata][:workflow_version])
       workflow_schema = JSON.parse(workflow.schema_loc.read)
 

app/services/workflow_executions/preparation_service.rb

@@ -131,7 +131,7 @@ def samplesheet_file
     end
 
     def samplesheet_headers
-      workflow = Irida::Pipelines.find_pipeline_by(@workflow_execution.metadata['workflow_name'],
+      workflow = Irida::Pipelines.instance.find_pipeline_by(@workflow_execution.metadata['workflow_name'],
                                                    @workflow_execution.metadata['workflow_version'])
       sample_sheet = JSON.parse(workflow.schema_input_loc.read)
       sample_sheet['items']['properties'].keys

app/views/workflow_executions/_params.html.erb

@@ -4,7 +4,7 @@
       <% definition[:properties].each do |name, property| %>
         <% unless %[input].include? name.to_s %>
           <div class="mb-4">
-            <p class="mb-1 text-sm font-medium text-slate-900 dark:text-white"><%= property[:description] %></p>
+            <p class="mb-1 text-sm font-medium text-slate-900 dark:text-white"><%= text_for(property[:description]) %></p>
             <div class="flex mb-2 <%= name %>-param">
               <span
                 class="
@@ -52,7 +52,7 @@
                   dark:focus:ring-primary-500
                   dark:focus:border-primary-500
                 "
-                value="<%= @workflow_execution.workflow_params[name.to_s] %>"
+                value="<%= formatted_workflow_param(property, @workflow_execution.workflow_params[name.to_s]) %>"
               >
             </div>
           </div>

config/initializers/pipelines.rb

@@ -2,4 +2,6 @@
 
 require 'irida/pipelines'
 
-Irida::Pipelines.register_pipelines unless Irida::Pipelines.initialized
+Rails.application.config.to_prepare do
+  Irida::Pipelines.instance = Irida::Pipelines.new if Irida::Pipelines.instance.nil?
+end

lib/irida/pipelines.rb

@@ -7,20 +7,29 @@
 require 'irida/pipeline'
 
 module Irida
-  # Module that reads a workflow config file and registers the available pipelines
-  module Pipelines
+  # Class that reads a workflow config file and registers the available pipelines
+  class Pipelines
     PipelinesJsonFormatException = Class.new StandardError
     PIPELINES_JSON_SCHEMA = Rails.root.join('config/schemas/pipelines_schema.json')
 
-    @pipeline_config_dir = 'config/pipelines'
-    @pipeline_schema_file_dir = 'private/pipelines'
-    @pipeline_config_file = 'pipelines.json'
-    @pipeline_schema_status_file = 'status.json'
-    @available_pipelines = {}
-    @automatable_pipelines = {}
-    @initialized = false
+    class_attribute :instance
 
-    module_function
+    attr_reader :available_pipelines, :automatable_pipelines
+
+    def initialize(**params)
+      @pipeline_config_dir =
+        params.key?(:pipeline_config_dir) ? params[:pipeline_config_dir] : 'config/pipelines'
+      @pipeline_schema_file_dir =
+        params.key?(:pipeline_schema_file_dir) ? params[:pipeline_schema_file_dir] : 'private/pipelines'
+      @pipeline_config_file =
+        params.key?(:pipeline_config_file) ? params[:pipeline_config_file] : 'pipelines.json'
+      @pipeline_schema_status_file =
+        params.key?(:pipeline_schema_status_file) ? params[:pipeline_schema_status_file] : 'status.json'
+      @available_pipelines = {}
+      @automatable_pipelines = {}
+
+      register_pipelines
+    end
 
     # Registers the available pipelines. This method is called
     # by an initializer which runs when the server is started up
@@ -39,7 +48,6 @@ def register_pipelines
           @automatable_pipelines["#{entry['name']}_#{version['name']}"] = pipeline if version['automatable']
         end
       end
-      @initialized = true
     end
 
     # read in the json pipeline config
@@ -59,7 +67,7 @@ def read_json_config
     def prepare_schema_download(entry, version, type)
       filename = "#{type}.json"
       uri = URI.parse(entry['url'])
-      pipeline_schema_files_path = "#{@pipeline_schema_file_dir}/#{uri.path}/#{version['name']}"
+      pipeline_schema_files_path = File.join(@pipeline_schema_file_dir, uri.path, version['name'])
 
       schema_file_url = if type == 'nextflow_schema'
                           "https://raw.githubusercontent.com#{uri.path}/#{version['name']}/#{filename}"
@@ -68,7 +76,7 @@ def prepare_schema_download(entry, version, type)
                         end
 
       schema_location =
-        Rails.root.join("#{pipeline_schema_files_path}/#{filename}")
+        Rails.root.join(pipeline_schema_files_path, filename)
 
       write_schema_file(schema_file_url, schema_location, pipeline_schema_files_path, filename, type)
 
@@ -78,7 +86,7 @@ def prepare_schema_download(entry, version, type)
     # Create directory if it doesn't exist and write the schema file unless the resource etag matches
     # the currently stored resource etag
     def write_schema_file(schema_file_url, schema_location, pipeline_schema_files_path, filename, type)
-      dir = Rails.root.join("#{pipeline_schema_files_path}/#{filename}").dirname
+      dir = Rails.root.join(pipeline_schema_files_path, filename).dirname
       FileUtils.mkdir_p(dir) unless File.directory?(dir)
 
       return if resource_etag_exists(schema_file_url, pipeline_schema_files_path, type)
@@ -91,7 +99,7 @@ def write_schema_file(schema_file_url, schema_location, pipeline_schema_files_pa
     # local stored file, otherwise we just write the new etag to the status.json
     # file
     def resource_etag_exists(resource_url, status_file_location, etag_type)
-      status_file_location = Rails.root.join("#{status_file_location}/#{@pipeline_schema_status_file}")
+      status_file_location = Rails.root.join(status_file_location, @pipeline_schema_status_file)
       # File currently at pipeline url
       current_file_etag = resource_etag(resource_url)
       existing_etag = false
@@ -139,26 +147,5 @@ def retrieve_headers(url)
     def find_pipeline_by(name, version)
       @available_pipelines["#{name}_#{version}"]
     end
-
-    def available_pipelines
-      @available_pipelines
-    end
-
-    def pipeline_config_dir=(dir)
-      @pipeline_config_dir = dir
-    end
-
-    def pipeline_schema_file_dir=(dir)
-      @pipeline_schema_file_dir = dir
-    end
-
-    def automatable_pipelines
-      @automatable_pipelines
-    end
-
-    # If the pipelines have been initialized or not for the current process
-    def initialized
-      @initialized
-    end
   end
 end

test/lib/irida/pipelines_overrides_test.rb

@@ -3,30 +3,13 @@
 require 'test_helper'
 require 'webmock/minitest'
 
-module Irida
-  module Pipelines
-    module_function
-
-    def pipeline_config_dir=(new_value)
-      @pipeline_config_dir = new_value
-    end
-
-    def pipeline_schema_file_dir=(new_value)
-      @pipeline_schema_file_dir = new_value
-    end
-  end
-end
-
 class PipelinesOverrides < ActiveSupport::TestCase
   setup do
     @pipeline_schema_file_dir = 'tmp/storage/pipelines'
 
     # Read in schema file to json
     body = Rails.root.join('test/fixtures/files/nextflow/nextflow_schema.json')
 
-    Irida::Pipelines.pipeline_config_dir = 'test/config/pipelines_with_overrides'
-    Irida::Pipelines.pipeline_schema_file_dir = @pipeline_schema_file_dir
-
     stub_request(:any, 'https://raw.githubusercontent.com/phac-nml/iridanextexample/2.0.2/nextflow_schema.json')
       .to_return(status: 200, body:, headers: { etag: '[W/"a1Ab"]' })
 
@@ -51,13 +34,14 @@ class PipelinesOverrides < ActiveSupport::TestCase
   end
 
   test 'pipelines with overrides' do
-    Irida::Pipelines.register_pipelines
+    pipelines = Irida::Pipelines.new(pipeline_config_dir: 'test/config/pipelines_with_overrides',
+                                     pipeline_schema_file_dir: @pipeline_schema_file_dir)
 
-    workflow1 = Irida::Pipelines.find_pipeline_by('phac-nml/iridanextexample', '2.0.2')
+    workflow1 = pipelines.find_pipeline_by('phac-nml/iridanextexample', '2.0.2')
     assert_equal 'DEFAULT PROJECT NAME',
                  workflow1.workflow_params[:input_output_options][:properties][:project_name][:default]
 
-    workflow2 = Irida::Pipelines.find_pipeline_by('phac-nml/iridanextexample', '2.0.1')
+    workflow2 = pipelines.find_pipeline_by('phac-nml/iridanextexample', '2.0.1')
     assert_equal 'UNIQUE PROJECT NAME',
                  workflow2.workflow_params[:input_output_options][:properties][:project_name][:default]
   end