Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Back-end: Edit workflow execution name post launch #889

Merged
merged 5 commits into from
Jan 9, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion app/controllers/concerns/workflow_execution_actions.rb
Original file line number Diff line number Diff line change
@@ -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?

4 changes: 4 additions & 0 deletions app/controllers/projects/workflow_executions_controller.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions app/controllers/workflow_executions_controller.rb
Original file line number Diff line number Diff line change
@@ -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,
8 changes: 8 additions & 0 deletions app/policies/workflow_execution_policy.rb
Original file line number Diff line number Diff line change
@@ -82,6 +82,14 @@ def cancel? # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
false
end

def update?
ericenns marked this conversation as resolved.
Show resolved Hide resolved
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]

16 changes: 16 additions & 0 deletions app/services/workflow_executions/update_service.rb
Original file line number Diff line number Diff line change
@@ -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
Empty file.
Empty file.
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
@@ -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:
1 change: 1 addition & 0 deletions config/locales/fr.yml
Original file line number Diff line number Diff line change
@@ -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:
2 changes: 1 addition & 1 deletion config/routes/project.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion config/routes/workflow_executions.rb
Original file line number Diff line number Diff line change
@@ -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
20 changes: 20 additions & 0 deletions test/controllers/projects/workflow_executions_controller_test.rb
Original file line number Diff line number Diff line change
@@ -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
22 changes: 22 additions & 0 deletions test/controllers/workflow_executions_controller_test.rb
Original file line number Diff line number Diff line change
@@ -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
60 changes: 60 additions & 0 deletions test/services/workflow_executions/update_service_test.rb
Original file line number Diff line number Diff line change
@@ -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