state-machines / state_machines

Adds support for creating state machines for attributes on any Ruby class
https://github.com/state-machines/state_machines
MIT License
814 stars 91 forks source link

Single boolean argument to event method is not passed as transition.args to after_transition method #12

Closed rosskevin closed 9 years ago

rosskevin commented 9 years ago
state_machine :state, initial: :uninitialized do
     event :cancel do
        transition [:trial, :active, :grace] => :canceled
      end

      after_transition on: :cancel, do: :process_cancel
end

def process_cancel(transition)
    terminate = transition.args[0] || false
end
  1. subscription.cancel!(true) yields transition.args empty
  2. subscription.cancel!(true, 'foo') yields transition.args length 2 # [true, 'foo']
  3. subscription.cancel!('foo') yields transition.args length 1 # ['foo']
rosskevin commented 9 years ago

This indeed seems to be a bug, but I think I'm going to switch to passing an options hash to the event methods, that way it is not order dependent.

damncabbage commented 9 years ago

Confirmed; pass it a boolean on the tail end of an arguments list and it gets eaten, eg. (true, 0, false) => (true, 0), (99, "abc", true) => (99, "abc"), etc.

damncabbage commented 9 years ago

I think this is unfortunately a works-as-designed:

From http://www.rubydoc.info/github/pluginaweek/state_machine/master/StateMachine/Machine:event:

park(..., run_action = true) - Fires the “park” event, transitioning from the current state to the next valid state. If the last argument is a boolean, it will control whether the machine's action gets run.

(It's the .cancel() itself that's swallowing the boolean.)

rosskevin commented 9 years ago

That feels...at least unexpected. Not hard to workaround, just really unexpected.

damncabbage commented 9 years ago

Agreed. I've personally tripped over it at least once myself; it's like you need to always put your args in a tuple or nested array to avoid the last-arg tag-check thing.