diff --git a/app/actors/curation_concerns/actors/base_actor.rb b/app/actors/curation_concerns/actors/base_actor.rb index 19fa79e6c..d1e5b0ec4 100644 --- a/app/actors/curation_concerns/actors/base_actor.rb +++ b/app/actors/curation_concerns/actors/base_actor.rb @@ -12,7 +12,7 @@ def create(attributes) @cloud_resources = attributes.delete(:cloud_resources.to_s) apply_creation_data_to_curation_concern apply_save_data_to_curation_concern(attributes) - next_actor.create(attributes) && save && run_callbacks(:after_create_concern) + save && next_actor.create(attributes) && run_callbacks(:after_create_concern) end def update(attributes) diff --git a/app/actors/curation_concerns/actors/initialize_workflow_actor.rb b/app/actors/curation_concerns/actors/initialize_workflow_actor.rb index 26fabfd12..d8557774f 100644 --- a/app/actors/curation_concerns/actors/initialize_workflow_actor.rb +++ b/app/actors/curation_concerns/actors/initialize_workflow_actor.rb @@ -12,7 +12,7 @@ def create(attributes) # @return [TrueClass] def create_workflow(attributes) - workflow_factory.create(curation_concern, attributes) + workflow_factory.create(curation_concern, attributes, user) end end end diff --git a/app/forms/curation_concerns/forms/workflow_action_form.rb b/app/forms/curation_concerns/forms/workflow_action_form.rb index 258e240bd..ab74cdee2 100644 --- a/app/forms/curation_concerns/forms/workflow_action_form.rb +++ b/app/forms/curation_concerns/forms/workflow_action_form.rb @@ -21,18 +21,16 @@ def initialize(current_ability:, work:, attributes: {}) @work = work @name = attributes.fetch(:name, false) @comment = attributes.fetch(:comment, nil) + convert_to_sipity_objects! end attr_reader :current_ability, :work, :name, :comment def save - convert_to_sipity_objects! return false unless valid? - update_sipity_workflow_state - comment = create_sipity_comment - handle_sipity_notifications(comment: comment) - handle_additional_sipity_workflow_action_processing(comment: comment) - work.update_index # So that the new actions and state are written into solr. + Workflow::WorkflowActionService.run(subject: subject, + action: sipity_workflow_action, + comment: comment) true end @@ -41,7 +39,9 @@ def save def authorized_for_processing return true if CurationConcerns::Workflow::PermissionQuery.authorized_for_processing?( - user: agent, entity: entity, action: sipity_workflow_action + user: subject.agent, + entity: subject.entity, + action: sipity_workflow_action ) errors.add(:base, :unauthorized) false @@ -50,39 +50,13 @@ def authorized_for_processing private def convert_to_sipity_objects! - @entity = PowerConverter.convert(work, to: :sipity_entity) - @agent = PowerConverter.convert(current_user, to: :sipity_agent) - @sipity_workflow_action = PowerConverter.convert_to_sipity_action(name, scope: entity.workflow) + @subject = WorkflowActionInfo.new(work, current_user) + @sipity_workflow_action = PowerConverter.convert_to_sipity_action(name, scope: subject.entity.workflow) end - attr_reader :entity, :agent, :sipity_workflow_action + attr_reader :subject, :sipity_workflow_action delegate :current_user, to: :current_ability - - def create_sipity_comment - return true unless comment.present? - Sipity::Comment.create!(entity: entity, agent: agent, comment: comment) - end - - def update_sipity_workflow_state - return true unless sipity_workflow_action.resulting_workflow_state_id.present? - entity.update_attribute(:workflow_state_id, sipity_workflow_action.resulting_workflow_state_id) - end - - def handle_sipity_notifications(comment:) - CurationConcerns::Workflow::NotificationService.deliver_on_action_taken( - entity: entity, - comment: comment, - action: sipity_workflow_action, - user: current_user - ) - end - - def handle_additional_sipity_workflow_action_processing(comment:) - CurationConcerns::Workflow::ActionTakenService.handle_action_taken( - entity: entity, comment: comment, action: sipity_workflow_action, user: current_user - ) - end end end end diff --git a/app/indexers/curation_concerns/indexes_workflow.rb b/app/indexers/curation_concerns/indexes_workflow.rb index 8903dea72..c6f8006aa 100644 --- a/app/indexers/curation_concerns/indexes_workflow.rb +++ b/app/indexers/curation_concerns/indexes_workflow.rb @@ -22,7 +22,7 @@ def index_workflow_fields(solr_document) entity = PowerConverter.convert_to_sipity_entity(object) return if entity.nil? solr_document[workflow_role_field] = workflow_roles(entity).map { |role| "#{entity.workflow.name}-#{role}" } - solr_document[workflow_state_name_field] = entity.workflow_state.name + solr_document[workflow_state_name_field] = entity.workflow_state.name if entity.workflow_state end def workflow_state_name_field diff --git a/app/models/curation_concerns/workflow_action_info.rb b/app/models/curation_concerns/workflow_action_info.rb new file mode 100644 index 000000000..55c7695e4 --- /dev/null +++ b/app/models/curation_concerns/workflow_action_info.rb @@ -0,0 +1,13 @@ +module CurationConcerns + # A simple data object for holding a user, work and their workflow proxies + class WorkflowActionInfo + def initialize(work, user) + @work = work + @user = user + @entity = PowerConverter.convert(work, to: :sipity_entity) + @agent = PowerConverter.convert(user, to: :sipity_agent) + end + + attr_reader :entity, :agent, :user, :work + end +end diff --git a/app/models/sipity/entity.rb b/app/models/sipity/entity.rb index 167ff8481..f8942ac03 100644 --- a/app/models/sipity/entity.rb +++ b/app/models/sipity/entity.rb @@ -18,8 +18,9 @@ class Entity < ActiveRecord::Base dependent: :destroy, class_name: 'Sipity::Comment' - # Defines the method #workflow_state_name - delegate :name, to: :workflow_state, prefix: :workflow_state + def workflow_state_name + workflow_state.name if workflow_state + end # Defines the method #workflow_name delegate :name, to: :workflow, prefix: :workflow diff --git a/app/services/curation_concerns/actors/actor_factory.rb b/app/services/curation_concerns/actors/actor_factory.rb index ac9186de9..8d30e883d 100644 --- a/app/services/curation_concerns/actors/actor_factory.rb +++ b/app/services/curation_concerns/actors/actor_factory.rb @@ -15,8 +15,8 @@ def self.stack_actors(curation_concern) ApplyOrderActor, InterpretVisibilityActor, GrantEditToDepositorActor, - InitializeWorkflowActor, - model_actor(curation_concern)] + model_actor(curation_concern), + InitializeWorkflowActor] end def self.model_actor(curation_concern) diff --git a/app/services/curation_concerns/workflow/workflow_action_service.rb b/app/services/curation_concerns/workflow/workflow_action_service.rb new file mode 100644 index 000000000..6bf5620dd --- /dev/null +++ b/app/services/curation_concerns/workflow/workflow_action_service.rb @@ -0,0 +1,56 @@ +module CurationConcerns + module Workflow + class WorkflowActionService + def self.run(subject:, action:, comment: nil) + new(subject: subject, action: action, comment: comment).run + end + + def initialize(subject:, action:, comment:) + @subject = subject + @action = action + @comment_text = comment + end + + attr_reader :subject, :action, :comment_text + + def run + update_sipity_workflow_state + comment = create_sipity_comment + handle_sipity_notifications(comment: comment) + handle_additional_sipity_workflow_action_processing(comment: comment) + subject.work.update_index # So that the new actions and state are written into solr. + end + + protected + + def update_sipity_workflow_state + return true unless action.resulting_workflow_state_id.present? + subject.entity.update_attribute(:workflow_state_id, action.resulting_workflow_state_id) + end + + def create_sipity_comment + return true unless comment_text.present? + Sipity::Comment.create!(entity: subject.entity, agent: subject.agent, comment: comment_text) + end + + def handle_sipity_notifications(comment:) + CurationConcerns::Workflow::NotificationService.deliver_on_action_taken( + entity: subject.entity, + comment: comment, + action: action, + user: subject.user + ) + end + + # Run any configured custom methods + def handle_additional_sipity_workflow_action_processing(comment:) + CurationConcerns::Workflow::ActionTakenService.handle_action_taken( + entity: subject.entity, + comment: comment, + action: action, + user: subject.user + ) + end + end + end +end diff --git a/app/services/curation_concerns/workflow/workflow_factory.rb b/app/services/curation_concerns/workflow/workflow_factory.rb index e5ab6d304..602f17d19 100644 --- a/app/services/curation_concerns/workflow/workflow_factory.rb +++ b/app/services/curation_concerns/workflow/workflow_factory.rb @@ -8,30 +8,35 @@ class WorkflowFactory # @param attributes [Hash] # @param strategy [#name] strategy for finding which workflow to use. Defaults to an instance of WorkflowByModelNameStrategy # @return [TrueClass] - def self.create(work, attributes, strategy = nil) + def self.create(work, attributes, user, strategy = nil) strategy ||= workflow_strategy.new(work, attributes) - new(work, attributes, strategy).create + new(work, attributes, user, strategy).create end # @param work [#to_global_id] # @param attributes [Hash] # @param strategy [#name] strategy for finding which workflow to use - def initialize(work, attributes, strategy) + def initialize(work, attributes, user, strategy) @work = work @attributes = attributes + @user = user @strategy = strategy end - attr_reader :work, :attributes, :strategy + attr_reader :work, :attributes, :user, :strategy private :work, :attributes, :strategy # Creates a Sipity::Entity for the work. # The Sipity::Entity acts as a proxy to a work within a workflow # @return [TrueClass] def create - Sipity::Entity.create(proxy_for_global_id: work.to_global_id.to_s, - workflow: workflow, - workflow_state: starting_workflow_state) + Sipity::Entity.create!(proxy_for_global_id: work.to_global_id.to_s, + workflow: workflow, + workflow_state: nil) + + subject = WorkflowActionInfo.new(work, user) + Workflow::WorkflowActionService.run(subject: subject, + action: find_deposit_action) true end @@ -46,14 +51,12 @@ def workflow delegate :workflow_name, to: :strategy - # This returns the initial workflow state. This is derived by finding a WorkflowState - # that has no WorkflowActions leading to it. - # @return [Sipity::WorkflowState] - def starting_workflow_state - action_ids = Sipity::WorkflowAction.where(workflow: workflow) - .pluck(:resulting_workflow_state_id) - relation = Sipity::WorkflowState.where(workflow: workflow) - relation = relation.where('id NOT IN (?)', action_ids) if action_ids.present? + # Find an action that has no starting state. This is the deposit action. + # # @return [Sipity::WorkflowAction] + def find_deposit_action + actions_that_lead_to_states = Sipity::WorkflowStateAction.all.pluck(:workflow_action_id) + relation = Sipity::WorkflowAction.where(workflow: workflow) + relation = relation.where('id NOT IN (?)', actions_that_lead_to_states) if actions_that_lead_to_states.any? relation.first! end end diff --git a/db/migrate/20160919151348_create_sipity.rb b/db/migrate/20160919151348_create_sipity.rb index 104b030cb..ed903aa60 100644 --- a/db/migrate/20160919151348_create_sipity.rb +++ b/db/migrate/20160919151348_create_sipity.rb @@ -61,7 +61,7 @@ def change create_table "sipity_entities" do |t| t.string "proxy_for_global_id", null: false t.integer "workflow_id", null: false - t.integer "workflow_state_id", null: false + t.integer "workflow_state_id", null: true t.datetime "created_at", null: false t.datetime "updated_at", null: false end diff --git a/lib/generators/curation_concerns/templates/workflow.json.erb b/lib/generators/curation_concerns/templates/workflow.json.erb index d78987b81..48e31a4af 100644 --- a/lib/generators/curation_concerns/templates/workflow.json.erb +++ b/lib/generators/curation_concerns/templates/workflow.json.erb @@ -5,6 +5,11 @@ "label": "Digital collections workflow", "description": "A multi-step workflow for digital collections", "actions": [ + { + "name": "deposit", + "from_states": [], + "transition_to": "pending" + }, { "name": "finalize_digitization", "from_states": [{"names": ["pending"], "roles": ["reviewing"]}], diff --git a/spec/actors/curation_concerns/interpret_visibility_actor_spec.rb b/spec/actors/curation_concerns/interpret_visibility_actor_spec.rb index 1379b2d13..3f0b3551b 100644 --- a/spec/actors/curation_concerns/interpret_visibility_actor_spec.rb +++ b/spec/actors/curation_concerns/interpret_visibility_actor_spec.rb @@ -15,6 +15,7 @@ let(:root_actor) { double } before do allow(CurationConcerns::Actors::RootActor).to receive(:new).and_return(root_actor) + allow(curation_concern).to receive(:save).and_return(true) end context 'when visibility is set to open' do diff --git a/spec/actors/curation_concerns/work_actor_spec.rb b/spec/actors/curation_concerns/work_actor_spec.rb index 5ee79f3d8..a51155993 100644 --- a/spec/actors/curation_concerns/work_actor_spec.rb +++ b/spec/actors/curation_concerns/work_actor_spec.rb @@ -36,7 +36,7 @@ context 'success' do before do redlock_client_stub - FactoryGirl.create(:workflow_state) + create(:workflow_action) end it "invokes the after_create_concern callback" do @@ -51,7 +51,7 @@ let(:visibility) { Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED } before do redlock_client_stub - FactoryGirl.create(:workflow_state) + create(:workflow_action) end context 'with embargo' do diff --git a/spec/controllers/curation_concerns/workflow_actions_controller_spec.rb b/spec/controllers/curation_concerns/workflow_actions_controller_spec.rb index 6ed9294e6..c654ce566 100644 --- a/spec/controllers/curation_concerns/workflow_actions_controller_spec.rb +++ b/spec/controllers/curation_concerns/workflow_actions_controller_spec.rb @@ -1,27 +1,32 @@ require 'spec_helper' RSpec.describe CurationConcerns::WorkflowActionsController, type: :controller do - # routes { Rails.application.class.routes } let(:user) { FactoryGirl.create(:user) } let(:generic_work) { GenericWork.new(id: '123') } + let(:form) { instance_double(CurationConcerns::Forms::WorkflowActionForm) } + before do allow(ActiveFedora::Base).to receive(:find).with(generic_work.to_param).and_return(generic_work) allow(generic_work).to receive(:persisted?).and_return(true) + allow(CurationConcerns::Forms::WorkflowActionForm).to receive(:new).and_return(form) end + describe '#update' do it 'will redirect to login path if user not authenticated' do put :update, params: { id: generic_work.to_param, workflow_action: { name: 'advance', comment: '' } } expect(response).to redirect_to(main_app.user_session_path) end + it 'will render :unauthorized when action is not valid for the given user' do - expect_any_instance_of(CurationConcerns::Forms::WorkflowActionForm).to receive(:save).and_return(false) + expect(form).to receive(:save).and_return(false) sign_in(user) put :update, params: { id: generic_work.to_param, workflow_action: { name: 'advance', comment: '' } } expect(response).to be_unauthorized end + it 'will redirect when the form is successfully save' do - expect_any_instance_of(CurationConcerns::Forms::WorkflowActionForm).to receive(:save).and_return(true) + expect(form).to receive(:save).and_return(true) sign_in(user) put :update, params: { id: generic_work.to_param, workflow_action: { name: 'advance', comment: '' } } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ec49a2d3a..59e43ebec 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -74,7 +74,7 @@ # This is a speedup for workflow specs. If we don't have this, it will import the # full workflow configuration files from config/workflows/* - FactoryGirl.create(:workflow_state) if example.metadata[:workflow] + FactoryGirl.create(:workflow_action) if example.metadata[:workflow] end config.include FactoryGirl::Syntax::Methods