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

Passing event arguments to guards #39

Open
rosskevin opened this issue Jan 15, 2016 · 6 comments
Open

Passing event arguments to guards #39

rosskevin opened this issue Jan 15, 2016 · 6 comments

Comments

@rosskevin
Copy link
Member

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

cspray commented Jun 20, 2016

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

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

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

@seuros
Copy link
Member

seuros commented Nov 30, 2023

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

@joegaudet
Copy link

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

@maedi
Copy link

maedi commented Jun 28, 2024

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

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