Skip to content

Commit

Permalink
♻️ Extract Action Logic for improved composibility (#6644)
Browse files Browse the repository at this point in the history
Prior to this commit, there was a precarity regarding the extensibility
of the create action for valkyrie objects.

The create method had several interacting objects, which made downstream
composibility harder to implement (without completely overriding the
action's behavior).  Downstream could:

1. Provide a different container for transactions
2. Inject additional steps into the transaction container, and those
   steps could require step args.
3. Remove/replace one of the transactions that has a step arg (which
   would then raise an error).

In all of these cases, the size of the original method makes for harder
downstream extensibility.

With this commit, I'm extracting an action object, putting forward a
minimum amount of work so we can discuss the approach.

This should be considered a noop change as I'm moving where the logic
happens.

If we deem that this is a good approach, I'd envision extracting some of
the work controller behavior specs into the given action.  I would also
anticipate extracting the update and delete actions.

Co-authored-by: Daniel Pierce <[email protected]>
  • Loading branch information
jeremyf and dlpierce authored Jan 30, 2024
1 parent a87d4ad commit 206cb5f
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 13 deletions.
25 changes: 12 additions & 13 deletions app/controllers/concerns/hyrax/works_controller_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ module WorksControllerBehavior

class_attribute :_curation_concern_type, :show_presenter, :work_form_service, :search_builder_class
class_attribute :iiif_manifest_builder, instance_accessor: false
class_attribute :create_valkyrie_work_action

self.show_presenter = Hyrax::WorkShowPresenter
self.work_form_service = Hyrax::WorkFormService
self.search_builder_class = WorkSearchBuilder
self.create_valkyrie_work_action = Hyrax::Action::CreateValkyrieWork
self.iiif_manifest_builder = nil
attr_accessor :curation_concern
helper_method :curation_concern, :contextual_path
Expand Down Expand Up @@ -187,20 +190,16 @@ def actor
# rubocop:disable Metrics/MethodLength
def create_valkyrie_work
form = build_form
# fallback to an empty hash to avoid: # NoMethodError: undefined method `has_key?` for nil:NilClass
original_input_params_for_form = params[hash_key_for_curation_concern] ? params[hash_key_for_curation_concern] : {}
return after_create_error(form_err_msg(form), original_input_params_for_form) unless form.validate(original_input_params_for_form)
action = create_valkyrie_work_action.new(form: form,
transactions: transactions,
user: current_user,
params: params,
work_attributes_key: hash_key_for_curation_concern)

return after_create_error(form_err_msg(action.form), action.work_attributes) unless action.validate

result = action.perform

result =
transactions['change_set.create_work']
.with_step_args(
'work_resource.add_to_parent' => { parent_id: params[:parent_id], user: current_user },
'work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: original_input_params_for_form[:file_set] },
'change_set.set_user_as_depositor' => { user: current_user },
'work_resource.change_depositor' => { user: ::User.find_by_user_key(form.on_behalf_of) },
'work_resource.save_acl' => { permissions_params: form.input_params["permissions"] }
)
.call(form)
@curation_concern = result.value_or { return after_create_error(transaction_err_msg(result)) }
after_create_response
end
Expand Down
80 changes: 80 additions & 0 deletions app/services/hyrax/action/create_valkyrie_work.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# frozen_string_literal: true

module Hyrax
module Action
##
# @since 5.0.0
# @api public
#
# Encapsulating the logic of several interacting objects. Namely the idea of using a form to
# {#validate} given parameters then to {#perform} the action by leveraging the {#transactions}'s
# {#transaction_name} configured with appropriate {#step_args} and ultimately providing the
# {#form} to then perform that work.
class CreateValkyrieWork
##
# @!group Class Attributes
#
# @!attribute transaction_name
# @return [String] the name of the transaction that we call for the CreateValkyrieWorkAction.
class_attribute :transaction_name, default: 'change_set.create_work'
# @!endgroup Class Attributes
##

##
# @param form [Object] the form that we'll use for validation and performing the action.
# @param transactions [Hyrax::Transactions::Container] the transactions
# @param user [User] the person performing the action
# @param params [Hash] the contextual parameters for the action; ApplicationController#params
# if you will.
# @param work_attribute_key [String] the name of the key within the params that contains
# the work's attributes
def initialize(form:, transactions:, user:, params:, work_attributes_key:)
@form = form
@transactions = transactions
@user = user
@params = params
@work_attributes_key = work_attributes_key
@work_attributes = @params.fetch(work_attributes_key, {})
end

attr_reader :form, :transactions, :user, :parent_id, :work_attributes, :uploaded_files, :params, :work_attributes_key

##
# @api public
# @return [TrueClass] when the object is valid.
# @return [FalseClass] when the object is valid.
def validate
form.validate(work_attributes)
end

##
# @api public
# @return [#value_or] the resulting created work, when successful. When not successful, the
# returned value call the given block.
def perform
transactions[transaction_name].with_step_args(**step_args).call(form)
end

##
# @api public
#
# @return [Hash<String,Hash>] the step args to use for the given {#transactions}'s
# {.transaction_name}
def step_args
{
'work_resource.add_to_parent' => { parent_id: params[:parent_id], user: user },
'work_resource.add_file_sets' => { uploaded_files: uploaded_files, file_set_params: work_attributes[:file_set] },
'change_set.set_user_as_depositor' => { user: user },
'work_resource.change_depositor' => { user: ::User.find_by_user_key(form.on_behalf_of) },
'work_resource.save_acl' => { permissions_params: form.input_params["permissions"] }
}
end

# rubocop:disable Lint/DuplicateMethods
def uploaded_files
UploadedFile.find(params.fetch(:uploaded_files, []))
end
# rubocop:enable Lint/DuplicateMethods
end
end
end

0 comments on commit 206cb5f

Please sign in to comment.