Skip to content

Commit

Permalink
Upgrade Valkyrie to 2.0
Browse files Browse the repository at this point in the history
The main impact of this is that `Dry::Validations` and `Dry::Struct` are
updated. Neither is backward compatible.

The `WorkflowSchema` is rewritten using the new syntax, and it is now slightly
more strict. It expects `notifications:` to have names that can be constantized
to a `Class`, not just to any registered constant.

`Wings::ConverterValueMapper` has to handle ids and nested structs/`Valkyrie::Resources`
slightly more aggressively, since types are more agressively coerced, and aren't
squashed into hashes when nested in a parent's `#attributes`.
  • Loading branch information
Tom Johnson authored and no-reply committed Sep 16, 2019
1 parent 0e68750 commit 5d8819f
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 53 deletions.
6 changes: 3 additions & 3 deletions app/models/hyrax/resource.rb
Expand Up @@ -6,9 +6,9 @@ module Hyrax
class Resource < Valkyrie::Resource
include Valkyrie::Resource::AccessControls

attribute :alternate_ids, ::Valkyrie::Types::Array
attribute :embargo, Hyrax::Embargo
attribute :lease, Hyrax::Lease
attribute :alternate_ids, Valkyrie::Types::Array.of(Valkyrie::Types::ID)
attribute :embargo, Hyrax::Embargo.optional
attribute :lease, Hyrax::Lease.optional

def visibility=(value)
visibility_writer.assign_access_for(visibility: value)
Expand Down
4 changes: 2 additions & 2 deletions app/services/hyrax/workflow/workflow_importer.rb
Expand Up @@ -113,7 +113,7 @@ def default_validator
end

def default_schema
Hyrax::Workflow::WorkflowSchema
Hyrax::Workflow::WorkflowSchema.new
end

def validate!
Expand Down Expand Up @@ -170,7 +170,7 @@ module SchemaValidator
def self.call(data:, schema:, logger:)
result = schema.call(data)
return true if result.success?
message = result.messages(full: true).inspect
message = result.errors(full: true).inspect
logger.error(message)
raise message
end
Expand Down
63 changes: 28 additions & 35 deletions app/services/hyrax/workflow/workflow_schema.rb
Expand Up @@ -14,48 +14,41 @@ module Workflow
# @see Sipity::Notification
# @see Sipity::Method
# @see ./lib/generators/hyrax/templates/workflow.json.erb
WorkflowSchema = Dry::Validation.Schema do
configure do
def self.messages
Dry::Validation::Messages.default.merge(
en: { errors: { constant_name?: 'must be an initialized Ruby constant' } }
)
end
class WorkflowSchema < Dry::Validation::Contract
class Types
include Dry::Types()

def constant_name?(value)
value.constantize
true
rescue NameError
false
Constant = Types::Class.constructor do |v|
begin
v.constantize
rescue NameError => _err
v
end
end
end

required(:workflows).each do
required(:name).filled(:str?) # Sipity::Workflow#name
optional(:label).filled(:str?) # Sipity::Workflow#label
optional(:description).filled(:str?) # Sipity::Workflow#description
optional(:allows_access_grant).filled(:bool?) # Sipity::Workflow#allows_access_grant?
required(:actions).each do
schema do
required(:name).filled(:str?) # Sipity::WorkflowAction#name
required(:from_states).each do
schema do
required(:names) { array? { each(:str?) } } # Sipity::WorkflowState#name
required(:roles) { array? { each(:str?) } } # Sipity::Role#name
end
params do
required(:workflows).array(:hash) do
required(:name).filled(:string) # Sipity::Workflow#name
optional(:label).filled(:string) # Sipity::Workflow#label
optional(:description).filled(:string) # Sipity::Workflow#description
optional(:allows_access_grant).filled(:bool) # Sipity::Workflow#allows_access_grant?
required(:actions).array(:hash) do
required(:name).filled(:string) # Sipity::WorkflowAction#name
required(:from_states).array(:hash) do
required(:names) { array? { each(:string) } } # Sipity::WorkflowState#name
required(:roles) { array? { each(:string) } } # Sipity::Role#name
end
optional(:transition_to).filled(:str?) # Sipity::WorkflowState#name
optional(:notifications).each do
schema do
required(:name).value(:constant_name?) # Sipity::Notification#name
required(:notification_type).value(included_in?: Sipity::Notification.valid_notification_types)
required(:to) { array? { each(:str?) } }
optional(:cc) { array? { each(:str?) } }
optional(:bcc) { array? { each(:str?) } }
end
optional(:transition_to).filled(:string) # Sipity::WorkflowState#name
optional(:notifications).array(:hash) do
required(:name).value(Types::Constant) # Sipity::Notification#name
required(:notification_type).value(included_in?: Sipity::Notification.valid_notification_types)
required(:to) { array? { each(:string) } }
optional(:cc) { array? { each(:string) } }
optional(:bcc) { array? { each(:string) } }
end
optional(:methods) { array? { each(:str?) } } # See it_behaves_like "a Hyrax workflow method"
end
optional(:methods) { array? { each(:string) } } # See it_behaves_like "a Hyrax workflow method"
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions hyrax.gemspec
Expand Up @@ -41,9 +41,9 @@ SUMMARY
spec.add_dependency 'carrierwave', '~> 1.0'
spec.add_dependency 'clipboard-rails', '~> 1.5'
spec.add_dependency 'dry-equalizer', '~> 0.2'
spec.add_dependency 'dry-struct', '~> 0.4'
spec.add_dependency 'dry-struct', '~> 1.0'
spec.add_dependency 'dry-transaction', '~> 0.11'
spec.add_dependency 'dry-validation', '~> 0.9'
spec.add_dependency 'dry-validation', '~> 1.3'
spec.add_dependency 'flipflop', '~> 2.3'
# Pin more tightly because 0.x gems are potentially unstable
spec.add_dependency 'flot-rails', '~> 0.0.6'
Expand Down Expand Up @@ -77,7 +77,7 @@ SUMMARY
spec.add_dependency 'select2-rails', '~> 3.5'
spec.add_dependency 'signet'
spec.add_dependency 'tinymce-rails', '~> 4.1'
spec.add_dependency 'valkyrie', '~> 1.6'
spec.add_dependency 'valkyrie', '~> 2.0'

