From 2f57f7c43d8632d7d63168e0f3e1c75f89ba58cd Mon Sep 17 00:00:00 2001 From: Pierre Rioux Date: Thu, 4 Dec 2025 13:08:20 -0500 Subject: [PATCH 1/2] Apply validations to task arrays --- .../app/controllers/tasks_controller.rb | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/BrainPortal/app/controllers/tasks_controller.rb b/BrainPortal/app/controllers/tasks_controller.rb index dad744a06..27ace3b99 100644 --- a/BrainPortal/app/controllers/tasks_controller.rb +++ b/BrainPortal/app/controllers/tasks_controller.rb @@ -394,7 +394,7 @@ def create #:nodoc: @task.errors.add(:tool_config_id, 'is on an Execution Server that is currently offline') end - unless @task.errors.empty? && @task.valid? + if @task.errors.any? || ! @task.valid? flash.now[:error] += messages initialize_common_form_values respond_to do |format| @@ -406,15 +406,37 @@ def create #:nodoc: end # Create a bunch of tasks and launch them, either in background or in foreground - tasklist,messages = create_tasklist_from_initial_task(@task) + tasklist,tl_messages = create_tasklist_from_initial_task(@task) # 'tasklist' messages + + # Revalidate everything for all the tasks in the new tasklist + # This is kind of costly, but we have no choices if special validation + # modules have been loaded for after_form(). + # As soon as any task in the task list is not valid, we stop checking and + # return to the form. + af_messages = "" # 'after_form' messages + tasklist.each do |task| + af_messages += task.wrapper_after_form + af_messages = af_messages.split("\n").uniq.join("\n") # remove duplicate messages, if any + next if task.errors.empty? && task.valid? # move on to next task if everything ok + + # Found a faulty task, so inform user and return to launch form. + @task.errors.add(:base, "While creating a set of tasks, one of them was found to be invalid, probably because a file selected as input was not appropriate.") + flash.now[:error] += af_messages + respond_to do |format| + format.html { render :action => 'new' } + format.xml { render :xml => @task.errors, :status => :unprocessable_entity } + format.json { render :json => @task.errors, :status => :unprocessable_entity } + end + return + end if tasklist.size == 1 flash[:notice] += "Launching a #{@task.pretty_name} task in background." else flash[:notice] += "Launching #{tasklist.size} #{@task.pretty_name} tasks in background." end - flash[:notice] += "\n" unless messages.blank? || messages =~ /\n$/ - flash[:notice] += messages + "\n" unless messages.blank? + flash[:notice] += "\n#{tk_messages.strip}" if tl_messages.present? + flash[:notice] += "\n#{af_messages.strip}" if af_messages.present? # Increment the number of times the user has launched this particular tool tool_id = @task.tool.id From 9e1a0f9be0f971c47d45ed4735c80eab4c7712d3 Mon Sep 17 00:00:00 2001 From: Pierre Rioux Date: Thu, 4 Dec 2025 15:26:56 -0500 Subject: [PATCH 2/2] More adjustments for validations of task arrays. --- .../app/controllers/tasks_controller.rb | 33 ++++++++++--------- .../app/models/boutiques_portal_task.rb | 14 +++++--- BrainPortal/app/models/portal_task.rb | 11 +++++++ 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/BrainPortal/app/controllers/tasks_controller.rb b/BrainPortal/app/controllers/tasks_controller.rb index 27ace3b99..451b86cdf 100644 --- a/BrainPortal/app/controllers/tasks_controller.rb +++ b/BrainPortal/app/controllers/tasks_controller.rb @@ -405,8 +405,9 @@ def create #:nodoc: return end - # Create a bunch of tasks and launch them, either in background or in foreground - tasklist,tl_messages = create_tasklist_from_initial_task(@task) # 'tasklist' messages + # Prepare final list of tasks; from the one maintask object we have, + # we get a full array of clones of that task in tasklist + tasklist,ftl_message = @task.wrapper_final_task_list # Revalidate everything for all the tasks in the new tasklist # This is kind of costly, but we have no choices if special validation @@ -414,14 +415,18 @@ def create #:nodoc: # As soon as any task in the task list is not valid, we stop checking and # return to the form. af_messages = "" # 'after_form' messages - tasklist.each do |task| + tasklist.each_with_index do |task,idx| af_messages += task.wrapper_after_form af_messages = af_messages.split("\n").uniq.join("\n") # remove duplicate messages, if any next if task.errors.empty? && task.valid? # move on to next task if everything ok - # Found a faulty task, so inform user and return to launch form. - @task.errors.add(:base, "While creating a set of tasks, one of them was found to be invalid, probably because a file selected as input was not appropriate.") + # Found at least one faulty task, so inform user + task.errors.each { |key,message| @task.errors.add(key,message) } # copy error messages, if any + @task.errors.add(:base, "While creating a set of tasks, at least one of them was found to be invalid, probably because a file selected as input was not appropriate.") flash.now[:error] += af_messages + + # Re-render the form + initialize_common_form_values respond_to do |format| format.html { render :action => 'new' } format.xml { render :xml => @task.errors, :status => :unprocessable_entity } @@ -430,13 +435,17 @@ def create #:nodoc: return end + # Create a bunch of tasks and launch them, either in background or in foreground + tl_messages = create_tasklist_from_initial_task(@task,tasklist) + if tasklist.size == 1 flash[:notice] += "Launching a #{@task.pretty_name} task in background." else flash[:notice] += "Launching #{tasklist.size} #{@task.pretty_name} tasks in background." end - flash[:notice] += "\n#{tk_messages.strip}" if tl_messages.present? flash[:notice] += "\n#{af_messages.strip}" if af_messages.present? + flash[:notice] += "\n#{ftl_message.strip}" if ftl_message.present? + flash[:notice] += "\n#{tl_messages.strip}" if tl_messages.present? # Increment the number of times the user has launched this particular tool tool_id = @task.tool.id @@ -1417,7 +1426,7 @@ def create_initial_task_from_form(new_task_info, tool_id = nil) #:nodoc: # Part of the create() process for a task # # Code extracted from the old monolithic 'create' - def create_tasklist_from_initial_task(maintask) #:nodoc: + def create_tasklist_from_initial_task(maintask,tasklist) #:nodoc: messages = "" @@ -1443,14 +1452,6 @@ def create_tasklist_from_initial_task(maintask) #:nodoc: messages += "Warning: parallelization cannot be performed until the admin configures a Tool for it.\n" end - # Prepare final list of tasks; from the one maintask object we have, - # we get a full array of clones of that task in tasklist - tasklist,task_list_message = maintask.wrapper_final_task_list - if task_list_message.present? - messages += "\n" if messages.present? - messages += task_list_message - end - # Spawn a background process to launch the tasks. # In case of API requests, we don't spawn. CBRAIN.spawn_with_active_records_if(! api_request?, :admin, "Spawn Tasks") do @@ -1524,7 +1525,7 @@ def create_tasklist_from_initial_task(maintask) #:nodoc: end # CBRAIN spawn_if block - return tasklist,messages + return messages end # Some useful variables for the views for 'new' and 'edit' diff --git a/BrainPortal/app/models/boutiques_portal_task.rb b/BrainPortal/app/models/boutiques_portal_task.rb index 05724fc33..6a16c397f 100644 --- a/BrainPortal/app/models/boutiques_portal_task.rb +++ b/BrainPortal/app/models/boutiques_portal_task.rb @@ -342,6 +342,10 @@ def final_task_list #:nodoc: end end + # Re-introduce the file IDs of the task list, in case the main form + # needs to be re-rendered. + self.params[:interface_userfile_ids] |= task_array_userfiles_ids + return tasklist.flatten end # When only one file input @@ -505,8 +509,8 @@ def validateCols(cbcsv,id) # Ensure that the +input+ parameter is not null and matches a generic tool # parameter type (:file, :numeric, :string or :flag) before converting the # parameter's value to the corresponding Ruby type (if appropriate). - # For example, sanitize_param(someinput) where someinput's name is 'deviation' - # and someinput's type is 'numeric' would validate that + # For example, sanitize_param(someinput) where someinput's name is 'deviation' + # and someinput's type is 'numeric' would validate that # self.params['invoke']['deviation'] is a number and then convert it to a Ruby Float or # Integer. # @@ -548,7 +552,7 @@ def sanitize_param(input) # Taken userfile names. An error will be raised if two input files have the # same name. - @taken_files ||= Set.new + @taken_files ||= {} # Fetch the parameter and convert to an Enumerable if required values = invoke_params[name] @@ -608,10 +612,10 @@ def sanitize_param(input) next nil # remove bad value end - if @taken_files.include?(file.id) + if @taken_files[file.id].present? && @taken_files[file.id] != input.id params_errors.add(invokename, ": file name already in use (#{file.name})") else - @taken_files.add(file.id) + @taken_files[file.id] = input.id end end diff --git a/BrainPortal/app/models/portal_task.rb b/BrainPortal/app/models/portal_task.rb index 3bb7197a3..3fdd4759b 100644 --- a/BrainPortal/app/models/portal_task.rb +++ b/BrainPortal/app/models/portal_task.rb @@ -743,6 +743,17 @@ def params_errors @params_errors_cache end + # Needed in case of a dup() + def params_errors_clear #:nodoc: + @params_errors_cache = nil + end + + def dup #:nodoc: + obj = super + obj.params_errors_clear + obj + end + # This method returns a 'pretty' name for a params attributes. # This implementation will try to look up a hash table returned # by the class method pretty_params_names() first, so an