From 7e922e651e3993a5a5302bda1a32fbbd650fc5d6 Mon Sep 17 00:00:00 2001 From: Alexandra Dunn Date: Wed, 8 Nov 2023 09:05:30 -0800 Subject: [PATCH 1/3] WorkflowActionForm: run service as a background job --- app/forms/hyrax/forms/workflow_action_form.rb | 12 +++++++++--- app/jobs/workflow_action_job.rb | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 app/jobs/workflow_action_job.rb diff --git a/app/forms/hyrax/forms/workflow_action_form.rb b/app/forms/hyrax/forms/workflow_action_form.rb index a7509971c7..be87ba76c1 100644 --- a/app/forms/hyrax/forms/workflow_action_form.rb +++ b/app/forms/hyrax/forms/workflow_action_form.rb @@ -33,9 +33,15 @@ def initialize(current_ability:, work:, attributes: {}) def save return false unless valid? - Workflow::WorkflowActionService.run(subject: subject, - action: sipity_workflow_action, - comment: comment) + + WorkflowActionJob.perform_later( + comment: comment, + name: name, + user: current_user, + work_id: work.id.to_s, + workflow: subject.entity.workflow + ) + true end diff --git a/app/jobs/workflow_action_job.rb b/app/jobs/workflow_action_job.rb new file mode 100644 index 0000000000..4e646cc208 --- /dev/null +++ b/app/jobs/workflow_action_job.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +class WorkflowActionJob < Hyrax::ApplicationJob + def perform(comment: nil, name: false, user:, work_id:, workflow:) + work = Hyrax.query_service.find_by(id: work_id) + subject = ::Hyrax::WorkflowActionInfo.new(work, user) + action = Sipity::WorkflowAction(name, workflow) + + ::Hyrax::Workflow::WorkflowActionService.run( + subject: subject, + action: action, + comment: comment + ) + end +end From d4397b0b726d875caee26221eb5cc2e8a3681688 Mon Sep 17 00:00:00 2001 From: Alexandra Dunn Date: Mon, 13 Nov 2023 11:51:34 -0800 Subject: [PATCH 2/3] update specs for background workflow action job --- .../hyrax/forms/workflow_action_form_spec.rb | 84 ++++--------------- .../workflow/workflow_action_service_spec.rb | 77 +++++++++++++++++ 2 files changed, 91 insertions(+), 70 deletions(-) create mode 100644 spec/services/hyrax/workflow/workflow_action_service_spec.rb diff --git a/spec/forms/hyrax/forms/workflow_action_form_spec.rb b/spec/forms/hyrax/forms/workflow_action_form_spec.rb index 3eda9f2f16..45af7854b3 100644 --- a/spec/forms/hyrax/forms/workflow_action_form_spec.rb +++ b/spec/forms/hyrax/forms/workflow_action_form_spec.rb @@ -43,30 +43,15 @@ it 'gives false for failure' do expect(form.save).to be false end - - it 'will not add a comment' do - expect { form.save }.not_to change { Sipity::Comment.count } - end - - it 'will not send the #deliver_on_action_taken message to Hyrax::Workflow::NotificationService' do - expect(Hyrax::Workflow::NotificationService) - .not_to receive(:deliver_on_action_taken) - - form.save - end - - it 'will not send the #handle_action_taken message to Hyrax::Workflow::ActionTakenService' do - expect(Hyrax::Workflow::ActionTakenService) - .not_to receive(:handle_action_taken) - - form.save - end end end context 'if the given user can perform the given action' do before do - allow(described_class).to receive(:workflow_action_for).with('an_action', scope: sipity_entity.workflow).and_return(an_action) + allow(described_class) + .to receive(:workflow_action_for) + .with('an_action', scope: sipity_entity.workflow) + .and_return(an_action) expect(Hyrax::Workflow::PermissionQuery) .to receive(:authorized_for_processing?) @@ -86,57 +71,16 @@ expect(form.save).to be true end - context 'and the action has a resulting_workflow_state_id' do - it 'will update the state of the given work and index it' do - expect(work).to receive(:update_index) - - expect { form.save } - .to change { sipity_entity.reload.workflow_state_id } - .from(2) - .to(an_action.resulting_workflow_state_id) - end - end - - context 'and the action does not have a resulting_workflow_state_id' do - let(:an_action) do - instance_double(Sipity::WorkflowAction, - resulting_workflow_state_id: nil, - notifiable_contexts: [], - triggered_methods: Sipity::Method.none) - end - - it 'will not update the state of the given work' do - expect { form.save } - .not_to change { sipity_entity.reload.workflow_state_id } - end - end - - it 'will create the given comment for the entity' do - expect { form.save } - .to change { Sipity::Comment.count } - .by(1) - end - - it 'will send the #deliver_on_action_taken message to Hyrax::Workflow::NotificationService' do - expect(Hyrax::Workflow::NotificationService) - .to receive(:deliver_on_action_taken) - .with(entity: sipity_entity, - comment: kind_of(Sipity::Comment), - action: an_action, - user: user) - - form.save - end - - it 'will send the #handle_action_taken message to Hyrax::Workflow::ActionTakenService' do - expect(Hyrax::Workflow::ActionTakenService) - .to receive(:handle_action_taken) - .with(target: work, - comment: kind_of(Sipity::Comment), - action: an_action, - user: user) - - form.save + it 'enqueues WorkflowActionJob' do + expect(WorkflowActionJob).to( + receive(:perform_later).with( + comment: 'a_comment', + name: 'an_action', + user: user, + work_id: work.id.to_s, + workflow: sipity_entity.workflow + ) + ) end end end diff --git a/spec/services/hyrax/workflow/workflow_action_service_spec.rb b/spec/services/hyrax/workflow/workflow_action_service_spec.rb new file mode 100644 index 0000000000..f2ee49960e --- /dev/null +++ b/spec/services/hyrax/workflow/workflow_action_service_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true +require "spec_helper" + +RSpec.describe Hyrax::Workflow::WorkflowActionService, :clean_repo do + let(:user) { FactoryBot.create(:user) } + let(:work) { FactoryBot.valkyrie_create(:hyrax_work) } + + let(:sipity_entity) do + FactoryBot + .create(:sipity_entity, + proxy_for_global_id: work.to_global_id.to_s, + workflow_state_id: 2) + end + + let(:an_action) do + instance_double(Sipity::WorkflowAction, + resulting_workflow_state_id: 3, + notifiable_contexts: [], + triggered_methods: Sipity::Method.none) + end + + describe '.run' do + subject do + described_class.run( + subject: ::Hyrax::WorkflowActionInfo.new(work, user), + action: an_action, + comment: 'a_comment' + ) + end + + context 'the action has a resulting_workflow_state_id' do + it 'will update the state of the given work and index it' do + expect(work).to receive(:update_index) + + expect(sipity_entity.reload.workflow_state_id) + .to change + .from(2) + .to(an_action.resulting_workflow_state_id) + end + end + + context 'and the action does not have a resulting_workflow_state_id' do + let(:an_action) do + instance_double(Sipity::WorkflowAction, + resulting_workflow_state_id: nil, + notifiable_contexts: [], + triggered_methods: Sipity::Method.none) + end + + it 'will not update the state of the given work' do + expect(sipity_entity.reload.workflow_state_id).not_to change + end + end + + it 'will create the given comment for the entity' do + expect(Sipity::Comment.count).to change.by(1) + end + + it 'will send the #deliver_on_action_taken message to Hyrax::Workflow::NotificationService' do + expect(Hyrax::Workflow::NotificationService) + .to receive(:deliver_on_action_taken) + .with(entity: sipity_entity, + comment: kind_of(Sipity::Comment), + action: an_action, + user: user) + end + + it 'will send the #handle_action_taken message to Hyrax::Workflow::ActionTakenService' do + expect(Hyrax::Workflow::ActionTakenService) + .to receive(:handle_action_taken) + .with(target: work, + comment: kind_of(Sipity::Comment), + action: an_action, + user: user) + end + end +end From 6e1d662aa1703982041403e7e5121198f5906639 Mon Sep 17 00:00:00 2001 From: Alexandra Dunn Date: Mon, 13 Nov 2023 12:53:10 -0800 Subject: [PATCH 3/3] remove workflow state feature spec because workflow state upstates are a background job, adding a comment is no longer a synchronous action --- spec/features/workflow_state_changes_spec.rb | 59 -------------------- 1 file changed, 59 deletions(-) delete mode 100644 spec/features/workflow_state_changes_spec.rb diff --git a/spec/features/workflow_state_changes_spec.rb b/spec/features/workflow_state_changes_spec.rb deleted file mode 100644 index 9caec83bcb..0000000000 --- a/spec/features/workflow_state_changes_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true -RSpec.describe "Workflow state changes", type: :feature do - let(:workflow_name) { 'with_comment' } - let(:approving_user) { create(:admin) } - let(:depositing_user) { create(:admin) } - let(:admin_set) { create(:admin_set, edit_users: [depositing_user.user_key]) } - let(:one_step_workflow) do - { - workflows: [ - { - name: workflow_name, - label: "One-step mediated deposit workflow", - description: "A single-step workflow for mediated deposit", - actions: [ - { - name: "deposit", from_states: [], transition_to: "pending_review" - }, { - name: "approve", from_states: [{ names: ["pending_review"], roles: ["approving"] }], - transition_to: "deposited", - methods: ["Hyrax::Workflow::ActivateObject"] - }, { - name: "leave_a_comment", from_states: [{ names: ["pending_review"], roles: ["approving"] }] - } - ] - } - ] - } - end - - let(:workflow) { Sipity::Workflow.find_by!(name: workflow_name, permission_template: permission_template) } - let(:work) { create(:work, user: depositing_user, admin_set: admin_set) } - let(:permission_template) { create(:permission_template, source_id: admin_set.id) } - - before do - Hyrax::Workflow::WorkflowImporter.generate_from_hash(data: one_step_workflow, permission_template: permission_template) - permission_template.available_workflows.first.update!(active: true) - Hyrax::Workflow::PermissionGenerator.call(roles: 'approving', workflow: workflow, agents: approving_user) - # Need to instantiate the Sipity::Entity for the given work. This is necessary as I'm not creating the work via the UI. - Hyrax::Workflow::WorkflowFactory.create(work, {}, depositing_user) - end - - describe 'leaving a comment for non-state changing' do - it 'will not advance the state' do - login_as(approving_user, scope: :user) - visit hyrax_generic_work_path(work) - - expect do - the_comment = 'I am leaving a great comment. A bigly comment. The best comment.' - page.choose('workflow_action[name]', option: 'leave_a_comment') - - page.within('.workflow-comments') do - page.fill_in('workflow_action_comment', with: the_comment) - page.click_on('Submit') - end - expect(page).to have_content(the_comment) - end.not_to change { Sipity::Entity(work).reload.workflow_state_name } - end - end -end