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

Passing event arguments to guards #39

Open rosskevin opened 8 years ago

rosskevin commented 8 years ago

TLDR;

Currently, arguments passed to an event are not passed to the guard, only the model object. I'm wondering if there is a reasoning behind this (i.e. is it poor practice), or if this is something that I could PR?

Rationale

For our subscription model, currently, we have two events start_trial and start_active. We just found out we need start_future_active and the nomenclature was a dead giveaway that we instead need one start event with not necessarily deterministic paths through the state machine.

I say not necessarily because there is one deterministic case where we want to start but we want to force it to skip the trial (trial is the default start behavior). In looking at this, I've found that a guard cannot inspect the arguments passed to an event, and that seems like exactly what we need. For example:

subscription.start(true) # skip trial plain arg

Details

Relevant existing code

event.rb

    def fire(object, *args)
      machine.reset(object)

      if transition = transition_for(object) # guards are checked here
        transition.perform(*args)            # args are passed here after guard check for execution
      else
        on_failure(object)
        false
      end
    end

Existing implementation that needs refactored

event :start_trial do
  transition :uninitialized => :trial, if: :trial_enabled?
end

event :start_active do
  transition [:uninitialized, :trial, :trial_expired, :system_grace, :grace] => :active
end

# now we need start_future - this needs to be refactored

With PR proposal 1

This would appear to be simple to implement based on the code involved:

class Subscription
  state_machine :state, initial: :uninitialized do

    # plain arg example (proposal #1)
    event :start do
      transition :uninitialized => :trial, if: ->(subscription, args) { subscription.trial_enabled? && args.length > 0 && args[0] != true }
      transition [:uninitialized, :trial, :trial_expired, :system_grace, :grace] => :active
      transition :uninitialized => :future_active, if: future_dated?
    end
  end
end

With PR proposal 2

This would require more significant changes, but still not too much. The transition would be initialized first then passed to the guard:

class Subscription
  state_machine :state, initial: :uninitialized do

    # transition example (proposal #2)
    event :start do
      transition :uninitialized => :trial, if: ->(subscription, transition) { subscription.trial_enabled? && transition.args[0] != true }
      transition [:uninitialized, :trial, :trial_expired, :system_grace, :grace] => :active
      transition :uninitialized => :future_active, if: future_dated?
    end
  end
end

Conclusion

@seuros - do you think this is a good addition? Is there an argument against allowing this?

/cc @bmcdaniel11

cspray commented 8 years ago

I also have some use cases where it would be helpful if the guard received arguments passed to the event. Right now I have a work around but it isn't very clean. From my perspective and use case Proposal 1 seems like a winner.

Is this something that is still on the table?

rosskevin commented 8 years ago

Definitely, if you want to PR it, we will review it. The codebase is quite stable so it will need adequate tests to accompany any change.

joegaudet commented 10 months ago

I take this is dead in the water as of now, would love to have this capability.

seuros commented 10 months ago

@joegaudet do you want to contribute to it ? I think with the kwarg release, the implementation is simpler now.

joegaudet commented 5 months ago

If you have some guidance as to how to do it, could consider it.

maedi commented 3 months ago

This would be a great feature, much needed, but here's a workaround for now:

Vehicle
  def initialize
    @condition = nil
    @argument = nil
  end

  state_machine :state, initial: :start do
    event :stop do
      transition start: :stopped, if: :stoppable?
    end
    after_transition to: :stopped do |vehicle|
      vehicle.handle_stop_event
    end
  end

  def update(event, condition, argument)
    @condition = condition
    @argument = argument
    fire_state_event(event)
  end

  def stoppable?
    @condition
  end

  def handle_stop_event
    do_something(@argument)
  end
end

Then call it like:

vehicle = Vehicle.new
vehicle.update(:stop, true, 'my argument')