geekq / workflow

Ruby finite-state-machine-inspired API for modeling workflow
MIT License
1.75k stars 207 forks source link

Documentation Inconsistent with Behavior #194

Closed kingdonb closed 4 years ago

kingdonb commented 7 years ago

I was reading the documentation under Accessing your workflow specification

Example 1 shows:

article2.current_state.events # lists possible events from here
article2.current_state.events[:reject].transitions_to # => :rejected

I tried this with my workflow object and I had to modify it slightly to get it to work. The output I got is below:

> p.current_state.events
=> {:return=>[#<Workflow::Event:0x007fe259bb2b80 @action=nil, @condition=nil, @meta={}, @name=:return, @transitions_to=:returned_from_hr>],
 :submit_to_payroll=>
  [#<Workflow::Event:0x007fe259bb2f18 @action=nil, @condition=nil, @meta={}, @name=:submit_to_payroll, @transitions_to=:locked_payroll>]}
> p.current_state.events[:submit_to_payroll].transitions_to
NoMethodError: undefined method `transitions_to' for #<Array:0x007fe259bb3238>
from (pry):10:in `block in <top (required)>'

> p.current_state.events[:submit_to_payroll]
=> [#<Workflow::Event:0x007fe259bb2f18 @action=nil, @condition=nil, @meta={}, @name=:submit_to_payroll, @transitions_to=:locked_payroll>]
> p.current_state.events[:submit_to_payroll][0]
=> #<Workflow::Event:0x007fe259bb2f18 @action=nil, @condition=nil, @meta={}, @name=:submit_to_payroll, @transitions_to=:locked_payroll>
> p.current_state.events[:submit_to_payroll][0].transitions_to
=> :locked_payroll

I couldn't think of any good reason why events[:key] should return an array. An event can only cause a transition to a single state, right? So I think this is actually a bug in the software, not in the documentation. If there is a reason I did not see, I'm all ears.

Could you weigh in on whether you returned an array here on purpose, or not?

geekq commented 4 years ago

Since "Conditional transitions" feature, there can be multiple event definitions with the same name for the same state like in https://github.com/geekq/workflow#conditional-event-transitions.

Will update the example and will ensure only working example code goes to the readme. Already started switching README to asciidoctor for that.

kingdonb commented 4 years ago

That's a good reason, thank you for weighing in. Makes sense to me now. (Closing)