praekelt/vumi-jssandbox-toolkit — pull request #206 by justinvdm (2014-09-16 10:38:11+00:00)
Title: State enter events are fired twice

State switches (and thus state creations) happen in two cases:
(a) a state is created so we can reply the user with its content
(b) a state is created to process the user's input

Since each state is created twice, we have checks to see whether the creation was because of (a) or (b), and fire an enter event if it was (a).

Problem is the checks, are broken. At the moment, state switching goes something like:

```
if im already on target_state:
return

create target_state
emit exit event on current_state

if im not already on target_state: # <-- problem is here
emit enter event

change user data to put them on target_state
current_state = target_state
```

I'm surprised that that problem wasn't revealed in tests, but I think I actually meant to check if the user is on the target state already, not the interaction machine (since we already check that higher up). Still, checking if the user is on the target state already is not enough, that check will always true, since states tell the interaction machine which target state to go to next by modifying the user's current state (which happens before `switch_state` is reached).

One fix would be to change states to tell the interaction machine what state to go to next instead of changing the users state (and leaving that up to the interaction machine). That will allow the interaction machine to figure out if (a) or (b) happened.

A better, longer term fix would be to structure the toolkit to differentiate between (a) and (b) as follows:
- create the state, then perform only the state things that should happen for (a)
- serialize the state and shut down the sandbox
- deserialize and recreate the state, then perform only the things that should happen for (b)

That would solve this issue, along with a number of other issues that we often encounter because of the fact that states are created twice. If we have that approach in place, whether (a) or (b) happened would be more explicit, instead of us needing all kinds of checks to try and infer which one happened in different places all over the interaction machine and each app. Obviously, that approach will be more work.