# temporary pin to 2.17 due to failures caused in 2.18.0
spec.add_development_dependency "capybara", '~> 2.4', '< 2.18.0'
Expand Down
40 changes: 37 additions & 3 deletions lib/wings/converter_value_mapper.rb
Expand Up @@ -23,6 +23,22 @@ def result
end
end

class IdValueMapper < ::Valkyrie::ValueMapper
ConverterValueMapper.register(self)

def self.handles?(value)
value.last.is_a? ::Valkyrie::ID
end

def result
[value.first, values]
end

def values
value.last.id
end
end

class NestedLeaseValue < ::Valkyrie::ValueMapper
ConverterValueMapper.register(self)

Expand All @@ -40,7 +56,7 @@ def result
class NestedResourceArrayValue < ::Valkyrie::ValueMapper
ConverterValueMapper.register(self)
def self.handles?(value)
value.last.is_a?(Array) && value.last.map { |x| x.try(:class) }.include?(Hash)
value.last.is_a?(Array) && value.last.any? { |x| x.is_a? Dry::Struct }
end

def result
Expand All @@ -54,14 +70,32 @@ def values
end
end

class ArrayValue < ::Valkyrie::ValueMapper
ConverterValueMapper.register(self)

def self.handles?(value)
value.last.is_a?(Array)
end

def result
[value.first, values]
end

def values
value.last.map do |val|
calling_mapper.for([value.first, val]).result
end.flat_map(&:last)
end
end

class NestedResourceValue < ::Valkyrie::ValueMapper
ConverterValueMapper.register(self)
def self.handles?(value)
value.last.is_a?(Hash)
value.last.is_a?(Dry::Struct)
end

def result
attrs = ActiveFedoraAttributes.new(value.last).result
attrs = ActiveFedoraAttributes.new(value.last.attributes).result
attrs.delete(:read_groups)
attrs.delete(:read_users)
attrs.delete(:edit_groups)
Expand Down
16 changes: 9 additions & 7 deletions spec/services/hyrax/workflow/workflow_schema_spec.rb
@@ -1,6 +1,8 @@
module Hyrax
module Workflow
RSpec.describe 'WorkflowSchema' do
RSpec.describe WorkflowSchema do
subject(:schema) { described_class.new }

let(:valid_data) do
{
workflows: [
Expand Down Expand Up @@ -51,20 +53,20 @@ class ::DoTheThing
Object.send(:remove_const, :DoTheThing)
end

it 'validates valid data by returning an empty message' do
expect(WorkflowSchema.call(valid_data).messages).to be_empty
it 'validates valid data by returning an empty error set' do
expect(schema.call(valid_data).errors).to be_empty
end

it 'reports invalid data via the returned messages' do
expect(WorkflowSchema.call(invalid_data).messages).not_to be_empty
expect(schema.call(invalid_data).errors).not_to be_empty
end

describe 'notification names' do
context 'with an uninitialized constant' do
let(:notification_name) { 'FooBar' }

it 'is invalid' do
expect(WorkflowSchema.call(valid_data).messages).not_to be_empty
expect(schema.call(valid_data).errors).not_to be_empty
end
end

Expand All @@ -76,8 +78,8 @@ class DoTheThing
after { Hyrax::Workflow.send(:remove_const, :DoTheThing) }
let(:notification_name) { 'Hyrax::Workflow::DoTheThing' }

it 'returns an empty message because valid' do
expect(WorkflowSchema.call(valid_data).messages).to be_empty
it 'returns an empty error set because valid' do
expect(schema.call(valid_data).errors).to be_empty
end
end
end
Expand Down

0 comments on commit 5d8819f

Please sign in to comment.