Booking system

Before refactoring

Initially booking system was not consistent. There was no single source of truth for state transition conditions and it was implemented by logic in entry files (e.g. signup.php) and lib.php functions (e.g. facetoface_user_import() and facetoface_user_signup() ). This lead to different, sometimes unexpected, results of how states were changed.

Implementation and adjusting of booking states

State

Booking system implements Finite State Machine (FSM) with current state and next states that can be moved with transition. All transitions are defined in function get_map() of each state subclass. Transitions are joined by "OR" rule, and there can be several transitions that lead to same state with different conditions and restrictions. All conditions/restrictions are processed with "AND" rule, so if "OR" required, then just create another transition with same state. This is very simple implementation can potentially lead to huge number of transitions for some complex rules, though it is not the case at the moment. As long as it practical it is better to keep it simple, unless we get to explosive that cannot be mitigated by special conditions.

Below is simplified example of booking FSM:

Simplified bookings state diagram

All booking states are classes that placed in mod/facetoface/classes/signup/state folder and extend abstract class mod_facetoface\signup\state\state.

If event need to be fired when user enters state, this state must implement  mod_facetoface\signup\state\interface_event interface. 

If some custom code needed to run when user enters some state, just implement function on_enter() on that state subclass. 

When state does not lead to booking, even potentially, it must implement function is_not_happening() which returns true. This is only used on signup page and we might deprecate this function in future, but so far it is needed.

Important rule to follow is to limit knowledge of any state outside of this state as much as possible. Coupling to state will make maintenance harder long-term. If state is part of the logic, try at least to group it in some class. For example see signup_helper. Class signup must not be coupled to any particular state.

Conditions/Restrictions

Conditions/Restrictions are single rules that are used in transition to define if user can go from current state to next one.

The semantical difference between conditions and restriction is that restrictions check user who performs transition - actor, not user that is subject of signup. If user is trying to signup themselves, then actor user and signup user are the same. However, when manager tries to signup someone from their team, then manager is actor and team member is signup user. This is small but important difference, which can easily lead to human error during development. To prevent it restrictions were separated from conditions. Otherwise they are similar.

Condition must extend mod_facetoface\signup\condition\condition and be located at mod/facetoface/classes/signup/condition folder. Restrictions must extend mod_facetoface\signup\restriction\restriction and be located at mod/facetoface/classes/signup/restriction folder.

Implement few simple simple over one complex Conditions and Restrictions. Prefer quantity over complexity. 

State "not_set"

This state is not supposed to be target state because it is "no state". It is similar to NULL in DB (which is no state, or state is not defined), unlike 0 (which is value).

when target state is not_set it means that FSM cannot resolve it and developer  responsibility is to write logic that will decide what state it should be (not FSM). 

For example, not_set is used as starting point for signup process. If there is no way to switch state from not_set with provided mapping, it means that seminar signup is not possible for user and error should be returned.

Another example, if not_set is passed to taking attendance, then it logic will depend on current state. For any attendance state it should revert to booking, while for any other (non attendance) it should leave it as is.

This means that state::can_switch() will return false, and state::switch_to() will fail when they passed not_set as desired state.