Skip to content

Commit

Permalink
Deprecate PowerConverter based to_sipity_action
Browse files Browse the repository at this point in the history
`PowerConverter` is a community-maintained gem, but is not in the Samvera
namespace. Depending on it under these circumstances is not in keeping with our
policy.

Rather than simply inline the `PowerConverter` class, this proposes pushing in
the direction a more generic ruby pattern that inspired
it. http://devblog.avdi.org/2012/05/07/a-ruby-conversion-idiom/
  • Loading branch information
Tom Johnson committed Oct 18, 2019
1 parent 220a9be commit 2258dc2
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 5 deletions.
1 change: 1 addition & 0 deletions app/conversions/power_converters/sipity_action.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
PowerConverter.define_conversion_for(:sipity_action) do |input, scope|
Deprecation.warn('PowerConverter is deprecated. Use `Sipity::WorkflowAction(input, workflow)` instead')
workflow_id = PowerConverter.convert_to_sipity_workflow_id(scope)
case input
when Sipity::WorkflowAction
Expand Down
8 changes: 7 additions & 1 deletion app/forms/hyrax/forms/workflow_action_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,15 @@ def authorized_for_processing

private

class << self
def workflow_action_for(name, scope:)
Sipity::WorkflowAction(name, scope) { nil }
end
end

def convert_to_sipity_objects!
@subject = WorkflowActionInfo.new(work, current_user)
@sipity_workflow_action = PowerConverter.convert_to_sipity_action(name, scope: subject.entity.workflow) { nil }
@sipity_workflow_action = self.class.workflow_action_for(name, scope: subject.entity.workflow)
end

attr_reader :subject, :sipity_workflow_action
Expand Down
33 changes: 33 additions & 0 deletions app/models/sipity.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# frozen_string_literal: true

# rubocop:disable Style/RaiseArgs
module Sipity
##
# Cast an object to a WorkflowAction in a given workflow
def WorkflowAction(input, workflow) # rubocop:disable Naming/MethodName
workflow_id = PowerConverter.convert_to_sipity_workflow_id(workflow)

result = case input
when WorkflowAction
input if input.workflow_id == workflow_id
when String, Symbol
WorkflowAction.find_by(workflow_id: workflow_id, name: input.to_s)
end

result ||= input.try(:to_sipity_action)
return result unless result.nil?
return yield if block_given?

raise ConversionError.new(input)
end
module_function :WorkflowAction

# rubocop:enable Style/RaiseArgs
class ConversionError < PowerConverter::ConversionError
def initialize(value, **options)
options[:scope] ||= nil
options[:to] ||= nil
super(value, options)
end
end
end
2 changes: 1 addition & 1 deletion app/services/hyrax/workflow/notification_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def assign_scope_and_reason_to(notification:)
attr_reader :scope

def assign_scope!
@scope = PowerConverter.convert_to_sipity_action(notification_configuration.scope, scope: workflow)
@scope = Sipity::WorkflowAction(notification_configuration.scope, workflow)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/forms/hyrax/forms/workflow_action_form_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

context 'if the given user cannot perform the given action' do
before do
allow(PowerConverter).to receive(:convert_to_sipity_action).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?).and_return(false)
end

Expand Down Expand Up @@ -53,7 +53,7 @@

context 'if the given user can perform the given action' do
before do
allow(PowerConverter).to receive(:convert_to_sipity_action).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?)
.and_return(true)
end
Expand Down
43 changes: 43 additions & 0 deletions spec/models/sipity_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
RSpec.describe Sipity do
describe '.WorkflowAction' do
let(:workflow_id) { 1 }
let(:action) { Sipity::WorkflowAction.new(id: 4, name: 'show', workflow_id: workflow_id) }

context "with workflow_id and action's workflow_id matching" do
it 'will return the object if it is a Sipity::WorkflowAction' do
expect(described_class.WorkflowAction(action, workflow_id)).to eq(action)
end

it 'will return the object if it responds to #to_sipity_action' do
object = double(to_sipity_action: action)
expect(described_class.WorkflowAction(object, workflow_id)).to eq(action)
end

it 'will raise an error if it cannot convert the object' do
object = double
expect { described_class.WorkflowAction(object, workflow_id) }
.to raise_error(Sipity::ConversionError)
end

it 'will use a found action based on the given string and workflow_id' do
expect(Sipity::WorkflowAction).to receive(:find_by).and_return(action)
expect(described_class.WorkflowAction(action.name, workflow_id)).to eq(action)
end

context "when the WorkflowAction can not be found" do
it 'will raise an error' do
expect(Sipity::WorkflowAction).to receive(:find_by).and_return(nil)
expect { described_class.WorkflowAction(action.name, workflow_id) }
.to raise_error(Sipity::ConversionError)
end
end
end

context "with mismatching workflow_id and action's workflow_id" do
it "will fail an error if the scope's workflow_id is different than the actions" do
expect { described_class.WorkflowAction(action, workflow_id + 1) }
.to raise_error(Sipity::ConversionError)
end
end
end
end
2 changes: 1 addition & 1 deletion spec/services/hyrax/workflow/permission_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module Workflow
before { Hyrax::Workflow::WorkflowImporter.generate_from_hash(data: workflow_config, permission_template: sipity_workflow.permission_template) }

def expect_actions_for(user:, entity:, actions:)
actions = Array.wrap(actions).map { |action| PowerConverter.convert_to_sipity_action(action, scope: entity.workflow) }
actions = Array.wrap(actions).map { |action| Sipity::WorkflowAction(action, entity.workflow) }
expect(described_class.scope_permitted_workflow_actions_available_for_current_state(user: user, entity: entity)).to eq(actions)
end

Expand Down

0 comments on commit 2258dc2

Please sign in to comment.