trailblazer-operation icon indicating copy to clipboard operation
trailblazer-operation copied to clipboard

Define list of exposed variables in Operation Result

Open gogiel opened this issue 6 years ago • 6 comments

Similar to https://github.com/trailblazer/trailblazer/issues/161

Problem

By looking at an operation it's possible to tell what input it takes with a help of Contracts.

Same can't be said about the output. The output of the operation, Trailblazer::Operation::Result, has a Hash-like behaviour. The list of possible fields is not defined anywhere so the interface is really unclear.

The output can be documented in a comment. It would be nicer if it was defined in the code.

Solution idea

Operation is able to define custom Result types with a list of accessors/getters. When defined instance of this custom Result class is returned from the operation.

DSL idea 1 - implicit

class MyOperation < Trailblazer::Operation
  ... # steps definition
  expose :model
end

DSL idea 2 - explicit

class MyOperation < Trailblazer::Operation
  ... # steps definition
  CustomResult = Result.expose(:model)
end

Usage/behaviour:

result = MyOperation.call
result.model # getter, equals result[:model]
result.is_a?(MyOperation::CustomResult) # true
result.is_a?(Trailblazer::Operation::Result) # true, subclass

Namespaces support

I'm not a big fan of namespacing using dots in String, but it should be possible to please both approaches. expose method could accepts nested Hashes and Arrays

class MyOperation < Trailblazer::Operation
  ... # steps definition
  CustomResult = Result.expose(model: [:type, :object])
end

result = MyOperation.call
type = result.model.type

Please let me know what you think. I can take care of the implementation. I'm not sure what's the current/future vision of handling the namespaces.

gogiel avatar Sep 10 '18 18:09 gogiel

Hi, thanks for your idea, we love it.

We were just discussing and came up with an idea that leads to your suggestion: we could, in addition to your idea, allow to alias keys in the context object, eg. renaming "contract.default" to :contract.

Will be back shortly! :grimacing: :beers:

apotonick avatar Oct 24 '18 11:10 apotonick

As a shortcut for accessing some of that stuff right now I just added a couple of convinience methods into .pryrc

class Trailblazer::Operation::Result
  def to_hash
    instance_variable_get(:@data).to_hash if instance_variable_get(:@data).respond_to?(:to_hash)
  rescue StandardError
    warn 'WARNING: Not a TRB Result object?'
    nil
  end

  def trb_vars
    result = self
    result.to_hash.keys
  rescue StandardError
    warn 'WARNING: Not a TRB Result object?'
    nil
  end
  


end

Resulting in this type of output

Quote::Show.(params: {id: 'a12'}).to_hash
#=>
{
         :params => {
        :id => "a12"
    },
         :errors => [
        [0] "Quote with id a12 not found"
    ],
    :search_hash => {
        :slug_eq => "a12"
    },
          :model => nil,
         :logger => #<Logger:0x00007fe7f6ac38d0 @level=0, @progname=nil, @default_formatter=#<Logger::Formatter:0x00007fe7f6ac3880 @datetime_format=nil>, @formatter=nil, @logdev=#<Logger::LogDevice:0x00007fe7f6ac3830 @shift_period_suffix=nil, @shift_size=nil, @shift_age=nil, @filename=nil, @dev=#<IO:<STDOUT>>, @mon_owner=nil, @mon_count=0, @mon_mutex=#<Thread::Mutex:0x00007fe7f6ac3768>>>
}

konung avatar Oct 24 '18 19:10 konung

My approach is mostly just for adhoc debugging. I like @gogiel 's suggestion.

"Exposing" would also be of great use for business logic within another Operation,

konung avatar Oct 24 '18 19:10 konung

It's been frustrating for me as well, and thanks @konung for the snippet, I'm totally using it!

Though, I stick to the format that when a result succeeds, the output is always within result[:model], so my operations outputs are always expectable.

The issue is on errors, when result.success? is false, then it's hard to tell what caused it, and what key to check to get insightful information.

Was it because of result.contract.params? or result.error? or something else?

I'm not sure what changes you planned about that in 2.1, but my guess is that if there was some method that could list all the options changes that happened during a failure operation, that might be helpful, not only for debugging but for error reporting.

Something like a method result.errors that would list [:"result.error", :"result.contract.params"].

guyzmo avatar Feb 23 '19 10:02 guyzmo

@gogiel @konung @guyzmo Are you using 2.1 already? What are your thoughts about that issue and the given solution now? Please join our new zulipchat: https://trailblazer.zulipchat.com/join/od4yhihn6tra3ix39u4gks72/

@apotonick was this addressed in 2.1 and if no do you think it would be nice to add this feature?

panSarin avatar Apr 28 '20 11:04 panSarin

I already loved this issue when it came up two yrs ago! With the new endpoint gem, you'd have something like to following pseudo code:

def create
  endpoint Memo::Create, expose: [:model, :contract]

That's just an idea, but it'd definitely be handled in the endpoint layer, not on the result object (as this is very operation-specific).

apotonick avatar Apr 28 '20 12:04 apotonick