alexpeachey / end_state

A State Machine implementation
MIT License
9 stars 5 forks source link

Add failure_message support for blocked transitions #3

Closed brianvh closed 10 years ago

brianvh commented 10 years ago

While I'm doing the build out in Versus, I've found a couple of places where it would be helpful if I could specify a failure_message addition, when a transition attempt is blocked.

I'm thinking about something along the lines of:

  transition 'reserved' => 'empty', as: :release do |t|
    t.blocked "You cannot release that invitation."
    t.persistence_on
  end

And the text send to #blocked would be added to the failure_messages array if the transition is blocked.

I'm happy to submit the PR for the change, if you're OK with it. That way I can actually claim to be working on the gem, with you. :smiley:

alexpeachey commented 10 years ago

The current way to do this is to put it in the failed method of the guard. I can see though how you might want a message if ANY of the guards failed, not just a specific one. I'm cool with more helpers. Feel free to put together a PR and let's see how it works.

brianvh commented 10 years ago

Actually, I'm talking about the case where an invalid transition is attempted. In the example above, if the invitation slot was in the 'empty' state, :release isn't even possible, so no guards would be called.

StateMachine simply fails out with a false return, in the #block_transition method (line 65). I want to give that method the ability to place a message into the failed_messages array.

alexpeachey commented 10 years ago

Oh right, duh, that makes sense. It didn't occur to me that someone would transition when they couldn't. So yeah, excellent feature. I look forward to your PR. :sunglasses:

alexpeachey commented 10 years ago

Actually...I think there is going to be a sticky issue. Where to store this message so it's around when it is needed. If you put it on the transition, it doesn't work because the transition is from a state to another state. But if you are trying to make an illegal transition it is because it doesn't exist, so how do you know which transition to grab the message from?

alexpeachey commented 10 years ago

Since multiple transitions can exist to the same end state:

transition a: :d
transition b: :d

If you are in state :c, and try and transition to :d, what message to you grab?

I'm guessing this has to be more generic and goes on the machine itself, not a specific transition.

brianvh commented 10 years ago

Hmm... that's a good point. Since I'm more or less forced to use the :as flag on all of my transitions (that's another discussion/issue :wink: ), I think I was focused on attaching the message to the event represented by :as.

But that only works if every transition has an event name...

alexpeachey commented 10 years ago

In that case I suppose it's possible but it's an odd case, where if we allow blocked but only if you use as, then do we just ignore it if you use blocked without as? Or do we blow up? Or what?

But it would be possible that when unravelling the transition definition, if there was an as, and you specified a blocked it associates the message with the alias, and if you tried to use the as action illegally it could use the message.

The larger problem is the as was initially added just as semantics, so if you wanted to refer to a transition as a specific action verb you could, but it was just sugar. There is no enforcement other than a transition has to exist.

The mechanics when using an action defined by as is it looks up the action and finds the associated end state. It then just uses that end state and operates as usual.

So technically you can do:

transition a: :c, as: go
transition b: :c

And this is great for your semantics and when you want to go from a to c, it might have some special meaning why you want to call it go, and if you are in state a and do machine.go you will end up in c, but if you are in state b and do machine.go you will also end up in c.

So it sounds like what we want is this to be less sugary and have an enforcement component. So in the above example, machine.go when in state b would fail.

So further it sounds almost what we want is another base level feature.

So you can have transitions that operate exactly as now, but you can also have actions.

So you can say

transition a: :b
transition b: :c
action :jump, a: :c

This would allow you to go from a -> b and b -> c and a -> c, but if you are in a and say jump, you go to c, but if you are in b and say jump, you stay in b.

This further allows us to have the action block understand the blocked option and give you what you are looking for.

I'm not convinced we want to go down that path but it's something to consider.

brianvh commented 10 years ago

Ok... lets table this change, for now. Currently, I've only found one real case where this would be helpful. And only from an encapsulation standpoint, which isn't really a big issue, since we're talking about a failure that's clearly expressed outside the StateMachine.

I do think there's a bit more meat on the action/event front, but I don't have a fully formed opinion on it, just yet.

alexpeachey commented 10 years ago

So thinking about this further, I think it is useful and I think the solution is to get rid of as.

The problem is as only makes sense for single transitions. However, as it stands right now you can say transition [:a, :b] => :c, as: :go but that shouldn't be legal. That's saying create 2 transitions, :a => :c and :b => :c and alias them both as :go which doesn't make any sense.

So I think the solution is to ditch the as that was just added and instead you define out all of your transitions. Then you can optionally define events or actions that are only legal for a specific transition. That can then take a block where we can put options. The first and only right now would be the option for blocked as you suggested.

So then it might look like this:

transition [:a, :b] => :c
transition a: :b
transition c: :d

event :go, a: :b
event :jump, a: :c do |e|
  e.blocked 'You cannot currently jump to :c' 
end

The event method would use the first arg as the name and the second arg as the transition which will be verified to be a hash with a single key/value and verify it to be a defined transition.

So think on that a bit before we go crazy deprecating as and adding this in.

alexpeachey commented 10 years ago

Resolved in v0.3.0 using the existing as syntax with support for blocking invalid events.