Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define list of exposed variables in Operation Result #28

Open
gogiel opened this issue Sep 10, 2018 · 6 comments
Open

Define list of exposed variables in Operation Result #28

gogiel opened this issue Sep 10, 2018 · 6 comments

Comments

@gogiel
Copy link

gogiel commented Sep 10, 2018

Similar to trailblazer/trailblazer#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.

@apotonick
Copy link
Member

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! 😬 🍻

@konung
Copy link

konung commented Oct 24, 2018

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
Copy link

konung commented Oct 24, 2018

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,

@guyzmo
Copy link

guyzmo commented Feb 23, 2019

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"].

@panSarin
Copy link
Collaborator

panSarin commented Apr 28, 2020

@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?

@apotonick
Copy link
Member

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants