Using Actors directly as form objects / make errors accessible in a ActiveModel::Validations way

[stex]

Hey @sunny!

First of all, thanks a lot for this gem, it makes structuring a lot of operations in my applications a lot easier and testable :)

In an application I'm currently working on, I'm using actors quite often as direct recipients of form parameters, meaning the form fields map directly to inputs on the actor.

Until today, I was usually passing in an instance of the model that the actor would create to the form and map its attributes to the actor inputs (and errors back to the record), but this got my controllers a bit messy over time.

What I did now is a bit of monkey patching to make actors valid objects to be used in forms, including ActiveModel::Naming and errors support, so the following is possible:

class PostsController < ApplicationController
  def new
    @actor = CreatePost.new
  end

  def create
    @actor = CreatePost.result(params.require(:create_post).permit_attributes(...))
  end
end

# posts/new.html.erb
<%= form_with model: @actor, as: :create_post, url: [:posts] do |f| %>
  <%= f.errors.full_messages %>
<% end %>

I had to create an individual result class per actor dynamically as I needed to control its name for i18n and make sure to pass down errors from Checkable into ActiveModel::Errors.

Do you think this would be a feature that's worth implementing properly, e.g. in the rails extension for this gem (or a new one)?

[sunny]

Hi @stex! I am glad this gem helps you as much as it helped me :)

That is an interesting approach! Having them act as form models directly sounds like a really good feature, as they often sit as proxies to form actions.

If this doesn’t add constraints or too strong tradeoffs, this is definitely something that could fit inside the rails extension. I’d be happy to help review and maintain this feature if you’d like to try a PR on actor-rails \o/

[stex]

Thanks a lot for your response! I'll create a PR when I have a bit of free time 😃

I think that the main gem will need a few (non-breaking) changes though, otherwise I'd essentially be monkey-patching again.

If it's ok for you, I'd give a quick explanation of what I've done regarding this in my app and how I would probably do it properly (though I haven't completely though this through).
If you have any ideas regarding these changes, it would help a lot! Especially if they are ok for you in terms of updating the base gem. Of course I'll remove all ActiveSupport or other helpers I might have used here for the base gem to not add dependencies.

1. I needed to be able to set an own Result class

In case a form submit was not valid, MyActor.result() is usually passed back as model for a re-display. This means that the form will get an actual instance of MyActor the first time and an instance of ServiceActor::Result the second time.

Both should behave the same way though regarding all the ActiveModel::Model API like human_attribute_name or errors.

The class methods were complicated as we don't have access to the original actor that caused the result. Therefore, I decided to make result_class an accessor, similar to the error class:

module ServiceActor::Core::ClassMethods
  def result_class = ServiceActor::Result

  def call(result = nil, **arguments)
    result = result_class.to_result(result).merge!(arguments)

    instance = new(result)
    instance._call

    result
  end
end

In my own base class for actors, I then dynamically subclassed it:

  def self.result_class
    actor_class = self

    @result_class ||= Class.new(ServiceActor::Result) do
      include ActiveModel::Conversion
      extend ActiveModel::Naming

      def read_attribute_for_validation(attr)
        try(attr) || attr
      end

      define_singleton_method :name do
        "#{actor_class}::Result"
      end

      define_singleton_method :human_attribute_name do |attr, options = {}|
        I18n.t(attr, scope: "actors.attributes.#{actor_class.name.underscore}")
      end

      def self.lookup_ancestors
        [self]
      end
    end
  end

This shouldn't break checks like .is_a?(ServiceActor::Result), but will of course create quite a few dynamic classes.

2. I needed to be able to pass down more error information than just a message

If the actor should behave like a record, it needs an errors object that maps its "attributes" to error details.
To do so, I had to override the error generation and raising methods:

class ServiceActor::Checks::Base
  def add_argument_error(message, **arguments)
    message = message.call(**arguments) if message.is_a?(Proc)

    argument_errors.push({message:, **arguments})
  end
end

module ServiceActor::Checkable::PrependedMethods
  private

  def raise_actor_argument_errors
    return if service_actor_argument_errors.empty?

    self.errors ||= ActiveModel::Errors.new(self)

    service_actor_argument_errors.each do |arguments|
      {input_key: nil, message: nil, check_name: nil, **arguments.symbolize_keys} => {input_key:, message:, check_name:}

      type = message.is_a?(Symbol) ? message : (check_name || message)

      errors.add(input_key, type)
    end

    raise self.class.argument_error_class,
      service_actor_argument_errors.first[:message]
  end
end

What I'd love to do instead is add various callbacks to the original gem, e.g. before_check, after_check, etc. This way, it's much easier to hook into the various parts of them without having to change it in the future.

This would also get rid of a few other monkey-patches I made in the past regarding error messages from Defaultable that I wasn't able to parse otherwise...

---

Would (for now) these two changes be ok for you in the main gem?

1. Ability to set an own Result class for Actors
2. Callbacks around various Checkable methods

[sunny]

I think we could get around the fact that we need to set the result class… but I have no idea how yet 😅 There are also other use cases to think of, such as actors calling other actors by passing their result set directly SubmitThing.call(CreateThing.call(…)). Although not documented (or tested) this is currently possible and could mess this up.

Perhaps adding callbacks around the various checkables would be a good start as these will most probably be usefull in other situations as well and help make the code more extensible.