diff --git a/app/controllers/concerns/workflow_execution_actions.rb b/app/controllers/concerns/workflow_execution_actions.rb index 00c87dc559..667e061035 100644 --- a/app/controllers/concerns/workflow_execution_actions.rb +++ b/app/controllers/concerns/workflow_execution_actions.rb @@ -7,7 +7,7 @@ module WorkflowExecutionActions # rubocop:disable Metrics/ModuleLength included do before_action :set_default_tab, only: :show before_action :current_page, only: %i[show index] - before_action :workflow_execution, only: %i[show cancel destroy] + before_action :workflow_execution, only: %i[show cancel destroy update edit] end TABS = %w[summary params samplesheet files].freeze @@ -22,6 +22,24 @@ def index @pagy, @workflow_executions = pagy_with_metadata_sort(@q.result) end + def edit + authorize! @namespace + end + + def update + respond_to do |format| + format.turbo_stream do + @updated = WorkflowExecutions::UpdateService.new(@workflow_execution, current_user, + workflow_execution_update_params).execute + if @updated + render status: :ok + else + render status: :unprocessable_entity + end + end + end + end + def show authorize! @namespace, to: :view_workflow_executions? unless @namespace.nil? diff --git a/app/controllers/projects/workflow_executions_controller.rb b/app/controllers/projects/workflow_executions_controller.rb index 58207291d1..b6d255ace9 100644 --- a/app/controllers/projects/workflow_executions_controller.rb +++ b/app/controllers/projects/workflow_executions_controller.rb @@ -19,6 +19,10 @@ def workflow_execution @workflow_execution = WorkflowExecution.find_by!(id: params[:id], submitter: @project.namespace.automation_bot) end + def workflow_execution_update_params + params.require(:workflow_execution).permit(:name) + end + def load_workflows authorized_scope(WorkflowExecution, type: :relation, as: :automated, scope_options: { project: @project }) end diff --git a/app/controllers/workflow_executions_controller.rb b/app/controllers/workflow_executions_controller.rb index 788276951b..83362f1914 100644 --- a/app/controllers/workflow_executions_controller.rb +++ b/app/controllers/workflow_executions_controller.rb @@ -30,6 +30,10 @@ def workflow_execution_params params.require(:workflow_execution).permit(workflow_execution_params_attributes) end + def workflow_execution_update_params + params.require(:workflow_execution).permit(:name) + end + def workflow_execution_params_attributes [ :name, diff --git a/app/policies/workflow_execution_policy.rb b/app/policies/workflow_execution_policy.rb index 52aa0db719..3adb434c00 100644 --- a/app/policies/workflow_execution_policy.rb +++ b/app/policies/workflow_execution_policy.rb @@ -82,6 +82,14 @@ def cancel? # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity false end + def update? + return true if record.submitter.id == user.id + return true if effective_access_level >= Member::AccessLevel::ANALYST + + details[:id] = record.id + false + end + scope_for :relation, :automated do |relation, options| project = options[:project] diff --git a/app/services/workflow_executions/update_service.rb b/app/services/workflow_executions/update_service.rb new file mode 100644 index 0000000000..bae4c8e76c --- /dev/null +++ b/app/services/workflow_executions/update_service.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module WorkflowExecutions + # Service used to Update a WorkflowExecution + class UpdateService < BaseService + def initialize(workflow_execution, user = nil, params = {}) + super(user, params) + @workflow_execution = workflow_execution + end + + def execute + authorize! @workflow_execution, to: :update? + @workflow_execution.update!(params) + end + end +end diff --git a/app/views/projects/workflow_executions/update.turbo_stream.erb b/app/views/projects/workflow_executions/update.turbo_stream.erb new file mode 100644 index 0000000000..e69de29bb2 diff --git a/app/views/workflow_executions/update.turbo_stream.erb b/app/views/workflow_executions/update.turbo_stream.erb new file mode 100644 index 0000000000..e69de29bb2 diff --git a/config/locales/en.yml b/config/locales/en.yml index 83af042ea1..26af7d1fd7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -97,6 +97,7 @@ en: destroy?: You are not authorized to destroy workflow executions for %{namespace_type} %{name} index?: You are not authorized to view workflow executions for %{namespace_type} %{name} read?: You are not authorized to view workflow execution %{id} + update?: You are not authorized to update workflow execution %{id} unauthorized: You are not authorized to perform this action activerecord: attributes: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index d1744de172..3df04efbb1 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -97,6 +97,7 @@ fr: destroy?: You are not authorized to destroy workflow executions for %{namespace_type} %{name} index?: You are not authorized to view workflow executions for %{namespace_type} %{name} read?: You are not authorized to view workflow execution %{id} + update?: You are not authorized to update workflow execution %{id} unauthorized: You are not authorized to perform this action activerecord: attributes: diff --git a/config/routes/project.rb b/config/routes/project.rb index fb80f5d485..41583c954a 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -35,7 +35,7 @@ if Irida::Pipelines.instance.available_pipelines.any? resources :automated_workflow_executions - resources :workflow_executions, only: %i[index destroy show] do + resources :workflow_executions, only: %i[index destroy show edit update] do member do put :cancel end diff --git a/config/routes/workflow_executions.rb b/config/routes/workflow_executions.rb index 3d69df9d2d..b4cf7f8fa4 100644 --- a/config/routes/workflow_executions.rb +++ b/config/routes/workflow_executions.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -resources :workflow_executions, only: %i[index create show destroy] do +resources :workflow_executions, only: %i[index create show destroy edit update] do member do put :cancel end diff --git a/test/controllers/projects/workflow_executions_controller_test.rb b/test/controllers/projects/workflow_executions_controller_test.rb index 0a3cb72468..c191cd561b 100644 --- a/test/controllers/projects/workflow_executions_controller_test.rb +++ b/test/controllers/projects/workflow_executions_controller_test.rb @@ -214,5 +214,25 @@ class WorkflowExecutionsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to namespace_project_workflow_executions_path(@namespace, @project) end + + test 'analyst or higher access level can update workflow execution name post launch' do + update_params = { workflow_execution: { name: 'New Name' } } + + put namespace_project_workflow_execution_path(@namespace, @project, @workflow_execution, format: :turbo_stream), + params: update_params + + assert_response :success + end + + test 'access level less than analyst cannot update workflow execution name post launch for an automated workflow' do + sign_in users(:ryan_doe) + + update_params = { workflow_execution: { name: 'New Name' } } + + put namespace_project_workflow_execution_path(@namespace, @project, @workflow_execution, format: :turbo_stream), + params: update_params + + assert_response :unauthorized + end end end diff --git a/test/controllers/workflow_executions_controller_test.rb b/test/controllers/workflow_executions_controller_test.rb index 4832330fd8..17388440b7 100644 --- a/test/controllers/workflow_executions_controller_test.rb +++ b/test/controllers/workflow_executions_controller_test.rb @@ -212,4 +212,26 @@ class WorkflowExecutionsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to workflow_executions_path end + + test 'Submitter can update workflow execution name post launch' do + workflow_execution = workflow_executions(:irida_next_example_new) + + update_params = { workflow_execution: { name: 'New Name' } } + + put workflow_execution_path(workflow_execution, format: :turbo_stream), params: update_params + + assert_response :success + end + + test 'Cannot update another user\'s personal workflow execution name' do + sign_in users(:ryan_doe) + + workflow_execution = workflow_executions(:irida_next_example_new) + + update_params = { workflow_execution: { name: 'New Name' } } + + put workflow_execution_path(workflow_execution, format: :turbo_stream), params: update_params + + assert_response :not_found + end end diff --git a/test/services/workflow_executions/update_service_test.rb b/test/services/workflow_executions/update_service_test.rb new file mode 100644 index 0000000000..0327849b41 --- /dev/null +++ b/test/services/workflow_executions/update_service_test.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'test_helper' + +module WorkflowExecutions + class UpdateServiceTest < ActiveSupport::TestCase + def setup + @user = users(:john_doe) + @workflow_execution = workflow_executions(:irida_next_example_new) + end + + test 'submitter can update name of workflow execution post launch' do + valid_params = { name: 'New Name' } + + assert_changes -> { @workflow_execution.name }, to: 'New Name' do + WorkflowExecutions::UpdateService.new(@workflow_execution, @user, valid_params).execute + end + end + + test 'user cannot update workflow execution name for another user\'s personal workflow execution' do + valid_params = { name: 'New Name' } + + exception = assert_raises(ActionPolicy::Unauthorized) do + WorkflowExecutions::UpdateService.new(@workflow_execution, users(:michelle_doe), valid_params).execute + end + + assert_equal WorkflowExecutionPolicy, exception.policy + assert_equal :update?, exception.rule + assert exception.result.reasons.is_a?(::ActionPolicy::Policy::FailureReasons) + assert_equal I18n.t(:'action_policy.policy.workflow_execution.update?', + id: @workflow_execution.id), + exception.result.message + end + + test 'analyst or higher can update workflow execution name for automated workflow execution' do + workflow_execution = workflow_executions(:automated_workflow_execution) + valid_params = { name: 'New Name Automated' } + + assert_changes -> { workflow_execution.name }, to: 'New Name Automated' do + WorkflowExecutions::UpdateService.new(workflow_execution, users(:james_doe), valid_params).execute + end + end + + test 'access level below analyst cannot update workflow execution name for automated workflow execution' do + workflow_execution = workflow_executions(:automated_workflow_execution) + valid_params = { name: 'New Name' } + + exception = assert_raises(ActionPolicy::Unauthorized) do + WorkflowExecutions::UpdateService.new(workflow_execution, users(:ryan_doe), valid_params).execute + end + + assert_equal WorkflowExecutionPolicy, exception.policy + assert_equal :update?, exception.rule + assert exception.result.reasons.is_a?(::ActionPolicy::Policy::FailureReasons) + assert_equal I18n.t(:'action_policy.policy.workflow_execution.update?', + id: workflow_execution.id), + exception.result.message + end + end +end