state_machines icon indicating copy to clipboard operation
state_machines copied to clipboard

Passing event arguments to guards

Open rosskevin opened this issue 9 years ago • 6 comments

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

rosskevin avatar Jan 15 '16 18:01 rosskevin

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?

cspray avatar Jun 20 '16 21:06 cspray

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.

rosskevin avatar Jun 20 '16 21:06 rosskevin

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

joegaudet avatar Nov 29 '23 21:11 joegaudet

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

seuros avatar Nov 30 '23 08:11 seuros

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

joegaudet avatar Apr 26 '24 17:04 joegaudet